diff --git a/pkg/cloudprovider/providers/azure/azure_loadbalancer.go b/pkg/cloudprovider/providers/azure/azure_loadbalancer.go index 6121d26b12b..741280f5821 100644 --- a/pkg/cloudprovider/providers/azure/azure_loadbalancer.go +++ b/pkg/cloudprovider/providers/azure/azure_loadbalancer.go @@ -24,7 +24,6 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/kubernetes/pkg/api/v1" serviceapi "k8s.io/kubernetes/pkg/api/v1/service" - "k8s.io/kubernetes/pkg/cloudprovider" "github.com/Azure/azure-sdk-for-go/arm/network" "github.com/Azure/go-autorest/autorest/to" @@ -63,7 +62,7 @@ func (az *Cloud) GetLoadBalancer(clusterName string, service *v1.Service) (statu } } else { // TODO: Consider also read address from lb's FrontendIPConfigurations - pipName, err := az.getPublicIPName(clusterName, service) + pipName, err := az.determinePublicIPName(clusterName, service) if err != nil { return nil, false, err } @@ -86,10 +85,10 @@ func (az *Cloud) GetLoadBalancer(clusterName string, service *v1.Service) (statu }, true, nil } -func (az *Cloud) getPublicIPName(clusterName string, service *v1.Service) (string, error) { +func (az *Cloud) determinePublicIPName(clusterName string, service *v1.Service) (string, error) { loadBalancerIP := service.Spec.LoadBalancerIP if len(loadBalancerIP) == 0 { - return fmt.Sprintf("%s-%s", clusterName, cloudprovider.GetLoadBalancerName(service)), nil + return getPublicIPName(clusterName, service), nil } az.operationPollRateLimiter.Accept() @@ -125,14 +124,6 @@ func (az *Cloud) EnsureLoadBalancer(clusterName string, service *v1.Service, nod return nil, err } - // Also clean up public ip resource, since service might be switched from public load balancer type. - if isInternal { - err = az.cleanupPublicIP(clusterName, service) - if err != nil { - return nil, err - } - } - serviceName := getServiceName(service) glog.V(5).Infof("ensure(%s): START clusterName=%q lbName=%q", serviceName, clusterName, lbName) @@ -209,7 +200,7 @@ func (az *Cloud) EnsureLoadBalancer(clusterName string, service *v1.Service, nod fipConfigurationProperties = &configProperties } else { - pipName, err := az.getPublicIPName(clusterName, service) + pipName, err := az.determinePublicIPName(clusterName, service) if err != nil { return nil, err } @@ -308,13 +299,6 @@ func (az *Cloud) EnsureLoadBalancerDeleted(clusterName string, service *v1.Servi return err } - if !isInternal { - err = az.cleanupPublicIP(clusterName, service) - if err != nil { - return err - } - } - sg, existsSg, err := az.getSecurityGroup() if err != nil { return err @@ -361,6 +345,22 @@ func (az *Cloud) cleanupLoadBalancer(clusterName string, service *v1.Service, is return err } if existsLb { + var publicIPToCleanup *string + + if !isInternalLb { + // Find public ip resource to clean up from IP configuration + lbFrontendIPConfigName := getFrontendIPConfigName(service) + for _, config := range *lb.FrontendIPConfigurations { + if strings.EqualFold(*config.Name, lbFrontendIPConfigName) { + if config.PublicIPAddress != nil { + // Only ID property is available + publicIPToCleanup = config.PublicIPAddress.ID + } + break + } + } + } + lb, lbNeedsUpdate, reconcileErr := az.reconcileLoadBalancer(lb, nil, clusterName, service, []*v1.Node{}) if reconcileErr != nil { return reconcileErr @@ -399,23 +399,23 @@ func (az *Cloud) cleanupLoadBalancer(clusterName string, service *v1.Service, is } } } - } - return nil -} - -func (az *Cloud) cleanupPublicIP(clusterName string, service *v1.Service) error { - serviceName := getServiceName(service) - - // Only delete an IP address if we created it. - if service.Spec.LoadBalancerIP == "" { - pipName, err := az.getPublicIPName(clusterName, service) - if err != nil { - return err - } - err = az.ensurePublicIPDeleted(serviceName, pipName) - if err != nil { - return err + // Public IP can be deleted after frontend ip configuration rule deleted. + if publicIPToCleanup != nil { + // Only delete an IP address if we created it, deducing by name. + if index := strings.LastIndex(*publicIPToCleanup, "/"); index != -1 { + managedPipName := getPublicIPName(clusterName, service) + pipName := (*publicIPToCleanup)[index+1:] + if strings.EqualFold(managedPipName, pipName) { + glog.V(5).Infof("Deleting public IP resource %q.", pipName) + err = az.ensurePublicIPDeleted(serviceName, pipName) + if err != nil { + return err + } + } else { + glog.V(5).Infof("Public IP resource %q found, but it does not match managed name %q, skip deleting.", pipName, managedPipName) + } + } } } diff --git a/pkg/cloudprovider/providers/azure/azure_util.go b/pkg/cloudprovider/providers/azure/azure_util.go index 6f8c4172490..9f092dcc564 100644 --- a/pkg/cloudprovider/providers/azure/azure_util.go +++ b/pkg/cloudprovider/providers/azure/azure_util.go @@ -180,6 +180,7 @@ func getPrimaryIPConfig(nic network.Interface) (*network.InterfaceIPConfiguratio // For a load balancer, all frontend ip should reference either a subnet or publicIpAddress. // Thus Azure do not allow mixed type (public and internal) load balancer. // So we'd have a separate name for internal load balancer. +// This would be the name for Azure LoadBalancer resource. func getLoadBalancerName(clusterName string, isInternal bool) string { if isInternal { return fmt.Sprintf("%s-internal", clusterName) @@ -212,6 +213,10 @@ func getRulePrefix(service *v1.Service) string { return cloudprovider.GetLoadBalancerName(service) } +func getPublicIPName(clusterName string, service *v1.Service) string { + return fmt.Sprintf("%s-%s", clusterName, cloudprovider.GetLoadBalancerName(service)) +} + func serviceOwnsRule(service *v1.Service, rule string) bool { prefix := getRulePrefix(service) return strings.HasPrefix(strings.ToUpper(rule), strings.ToUpper(prefix)) diff --git a/test/e2e/service.go b/test/e2e/service.go index f13bf2bb652..01cdcf9ef5d 100644 --- a/test/e2e/service.go +++ b/test/e2e/service.go @@ -1241,6 +1241,77 @@ var _ = framework.KubeDescribe("Services", func() { framework.CheckReachabilityFromPod(true, normalReachabilityTimeout, namespace, acceptPodName, svcIP) framework.CheckReachabilityFromPod(true, normalReachabilityTimeout, namespace, dropPodName, svcIP) }) + + It("should be able to create an internal type load balancer on Azure [Slow]", func() { + framework.SkipUnlessProviderIs("azure") + + createTimeout := framework.LoadBalancerCreateTimeoutDefault + pollInterval := framework.Poll * 10 + + serviceAnnotationLoadBalancerInternal := "service.beta.kubernetes.io/azure-load-balancer-internal" + namespace := f.Namespace.Name + serviceName := "lb-internal" + jig := framework.NewServiceTestJig(cs, serviceName) + + isInternalEndpoint := func(lbIngress *v1.LoadBalancerIngress) bool { + ingressEndpoint := framework.GetIngressPoint(lbIngress) + // Needs update for providers using hostname as endpoint. + return strings.HasPrefix(ingressEndpoint, "10.") + } + + By("creating a service with type LoadBalancer and LoadBalancerInternal annotation set to true") + svc := jig.CreateTCPServiceOrFail(namespace, func(svc *v1.Service) { + svc.Spec.Type = v1.ServiceTypeLoadBalancer + svc.ObjectMeta.Annotations = map[string]string{ + serviceAnnotationLoadBalancerInternal: "true", + } + }) + svc = jig.WaitForLoadBalancerOrFail(namespace, serviceName, createTimeout) + jig.SanityCheckService(svc, v1.ServiceTypeLoadBalancer) + lbIngress := &svc.Status.LoadBalancer.Ingress[0] + // should have an internal IP. + Expect(isInternalEndpoint(lbIngress)).To(BeTrue()) + + By("switiching to external type LoadBalancer") + svc = jig.UpdateServiceOrFail(namespace, serviceName, func(svc *v1.Service) { + svc.ObjectMeta.Annotations[serviceAnnotationLoadBalancerInternal] = "false" + }) + framework.Logf("Waiting up to %v for service %q to have an external LoadBalancer", createTimeout, serviceName) + if pollErr := wait.PollImmediate(pollInterval, createTimeout, func() (bool, error) { + svc, err := jig.Client.Core().Services(namespace).Get(serviceName, metav1.GetOptions{}) + if err != nil { + return false, err + } + lbIngress = &svc.Status.LoadBalancer.Ingress[0] + return !isInternalEndpoint(lbIngress), nil + }); pollErr != nil { + framework.Failf("Loadbalancer IP not changed to external.") + } + // should have an external IP. + jig.SanityCheckService(svc, v1.ServiceTypeLoadBalancer) + Expect(isInternalEndpoint(lbIngress)).To(BeFalse()) + + By("switiching back to interal type LoadBalancer, with static IP specified.") + internalStaticIP := "10.240.11.11" + svc = jig.UpdateServiceOrFail(namespace, serviceName, func(svc *v1.Service) { + svc.Spec.LoadBalancerIP = internalStaticIP + svc.ObjectMeta.Annotations[serviceAnnotationLoadBalancerInternal] = "true" + }) + framework.Logf("Waiting up to %v for service %q to have an internal LoadBalancer", createTimeout, serviceName) + if pollErr := wait.PollImmediate(pollInterval, createTimeout, func() (bool, error) { + svc, err := jig.Client.Core().Services(namespace).Get(serviceName, metav1.GetOptions{}) + if err != nil { + return false, err + } + lbIngress = &svc.Status.LoadBalancer.Ingress[0] + return isInternalEndpoint(lbIngress), nil + }); pollErr != nil { + framework.Failf("Loadbalancer IP not changed to internal.") + } + // should have the given static internal IP. + jig.SanityCheckService(svc, v1.ServiceTypeLoadBalancer) + Expect(framework.GetIngressPoint(lbIngress)).To(Equal(internalStaticIP)) + }) }) var _ = framework.KubeDescribe("ESIPP [Slow]", func() {