From b807f4f7d50321b6aa65947d31171620128907de Mon Sep 17 00:00:00 2001 From: Qi Ni Date: Fri, 8 Nov 2024 22:53:26 +1100 Subject: [PATCH] fix: Support switching from loadbalancer to externalName for services --- pkg/provider/azure_loadbalancer.go | 25 +++++++++--------------- pkg/provider/azure_loadbalancer_test.go | 2 ++ pkg/provider/azure_privatelinkservice.go | 3 +-- pkg/provider/azure_utils.go | 5 ++++- pkg/provider/azure_utils_test.go | 11 ++++++++++- pkg/provider/azure_vmss.go | 4 +++- 6 files changed, 29 insertions(+), 21 deletions(-) diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go index c846ff1785..7a369a4e3c 100644 --- 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 index ca8b3ffb01..3385b30723 100644 --- 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 index de8fb3a9b4..e54d3972c0 100644 --- a/pkg/provider/azure_privatelinkservice.go +++ b/pkg/provider/azure_privatelinkservice.go @@ -46,7 +46,6 @@ 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 @@ -54,7 +53,7 @@ func (az *Cloud) reconcilePrivateLinkService( 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 index 65fef0f218..6cb2def641 100644 --- 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 index 2705f73998..7d1ae10ff0 100644 --- 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 index c48a1a5cf2..06222ab63d 100644 --- 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