Repository: kubernetes-sigs/cloud-provider-azure
Pull request #445 (2020-12-22 06:03:21)

Support change the LB selection mode on the existing services
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://git.k8s.io/community/contributors/devel/release.md#issue-kind-label
3. Ensure you have added or ran the appropriate tests for your PR: https://git.k8s.io/community/contributors/devel/testing.md
4. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
5. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
6. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
> Uncomment only one ` /kind <>` line, hit enter to put that in a new line, and remove leading whitespaces from that line:
>
> /kind api-change
> /kind bug
> /kind cleanup
> /kind design
> /kind documentation
> /kind failing-test
> /kind feature
> /kind flake
/kind bug

**What this PR does / why we need it**:
Support change the LB selection mode annotation on the existing services.
Mirror: https://github.com/kubernetes/kubernetes/pull/96937.

**Which issue(s) this PR fixes**:
<!-- 
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes: https://github.com/kubernetes-sigs/cloud-provider-azure/issues/416
Ref: https://github.com/kubernetes-sigs/cloud-provider-azure/issues/427

**Special notes for your reviewer**:


**Release note**:
```
Support change the LB selection mode on the existing services
```


k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/cloud-provider-azure/pull/445#" title="Author self-approved">nilo19</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fcloud-provider-azure).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/cloud-provider-azure/blob/master/OWNERS)~~ [nilo19]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
k8s-ci-robot: @nilo19: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-cloud-provider-azure-check | 4d68fccc57fb2cf9758b2c3909cbe60e1183916a | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cloud-provider-azure/445/pull-cloud-provider-azure-check/1341263040087068672) | `/test pull-cloud-provider-azure-check`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=cloud-provider-azure&pr=445). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Anilo19). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/cloud-provider-azure/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->

