fix: Support switching from loadbalancer to externalName for services

k8s-infra-cherrypick-robot · Nov 13, 2024 · b807f4f · b807f4f
1 parent 31fd487
commit b807f4f
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 21 deletions.
diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go
@@ -711,15 +711,12 @@ func (az *Cloud) cleanOrphanedLoadBalancer(ctx context.Context, lb *network.Load
 }
 
 // safeDeleteLoadBalancer deletes the load balancer after decoupling it from the vmSet
-func (az *Cloud) safeDeleteLoadBalancer(ctx context.Context, lb network.LoadBalancer, clusterName, vmSetName string, service *v1.Service) *retry.Error {
-	lbBackendPoolIDs := az.getBackendPoolIDs(clusterName, ptr.Deref(lb.Name, ""))
+func (az *Cloud) safeDeleteLoadBalancer(ctx context.Context, lb network.LoadBalancer, _, vmSetName string, service *v1.Service) *retry.Error {
 	lbBackendPoolIDsToDelete := []string{}
-	v4Enabled, v6Enabled := getIPFamiliesEnabled(service)
-	if v4Enabled {
-		lbBackendPoolIDsToDelete = append(lbBackendPoolIDsToDelete, lbBackendPoolIDs[consts.IPVersionIPv4])
-	}
-	if v6Enabled {
-		lbBackendPoolIDsToDelete = append(lbBackendPoolIDsToDelete, lbBackendPoolIDs[consts.IPVersionIPv6])
+	if lb.LoadBalancerPropertiesFormat != nil && lb.BackendAddressPools != nil {
+		for _, bp := range *lb.BackendAddressPools {
+			lbBackendPoolIDsToDelete = append(lbBackendPoolIDsToDelete, ptr.Deref(bp.ID, ""))
+		}
 	}
 	if _, err := az.VMSet.EnsureBackendPoolDeleted(ctx, service, lbBackendPoolIDsToDelete, vmSetName, lb.BackendAddressPools, true); err != nil {
 		return retry.NewError(false, fmt.Errorf("safeDeleteLoadBalancer: failed to EnsureBackendPoolDeleted: %w", err))
@@ -1507,7 +1504,7 @@ func (az *Cloud) isFrontendIPChanged(
 	if fipIPVersion != "" {
 		isIPv6 = fipIPVersion == network.IPv6
 	} else {
-		if isIPv6, err = az.isFIPIPv6(service, pipRG, &config); err != nil {
+		if isIPv6, err = az.isFIPIPv6(service, &config); err != nil {
 			return false, err
 		}
 	}
@@ -1657,7 +1654,6 @@ func (az *Cloud) findFrontendIPConfigsOfService(
 	service *v1.Service,
 ) (map[bool]*network.FrontendIPConfiguration, error) {
 	fipsOfServiceMap := map[bool]*network.FrontendIPConfiguration{}
-	pipRG := az.getPublicIPAddressResourceGroup(service)
 	for _, config := range *fipConfigs {
 		config := config
 		owns, _, fipIPVersion := az.serviceOwnsFrontendIP(ctx, config, service)
@@ -1667,7 +1663,7 @@ func (az *Cloud) findFrontendIPConfigsOfService(
 			if fipIPVersion != "" {
 				fipIsIPv6 = fipIPVersion == network.IPv6
 			} else {
-				if fipIsIPv6, err = az.isFIPIPv6(service, pipRG, &config); err != nil {
+				if fipIsIPv6, err = az.isFIPIPv6(service, &config); err != nil {
 					return nil, err
 				}
 			}
@@ -1843,7 +1839,6 @@ func (az *Cloud) reconcileLoadBalancer(ctx context.Context, clusterName string,
 	}
 
 	// update probes/rules
-	pipRG := az.getPublicIPAddressResourceGroup(service)
 	for _, ownedFIPConfig := range ownedFIPConfigs {
 		if ownedFIPConfig == nil {
 			continue
@@ -1858,7 +1853,7 @@ func (az *Cloud) reconcileLoadBalancer(ctx context.Context, clusterName string,
 		if fipIPVersion != "" {
 			isIPv6 = fipIPVersion == network.IPv6
 		} else {
-			if isIPv6, err = az.isFIPIPv6(service, pipRG, ownedFIPConfig); err != nil {
+			if isIPv6, err = az.isFIPIPv6(service, ownedFIPConfig); err != nil {
 				return nil, err
 			}
 		}
@@ -2519,8 +2514,6 @@ func (az *Cloud) reconcileFrontendIPConfigs(
 			}
 		}
 
-		pipRG := az.getPublicIPAddressResourceGroup(service)
-
 		for i := len(newConfigs) - 1; i >= 0; i-- {
 			config := newConfigs[i]
 			isServiceOwnsFrontendIP, _, fipIPVersion := az.serviceOwnsFrontendIP(ctx, config, service)
@@ -2534,7 +2527,7 @@ func (az *Cloud) reconcileFrontendIPConfigs(
 			if fipIPVersion != "" {
 				isIPv6 = fipIPVersion == network.IPv6
 			} else {
-				if isIPv6, err = az.isFIPIPv6(service, pipRG, &config); err != nil {
+				if isIPv6, err = az.isFIPIPv6(service, &config); err != nil {
 					return nil, toDeleteConfigs, false, err
 				}
 			}

diff --git a/pkg/provider/azure_loadbalancer_test.go b/pkg/provider/azure_loadbalancer_test.go
@@ -6236,6 +6236,8 @@ func TestCleanOrphanedLoadBalancerLBInUseByVMSS(t *testing.T) {
 		expectedVMSS := buildTestVMSSWithLB(testVMSSName, "vmss-vm-", []string{testLBBackendpoolID0}, false)
 		mockVMSSClient := cloud.VirtualMachineScaleSetsClient.(*mockvmssclient.MockInterface)
 		mockVMSSClient.EXPECT().List(gomock.Any(), "rg").Return([]compute.VirtualMachineScaleSet{expectedVMSS}, nil)
+		mockVMSSClient.EXPECT().Get(gomock.Any(), "rg", testVMSSName).Return(expectedVMSS, nil)
+		mockVMSSClient.EXPECT().CreateOrUpdate(gomock.Any(), "rg", testVMSSName, gomock.Any()).Return(nil)
 
 		service := getTestService("test", v1.ProtocolTCP, nil, false, 80)
 		lb := getTestLoadBalancer(ptr.To("test"), ptr.To("rg"), ptr.To("test"), ptr.To("test"), service, consts.LoadBalancerSkuStandard)

diff --git a/pkg/provider/azure_privatelinkservice.go b/pkg/provider/azure_privatelinkservice.go
@@ -46,15 +46,14 @@ func (az *Cloud) reconcilePrivateLinkService(
 	wantPLS bool,
 ) error {
 	isinternal := requiresInternalLoadBalancer(service)
-	pipRG := az.getPublicIPAddressResourceGroup(service)
 	_, _, fipIPVersion := az.serviceOwnsFrontendIP(ctx, *fipConfig, service)
 	serviceName := getServiceName(service)
 	var isIPv6 bool
 	var err error
 	if fipIPVersion != "" {
 		isIPv6 = fipIPVersion == network.IPv6
 	} else {
-		if isIPv6, err = az.isFIPIPv6(service, pipRG, fipConfig); err != nil {
+		if isIPv6, err = az.isFIPIPv6(service, fipConfig); err != nil {
 			klog.Errorf("reconcilePrivateLinkService for service(%s): failed to get FIP IP family: %v", serviceName, err)
 			return err
 		}

diff --git a/pkg/provider/azure_utils.go b/pkg/provider/azure_utils.go
@@ -438,9 +438,12 @@ func getResourceByIPFamily(resource string, isDualStack, isIPv6 bool) string {
 
 // isFIPIPv6 checks if the frontend IP configuration is of IPv6.
 // NOTICE: isFIPIPv6 assumes the FIP is owned by the Service and it is the primary Service.
-func (az *Cloud) isFIPIPv6(service *v1.Service, _ string, fip *network.FrontendIPConfiguration) (bool, error) {
+func (az *Cloud) isFIPIPv6(service *v1.Service, fip *network.FrontendIPConfiguration) (bool, error) {
 	isDualStack := isServiceDualStack(service)
 	if !isDualStack {
+		if len(service.Spec.IPFamilies) == 0 {
+			return false, nil
+		}
 		return service.Spec.IPFamilies[0] == v1.IPv6Protocol, nil
 	}
 	return managedResourceHasIPv6Suffix(ptr.Deref(fip.Name, "")), nil

diff --git a/pkg/provider/azure_utils_test.go b/pkg/provider/azure_utils_test.go
@@ -873,12 +873,21 @@ func TestIsFIPIPv6(t *testing.T) {
 			},
 			expectedIsIPv6: true,
 		},
+		{
+			desc: "enpty ip families",
+			svc: v1.Service{
+				Spec: v1.ServiceSpec{
+					IPFamilies: []v1.IPFamily{},
+				},
+			},
+			expectedIsIPv6: false,
+		},
 	}
 	for _, tc := range testcases {
 		tc := tc
 		t.Run(tc.desc, func(t *testing.T) {
 			az := GetTestCloud(ctrl)
-			isIPv6, err := az.isFIPIPv6(&tc.svc, "rg", tc.fip)
+			isIPv6, err := az.isFIPIPv6(&tc.svc, tc.fip)
 			assert.Nil(t, err)
 			assert.Equal(t, tc.expectedIsIPv6, isIPv6)
 		})

diff --git a/pkg/provider/azure_vmss.go b/pkg/provider/azure_vmss.go
@@ -1967,7 +1967,9 @@ func (ss *ScaleSet) EnsureBackendPoolDeleted(ctx context.Context, service *v1.Se
 
 	for _, backendPool := range *backendAddressPools {
 		for _, backendPoolID := range backendPoolIDs {
-			if strings.EqualFold(*backendPool.ID, backendPoolID) && backendPool.BackendIPConfigurations != nil {
+			if strings.EqualFold(*backendPool.ID, backendPoolID) &&
+				backendPool.BackendAddressPoolPropertiesFormat != nil &&
+				backendPool.BackendIPConfigurations != nil {
 				for _, ipConf := range *backendPool.BackendIPConfigurations {
 					if ipConf.ID == nil {
 						continue