fix: remove lb from vmss when the backend pool is empty

kubernetes-sigs · Nov 16, 2022 · a3541e0 · a3541e0
1 parent e9a699a
commit a3541e0
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 23 deletions.
diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go
@@ -522,12 +522,10 @@ 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 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 {
-			return retry.NewError(false, fmt.Errorf("safeDeleteLoadBalancer: failed to EnsureBackendPoolDeleted: %w", err))
-		}
+	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 {
+		return retry.NewError(false, fmt.Errorf("safeDeleteLoadBalancer: failed to EnsureBackendPoolDeleted: %w", err))
 	}
 
 	klog.V(2).Infof("safeDeleteLoadBalancer: deleting LB %s", to.String(lb.Name))

diff --git a/pkg/provider/azure_loadbalancer_test.go b/pkg/provider/azure_loadbalancer_test.go
@@ -5733,3 +5733,60 @@ func TestGetClusterFromPIPClusterTags(t *testing.T) {
 		assert.Equal(t, actual, c.expected, "TestCase[%d]: %s", i, c.desc)
 	}
 }
+
+func TestSafeDeleteLoadBalancer(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	cloud := GetTestCloud(ctrl)
+
+	testCases := []struct {
+		desc                string
+		expectedDeleteCall  bool
+		expectedDecoupleErr error
+		expectedErr         *retry.Error
+	}{
+		{
+			desc:               "Standard SKU: should delete the load balancer",
+			expectedDeleteCall: true,
+			expectedErr:        nil,
+		},
+		{
+			desc:                "Standard SKU: should not delete the load balancer if failed to ensure backend pool deleted",
+			expectedDeleteCall:  false,
+			expectedDecoupleErr: errors.New("error"),
+			expectedErr: retry.NewError(
+				false,
+				fmt.Errorf("safeDeleteLoadBalancer: failed to EnsureBackendPoolDeleted: %w", errors.New("error")),
+			),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			mockLBClient := mockloadbalancerclient.NewMockInterface(ctrl)
+			if tc.expectedDeleteCall {
+				mockLBClient.EXPECT().Delete(gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.expectedErr).Times(1)
+			}
+			mockVMSet := NewMockVMSet(ctrl)
+			mockVMSet.EXPECT().EnsureBackendPoolDeleted(
+				gomock.Any(),
+				gomock.Any(),
+				gomock.Any(),
+				gomock.Any(),
+				gomock.Any(),
+			).Return(tc.expectedDecoupleErr)
+			cloud.VMSet = mockVMSet
+			cloud.LoadBalancerClient = mockLBClient
+			svc := getTestService("svc", v1.ProtocolTCP, nil, false, 80)
+			lb := network.LoadBalancer{
+				Name: to.StringPtr("test"),
+				LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{
+					BackendAddressPools: &[]network.BackendAddressPool{},
+				},
+			}
+			err := cloud.safeDeleteLoadBalancer(lb, "cluster", "vmss", &svc)
+			assert.Equal(t, tc.expectedErr, err)
+		})
+	}
+}
diff --git a/pkg/provider/azure_utils.go b/pkg/provider/azure_utils.go
@@ -261,22 +261,6 @@ func getNodePrivateIPAddresses(node *v1.Node) []string {
 	return addresses
 }
 
-func isLBBackendPoolTypeIPConfig(service *v1.Service, lb *network.LoadBalancer, clusterName string) bool {
-	if lb == nil || lb.LoadBalancerPropertiesFormat == nil || lb.BackendAddressPools == nil {
-		klog.V(4).Infof("isLBBackendPoolTypeIPConfig: no backend pools in the LB %s", to.String(lb.Name))
-		return false
-	}
-	lbBackendPoolName := getBackendPoolName(clusterName, service)
-	for _, bp := range *lb.BackendAddressPools {
-		if strings.EqualFold(to.String(bp.Name), lbBackendPoolName) {
-			return bp.BackendAddressPoolPropertiesFormat != nil &&
-				bp.BackendIPConfigurations != nil &&
-				len(*bp.BackendIPConfigurations) != 0
-		}
-	}
-	return false
-}
-
 func getBoolValueFromServiceAnnotations(service *v1.Service, key string) bool {
 	if l, found := service.Annotations[key]; found {
 		return strings.EqualFold(strings.TrimSpace(l), consts.TrueAnnotationValue)

diff --git a/pkg/provider/azure_vmss.go b/pkg/provider/azure_vmss.go
@@ -1732,7 +1732,7 @@ func (ss *ScaleSet) ensureBackendPoolDeletedFromVMSS(backendPoolID, vmSetName st
 	if err != nil {
 		return err
 	}
-	if !ss.DisableAvailabilitySetNodes || ss.EnableVmssFlexNodes {
+	if ss.EnableVmssFlexNodes {
 		flexScaleSet := ss.flexScaleSet.(*FlexScaleSet)
 		err = flexScaleSet.ensureBackendPoolDeletedFromVmssFlex(backendPoolID, vmSetName)
 	}