diff --git a/pkg/consts/consts.go b/pkg/consts/consts.go index 9d0adab92c..34dc01e1cc 100644 --- a/pkg/consts/consts.go +++ b/pkg/consts/consts.go @@ -193,6 +193,9 @@ const ( BackoffJitterDefault = 1.0 ) +// After dual-stack is supported, all references should be updated. +var DualstackSupported = false + // LB variables for dual-stack var ( // Service.Spec.LoadBalancerIP has been deprecated and may be removed in a future release. Those two annotations are introduced as alternatives to set IPv4/IPv6 LoadBalancer IPs. diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go index 6e5e5d609d..8071e40aea 100644 --- a/pkg/provider/azure_loadbalancer.go +++ b/pkg/provider/azure_loadbalancer.go @@ -22,7 +22,6 @@ import ( "errors" "fmt" "math" - "net" "net/netip" "reflect" "sort" @@ -48,36 +47,6 @@ import ( "sigs.k8s.io/cloud-provider-azure/pkg/retry" ) -// getServiceLoadBalancerIP retrieves LB IP from IPv4 annotation, then IPv6 annotation, then service.Spec.LoadBalancerIP. -// TODO: Dual-stack support is not implemented. -func getServiceLoadBalancerIP(service *v1.Service) string { - if service == nil { - return "" - } - - if ip, ok := service.Annotations[consts.ServiceAnnotationLoadBalancerIPDualStack[false]]; ok && ip != "" { - return ip - } - if ip, ok := service.Annotations[consts.ServiceAnnotationLoadBalancerIPDualStack[true]]; ok && ip != "" { - return ip - } - - // Retrieve LB IP from service.Spec.LoadBalancerIP (will be deprecated) - return service.Spec.LoadBalancerIP -} - -// setServiceLoadBalancerIP sets LB IP to a Service -func setServiceLoadBalancerIP(service *v1.Service, ip string) { - if service.Annotations == nil { - service.Annotations = map[string]string{} - } - if net.ParseIP(ip).To4() != nil { - service.Annotations[consts.ServiceAnnotationLoadBalancerIPDualStack[false]] = ip - return - } - service.Annotations[consts.ServiceAnnotationLoadBalancerIPDualStack[true]] = ip -} - // GetLoadBalancer returns whether the specified load balancer and its components exist, and // if so, what its status is. func (az *Cloud) GetLoadBalancer(ctx context.Context, clusterName string, service *v1.Service) (status *v1.LoadBalancerStatus, exists bool, err error) { @@ -452,7 +421,8 @@ func (az *Cloud) cleanOrphanedLoadBalancer(lb *network.LoadBalancer, existingLBs serviceName := getServiceName(service) isBackendPoolPreConfigured := az.isBackendPoolPreConfigured(service) lbResourceGroup := az.getLoadBalancerResourceGroup() - lbBackendPoolName := getBackendPoolName(clusterName, service) + isIPv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) + lbBackendPoolName := getBackendPoolName(clusterName, isIPv6) 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) @@ -524,7 +494,8 @@ 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 { - lbBackendPoolID := az.getBackendPoolID(pointer.StringDeref(lb.Name, ""), az.getLoadBalancerResourceGroup(), getBackendPoolName(clusterName, service)) + isIPv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) + lbBackendPoolID := az.getBackendPoolID(pointer.StringDeref(lb.Name, ""), az.getLoadBalancerResourceGroup(), getBackendPoolName(clusterName, isIPv6)) _, 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)) @@ -890,17 +861,18 @@ func (az *Cloud) determinePublicIPName(clusterName string, service *v1.Service, if name, found := service.Annotations[consts.ServiceAnnotationPIPName]; found && name != "" { return name, true, nil } + isIPv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) if ipPrefix, ok := service.Annotations[consts.ServiceAnnotationPIPPrefixID]; ok && ipPrefix != "" { - return az.getPublicIPName(clusterName, service), false, nil + return az.getPublicIPName(clusterName, service, isIPv6), false, nil } pipResourceGroup := az.getPublicIPAddressResourceGroup(service) - loadBalancerIP := getServiceLoadBalancerIP(service) + loadBalancerIP := getServiceLoadBalancerIP(service, isIPv6) // Assume that the service without loadBalancerIP set is a primary service. // If a secondary service doesn't set the loadBalancerIP, it is not allowed to share the IP. if len(loadBalancerIP) == 0 { - return az.getPublicIPName(clusterName, service), false, nil + return az.getPublicIPName(clusterName, service, isIPv6), false, nil } // For the services with loadBalancerIP set, an existing public IP is required, primary @@ -917,18 +889,25 @@ func (az *Cloud) determinePublicIPName(clusterName string, service *v1.Service, return "", false, fmt.Errorf("user supplied IP Address %s was not found in resource group %s", loadBalancerIP, pipResourceGroup) } -// pips: a non-nil pointer to a slice of existing PIPs, if the slice being pointed to is nil, listPIP would be called when needed and the slice would be filled -func (az *Cloud) findMatchedPIPByLoadBalancerIP(service *v1.Service, loadBalancerIP, pipResourceGroup string, pips *[]network.PublicIPAddress) (*network.PublicIPAddress, error) { +func (az *Cloud) ensurePIP(pipResourceGroup string, pips *[]network.PublicIPAddress) error { if pips == nil { // this should not happen - return nil, fmt.Errorf("findMatchedPIPByLoadBalancerIP: nil pip list passed") + return fmt.Errorf("nil pip list passed") } else if *pips == nil { pipList, err := az.listPIP(pipResourceGroup) if err != nil { - return nil, err + return err } *pips = pipList } + return nil +} + +// pips: a non-nil pointer to a slice of existing PIPs, if the slice being pointed to is nil, listPIP would be called when needed and the slice would be filled +func (az *Cloud) findMatchedPIPByLoadBalancerIP(service *v1.Service, loadBalancerIP, pipResourceGroup string, pips *[]network.PublicIPAddress) (*network.PublicIPAddress, error) { + if err := az.ensurePIP(pipResourceGroup, pips); err != nil { + return nil, fmt.Errorf("findMatchedPIPByLoadBalancerIP: failed to ensurePIP: %w", err) + } for _, pip := range *pips { if pip.PublicIPAddressPropertiesFormat.IPAddress != nil && *pip.PublicIPAddressPropertiesFormat.IPAddress == loadBalancerIP { @@ -963,7 +942,8 @@ func updateServiceLoadBalancerIP(service *v1.Service, serviceIP string) *v1.Serv } func (az *Cloud) findServiceIPAddress(ctx context.Context, clusterName string, service *v1.Service) (string, error) { - lbIP := getServiceLoadBalancerIP(service) + isIPv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) + lbIP := getServiceLoadBalancerIP(service, isIPv6) if len(lbIP) > 0 { return lbIP, nil } @@ -1360,7 +1340,8 @@ func (az *Cloud) isFrontendIPChanged(clusterName string, config network.Frontend if !strings.EqualFold(pointer.StringDeref(config.Name, ""), lbFrontendIPConfigName) { return false, nil } - loadBalancerIP := getServiceLoadBalancerIP(service) + isIPv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) + loadBalancerIP := getServiceLoadBalancerIP(service, isIPv6) isInternal := requiresInternalLoadBalancer(service) if isInternal { // Judge subnet @@ -1546,7 +1527,8 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service, lbName := *lb.Name lbResourceGroup := az.getLoadBalancerResourceGroup() - lbBackendPoolID := az.getBackendPoolID(lbName, az.getLoadBalancerResourceGroup(), getBackendPoolName(clusterName, service)) + isIPv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) + lbBackendPoolID := az.getBackendPoolID(lbName, az.getLoadBalancerResourceGroup(), getBackendPoolName(clusterName, isIPv6)) klog.V(2).Infof("reconcileLoadBalancer for service(%s): lb(%s/%s) wantLb(%t) resolved load balancer name", serviceName, lbResourceGroup, lbName, wantLb) defaultLBFrontendIPConfigName := az.getDefaultFrontendIPConfigName(service) defaultLBFrontendIPConfigID := az.getFrontendIPConfigID(lbName, lbResourceGroup, defaultLBFrontendIPConfigName) @@ -1668,7 +1650,8 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service, if lb.LoadBalancerPropertiesFormat != nil && lb.BackendAddressPools != nil { backendPools := *lb.BackendAddressPools for _, backendPool := range backendPools { - if strings.EqualFold(pointer.StringDeref(backendPool.Name, ""), getBackendPoolName(clusterName, service)) { + isIPv6 := ifBackendPoolIPv6(pointer.StringDeref(backendPool.Name, "")) + if strings.EqualFold(pointer.StringDeref(backendPool.Name, ""), getBackendPoolName(clusterName, isIPv6)) { if err := az.LoadBalancerBackendPool.EnsureHostsInPool(service, nodes, lbBackendPoolID, vmSetName, clusterName, lbName, backendPool); err != nil { return nil, err } @@ -1875,11 +1858,12 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv Subnet: &subnet, } - if utilnet.IsIPv6String(service.Spec.ClusterIP) { + isIPv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) + if isIPv6 { configProperties.PrivateIPAddressVersion = network.IPv6 } - loadBalancerIP := getServiceLoadBalancerIP(service) + loadBalancerIP := getServiceLoadBalancerIP(service, isIPv6) if loadBalancerIP != "" { configProperties.PrivateIPAllocationMethod = network.Static configProperties.PrivateIPAddress = &loadBalancerIP @@ -2285,7 +2269,7 @@ func (az *Cloud) getExpectedLBRules( var nodeEndpointHealthprobe *network.Probe if servicehelpers.NeedsHealthCheck(service) { podPresencePath, podPresencePort := servicehelpers.GetServiceHealthCheckPathPort(service) - lbRuleName := az.getLoadBalancerRuleName(service, v1.ProtocolTCP, podPresencePort) + lbRuleName := az.getLoadBalancerRuleName(service, v1.ProtocolTCP, podPresencePort, utilnet.IsIPv6String(service.Spec.ClusterIP)) nodeEndpointHealthprobe = &network.Probe{ Name: &lbRuleName, @@ -2306,7 +2290,7 @@ func (az *Cloud) getExpectedLBRules( az.useStandardLoadBalancer() && consts.IsK8sServiceHasHAModeEnabled(service) { - lbRuleName := az.getloadbalancerHAmodeRuleName(service) + lbRuleName := az.getloadbalancerHAmodeRuleName(service, utilnet.IsIPv6String(service.Spec.ClusterIP)) klog.V(2).Infof("getExpectedLBRules lb name (%s) rule name (%s)", lbName, lbRuleName) props, err := az.getExpectedHAModeLoadBalancingRuleProperties(service, lbFrontendIPConfigID, lbBackendPoolID) @@ -2346,7 +2330,7 @@ func (az *Cloud) getExpectedLBRules( // generate lb rule for each port defined in svc object for _, port := range service.Spec.Ports { - lbRuleName := az.getLoadBalancerRuleName(service, port.Protocol, port.Port) + lbRuleName := az.getLoadBalancerRuleName(service, port.Protocol, port.Port, utilnet.IsIPv6String(service.Spec.ClusterIP)) klog.V(2).Infof("getExpectedLBRules lb name (%s) rule name (%s)", lbName, lbRuleName) isNoLBRuleRequired, err := consts.IsLBRuleOnK8sServicePortDisabled(service.Annotations, port.Port) if err != nil { @@ -2630,7 +2614,7 @@ func (az *Cloud) reconcileSecurityRules(sg network.SecurityGroup, service *v1.Se if useSharedSecurityRule(service) && !wantLb { for _, port := range ports { for _, sourceAddressPrefix := range sourceAddressPrefixes { - sharedRuleName := az.getSecurityRuleName(service, port, sourceAddressPrefix) + sharedRuleName := az.getSecurityRuleName(service, port, sourceAddressPrefix, utilnet.IsIPv6String(service.Spec.ClusterIP)) sharedIndex, sharedRule, sharedRuleFound := findSecurityRuleByName(updatedRules, sharedRuleName) if !sharedRuleFound { klog.V(4).Infof("Didn't find shared rule %s for service %s", sharedRuleName, service.Name) @@ -2723,7 +2707,7 @@ func (az *Cloud) getExpectedSecurityRules(wantLb bool, ports []v1.ServicePort, s } for j := range sourceAddressPrefixes { ix := i*len(sourceAddressPrefixes) + j - securityRuleName := az.getSecurityRuleName(service, port, sourceAddressPrefixes[j]) + securityRuleName := az.getSecurityRuleName(service, port, sourceAddressPrefixes[j], utilnet.IsIPv6String(service.Spec.ClusterIP)) nsgRule := network.SecurityRule{ Name: pointer.String(securityRuleName), SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{ @@ -2760,7 +2744,7 @@ func (az *Cloud) getExpectedSecurityRules(wantLb bool, ports []v1.ServicePort, s if err != nil { return nil, err } - securityRuleName := az.getSecurityRuleName(service, port, "deny_all") + securityRuleName := az.getSecurityRuleName(service, port, "deny_all", utilnet.IsIPv6String(service.Spec.ClusterIP)) nsgRule := network.SecurityRule{ Name: pointer.String(securityRuleName), SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{ @@ -3494,7 +3478,7 @@ func serviceOwnsPublicIP(service *v1.Service, pip *network.PublicIPAddress, clus // if there is no service tag on the pip, it is user-created pip if serviceTag == "" { - return strings.EqualFold(pointer.StringDeref(pip.IPAddress, ""), getServiceLoadBalancerIP(service)), true + return strings.EqualFold(pointer.StringDeref(pip.IPAddress, ""), getServiceLoadBalancerIP(service, utilnet.IsIPv6String(service.Spec.ClusterIP))), true } // if there is service tag on the pip, it is system-created pip @@ -3512,7 +3496,7 @@ func serviceOwnsPublicIP(service *v1.Service, pip *network.PublicIPAddress, clus } else { // if the service is not included in the tags of the system-created pip, check the ip address // this could happen for secondary services - return strings.EqualFold(pointer.StringDeref(pip.IPAddress, ""), getServiceLoadBalancerIP(service)), false + return strings.EqualFold(pointer.StringDeref(pip.IPAddress, ""), getServiceLoadBalancerIP(service, utilnet.IsIPv6String(service.Spec.ClusterIP))), false } } diff --git a/pkg/provider/azure_loadbalancer_backendpool.go b/pkg/provider/azure_loadbalancer_backendpool.go index 01ab84453c..50d044a99e 100644 --- a/pkg/provider/azure_loadbalancer_backendpool.go +++ b/pkg/provider/azure_loadbalancer_backendpool.go @@ -70,18 +70,39 @@ func (bc *backendPoolTypeNodeIPConfig) EnsureHostsInPool(service *v1.Service, no return bc.VMSet.EnsureHostsInPool(service, nodes, backendPoolID, vmSetName) } +func ifLBBackendPoolsExist(lbBackendPoolNames map[bool]string, bpName *string) (found, isIPv6 bool) { + if strings.EqualFold(pointer.StringDeref(bpName, ""), lbBackendPoolNames[false]) { + isIPv6 = false + found = true + } + if strings.EqualFold(pointer.StringDeref(bpName, ""), lbBackendPoolNames[true]) { + isIPv6 = true + found = true + } + return found, isIPv6 +} + func (bc *backendPoolTypeNodeIPConfig) CleanupVMSetFromBackendPoolByCondition(slb *network.LoadBalancer, service *v1.Service, nodes []*v1.Node, clusterName string, shouldRemoveVMSetFromSLB func(string) bool) (*network.LoadBalancer, error) { - lbBackendPoolName := getBackendPoolName(clusterName, service) + v4Enabled, v6Enabled := getIPFamiliesEnabled(service) + + lbBackendPoolNames := map[bool]string{ + false: getBackendPoolName(clusterName, false), + true: getBackendPoolName(clusterName, true), + } lbResourceGroup := bc.getLoadBalancerResourceGroup() - lbBackendPoolID := bc.getBackendPoolID(pointer.StringDeref(slb.Name, ""), lbResourceGroup, lbBackendPoolName) + lbBackendPoolIDs := map[bool]string{ + false: bc.getBackendPoolID(pointer.StringDeref(slb.Name, ""), lbResourceGroup, lbBackendPoolNames[false]), + true: bc.getBackendPoolID(pointer.StringDeref(slb.Name, ""), lbResourceGroup, lbBackendPoolNames[true]), + } newBackendPools := make([]network.BackendAddressPool, 0) if slb.LoadBalancerPropertiesFormat != nil && slb.BackendAddressPools != nil { newBackendPools = *slb.BackendAddressPools } vmSetNameToBackendIPConfigurationsToBeDeleted := make(map[string][]network.InterfaceIPConfiguration) + count, ipFamiliesCount := 0, len(service.Spec.IPFamilies) for j, bp := range newBackendPools { - if strings.EqualFold(pointer.StringDeref(bp.Name, ""), lbBackendPoolName) { + if found, _ := ifLBBackendPoolsExist(lbBackendPoolNames, bp.Name); found { klog.V(2).Infof("bc.CleanupVMSetFromBackendPoolByCondition: checking the backend pool %s from standard load balancer %s", pointer.StringDeref(bp.Name, ""), pointer.StringDeref(slb.Name, "")) if bp.BackendAddressPoolPropertiesFormat != nil && bp.BackendIPConfigurations != nil { for i := len(*bp.BackendIPConfigurations) - 1; i >= 0; i-- { @@ -105,27 +126,45 @@ func (bc *backendPoolTypeNodeIPConfig) CleanupVMSetFromBackendPoolByCondition(sl } newBackendPools[j] = bp - break + count++ + if count == ipFamiliesCount { + break + } } } for vmSetName := range vmSetNameToBackendIPConfigurationsToBeDeleted { + shouldRefreshLB := false backendIPConfigurationsToBeDeleted := vmSetNameToBackendIPConfigurationsToBeDeleted[vmSetName] - backendpoolToBeDeleted := &[]network.BackendAddressPool{ - { - ID: pointer.String(lbBackendPoolID), - BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{ - BackendIPConfigurations: &backendIPConfigurationsToBeDeleted, + deleteBackendPool := func(isIPv6 bool) error { + backendpoolToBeDeleted := &[]network.BackendAddressPool{ + { + ID: pointer.String(lbBackendPoolIDs[isIPv6]), + BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{ + BackendIPConfigurations: &backendIPConfigurationsToBeDeleted, + }, }, - }, + } + // decouple the backendPool from the node + refresh, err := bc.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolIDs[isIPv6], vmSetName, backendpoolToBeDeleted, true) + if refresh { + shouldRefreshLB = true + } + return err } - // decouple the backendPool from the node - shouldRefreshLB, err := bc.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, backendpoolToBeDeleted, true) - if err != nil { - return nil, err + if v4Enabled { + if err := deleteBackendPool(false); err != nil { + return nil, err + } + } + if v6Enabled { + if err := deleteBackendPool(true); err != nil { + return nil, err + } } + if shouldRefreshLB { - slb, _, err = bc.getAzureLoadBalancer(pointer.StringDeref(slb.Name, ""), cache.CacheReadTypeForceRefresh) + slb, _, err := bc.getAzureLoadBalancer(pointer.StringDeref(slb.Name, ""), cache.CacheReadTypeForceRefresh) if err != nil { return nil, fmt.Errorf("bc.CleanupVMSetFromBackendPoolByCondition: failed to get load balancer %s, err: %w", pointer.StringDeref(slb.Name, ""), err) } @@ -142,22 +181,31 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string, newBackendPools = *lb.BackendAddressPools } - var foundBackendPool, changed, shouldRefreshLB, isOperationSucceeded, isMigration bool + var changed, shouldRefreshLB, isOperationSucceeded, isMigration bool + foundBackendPools := map[bool]bool{} lbName := *lb.Name serviceName := getServiceName(service) - lbBackendPoolName := getBackendPoolName(clusterName, service) - lbBackendPoolID := bc.getBackendPoolID(lbName, bc.getLoadBalancerResourceGroup(), lbBackendPoolName) + lbBackendPoolNames := map[bool]string{ + false: getBackendPoolName(clusterName, false), + true: getBackendPoolName(clusterName, true), + } + lbBackendPoolIDs := map[bool]string{ + false: bc.getBackendPoolID(lbName, bc.getLoadBalancerResourceGroup(), lbBackendPoolNames[false]), + true: bc.getBackendPoolID(lbName, bc.getLoadBalancerResourceGroup(), lbBackendPoolNames[true]), + } vmSetName := bc.mapLoadBalancerNameToVMSet(lbName, clusterName) isBackendPoolPreConfigured := bc.isBackendPoolPreConfigured(service) mc := metrics.NewMetricContext("services", "migrate_to_nic_based_backend_pool", bc.ResourceGroup, bc.getNetworkResourceSubscriptionID(), serviceName) + count, ipFamiliesCount := 0, len(service.Spec.IPFamilies) for i := len(newBackendPools) - 1; i >= 0; i-- { bp := newBackendPools[i] - if strings.EqualFold(*bp.Name, lbBackendPoolName) { + found, isIPv6 := ifLBBackendPoolsExist(lbBackendPoolNames, bp.Name) + if found { klog.V(10).Infof("bc.ReconcileBackendPools for service (%s): lb backendpool - found wanted backendpool. not adding anything", serviceName) - foundBackendPool = true + foundBackendPools[ifBackendPoolIPv6(pointer.StringDeref(bp.Name, ""))] = true // Don't bother to remove unused nodeIPConfiguration if backend pool is pre configured if isBackendPoolPreConfigured { @@ -174,8 +222,8 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string, isMigration = true bp.VirtualNetwork = nil if err := bc.CreateOrUpdateLBBackendPool(lbName, bp); err != nil { - klog.Errorf("bc.ReconcileBackendPools for service (%s): failed to cleanup IP based backend pool %s: %s", serviceName, lbBackendPoolName, err.Error()) - return false, false, fmt.Errorf("bc.ReconcileBackendPools for service (%s): failed to cleanup IP based backend pool %s: %w", serviceName, lbBackendPoolName, err) + klog.Errorf("bc.ReconcileBackendPools for service (%s): failed to cleanup IP based backend pool %s: %s", serviceName, lbBackendPoolNames[isIPv6], err.Error()) + return false, false, fmt.Errorf("bc.ReconcileBackendPools for service (%s): failed to cleanup IP based backend pool %s: %w", serviceName, lbBackendPoolNames[isIPv6], err) } newBackendPools[i] = bp lb.BackendAddressPools = &newBackendPools @@ -217,14 +265,14 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string, if len(backendIPConfigurationsToBeDeleted) > 0 { backendpoolToBeDeleted := &[]network.BackendAddressPool{ { - ID: pointer.String(lbBackendPoolID), + ID: pointer.String(lbBackendPoolIDs[isIPv6]), BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{ BackendIPConfigurations: &backendIPConfigurationsToBeDeleted, }, }, } // decouple the backendPool from the node - updated, err := bc.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, backendpoolToBeDeleted, false) + updated, err := bc.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolIDs[isIPv6], vmSetName, backendpoolToBeDeleted, false) if err != nil { return false, false, err } @@ -232,9 +280,12 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string, shouldRefreshLB = true } } - break + count++ + if count == ipFamiliesCount { + break + } } else { - klog.V(10).Infof("bc.ReconcileBackendPools for service (%s): lb backendpool - found unmanaged backendpool %s", serviceName, *bp.Name) + klog.V(10).Infof("bc.ReconcileBackendPools for service (%s): lb backendpool - found unmanaged backendpool %s", serviceName, pointer.StringDeref(bp.Name, "")) } } @@ -245,8 +296,14 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string, } } - if !foundBackendPool { - isBackendPoolPreConfigured = newBackendPool(lb, isBackendPoolPreConfigured, bc.PreConfiguredBackendPoolLoadBalancerTypes, getServiceName(service), getBackendPoolName(clusterName, service)) + for _, ipFamily := range service.Spec.IPFamilies { + if foundBackendPools[ipFamily == v1.IPFamily(network.IPv6)] { + continue + } + // TODO: be cautious about isBackendPoolPreConfigured + isBackendPoolPreConfigured = newBackendPool(lb, isBackendPoolPreConfigured, + bc.PreConfiguredBackendPoolLoadBalancerTypes, serviceName, + lbBackendPoolNames[ipFamily == v1.IPv6Protocol]) changed = true } @@ -303,14 +360,19 @@ func getBackendIPConfigurationsToBeDeleted( func (bc *backendPoolTypeNodeIPConfig) GetBackendPrivateIPs(clusterName string, service *v1.Service, lb *network.LoadBalancer) ([]string, []string) { serviceName := getServiceName(service) - lbBackendPoolName := getBackendPoolName(clusterName, service) + lbBackendPoolNames := map[bool]string{ + false: getBackendPoolName(clusterName, false), + true: getBackendPoolName(clusterName, true), + } if lb.LoadBalancerPropertiesFormat == nil || lb.LoadBalancerPropertiesFormat.BackendAddressPools == nil { return nil, nil } backendPrivateIPv4s, backendPrivateIPv6s := sets.NewString(), sets.NewString() + count, ipFamiliesCount := 0, len(service.Spec.IPFamilies) for _, bp := range *lb.BackendAddressPools { - if strings.EqualFold(pointer.StringDeref(bp.Name, ""), lbBackendPoolName) { + found, _ := ifLBBackendPoolsExist(lbBackendPoolNames, bp.Name) + if found { klog.V(10).Infof("bc.GetBackendPrivateIPs for service (%s): found wanted backendpool %s", serviceName, pointer.StringDeref(bp.Name, "")) if bp.BackendAddressPoolPropertiesFormat != nil && bp.BackendIPConfigurations != nil { for _, backendIPConfig := range *bp.BackendIPConfigurations { @@ -336,6 +398,10 @@ func (bc *backendPoolTypeNodeIPConfig) GetBackendPrivateIPs(clusterName string, } } } + count++ + if count == ipFamiliesCount { + break + } } else { klog.V(10).Infof("bc.GetBackendPrivateIPs for service (%s): found unmanaged backendpool %s", serviceName, pointer.StringDeref(bp.Name, "")) } @@ -352,6 +418,7 @@ func newBackendPoolTypeNodeIP(c *Cloud) BackendPool { } func (bi *backendPoolTypeNodeIP) EnsureHostsInPool(service *v1.Service, nodes []*v1.Node, backendPoolID, vmSetName, clusterName, lbName string, backendPool network.BackendAddressPool) error { + isIPv6 := ifBackendPoolIPv6(pointer.StringDeref(backendPool.Name, "")) vnetResourceGroup := bi.ResourceGroup if len(bi.VnetResourceGroup) > 0 { vnetResourceGroup = bi.VnetResourceGroup @@ -360,7 +427,7 @@ func (bi *backendPoolTypeNodeIP) EnsureHostsInPool(service *v1.Service, nodes [] changed := false numOfAdd := 0 - lbBackendPoolName := getBackendPoolName(clusterName, service) + lbBackendPoolName := getBackendPoolName(clusterName, isIPv6) if strings.EqualFold(pointer.StringDeref(backendPool.Name, ""), lbBackendPoolName) && backendPool.BackendAddressPoolPropertiesFormat != nil { backendPool.VirtualNetwork = &network.SubResource{ @@ -414,11 +481,11 @@ func (bi *backendPoolTypeNodeIP) EnsureHostsInPool(service *v1.Service, nodes [] continue } - privateIP := getNodePrivateIPAddress(service, node) + privateIP := getNodePrivateIPAddress(service, node, isIPv6) if !existingIPs.Has(privateIP) { name := node.Name if utilnet.IsIPv6String(privateIP) { - name = fmt.Sprintf("%s-ipv6", name) + name = fmt.Sprintf("%s-%s", name, v6Suffix) } klog.V(6).Infof("bi.EnsureHostsInPool: adding %s with ip address %s", name, privateIP) @@ -444,15 +511,20 @@ func (bi *backendPoolTypeNodeIP) EnsureHostsInPool(service *v1.Service, nodes [] } func (bi *backendPoolTypeNodeIP) CleanupVMSetFromBackendPoolByCondition(slb *network.LoadBalancer, service *v1.Service, nodes []*v1.Node, clusterName string, shouldRemoveVMSetFromSLB func(string) bool) (*network.LoadBalancer, error) { - lbBackendPoolName := getBackendPoolName(clusterName, service) + lbBackendPoolNames := map[bool]string{ + false: getBackendPoolName(clusterName, false), + true: getBackendPoolName(clusterName, true), + } newBackendPools := make([]network.BackendAddressPool, 0) if slb.LoadBalancerPropertiesFormat != nil && slb.BackendAddressPools != nil { newBackendPools = *slb.BackendAddressPools } - var updatedPrivateIPs bool + updatedPrivateIPs := map[bool]bool{} + count, ipFamiliesCount := 0, len(service.Spec.IPFamilies) for j, bp := range newBackendPools { - if strings.EqualFold(pointer.StringDeref(bp.Name, ""), lbBackendPoolName) { + found, isIPv6 := ifLBBackendPoolsExist(lbBackendPoolNames, bp.Name) + if found { klog.V(2).Infof("bi.CleanupVMSetFromBackendPoolByCondition: checking the backend pool %s from standard load balancer %s", pointer.StringDeref(bp.Name, ""), pointer.StringDeref(slb.Name, "")) vmIPsToBeDeleted := sets.NewString() for _, node := range nodes { @@ -462,8 +534,8 @@ func (bi *backendPoolTypeNodeIP) CleanupVMSetFromBackendPoolByCondition(slb *net } if shouldRemoveVMSetFromSLB(vmSetName) { - privateIP := getNodePrivateIPAddress(service, node) - klog.V(4).Infof("bi.CleanupVMSetFromBackendPoolByCondition: removing ip %s from the backend pool %s", privateIP, lbBackendPoolName) + privateIP := getNodePrivateIPAddress(service, node, isIPv6) + klog.V(4).Infof("bi.CleanupVMSetFromBackendPoolByCondition: removing ip %s from the backend pool %s", privateIP, lbBackendPoolNames[isIPv6]) vmIPsToBeDeleted.Insert(privateIP) } } @@ -473,23 +545,30 @@ func (bi *backendPoolTypeNodeIP) CleanupVMSetFromBackendPoolByCondition(slb *net if (*bp.LoadBalancerBackendAddresses)[i].LoadBalancerBackendAddressPropertiesFormat != nil && vmIPsToBeDeleted.Has(pointer.StringDeref((*bp.LoadBalancerBackendAddresses)[i].IPAddress, "")) { *bp.LoadBalancerBackendAddresses = append((*bp.LoadBalancerBackendAddresses)[:i], (*bp.LoadBalancerBackendAddresses)[i+1:]...) - updatedPrivateIPs = true + updatedPrivateIPs[isIPv6] = true } } } newBackendPools[j] = bp - break + count++ + if count == ipFamiliesCount { + break + } + } else { + klog.V(10).Infof("bi.CleanupVMSetFromBackendPoolByCondition: found unmanaged backendpool %s from standard load balancer %q", pointer.StringDeref(bp.Name, ""), pointer.StringDeref(slb.Name, "")) } + } - if updatedPrivateIPs { + for isIPv6 := range updatedPrivateIPs { klog.V(2).Infof("bi.CleanupVMSetFromBackendPoolByCondition: updating lb %s since there are private IP updates", pointer.StringDeref(slb.Name, "")) slb.BackendAddressPools = &newBackendPools for _, backendAddressPool := range *slb.BackendAddressPools { - if strings.EqualFold(lbBackendPoolName, pointer.StringDeref(backendAddressPool.Name, "")) { + if strings.EqualFold(lbBackendPoolNames[isIPv6], pointer.StringDeref(backendAddressPool.Name, "")) { if err := bi.CreateOrUpdateLBBackendPool(pointer.StringDeref(slb.Name, ""), backendAddressPool); err != nil { - return nil, fmt.Errorf("bi.CleanupVMSetFromBackendPoolByCondition: failed to create or update backend pool %s: %w", lbBackendPoolName, err) + return nil, fmt.Errorf("bi.CleanupVMSetFromBackendPoolByCondition: "+ + "failed to create or update backend pool %s: %w", lbBackendPoolNames[isIPv6], err) } } } @@ -504,22 +583,30 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi newBackendPools = *lb.BackendAddressPools } - var foundBackendPool, changed, shouldRefreshLB, isOperationSucceeded, isMigration, updated bool + var changed, shouldRefreshLB, isOperationSucceeded, isMigration, updated bool + foundBackendPools := map[bool]bool{} lbName := *lb.Name serviceName := getServiceName(service) - lbBackendPoolName := getBackendPoolName(clusterName, service) + lbBackendPoolNames := map[bool]string{ + false: getBackendPoolName(clusterName, false), + true: getBackendPoolName(clusterName, true), + } vmSetName := bi.mapLoadBalancerNameToVMSet(lbName, clusterName) - lbBackendPoolID := bi.getBackendPoolID(pointer.StringDeref(lb.Name, ""), bi.getLoadBalancerResourceGroup(), getBackendPoolName(clusterName, service)) + lbBackendPoolIDs := map[bool]string{ + false: bi.getBackendPoolID(pointer.StringDeref(lb.Name, ""), bi.getLoadBalancerResourceGroup(), lbBackendPoolNames[false]), + true: bi.getBackendPoolID(pointer.StringDeref(lb.Name, ""), bi.getLoadBalancerResourceGroup(), lbBackendPoolNames[true]), + } isBackendPoolPreConfigured := bi.isBackendPoolPreConfigured(service) mc := metrics.NewMetricContext("services", "migrate_to_ip_based_backend_pool", bi.ResourceGroup, bi.getNetworkResourceSubscriptionID(), serviceName) - var err error + count, ipFamiliesCount := 0, len(service.Spec.IPFamilies) for i := len(newBackendPools) - 1; i >= 0; i-- { bp := newBackendPools[i] - if strings.EqualFold(*bp.Name, lbBackendPoolName) { + found, isIPv6 := ifLBBackendPoolsExist(lbBackendPoolNames, bp.Name) + if found { klog.V(10).Infof("bi.ReconcileBackendPools for service (%s): found wanted backendpool. not adding anything", serviceName) - foundBackendPool = true + foundBackendPools[ifBackendPoolIPv6(pointer.StringDeref(bp.Name, ""))] = true // Don't bother to remove unused nodeIP if backend pool is pre configured if isBackendPoolPreConfigured { @@ -530,7 +617,7 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi // to nodeIP, we need to decouple the VM NICs from the LB // before attaching nodeIPs/podIPs to the LB backend pool. klog.V(2).Infof("bi.ReconcileBackendPools for service (%s) and vmSet (%s): ensuring the LB is decoupled from the VMSet", serviceName, vmSetName) - shouldRefreshLB, err = bi.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, lb.BackendAddressPools, true) + shouldRefreshLB, err = bi.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolIDs[isIPv6], vmSetName, lb.BackendAddressPools, true) if err != nil { klog.Errorf("bi.ReconcileBackendPools for service (%s): failed to EnsureBackendPoolDeleted: %s", serviceName, err.Error()) return false, false, err @@ -551,7 +638,6 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi updated = true } } - // delete the vnet in LoadBalancerBackendAddresses and ensure it is in the backend pool level var vnet string if bp.BackendAddressPoolPropertiesFormat != nil { @@ -580,11 +666,14 @@ 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): lb backendpool - failed to update backend pool %s for load balancer %s: %w", serviceName, 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, lbBackendPoolNames[isIPv6], lbName, err) } shouldRefreshLB = true } - break + count++ + if count == ipFamiliesCount { + break + } } else { klog.V(10).Infof("bi.ReconcileBackendPools for service (%s): found unmanaged backendpool %s", serviceName, *bp.Name) } @@ -597,8 +686,14 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi } } - if !foundBackendPool { - isBackendPoolPreConfigured = newBackendPool(lb, isBackendPoolPreConfigured, bi.PreConfiguredBackendPoolLoadBalancerTypes, getServiceName(service), getBackendPoolName(clusterName, service)) + for _, ipFamily := range service.Spec.IPFamilies { + if foundBackendPools[ipFamily == v1.IPFamily(network.IPv6)] { + continue + } + // TODO: be cautious about isBackendPoolPreConfigured + isBackendPoolPreConfigured = newBackendPool(lb, isBackendPoolPreConfigured, + bi.PreConfiguredBackendPoolLoadBalancerTypes, serviceName, + lbBackendPoolNames[ipFamily == v1.IPv6Protocol]) changed = true } @@ -614,14 +709,19 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi func (bi *backendPoolTypeNodeIP) GetBackendPrivateIPs(clusterName string, service *v1.Service, lb *network.LoadBalancer) ([]string, []string) { serviceName := getServiceName(service) - lbBackendPoolName := getBackendPoolName(clusterName, service) + lbBackendPoolNames := map[bool]string{ + false: getBackendPoolName(clusterName, false), + true: getBackendPoolName(clusterName, true), + } if lb.LoadBalancerPropertiesFormat == nil || lb.LoadBalancerPropertiesFormat.BackendAddressPools == nil { return nil, nil } backendPrivateIPv4s, backendPrivateIPv6s := sets.NewString(), sets.NewString() + count, ipFamiliesCount := 0, len(service.Spec.IPFamilies) for _, bp := range *lb.BackendAddressPools { - if strings.EqualFold(pointer.StringDeref(bp.Name, ""), lbBackendPoolName) { + found, _ := ifLBBackendPoolsExist(lbBackendPoolNames, bp.Name) + if found { klog.V(10).Infof("bi.GetBackendPrivateIPs for service (%s): found wanted backendpool %s", serviceName, pointer.StringDeref(bp.Name, "")) if bp.BackendAddressPoolPropertiesFormat != nil && bp.LoadBalancerBackendAddresses != nil { for _, backendAddress := range *bp.LoadBalancerBackendAddresses { @@ -638,6 +738,10 @@ func (bi *backendPoolTypeNodeIP) GetBackendPrivateIPs(clusterName string, servic } } } + count++ + if count == ipFamiliesCount { + break + } } else { klog.V(10).Infof("bi.GetBackendPrivateIPs for service (%s): found unmanaged backendpool %s", serviceName, pointer.StringDeref(bp.Name, "")) } @@ -647,9 +751,10 @@ func (bi *backendPoolTypeNodeIP) GetBackendPrivateIPs(clusterName string, servic func newBackendPool(lb *network.LoadBalancer, isBackendPoolPreConfigured bool, preConfiguredBackendPoolLoadBalancerTypes, serviceName, lbBackendPoolName string) bool { if isBackendPoolPreConfigured { - klog.V(2).Infof("newBackendPool for service (%s)(true): lb backendpool - PreConfiguredBackendPoolLoadBalancerTypes %s has been set but can not find corresponding backend pool, ignoring it", + klog.V(2).Infof("newBackendPool for service (%s)(true): lb backendpool - PreConfiguredBackendPoolLoadBalancerTypes %s has been set but can not find corresponding backend pool %q, ignoring it", serviceName, - preConfiguredBackendPoolLoadBalancerTypes) + preConfiguredBackendPoolLoadBalancerTypes, + lbBackendPoolName) isBackendPoolPreConfigured = false } diff --git a/pkg/provider/azure_loadbalancer_backendpool_test.go b/pkg/provider/azure_loadbalancer_backendpool_test.go index 4087ae9ac8..f9009518d0 100644 --- a/pkg/provider/azure_loadbalancer_backendpool_test.go +++ b/pkg/provider/azure_loadbalancer_backendpool_test.go @@ -47,47 +47,6 @@ func TestEnsureHostsInPoolNodeIP(t *testing.T) { az.EnableMultipleStandardLoadBalancers = true bi := newBackendPoolTypeNodeIP(az) - backendPool := network.BackendAddressPool{ - Name: pointer.String("kubernetes"), - BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{ - LoadBalancerBackendAddresses: &[]network.LoadBalancerBackendAddress{ - { - LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{ - IPAddress: pointer.String("10.0.0.1"), - }, - }, - }, - }, - } - expectedBackendPool := network.BackendAddressPool{ - Name: pointer.String("kubernetes"), - BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{ - VirtualNetwork: &network.SubResource{ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/vnet")}, - LoadBalancerBackendAddresses: &[]network.LoadBalancerBackendAddress{ - { - LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{ - IPAddress: pointer.String("10.0.0.1"), - }, - }, - { - Name: pointer.String("vmss-0"), - LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{ - IPAddress: pointer.String("10.0.0.2"), - }, - }, - }, - }, - } - - mockVMSet := NewMockVMSet(ctrl) - mockVMSet.EXPECT().GetNodeVMSetName(gomock.Any()).Return("vmss-0", nil) - mockVMSet.EXPECT().GetPrimaryVMSetName().Return("vmss-0") - az.VMSet = mockVMSet - - lbClient := mockloadbalancerclient.NewMockInterface(ctrl) - lbClient.EXPECT().CreateOrUpdateBackendPools(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) - az.LoadBalancerClient = lbClient - nodes := []*v1.Node{ { ObjectMeta: metav1.ObjectMeta{ @@ -105,15 +64,156 @@ func TestEnsureHostsInPoolNodeIP(t *testing.T) { Type: v1.NodeInternalIP, Address: "10.0.0.2", }, + { + Type: v1.NodeInternalIP, + Address: "2001::2", + }, }, }, }, } - service := getTestService("svc-1", v1.ProtocolTCP, nil, false, 80) - err := bi.EnsureHostsInPool(&service, nodes, "", "", "kubernetes", "kubernetes", backendPool) - assert.NoError(t, err) - assert.Equal(t, expectedBackendPool, backendPool) + testcases := []struct { + desc string + backendPool network.BackendAddressPool + expectedBackendPool network.BackendAddressPool + }{ + { + desc: "IPv4", + backendPool: network.BackendAddressPool{ + Name: pointer.String("kubernetes"), + BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{ + LoadBalancerBackendAddresses: &[]network.LoadBalancerBackendAddress{ + { + LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{ + IPAddress: pointer.String("10.0.0.1"), + }, + }, + }, + }, + }, + expectedBackendPool: network.BackendAddressPool{ + Name: pointer.String("kubernetes"), + BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{ + VirtualNetwork: &network.SubResource{ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/vnet")}, + LoadBalancerBackendAddresses: &[]network.LoadBalancerBackendAddress{ + { + LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{ + IPAddress: pointer.String("10.0.0.1"), + }, + }, + { + Name: pointer.String("vmss-0"), + LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{ + IPAddress: pointer.String("10.0.0.2"), + }, + }, + }, + }, + }, + }, + { + desc: "IPv6", + backendPool: network.BackendAddressPool{ + Name: pointer.String("kubernetes-IPv6"), + BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{ + LoadBalancerBackendAddresses: &[]network.LoadBalancerBackendAddress{ + { + LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{ + IPAddress: pointer.String("2001::1"), + }, + }, + }, + }, + }, + expectedBackendPool: network.BackendAddressPool{ + Name: pointer.String("kubernetes-IPv6"), + BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{ + VirtualNetwork: &network.SubResource{ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/vnet")}, + LoadBalancerBackendAddresses: &[]network.LoadBalancerBackendAddress{ + { + LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{ + IPAddress: pointer.String("2001::1"), + }, + }, + { + Name: pointer.String("vmss-0-IPv6"), + LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{ + IPAddress: pointer.String("2001::2"), + }, + }, + }, + }, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + mockVMSet := NewMockVMSet(ctrl) + mockVMSet.EXPECT().GetNodeVMSetName(gomock.Any()).Return("vmss-0", nil) + mockVMSet.EXPECT().GetPrimaryVMSetName().Return("vmss-0") + az.VMSet = mockVMSet + + lbClient := mockloadbalancerclient.NewMockInterface(ctrl) + lbClient.EXPECT().CreateOrUpdateBackendPools(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + az.LoadBalancerClient = lbClient + + service := getTestService("svc-1", v1.ProtocolTCP, nil, false, 80) + makeTestServiceDualStack(&service) + err := bi.EnsureHostsInPool(&service, nodes, "", "", "kubernetes", "kubernetes", tc.backendPool) + assert.NoError(t, err) + assert.Equal(t, tc.expectedBackendPool, tc.backendPool) + }) + } +} + +func TestIfLBBackendPoolsExist(t *testing.T) { + testcases := []struct { + desc string + lbBackendPoolNames map[bool]string + bpName *string + expectedFound bool + expectedIsIPv6 bool + }{ + { + desc: "IPv4 backendpool exists", + lbBackendPoolNames: map[bool]string{ + false: "bp", + true: "bp-IPv6", + }, + bpName: pointer.String("bp"), + expectedFound: true, + expectedIsIPv6: false, + }, + { + desc: "IPv6 backendpool exists", + lbBackendPoolNames: map[bool]string{ + false: "bp", + true: "bp-IPv6", + }, + bpName: pointer.String("bp-IPv6"), + expectedFound: true, + expectedIsIPv6: true, + }, + { + desc: "backendpool not exists", + lbBackendPoolNames: map[bool]string{ + false: "bp", + true: "bp-IPv6", + }, + bpName: pointer.String("bp0"), + expectedFound: false, + expectedIsIPv6: false, + }, + } + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + found, isIPv6 := ifLBBackendPoolsExist(tc.lbBackendPoolNames, tc.bpName) + assert.Equal(t, tc.expectedFound, found) + assert.Equal(t, tc.expectedIsIPv6, isIPv6) + }) + } } func TestCleanupVMSetFromBackendPoolByConditionNodeIPConfig(t *testing.T) { diff --git a/pkg/provider/azure_standard.go b/pkg/provider/azure_standard.go index ba9e1ce056..c4b19b772b 100644 --- a/pkg/provider/azure_standard.go +++ b/pkg/provider/azure_standard.go @@ -39,7 +39,6 @@ import ( "k8s.io/apimachinery/pkg/util/uuid" cloudprovider "k8s.io/cloud-provider" "k8s.io/klog/v2" - utilnet "k8s.io/utils/net" "k8s.io/utils/pointer" azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache" @@ -54,6 +53,9 @@ var ( nicIDRE = regexp.MustCompile(`(?i)/subscriptions/(?:.*)/resourceGroups/(.+)/providers/Microsoft.Network/networkInterfaces/(.+)/ipConfigurations/(?:.*)`) vmIDRE = regexp.MustCompile(`(?i)/subscriptions/(?:.*)/resourceGroups/(?:.*)/providers/Microsoft.Compute/virtualMachines/(.+)`) vmasIDRE = regexp.MustCompile(`/subscriptions/(?:.*)/resourceGroups/(?:.*)/providers/Microsoft.Compute/availabilitySets/(.+)`) + + v4Suffix = "IPv4" + v6Suffix = "IPv6" ) // returns the full identifier of an availabilitySet @@ -261,44 +263,51 @@ func isInternalLoadBalancer(lb *network.LoadBalancer) bool { // This means: // clusters moving from IPv4 to dualstack will require no changes // clusters moving from IPv6 to dualstack will require no changes as the IPv4 backend pool will created with -func getBackendPoolName(clusterName string, service *v1.Service) string { - IPv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) - if IPv6 { - return fmt.Sprintf("%v-IPv6", clusterName) +func getBackendPoolName(clusterName string, isIPv6 bool) string { + if isIPv6 { + return fmt.Sprintf("%s-%s", clusterName, v6Suffix) } return clusterName } -func (az *Cloud) getLoadBalancerRuleName(service *v1.Service, protocol v1.Protocol, port int32) string { +// ifBackendPoolIPv6 checks if a backend pool is of IPv6 according to name/ID. +func ifBackendPoolIPv6(name string) bool { + return strings.HasSuffix(name, "-IPv6") +} + +func (az *Cloud) getLoadBalancerRuleName(service *v1.Service, protocol v1.Protocol, port int32, isIPv6 bool) string { prefix := az.getRulePrefix(service) ruleName := fmt.Sprintf("%s-%s-%d", prefix, protocol, port) subnet := subnet(service) if subnet == nil { - return ruleName + return getResourceByIPFamily(ruleName, isIPv6) } // Load balancer rule name must be less or equal to 80 characters, so excluding the hyphen two segments cannot exceed 79 subnetSegment := *subnet - if len(ruleName)+len(subnetSegment)+1 > consts.LoadBalancerRuleNameMaxLength { - subnetSegment = subnetSegment[:consts.LoadBalancerRuleNameMaxLength-len(ruleName)-1] + maxLength := consts.LoadBalancerRuleNameMaxLength - consts.IPFamilySuffixLength + if len(ruleName)+len(subnetSegment)+1 > maxLength { + subnetSegment = subnetSegment[:maxLength-len(ruleName)-1] } - return fmt.Sprintf("%s-%s-%s-%d", prefix, subnetSegment, protocol, port) + return getResourceByIPFamily(fmt.Sprintf("%s-%s-%s-%d", prefix, subnetSegment, protocol, port), isIPv6) } -func (az *Cloud) getloadbalancerHAmodeRuleName(service *v1.Service) string { - return az.getLoadBalancerRuleName(service, service.Spec.Ports[0].Protocol, service.Spec.Ports[0].Port) +func (az *Cloud) getloadbalancerHAmodeRuleName(service *v1.Service, isIPv6 bool) string { + return az.getLoadBalancerRuleName(service, service.Spec.Ports[0].Protocol, service.Spec.Ports[0].Port, isIPv6) } -func (az *Cloud) getSecurityRuleName(service *v1.Service, port v1.ServicePort, sourceAddrPrefix string) string { +func (az *Cloud) getSecurityRuleName(service *v1.Service, port v1.ServicePort, sourceAddrPrefix string, isIPv6 bool) string { if useSharedSecurityRule(service) { safePrefix := strings.Replace(sourceAddrPrefix, "/", "_", -1) - return fmt.Sprintf("shared-%s-%d-%s", port.Protocol, port.Port, safePrefix) + name := fmt.Sprintf("shared-%s-%d-%s", port.Protocol, port.Port, safePrefix) + return getResourceByIPFamily(name, isIPv6) } safePrefix := strings.Replace(sourceAddrPrefix, "/", "_", -1) rulePrefix := az.getRulePrefix(service) - return fmt.Sprintf("%s-%s-%d-%s", rulePrefix, port.Protocol, port.Port, safePrefix) + name := fmt.Sprintf("%s-%s-%d-%s", rulePrefix, port.Protocol, port.Port, safePrefix) + return getResourceByIPFamily(name, isIPv6) } // This returns a human-readable version of the Service used to tag some resources. @@ -312,14 +321,15 @@ func (az *Cloud) getRulePrefix(service *v1.Service) string { return az.GetLoadBalancerName(context.TODO(), "", service) } -func (az *Cloud) getPublicIPName(clusterName string, service *v1.Service) string { +func (az *Cloud) getPublicIPName(clusterName string, service *v1.Service, isIPv6 bool) string { pipName := fmt.Sprintf("%s-%s", clusterName, az.GetLoadBalancerName(context.TODO(), clusterName, service)) - if prefixID, ok := service.Annotations[consts.ServiceAnnotationPIPPrefixID]; ok && prefixID != "" { - prefixName, err := getLastSegment(prefixID, "/") + pipName = getResourceByIPFamily(pipName, isIPv6) + if id := getServicePIPPrefixID(service, isIPv6); id != "" { + id, err := getLastSegment(id, "/") if err != nil { return pipName } - pipName = fmt.Sprintf("%s-%s", pipName, prefixName) + pipName = fmt.Sprintf("%s-%s", pipName, id) } return pipName } @@ -343,8 +353,8 @@ func (az *Cloud) serviceOwnsFrontendIP(fip network.FrontendIPConfiguration, serv return true, isPrimaryService, nil } - loadBalancerIP := getServiceLoadBalancerIP(service) - if loadBalancerIP == "" { + loadBalancerIPs := getServiceLoadBalancerIPs(service) + if len(loadBalancerIPs) == 0 { // it is a must that the secondary services set the loadBalancer IP return false, isPrimaryService, nil } @@ -352,24 +362,26 @@ func (az *Cloud) serviceOwnsFrontendIP(fip network.FrontendIPConfiguration, serv // for external secondary service the public IP address should be checked if !requiresInternalLoadBalancer(service) { pipResourceGroup := az.getPublicIPAddressResourceGroup(service) - pip, err := az.findMatchedPIPByLoadBalancerIP(service, loadBalancerIP, pipResourceGroup, pips) - if err != nil { - klog.Warningf("serviceOwnsFrontendIP: unexpected error when finding match public IP of the service %s with loadBalancerLP %s: %v", service.Name, loadBalancerIP, err) - return false, isPrimaryService, nil - } - - if pip != nil && - pip.ID != nil && - pip.PublicIPAddressPropertiesFormat != nil && - pip.IPAddress != nil && - fip.FrontendIPConfigurationPropertiesFormat != nil && - fip.FrontendIPConfigurationPropertiesFormat.PublicIPAddress != nil { - if strings.EqualFold(pointer.StringDeref(pip.ID, ""), pointer.StringDeref(fip.PublicIPAddress.ID, "")) { - klog.V(4).Infof("serviceOwnsFrontendIP: found secondary service %s of the frontend IP config %s", service.Name, *fip.Name) + for _, loadBalancerIP := range loadBalancerIPs { + pip, err := az.findMatchedPIPByLoadBalancerIP(service, loadBalancerIP, pipResourceGroup, pips) + if err != nil { + klog.Warningf("serviceOwnsFrontendIP: unexpected error when finding match public IP of the service %s with loadBalancerIP %s: %v", service.Name, loadBalancerIP, err) + return false, isPrimaryService, nil + } - return true, isPrimaryService, nil + if pip != nil && + pip.ID != nil && + pip.PublicIPAddressPropertiesFormat != nil && + pip.PublicIPAddressPropertiesFormat.IPAddress != nil && + fip.FrontendIPConfigurationPropertiesFormat != nil && + fip.FrontendIPConfigurationPropertiesFormat.PublicIPAddress != nil { + if strings.EqualFold(pointer.StringDeref(pip.ID, ""), pointer.StringDeref(fip.PublicIPAddress.ID, "")) { + klog.Infof("serviceOwnsFrontendIP: found secondary service %s of the frontend IP config %s", service.Name, *fip.Name) + return true, isPrimaryService, nil + } } - klog.V(4).Infof("serviceOwnsFrontendIP: the public IP with ID %s is being referenced by other service with public IP address %s", *pip.ID, *pip.IPAddress) + klog.Infof("serviceOwnsFrontendIP: the public IP with ID %s is being referenced by other service with public IP address %s "+ + "OR it is of incorrect IP version", *pip.ID, *pip.IPAddress) } return false, isPrimaryService, nil @@ -380,7 +392,14 @@ func (az *Cloud) serviceOwnsFrontendIP(fip network.FrontendIPConfiguration, serv return false, isPrimaryService, nil } - return strings.EqualFold(*fip.PrivateIPAddress, loadBalancerIP), isPrimaryService, nil + privateIPEquals := false + for _, loadBalancerIP := range loadBalancerIPs { + if strings.EqualFold(*fip.PrivateIPAddress, loadBalancerIP) { + privateIPEquals = true + break + } + } + return privateIPEquals, isPrimaryService, nil } func (az *Cloud) getDefaultFrontendIPConfigName(service *v1.Service) string { @@ -390,8 +409,9 @@ func (az *Cloud) getDefaultFrontendIPConfigName(service *v1.Service) string { ipcName := fmt.Sprintf("%s-%s", baseName, *subnetName) // Azure lb front end configuration name must not exceed 80 characters - if len(ipcName) > consts.FrontendIPConfigNameMaxLength { - ipcName = ipcName[:consts.FrontendIPConfigNameMaxLength] + maxLength := consts.FrontendIPConfigNameMaxLength - consts.IPFamilySuffixLength + if len(ipcName) > maxLength { + ipcName = ipcName[:maxLength] // Cutting the string may result in char like "-" as the string end. // If the last char is not a letter or '_', replace it with "_". if !unicode.IsLetter(rune(ipcName[len(ipcName)-1:][0])) && ipcName[len(ipcName)-1:] != "_" { @@ -929,7 +949,7 @@ func (as *availabilitySet) EnsureHostInPool(service *v1.Service, nodeName types. } var primaryIPConfig *network.InterfaceIPConfiguration - ipv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) + ipv6 := ifBackendPoolIPv6(backendPoolID) if !as.Cloud.ipv6DualStackEnabled && !ipv6 { primaryIPConfig, err = getPrimaryIPConfig(nic) if err != nil { diff --git a/pkg/provider/azure_standard_test.go b/pkg/provider/azure_standard_test.go index e59fa951f9..2454f4ae62 100644 --- a/pkg/provider/azure_standard_test.go +++ b/pkg/provider/azure_standard_test.go @@ -33,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" cloudprovider "k8s.io/cloud-provider" + utilnet "k8s.io/utils/net" "k8s.io/utils/pointer" "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/interfaceclient/mockinterfaceclient" @@ -376,22 +377,22 @@ func TestGetLoadBalancingRuleName(t *testing.T) { expected: "a257b965551374ad2b091ef3f07043ad-shortsubnet-TCP-9000", }, { - description: "internal standard lb should have subnet name on the rule name but truncated to 80 characters", + description: "internal standard lb should have subnet name on the rule name but truncated to 80 (-5) characters", subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet", isInternal: true, useStandardLB: true, protocol: v1.ProtocolTCP, port: 9000, - expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnngg-TCP-9000", + expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnn-TCP-9000", }, { - description: "internal basic lb should have subnet name on the rule name but truncated to 80 characters", + description: "internal basic lb should have subnet name on the rule name but truncated to 80 (-5) characters", subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet", isInternal: true, useStandardLB: false, protocol: v1.ProtocolTCP, port: 9000, - expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnngg-TCP-9000", + expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnn-TCP-9000", }, { description: "external standard lb should not have subnet name on the rule name", @@ -422,7 +423,8 @@ func TestGetLoadBalancingRuleName(t *testing.T) { svc.Annotations[consts.ServiceAnnotationLoadBalancerInternalSubnet] = c.subnetName svc.Annotations[consts.ServiceAnnotationLoadBalancerInternal] = strconv.FormatBool(c.isInternal) - loadbalancerRuleName := az.getLoadBalancerRuleName(svc, c.protocol, c.port) + isIPv6 := utilnet.IsIPv6String(svc.Spec.ClusterIP) + loadbalancerRuleName := az.getLoadBalancerRuleName(svc, c.protocol, c.port, isIPv6) assert.Equal(t, c.expected, loadbalancerRuleName, c.description) } } @@ -462,21 +464,21 @@ func TestGetFrontendIPConfigName(t *testing.T) { subnetName: "a--------------------------------------------------z", isInternal: true, useStandardLB: true, - expected: "a257b965551374ad2b091ef3f07043ad-a---------------------------------------------_", + expected: "a257b965551374ad2b091ef3f07043ad-a----------------------------------------_", }, { description: "internal standard lb should have subnet name on the frontend ip configuration name but truncated to 80 characters", subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet", isInternal: true, useStandardLB: true, - expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnnggggggggggg", + expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggg", }, { description: "internal basic lb should have subnet name on the frontend ip configuration name but truncated to 80 characters", subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet", isInternal: true, useStandardLB: false, - expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnnggggggggggg", + expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggg", }, { description: "external standard lb should not have subnet name on the frontend ip configuration name", @@ -902,26 +904,45 @@ func TestGetBackendPoolName(t *testing.T) { service v1.Service clusterName string expectedPoolName string + isIPv6 bool }{ { name: "GetBackendPoolName should return -IPv6", service: getTestService("test1", v1.ProtocolTCP, nil, true, 80), clusterName: "azure", expectedPoolName: "azure-IPv6", + isIPv6: true, }, { name: "GetBackendPoolName should return ", service: getTestService("test1", v1.ProtocolTCP, nil, false, 80), clusterName: "azure", expectedPoolName: "azure", + isIPv6: false, }, } for _, test := range testcases { - backPoolName := getBackendPoolName(test.clusterName, &test.service) + backPoolName := getBackendPoolName(test.clusterName, test.isIPv6) assert.Equal(t, test.expectedPoolName, backPoolName, test.name) } } +func TestIfBackendPoolIPv6(t *testing.T) { + testcases := []struct { + name string + expectedIsIPv6 bool + }{ + {"bp-IPv6", true}, + {"bp-IPv4", false}, + {"bp", false}, + {"bp-ipv6", false}, + } + for _, test := range testcases { + isIPv6 := ifBackendPoolIPv6(test.name) + assert.Equal(t, test.expectedIsIPv6, isIPv6) + } +} + func TestGetStandardInstanceIDByNodeName(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -1403,15 +1424,6 @@ func TestStandardEnsureHostInPool(t *testing.T) { nicProvisionState: consts.NicFailedState, vmSetName: "myAvailabilitySet", }, - { - name: "EnsureHostInPool should report error if service.Spec.ClusterIP is ipv6 but node don't have IPv6 address", - service: &v1.Service{Spec: v1.ServiceSpec{ClusterIP: "2001:0db8:85a3:0000:0000:8a2e:0370:7334"}}, - nodeName: "vm4", - nicName: "nic4", - nicID: "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/nic4", - vmSetName: "myAvailabilitySet", - expectedErrMsg: fmt.Errorf("failed to determine the ipconfig(IPv6=true). nicname=%q", "nic4"), - }, { name: "EnsureHostInPool should return nil if there is matched backend pool", service: &v1.Service{}, @@ -1571,64 +1583,39 @@ func TestStandardEnsureHostsInPool(t *testing.T) { nicID: "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/nic4", vmSetName: "availabilityset-1", }, - { - name: "EnsureHostsInPool should report error if service.Spec.ClusterIP is ipv6 but node don't have IPv6 address", - service: &v1.Service{ - ObjectMeta: meta.ObjectMeta{ - Name: "svc", - Namespace: "default", - }, - Spec: v1.ServiceSpec{ - ClusterIP: "2001:0db8:85a3:0000:0000:8a2e:0370:7334", - }, - }, - nodeName: "vm5", - nodes: []*v1.Node{ - { - ObjectMeta: meta.ObjectMeta{ - Name: "vm5", - }, - }, - }, - nicName: "nic5", - backendPoolID: backendAddressPoolID, - nicID: "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/nic5", - vmSetName: "myAvailabilitySet", - expectedErr: true, - expectedErrMsg: fmt.Errorf("ensure(default/svc): backendPoolID(%s) - failed to ensure host in pool: %w", backendAddressPoolID, fmt.Errorf("failed to determine the ipconfig(IPv6=true). nicname=%q", "nic5")), - }, } for _, test := range testCases { - cloud.Config.LoadBalancerSku = consts.LoadBalancerSkuStandard - cloud.Config.ExcludeMasterFromStandardLB = pointer.Bool(true) - cloud.excludeLoadBalancerNodes = sets.NewString(test.excludeLBNodes...) + t.Run(test.name, func(t *testing.T) { + cloud.Config.LoadBalancerSku = consts.LoadBalancerSkuStandard + cloud.Config.ExcludeMasterFromStandardLB = pointer.Bool(true) + cloud.excludeLoadBalancerNodes = sets.NewString(test.excludeLBNodes...) - testVM := buildDefaultTestVirtualMachine(availabilitySetID, []string{test.nicID}) - testNIC := buildDefaultTestInterface(false, []string{backendAddressPoolID}) - testNIC.Name = pointer.String(test.nicName) - testNIC.ID = pointer.String(test.nicID) + testVM := buildDefaultTestVirtualMachine(availabilitySetID, []string{test.nicID}) + testNIC := buildDefaultTestInterface(false, []string{backendAddressPoolID}) + testNIC.Name = pointer.String(test.nicName) + testNIC.ID = pointer.String(test.nicID) - mockVMClient := cloud.VirtualMachinesClient.(*mockvmclient.MockInterface) - mockVMClient.EXPECT().Get(gomock.Any(), cloud.ResourceGroup, test.nodeName, gomock.Any()).Return(testVM, nil).AnyTimes() + mockVMClient := cloud.VirtualMachinesClient.(*mockvmclient.MockInterface) + mockVMClient.EXPECT().Get(gomock.Any(), cloud.ResourceGroup, test.nodeName, gomock.Any()).Return(testVM, nil).AnyTimes() - mockInterfaceClient := cloud.InterfacesClient.(*mockinterfaceclient.MockInterface) - mockInterfaceClient.EXPECT().Get(gomock.Any(), cloud.ResourceGroup, test.nicName, gomock.Any()).Return(testNIC, nil).AnyTimes() - mockInterfaceClient.EXPECT().CreateOrUpdate(gomock.Any(), cloud.ResourceGroup, gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + mockInterfaceClient := cloud.InterfacesClient.(*mockinterfaceclient.MockInterface) + mockInterfaceClient.EXPECT().Get(gomock.Any(), cloud.ResourceGroup, test.nicName, gomock.Any()).Return(testNIC, nil).AnyTimes() + mockInterfaceClient.EXPECT().CreateOrUpdate(gomock.Any(), cloud.ResourceGroup, gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - err := cloud.VMSet.EnsureHostsInPool(test.service, test.nodes, test.backendPoolID, test.vmSetName) - if test.expectedErr { - assert.EqualError(t, test.expectedErrMsg, err.Error(), test.name) - } else { - assert.Nil(t, err, test.name) - } + err := cloud.VMSet.EnsureHostsInPool(test.service, test.nodes, test.backendPoolID, test.vmSetName) + if test.expectedErr { + assert.EqualError(t, test.expectedErrMsg, err.Error(), test.name) + } else { + assert.Nil(t, err, test.name) + } + }) } } func TestServiceOwnsFrontendIP(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - cloud := GetTestCloud(ctrl) testCases := []struct { desc string @@ -1735,8 +1722,10 @@ func TestServiceOwnsFrontendIP(t *testing.T) { }, service: &v1.Service{ ObjectMeta: meta.ObjectMeta{ - UID: types.UID("secondary"), - Annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIPDualStack[false]: "4.3.2.1"}, + UID: types.UID("secondary"), + Annotations: map[string]string{ + consts.ServiceAnnotationLoadBalancerIPDualStack[false]: "4.3.2.1", + }, }, }, }, @@ -1760,12 +1749,56 @@ func TestServiceOwnsFrontendIP(t *testing.T) { }, service: &v1.Service{ ObjectMeta: meta.ObjectMeta{ - UID: types.UID("secondary"), - Annotations: map[string]string{consts.ServiceAnnotationLoadBalancerIPDualStack[false]: "4.3.2.1"}, + UID: types.UID("secondary"), + Annotations: map[string]string{ + consts.ServiceAnnotationLoadBalancerIPDualStack[false]: "4.3.2.1", + consts.ServiceAnnotationLoadBalancerIPDualStack[true]: "fd00::eef0", + }, }, }, isOwned: true, }, + { + desc: "serviceOwnsFrontendIP should detect the secondary external service dual-stack", + existingPIPs: []network.PublicIPAddress{ + { + ID: pointer.String("pip"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv4, + IPAddress: pointer.String("4.3.2.1"), + }, + }, + { + ID: pointer.String("pip1"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv6, + IPAddress: pointer.String("fd00::eef0"), + }, + }, + }, + fip: network.FrontendIPConfiguration{ + Name: pointer.String("auid"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv6, + }, + ID: pointer.String("pip1"), + }, + }, + }, + service: &v1.Service{ + ObjectMeta: meta.ObjectMeta{ + UID: types.UID("secondary"), + Annotations: map[string]string{ + consts.ServiceAnnotationLoadBalancerIPDualStack[false]: "4.3.2.1", + consts.ServiceAnnotationLoadBalancerIPDualStack[true]: "fd00::eef0", + }, + }, + }, + isOwned: true, + isPrimary: false, + }, { desc: "serviceOwnsFrontendIP should detect the secondary internal service", fip: network.FrontendIPConfiguration{ @@ -1785,13 +1818,36 @@ func TestServiceOwnsFrontendIP(t *testing.T) { }, isOwned: true, }, + { + desc: "serviceOwnsFrontendIP should detect the secondary internal service - dualstack", + fip: network.FrontendIPConfiguration{ + Name: pointer.String("auid"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PrivateIPAddress: pointer.String("fd00::eef0"), + }, + }, + service: &v1.Service{ + ObjectMeta: meta.ObjectMeta{ + UID: types.UID("secondary"), + Annotations: map[string]string{ + consts.ServiceAnnotationLoadBalancerInternal: "true", + consts.ServiceAnnotationLoadBalancerIPDualStack[false]: "4.3.2.1", + consts.ServiceAnnotationLoadBalancerIPDualStack[true]: "fd00::eef0", + }, + }, + }, + isOwned: true, + }, } for _, test := range testCases { - isOwned, isPrimary, err := cloud.serviceOwnsFrontendIP(test.fip, test.service, &test.existingPIPs) - assert.Equal(t, test.expectedErr, err, test.desc) - assert.Equal(t, test.isOwned, isOwned, test.desc) - assert.Equal(t, test.isPrimary, isPrimary, test.desc) + t.Run(test.desc, func(t *testing.T) { + cloud := GetTestCloud(ctrl) + isOwned, isPrimary, err := cloud.serviceOwnsFrontendIP(test.fip, test.service, &test.existingPIPs) + assert.Equal(t, test.expectedErr, err) + assert.Equal(t, test.isOwned, isOwned) + assert.Equal(t, test.isPrimary, isPrimary) + }) } } diff --git a/pkg/provider/azure_test.go b/pkg/provider/azure_test.go index 19d081d44b..9e82cb27f1 100644 --- a/pkg/provider/azure_test.go +++ b/pkg/provider/azure_test.go @@ -39,6 +39,7 @@ import ( cloudprovider "k8s.io/cloud-provider" cloudproviderapi "k8s.io/cloud-provider/api" servicehelpers "k8s.io/cloud-provider/service/helpers" + utilnet "k8s.io/utils/net" "k8s.io/utils/pointer" "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/interfaceclient/mockinterfaceclient" @@ -727,7 +728,8 @@ func TestReconcileSecurityGroupFromAnyDestinationAddressPrefixToLoadBalancerIP(t if err != nil { t.Errorf("Unexpected error: %q", err) } - sg, err = az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1)), nil, true) + isIPv6 := utilnet.IsIPv6String(svc1.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error: %q", err) } @@ -1662,6 +1664,12 @@ func getBackendPort(port int32) int32 { return port + 10000 } +// TODO: This function should be merged into getTestService() +func makeTestServiceDualStack(svc *v1.Service) { + svc.Spec.ClusterIPs = []string{"10.0.0.2", "fd00::1907"} + svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} +} + func getTestService(identifier string, proto v1.Protocol, annotations map[string]string, isIPv6 bool, requestedPorts ...int32) v1.Service { ports := []v1.ServicePort{} for _, port := range requestedPorts { @@ -1689,8 +1697,10 @@ func getTestService(identifier string, proto v1.Protocol, annotations map[string } svc.Spec.ClusterIP = "10.0.0.2" + svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv4Protocol} if isIPv6 { svc.Spec.ClusterIP = "fd00::1907" + svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv6Protocol} } return svc @@ -1740,7 +1750,8 @@ func getTestSecurityGroup(az *Cloud, services ...v1.Service) *network.SecurityGr for _, port := range service.Spec.Ports { sources := getServiceSourceRanges(&services[i]) for _, src := range sources { - ruleName := az.getSecurityRuleName(&services[i], port, src) + isIPv6 := utilnet.IsIPv6String(services[i].Spec.ClusterIP) + ruleName := az.getSecurityRuleName(&services[i], port, src, isIPv6) rules = append(rules, network.SecurityRule{ Name: pointer.String(ruleName), SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{ @@ -1790,7 +1801,8 @@ func validateLoadBalancer(t *testing.T, loadBalancer *network.LoadBalancer, serv } for _, wantedRule := range svc.Spec.Ports { expectedRuleCount++ - wantedRuleName := az.getLoadBalancerRuleName(&services[i], wantedRule.Protocol, wantedRule.Port) + isIPv6 := utilnet.IsIPv6String(services[i].Spec.ClusterIP) + wantedRuleName := az.getLoadBalancerRuleName(&services[i], wantedRule.Protocol, wantedRule.Port, isIPv6) foundRule := false for _, actualRule := range *loadBalancer.LoadBalancingRules { if strings.EqualFold(*actualRule.Name, wantedRuleName) && @@ -1813,7 +1825,8 @@ func validateLoadBalancer(t *testing.T, loadBalancer *network.LoadBalancer, serv foundProbe := false if servicehelpers.NeedsHealthCheck(&services[i]) { path, port := servicehelpers.GetServiceHealthCheckPathPort(&services[i]) - wantedRuleName := az.getLoadBalancerRuleName(&services[i], v1.ProtocolTCP, port) + isIPv6 := utilnet.IsIPv6String(services[i].Spec.ClusterIP) + wantedRuleName := az.getLoadBalancerRuleName(&services[i], v1.ProtocolTCP, port, isIPv6) for _, actualProbe := range *loadBalancer.Probes { if strings.EqualFold(*actualProbe.Name, wantedRuleName) && *actualProbe.Port == port && @@ -2001,12 +2014,13 @@ func validateSecurityGroup(t *testing.T, securityGroup *network.SecurityGroup, s for _, wantedRule := range svc.Spec.Ports { sources := getServiceSourceRanges(&services[i]) for _, source := range sources { - wantedRuleName := az.getSecurityRuleName(&services[i], wantedRule, source) + isIPv6 := utilnet.IsIPv6String(services[i].Spec.ClusterIP) + wantedRuleName := az.getSecurityRuleName(&services[i], wantedRule, source, isIPv6) seenRules[wantedRuleName] = wantedRuleName foundRule := false for _, actualRule := range *securityGroup.SecurityRules { if strings.EqualFold(*actualRule.Name, wantedRuleName) { - err := securityRuleMatches(source, wantedRule, getServiceLoadBalancerIP(&svc), actualRule) + err := securityRuleMatches(source, wantedRule, getServiceLoadBalancerIP(&svc, isIPv6), actualRule) if err != nil { t.Errorf("Found matching security rule %q but properties were incorrect: %v", wantedRuleName, err) } @@ -2452,7 +2466,8 @@ func TestIfServiceSpecifiesSharedRuleAndRuleDoesNotExistItIsCreated(t *testing.T sg := getTestSecurityGroup(az) setMockSecurityGroup(az, ctrl, sg) - sg, err := az.reconcileSecurityGroup(testClusterName, &svc, pointer.String(getServiceLoadBalancerIP(&svc)), nil, true) + isIPv6 := utilnet.IsIPv6String(svc.Spec.ClusterIP) + sg, err := az.reconcileSecurityGroup(testClusterName, &svc, pointer.String(getServiceLoadBalancerIP(&svc, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error: %q", err) } @@ -2511,7 +2526,8 @@ func TestIfServiceSpecifiesSharedRuleAndRuleExistsThenTheServicesPortAndAddressA } setMockSecurityGroup(az, ctrl, sg) - sg, err := az.reconcileSecurityGroup(testClusterName, &svc, pointer.String(getServiceLoadBalancerIP(&svc)), nil, true) + isIPv6 := utilnet.IsIPv6String(svc.Spec.ClusterIP) + sg, err := az.reconcileSecurityGroup(testClusterName, &svc, pointer.String(getServiceLoadBalancerIP(&svc, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error: %q", err) } @@ -2555,13 +2571,15 @@ func TestIfServicesSpecifySharedRuleButDifferentPortsThenSeparateRulesAreCreated sg := getTestSecurityGroup(az) setMockSecurityGroup(az, ctrl, sg) - sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1)), nil, true) + isIPv6 := utilnet.IsIPv6String(svc1.Spec.ClusterIP) + sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc1: %q", err) } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2)), nil, true) + isIPv6 = utilnet.IsIPv6String(svc2.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc2: %q", err) } @@ -2628,13 +2646,15 @@ func TestIfServicesSpecifySharedRuleButDifferentProtocolsThenSeparateRulesAreCre sg := getTestSecurityGroup(az) setMockSecurityGroup(az, ctrl, sg) - sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1)), nil, true) + isIPv6 := utilnet.IsIPv6String(svc1.Spec.ClusterIP) + sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc1: %q", err) } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2)), nil, true) + isIPv6 = utilnet.IsIPv6String(svc2.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc2: %q", err) } @@ -2701,13 +2721,15 @@ func TestIfServicesSpecifySharedRuleButDifferentSourceAddressesThenSeparateRules sg := getTestSecurityGroup(az) setMockSecurityGroup(az, ctrl, sg) - sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1)), nil, true) + isIPv6 := utilnet.IsIPv6String(svc1.Spec.ClusterIP) + sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc1: %q", err) } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2)), nil, true) + isIPv6 = utilnet.IsIPv6String(svc2.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc2: %q", err) } @@ -2776,19 +2798,22 @@ func TestIfServicesSpecifySharedRuleButSomeAreOnDifferentPortsThenRulesAreSepara sg := getTestSecurityGroup(az) setMockSecurityGroup(az, ctrl, sg) - sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1)), nil, true) + isIPv6 := utilnet.IsIPv6String(svc1.Spec.ClusterIP) + sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc1: %q", err) } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2)), nil, true) + isIPv6 = utilnet.IsIPv6String(svc2.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc2: %q", err) } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc3, pointer.String(getServiceLoadBalancerIP(&svc3)), nil, true) + isIPv6 = utilnet.IsIPv6String(svc3.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc3, pointer.String(getServiceLoadBalancerIP(&svc3, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc3: %q", err) } @@ -2876,13 +2901,15 @@ func TestIfServiceSpecifiesSharedRuleAndServiceIsDeletedThenTheServicesPortAndAd sg := getTestSecurityGroup(az) setMockSecurityGroup(az, ctrl, sg) - sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1)), nil, true) + isIPv6 := utilnet.IsIPv6String(svc1.Spec.ClusterIP) + sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc1: %q", err) } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2)), nil, true) + isIPv6 = utilnet.IsIPv6String(svc2.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc2: %q", err) } @@ -2890,7 +2917,8 @@ func TestIfServiceSpecifiesSharedRuleAndServiceIsDeletedThenTheServicesPortAndAd validateSecurityGroup(t, sg, svc1, svc2) setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1)), nil, false) + isIPv6 = utilnet.IsIPv6String(svc1.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1, isIPv6)), nil, false) if err != nil { t.Errorf("Unexpected error removing svc1: %q", err) } @@ -2939,19 +2967,22 @@ func TestIfSomeServicesShareARuleAndOneIsDeletedItIsRemovedFromTheRightRule(t *t sg := getTestSecurityGroup(az) setMockSecurityGroup(az, ctrl, sg) - sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1)), nil, true) + isIPv6 := utilnet.IsIPv6String(svc1.Spec.ClusterIP) + sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc1: %q", err) } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2)), nil, true) + isIPv6 = utilnet.IsIPv6String(svc2.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc2: %q", err) } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc3, pointer.String(getServiceLoadBalancerIP(&svc3)), nil, true) + isIPv6 = utilnet.IsIPv6String(svc3.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc3, pointer.String(getServiceLoadBalancerIP(&svc3, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc3: %q", err) } @@ -2959,7 +2990,8 @@ func TestIfSomeServicesShareARuleAndOneIsDeletedItIsRemovedFromTheRightRule(t *t validateSecurityGroup(t, sg, svc1, svc2, svc3) setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1)), nil, false) + isIPv6 = utilnet.IsIPv6String(svc1.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1, isIPv6)), nil, false) if err != nil { t.Errorf("Unexpected error removing svc1: %q", err) } @@ -3050,19 +3082,22 @@ func TestIfServiceSpecifiesSharedRuleAndLastServiceIsDeletedThenRuleIsDeleted(t sg := getTestSecurityGroup(az) setMockSecurityGroup(az, ctrl, sg) - sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1)), nil, true) + isIPv6 := utilnet.IsIPv6String(svc1.Spec.ClusterIP) + sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc1: %q", err) } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2)), nil, true) + isIPv6 = utilnet.IsIPv6String(svc2.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc2, pointer.String(getServiceLoadBalancerIP(&svc2, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc2: %q", err) } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc3, pointer.String(getServiceLoadBalancerIP(&svc3)), nil, true) + isIPv6 = utilnet.IsIPv6String(svc3.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc3, pointer.String(getServiceLoadBalancerIP(&svc3, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc3: %q", err) } @@ -3070,13 +3105,15 @@ func TestIfServiceSpecifiesSharedRuleAndLastServiceIsDeletedThenRuleIsDeleted(t validateSecurityGroup(t, sg, svc1, svc2, svc3) setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1)), nil, false) + isIPv6 = utilnet.IsIPv6String(svc1.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1, isIPv6)), nil, false) if err != nil { t.Errorf("Unexpected error removing svc1: %q", err) } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc3, pointer.String(getServiceLoadBalancerIP(&svc3)), nil, false) + isIPv6 = utilnet.IsIPv6String(svc3.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc3, pointer.String(getServiceLoadBalancerIP(&svc3, isIPv6)), nil, false) if err != nil { t.Errorf("Unexpected error removing svc3: %q", err) } @@ -3143,15 +3180,18 @@ func TestCanCombineSharedAndPrivateRulesInSameGroup(t *testing.T) { testServices := []v1.Service{svc1, svc2, svc3, svc4, svc5} testRuleName23 := testRuleName2 - expectedRuleName4 := az.getSecurityRuleName(&svc4, v1.ServicePort{Port: 4444, Protocol: v1.ProtocolTCP}, "Internet") - expectedRuleName5 := az.getSecurityRuleName(&svc5, v1.ServicePort{Port: 8888, Protocol: v1.ProtocolTCP}, "Internet") + isIPv6 := utilnet.IsIPv6String(svc4.Spec.ClusterIP) + expectedRuleName4 := az.getSecurityRuleName(&svc4, v1.ServicePort{Port: 4444, Protocol: v1.ProtocolTCP}, "Internet", isIPv6) + isIPv6 = utilnet.IsIPv6String(svc5.Spec.ClusterIP) + expectedRuleName5 := az.getSecurityRuleName(&svc5, v1.ServicePort{Port: 8888, Protocol: v1.ProtocolTCP}, "Internet", isIPv6) sg := getTestSecurityGroup(az) for i, svc := range testServices { svc := svc setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &testServices[i], pointer.String(getServiceLoadBalancerIP(&svc)), nil, true) + isIPv6 := utilnet.IsIPv6String(svc.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &testServices[i], pointer.String(getServiceLoadBalancerIP(&svc, isIPv6)), nil, true) if err != nil { t.Errorf("Unexpected error adding svc%d: %q", i+1, err) } @@ -3226,8 +3266,9 @@ func TestCanCombineSharedAndPrivateRulesInSameGroup(t *testing.T) { if securityRule4.DestinationAddressPrefix == nil { t.Errorf("Expected unshared rule %s to have a destination IP address", expectedRuleName4) } else { - if !strings.EqualFold(*securityRule4.DestinationAddressPrefix, getServiceLoadBalancerIP(&svc4)) { - t.Errorf("Expected unshared rule %s to have a destination %s but had %s", expectedRuleName4, getServiceLoadBalancerIP(&svc4), *securityRule4.DestinationAddressPrefix) + isIPv6 := utilnet.IsIPv6String(svc4.Spec.ClusterIP) + if !strings.EqualFold(*securityRule4.DestinationAddressPrefix, getServiceLoadBalancerIP(&svc4, isIPv6)) { + t.Errorf("Expected unshared rule %s to have a destination %s but had %s", expectedRuleName4, getServiceLoadBalancerIP(&svc4, isIPv6), *securityRule4.DestinationAddressPrefix) } } @@ -3238,19 +3279,22 @@ func TestCanCombineSharedAndPrivateRulesInSameGroup(t *testing.T) { if securityRule5.DestinationAddressPrefix == nil { t.Errorf("Expected unshared rule %s to have a destination IP address", expectedRuleName5) } else { - if !strings.EqualFold(*securityRule5.DestinationAddressPrefix, getServiceLoadBalancerIP(&svc5)) { - t.Errorf("Expected unshared rule %s to have a destination %s but had %s", expectedRuleName5, getServiceLoadBalancerIP(&svc5), *securityRule5.DestinationAddressPrefix) + isIPv6 := utilnet.IsIPv6String(svc5.Spec.ClusterIP) + if !strings.EqualFold(*securityRule5.DestinationAddressPrefix, getServiceLoadBalancerIP(&svc5, isIPv6)) { + t.Errorf("Expected unshared rule %s to have a destination %s but had %s", expectedRuleName5, getServiceLoadBalancerIP(&svc5, isIPv6), *securityRule5.DestinationAddressPrefix) } } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1)), nil, false) + isIPv6 = utilnet.IsIPv6String(svc1.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(getServiceLoadBalancerIP(&svc1, isIPv6)), nil, false) if err != nil { t.Errorf("Unexpected error removing svc1: %q", err) } setMockSecurityGroup(az, ctrl, sg) - sg, err = az.reconcileSecurityGroup(testClusterName, &svc5, pointer.String(getServiceLoadBalancerIP(&svc5)), nil, false) + isIPv6 = utilnet.IsIPv6String(svc5.Spec.ClusterIP) + sg, err = az.reconcileSecurityGroup(testClusterName, &svc5, pointer.String(getServiceLoadBalancerIP(&svc5, isIPv6)), nil, false) if err != nil { t.Errorf("Unexpected error removing svc5: %q", err) } diff --git a/pkg/provider/azure_utils.go b/pkg/provider/azure_utils.go index 27de6f4b2d..e07ba0198f 100644 --- a/pkg/provider/azure_utils.go +++ b/pkg/provider/azure_utils.go @@ -227,11 +227,10 @@ func getServiceAdditionalPublicIPs(service *v1.Service) ([]string, error) { return result, nil } -func getNodePrivateIPAddress(service *v1.Service, node *v1.Node) string { - isIPV6SVC := utilnet.IsIPv6String(service.Spec.ClusterIP) +func getNodePrivateIPAddress(service *v1.Service, node *v1.Node, isIPv6 bool) string { for _, nodeAddress := range node.Status.Addresses { if strings.EqualFold(string(nodeAddress.Type), string(v1.NodeInternalIP)) && - utilnet.IsIPv6String(nodeAddress.Address) == isIPV6SVC { + utilnet.IsIPv6String(nodeAddress.Address) == isIPv6 { klog.V(6).Infof("getNodePrivateIPAddress: node %s, ip %s", node.Name, nodeAddress.Address) return nodeAddress.Address } @@ -342,3 +341,143 @@ func extractVmssVMName(name string) (string, string, error) { instanceID := split[len(split)-1] return ssName, instanceID, nil } + +// getIPFamiliesEnabled checks if IPv4, IPv6 are enabled according to svc.Spec.IPFamilies. +func getIPFamiliesEnabled(svc *v1.Service) (v4Enabled bool, v6Enabled bool) { + for _, ipFamily := range svc.Spec.IPFamilies { + if ipFamily == v1.IPv4Protocol { + v4Enabled = true + } else { + v6Enabled = true + } + } + return +} + +// getServiceLoadBalancerIP retrieves LB IP from IPv4 annotation, then IPv6 annotation, then service.Spec.LoadBalancerIP. +func getServiceLoadBalancerIP(service *v1.Service, isIPv6 bool) string { + if service == nil { + return "" + } + + if ip, ok := service.Annotations[consts.ServiceAnnotationLoadBalancerIPDualStack[isIPv6]]; ok && ip != "" { + return ip + } + + // Retrieve LB IP from service.Spec.LoadBalancerIP (will be deprecated) + svcLBIP := service.Spec.LoadBalancerIP + if (net.ParseIP(svcLBIP).To4() != nil && !isIPv6) || + (net.ParseIP(svcLBIP).To4() == nil && isIPv6) { + return svcLBIP + } + return "" +} + +func getServiceLoadBalancerIPs(service *v1.Service) []string { + if service == nil { + return []string{} + } + + ips := []string{} + if ip, ok := service.Annotations[consts.ServiceAnnotationLoadBalancerIPDualStack[false]]; ok && ip != "" { + ips = append(ips, ip) + } + if ip, ok := service.Annotations[consts.ServiceAnnotationLoadBalancerIPDualStack[true]]; ok && ip != "" { + ips = append(ips, ip) + } + if len(ips) != 0 { + return ips + } + + lbIP := service.Spec.LoadBalancerIP + if lbIP != "" { + ips = append(ips, lbIP) + } + + return ips +} + +// setServiceLoadBalancerIP sets LB IP to a Service +func setServiceLoadBalancerIP(service *v1.Service, ip string) { + if service.Annotations == nil { + service.Annotations = map[string]string{} + } + isIPv6 := net.ParseIP(ip).To4() == nil + service.Annotations[consts.ServiceAnnotationLoadBalancerIPDualStack[isIPv6]] = ip +} + +func getServicePIPName(service *v1.Service, isIPv6 bool) string { + if service == nil { + return "" + } + + if consts.DualstackSupported { + if name, ok := service.Annotations[consts.ServiceAnnotationPIPNameDualStack[isIPv6]]; ok && name != "" { + return name + } + } + return service.Annotations[consts.ServiceAnnotationPIPName] +} + +func getServicePIPPrefixID(service *v1.Service, isIPv6 bool) string { + if service == nil { + return "" + } + + if consts.DualstackSupported { + if name, ok := service.Annotations[consts.ServiceAnnotationPIPPrefixIDDualStack[isIPv6]]; ok && name != "" { + return name + } + } + return service.Annotations[consts.ServiceAnnotationPIPPrefixID] +} + +func getResourceByIPFamily(resource string, isIPv6 bool) string { + if !consts.DualstackSupported { + return resource + } + + if isIPv6 { + return fmt.Sprintf("%s-%s", resource, v6Suffix) + } + return fmt.Sprintf("%s-%s", resource, v4Suffix) +} + +// ifFIPIPv6 checks if the frontend IP configuration is of IPv6. +func (az *Cloud) ifFIPIPv6(fip *network.FrontendIPConfiguration, pips *[]network.PublicIPAddress, isInternal bool) (isIPv6 bool, err error) { + if err := az.ensurePIP(az.ResourceGroup, pips); err != nil { + return false, fmt.Errorf("failed to ensure PIP is refreshed: %w", err) + } + if isInternal { + if fip.FrontendIPConfigurationPropertiesFormat != nil { + if fip.FrontendIPConfigurationPropertiesFormat.PrivateIPAddressVersion != "" { + return fip.FrontendIPConfigurationPropertiesFormat.PrivateIPAddressVersion == network.IPv6, nil + } + return net.ParseIP(pointer.StringDeref(fip.FrontendIPConfigurationPropertiesFormat.PrivateIPAddress, "")).To4() == nil, nil + } + klog.Errorf("Checking IP Family of frontend IP configuration %q of internal Service but its"+ + " FrontendIPConfigurationPropertiesFormat is nil. It's considered to be IPv4", + pointer.StringDeref(fip.Name, "")) + return + } + var fipPIPID string + if fip.FrontendIPConfigurationPropertiesFormat != nil && fip.FrontendIPConfigurationPropertiesFormat.PublicIPAddress != nil { + fipPIPID = pointer.StringDeref(fip.FrontendIPConfigurationPropertiesFormat.PublicIPAddress.ID, "") + } + for _, pip := range *pips { + id := pointer.StringDeref(pip.ID, "") + if fipPIPID != id { + continue + } + if pip.PublicIPAddressPropertiesFormat != nil { + // First check PublicIPAddressVersion, then IPAddress + if pip.PublicIPAddressPropertiesFormat.PublicIPAddressVersion == network.IPv6 || + net.ParseIP(pointer.StringDeref(pip.PublicIPAddressPropertiesFormat.IPAddress, "")).To4() == nil { + isIPv6 = true + break + } + } + break + } + return isIPv6, nil +} diff --git a/pkg/provider/azure_utils_test.go b/pkg/provider/azure_utils_test.go index afb53b65b6..98762ec04a 100644 --- a/pkg/provider/azure_utils_test.go +++ b/pkg/provider/azure_utils_test.go @@ -23,6 +23,7 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-07-01/network" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" @@ -448,3 +449,277 @@ func TestExtractVmssVMName(t *testing.T) { assert.Equal(t, c.expectedInstanceID, instanceID, c.description) } } + +func TestGetIPFamiliesEnabled(t *testing.T) { + testcases := []struct { + desc string + svc *v1.Service + ExpectedV4Enabled bool + ExpectedV6Enabled bool + }{ + { + "IPv4", + &v1.Service{ + Spec: v1.ServiceSpec{IPFamilies: []v1.IPFamily{v1.IPv4Protocol}}, + }, + true, + false, + }, + { + "DualStack", + &v1.Service{ + Spec: v1.ServiceSpec{IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}}, + }, + true, + true, + }, + } + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + v4Enabled, v6Enabled := getIPFamiliesEnabled(tc.svc) + assert.Equal(t, tc.ExpectedV4Enabled, v4Enabled) + assert.Equal(t, tc.ExpectedV6Enabled, v6Enabled) + }) + } +} + +func TestGetServiceLoadBalancerIP(t *testing.T) { + testcases := []struct { + desc string + svc *v1.Service + isIPv6 bool + expectedIP string + }{ + { + "IPv6", + &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + consts.ServiceAnnotationLoadBalancerIPDualStack[false]: "10.0.0.1", + consts.ServiceAnnotationLoadBalancerIPDualStack[true]: "2001::1", + }, + }, + Spec: v1.ServiceSpec{ + LoadBalancerIP: "10.0.0.2", + }, + }, + true, + "2001::1", + }, + { + "IPv4 but from LoadBalancerIP", + &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + Spec: v1.ServiceSpec{ + LoadBalancerIP: "10.0.0.2", + }, + }, + false, + "10.0.0.2", + }, + } + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + ip := getServiceLoadBalancerIP(tc.svc, tc.isIPv6) + assert.Equal(t, tc.expectedIP, ip) + }) + } +} + +func TestGetServiceLoadBalancerIPs(t *testing.T) { + testcases := []struct { + desc string + svc *v1.Service + expectedIPs []string + }{ + { + "Get IPv4 and IPv6 IPs", + &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + consts.ServiceAnnotationLoadBalancerIPDualStack[false]: "10.0.0.1", + consts.ServiceAnnotationLoadBalancerIPDualStack[true]: "2001::1", + }, + }, + }, + []string{"10.0.0.1", "2001::1"}, + }, + } + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + ips := getServiceLoadBalancerIPs(tc.svc) + assert.Equal(t, tc.expectedIPs, ips) + }) + } +} + +func TestSetServiceLoadBalancerIP(t *testing.T) { + testcases := []struct { + desc string + ip string + svc *v1.Service + expectedSvc *v1.Service + }{ + { + "IPv6", + "2001::1", + &v1.Service{}, + &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + consts.ServiceAnnotationLoadBalancerIPDualStack[true]: "2001::1", + }, + }, + }, + }, + } + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + setServiceLoadBalancerIP(tc.svc, tc.ip) + assert.Equal(t, tc.expectedSvc, tc.svc) + }) + } +} + +func TestGetServicePIPName(t *testing.T) { + testcases := []struct { + desc string + svc *v1.Service + isIPv6 bool + expectedName string + }{ + // TODO: Add new after DualStack finishes + { + "From ServiceAnnotationPIPName", + &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + consts.ServiceAnnotationPIPName: "pip-name", + }, + }, + }, + false, + "pip-name", + }, + } + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + name := getServicePIPName(tc.svc, tc.isIPv6) + assert.Equal(t, tc.expectedName, name) + }) + } +} + +func TestGetServicePIPPrefixID(t *testing.T) { + testcases := []struct { + desc string + svc *v1.Service + isIPv6 bool + expectedID string + }{ + // TODO: Add new after DualStack finishes + { + "From ServiceAnnotationPIPName", + &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + consts.ServiceAnnotationPIPPrefixID: "pip-prefix-id", + }, + }, + }, + false, + "pip-prefix-id", + }, + } + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + id := getServicePIPPrefixID(tc.svc, tc.isIPv6) + assert.Equal(t, tc.expectedID, id) + }) + } +} + +func TestGetResourceByIPFamily(t *testing.T) { + testcases := []struct { + desc string + resource string + isIPv6 bool + expectedResource string + }{ + // TODO: Add new test after DualStack finishes + { + "Direct", + "resource0", + false, + "resource0", + }, + } + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + resource := getResourceByIPFamily(tc.resource, tc.isIPv6) + assert.Equal(t, tc.expectedResource, resource) + }) + } +} + +func TestIfFIPIPv6(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + testcases := []struct { + desc string + fip *network.FrontendIPConfiguration + pips *[]network.PublicIPAddress + isInternal bool + expectedIsIPv6 bool + }{ + { + "Internal IPv4", + &network.FrontendIPConfiguration{ + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PrivateIPAddressVersion: network.IPv4, + PrivateIPAddress: pointer.String("10.0.0.1"), + }, + }, + &[]network.PublicIPAddress{}, + true, + false, + }, + { + "External IPv6", + &network.FrontendIPConfiguration{ + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("pip-id0")}, + }, + }, + &[]network.PublicIPAddress{ + { + ID: pointer.String("pip-id0"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv6, + IPAddress: pointer.String("2001::1"), + }, + }, + { + ID: pointer.String("pip-id1"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv4, + IPAddress: pointer.String("10.0.0.1"), + }, + }, + }, + false, + true, + }, + } + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + az := GetTestCloud(ctrl) + isIPv6, err := az.ifFIPIPv6(tc.fip, tc.pips, tc.isInternal) + 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 c439572cc4..c8aa811335 100644 --- a/pkg/provider/azure_vmss.go +++ b/pkg/provider/azure_vmss.go @@ -33,7 +33,6 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" cloudprovider "k8s.io/cloud-provider" "k8s.io/klog/v2" - utilnet "k8s.io/utils/net" "k8s.io/utils/pointer" azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache" @@ -1143,7 +1142,7 @@ func (ss *ScaleSet) EnsureHostInPool(service *v1.Service, nodeName types.NodeNam } var primaryIPConfiguration *compute.VirtualMachineScaleSetIPConfiguration - ipv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) + ipv6 := ifBackendPoolIPv6(backendPoolID) // Find primary network interface configuration. if !ss.Cloud.ipv6DualStackEnabled && !ipv6 { // Find primary IP configuration. @@ -1317,7 +1316,7 @@ func (ss *ScaleSet) ensureVMSSInPool(service *v1.Service, nodes []*v1.Node, back return err } var primaryIPConfig *compute.VirtualMachineScaleSetIPConfiguration - ipv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) + ipv6 := ifBackendPoolIPv6(backendPoolID) // Find primary network interface configuration. if !ss.Cloud.ipv6DualStackEnabled && !ipv6 { // Find primary IP configuration. diff --git a/pkg/provider/azure_vmss_test.go b/pkg/provider/azure_vmss_test.go index ebb2f57482..c2957481fd 100644 --- a/pkg/provider/azure_vmss_test.go +++ b/pkg/provider/azure_vmss_test.go @@ -57,21 +57,22 @@ const ( ) func buildTestVMSSWithLB(name, namePrefix string, lbBackendpoolIDs []string, ipv6 bool) compute.VirtualMachineScaleSet { - lbBackendpools := make([]compute.SubResource, 0) + lbBackendpoolsV4, lbBackendpoolsV6 := make([]compute.SubResource, 0), make([]compute.SubResource, 0) for _, id := range lbBackendpoolIDs { - lbBackendpools = append(lbBackendpools, compute.SubResource{ID: pointer.String(id)}) + lbBackendpoolsV4 = append(lbBackendpoolsV4, compute.SubResource{ID: pointer.String(id)}) + lbBackendpoolsV6 = append(lbBackendpoolsV6, compute.SubResource{ID: pointer.String(id + "-" + v6Suffix)}) } ipConfig := []compute.VirtualMachineScaleSetIPConfiguration{ { VirtualMachineScaleSetIPConfigurationProperties: &compute.VirtualMachineScaleSetIPConfigurationProperties{ - LoadBalancerBackendAddressPools: &lbBackendpools, + LoadBalancerBackendAddressPools: &lbBackendpoolsV4, }, }, } if ipv6 { ipConfig = append(ipConfig, compute.VirtualMachineScaleSetIPConfiguration{ VirtualMachineScaleSetIPConfigurationProperties: &compute.VirtualMachineScaleSetIPConfigurationProperties{ - LoadBalancerBackendAddressPools: &lbBackendpools, + LoadBalancerBackendAddressPools: &lbBackendpoolsV6, PrivateIPAddressVersion: compute.IPv6, }, }) @@ -167,7 +168,7 @@ func buildTestVirtualMachineEnv(ss *Cloud, scaleSetName, zone string, faultDomai Name: pointer.String("ipconfig1"), VirtualMachineScaleSetIPConfigurationProperties: &compute.VirtualMachineScaleSetIPConfigurationProperties{ Primary: pointer.Bool(false), - LoadBalancerBackendAddressPools: &[]compute.SubResource{{ID: pointer.String(testLBBackendpoolID0)}}, + LoadBalancerBackendAddressPools: &[]compute.SubResource{{ID: pointer.String(testLBBackendpoolID0 + "-" + v6Suffix)}}, PrivateIPAddressVersion: compute.IPv6, }, }, @@ -2133,7 +2134,7 @@ func TestEnsureVMSSInPool(t *testing.T) { }, }, isBasicLB: false, - backendPoolID: testLBBackendpoolID1, + backendPoolID: testLBBackendpoolID1 + "-" + v6Suffix, clusterIP: "fd00::e68b", expectedPutVMSS: false, expectedErr: fmt.Errorf("failed to find a IPconfiguration(IPv6=true) for the scale set VM \"\""), @@ -2148,7 +2149,7 @@ func TestEnsureVMSSInPool(t *testing.T) { }, }, isBasicLB: false, - backendPoolID: testLBBackendpoolID1, + backendPoolID: testLBBackendpoolID1 + "-" + v6Suffix, setIPv6Config: true, clusterIP: "fd00::e68b", expectedPutVMSS: true, @@ -2214,41 +2215,43 @@ func TestEnsureVMSSInPool(t *testing.T) { } for _, test := range testCases { - ss, err := NewTestScaleSet(ctrl) - assert.NoError(t, err, test.description) + t.Run(test.description, func(t *testing.T) { + ss, err := NewTestScaleSet(ctrl) + assert.NoError(t, err, test.description) - if !test.isBasicLB { - ss.LoadBalancerSku = consts.LoadBalancerSkuStandard - } + if !test.isBasicLB { + ss.LoadBalancerSku = consts.LoadBalancerSkuStandard + } - expectedVMSS := buildTestVMSSWithLB(testVMSSName, "vmss-vm-", []string{testLBBackendpoolID0}, test.setIPv6Config) - if test.isVMSSDeallocating { - expectedVMSS.ProvisioningState = pointer.String(consts.VirtualMachineScaleSetsDeallocating) - } - if test.isVMSSNilNICConfig { - expectedVMSS.VirtualMachineProfile.NetworkProfile.NetworkInterfaceConfigurations = nil - } - mockVMSSClient := ss.cloud.VirtualMachineScaleSetsClient.(*mockvmssclient.MockInterface) - mockVMSSClient.EXPECT().List(gomock.Any(), ss.ResourceGroup).Return([]compute.VirtualMachineScaleSet{expectedVMSS}, nil).AnyTimes() - vmssPutTimes := 0 - if test.expectedPutVMSS { - vmssPutTimes = 1 - mockVMSSClient.EXPECT().Get(gomock.Any(), ss.ResourceGroup, testVMSSName).Return(expectedVMSS, nil) - } - mockVMSSClient.EXPECT().CreateOrUpdate(gomock.Any(), ss.ResourceGroup, testVMSSName, gomock.Any()).Return(nil).Times(vmssPutTimes) + expectedVMSS := buildTestVMSSWithLB(testVMSSName, "vmss-vm-", []string{testLBBackendpoolID0}, test.setIPv6Config) + if test.isVMSSDeallocating { + expectedVMSS.ProvisioningState = pointer.String(consts.VirtualMachineScaleSetsDeallocating) + } + if test.isVMSSNilNICConfig { + expectedVMSS.VirtualMachineProfile.NetworkProfile.NetworkInterfaceConfigurations = nil + } + mockVMSSClient := ss.cloud.VirtualMachineScaleSetsClient.(*mockvmssclient.MockInterface) + mockVMSSClient.EXPECT().List(gomock.Any(), ss.ResourceGroup).Return([]compute.VirtualMachineScaleSet{expectedVMSS}, nil).AnyTimes() + vmssPutTimes := 0 + if test.expectedPutVMSS { + vmssPutTimes = 1 + mockVMSSClient.EXPECT().Get(gomock.Any(), ss.ResourceGroup, testVMSSName).Return(expectedVMSS, nil) + } + mockVMSSClient.EXPECT().CreateOrUpdate(gomock.Any(), ss.ResourceGroup, testVMSSName, gomock.Any()).Return(nil).Times(vmssPutTimes) - expectedVMSSVMs, _, _ := buildTestVirtualMachineEnv(ss.cloud, testVMSSName, "", 0, []string{"vmss-vm-000000"}, "", test.setIPv6Config) - mockVMSSVMClient := ss.cloud.VirtualMachineScaleSetVMsClient.(*mockvmssvmclient.MockInterface) - mockVMSSVMClient.EXPECT().List(gomock.Any(), ss.ResourceGroup, testVMSSName, gomock.Any()).Return(expectedVMSSVMs, nil).AnyTimes() + expectedVMSSVMs, _, _ := buildTestVirtualMachineEnv(ss.cloud, testVMSSName, "", 0, []string{"vmss-vm-000000"}, "", test.setIPv6Config) + mockVMSSVMClient := ss.cloud.VirtualMachineScaleSetVMsClient.(*mockvmssvmclient.MockInterface) + mockVMSSVMClient.EXPECT().List(gomock.Any(), ss.ResourceGroup, testVMSSName, gomock.Any()).Return(expectedVMSSVMs, nil).AnyTimes() - if test.expectedGetInstanceID != "" { - mockVMSet := NewMockVMSet(ctrl) - mockVMSet.EXPECT().GetInstanceIDByNodeName(gomock.Any()).Return(test.expectedGetInstanceID, test.getInstanceIDErr) - ss.VMSet = mockVMSet - } + if test.expectedGetInstanceID != "" { + mockVMSet := NewMockVMSet(ctrl) + mockVMSet.EXPECT().GetInstanceIDByNodeName(gomock.Any()).Return(test.expectedGetInstanceID, test.getInstanceIDErr) + ss.VMSet = mockVMSet + } - err = ss.ensureVMSSInPool(&v1.Service{Spec: v1.ServiceSpec{ClusterIP: test.clusterIP}}, test.nodes, test.backendPoolID, test.vmSetName) - assert.Equal(t, test.expectedErr, err, test.description+", but an error occurs") + err = ss.ensureVMSSInPool(&v1.Service{Spec: v1.ServiceSpec{ClusterIP: test.clusterIP}}, test.nodes, test.backendPoolID, test.vmSetName) + assert.Equal(t, test.expectedErr, err, test.description+", but an error occurs") + }) } } diff --git a/pkg/provider/azure_vmssflex.go b/pkg/provider/azure_vmssflex.go index 0dc34db35f..54453f4b9c 100644 --- a/pkg/provider/azure_vmssflex.go +++ b/pkg/provider/azure_vmssflex.go @@ -31,7 +31,6 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" cloudprovider "k8s.io/cloud-provider" "k8s.io/klog/v2" - utilnet "k8s.io/utils/net" "k8s.io/utils/pointer" azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache" @@ -511,7 +510,7 @@ func (fs *FlexScaleSet) EnsureHostInPool(service *v1.Service, nodeName types.Nod } var primaryIPConfig *network.InterfaceIPConfiguration - ipv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) + ipv6 := ifBackendPoolIPv6(backendPoolID) if !fs.Cloud.ipv6DualStackEnabled && !ipv6 { primaryIPConfig, err = getPrimaryIPConfig(nic) if err != nil { @@ -662,7 +661,7 @@ func (fs *FlexScaleSet) ensureVMSSFlexInPool(service *v1.Service, nodes []*v1.No return err } var primaryIPConfig *compute.VirtualMachineScaleSetIPConfiguration - ipv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) + ipv6 := ifBackendPoolIPv6(backendPoolID) // Find primary network interface configuration. if !fs.Cloud.ipv6DualStackEnabled && !ipv6 { // Find primary IP configuration.