[release-1.23] feat: support changing LB backend pool type between nodeIP and nodeIP…

pkg/azureclients/loadbalancerclient/azure_loadbalancerclient.go

@@ -492,6 +492,49 @@ func (c *Client) createOrUpdateLBBackendPool(ctx context.Context, resourceGroupN
 	return nil
 }
 
+// DeleteLBBackendPool deletes a LoadBalancer backend pool by name.
+func (c *Client) DeleteLBBackendPool(ctx context.Context, resourceGroupName, loadBalancerName, backendPoolName string) *retry.Error {
+	mc := metrics.NewMetricContext("load_balancers", "delete_backend_pool", resourceGroupName, c.subscriptionID, "")
+
+	// Report errors if the client is rate limited.
+	if !c.rateLimiterWriter.TryAccept() {
+		mc.RateLimitedCount()
+		return retry.GetRateLimitError(true, "LBDeleteBackendPool")
+	}
+
+	// Report errors if the client is throttled.
+	if c.RetryAfterWriter.After(time.Now()) {
+		mc.ThrottledCount()
+		rerr := retry.GetThrottlingError("LBDeleteBackendPool", "client throttled", c.RetryAfterWriter)
+		return rerr
+	}
+
+	rerr := c.deleteLBBackendPool(ctx, resourceGroupName, loadBalancerName, backendPoolName)
+	mc.Observe(rerr)
+	if rerr != nil {
+		if rerr.IsThrottled() {
+			// Update RetryAfterReader so that no more requests would be sent until RetryAfter expires.
+			c.RetryAfterWriter = rerr.RetryAfter
+		}
+
+		return rerr
+	}
+
+	return nil
+}
+
+func (c *Client) deleteLBBackendPool(ctx context.Context, resourceGroupName, loadBalancerName, backendPoolName string) *retry.Error {
+	resourceID := armclient.GetChildResourceID(
+		c.subscriptionID,
+		resourceGroupName,
+		"Microsoft.Network/loadBalancers",
+		loadBalancerName,
+		"backendAddressPools",
+		backendPoolName,
+	)
+	return c.armClient.DeleteResource(ctx, resourceID, "")
+}
+
 func (c *Client) createOrUpdateBackendPoolResponder(resp *http.Response) (*network.BackendAddressPool, *retry.Error) {
 	result := &network.BackendAddressPool{}
 	err := autorest.Respond(

pkg/azureclients/loadbalancerclient/interface.go

@@ -50,4 +50,7 @@ type Interface interface {
 
 	// Delete deletes a LoadBalancer by name.
 	Delete(ctx context.Context, resourceGroupName string, loadBalancerName string) *retry.Error
+
+	// DeleteLBBackendPool deletes a LoadBalancer backend pool by name.
+	DeleteLBBackendPool(ctx context.Context, resourceGroupName, loadBalancerName, backendPoolName string) *retry.Error
 }

pkg/provider/azure_backoff.go

@@ -293,6 +293,34 @@ func (az *Cloud) CreateOrUpdateLBBackendPool(lbName string, backendPool network.
 	return rerr.Error()
 }
 
+func (az *Cloud) DeleteLBBackendPool(lbName, backendPoolName string) error {
+	ctx, cancel := getContextWithCancel()
+	defer cancel()
+
+	klog.V(4).Infof("DeleteLBBackendPool: deleting backend pool %s in LB %s", backendPoolName, lbName)
+	rerr := az.LoadBalancerClient.DeleteLBBackendPool(ctx, az.getLoadBalancerResourceGroup(), lbName, backendPoolName)
+	if rerr == nil {
+		// Invalidate the cache right after updating
+		_ = az.lbCache.Delete(lbName)
+		return nil
+	}
+
+	// Invalidate the cache because ETAG precondition mismatch.
+	if rerr.HTTPStatusCode == http.StatusPreconditionFailed {
+		klog.V(3).Infof("LoadBalancer cache for %s is cleanup because of http.StatusPreconditionFailed", lbName)
+		_ = az.lbCache.Delete(lbName)
+	}
+
+	retryErrorMessage := rerr.Error().Error()
+	// Invalidate the cache because another new operation has canceled the current request.
+	if strings.Contains(strings.ToLower(retryErrorMessage), consts.OperationCanceledErrorMessage) {
+		klog.V(3).Infof("LoadBalancer cache for %s is cleanup because CreateOrUpdate is canceled by another operation", lbName)
+		_ = az.lbCache.Delete(lbName)
+	}
+
+	return rerr.Error()
+}
+
 // ListManagedLBs invokes az.LoadBalancerClient.List and filter out
 // those that are not managed by cloud provider azure or not associated to a managed VMSet.
 func (az *Cloud) ListManagedLBs(service *v1.Service, nodes []*v1.Node, clusterName string) ([]network.LoadBalancer, error) {

pkg/provider/azure_backoff_test.go

@@ -621,3 +621,34 @@ func TestCreateOrUpdateLBBackendPool(t *testing.T) {
 		assert.Equal(t, tc.expectedErr, err != nil)
 	}
 }
+
+func TestDeleteLBBackendPool(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	for _, tc := range []struct {
+		description string
+		deleteErr   *retry.Error
+		expectedErr bool
+	}{
+		{
+			description: "DeleteLBBackendPool should not report an error if the api call succeeds",
+		},
+		{
+			description: "DeleteLBBackendPool should report an error if the api call fails",
+			deleteErr: &retry.Error{
+				HTTPStatusCode: http.StatusPreconditionFailed,
+				RawError:       errors.New(consts.OperationCanceledErrorMessage),
+			},
+			expectedErr: true,
+		},
+	} {
+		az := GetTestCloud(ctrl)
+		lbClient := mockloadbalancerclient.NewMockInterface(ctrl)
+		lbClient.EXPECT().DeleteLBBackendPool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.deleteErr)
+		az.LoadBalancerClient = lbClient
+
+		err := az.DeleteLBBackendPool("kubernetes", "kubernetes")
+		assert.Equal(t, tc.expectedErr, err != nil)
+	}
+}

pkg/provider/azure_loadbalancer.go

@@ -379,7 +379,7 @@ func (az *Cloud) cleanOrphanedLoadBalancer(lb *network.LoadBalancer, existingLBs
 
 // safeDeleteLoadBalancer deletes the load balancer after decoupling it from the vmSet
 func (az *Cloud) safeDeleteLoadBalancer(lb network.LoadBalancer, clusterName, vmSetName string, service *v1.Service) *retry.Error {
-	if strings.EqualFold(az.LoadBalancerBackendPoolConfigurationType, consts.LoadBalancerBackendPoolConfigurationTypeNodeIPConfiguration) {
+	if isLBBackendPoolTypeIPConfig(service, &lb, clusterName) {
 		lbBackendPoolID := az.getBackendPoolID(to.String(lb.Name), az.getLoadBalancerResourceGroup(), getBackendPoolName(clusterName, service))
 		err := az.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, lb.BackendAddressPools, true)
 		if err != nil {

pkg/provider/azure_loadbalancer_backendpool.go

@@ -137,7 +137,6 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string,
 	}
 
 	foundBackendPool := false
-	wantLb := true
 	changed := false
 	lbName := *lb.Name
 
@@ -146,11 +145,30 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string,
 	lbBackendPoolID := bc.getBackendPoolID(lbName, bc.getLoadBalancerResourceGroup(), lbBackendPoolName)
 	vmSetName := bc.mapLoadBalancerNameToVMSet(lbName, clusterName)
 
-	for _, bp := range newBackendPools {
+	for i := len(newBackendPools) - 1; i >= 0; i-- {
+		bp := newBackendPools[i]
 		if strings.EqualFold(*bp.Name, lbBackendPoolName) {
-			klog.V(10).Infof("bc.ReconcileBackendPools for service (%s)(%t): lb backendpool - found wanted backendpool. not adding anything", serviceName, wantLb)
+			klog.V(10).Infof("bc.ReconcileBackendPools for service (%s): lb backendpool - found wanted backendpool. not adding anything", serviceName)
 			foundBackendPool = true
 
+			// If the LB backend pool type is configured from nodeIP or podIP
+			// to nodeIPConfiguration, we need to decouple the VM NICs from the LB
+			// before attaching nodeIPs/podIPs to the LB backend pool.
+			if bp.BackendAddressPoolPropertiesFormat != nil &&
+				(bp.BackendIPConfigurations == nil || len(*bp.BackendIPConfigurations) == 0) {
+				// It is not allowed to change the type of the backend pool, so we delete it
+				// and create a new empty backend pool.
+				if err := bc.DeleteLBBackendPool(lbName, to.String(bp.Name)); err != nil {
+					klog.Errorf("bc.ReconcileBackendPools for service (%s): failed to DeleteLBBackendPool: %s", serviceName, err.Error())
+					return false, false, err
+				}
+				newBackendPools = append(newBackendPools[:i], newBackendPools[i+1:]...)
+				lb.BackendAddressPools = &newBackendPools
+				lb.Etag = nil
+				foundBackendPool = false
+				break
+			}
+
 			var backendIPConfigurationsToBeDeleted []network.InterfaceIPConfiguration
 			if bp.BackendAddressPoolPropertiesFormat != nil && bp.BackendIPConfigurations != nil {
 				for _, ipConf := range *bp.BackendIPConfigurations {
@@ -170,7 +188,7 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string,
 						return false, false, err
 					}
 					if shouldExcludeLoadBalancer {
-						klog.V(2).Infof("bc.ReconcileBackendPools for service (%s)(%t): lb backendpool - found unwanted node %s, decouple it from the LB %s", serviceName, wantLb, nodeName, lbName)
+						klog.V(2).Infof("bc.ReconcileBackendPools for service (%s): lb backendpool - found unwanted node %s, decouple it from the LB %s", serviceName, nodeName, lbName)
 						// construct a backendPool that only contains the IP config of the node to be deleted
 						backendIPConfigurationsToBeDeleted = append(backendIPConfigurationsToBeDeleted, network.InterfaceIPConfiguration{ID: to.StringPtr(ipConfID)})
 					}
@@ -193,7 +211,7 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string,
 			}
 			break
 		} else {
-			klog.V(10).Infof("bc.ReconcileBackendPools for service (%s)(%t): lb backendpool - found unmanaged backendpool %s", serviceName, wantLb, *bp.Name)
+			klog.V(10).Infof("bc.ReconcileBackendPools for service (%s): lb backendpool - found unmanaged backendpool %s", serviceName, *bp.Name)
 		}
 	}
 
@@ -357,27 +375,53 @@ func (bi *backendPoolTypeNodeIP) CleanupVMSetFromBackendPoolByCondition(slb *net
 
 func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, service *v1.Service, lb *network.LoadBalancer) (bool, bool, error) {
 	var newBackendPools []network.BackendAddressPool
-	var err error
 	if lb.BackendAddressPools != nil {
 		newBackendPools = *lb.BackendAddressPools
 	}
 
 	foundBackendPool := false
-	wantLb := true
 	changed := false
 	lbName := *lb.Name
 	serviceName := getServiceName(service)
 	lbBackendPoolName := getBackendPoolName(clusterName, service)
+	vmSetName := bi.mapLoadBalancerNameToVMSet(lbName, clusterName)
+	lbBackendPoolID := bi.getBackendPoolID(to.String(lb.Name), bi.getLoadBalancerResourceGroup(), getBackendPoolName(clusterName, service))
 
-	for i, bp := range newBackendPools {
+	for i := len(newBackendPools) - 1; i >= 0; i-- {
+		bp := newBackendPools[i]
 		if strings.EqualFold(*bp.Name, lbBackendPoolName) {
-			klog.V(10).Infof("bi.ReconcileBackendPools for service (%s)(%t): lb backendpool - found wanted backendpool. not adding anything", serviceName, wantLb)
+			klog.V(10).Infof("bi.ReconcileBackendPools for service (%s): found wanted backendpool. not adding anything", serviceName)
 			foundBackendPool = true
 
+			// If the LB backend pool type is configured from nodeIPConfiguration
+			// to nodeIP, we need to decouple the VM NICs from the LB
+			// before attaching nodeIPs/podIPs to the LB backend pool.
+			if bp.BackendAddressPoolPropertiesFormat != nil &&
+				bp.BackendIPConfigurations != nil &&
+				len(*bp.BackendIPConfigurations) > 0 {
+				// It is not allowed to change the type of the backend pool, so we delete it
+				// and create a new empty backend pool. Note that for IP configuration based
+				// backend pools, we must decouple it from the vmSets before deleting.
+				klog.V(2).Infof("bi.ReconcileBackendPools for service (%s): ensuring the LB is decoupled from the VMSet", serviceName)
+				if err := bi.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, lb.BackendAddressPools, true); err != nil {
+					klog.Errorf("bi.ReconcileBackendPools for service (%s): failed to EnsureBackendPoolDeleted: %s", serviceName, err.Error())
+					return false, false, err
+				}
+				if err := bi.DeleteLBBackendPool(lbName, to.String(bp.Name)); err != nil {
+					klog.Errorf("bi.ReconcileBackendPools for service (%s): failed to DeleteLBBackendPool: %s", serviceName, err.Error())
+					return false, false, err
+				}
+				newBackendPools = append(newBackendPools[:i], newBackendPools[i+1:]...)
+				lb.BackendAddressPools = &newBackendPools
+				lb.Etag = nil
+				foundBackendPool = false
+				break
+			}
+
 			var nodeIPAddressesToBeDeleted []string
 			for nodeName := range bi.excludeLoadBalancerNodes {
 				for ip := range bi.nodePrivateIPs[nodeName] {
-					klog.V(2).Infof("bi.ReconcileBackendPools for service (%s)(%t): lb backendpool - found unwanted node private IP %s, decouple it from the LB %s", serviceName, wantLb, ip, lbName)
+					klog.V(2).Infof("bi.ReconcileBackendPools for service (%s): found unwanted node private IP %s, decoupling it from the LB %s", serviceName, ip, lbName)
 					nodeIPAddressesToBeDeleted = append(nodeIPAddressesToBeDeleted, ip)
 				}
 			}
@@ -386,13 +430,13 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi
 				if updated {
 					(*lb.BackendAddressPools)[i] = bp
 					if err := bi.CreateOrUpdateLBBackendPool(lbName, bp); err != nil {
-						return false, false, fmt.Errorf("bi.ReconcileBackendPools for service (%s)(%t): lb backendpool - failed to update backend pool %s for load balancer %s: %w", serviceName, wantLb, lbBackendPoolName, lbName, err)
+						return false, false, fmt.Errorf("bi.ReconcileBackendPools for service (%s): lb backendpool - failed to update backend pool %s for load balancer %s: %w", serviceName, lbBackendPoolName, lbName, err)
 					}
 				}
 			}
 			break
 		} else {
-			klog.V(10).Infof("bi.ReconcileBackendPools for service (%s)(%t): lb backendpool - found unmanaged backendpool %s", serviceName, wantLb, *bp.Name)
+			klog.V(10).Infof("bi.ReconcileBackendPools for service (%s): found unmanaged backendpool %s", serviceName, *bp.Name)
 		}
 	}
 
@@ -402,7 +446,7 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi
 		changed = true
 	}
 
-	return isBackendPoolPreConfigured, changed, err
+	return isBackendPoolPreConfigured, changed, nil
 }
 
 func newBackendPool(lb *network.LoadBalancer, isBackendPoolPreConfigured bool, preConfiguredBackendPoolLoadBalancerTypes, serviceName, lbBackendPoolName string) bool {