Patch:
diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go
index 84d2d1da2..8204c756a 100644
--- a/pkg/provider/azure_loadbalancer.go
+++ b/pkg/provider/azure_loadbalancer.go
@@ -49,15 +49,15 @@ const (
 	// to specify what subnet it is exposed on
 	ServiceAnnotationLoadBalancerInternalSubnet = "service.beta.kubernetes.io/azure-load-balancer-internal-subnet"
 
-	// ServiceAnnotationLoadBalancerMode is the annotation used on the service to specify the
-	// Azure load balancer selection based on availability sets
-	// There are currently three possible load balancer selection modes :
+	// ServiceAnnotationLoadBalancerMode is the annotation used on the service to specify
+	// which load balancer should be associated with the service. This is valid when using the basic
+	// load balancer or turn on the multiple standard load balancers mode, or it would be ignored.
 	// 1. Default mode - service has no annotation ("service.beta.kubernetes.io/azure-load-balancer-mode")
-	//	  In this case the Loadbalancer of the primary Availability set is selected
-	// 2. "__auto__" mode - service is annotated with __auto__ value, this when loadbalancer from any availability set
+	//	  In this case the Loadbalancer of the primary VMSS/VMAS is selected.
+	// 2. "__auto__" mode - service is annotated with __auto__ value, this when loadbalancer from any VMSS/VMAS
 	//    is selected which has the minimum rules associated with it.
-	// 3. "as1,as2" mode - this is when the load balancer from the specified availability sets is selected that has the
-	//    minimum rules associated with it.
+	// 3. "name" mode - this is when the load balancer from the specified VMSS/VMAS that has the
+	//    minimum rules associated with it is selected.
 	ServiceAnnotationLoadBalancerMode = "service.beta.kubernetes.io/azure-load-balancer-mode"
 
 	// ServiceAnnotationLoadBalancerAutoModeValue is the annotation used on the service to specify the
@@ -189,7 +189,7 @@ func (az *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, ser
 		return nil, err
 	}
 
-	lbStatus, err := az.getServiceLoadBalancerStatus(service, lb)
+	lbStatus, _, err := az.getServiceLoadBalancerStatus(service, lb)
 	if err != nil {
 		klog.Errorf("getServiceLoadBalancerStatus(%s) failed: %v", serviceName, err)
 		return nil, err
@@ -346,6 +346,125 @@ func (az *Cloud) cleanBackendpoolForPrimarySLB(primarySLB *network.LoadBalancer,
 	return primarySLB, nil
 }
 
+// shouldChangeLoadBalancer determines if the load balancer of the service should be switched to another one
+// according to the mode annotation on the service. This could be happened when the LB selection mode of an
+// existing service is changed to another VMSS/VMAS.
+func (az *Cloud) shouldChangeLoadBalancer(service *v1.Service, currLBName, clusterName string) bool {
+	hasMode, isAuto, vmSetName := az.getServiceLoadBalancerMode(service)
+	// if no mode is given or the mode is `__auto__`, the current LB should be kept
+	if !hasMode || isAuto {
+		return false
+	}
+	// if using the single standard load balancer, the current LB should be kept
+	useSingleSLB := az.useStandardLoadBalancer() && !az.EnableMultipleStandardLoadBalancers
+	if useSingleSLB {
+		return false
+	}
+	// if the current LB is what we want, keep it
+	lbName := strings.TrimSuffix(currLBName, InternalLoadBalancerNameSuffix)
+	if strings.EqualFold(lbName, vmSetName) {
+		return false
+	}
+	if strings.EqualFold(vmSetName, az.VMSet.GetPrimaryVMSetName()) && strings.EqualFold(clusterName, lbName) {
+		return false
+	}
+	// if the VMSS/VMAS of the current LB is different from the mode, change the LB
+	// to another one
+	klog.V(2).Infof("shouldChangeLoadBalancer(%s, %s, %s): change the LB to another one", service.Name, currLBName, clusterName)
+	return true
+}
+
+func (az *Cloud) removeFrontendIPConfigurationFromLoadBalancer(lb *network.LoadBalancer, fip *network.FrontendIPConfiguration, clusterName string, service *v1.Service) error {
+	if lb == nil || lb.LoadBalancerPropertiesFormat == nil || lb.FrontendIPConfigurations == nil {
+		return nil
+	}
+	fipConfigs := *lb.FrontendIPConfigurations
+	for i, fipConfig := range fipConfigs {
+		if strings.EqualFold(to.String(fipConfig.Name), to.String(fip.Name)) {
+			fipConfigs = append(fipConfigs[:i], fipConfigs[i+1:]...)
+			break
+		}
+	}
+	lb.FrontendIPConfigurations = &fipConfigs
+
+	// also remove the corresponding rules/probes
+	if lb.LoadBalancingRules != nil {
+		lbRules := *lb.LoadBalancingRules
+		for i := len(lbRules) - 1; i >= 0; i-- {
+			if strings.Contains(to.String(lbRules[i].Name), to.String(fip.Name)) {
+				lbRules = append(lbRules[:i], lbRules[i+1:]...)
+			}
+		}
+		lb.LoadBalancingRules = &lbRules
+	}
+	if lb.Probes != nil {
+		lbProbes := *lb.Probes
+		for i := len(lbProbes) - 1; i >= 0; i-- {
+			if strings.Contains(to.String(lbProbes[i].Name), to.String(fip.Name)) {
+				lbProbes = append(lbProbes[:i], lbProbes[i+1:]...)
+			}
+		}
+		lb.Probes = &lbProbes
+	}
+
+	if len(fipConfigs) == 0 {
+		klog.V(2).Infof("removeFrontendIPConfigurationFromLoadBalancer(%s, %s, %s, %s): deleting load balancer because there is no remaining frontend IP configurations", to.String(lb.Name), to.String(fip.Name), clusterName, service.Name)
+		err := az.cleanOrphanedLoadBalancer(lb, service, clusterName)
+		if err != nil {
+			klog.Errorf("removeFrontendIPConfigurationFromLoadBalancer(%s, %s, %s, %s): failed to cleanupOrphanedLoadBalancer: %v", to.String(lb.Name), to.String(fip.Name), clusterName, service.Name, err)
+			return err
+		}
+	} else {
+		klog.V(2).Infof("removeFrontendIPConfigurationFromLoadBalancer(%s, %s, %s, %s): updating the load balancer", to.String(lb.Name), to.String(fip.Name), clusterName, service.Name)
+		err := az.CreateOrUpdateLB(service, *lb)
+		if err != nil {
+			klog.Errorf("removeFrontendIPConfigurationFromLoadBalancer(%s, %s, %s, %s): failed to CreateOrUpdateLB: %v", to.String(lb.Name), to.String(fip.Name), clusterName, service.Name, err)
+			return err
+		}
+		_ = az.lbCache.Delete(to.String(lb.Name))
+	}
+	return nil
+}
+
+func (az *Cloud) cleanOrphanedLoadBalancer(lb *network.LoadBalancer, service *v1.Service, clusterName string) error {
+	lbName := to.String(lb.Name)
+	serviceName := getServiceName(service)
+	isBackendPoolPreConfigured := az.isBackendPoolPreConfigured(service)
+	lbResourceGroup := az.getLoadBalancerResourceGroup()
+	lbBackendPoolName := getBackendPoolName(clusterName, service)
+	lbBackendPoolID := az.getBackendPoolID(lbName, lbResourceGroup, lbBackendPoolName)
+	if isBackendPoolPreConfigured {
+		klog.V(2).Infof("cleanOrphanedLoadBalancer(%s, %s, %s): ignore cleanup of dirty lb because the lb is pre-configured", lbName, serviceName, clusterName)
+	} else {
+		// When FrontendIPConfigurations is empty, we need to delete the Azure load balancer resource itself,
+		// because an Azure load balancer cannot have an empty FrontendIPConfigurations collection
+		klog.V(2).Infof("cleanOrphanedLoadBalancer(%s, %s, %s): deleting the LB since there are no remaining frontendIPConfigurations", lbName, serviceName, clusterName)
+
+		// Remove backend pools from vmSets. This is required for virtual machine scale sets before removing the LB.
+		vmSetName := az.mapLoadBalancerNameToVMSet(lbName, clusterName)
+		if _, ok := az.VMSet.(*availabilitySet); ok {
+			// do nothing for availability set
+			lb.BackendAddressPools = nil
+		}
+		err := az.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, lb.BackendAddressPools)
+		if err != nil {
+			klog.Errorf("cleanOrphanedLoadBalancer(%s, %s, %s): failed to EnsureBackendPoolDeleted: %v", lbName, serviceName, clusterName, err)
+			return err
+		}
+		klog.V(10).Infof("cleanOrphanedLoadBalancer(%s, %s, %s): EnsureBackendPoolDeleted finished", lbName, serviceName, clusterName)
+
+		// Remove the LB.
+		klog.V(10).Infof("cleanOrphanedLoadBalancer(%s, %s, %s): az.DeleteLB starts", lbName, serviceName, clusterName)
+		err = az.DeleteLB(service, lbName)
+		if err != nil {
+			klog.V(2).Infof("cleanOrphanedLoadBalancer(%s, %s, %s): failed to DeleteLB: %v", lbName, serviceName, clusterName, err)
+			return err
+		}
+		klog.V(10).Infof("cleanOrphanedLoadBalancer(%s, %s, %s): az.DeleteLB finished", lbName, serviceName, clusterName)
+	}
+	return nil
+}
+
 // getServiceLoadBalancer gets the loadbalancer for the service if it already exists.
 // If wantLb is TRUE then -it selects a new load balancer.
 // In case the selected load balancer does not exist it returns network.LoadBalancer struct
@@ -379,7 +498,7 @@ func (az *Cloud) getServiceLoadBalancer(service *v1.Service, clusterName string,
 		if isInternalLoadBalancer(&existingLB) != isInternal {
 			continue
 		}
-		status, err = az.getServiceLoadBalancerStatus(service, &existingLB)
+		status, fipConfig, err := az.getServiceLoadBalancerStatus(service, &existingLB)
 		if err != nil {
 			return nil, nil, false, err
 		}
@@ -388,18 +507,23 @@ func (az *Cloud) getServiceLoadBalancer(service *v1.Service, clusterName string,
 			continue
 		}
 
-		return &existingLB, status, true, nil
-	}
+		// select another load balancer instead of returning
+		// the current one if the change is needed
+		if wantLb && az.shouldChangeLoadBalancer(service, to.String(existingLB.Name), clusterName) {
+			if err := az.removeFrontendIPConfigurationFromLoadBalancer(&existingLB, fipConfig, clusterName, service); err != nil {
+				klog.Errorf("getServiceLoadBalancer(%s, %s, %v): failed to remove frontend IP configuration from load balancer: %v", service.Name, clusterName, wantLb, err)
+				return nil, nil, false, err
+			}
+			break
+		}
 
-	hasMode, _, _ := getServiceLoadBalancerMode(service)
-	useSingleSLB := az.useStandardLoadBalancer() && !az.EnableMultipleStandardLoadBalancers
-	if useSingleSLB && hasMode {
-		klog.Warningf("single standard load balancer doesn't work with annotation %q, would ignore it", ServiceAnnotationLoadBalancerMode)
+		return &existingLB, status, true, nil
 	}
 
 	// Service does not have a load balancer, select one.
 	// Single standard load balancer doesn't need this because
 	// all backends nodes should be added to same LB.
+	useSingleSLB := az.useStandardLoadBalancer() && !az.EnableMultipleStandardLoadBalancers
 	if wantLb && !useSingleSLB {
 		// select new load balancer for service
 		selectedLB, exists, err := az.selectLoadBalancer(clusterName, service, &existingLBs, nodes)
@@ -447,8 +571,8 @@ func (az *Cloud) selectLoadBalancer(clusterName string, service *v1.Service, exi
 		mapExistingLBs[*lb.Name] = lb
 	}
 	selectedLBRuleCount := math.MaxInt32
-	for _, currASName := range *vmSetNames {
-		currLBName := az.getAzureLoadBalancerName(clusterName, currASName, isInternal)
+	for _, currVMSetName := range *vmSetNames {
+		currLBName := az.getAzureLoadBalancerName(clusterName, currVMSetName, isInternal)
 		lb, exists := mapExistingLBs[currLBName]
 		if !exists {
 			// select this LB as this is a new LB and will have minimum rules
@@ -495,21 +619,21 @@ func (az *Cloud) selectLoadBalancer(clusterName string, service *v1.Service, exi
 	return selectedLB, existsLb, nil
 }
 
-func (az *Cloud) getServiceLoadBalancerStatus(service *v1.Service, lb *network.LoadBalancer) (status *v1.LoadBalancerStatus, err error) {
+func (az *Cloud) getServiceLoadBalancerStatus(service *v1.Service, lb *network.LoadBalancer) (status *v1.LoadBalancerStatus, fipConfig *network.FrontendIPConfiguration, err error) {
 	if lb == nil {
 		klog.V(10).Info("getServiceLoadBalancerStatus: lb is nil")
-		return nil, nil
+		return nil, nil, nil
 	}
 	if lb.FrontendIPConfigurations == nil || *lb.FrontendIPConfigurations == nil {
 		klog.V(10).Info("getServiceLoadBalancerStatus: lb.FrontendIPConfigurations is nil")
-		return nil, nil
+		return nil, nil, nil
 	}
 	isInternal := requiresInternalLoadBalancer(service)
 	serviceName := getServiceName(service)
 	for _, ipConfiguration := range *lb.FrontendIPConfigurations {
 		owns, isPrimaryService, err := az.serviceOwnsFrontendIP(ipConfiguration, service)
 		if err != nil {
-			return nil, fmt.Errorf("get(%s): lb(%s) - failed to filter frontend IP configs with error: %v", serviceName, to.String(lb.Name), err)
+			return nil, nil, fmt.Errorf("get(%s): lb(%s) - failed to filter frontend IP configs with error: %v", serviceName, to.String(lb.Name), err)
 		}
 		if owns {
 			klog.V(2).Infof("get(%s): lb(%s) - found frontend IP config, primary service: %v", serviceName, to.String(lb.Name), isPrimaryService)
@@ -519,19 +643,19 @@ func (az *Cloud) getServiceLoadBalancerStatus(service *v1.Service, lb *network.L
 				lbIP = ipConfiguration.PrivateIPAddress
 			} else {
 				if ipConfiguration.PublicIPAddress == nil {
-					return nil, fmt.Errorf("get(%s): lb(%s) - failed to get LB PublicIPAddress is Nil", serviceName, *lb.Name)
+					return nil, nil, fmt.Errorf("get(%s): lb(%s) - failed to get LB PublicIPAddress is Nil", serviceName, *lb.Name)
 				}
 				pipID := ipConfiguration.PublicIPAddress.ID
 				if pipID == nil {
-					return nil, fmt.Errorf("get(%s): lb(%s) - failed to get LB PublicIPAddress ID is Nil", serviceName, *lb.Name)
+					return nil, nil, fmt.Errorf("get(%s): lb(%s) - failed to get LB PublicIPAddress ID is Nil", serviceName, *lb.Name)
 				}
 				pipName, err := getLastSegment(*pipID, "/")
 				if err != nil {
-					return nil, fmt.Errorf("get(%s): lb(%s) - failed to get LB PublicIPAddress Name from ID(%s)", serviceName, *lb.Name, *pipID)
+					return nil, nil, fmt.Errorf("get(%s): lb(%s) - failed to get LB PublicIPAddress Name from ID(%s)", serviceName, *lb.Name, *pipID)
 				}
 				pip, existsPip, err := az.getPublicIPAddress(az.getPublicIPAddressResourceGroup(service), pipName)
 				if err != nil {
-					return nil, err
+					return nil, nil, err
 				}
 				if existsPip {
 					lbIP = pip.IPAddress
@@ -539,11 +663,11 @@ func (az *Cloud) getServiceLoadBalancerStatus(service *v1.Service, lb *network.L
 			}
 
 			klog.V(2).Infof("getServiceLoadBalancerStatus gets ingress IP %q from frontendIPConfiguration %q for service %q", to.String(lbIP), to.String(ipConfiguration.Name), serviceName)
-			return &v1.LoadBalancerStatus{Ingress: []v1.LoadBalancerIngress{{IP: to.String(lbIP)}}}, nil
+			return &v1.LoadBalancerStatus{Ingress: []v1.LoadBalancerIngress{{IP: to.String(lbIP)}}}, &ipConfiguration, nil
 		}
 	}
 
-	return nil, nil
+	return nil, nil, nil
 }
 
 func (az *Cloud) determinePublicIPName(clusterName string, service *v1.Service) (string, bool, error) {
@@ -1128,6 +1252,10 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
 						if err != nil {
 							return nil, err
 						}
+						if nodeName == "" {
+							// VM may under deletion
+							continue
+						}
 						// If a node is not supposed to be included in the LB, it
 						// would not be in the `nodes` slice. We need to check the nodes that
 						// have been added to the LB's backendpool, find the unwanted ones and
@@ -1422,35 +1550,10 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
 	// If it is not exist, and no change to that, we don't CreateOrUpdate LB
 	if dirtyLb {
 		if lb.FrontendIPConfigurations == nil || len(*lb.FrontendIPConfigurations) == 0 {
-			if isBackendPoolPreConfigured {
-				klog.V(2).Infof("reconcileLoadBalancer for service(%s): lb(%s) - ignore cleanup of dirty lb because the lb is pre-configured", serviceName, lbName)
-			} else {
-				// When FrontendIPConfigurations is empty, we need to delete the Azure load balancer resource itself,
-				// because an Azure load balancer cannot have an empty FrontendIPConfigurations collection
-				klog.V(2).Infof("reconcileLoadBalancer for service(%s): lb(%s) - deleting; no remaining frontendIPConfigurations", serviceName, lbName)
-
-				// Remove backend pools from vmSets. This is required for virtual machine scale sets before removing the LB.
-				vmSetName := az.mapLoadBalancerNameToVMSet(lbName, clusterName)
-				klog.V(10).Infof("EnsureBackendPoolDeleted(%s,%s) for service %s: start", lbBackendPoolID, vmSetName, serviceName)
-				if _, ok := az.VMSet.(*availabilitySet); ok {
-					// do nothing for availability set
-					lb.BackendAddressPools = nil
-				}
-				err := az.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, lb.BackendAddressPools)
-				if err != nil {
-					klog.Errorf("EnsureBackendPoolDeleted(%s) for service %s failed: %v", lbBackendPoolID, serviceName, err)
-					return nil, err
-				}
-				klog.V(10).Infof("EnsureBackendPoolDeleted(%s) for service %s: end", lbBackendPoolID, serviceName)
-
-				// Remove the LB.
-				klog.V(10).Infof("reconcileLoadBalancer: az.DeleteLB(%q): start", lbName)
-				err = az.DeleteLB(service, lbName)
-				if err != nil {
-					klog.V(2).Infof("reconcileLoadBalancer for service(%s) abort backoff: lb(%s) - deleting; no remaining frontendIPConfigurations", serviceName, lbName)
-					return nil, err
-				}
-				klog.V(10).Infof("az.DeleteLB(%q): end", lbName)
+			err := az.cleanOrphanedLoadBalancer(lb, service, clusterName)
+			if err != nil {
+				klog.V(2).Infof("reconcileLoadBalancer for service(%s): lb(%s) - failed to cleanOrphanedLoadBalancer: %v", serviceName, lbName)
+				return nil, err
 			}
 		} else {
 			klog.V(2).Infof("reconcileLoadBalancer: reconcileLoadBalancer for service(%s): lb(%s) - updating", serviceName, lbName)
@@ -2487,25 +2590,16 @@ func subnet(service *v1.Service) *string {
 // getServiceLoadBalancerMode parses the mode value.
 // if the value is __auto__ it returns isAuto = TRUE.
 // if anything else it returns the unique VM set names after trimming spaces.
-func getServiceLoadBalancerMode(service *v1.Service) (hasMode bool, isAuto bool, vmSetNames []string) {
+func (az *Cloud) getServiceLoadBalancerMode(service *v1.Service) (bool, bool, string) {
 	mode, hasMode := service.Annotations[ServiceAnnotationLoadBalancerMode]
-	mode = strings.TrimSpace(mode)
-	isAuto = strings.EqualFold(mode, ServiceAnnotationLoadBalancerAutoModeValue)
-	if !isAuto {
-		// Break up list of "AS1,AS2"
-		vmSetParsedList := strings.Split(mode, ",")
-
-		// Trim the VM set names and remove duplicates
-		//  e.g. {"AS1"," AS2", "AS3", "AS3"} => {"AS1", "AS2", "AS3"}
-		vmSetNameSet := sets.NewString()
-		for _, v := range vmSetParsedList {
-			vmSetNameSet.Insert(strings.TrimSpace(v))
-		}
-
-		vmSetNames = vmSetNameSet.List()
+	useSingleSLB := az.useStandardLoadBalancer() && !az.EnableMultipleStandardLoadBalancers
+	if useSingleSLB && hasMode {
+		klog.Warningf("single standard load balancer doesn't work with annotation %q, would ignore it", ServiceAnnotationLoadBalancerMode)
 	}
+	mode = strings.TrimSpace(mode)
+	isAuto := strings.EqualFold(mode, ServiceAnnotationLoadBalancerAutoModeValue)
 
-	return hasMode, isAuto, vmSetNames
+	return hasMode, isAuto, mode
 }
 
 func useSharedSecurityRule(service *v1.Service) bool {
diff --git a/pkg/provider/azure_standard.go b/pkg/provider/azure_standard.go
index 14b3a1522..922f72101 100644
--- a/pkg/provider/azure_standard.go
+++ b/pkg/provider/azure_standard.go
@@ -22,7 +22,6 @@ import (
 	"fmt"
 	"hash/crc32"
 	"regexp"
-	"sort"
 	"strconv"
 	"strings"
 
@@ -664,7 +663,7 @@ func (as *availabilitySet) getAgentPoolAvailabilitySets(nodes []*v1.Node) (agent
 // for loadbalancer exists then returns the eligible VMSet. The mode selection
 // annotation would be ignored when using one SLB per cluster.
 func (as *availabilitySet) GetVMSetNames(service *v1.Service, nodes []*v1.Node) (availabilitySetNames *[]string, err error) {
-	hasMode, isAuto, serviceAvailabilitySetNames := getServiceLoadBalancerMode(service)
+	hasMode, isAuto, serviceAvailabilitySetName := as.getServiceLoadBalancerMode(service)
 	useSingleSLB := as.useStandardLoadBalancer() && !as.EnableMultipleStandardLoadBalancers
 	if !hasMode || useSingleSLB {
 		// no mode specified in service annotation or use single SLB mode
@@ -681,28 +680,19 @@ func (as *availabilitySet) GetVMSetNames(service *v1.Service, nodes []*v1.Node)
 		klog.Errorf("as.GetVMSetNames - No availability sets found for nodes in the cluster, node count(%d)", len(nodes))
 		return nil, fmt.Errorf("no availability sets found for nodes, node count(%d)", len(nodes))
 	}
-	// sort the list to have deterministic selection
-	sort.Strings(*availabilitySetNames)
 	if !isAuto {
-		if len(serviceAvailabilitySetNames) == 0 {
-			return nil, fmt.Errorf("service annotation for LoadBalancerMode is empty, it should have __auto__ or availability sets value")
-		}
-		// validate availability set exists
-		var found bool
-		for sasx := range serviceAvailabilitySetNames {
-			for asx := range *availabilitySetNames {
-				if strings.EqualFold((*availabilitySetNames)[asx], serviceAvailabilitySetNames[sasx]) {
-					found = true
-					serviceAvailabilitySetNames[sasx] = (*availabilitySetNames)[asx]
-					break
-				}
-			}
-			if !found {
-				klog.Errorf("as.GetVMSetNames - Availability set (%s) in service annotation not found", serviceAvailabilitySetNames[sasx])
-				return nil, fmt.Errorf("availability set (%s) - not found", serviceAvailabilitySetNames[sasx])
+		found := false
+		for asx := range *availabilitySetNames {
+			if strings.EqualFold((*availabilitySetNames)[asx], serviceAvailabilitySetName) {
+				found = true
+				break
 			}
 		}
-		availabilitySetNames = &serviceAvailabilitySetNames
+		if !found {
+			klog.Errorf("as.GetVMSetNames - Availability set (%s) in service annotation not found", serviceAvailabilitySetName)
+			return nil, fmt.Errorf("availability set (%s) - not found", serviceAvailabilitySetName)
+		}
+		return &[]string{serviceAvailabilitySetName}, nil
 	}
 
 	return availabilitySetNames, nil
diff --git a/pkg/provider/azure_vmss.go b/pkg/provider/azure_vmss.go
index 489cb2864..352214b81 100644
--- a/pkg/provider/azure_vmss.go
+++ b/pkg/provider/azure_vmss.go
@@ -20,7 +20,6 @@ import (
 	"errors"
 	"fmt"
 	"regexp"
-	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -735,12 +734,11 @@ func (ss *scaleSet) getAgentPoolScaleSets(nodes []*v1.Node) (*[]string, error) {
 	return agentPoolScaleSets, nil
 }
 
-// GetVMSetNames selects all possible availability sets or scale sets
-// (depending vmType configured) for service load balancer. If the service has
+// GetVMSetNames selects all possible scale sets for service load balancer. If the service has
 // no loadbalancer mode annotation returns the primary VMSet. If service annotation
 // for loadbalancer exists then return the eligible VMSet.
 func (ss *scaleSet) GetVMSetNames(service *v1.Service, nodes []*v1.Node) (*[]string, error) {
-	hasMode, isAuto, serviceVMSetNames := getServiceLoadBalancerMode(service)
+	hasMode, isAuto, serviceVMSetName := ss.getServiceLoadBalancerMode(service)
 	useSingleSLB := ss.useStandardLoadBalancer() && !ss.EnableMultipleStandardLoadBalancers
 	if !hasMode || useSingleSLB {
 		// no mode specified in service annotation or use single SLB mode
@@ -759,29 +757,20 @@ func (ss *scaleSet) GetVMSetNames(service *v1.Service, nodes []*v1.Node) (*[]str
 		return nil, fmt.Errorf("no scale sets found for nodes, node count(%d)", len(nodes))
 	}
 
-	// sort the list to have deterministic selection
-	sort.Strings(*scaleSetNames)
-
 	if !isAuto {
-		if len(serviceVMSetNames) == 0 {
-			return nil, fmt.Errorf("service annotation for LoadBalancerMode is empty, it should have __auto__ or availability sets value")
-		}
-		// validate scale set exists
-		var found bool
-		for sasx := range serviceVMSetNames {
-			for asx := range *scaleSetNames {
-				if strings.EqualFold((*scaleSetNames)[asx], serviceVMSetNames[sasx]) {
-					found = true
-					serviceVMSetNames[sasx] = (*scaleSetNames)[asx]
-					break
-				}
-			}
-			if !found {
-				klog.Errorf("ss.GetVMSetNames - scale set (%s) in service annotation not found", serviceVMSetNames[sasx])
-				return nil, fmt.Errorf("scale set (%s) - not found", serviceVMSetNames[sasx])
+		found := false
+		for asx := range *scaleSetNames {
+			if strings.EqualFold((*scaleSetNames)[asx], serviceVMSetName) {
+				found = true
+				serviceVMSetName = (*scaleSetNames)[asx]
+				break
 			}
 		}
-		scaleSetNames = &serviceVMSetNames
+		if !found {
+			klog.Errorf("ss.GetVMSetNames - scale set (%s) in service annotation not found", serviceVMSetName)
+			return nil, fmt.Errorf("scale set (%s) - not found", serviceVMSetName)
+		}
+		return &[]string{serviceVMSetName}, nil
 	}
 
 	return scaleSetNames, nil


Test patch:
diff --git a/pkg/provider/azure_loadbalancer_test.go b/pkg/provider/azure_loadbalancer_test.go
index 028c04fa1..94e232886 100644
--- a/pkg/provider/azure_loadbalancer_test.go
+++ b/pkg/provider/azure_loadbalancer_test.go
@@ -2260,7 +2260,7 @@ func TestGetServiceLoadBalancerStatus(t *testing.T) {
 	}
 
 	for i, test := range testCases {
-		status, err := az.getServiceLoadBalancerStatus(test.service, test.lb)
+		status, _, err := az.getServiceLoadBalancerStatus(test.service, test.lb)
 		assert.Equal(t, test.expectedStatus, status, "TestCase[%d]: %s", i, test.desc)
 		assert.Equal(t, test.expectedError, err != nil, "TestCase[%d]: %s", i, test.desc)
 	}
@@ -3594,3 +3594,60 @@ func TestEnsurePIPTagged(t *testing.T) {
 		assert.Equal(t, expectedPIP, pip)
 	})
 }
+
+func TestShouldChangeLoadBalancer(t *testing.T) {
+	t.Run("shouldChangeLoadBalancer should return true if the mode is different from the current vm set", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		cloud := GetTestCloud(ctrl)
+		cloud.LoadBalancerSku = loadBalancerSkuBasic
+		annotations := map[string]string{
+			ServiceAnnotationLoadBalancerMode: "as2",
+		}
+		service := getTestService("service1", v1.ProtocolTCP, annotations, false, 80)
+		res := cloud.shouldChangeLoadBalancer(&service, "as1", "testCluster")
+		assert.True(t, res)
+	})
+}
+
+func TestRemoveFrontendIPConfigurationFromLoadBalancerDelete(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	t.Run("removeFrontendIPConfigurationFromLoadBalancer should remove the unwanted frontend IP configuration and delete the orphaned LB", func(t *testing.T) {
+		fip := &network.FrontendIPConfiguration{Name: to.StringPtr("testCluster")}
+		service := getTestService("svc1", v1.ProtocolTCP, nil, false, 80)
+		lb := getTestLoadBalancer(to.StringPtr("lb"), to.StringPtr("rg"), to.StringPtr("testCluster"), to.StringPtr("testCluster"), service, "standard")
+		bid := "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/k8s-agentpool1-00000000-nic-0/ipConfigurations/ipconfig1"
+		lb.BackendAddressPools = &[]network.BackendAddressPool{
+			{
+				Name: to.StringPtr("testCluster"),
+				BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{
+					BackendIPConfigurations: &[]network.InterfaceIPConfiguration{
+						{ID: to.StringPtr(bid)},
+					},
+				},
+			},
+		}
+		cloud := GetTestCloud(ctrl)
+		mockLBClient := cloud.LoadBalancerClient.(*mockloadbalancerclient.MockInterface)
+		mockLBClient.EXPECT().Delete(gomock.Any(), "rg", "lb").Return(nil)
+		err := cloud.removeFrontendIPConfigurationFromLoadBalancer(&lb, fip, "testCluster", &service)
+		assert.NoError(t, err)
+	})
+}
+
+func TestRemoveFrontendIPConfigurationFromLoadBalancerUpdate(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	t.Run("removeFrontendIPConfigurationFromLoadBalancer should remove the unwanted frontend IP configuration and update the LB if there are remaining frontend IP configurations", func(t *testing.T) {
+		fip := &network.FrontendIPConfiguration{Name: to.StringPtr("testCluster")}
+		service := getTestService("svc1", v1.ProtocolTCP, nil, false, 80)
+		lb := getTestLoadBalancer(to.StringPtr("lb"), to.StringPtr("rg"), to.StringPtr("testCluster"), to.StringPtr("testCluster"), service, "standard")
+		*lb.FrontendIPConfigurations = append(*lb.FrontendIPConfigurations, network.FrontendIPConfiguration{Name: to.StringPtr("fip1")})
+		cloud := GetTestCloud(ctrl)
+		mockLBClient := cloud.LoadBalancerClient.(*mockloadbalancerclient.MockInterface)
+		mockLBClient.EXPECT().CreateOrUpdate(gomock.Any(), "rg", "lb", gomock.Any(), gomock.Any()).Return(nil)
+		err := cloud.removeFrontendIPConfigurationFromLoadBalancer(&lb, fip, "testCluster", &service)
+		assert.NoError(t, err)
+	})
+}
diff --git a/pkg/provider/azure_standard_test.go b/pkg/provider/azure_standard_test.go
index 2a65e387f..ebae21294 100644
--- a/pkg/provider/azure_standard_test.go
+++ b/pkg/provider/azure_standard_test.go
@@ -1156,7 +1156,7 @@ func TestGetStandardVMSetNames(t *testing.T) {
 					},
 				},
 			},
-			expectedVMSetNames: &[]string{"myavailabilityset"},
+			expectedVMSetNames: &[]string{"myAvailabilitySet"},
 		},
 	}
 
diff --git a/pkg/provider/azure_test.go b/pkg/provider/azure_test.go
index a5d46818d..4c6c4329e 100644
--- a/pkg/provider/azure_test.go
+++ b/pkg/provider/azure_test.go
@@ -414,7 +414,6 @@ func testLoadBalancerServicesSpecifiedSelection(t *testing.T, isInternal bool) {
 	setMockEnv(az, ctrl, expectedInterfaces, expectedVirtualMachines, serviceCount)
 
 	selectedAvailabilitySetName1 := getAvailabilitySetName(az, 0, availabilitySetCount)
-	selectedAvailabilitySetName2 := getAvailabilitySetName(az, 1, availabilitySetCount)
 
 	expectedLBs := make([]network.LoadBalancer, 0)
 
@@ -427,10 +426,10 @@ func testLoadBalancerServicesSpecifiedSelection(t *testing.T, isInternal bool) {
 		} else {
 			svc = getTestService(svcName, v1.ProtocolTCP, nil, false, int32(index))
 		}
-		lbMode := fmt.Sprintf("%s,%s", selectedAvailabilitySetName1, selectedAvailabilitySetName2)
+		lbMode := selectedAvailabilitySetName1
 		setLoadBalancerModeAnnotation(&svc, lbMode)
 
-		setMockLBs(az, ctrl, &expectedLBs, "service", 2, index, isInternal)
+		setMockLBs(az, ctrl, &expectedLBs, "service", 1, index, isInternal)
 
 		lbStatus, err := az.EnsureLoadBalancer(context.TODO(), testClusterName, &svc, clusterResources.nodes)
 		if err != nil {
@@ -441,7 +440,7 @@ func testLoadBalancerServicesSpecifiedSelection(t *testing.T, isInternal bool) {
 		}
 
 		// expected is MIN(index, 2)
-		expectedNumOfLB := int(math.Min(float64(index), float64(2)))
+		expectedNumOfLB := int(math.Min(float64(index), float64(1)))
 		ctx, cancel := getContextWithCancel()
 		defer cancel()
 		result, _ := az.LoadBalancerClient.List(ctx, az.Config.ResourceGroup)
@@ -1114,7 +1113,7 @@ func TestReconcileSecurityGroupNewServiceAddsPort(t *testing.T) {
 	expectedLBs := make([]network.LoadBalancer, 0)
 	setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, false)
 	lb, _ := az.reconcileLoadBalancer(testClusterName, &svc1, clusterResources.nodes, true)
-	lbStatus, _ := az.getServiceLoadBalancerStatus(&svc1, lb)
+	lbStatus, _, _ := az.getServiceLoadBalancerStatus(&svc1, lb)
 
 	sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, &lbStatus.Ingress[0].IP, true /* wantLb */)
 	if err != nil {
@@ -1139,7 +1138,7 @@ func TestReconcileSecurityGroupNewInternalServiceAddsPort(t *testing.T) {
 	setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, true)
 
 	lb, _ := az.reconcileLoadBalancer(testClusterName, &svc1, clusterResources.nodes, true)
-	lbStatus, _ := az.getServiceLoadBalancerStatus(&svc1, lb)
+	lbStatus, _, _ := az.getServiceLoadBalancerStatus(&svc1, lb)
 	sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, &lbStatus.Ingress[0].IP, true /* wantLb */)
 	if err != nil {
 		t.Errorf("Unexpected error: %q", err)
@@ -1164,7 +1163,7 @@ func TestReconcileSecurityGroupRemoveService(t *testing.T) {
 	lb, _ := az.reconcileLoadBalancer(testClusterName, &service1, clusterResources.nodes, true)
 	_, _ = az.reconcileLoadBalancer(testClusterName, &service2, clusterResources.nodes, true)
 
-	lbStatus, _ := az.getServiceLoadBalancerStatus(&service1, lb)
+	lbStatus, _, _ := az.getServiceLoadBalancerStatus(&service1, lb)
 
 	sg := getTestSecurityGroup(az, service1, service2)
 	validateSecurityGroup(t, sg, service1, service2)
@@ -1191,7 +1190,7 @@ func TestReconcileSecurityGroupRemoveServiceRemovesPort(t *testing.T) {
 	expectedLBs := make([]network.LoadBalancer, 0)
 	setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, true)
 	lb, _ := az.reconcileLoadBalancer(testClusterName, &svc, clusterResources.nodes, true)
-	lbStatus, _ := az.getServiceLoadBalancerStatus(&svc, lb)
+	lbStatus, _, _ := az.getServiceLoadBalancerStatus(&svc, lb)
 
 	sg, err := az.reconcileSecurityGroup(testClusterName, &svcUpdated, &lbStatus.Ingress[0].IP, true /* wantLb */)
 	if err != nil {
@@ -1218,7 +1217,7 @@ func TestReconcileSecurityWithSourceRanges(t *testing.T) {
 	expectedLBs := make([]network.LoadBalancer, 0)
 	setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, false)
 	lb, _ := az.reconcileLoadBalancer(testClusterName, &svc, clusterResources.nodes, true)
-	lbStatus, _ := az.getServiceLoadBalancerStatus(&svc, lb)
+	lbStatus, _, _ := az.getServiceLoadBalancerStatus(&svc, lb)
 
 	sg, err := az.reconcileSecurityGroup(testClusterName, &svc, &lbStatus.Ingress[0].IP, true /* wantLb */)
 	if err != nil {
@@ -1254,7 +1253,7 @@ func TestReconcileSecurityGroupEtagMismatch(t *testing.T) {
 	setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, true)
 
 	lb, _ := az.reconcileLoadBalancer(testClusterName, &svc1, clusterResources.nodes, true)
-	lbStatus, _ := az.getServiceLoadBalancerStatus(&svc1, lb)
+	lbStatus, _, _ := az.getServiceLoadBalancerStatus(&svc1, lb)
 
 	newSG, err := az.reconcileSecurityGroup(testClusterName, &svc1, &lbStatus.Ingress[0].IP, true /* wantLb */)
 	assert.Nil(t, newSG)