From 822139e378f2ca9445989c60a5e3b95627c01270 Mon Sep 17 00:00:00 2001 From: Zhecheng Li Date: Tue, 18 Apr 2023 08:15:41 +0000 Subject: [PATCH] [DualStack] Support FrontendIPConfig and reconcileLB() * DualStack feature code * Refactor related functions and methods * Refactor and add new UTs Signed-off-by: Zhecheng Li --- pkg/provider/azure_loadbalancer.go | 236 ++++-- .../azure_loadbalancer_backendpool.go | 45 +- pkg/provider/azure_loadbalancer_test.go | 793 ++++++++++++++---- pkg/provider/azure_privatelinkservice.go | 13 +- pkg/provider/azure_privatelinkservice_test.go | 72 +- pkg/provider/azure_standard.go | 39 +- pkg/provider/azure_standard_test.go | 278 +++++- pkg/provider/azure_test.go | 475 ++++++----- pkg/provider/azure_utils.go | 53 +- pkg/provider/azure_utils_test.go | 134 ++- pkg/provider/azure_wrap.go | 2 - 11 files changed, 1632 insertions(+), 508 deletions(-) diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go index 6ae6c70c4a..677d42999d 100644 --- a/pkg/provider/azure_loadbalancer.go +++ b/pkg/provider/azure_loadbalancer.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "math" + "net" "net/netip" "reflect" "sort" @@ -1326,7 +1327,14 @@ func getDomainNameLabel(pip *network.PublicIPAddress) string { return pointer.StringDeref(pip.PublicIPAddressPropertiesFormat.DNSSettings.DomainNameLabel, "") } -func (az *Cloud) isFrontendIPChanged(clusterName string, config network.FrontendIPConfiguration, service *v1.Service, lbFrontendIPConfigName string) (bool, error) { +// subnet is reused to reduce API calls when dualstack. +func (az *Cloud) isFrontendIPChanged( + clusterName string, + config network.FrontendIPConfiguration, + service *v1.Service, + lbFrontendIPConfigName string, + subnet *network.Subnet, +) (bool, error) { isServiceOwnsFrontendIP, isPrimaryService, err := az.serviceOwnsFrontendIP(config, service) if err != nil { return false, err @@ -1337,20 +1345,20 @@ func (az *Cloud) isFrontendIPChanged(clusterName string, config network.Frontend if !strings.EqualFold(pointer.StringDeref(config.Name, ""), lbFrontendIPConfigName) { return false, nil } - isIPv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) - loadBalancerIP := getServiceLoadBalancerIP(service, isIPv6) isInternal := requiresInternalLoadBalancer(service) + isIPv6, err := az.isFIPIPv6(service, &config, isInternal) + if err != nil { + return false, err + } + loadBalancerIP := getServiceLoadBalancerIP(service, isIPv6) if isInternal { // Judge subnet - subnetName := subnet(service) + subnetName := getInternalSubnet(service) if subnetName != nil { - subnet, existsSubnet, err := az.getSubnet(az.VnetName, *subnetName) - if err != nil { + if err := az.fillSubnet(subnet, *subnetName); err != nil { return false, err } - if !existsSubnet { - return false, fmt.Errorf("failed to get subnet") - } + if config.Subnet != nil && !strings.EqualFold(pointer.StringDeref(config.Subnet.ID, ""), pointer.StringDeref(subnet.ID, "")) { return true, nil } @@ -1486,9 +1494,18 @@ func findMatchedOutboundRuleFIPConfig(fipConfigID *string, outboundRuleFIPConfig func (az *Cloud) findFrontendIPConfigOfService( fipConfigs *[]network.FrontendIPConfiguration, service *v1.Service, + isInternal bool, + isIPv6 bool, ) (*network.FrontendIPConfiguration, error) { for _, config := range *fipConfigs { config := config + fipIsIPv6, err := az.isFIPIPv6(service, &config, isInternal) + if err != nil { + return nil, err + } + if fipIsIPv6 != isIPv6 { + continue + } owns, _, err := az.serviceOwnsFrontendIP(config, service) if err != nil { return nil, err @@ -1523,12 +1540,15 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service, } lbName := *lb.Name - isIPv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) - lbBackendPoolID := az.getBackendPoolID(lbName, getBackendPoolName(clusterName, isIPv6)) + lbResourceGroup := az.getLoadBalancerResourceGroup() + lbBackendPoolIDs := az.getBackendPoolIDs(clusterName, lbName) klog.V(2).Infof("reconcileLoadBalancer for service(%s): lb(%s/%s) wantLb(%t) resolved load balancer name", - serviceName, az.getLoadBalancerResourceGroup(), lbName, wantLb) - defaultLBFrontendIPConfigName := az.getDefaultFrontendIPConfigName(service) - defaultLBFrontendIPConfigID := az.getFrontendIPConfigID(lbName, defaultLBFrontendIPConfigName) + serviceName, lbResourceGroup, lbName, wantLb) + lbFrontendIPConfigNames := az.getFrontendIPConfigNames(service) + lbFrontendIPConfigIDs := map[bool]string{ + false: az.getFrontendIPConfigID(lbName, lbFrontendIPConfigNames[false]), + true: az.getFrontendIPConfigID(lbName, lbFrontendIPConfigNames[true]), + } dirtyLb := false // reconcile the load balancer's backend pool configuration. @@ -1544,7 +1564,7 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service, } // reconcile the load balancer's frontend IP configurations. - ownedFIPConfig, toDeleteConfigs, changed, err := az.reconcileFrontendIPConfigs(clusterName, service, lb, lbStatus, wantLb, defaultLBFrontendIPConfigName) + ownedFIPConfigs, toDeleteConfigs, changed, err := az.reconcileFrontendIPConfigs(clusterName, service, lb, lbStatus, wantLb, lbFrontendIPConfigNames) if err != nil { return lb, err } @@ -1553,26 +1573,47 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service, } // update probes/rules - if ownedFIPConfig != nil { - if ownedFIPConfig.ID != nil { - defaultLBFrontendIPConfigID = *ownedFIPConfig.ID - } else { + isInternal := requiresInternalLoadBalancer(service) + for _, ownedFIPConfig := range ownedFIPConfigs { + if ownedFIPConfig == nil { + continue + } + if ownedFIPConfig.ID == nil { return nil, fmt.Errorf("reconcileLoadBalancer for service (%s)(%t): nil ID for frontend IP config", serviceName, wantLb) } - } - if wantLb { - err = az.checkLoadBalancerResourcesConflicts(lb, defaultLBFrontendIPConfigID, service) + isIPv6, err := az.isFIPIPv6(service, ownedFIPConfig, isInternal) if err != nil { return nil, err } + lbFrontendIPConfigIDs[isIPv6] = *ownedFIPConfig.ID } var expectedProbes []network.Probe var expectedRules []network.LoadBalancingRule - if wantLb { - expectedProbes, expectedRules, err = az.getExpectedLBRules(service, defaultLBFrontendIPConfigID, lbBackendPoolID, lbName) + getExpectedLBRule := func(isIPv6 bool) error { + expectedProbesSingleStack, expectedRulesSingleStack, err := az.getExpectedLBRules(service, lbFrontendIPConfigIDs[isIPv6], lbBackendPoolIDs[isIPv6], lbName, isIPv6) if err != nil { + return err + } + expectedProbes = append(expectedProbes, expectedProbesSingleStack...) + expectedRules = append(expectedRules, expectedRulesSingleStack...) + return nil + } + v4Enabled, v6Enabled := getIPFamiliesEnabled(service) + if wantLb && v4Enabled { + if err = az.checkLoadBalancerResourcesConflicts(lb, lbFrontendIPConfigIDs[false], service); err != nil { + return nil, err + } + if err := getExpectedLBRule(false); err != nil { + return nil, err + } + } + if wantLb && v6Enabled { + if err = az.checkLoadBalancerResourcesConflicts(lb, lbFrontendIPConfigIDs[true], service); err != nil { + return nil, err + } + if err := getExpectedLBRule(true); err != nil { return nil, err } } @@ -1644,12 +1685,11 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service, _ = az.lbCache.Delete(lbName) }() - if lb.LoadBalancerPropertiesFormat != nil && lb.BackendAddressPools != nil { - backendPools := *lb.BackendAddressPools - for _, backendPool := range backendPools { + if lb.LoadBalancerPropertiesFormat != nil && lb.LoadBalancerPropertiesFormat.BackendAddressPools != nil { + for _, backendPool := range *lb.LoadBalancerPropertiesFormat.BackendAddressPools { isIPv6 := isBackendPoolIPv6(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 { + if err := az.LoadBalancerBackendPool.EnsureHostsInPool(service, nodes, lbBackendPoolIDs[isIPv6], vmSetName, clusterName, lbName, backendPool); err != nil { return nil, err } } @@ -1751,7 +1791,52 @@ func (az *Cloud) reconcileLBRules(lb *network.LoadBalancer, service *v1.Service, return dirtyRules } -func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Service, lb *network.LoadBalancer, status *v1.LoadBalancerStatus, wantLb bool, defaultLBFrontendIPConfigName string) (*network.FrontendIPConfiguration, []network.FrontendIPConfiguration, bool, error) { +func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Service, lb *network.LoadBalancer, lbStatus *v1.LoadBalancerStatus, wantLb bool, lbFrontendIPConfigName map[bool]string) ([]*network.FrontendIPConfiguration, []network.FrontendIPConfiguration, bool, error) { + ownedFIPConfigs := []*network.FrontendIPConfiguration{} + toDeleteConfigs := []network.FrontendIPConfiguration{} + lbFrontEndIPConfigNewConfigs := []network.FrontendIPConfiguration{} + changedTotal := false + var subnet network.Subnet + handleFrontendIPConfig := func(isIPv6 bool) error { + ownedFIPConfig, toDeleteConfigsSingleStack, changed, newConfigs, err := az.reconcileFrontendIPConfigsSingleStack(clusterName, service, lb, lbStatus, wantLb, isIPv6, lbFrontendIPConfigName[isIPv6], &subnet) + if err != nil { + return err + } + + if changed { + lbFrontEndIPConfigNewConfigs = append(lbFrontEndIPConfigNewConfigs, newConfigs...) + changedTotal = true + } + ownedFIPConfigs = append(ownedFIPConfigs, ownedFIPConfig) + toDeleteConfigs = append(toDeleteConfigs, toDeleteConfigsSingleStack...) + return nil + } + v4Enabled, v6Enabled := getIPFamiliesEnabled(service) + if v4Enabled { + if err := handleFrontendIPConfig(false); err != nil { + return ownedFIPConfigs, toDeleteConfigs, changedTotal, err + } + } + if v6Enabled { + if err := handleFrontendIPConfig(true); err != nil { + return ownedFIPConfigs, toDeleteConfigs, changedTotal, err + } + } + if changedTotal { + lb.FrontendIPConfigurations = &lbFrontEndIPConfigNewConfigs + } + return ownedFIPConfigs, toDeleteConfigs, changedTotal, nil +} + +func (az *Cloud) reconcileFrontendIPConfigsSingleStack( + clusterName string, + service *v1.Service, + lb *network.LoadBalancer, + status *v1.LoadBalancerStatus, + wantLb, isIPv6 bool, + lbFrontendIPConfigName string, + subnet *network.Subnet, +) (*network.FrontendIPConfiguration, []network.FrontendIPConfiguration, bool, []network.FrontendIPConfiguration, error) { var err error lbName := *lb.Name serviceName := getServiceName(service) @@ -1760,7 +1845,16 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv var newConfigs []network.FrontendIPConfiguration var toDeleteConfigs []network.FrontendIPConfiguration if lb.FrontendIPConfigurations != nil { - newConfigs = *lb.FrontendIPConfigurations + for i := range *lb.FrontendIPConfigurations { + fip := (*lb.FrontendIPConfigurations)[i] + fipIsIPv6, err := az.isFIPIPv6(service, &fip, isInternal) + if err != nil { + return nil, toDeleteConfigs, false, newConfigs, err + } + if fipIsIPv6 == isIPv6 { + newConfigs = append(newConfigs, fip) + } + } } var ownedFIPConfig *network.FrontendIPConfiguration @@ -1769,12 +1863,12 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv config := newConfigs[i] isServiceOwnsFrontendIP, _, err := az.serviceOwnsFrontendIP(config, service) if err != nil { - return nil, toDeleteConfigs, false, err + return nil, toDeleteConfigs, false, newConfigs, err } if isServiceOwnsFrontendIP { unsafe, err := az.isFrontendIPConfigUnsafeToDelete(lb, service, config.ID) if err != nil { - return nil, toDeleteConfigs, false, err + return nil, toDeleteConfigs, false, newConfigs, err } // If the frontend IP configuration is not being referenced by: @@ -1811,9 +1905,9 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv continue } klog.V(4).Infof("reconcileFrontendIPConfigs for service (%s): checking owned frontend IP configuration %s", serviceName, pointer.StringDeref(config.Name, "")) - isFipChanged, err = az.isFrontendIPChanged(clusterName, config, service, defaultLBFrontendIPConfigName) + isFipChanged, err = az.isFrontendIPChanged(clusterName, config, service, lbFrontendIPConfigName, subnet) if err != nil { - return nil, toDeleteConfigs, false, err + return nil, toDeleteConfigs, false, newConfigs, err } if isFipChanged { klog.V(2).Infof("reconcileLoadBalancer for service (%s)(%t): lb frontendconfig(%s) - dropping", serviceName, wantLb, *config.Name) @@ -1825,9 +1919,9 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv break } - ownedFIPConfig, err = az.findFrontendIPConfigOfService(&newConfigs, service) + ownedFIPConfig, err = az.findFrontendIPConfigOfService(&newConfigs, service, isInternal, isIPv6) if err != nil { - return nil, toDeleteConfigs, false, err + return nil, toDeleteConfigs, false, newConfigs, err } if ownedFIPConfig == nil { @@ -1836,36 +1930,42 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv // construct FrontendIPConfigurationPropertiesFormat var fipConfigurationProperties *network.FrontendIPConfigurationPropertiesFormat if isInternal { - subnetName := subnet(service) + subnetName := getInternalSubnet(service) if subnetName == nil { subnetName = &az.SubnetName } - subnet, existsSubnet, err := az.getSubnet(az.VnetName, *subnetName) - if err != nil { - return nil, toDeleteConfigs, false, err - } - - if !existsSubnet { - return nil, toDeleteConfigs, false, fmt.Errorf("ensure(%s): lb(%s) - failed to get subnet: %s/%s", serviceName, lbName, az.VnetName, *subnetName) + if err := az.fillSubnet(subnet, *subnetName); err != nil { + return nil, toDeleteConfigs, false, newConfigs, fmt.Errorf("ensure(%s): lb(%s) - %w", serviceName, lbName, err) } configProperties := network.FrontendIPConfigurationPropertiesFormat{ - Subnet: &subnet, + Subnet: subnet, + PrivateIPAddressVersion: network.IPv4, } - isIPv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) if isIPv6 { configProperties.PrivateIPAddressVersion = network.IPv6 } loadBalancerIP := getServiceLoadBalancerIP(service, isIPv6) + privateIP := "" + ingressIPInSubnet := func(ingresses []v1.LoadBalancerIngress) bool { + for _, ingress := range ingresses { + ingressIP := ingress.IP + if (net.ParseIP(ingressIP).To4() == nil) == isIPv6 && ipInSubnet(ingressIP, subnet) { + privateIP = ingressIP + break + } + } + return privateIP != "" + } if loadBalancerIP != "" { configProperties.PrivateIPAllocationMethod = network.Static configProperties.PrivateIPAddress = &loadBalancerIP - } else if status != nil && len(status.Ingress) > 0 && ipInSubnet(status.Ingress[0].IP, &subnet) { - klog.V(4).Infof("reconcileFrontendIPConfigs for service (%s): keep the original private IP %s", serviceName, status.Ingress[0].IP) + } else if status != nil && len(status.Ingress) > 0 && ingressIPInSubnet(status.Ingress) { + klog.V(4).Infof("reconcileFrontendIPConfigs for service (%s): keep the original private IP %s", serviceName, privateIP) configProperties.PrivateIPAllocationMethod = network.Static - configProperties.PrivateIPAddress = pointer.String(status.Ingress[0].IP) + configProperties.PrivateIPAddress = pointer.String(privateIP) } else { // We'll need to call GetLoadBalancer later to retrieve allocated IP. klog.V(4).Infof("reconcileFrontendIPConfigs for service (%s): dynamically allocate the private IP", serviceName) @@ -1874,15 +1974,14 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv fipConfigurationProperties = &configProperties } else { - isIPv6 := utilnet.IsIPv6String(service.Spec.ClusterIP) // TODO: dualstack support pipName, shouldPIPExisted, err := az.determinePublicIPName(clusterName, service, isIPv6) if err != nil { - return nil, toDeleteConfigs, false, err + return nil, toDeleteConfigs, false, newConfigs, err } domainNameLabel, found := getPublicIPDomainNameLabel(service) pip, err := az.ensurePublicIPExists(service, pipName, domainNameLabel, clusterName, shouldPIPExisted, found, isIPv6) if err != nil { - return nil, toDeleteConfigs, false, err + return nil, toDeleteConfigs, false, newConfigs, err } fipConfigurationProperties = &network.FrontendIPConfigurationPropertiesFormat{ PublicIPAddress: &network.PublicIPAddress{ID: pip.ID}, @@ -1890,28 +1989,24 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv } newConfig := network.FrontendIPConfiguration{ - Name: pointer.String(defaultLBFrontendIPConfigName), - ID: pointer.String(fmt.Sprintf(consts.FrontendIPConfigIDTemplate, az.getNetworkResourceSubscriptionID(), az.ResourceGroup, *lb.Name, defaultLBFrontendIPConfigName)), + Name: pointer.String(lbFrontendIPConfigName), + ID: pointer.String(fmt.Sprintf(consts.FrontendIPConfigIDTemplate, az.getNetworkResourceSubscriptionID(), az.ResourceGroup, *lb.Name, lbFrontendIPConfigName)), FrontendIPConfigurationPropertiesFormat: fipConfigurationProperties, } if isInternal { - if err := az.getFrontendZones(&newConfig, previousZone, isFipChanged, serviceName, defaultLBFrontendIPConfigName); err != nil { + if err := az.getFrontendZones(&newConfig, previousZone, isFipChanged, serviceName, lbFrontendIPConfigName); err != nil { klog.Errorf("reconcileLoadBalancer for service (%s)(%t): failed to getFrontendZones: %s", serviceName, wantLb, err.Error()) - return nil, toDeleteConfigs, false, err + return nil, toDeleteConfigs, false, newConfigs, err } } newConfigs = append(newConfigs, newConfig) - klog.V(2).Infof("reconcileLoadBalancer for service (%s)(%t): lb frontendconfig(%s) - adding", serviceName, wantLb, defaultLBFrontendIPConfigName) + klog.V(2).Infof("reconcileLoadBalancer for service (%s)(%t): lb frontendconfig(%s) - adding", serviceName, wantLb, lbFrontendIPConfigName) dirtyConfigs = true } } - if dirtyConfigs { - lb.FrontendIPConfigurations = &newConfigs - } - - return ownedFIPConfig, toDeleteConfigs, dirtyConfigs, err + return ownedFIPConfig, toDeleteConfigs, dirtyConfigs, newConfigs, err } func (az *Cloud) getFrontendZones( @@ -2252,7 +2347,8 @@ func (az *Cloud) getExpectedLBRules( service *v1.Service, lbFrontendIPConfigID string, lbBackendPoolID string, - lbName string) ([]network.Probe, []network.LoadBalancingRule, error) { + lbName string, + isIPv6 bool) ([]network.Probe, []network.LoadBalancingRule, error) { var expectedRules []network.LoadBalancingRule var expectedProbes []network.Probe @@ -2264,7 +2360,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, utilnet.IsIPv6String(service.Spec.ClusterIP)) + lbRuleName := az.getLoadBalancerRuleName(service, v1.ProtocolTCP, podPresencePort, isIPv6) nodeEndpointHealthprobe = &network.Probe{ Name: &lbRuleName, @@ -2285,7 +2381,7 @@ func (az *Cloud) getExpectedLBRules( az.useStandardLoadBalancer() && consts.IsK8sServiceHasHAModeEnabled(service) { - lbRuleName := az.getloadbalancerHAmodeRuleName(service, utilnet.IsIPv6String(service.Spec.ClusterIP)) + lbRuleName := az.getloadbalancerHAmodeRuleName(service, isIPv6) klog.V(2).Infof("getExpectedLBRules lb name (%s) rule name (%s)", lbName, lbRuleName) props, err := az.getExpectedHAModeLoadBalancingRuleProperties(service, lbFrontendIPConfigID, lbBackendPoolID) @@ -2325,7 +2421,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, utilnet.IsIPv6String(service.Spec.ClusterIP)) + lbRuleName := az.getLoadBalancerRuleName(service, port.Protocol, port.Port, isIPv6) klog.V(2).Infof("getExpectedLBRules lb name (%s) rule name (%s)", lbName, lbRuleName) isNoLBRuleRequired, err := consts.IsLBRuleOnK8sServicePortDisabled(service.Annotations, port.Port) if err != nil { @@ -3309,6 +3405,7 @@ func findRule(rules []network.LoadBalancingRule, rule network.LoadBalancingRule, // equalLoadBalancingRulePropertiesFormat checks whether the provided LoadBalancingRulePropertiesFormat are equal. // Note: only fields used in reconcileLoadBalancer are considered. +// s: existing, t: target func equalLoadBalancingRulePropertiesFormat(s *network.LoadBalancingRulePropertiesFormat, t *network.LoadBalancingRulePropertiesFormat, wantLB bool) bool { if s == nil || t == nil { return false @@ -3425,7 +3522,7 @@ func requiresInternalLoadBalancer(service *v1.Service) bool { return false } -func subnet(service *v1.Service) *string { +func getInternalSubnet(service *v1.Service) *string { if requiresInternalLoadBalancer(service) { if l, found := service.Annotations[consts.ServiceAnnotationLoadBalancerInternalSubnet]; found && strings.TrimSpace(l) != "" { return &l @@ -3531,8 +3628,9 @@ func serviceOwnsPublicIP(service *v1.Service, pip *network.PublicIPAddress, clus clusterTag := getClusterFromPIPClusterTags(pip.Tags) // if there is no service tag on the pip, it is user-created pip + isIPv6 := pip.PublicIPAddressVersion == network.IPv6 if serviceTag == "" { - return strings.EqualFold(pointer.StringDeref(pip.IPAddress, ""), getServiceLoadBalancerIP(service, utilnet.IsIPv6String(service.Spec.ClusterIP))), true + return strings.EqualFold(pointer.StringDeref(pip.IPAddress, ""), getServiceLoadBalancerIP(service, isIPv6)), true } // if there is service tag on the pip, it is system-created pip @@ -3550,7 +3648,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, utilnet.IsIPv6String(service.Spec.ClusterIP))), false + return strings.EqualFold(pointer.StringDeref(pip.IPAddress, ""), getServiceLoadBalancerIP(service, isIPv6)), false } } diff --git a/pkg/provider/azure_loadbalancer_backendpool.go b/pkg/provider/azure_loadbalancer_backendpool.go index 19d283a90e..993048b949 100644 --- a/pkg/provider/azure_loadbalancer_backendpool.go +++ b/pkg/provider/azure_loadbalancer_backendpool.go @@ -85,14 +85,8 @@ func isLBBackendPoolsExisting(lbBackendPoolNames map[bool]string, bpName *string func (bc *backendPoolTypeNodeIPConfig) CleanupVMSetFromBackendPoolByCondition(slb *network.LoadBalancer, service *v1.Service, nodes []*v1.Node, clusterName string, shouldRemoveVMSetFromSLB func(string) bool) (*network.LoadBalancer, error) { v4Enabled, v6Enabled := getIPFamiliesEnabled(service) - lbBackendPoolNames := map[bool]string{ - false: getBackendPoolName(clusterName, false), - true: getBackendPoolName(clusterName, true), - } - lbBackendPoolIDs := map[bool]string{ - false: bc.getBackendPoolID(pointer.StringDeref(slb.Name, ""), lbBackendPoolNames[false]), - true: bc.getBackendPoolID(pointer.StringDeref(slb.Name, ""), lbBackendPoolNames[true]), - } + lbBackendPoolNames := getBackendPoolNames(clusterName) + lbBackendPoolIDs := bc.getBackendPoolIDs(clusterName, pointer.StringDeref(slb.Name, "")) newBackendPools := make([]network.BackendAddressPool, 0) if slb.LoadBalancerPropertiesFormat != nil && slb.BackendAddressPools != nil { newBackendPools = *slb.BackendAddressPools @@ -175,14 +169,8 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string, lbName := *lb.Name serviceName := getServiceName(service) - lbBackendPoolNames := map[bool]string{ - false: getBackendPoolName(clusterName, false), - true: getBackendPoolName(clusterName, true), - } - lbBackendPoolIDs := map[bool]string{ - false: bc.getBackendPoolID(lbName, lbBackendPoolNames[false]), - true: bc.getBackendPoolID(lbName, lbBackendPoolNames[true]), - } + lbBackendPoolNames := getBackendPoolNames(clusterName) + lbBackendPoolIDs := bc.getBackendPoolIDs(clusterName, lbName) vmSetName := bc.mapLoadBalancerNameToVMSet(lbName, clusterName) isBackendPoolPreConfigured := bc.isBackendPoolPreConfigured(service) @@ -347,10 +335,7 @@ func getBackendIPConfigurationsToBeDeleted( func (bc *backendPoolTypeNodeIPConfig) GetBackendPrivateIPs(clusterName string, service *v1.Service, lb *network.LoadBalancer) ([]string, []string) { serviceName := getServiceName(service) - lbBackendPoolNames := map[bool]string{ - false: getBackendPoolName(clusterName, false), - true: getBackendPoolName(clusterName, true), - } + lbBackendPoolNames := getBackendPoolNames(clusterName) if lb.LoadBalancerPropertiesFormat == nil || lb.LoadBalancerPropertiesFormat.BackendAddressPools == nil { return nil, nil } @@ -493,10 +478,7 @@ 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) { - lbBackendPoolNames := map[bool]string{ - false: getBackendPoolName(clusterName, false), - true: getBackendPoolName(clusterName, true), - } + lbBackendPoolNames := getBackendPoolNames(clusterName) newBackendPools := make([]network.BackendAddressPool, 0) if slb.LoadBalancerPropertiesFormat != nil && slb.BackendAddressPools != nil { newBackendPools = *slb.BackendAddressPools @@ -564,15 +546,9 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi foundBackendPools := map[bool]bool{} lbName := *lb.Name serviceName := getServiceName(service) - lbBackendPoolNames := map[bool]string{ - false: getBackendPoolName(clusterName, false), - true: getBackendPoolName(clusterName, true), - } + lbBackendPoolNames := getBackendPoolNames(clusterName) vmSetName := bi.mapLoadBalancerNameToVMSet(lbName, clusterName) - lbBackendPoolIDs := map[bool]string{ - false: bi.getBackendPoolID(pointer.StringDeref(lb.Name, ""), lbBackendPoolNames[false]), - true: bi.getBackendPoolID(pointer.StringDeref(lb.Name, ""), lbBackendPoolNames[true]), - } + lbBackendPoolIDs := bi.getBackendPoolIDs(clusterName, pointer.StringDeref(lb.Name, "")) isBackendPoolPreConfigured := bi.isBackendPoolPreConfigured(service) mc := metrics.NewMetricContext("services", "migrate_to_ip_based_backend_pool", bi.ResourceGroup, bi.getNetworkResourceSubscriptionID(), serviceName) @@ -685,10 +661,7 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi func (bi *backendPoolTypeNodeIP) GetBackendPrivateIPs(clusterName string, service *v1.Service, lb *network.LoadBalancer) ([]string, []string) { serviceName := getServiceName(service) - lbBackendPoolNames := map[bool]string{ - false: getBackendPoolName(clusterName, false), - true: getBackendPoolName(clusterName, true), - } + lbBackendPoolNames := getBackendPoolNames(clusterName) if lb.LoadBalancerPropertiesFormat == nil || lb.LoadBalancerPropertiesFormat.BackendAddressPools == nil { return nil, nil } diff --git a/pkg/provider/azure_loadbalancer_test.go b/pkg/provider/azure_loadbalancer_test.go index 490eee3bb8..6d6f450e57 100644 --- a/pkg/provider/azure_loadbalancer_test.go +++ b/pkg/provider/azure_loadbalancer_test.go @@ -441,9 +441,11 @@ func TestFindProbe(t *testing.T) { }, } - for i, test := range tests { - findResult := findProbe(test.existingProbe, test.curProbe) - assert.Equal(t, test.expected, findResult, fmt.Sprintf("TestCase[%d]: %s", i, test.msg)) + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + findResult := findProbe(test.existingProbe, test.curProbe) + assert.Equal(t, test.expected, findResult) + }) } } @@ -734,9 +736,11 @@ func TestFindRule(t *testing.T) { }, } - for i, test := range tests { - findResult := findRule(test.existingRule, test.curRule, true) - assert.Equal(t, test.expected, findResult, fmt.Sprintf("TestCase[%d]: %s", i, test.msg)) + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + findResult := findRule(test.existingRule, test.curRule, true) + assert.Equal(t, test.expected, findResult) + }) } } @@ -799,7 +803,7 @@ func TestSubnet(t *testing.T) { expected: pointer.String("subnet"), }, } { - real := subnet(c.service) + real := getInternalSubnet(c.service) assert.Equal(t, c.expected, real, fmt.Sprintf("TestCase[%d]: %s", i, c.desc)) } } @@ -2182,8 +2186,9 @@ func TestIsFrontendIPChanged(t *testing.T) { service := test.service setServiceLoadBalancerIP(&service, test.loadBalancerIP) test.service.Annotations[consts.ServiceAnnotationLoadBalancerInternalSubnet] = test.annotations + var subnet network.Subnet flag, rerr := az.isFrontendIPChanged("testCluster", test.config, - &service, test.lbFrontendIPConfigName) + &service, test.lbFrontendIPConfigName, &subnet) if rerr != nil { fmt.Println(rerr.Error()) } @@ -2307,8 +2312,8 @@ func TestReconcileLoadBalancerRule(t *testing.T) { loadBalancerSku string probeProtocol string probePath string - expectedProbes []network.Probe - expectedRules []network.LoadBalancingRule + expectedProbes map[bool][]network.Probe + expectedRules map[bool][]network.LoadBalancingRule expectedErr bool }{ { @@ -2382,7 +2387,10 @@ func TestReconcileLoadBalancerRule(t *testing.T) { }, false, 80), loadBalancerSku: "standard", expectedProbes: getDefaultTestProbes("Tcp", ""), - expectedRules: getHATestRules(true, true, v1.ProtocolTCP), + expectedRules: map[bool][]network.LoadBalancingRule{ + false: getHATestRules(true, true, v1.ProtocolTCP, false), + true: getHATestRules(true, true, v1.ProtocolTCP, true), + }, }, { desc: "getExpectedLBRules shall return corresponding probe and lbRule (slb with HA mode and SCTP)", @@ -2391,7 +2399,10 @@ func TestReconcileLoadBalancerRule(t *testing.T) { consts.ServiceAnnotationLoadBalancerInternal: "true", }, false, 80), loadBalancerSku: "standard", - expectedRules: getHATestRules(true, false, v1.ProtocolSCTP), + expectedRules: map[bool][]network.LoadBalancingRule{ + false: getHATestRules(true, false, v1.ProtocolSCTP, false), + true: getHATestRules(true, false, v1.ProtocolSCTP, true), + }, }, { desc: "getExpectedLBRules shall return corresponding probe and lbRule (slb with HA enabled multi-ports services)", @@ -2401,7 +2412,10 @@ func TestReconcileLoadBalancerRule(t *testing.T) { }, false, 80, 8080), loadBalancerSku: "standard", expectedProbes: getDefaultTestProbes("Tcp", ""), - expectedRules: getHATestRules(true, true, v1.ProtocolTCP), + expectedRules: map[bool][]network.LoadBalancingRule{ + false: getHATestRules(true, true, v1.ProtocolTCP, false), + true: getHATestRules(true, true, v1.ProtocolTCP, true), + }, }, { desc: "getExpectedLBRules should leave probe path empty when using TCP probe", @@ -2582,8 +2596,9 @@ func TestReconcileLoadBalancerRule(t *testing.T) { desc: "getExpectedLBRules should return correct rule when floating ip annotations are added", service: getTestService("test1", v1.ProtocolTCP, map[string]string{consts.ServiceAnnotationDisableLoadBalancerFloatingIP: "true"}, false, 80), loadBalancerSku: "basic", - expectedRules: []network.LoadBalancingRule{ - getFloatingIPTestRule(false, false, 80), + expectedRules: map[bool][]network.LoadBalancingRule{ + false: {getFloatingIPTestRule(false, false, 80, false)}, + true: {getFloatingIPTestRule(false, false, 80, true)}, }, expectedProbes: getDefaultTestProbes("Tcp", ""), }, @@ -2629,13 +2644,19 @@ func TestReconcileLoadBalancerRule(t *testing.T) { service: getTestService("test1", v1.ProtocolTCP, map[string]string{ "service.beta.kubernetes.io/port_8000_health-probe_port": "port-tcp-80", }, false, 80, 8000), - expectedRules: []network.LoadBalancingRule{ - getTestRule(false, 80), - getTestRule(false, 8000), + expectedRules: map[bool][]network.LoadBalancingRule{ + false: {getTestRule(false, 80, false), getTestRule(false, 8000, false)}, + true: {getTestRule(false, 80, true), getTestRule(false, 8000, true)}, }, - expectedProbes: []network.Probe{ - getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2)), - getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(8000), pointer.Int32(10080), pointer.Int32(2)), + expectedProbes: map[bool][]network.Probe{ + false: { + getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2), false), + getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(8000), pointer.Int32(10080), pointer.Int32(2), false), + }, + true: { + getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2), true), + getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(8000), pointer.Int32(10080), pointer.Int32(2), true), + }, }, }, { @@ -2643,13 +2664,25 @@ func TestReconcileLoadBalancerRule(t *testing.T) { service: getTestService("test1", v1.ProtocolTCP, map[string]string{ "service.beta.kubernetes.io/port_8000_health-probe_port": "80", }, false, 80, 8000), - expectedRules: []network.LoadBalancingRule{ - getTestRule(false, 80), - getTestRule(false, 8000), + expectedRules: map[bool][]network.LoadBalancingRule{ + false: { + getTestRule(false, 80, false), + getTestRule(false, 8000, false), + }, + true: { + getTestRule(false, 80, true), + getTestRule(false, 8000, true), + }, }, - expectedProbes: []network.Probe{ - getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2)), - getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(8000), pointer.Int32(10080), pointer.Int32(2)), + expectedProbes: map[bool][]network.Probe{ + false: { + getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2), false), + getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(8000), pointer.Int32(10080), pointer.Int32(2), false), + }, + true: { + getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2), true), + getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(8000), pointer.Int32(10080), pointer.Int32(2), true), + }, }, }, { @@ -2657,16 +2690,31 @@ func TestReconcileLoadBalancerRule(t *testing.T) { service: getTestService("test1", v1.ProtocolTCP, map[string]string{ "service.beta.kubernetes.io/port_8000_no_probe_rule": "true", }, false, 80, 8000), - expectedRules: []network.LoadBalancingRule{ - getTestRule(false, 80), - func() network.LoadBalancingRule { - rule := getTestRule(false, 8000) - rule.Probe = nil - return rule - }(), + expectedRules: map[bool][]network.LoadBalancingRule{ + false: { + getTestRule(false, 80, false), + func() network.LoadBalancingRule { + rule := getTestRule(false, 8000, false) + rule.Probe = nil + return rule + }(), + }, + true: { + getTestRule(false, 80, true), + func() network.LoadBalancingRule { + rule := getTestRule(false, 8000, true) + rule.Probe = nil + return rule + }(), + }, }, - expectedProbes: []network.Probe{ - getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2)), + expectedProbes: map[bool][]network.Probe{ + false: { + getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2), false), + }, + true: { + getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2), true), + }, }, }, { @@ -2674,24 +2722,38 @@ func TestReconcileLoadBalancerRule(t *testing.T) { service: getTestService("test1", v1.ProtocolTCP, map[string]string{ "service.beta.kubernetes.io/port_8000_no_lb_rule": "true", }, false, 80, 8000), - expectedRules: []network.LoadBalancingRule{ - getTestRule(false, 80), + expectedRules: map[bool][]network.LoadBalancingRule{ + false: { + getTestRule(false, 80, false), + }, + true: { + getTestRule(false, 80, true), + }, }, - expectedProbes: []network.Probe{ - getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2)), + expectedProbes: map[bool][]network.Probe{ + false: { + getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2), false), + }, + true: { + getTestProbe("Tcp", "/", pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2), true), + }, }, }, } - rules := getDefaultTestRules(true) - rules[0].IdleTimeoutInMinutes = pointer.Int32(5) + rulesDualStack := getDefaultTestRules(true) + for _, rules := range rulesDualStack { + for _, rule := range rules { + rule.IdleTimeoutInMinutes = pointer.Int32(5) + } + } testCases = append(testCases, struct { desc string service v1.Service loadBalancerSku string probeProtocol string probePath string - expectedProbes []network.Probe - expectedRules []network.LoadBalancingRule + expectedProbes map[bool][]network.Probe + expectedRules map[bool][]network.LoadBalancingRule expectedErr bool }{ desc: "getExpectedLBRules should expected rules when timeout are added", @@ -2703,26 +2765,36 @@ func TestReconcileLoadBalancerRule(t *testing.T) { loadBalancerSku: "standard", probeProtocol: "Tcp", expectedProbes: getTestProbes("Tcp", "", pointer.Int32(10), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(10)), - expectedRules: rules, + expectedRules: rulesDualStack, }) - rules1 := []network.LoadBalancingRule{ - getTestRule(true, 80), - getTestRule(true, 443), - getTestRule(true, 421), + rules1DualStack := map[bool][]network.LoadBalancingRule{ + false: { + getTestRule(true, 80, false), + getTestRule(true, 443, false), + getTestRule(true, 421, false), + }, + true: { + getTestRule(true, 80, true), + getTestRule(true, 443, true), + getTestRule(true, 421, true), + }, + } + for _, rule := range rules1DualStack[false] { + rule.Probe.ID = pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lbname/probes/atest1-TCP-34567") + } + for _, rule := range rules1DualStack[true] { + rule.Probe.ID = pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lbname/probes/atest1-TCP-34567-IPv6") } - rules1[0].Probe.ID = pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lbname/probes/atest1-TCP-34567") - rules1[1].Probe.ID = pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lbname/probes/atest1-TCP-34567") - rules1[2].Probe.ID = pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lbname/probes/atest1-TCP-34567") // When the service spec externalTrafficPolicy is Local all of these annotations should be ignored - svc := getTestService("test1", v1.ProtocolTCP, map[string]string{ + svc := getTestServiceDualStack("test1", v1.ProtocolTCP, map[string]string{ consts.ServiceAnnotationLoadBalancerHealthProbeProtocol: "tcp", consts.ServiceAnnotationLoadBalancerHealthProbeRequestPath: "/broken/global/path", consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsProbeInterval): "10", consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsProtocol): "https", consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsRequestPath): "/broken/local/path", consts.BuildHealthProbeAnnotationKeyForPort(80, consts.HealthProbeParamsNumOfProbe): "10", - }, false, 80, 443, 421) + }, 80, 443, 421) svc.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeLocal svc.Spec.HealthCheckNodePort = 34567 probes := getTestProbes("Http", "/healthz", pointer.Int32(5), pointer.Int32(34567), pointer.Int32(34567), pointer.Int32(2)) @@ -2732,8 +2804,8 @@ func TestReconcileLoadBalancerRule(t *testing.T) { loadBalancerSku string probeProtocol string probePath string - expectedProbes []network.Probe - expectedRules []network.LoadBalancingRule + expectedProbes map[bool][]network.Probe + expectedRules map[bool][]network.LoadBalancingRule expectedErr bool }{ desc: "getExpectedLBRules should expected rules when externalTrafficPolicy is local", @@ -2741,13 +2813,14 @@ func TestReconcileLoadBalancerRule(t *testing.T) { loadBalancerSku: "standard", probeProtocol: "Http", expectedProbes: probes, - expectedRules: rules1, + expectedRules: rules1DualStack, }) for _, test := range testCases { t.Run(test.desc, func(t *testing.T) { az := GetTestCloud(ctrl) az.Config.LoadBalancerSku = test.loadBalancerSku service := test.service + service.Spec.IPFamilies = []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} firstPort := service.Spec.Ports[0] probeProtocol := test.probeProtocol if test.probeProtocol != "" { @@ -2756,29 +2829,39 @@ func TestReconcileLoadBalancerRule(t *testing.T) { if test.probePath != "" { service.Annotations[consts.BuildHealthProbeAnnotationKeyForPort(firstPort.Port, consts.HealthProbeParamsRequestPath)] = test.probePath } - probe, lbrule, err := az.getExpectedLBRules(&service, - "frontendIPConfigID", "backendPoolID", "lbname") - + probeV4, lbruleV4, errV4 := az.getExpectedLBRules(&service, + "frontendIPConfigID", "backendPoolID", "lbname", false) + probeV6, lbruleV6, errV6 := az.getExpectedLBRules(&service, + "frontendIPConfigID-IPv6", "backendPoolID-IPv6", "lbname", true) if test.expectedErr { - assert.Error(t, err) + assert.Error(t, errV4) + assert.Error(t, errV6) } else { - assert.Equal(t, test.expectedProbes, probe) - assert.Equal(t, test.expectedRules, lbrule) - assert.NoError(t, err) + assert.Equal(t, test.expectedProbes[false], probeV4) + assert.Equal(t, test.expectedProbes[true], probeV6) + assert.Equal(t, test.expectedRules[false], lbruleV4) + assert.Equal(t, test.expectedRules[true], lbruleV6) + assert.NoError(t, errV4) + assert.NoError(t, errV6) } }) } } -func getTestProbes(protocol, path string, interval, servicePort, probePort, numOfProbe *int32) []network.Probe { - return []network.Probe{ - getTestProbe(protocol, path, interval, servicePort, probePort, numOfProbe), +func getTestProbes(protocol, path string, interval, servicePort, probePort, numOfProbe *int32) map[bool][]network.Probe { + return map[bool][]network.Probe{ + false: {getTestProbe(protocol, path, interval, servicePort, probePort, numOfProbe, false)}, + true: {getTestProbe(protocol, path, interval, servicePort, probePort, numOfProbe, true)}, } } -func getTestProbe(protocol, path string, interval, servicePort, probePort, numOfProbe *int32) network.Probe { +func getTestProbe(protocol, path string, interval, servicePort, probePort, numOfProbe *int32, isIPv6 bool) network.Probe { + suffix := "" + if isIPv6 { + suffix = "-" + v6Suffix + } expectedProbes := network.Probe{ - Name: pointer.String(fmt.Sprintf("atest1-TCP-%d", *servicePort)), + Name: pointer.String(fmt.Sprintf("atest1-TCP-%d", *servicePort) + suffix), ProbePropertiesFormat: &network.ProbePropertiesFormat{ Protocol: network.ProbeProtocol(protocol), Port: probePort, @@ -2791,35 +2874,43 @@ func getTestProbe(protocol, path string, interval, servicePort, probePort, numOf } return expectedProbes } -func getDefaultTestProbes(protocol, path string) []network.Probe { + +func getDefaultTestProbes(protocol, path string) map[bool][]network.Probe { return getTestProbes(protocol, path, pointer.Int32(5), pointer.Int32(80), pointer.Int32(10080), pointer.Int32(2)) } -func getDefaultTestRules(enableTCPReset bool) []network.LoadBalancingRule { - return []network.LoadBalancingRule{ - getTestRule(enableTCPReset, 80), +func getDefaultTestRules(enableTCPReset bool) map[bool][]network.LoadBalancingRule { + return map[bool][]network.LoadBalancingRule{ + false: {getTestRule(enableTCPReset, 80, false)}, + true: {getTestRule(enableTCPReset, 80, true)}, } } -func getDefaultInternalIPv6Rules(enableTCPReset bool) []network.LoadBalancingRule { - rules := getDefaultTestRules(true) - for _, rule := range rules { - rule.EnableFloatingIP = pointer.Bool(false) - rule.BackendPort = pointer.Int32(getBackendPort(*rule.FrontendPort)) +func getDefaultInternalIPv6Rules(enableTCPReset bool) map[bool][]network.LoadBalancingRule { + rulesDualStack := getDefaultTestRules(true) + for _, rules := range rulesDualStack { + for _, rule := range rules { + rule.EnableFloatingIP = pointer.Bool(false) + rule.BackendPort = pointer.Int32(getBackendPort(*rule.FrontendPort)) + } } - return rules + return rulesDualStack } -func getTestRule(enableTCPReset bool, port int32) network.LoadBalancingRule { +func getTestRule(enableTCPReset bool, port int32, isIPv6 bool) network.LoadBalancingRule { + suffix := "" + if isIPv6 { + suffix = "-" + v6Suffix + } expectedRules := network.LoadBalancingRule{ - Name: pointer.String(fmt.Sprintf("atest1-TCP-%d", port)), + Name: pointer.String(fmt.Sprintf("atest1-TCP-%d", port) + suffix), LoadBalancingRulePropertiesFormat: &network.LoadBalancingRulePropertiesFormat{ Protocol: network.TransportProtocol("Tcp"), FrontendIPConfiguration: &network.SubResource{ - ID: pointer.String("frontendIPConfigID"), + ID: pointer.String("frontendIPConfigID" + suffix), }, BackendAddressPool: &network.SubResource{ - ID: pointer.String("backendPoolID"), + ID: pointer.String("backendPoolID" + suffix), }, LoadDistribution: "Default", FrontendPort: pointer.Int32(port), @@ -2829,7 +2920,7 @@ func getTestRule(enableTCPReset bool, port int32) network.LoadBalancingRule { IdleTimeoutInMinutes: pointer.Int32(4), Probe: &network.SubResource{ ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/" + - fmt.Sprintf("Microsoft.Network/loadBalancers/lbname/probes/atest1-TCP-%d", port)), + fmt.Sprintf("Microsoft.Network/loadBalancers/lbname/probes/atest1-TCP-%d%s", port, suffix)), }, }, } @@ -2839,17 +2930,21 @@ func getTestRule(enableTCPReset bool, port int32) network.LoadBalancingRule { return expectedRules } -func getHATestRules(enableTCPReset, hasProbe bool, protocol v1.Protocol) []network.LoadBalancingRule { +func getHATestRules(enableTCPReset, hasProbe bool, protocol v1.Protocol, isIPv6 bool) []network.LoadBalancingRule { + suffix := "" + if isIPv6 { + suffix = "-" + v6Suffix + } expectedRules := []network.LoadBalancingRule{ { - Name: pointer.String(fmt.Sprintf("atest1-%s-80", string(protocol))), + Name: pointer.String(fmt.Sprintf("atest1-%s-80", string(protocol)) + suffix), LoadBalancingRulePropertiesFormat: &network.LoadBalancingRulePropertiesFormat{ Protocol: network.TransportProtocol("All"), FrontendIPConfiguration: &network.SubResource{ - ID: pointer.String("frontendIPConfigID"), + ID: pointer.String("frontendIPConfigID" + suffix), }, BackendAddressPool: &network.SubResource{ - ID: pointer.String("backendPoolID"), + ID: pointer.String("backendPoolID" + suffix), }, LoadDistribution: "Default", FrontendPort: pointer.Int32(0), @@ -2864,22 +2959,26 @@ func getHATestRules(enableTCPReset, hasProbe bool, protocol v1.Protocol) []netwo if hasProbe { expectedRules[0].Probe = &network.SubResource{ ID: pointer.String(fmt.Sprintf("/subscriptions/subscription/resourceGroups/rg/providers/"+ - "Microsoft.Network/loadBalancers/lbname/probes/atest1-%s-80", string(protocol))), + "Microsoft.Network/loadBalancers/lbname/probes/atest1-%s-80%s", string(protocol), suffix)), } } return expectedRules } -func getFloatingIPTestRule(enableTCPReset, enableFloatingIP bool, port int32) network.LoadBalancingRule { +func getFloatingIPTestRule(enableTCPReset, enableFloatingIP bool, port int32, isIPv6 bool) network.LoadBalancingRule { + suffix := "" + if isIPv6 { + suffix = "-" + v6Suffix + } expectedRules := network.LoadBalancingRule{ - Name: pointer.String(fmt.Sprintf("atest1-TCP-%d", port)), + Name: pointer.String(fmt.Sprintf("atest1-TCP-%d%s", port, suffix)), LoadBalancingRulePropertiesFormat: &network.LoadBalancingRulePropertiesFormat{ Protocol: network.TransportProtocol("Tcp"), FrontendIPConfiguration: &network.SubResource{ - ID: pointer.String("frontendIPConfigID"), + ID: pointer.String("frontendIPConfigID" + suffix), }, BackendAddressPool: &network.SubResource{ - ID: pointer.String("backendPoolID"), + ID: pointer.String("backendPoolID" + suffix), }, LoadDistribution: "Default", FrontendPort: pointer.Int32(port), @@ -2889,7 +2988,7 @@ func getFloatingIPTestRule(enableTCPReset, enableFloatingIP bool, port int32) ne IdleTimeoutInMinutes: pointer.Int32(4), Probe: &network.SubResource{ ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/" + - fmt.Sprintf("Microsoft.Network/loadBalancers/lbname/probes/atest1-TCP-%d", port)), + fmt.Sprintf("Microsoft.Network/loadBalancers/lbname/probes/atest1-TCP-%d%s", port, suffix)), }, }, } @@ -2964,15 +3063,69 @@ func getTestLoadBalancer(name, rgName, clusterName, identifier *string, service return lb } -func TestReconcileLoadBalancer(t *testing.T) { +func getTestLoadBalancerDualStack(name, rgName, clusterName, identifier *string, service v1.Service, lbSku string) network.LoadBalancer { + caser := cases.Title(language.English) + lb := getTestLoadBalancer(name, rgName, clusterName, identifier, service, lbSku) + *lb.LoadBalancerPropertiesFormat.FrontendIPConfigurations = append(*lb.LoadBalancerPropertiesFormat.FrontendIPConfigurations, network.FrontendIPConfiguration{ + Name: pointer.String(*identifier + "-IPv6"), + ID: pointer.String("/subscriptions/subscription/resourceGroups/" + *rgName + "/providers/" + + "Microsoft.Network/loadBalancers/" + *name + "/frontendIPConfigurations/" + *identifier + "-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ + ID: pointer.String("testCluster-aservice1-IPv6"), + }, + }, + }) + *lb.LoadBalancerPropertiesFormat.BackendAddressPools = append(*lb.LoadBalancerPropertiesFormat.BackendAddressPools, network.BackendAddressPool{ + Name: pointer.String(*clusterName + "-IPv6"), + }) + *lb.LoadBalancerPropertiesFormat.Probes = append(*lb.LoadBalancerPropertiesFormat.Probes, network.Probe{ + Name: pointer.String(*identifier + "-" + string(service.Spec.Ports[0].Protocol) + + "-" + strconv.Itoa(int(service.Spec.Ports[0].Port)) + "-IPv6"), + ProbePropertiesFormat: &network.ProbePropertiesFormat{ + Port: pointer.Int32(10080), + Protocol: network.ProbeProtocolTCP, + IntervalInSeconds: pointer.Int32(5), + ProbeThreshold: pointer.Int32(2), + }, + }) + *lb.LoadBalancerPropertiesFormat.LoadBalancingRules = append(*lb.LoadBalancerPropertiesFormat.LoadBalancingRules, network.LoadBalancingRule{ + Name: pointer.String(*identifier + "-" + string(service.Spec.Ports[0].Protocol) + + "-" + strconv.Itoa(int(service.Spec.Ports[0].Port)) + "-IPv6"), + LoadBalancingRulePropertiesFormat: &network.LoadBalancingRulePropertiesFormat{ + Protocol: network.TransportProtocol(caser.String((strings.ToLower(string(service.Spec.Ports[0].Protocol))))), + FrontendIPConfiguration: &network.SubResource{ + ID: pointer.String("/subscriptions/subscription/resourceGroups/" + *rgName + "/providers/" + + "Microsoft.Network/loadBalancers/" + *name + "/frontendIPConfigurations/aservice1-IPv6"), + }, + BackendAddressPool: &network.SubResource{ + ID: pointer.String("/subscriptions/subscription/resourceGroups/" + *rgName + "/providers/" + + "Microsoft.Network/loadBalancers/" + *name + "/backendAddressPools/" + *clusterName + "-IPv6"), + }, + LoadDistribution: network.LoadDistribution("Default"), + FrontendPort: pointer.Int32(service.Spec.Ports[0].Port), + BackendPort: pointer.Int32(service.Spec.Ports[0].Port), + EnableFloatingIP: pointer.Bool(true), + EnableTCPReset: pointer.Bool(strings.EqualFold(lbSku, "standard")), + DisableOutboundSnat: pointer.Bool(false), + IdleTimeoutInMinutes: pointer.Int32(4), + Probe: &network.SubResource{ + ID: pointer.String("/subscriptions/subscription/resourceGroups/" + *rgName + "/providers/Microsoft.Network/loadBalancers/testCluster/probes/aservice1-TCP-80-IPv6"), + }, + }, + }) + return lb +} + +func TestReconcileLoadBalancerCommon(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - service1 := getTestService("service1", v1.ProtocolTCP, nil, false, 80) - basicLb1 := getTestLoadBalancer(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service1, "Basic") + service1 := getTestServiceDualStack("service1", v1.ProtocolTCP, nil, 80) + basicLb1 := getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service1, "Basic") - service2 := getTestService("test1", v1.ProtocolTCP, nil, false, 80) - basicLb2 := getTestLoadBalancer(pointer.String("lb1"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("bservice1"), service2, "Basic") + service2 := getTestServiceDualStack("test1", v1.ProtocolTCP, nil, 80) + basicLb2 := getTestLoadBalancerDualStack(pointer.String("lb1"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("bservice1"), service2, "Basic") basicLb2.Name = pointer.String("testCluster") basicLb2.FrontendIPConfigurations = &[]network.FrontendIPConfiguration{ { @@ -2982,12 +3135,19 @@ func TestReconcileLoadBalancer(t *testing.T) { PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1")}, }, }, + { + Name: pointer.String("bservice1-IPv6"), + ID: pointer.String("bservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1-IPv6")}, + }, + }, } - service3 := getTestService("service1", v1.ProtocolTCP, nil, false, 80) + service3 := getTestServiceDualStack("service1", v1.ProtocolTCP, nil, 80) modifiedLbs := make([]network.LoadBalancer, 2) for i := range modifiedLbs { - modifiedLbs[i] = getTestLoadBalancer(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service3, "Basic") + modifiedLbs[i] = getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service3, "Basic") modifiedLbs[i].FrontendIPConfigurations = &[]network.FrontendIPConfiguration{ { Name: pointer.String("aservice1"), @@ -3003,6 +3163,20 @@ func TestReconcileLoadBalancer(t *testing.T) { PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1")}, }, }, + { + Name: pointer.String("aservice1-IPv6"), + ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/testCluster/frontendIPConfigurations/aservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-aservice1-IPv6")}, + }, + }, + { + Name: pointer.String("bservice1-IPv6"), + ID: pointer.String("bservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1-IPv6")}, + }, + }, } modifiedLbs[i].Probes = &[]network.Probe{ { @@ -3019,9 +3193,23 @@ func TestReconcileLoadBalancer(t *testing.T) { Port: pointer.Int32(10081), }, }, + { + Name: pointer.String("aservice1-" + string(service3.Spec.Ports[0].Protocol) + + "-" + strconv.Itoa(int(service3.Spec.Ports[0].Port)) + "-IPv6"), + ProbePropertiesFormat: &network.ProbePropertiesFormat{ + Port: pointer.Int32(10080), + }, + }, + { + Name: pointer.String("aservice1-" + string(service3.Spec.Ports[0].Protocol) + + "-" + strconv.Itoa(int(service3.Spec.Ports[0].Port)) + "-IPv6"), + ProbePropertiesFormat: &network.ProbePropertiesFormat{ + Port: pointer.Int32(10081), + }, + }, } } - expectedLb1 := getTestLoadBalancer(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service3, "Basic") + expectedLb1 := getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service3, "Basic") expectedLb1.FrontendIPConfigurations = &[]network.FrontendIPConfiguration{ { Name: pointer.String("aservice1"), @@ -3037,10 +3225,24 @@ func TestReconcileLoadBalancer(t *testing.T) { PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1")}, }, }, + { + Name: pointer.String("aservice1-IPv6"), + ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/testCluster/frontendIPConfigurations/aservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-aservice1-IPv6")}, + }, + }, + { + Name: pointer.String("bservice1-IPv6"), + ID: pointer.String("bservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1-IPv6")}, + }, + }, } - service4 := getTestService("service1", v1.ProtocolTCP, map[string]string{}, false, 80) - existingSLB := getTestLoadBalancer(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service4, "Standard") + service4 := getTestServiceDualStack("service1", v1.ProtocolTCP, map[string]string{}, 80) + existingSLB := getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service4, "Standard") existingSLB.FrontendIPConfigurations = &[]network.FrontendIPConfiguration{ { Name: pointer.String("aservice1"), @@ -3056,6 +3258,20 @@ func TestReconcileLoadBalancer(t *testing.T) { PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1")}, }, }, + { + Name: pointer.String("aservice1-IPv6"), + ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/testCluster/frontendIPConfigurations/aservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-aservice1-IPv6")}, + }, + }, + { + Name: pointer.String("bservice1-IPv6"), + ID: pointer.String("bservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1-IPv6")}, + }, + }, } existingSLB.Probes = &[]network.Probe{ { @@ -3072,12 +3288,29 @@ func TestReconcileLoadBalancer(t *testing.T) { Port: pointer.Int32(10081), }, }, + { + Name: pointer.String("aservice1-" + string(service4.Spec.Ports[0].Protocol) + + "-" + strconv.Itoa(int(service4.Spec.Ports[0].Port)) + "-IPv6"), + ProbePropertiesFormat: &network.ProbePropertiesFormat{ + Port: pointer.Int32(10080), + }, + }, + { + Name: pointer.String("aservice1-" + string(service4.Spec.Ports[0].Protocol) + + "-" + strconv.Itoa(int(service4.Spec.Ports[0].Port)) + "-IPv6"), + ProbePropertiesFormat: &network.ProbePropertiesFormat{ + Port: pointer.Int32(10081), + }, + }, } - expectedSLb := getTestLoadBalancer(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service4, "Standard") + expectedSLb := getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service4, "Standard") (*expectedSLb.LoadBalancerPropertiesFormat.LoadBalancingRules)[0].DisableOutboundSnat = pointer.Bool(true) (*expectedSLb.LoadBalancerPropertiesFormat.LoadBalancingRules)[0].EnableTCPReset = pointer.Bool(true) (*expectedSLb.LoadBalancerPropertiesFormat.LoadBalancingRules)[0].IdleTimeoutInMinutes = pointer.Int32(4) + (*expectedSLb.LoadBalancerPropertiesFormat.LoadBalancingRules)[1].DisableOutboundSnat = pointer.Bool(true) + (*expectedSLb.LoadBalancerPropertiesFormat.LoadBalancingRules)[1].EnableTCPReset = pointer.Bool(true) + (*expectedSLb.LoadBalancerPropertiesFormat.LoadBalancingRules)[1].IdleTimeoutInMinutes = pointer.Int32(4) expectedSLb.FrontendIPConfigurations = &[]network.FrontendIPConfiguration{ { Name: pointer.String("aservice1"), @@ -3093,10 +3326,24 @@ func TestReconcileLoadBalancer(t *testing.T) { PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1")}, }, }, + { + Name: pointer.String("aservice1-IPv6"), + ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/testCluster/frontendIPConfigurations/aservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-aservice1-IPv6")}, + }, + }, + { + Name: pointer.String("bservice1-IPv6"), + ID: pointer.String("bservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1-IPv6")}, + }, + }, } - service5 := getTestService("service1", v1.ProtocolTCP, nil, false, 80) - slb5 := getTestLoadBalancer(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service5, "Standard") + service5 := getTestServiceDualStack("service1", v1.ProtocolTCP, nil, 80) + slb5 := getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service5, "Standard") slb5.FrontendIPConfigurations = &[]network.FrontendIPConfiguration{ { Name: pointer.String("aservice1"), @@ -3112,6 +3359,20 @@ func TestReconcileLoadBalancer(t *testing.T) { PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1")}, }, }, + { + Name: pointer.String("aservice1-IPv6"), + ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/testCluster/frontendIPConfigurations/aservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-aservice1-IPv6")}, + }, + }, + { + Name: pointer.String("bservice1-IPv6"), + ID: pointer.String("bservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1-IPv6")}, + }, + }, } slb5.Probes = &[]network.Probe{ { @@ -3128,14 +3389,31 @@ func TestReconcileLoadBalancer(t *testing.T) { Port: pointer.Int32(10081), }, }, + { + Name: pointer.String("aservice1-" + string(service4.Spec.Ports[0].Protocol) + + "-" + strconv.Itoa(int(service4.Spec.Ports[0].Port)) + "-IPv6"), + ProbePropertiesFormat: &network.ProbePropertiesFormat{ + Port: pointer.Int32(10080), + }, + }, + { + Name: pointer.String("aservice1-" + string(service4.Spec.Ports[0].Protocol) + + "-" + strconv.Itoa(int(service4.Spec.Ports[0].Port)) + "-IPv6"), + ProbePropertiesFormat: &network.ProbePropertiesFormat{ + Port: pointer.Int32(10081), + }, + }, } //change to false to test that reconciliation will fix it (despite the fact that disable-tcp-reset was removed in 1.20) (*slb5.LoadBalancerPropertiesFormat.LoadBalancingRules)[0].EnableTCPReset = pointer.Bool(false) + (*slb5.LoadBalancerPropertiesFormat.LoadBalancingRules)[1].EnableTCPReset = pointer.Bool(false) - expectedSLb5 := getTestLoadBalancer(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service5, "Standard") + expectedSLb5 := getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service5, "Standard") (*expectedSLb5.LoadBalancerPropertiesFormat.LoadBalancingRules)[0].DisableOutboundSnat = pointer.Bool(true) (*expectedSLb5.LoadBalancerPropertiesFormat.LoadBalancingRules)[0].IdleTimeoutInMinutes = pointer.Int32(4) + (*expectedSLb5.LoadBalancerPropertiesFormat.LoadBalancingRules)[1].DisableOutboundSnat = pointer.Bool(true) + (*expectedSLb5.LoadBalancerPropertiesFormat.LoadBalancingRules)[1].IdleTimeoutInMinutes = pointer.Int32(4) expectedSLb5.FrontendIPConfigurations = &[]network.FrontendIPConfiguration{ { Name: pointer.String("aservice1"), @@ -3151,16 +3429,32 @@ func TestReconcileLoadBalancer(t *testing.T) { PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1")}, }, }, + { + Name: pointer.String("aservice1-IPv6"), + ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/testCluster/frontendIPConfigurations/aservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-aservice1-IPv6")}, + }, + }, + { + Name: pointer.String("bservice1-IPv6"), + ID: pointer.String("bservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-bservice1-IPv6")}, + }, + }, } - service6 := getTestService("service1", v1.ProtocolUDP, nil, false, 80) - lb6 := getTestLoadBalancer(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service6, "basic") + service6 := getTestServiceDualStack("service1", v1.ProtocolUDP, nil, 80) + lb6 := getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service6, "basic") lb6.FrontendIPConfigurations = &[]network.FrontendIPConfiguration{} lb6.Probes = &[]network.Probe{} - expectedLB6 := getTestLoadBalancer(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service6, "basic") + expectedLB6 := getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service6, "basic") expectedLB6.Probes = &[]network.Probe{} (*expectedLB6.LoadBalancerPropertiesFormat.LoadBalancingRules)[0].Probe = nil (*expectedLB6.LoadBalancerPropertiesFormat.LoadBalancingRules)[0].EnableTCPReset = nil + (*expectedLB6.LoadBalancerPropertiesFormat.LoadBalancingRules)[1].Probe = nil + (*expectedLB6.LoadBalancerPropertiesFormat.LoadBalancingRules)[1].EnableTCPReset = nil expectedLB6.FrontendIPConfigurations = &[]network.FrontendIPConfiguration{ { Name: pointer.String("aservice1"), @@ -3169,20 +3463,32 @@ func TestReconcileLoadBalancer(t *testing.T) { PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-aservice1")}, }, }, + { + Name: pointer.String("aservice1-IPv6"), + ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/testCluster/frontendIPConfigurations/aservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-aservice1-IPv6")}, + }, + }, } - service7 := getTestService("service1", v1.ProtocolUDP, nil, false, 80) + service7 := getTestServiceDualStack("service1", v1.ProtocolUDP, nil, 80) service7.Spec.HealthCheckNodePort = 10081 service7.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeLocal - lb7 := getTestLoadBalancer(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service7, "basic") + lb7 := getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service7, "basic") lb7.FrontendIPConfigurations = &[]network.FrontendIPConfiguration{} lb7.Probes = &[]network.Probe{} - expectedLB7 := getTestLoadBalancer(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service7, "basic") + expectedLB7 := getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("aservice1"), service7, "basic") (*expectedLB7.LoadBalancerPropertiesFormat.LoadBalancingRules)[0].Probe = &network.SubResource{ ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/testCluster/probes/aservice1-TCP-10081"), } + (*expectedLB7.LoadBalancerPropertiesFormat.LoadBalancingRules)[1].Probe = &network.SubResource{ + ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/testCluster/probes/aservice1-TCP-10081-IPv6"), + } (*expectedLB7.LoadBalancerPropertiesFormat.LoadBalancingRules)[0].EnableTCPReset = nil (*lb7.LoadBalancerPropertiesFormat.LoadBalancingRules)[0].DisableOutboundSnat = pointer.Bool(true) + (*expectedLB7.LoadBalancerPropertiesFormat.LoadBalancingRules)[1].EnableTCPReset = nil + (*lb7.LoadBalancerPropertiesFormat.LoadBalancingRules)[1].DisableOutboundSnat = pointer.Bool(true) expectedLB7.FrontendIPConfigurations = &[]network.FrontendIPConfiguration{ { Name: pointer.String("aservice1"), @@ -3191,6 +3497,13 @@ func TestReconcileLoadBalancer(t *testing.T) { PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-aservice1")}, }, }, + { + Name: pointer.String("aservice1-IPv6"), + ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/testCluster/frontendIPConfigurations/aservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-aservice1-IPv6")}, + }, + }, } expectedLB7.Probes = &[]network.Probe{ { @@ -3204,14 +3517,26 @@ func TestReconcileLoadBalancer(t *testing.T) { ProbeThreshold: pointer.Int32(2), }, }, + { + Name: pointer.String("aservice1-" + string(v1.ProtocolTCP) + + "-" + strconv.Itoa(int(service7.Spec.HealthCheckNodePort)) + "-IPv6"), + ProbePropertiesFormat: &network.ProbePropertiesFormat{ + Port: pointer.Int32(10081), + RequestPath: pointer.String("/healthz"), + Protocol: network.ProbeProtocolHTTP, + IntervalInSeconds: pointer.Int32(5), + ProbeThreshold: pointer.Int32(2), + }, + }, } - service8 := getTestService("service1", v1.ProtocolTCP, nil, false, 80) - lb8 := getTestLoadBalancer(pointer.String("testCluster"), pointer.String("anotherRG"), pointer.String("testCluster"), pointer.String("aservice1"), service8, "Standard") + service8 := getTestServiceDualStack("service1", v1.ProtocolTCP, nil, 80) + lb8 := getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("anotherRG"), pointer.String("testCluster"), pointer.String("aservice1"), service8, "Standard") lb8.FrontendIPConfigurations = &[]network.FrontendIPConfiguration{} lb8.Probes = &[]network.Probe{} - expectedLB8 := getTestLoadBalancer(pointer.String("testCluster"), pointer.String("anotherRG"), pointer.String("testCluster"), pointer.String("aservice1"), service8, "Standard") + expectedLB8 := getTestLoadBalancerDualStack(pointer.String("testCluster"), pointer.String("anotherRG"), pointer.String("testCluster"), pointer.String("aservice1"), service8, "Standard") (*expectedLB8.LoadBalancerPropertiesFormat.LoadBalancingRules)[0].DisableOutboundSnat = pointer.Bool(false) + (*expectedLB8.LoadBalancerPropertiesFormat.LoadBalancingRules)[1].DisableOutboundSnat = pointer.Bool(false) expectedLB8.FrontendIPConfigurations = &[]network.FrontendIPConfiguration{ { Name: pointer.String("aservice1"), @@ -3220,6 +3545,13 @@ func TestReconcileLoadBalancer(t *testing.T) { PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-aservice1")}, }, }, + { + Name: pointer.String("aservice1-IPv6"), + ID: pointer.String("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/testCluster/frontendIPConfigurations/aservice1-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("testCluster-aservice1-IPv6")}, + }, + }, } expectedLB8.Probes = &[]network.Probe{ { @@ -3232,6 +3564,16 @@ func TestReconcileLoadBalancer(t *testing.T) { ProbeThreshold: pointer.Int32(2), }, }, + { + Name: pointer.String("aservice1-" + string(service8.Spec.Ports[0].Protocol) + + "-" + strconv.Itoa(int(service7.Spec.Ports[0].Port)) + "-IPv6"), + ProbePropertiesFormat: &network.ProbePropertiesFormat{ + Port: pointer.Int32(10080), + Protocol: network.ProbeProtocolTCP, + IntervalInSeconds: pointer.Int32(5), + ProbeThreshold: pointer.Int32(2), + }, + }, } testCases := []struct { @@ -3354,15 +3696,25 @@ func TestReconcileLoadBalancer(t *testing.T) { az.LoadBalancerResourceGroup = test.loadBalancerResourceGroup clusterResources, expectedInterfaces, expectedVirtualMachines := getClusterResources(az, 3, 3) - setMockEnv(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) + setMockEnvDualStack(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) service := test.service setServiceLoadBalancerIP(&service, "1.2.3.4") + setServiceLoadBalancerIP(&service, "fd00::eef0") err := az.PublicIPAddressesClient.CreateOrUpdate(context.TODO(), "rg", "pipName", network.PublicIPAddress{ Name: pointer.String("pipName"), PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ - IPAddress: pointer.String("1.2.3.4"), + IPAddress: pointer.String("1.2.3.4"), + PublicIPAddressVersion: network.IPv4, + }, + }) + assert.NoError(t, err.Error()) + err = az.PublicIPAddressesClient.CreateOrUpdate(context.TODO(), "rg", "pipName-IPv6", network.PublicIPAddress{ + Name: pointer.String("pipName-IPv6"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + IPAddress: pointer.String("fd00::eef0"), + PublicIPAddressVersion: network.IPv6, }, }) assert.NoError(t, err.Error()) @@ -5592,18 +5944,48 @@ func TestCheckLoadBalancerResourcesConflicted(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - service := getTestService("service1", v1.ProtocolTCP, nil, false, 80) + service := getTestServiceDualStack("service1", v1.ProtocolTCP, nil, 80) az := GetTestCloud(ctrl) - fipID := "fip" testCases := []struct { desc string + fipID string existingLB *network.LoadBalancer expectedErr bool }{ { desc: "checkLoadBalancerResourcesConflicts should report the conflict error if " + - "there is a conflicted loadBalancing rule", + "there is a conflicted loadBalancing rule - IPv4", + fipID: "fip", + existingLB: &network.LoadBalancer{ + Name: pointer.String("lb"), + LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{ + LoadBalancingRules: &[]network.LoadBalancingRule{ + { + Name: pointer.String("aservice2-rule"), + LoadBalancingRulePropertiesFormat: &network.LoadBalancingRulePropertiesFormat{ + FrontendIPConfiguration: &network.SubResource{ID: pointer.String("fip")}, + FrontendPort: pointer.Int32(80), + Protocol: network.TransportProtocol(v1.ProtocolTCP), + }, + }, + { + Name: pointer.String("aservice2-rule-IPv6"), + LoadBalancingRulePropertiesFormat: &network.LoadBalancingRulePropertiesFormat{ + FrontendIPConfiguration: &network.SubResource{ID: pointer.String("fip-IPv6")}, + FrontendPort: pointer.Int32(80), + Protocol: network.TransportProtocol(v1.ProtocolTCP), + }, + }, + }, + }, + }, + expectedErr: true, + }, + { + desc: "checkLoadBalancerResourcesConflicts should report the conflict error if " + + "there is a conflicted loadBalancing rule - IPv6", + fipID: "fip-IPv6", existingLB: &network.LoadBalancer{ Name: pointer.String("lb"), LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{ @@ -5616,6 +5998,14 @@ func TestCheckLoadBalancerResourcesConflicted(t *testing.T) { Protocol: network.TransportProtocol(v1.ProtocolTCP), }, }, + { + Name: pointer.String("aservice2-rule-IPv6"), + LoadBalancingRulePropertiesFormat: &network.LoadBalancingRulePropertiesFormat{ + FrontendIPConfiguration: &network.SubResource{ID: pointer.String("fip-IPv6")}, + FrontendPort: pointer.Int32(80), + Protocol: network.TransportProtocol(v1.ProtocolTCP), + }, + }, }, }, }, @@ -5624,6 +6014,7 @@ func TestCheckLoadBalancerResourcesConflicted(t *testing.T) { { desc: "checkLoadBalancerResourcesConflicts should report the conflict error if " + "there is a conflicted inbound NAT rule", + fipID: "fip", existingLB: &network.LoadBalancer{ Name: pointer.String("lb"), LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{ @@ -5644,6 +6035,7 @@ func TestCheckLoadBalancerResourcesConflicted(t *testing.T) { { desc: "checkLoadBalancerResourcesConflicts should report the conflict error if " + "there is a conflicted inbound NAT pool", + fipID: "fip", existingLB: &network.LoadBalancer{ Name: pointer.String("lb"), LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{ @@ -5665,6 +6057,7 @@ func TestCheckLoadBalancerResourcesConflicted(t *testing.T) { { desc: "checkLoadBalancerResourcesConflicts should not report the conflict error if there " + "is no conflicted loadBalancer resources", + fipID: "fip", existingLB: &network.LoadBalancer{ Name: pointer.String("lb"), LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{ @@ -5705,8 +6098,10 @@ func TestCheckLoadBalancerResourcesConflicted(t *testing.T) { } for _, testCase := range testCases { - err := az.checkLoadBalancerResourcesConflicts(testCase.existingLB, fipID, &service) - assert.Equal(t, testCase.expectedErr, err != nil, testCase.desc) + t.Run(testCase.desc, func(t *testing.T) { + err := az.checkLoadBalancerResourcesConflicts(testCase.existingLB, testCase.fipID, &service) + assert.Equal(t, testCase.expectedErr, err != nil) + }) } } @@ -5929,6 +6324,15 @@ func TestRemoveFrontendIPConfigurationFromLoadBalancerDelete(t *testing.T) { fip := &network.FrontendIPConfiguration{ Name: pointer.String("testCluster"), ID: pointer.String("testCluster-fip"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ + ID: pointer.String("pipID"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv4, + }, + }, + PrivateIPAddressVersion: network.IPv4, + }, } service := getTestService("svc1", v1.ProtocolTCP, nil, false, 80) lb := getTestLoadBalancer(pointer.String("lb"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("testCluster"), service, "standard") @@ -5950,6 +6354,17 @@ func TestRemoveFrontendIPConfigurationFromLoadBalancerDelete(t *testing.T) { mockPLSClient := cloud.PrivateLinkServiceClient.(*mockprivatelinkserviceclient.MockInterface) mockPLSClient.EXPECT().List(gomock.Any(), "rg").Return(expectedPLS, nil).MaxTimes(1) existingLBs := []network.LoadBalancer{{Name: pointer.String("lb")}} + // external Service + existingPIPS := []network.PublicIPAddress{ + { + ID: pointer.String("pipID"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv4, + }, + }, + } + mockPIPsClient := cloud.PublicIPAddressesClient.(*mockpublicipclient.MockInterface) + mockPIPsClient.EXPECT().List(gomock.Any(), "rg").Return(existingPIPS, nil).Times(1) err := cloud.removeFrontendIPConfigurationFromLoadBalancer(&lb, existingLBs, fip, "testCluster", &service) assert.NoError(t, err) }) @@ -5962,6 +6377,15 @@ func TestRemoveFrontendIPConfigurationFromLoadBalancerUpdate(t *testing.T) { fip := &network.FrontendIPConfiguration{ Name: pointer.String("testCluster"), ID: pointer.String("testCluster-fip"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ + ID: pointer.String("pipID"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv4, + }, + }, + PrivateIPAddressVersion: network.IPv4, + }, } service := getTestService("svc1", v1.ProtocolTCP, nil, false, 80) lb := getTestLoadBalancer(pointer.String("lb"), pointer.String("rg"), pointer.String("testCluster"), pointer.String("testCluster"), service, "standard") @@ -5972,6 +6396,17 @@ func TestRemoveFrontendIPConfigurationFromLoadBalancerUpdate(t *testing.T) { expectedPLS := make([]network.PrivateLinkService, 0) mockPLSClient := cloud.PrivateLinkServiceClient.(*mockprivatelinkserviceclient.MockInterface) mockPLSClient.EXPECT().List(gomock.Any(), "rg").Return(expectedPLS, nil).MaxTimes(1) + // external Service + existingPIPS := []network.PublicIPAddress{ + { + ID: pointer.String("pipID"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv4, + }, + }, + } + mockPIPsClient := cloud.PublicIPAddressesClient.(*mockpublicipclient.MockInterface) + mockPIPsClient.EXPECT().List(gomock.Any(), "rg").Return(existingPIPS, nil).Times(1) err := cloud.removeFrontendIPConfigurationFromLoadBalancer(&lb, []network.LoadBalancer{}, fip, "testCluster", &service) assert.NoError(t, err) }) @@ -6032,35 +6467,39 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) { description string service v1.Service existingFrontendIPConfigs []network.FrontendIPConfiguration - existingPIP network.PublicIPAddress + existingPIPV4 network.PublicIPAddress + existingPIPV6 network.PublicIPAddress status *v1.LoadBalancerStatus getZoneError *retry.Error regionZonesMap map[string][]string expectedZones *[]string expectedDirty bool - expectedIP *string + expectedIPv4 *string + expectedIPv6 *string expectedErr error }{ { description: "reconcileFrontendIPConfigs should reconcile the zones for the new fip config", - service: getTestService("test", v1.ProtocolTCP, nil, false, 80), + service: getTestServiceDualStack("test", v1.ProtocolTCP, nil, 80), existingFrontendIPConfigs: []network.FrontendIPConfiguration{}, - existingPIP: network.PublicIPAddress{Location: pointer.String("eastus")}, + existingPIPV4: network.PublicIPAddress{Name: pointer.String("testCluster-atest"), Location: pointer.String("eastus")}, + existingPIPV6: network.PublicIPAddress{Name: pointer.String("testCluster-atest-IPv6"), Location: pointer.String("eastus")}, regionZonesMap: map[string][]string{"westus": {"1", "2", "3"}, "eastus": {"1", "2"}}, expectedDirty: true, }, { description: "reconcileFrontendIPConfigs should reconcile the zones for the new internal fip config", - service: getInternalTestService("test", 80), + service: getInternalTestServiceDualStack("test", 80), existingFrontendIPConfigs: []network.FrontendIPConfiguration{}, - existingPIP: network.PublicIPAddress{Location: pointer.String("eastus")}, + existingPIPV4: network.PublicIPAddress{Name: pointer.String("testCluster-atest"), Location: pointer.String("eastus")}, + existingPIPV6: network.PublicIPAddress{Name: pointer.String("testCluster-atest-IPv6"), Location: pointer.String("eastus")}, regionZonesMap: map[string][]string{"westus": {"1", "2", "3"}, "eastus": {"1", "2"}}, expectedZones: &[]string{"1", "2", "3"}, expectedDirty: true, }, { description: "reconcileFrontendIPConfigs should report an error if failed to get zones", - service: getInternalTestService("test", 80), + service: getInternalTestServiceDualStack("test", 80), getZoneError: retry.NewError(false, errors.New("get zone failed")), expectedErr: errors.New("get zone failed"), }, @@ -6068,7 +6507,7 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) { description: "reconcileFrontendIPConfigs should use the nil zones of the existing frontend", service: getTestServiceWithAnnotation("test", map[string]string{ consts.ServiceAnnotationLoadBalancerInternalSubnet: "subnet", - consts.ServiceAnnotationLoadBalancerInternal: consts.TrueAnnotationValue}, false, 80), + consts.ServiceAnnotationLoadBalancerInternal: consts.TrueAnnotationValue}, true, 80), existingFrontendIPConfigs: []network.FrontendIPConfiguration{ { Name: pointer.String("atest1"), @@ -6085,7 +6524,7 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) { description: "reconcileFrontendIPConfigs should use the non-nil zones of the existing frontend", service: getTestServiceWithAnnotation("test", map[string]string{ consts.ServiceAnnotationLoadBalancerInternalSubnet: "subnet", - consts.ServiceAnnotationLoadBalancerInternal: consts.TrueAnnotationValue}, false, 80), + consts.ServiceAnnotationLoadBalancerInternal: consts.TrueAnnotationValue}, true, 80), existingFrontendIPConfigs: []network.FrontendIPConfiguration{ { Name: pointer.String("not-this-one"), @@ -6111,24 +6550,28 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) { }, { description: "reconcileFrontendIPConfigs should reuse the existing private IP for internal services when subnet does not change", - service: getInternalTestService("test", 80), + service: getInternalTestServiceDualStack("test", 80), status: &v1.LoadBalancerStatus{ Ingress: []v1.LoadBalancerIngress{ {IP: "1.2.3.4"}, + {IP: "2001::1"}, }, }, - expectedIP: pointer.String("1.2.3.4"), + expectedIPv4: pointer.String("1.2.3.4"), + expectedIPv6: pointer.String("2001::1"), expectedDirty: true, }, { description: "reconcileFrontendIPConfigs should not reuse the existing private IP for internal services when subnet changes", - service: getInternalTestService("test", 80), + service: getInternalTestServiceDualStack("test", 80), status: &v1.LoadBalancerStatus{ Ingress: []v1.LoadBalancerIngress{ {IP: "1.2.3.6"}, + {IP: "2001::3"}, }, }, - expectedIP: pointer.String(""), + expectedIPv4: pointer.String(""), + expectedIPv6: pointer.String(""), expectedDirty: true, }, } { @@ -6142,21 +6585,28 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) { lb.FrontendIPConfigurations = &existingFrontendIPConfigs mockPIPClient := cloud.PublicIPAddressesClient.(*mockpublicipclient.MockInterface) - first := mockPIPClient.EXPECT().List(gomock.Any(), "rg").Return([]network.PublicIPAddress{}, nil).MaxTimes(2) - mockPIPClient.EXPECT().Get(gomock.Any(), "rg", gomock.Any(), gomock.Any()).Return(tc.existingPIP, nil).MaxTimes(1).After(first) - mockPIPClient.EXPECT().CreateOrUpdate(gomock.Any(), "rg", gomock.Any(), gomock.Any()).Return(nil).MaxTimes(1) + firstV4 := mockPIPClient.EXPECT().List(gomock.Any(), "rg").Return([]network.PublicIPAddress{}, nil).MaxTimes(2) + firstV6 := mockPIPClient.EXPECT().List(gomock.Any(), "rg").Return([]network.PublicIPAddress{}, nil).MaxTimes(2) + mockPIPClient.EXPECT().Get(gomock.Any(), "rg", gomock.Any(), gomock.Any()).Return(tc.existingPIPV4, nil).MaxTimes(1).After(firstV4) + mockPIPClient.EXPECT().Get(gomock.Any(), "rg", gomock.Any(), gomock.Any()).Return(tc.existingPIPV6, nil).MaxTimes(1).After(firstV6) + mockPIPClient.EXPECT().CreateOrUpdate(gomock.Any(), "rg", gomock.Any(), gomock.Any()).Return(nil).MaxTimes(2) subnetClient := cloud.SubnetsClient.(*mocksubnetclient.MockInterface) subnetClient.EXPECT().Get(gomock.Any(), "rg", "vnet", "subnet", gomock.Any()).Return( - network.Subnet{SubnetPropertiesFormat: &network.SubnetPropertiesFormat{AddressPrefix: pointer.String("1.2.3.4/31")}}, nil).MaxTimes(1) + network.Subnet{ID: pointer.String("subnet0"), SubnetPropertiesFormat: &network.SubnetPropertiesFormat{AddressPrefixes: &[]string{"1.2.3.4/31", "2001::1/127"}}}, nil).MaxTimes(1) zoneClient := mockzoneclient.NewMockInterface(ctrl) - zoneClient.EXPECT().GetZones(gomock.Any(), gomock.Any()).Return(map[string][]string{}, tc.getZoneError).MaxTimes(1) + zoneClient.EXPECT().GetZones(gomock.Any(), gomock.Any()).Return(map[string][]string{}, tc.getZoneError).MaxTimes(2) cloud.ZoneClient = zoneClient service := tc.service + isDualStack := isServiceDualStack(&service) defaultLBFrontendIPConfigName := cloud.getDefaultFrontendIPConfigName(&service) - _, _, dirty, err := cloud.reconcileFrontendIPConfigs("testCluster", &service, &lb, tc.status, true, defaultLBFrontendIPConfigName) + lbFrontendIPConfigNames := map[bool]string{ + false: getResourceByIPFamily(defaultLBFrontendIPConfigName, isDualStack, false), + true: getResourceByIPFamily(defaultLBFrontendIPConfigName, isDualStack, true), + } + _, _, dirty, err := cloud.reconcileFrontendIPConfigs("testCluster", &service, &lb, tc.status, true, lbFrontendIPConfigNames) if tc.expectedErr == nil { assert.NoError(t, err) } else { @@ -6170,14 +6620,22 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) { } } - if tc.expectedIP != nil { - assert.Equal(t, *tc.expectedIP, pointer.StringDeref((*lb.FrontendIPConfigurations)[0].PrivateIPAddress, "")) - if *tc.expectedIP != "" { - assert.Equal(t, network.Static, (*lb.FrontendIPConfigurations)[0].PrivateIPAllocationMethod) - } else { - assert.Equal(t, network.Dynamic, (*lb.FrontendIPConfigurations)[0].PrivateIPAllocationMethod) + checkExpectedIP := func(isIPv6 bool, expectedIP *string) { + if expectedIP != nil { + for _, fip := range *lb.FrontendIPConfigurations { + if strings.EqualFold(pointer.StringDeref(fip.Name, ""), lbFrontendIPConfigNames[isIPv6]) { + assert.Equal(t, *expectedIP, pointer.StringDeref(fip.PrivateIPAddress, "")) + if *expectedIP != "" { + assert.Equal(t, network.Static, (*lb.FrontendIPConfigurations)[0].PrivateIPAllocationMethod) + } else { + assert.Equal(t, network.Dynamic, (*lb.FrontendIPConfigurations)[0].PrivateIPAllocationMethod) + } + } + } } } + checkExpectedIP(false, tc.expectedIPv4) + checkExpectedIP(true, tc.expectedIPv6) }) } } @@ -6479,7 +6937,7 @@ func TestReconcileIPSettings(t *testing.T) { az := GetTestCloud(ctrl) az.LoadBalancerSku = tc.sku pip := tc.pip - pip.Name = pointer.StringPtr("pip") + pip.Name = pointer.String("pip") service := tc.service changed := az.reconcileIPSettings(pip, &service, tc.isIPv6) assert.Equal(t, tc.expectedChanged, changed) @@ -6633,3 +7091,38 @@ func TestSafeDeleteLoadBalancer(t *testing.T) { }) } } + +func TestEqualSubResource(t *testing.T) { + testcases := []struct { + desc string + subResource1 *network.SubResource + subResource2 *network.SubResource + expected bool + }{ + { + desc: "both nil", + subResource1: nil, + subResource2: nil, + expected: true, + }, + { + desc: "one nil", + subResource1: &network.SubResource{}, + subResource2: nil, + expected: false, + }, + { + desc: "equal", + subResource1: &network.SubResource{ID: pointer.String("id")}, + subResource2: &network.SubResource{ID: pointer.String("id")}, + expected: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + equal := equalSubResource(tc.subResource1, tc.subResource2) + assert.Equal(t, tc.expected, equal) + }) + } +} diff --git a/pkg/provider/azure_privatelinkservice.go b/pkg/provider/azure_privatelinkservice.go index bdd96d3f55..f3a5c2f6ec 100644 --- a/pkg/provider/azure_privatelinkservice.go +++ b/pkg/provider/azure_privatelinkservice.go @@ -41,6 +41,17 @@ func (az *Cloud) reconcilePrivateLinkService( fipConfig *network.FrontendIPConfiguration, wantPLS bool, ) error { + isinternal := requiresInternalLoadBalancer(service) + isIPv6, err := az.isFIPIPv6(service, fipConfig, isinternal) + if err != nil { + klog.Errorf("reconcilePrivateLinkService for service(%s): failed to get FIP IP family: %v", service, err) + return err + } + if isIPv6 { + klog.V(2).Infof("IPv6 is not supported for private link service, skip reconcilePrivateLinkService for service(%s)", service) + return nil + } + createPLS := wantPLS && serviceRequiresPLS(service) serviceName := getServiceName(service) fipConfigID := fipConfig.ID @@ -59,7 +70,7 @@ func (az *Cloud) reconcilePrivateLinkService( if createPLS { // Firstly, make sure it's internal service - if !requiresInternalLoadBalancer(service) && !consts.IsK8sServiceDisableLoadBalancerFloatingIP(service) { + if !isinternal && !consts.IsK8sServiceDisableLoadBalancerFloatingIP(service) { return fmt.Errorf("reconcilePrivateLinkService for service(%s): service requiring private link service must be internal or disable floating ip", serviceName) } diff --git a/pkg/provider/azure_privatelinkservice_test.go b/pkg/provider/azure_privatelinkservice_test.go index d44f1149c5..7717c31854 100644 --- a/pkg/provider/azure_privatelinkservice_test.go +++ b/pkg/provider/azure_privatelinkservice_test.go @@ -29,6 +29,7 @@ import ( "k8s.io/utils/pointer" "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/privatelinkserviceclient/mockprivatelinkserviceclient" + "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/publicipclient/mockpublicipclient" "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/subnetclient/mocksubnetclient" "sigs.k8s.io/cloud-provider-azure/pkg/consts" "sigs.k8s.io/cloud-provider-azure/pkg/retry" @@ -317,31 +318,54 @@ func TestReconcilePrivateLinkService(t *testing.T) { expectedPLSDelete: true, }, } - for i, test := range testCases { - az := GetTestCloud(ctrl) - service := getTestServiceWithAnnotation("test", test.annotations, false, 80) - fipConfig := &network.FrontendIPConfiguration{ - Name: pointer.String("fipConfig"), - ID: pointer.String("fipConfigID"), - } - clusterName := testClusterName + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + az := GetTestCloud(ctrl) + service := getTestServiceWithAnnotation("test", test.annotations, false, 80) + fipConfig := &network.FrontendIPConfiguration{ + Name: pointer.String("fipConfig"), + ID: pointer.String("fipConfigID"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ + ID: pointer.String("pipID"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv4, + }, + }, + PrivateIPAddressVersion: network.IPv4, + }, + } + clusterName := testClusterName - mockSubnetsClient := az.SubnetsClient.(*mocksubnetclient.MockInterface) - mockPLSsClient := az.PrivateLinkServiceClient.(*mockprivatelinkserviceclient.MockInterface) - if test.expectedSubnetGet { - mockSubnetsClient.EXPECT().Get(gomock.Any(), "rg", "vnet", "subnet", "").Return(*test.existingSubnet, nil).MaxTimes(2) - } - if test.expectedPLSList { - mockPLSsClient.EXPECT().List(gomock.Any(), "rg").Return(test.existingPLSList, nil).MaxTimes(1) - } - if test.expectedPLSCreate { - mockPLSsClient.EXPECT().CreateOrUpdate(gomock.Any(), "rg", pointer.StringDeref(test.expectedPLS.Name, ""), gomock.Any(), gomock.Any()).Return(nil).Times(1) - } - if test.expectedPLSDelete { - mockPLSsClient.EXPECT().Delete(gomock.Any(), "rg", "testpls").Return(nil).Times(1) - } - err := az.reconcilePrivateLinkService(clusterName, &service, fipConfig, test.wantPLS) - assert.Equal(t, test.expectedError, err != nil, "TestCase[%d]: %s", i, test.desc) + if isInternal, ok := test.annotations[consts.ServiceAnnotationLoadBalancerInternal]; !ok || isInternal != "true" { + existingPIPS := []network.PublicIPAddress{ + { + ID: pointer.String("pipID"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv4, + }, + }, + } + mockPIPsClient := az.PublicIPAddressesClient.(*mockpublicipclient.MockInterface) + mockPIPsClient.EXPECT().List(gomock.Any(), "rg").Return(existingPIPS, nil).Times(1) + } + mockSubnetsClient := az.SubnetsClient.(*mocksubnetclient.MockInterface) + mockPLSsClient := az.PrivateLinkServiceClient.(*mockprivatelinkserviceclient.MockInterface) + if test.expectedSubnetGet { + mockSubnetsClient.EXPECT().Get(gomock.Any(), "rg", "vnet", "subnet", "").Return(*test.existingSubnet, nil).MaxTimes(2) + } + if test.expectedPLSList { + mockPLSsClient.EXPECT().List(gomock.Any(), "rg").Return(test.existingPLSList, nil).MaxTimes(1) + } + if test.expectedPLSCreate { + mockPLSsClient.EXPECT().CreateOrUpdate(gomock.Any(), "rg", pointer.StringDeref(test.expectedPLS.Name, ""), gomock.Any(), gomock.Any()).Return(nil).Times(1) + } + if test.expectedPLSDelete { + mockPLSsClient.EXPECT().Delete(gomock.Any(), "rg", "testpls").Return(nil).Times(1) + } + err := az.reconcilePrivateLinkService(clusterName, &service, fipConfig, test.wantPLS) + assert.Equal(t, test.expectedError, err != nil) + }) } } diff --git a/pkg/provider/azure_standard.go b/pkg/provider/azure_standard.go index 5fb8cbaf20..c1abc832f6 100644 --- a/pkg/provider/azure_standard.go +++ b/pkg/provider/azure_standard.go @@ -96,6 +96,13 @@ func (az *Cloud) getBackendPoolIDWithRG(lbName, rgName, backendPoolName string) backendPoolName) } +func (az *Cloud) getBackendPoolIDs(clusterName, lbName string) map[bool]string { + return map[bool]string{ + false: az.getBackendPoolID(lbName, getBackendPoolName(clusterName, false)), + true: az.getBackendPoolID(lbName, getBackendPoolName(clusterName, true)), + } +} + // returns the full identifier of a loadbalancer probe. func (az *Cloud) getLoadBalancerProbeID(lbName, lbRuleName string) string { return az.getLoadBalancerProbeIDWithRG(lbName, az.getLoadBalancerResourceGroup(), lbRuleName) @@ -286,6 +293,14 @@ func getBackendPoolName(clusterName string, isIPv6 bool) string { return clusterName } +// getBackendPoolNames returns the IPv4 and IPv6 backend pool names. +func getBackendPoolNames(clusterName string) map[bool]string { + return map[bool]string{ + false: getBackendPoolName(clusterName, false), + true: getBackendPoolName(clusterName, true), + } +} + // ifBackendPoolIPv6 checks if a backend pool is of IPv6 according to name/ID. func isBackendPoolIPv6(name string) bool { return strings.HasSuffix(name, fmt.Sprintf("-%s", v6Suffix)) @@ -294,10 +309,10 @@ func isBackendPoolIPv6(name string) bool { 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) + subnet := getInternalSubnet(service) + isDualStack := isServiceDualStack(service) if subnet == nil { - // TODO: Use getResourceByIPFamily() - return ruleName + return getResourceByIPFamily(ruleName, isDualStack, isIPv6) } // Load balancer rule name must be less or equal to 80 characters, so excluding the hyphen two segments cannot exceed 79 @@ -307,8 +322,7 @@ func (az *Cloud) getLoadBalancerRuleName(service *v1.Service, protocol v1.Protoc subnetSegment = subnetSegment[:maxLength-len(ruleName)-1] } - // TODO: Use getResourceByIPFamily() - return fmt.Sprintf("%s-%s-%s-%d", prefix, subnetSegment, protocol, port) + return getResourceByIPFamily(fmt.Sprintf("%s-%s-%s-%d", prefix, subnetSegment, protocol, port), isDualStack, isIPv6) } func (az *Cloud) getloadbalancerHAmodeRuleName(service *v1.Service, isIPv6 bool) string { @@ -393,11 +407,11 @@ func (az *Cloud) serviceOwnsFrontendIP(fip network.FrontendIPConfiguration, serv 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) + klog.V(6).Infof("serviceOwnsFrontendIP:found secondary service %s of the frontend IP config %s", service.Name, *fip.Name) return true, isPrimaryService, nil } } - klog.Infof("serviceOwnsFrontendIP: the public IP with ID %s is being referenced by other service with public IP address %s "+ + klog.V(6).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) } @@ -419,9 +433,18 @@ func (az *Cloud) serviceOwnsFrontendIP(fip network.FrontendIPConfiguration, serv return privateIPEquals, isPrimaryService, nil } +func (az *Cloud) getFrontendIPConfigNames(service *v1.Service) map[bool]string { + isDualStack := isServiceDualStack(service) + defaultLBFrontendIPConfigName := az.getDefaultFrontendIPConfigName(service) + return map[bool]string{ + false: getResourceByIPFamily(defaultLBFrontendIPConfigName, isDualStack, false), + true: getResourceByIPFamily(defaultLBFrontendIPConfigName, isDualStack, true), + } +} + func (az *Cloud) getDefaultFrontendIPConfigName(service *v1.Service) string { baseName := az.GetLoadBalancerName(context.TODO(), "", service) - subnetName := subnet(service) + subnetName := getInternalSubnet(service) if subnetName != nil { ipcName := fmt.Sprintf("%s-%s", baseName, *subnetName) diff --git a/pkg/provider/azure_standard_test.go b/pkg/provider/azure_standard_test.go index 314a974f22..0e6804b24a 100644 --- a/pkg/provider/azure_standard_test.go +++ b/pkg/provider/azure_standard_test.go @@ -33,7 +33,6 @@ 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" @@ -357,6 +356,9 @@ func TestGetLoadBalancingRuleName(t *testing.T) { Annotations: map[string]string{}, UID: "257b9655-5137-4ad2-b091-ef3f07043ad3", }, + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, + }, } cases := []struct { @@ -365,6 +367,7 @@ func TestGetLoadBalancingRuleName(t *testing.T) { expected string protocol v1.Protocol isInternal bool + isIPv6 bool useStandardLB bool port int32 }{ @@ -377,6 +380,16 @@ func TestGetLoadBalancingRuleName(t *testing.T) { port: 9000, expected: "a257b965551374ad2b091ef3f07043ad-shortsubnet-TCP-9000", }, + { + description: "internal lb should have subnet name on the rule name IPv6", + subnetName: "shortsubnet", + isInternal: true, + isIPv6: true, + useStandardLB: true, + protocol: v1.ProtocolTCP, + port: 9000, + expected: "a257b965551374ad2b091ef3f07043ad-shortsubnet-TCP-9000-IPv6", + }, { description: "internal standard lb should have subnet name on the rule name but truncated to 80 (-5) characters", subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet", @@ -386,6 +399,16 @@ func TestGetLoadBalancingRuleName(t *testing.T) { port: 9000, expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnn-TCP-9000", }, + { + description: "internal standard lb should have subnet name on the rule name but truncated to 80 (-5) characters IPv6", + subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet", + isInternal: true, + isIPv6: true, + useStandardLB: true, + protocol: v1.ProtocolTCP, + port: 9000, + expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnn-TCP-9000-IPv6", + }, { description: "internal basic lb should have subnet name on the rule name but truncated to 80 (-5) characters", subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet", @@ -395,6 +418,16 @@ func TestGetLoadBalancingRuleName(t *testing.T) { port: 9000, expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnn-TCP-9000", }, + { + description: "internal basic lb should have subnet name on the rule name but truncated to 80 (-5) characters IPv6", + subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet", + isInternal: true, + isIPv6: true, + useStandardLB: false, + protocol: v1.ProtocolTCP, + port: 9000, + expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnn-TCP-9000-IPv6", + }, { description: "external standard lb should not have subnet name on the rule name", subnetName: "shortsubnet", @@ -404,6 +437,16 @@ func TestGetLoadBalancingRuleName(t *testing.T) { port: 9000, expected: "a257b965551374ad2b091ef3f07043ad-TCP-9000", }, + { + description: "external standard lb should not have subnet name on the rule name IPv6", + subnetName: "shortsubnet", + isInternal: false, + isIPv6: true, + useStandardLB: true, + protocol: v1.ProtocolTCP, + port: 9000, + expected: "a257b965551374ad2b091ef3f07043ad-TCP-9000-IPv6", + }, { description: "external basic lb should not have subnet name on the rule name", subnetName: "shortsubnet", @@ -413,20 +456,80 @@ func TestGetLoadBalancingRuleName(t *testing.T) { port: 9000, expected: "a257b965551374ad2b091ef3f07043ad-TCP-9000", }, + { + description: "external basic lb should not have subnet name on the rule name IPv6", + subnetName: "shortsubnet", + isInternal: false, + isIPv6: true, + useStandardLB: false, + protocol: v1.ProtocolTCP, + port: 9000, + expected: "a257b965551374ad2b091ef3f07043ad-TCP-9000-IPv6", + }, } for _, c := range cases { - if c.useStandardLB { - az.Config.LoadBalancerSku = consts.LoadBalancerSkuStandard - } else { - az.Config.LoadBalancerSku = consts.LoadBalancerSkuBasic - } - svc.Annotations[consts.ServiceAnnotationLoadBalancerInternalSubnet] = c.subnetName - svc.Annotations[consts.ServiceAnnotationLoadBalancerInternal] = strconv.FormatBool(c.isInternal) + t.Run(c.description, func(t *testing.T) { + if c.useStandardLB { + az.Config.LoadBalancerSku = consts.LoadBalancerSkuStandard + } else { + az.Config.LoadBalancerSku = consts.LoadBalancerSkuBasic + } + svc.Annotations[consts.ServiceAnnotationLoadBalancerInternalSubnet] = c.subnetName + svc.Annotations[consts.ServiceAnnotationLoadBalancerInternal] = strconv.FormatBool(c.isInternal) - isIPv6 := utilnet.IsIPv6String(svc.Spec.ClusterIP) - loadbalancerRuleName := az.getLoadBalancerRuleName(svc, c.protocol, c.port, isIPv6) - assert.Equal(t, c.expected, loadbalancerRuleName, c.description) + loadbalancerRuleName := az.getLoadBalancerRuleName(svc, c.protocol, c.port, c.isIPv6) + assert.Equal(t, c.expected, loadbalancerRuleName) + }) + } +} + +func TestGetloadbalancerHAmodeRuleName(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + az := GetTestCloud(ctrl) + az.PrimaryAvailabilitySetName = primary + + svc := &v1.Service{ + ObjectMeta: meta.ObjectMeta{ + UID: "257b9655-5137-4ad2-b091-ef3f07043ad3", + Annotations: map[string]string{ + consts.ServiceAnnotationLoadBalancerInternal: "true", + consts.ServiceAnnotationLoadBalancerInternalSubnet: "shortsubnet", + }, + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Port: 9000, + Protocol: v1.ProtocolTCP, + }, + }, + IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, + }, + } + + cases := []struct { + description string + expected string + isIPv6 bool + }{ + { + description: "IPv4", + expected: "a257b965551374ad2b091ef3f07043ad-shortsubnet-TCP-9000", + }, + { + description: "IPv6", + isIPv6: true, + expected: "a257b965551374ad2b091ef3f07043ad-shortsubnet-TCP-9000-IPv6", + }, + } + + for _, c := range cases { + t.Run(c.description, func(t *testing.T) { + lbHAModeRuleName := az.getloadbalancerHAmodeRuleName(svc, c.isIPv6) + assert.Equal(t, c.expected, lbHAModeRuleName) + }) } } @@ -508,7 +611,61 @@ func TestGetFrontendIPConfigName(t *testing.T) { svc.Annotations[consts.ServiceAnnotationLoadBalancerInternal] = strconv.FormatBool(c.isInternal) ipconfigName := az.getDefaultFrontendIPConfigName(svc) - assert.Equal(t, c.expected, ipconfigName, c) + assert.Equal(t, c.expected, ipconfigName) + }) + } +} + +func TestGetFrontendIPConfigNames(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + az := GetTestCloud(ctrl) + az.PrimaryAvailabilitySetName = primary + + svc := &v1.Service{ + ObjectMeta: meta.ObjectMeta{ + Annotations: map[string]string{ + consts.ServiceAnnotationLoadBalancerInternalSubnet: "subnet", + consts.ServiceAnnotationLoadBalancerInternal: "true", + }, + UID: "257b9655-5137-4ad2-b091-ef3f07043ad3", + }, + Spec: v1.ServiceSpec{ + IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, + }, + } + + cases := []struct { + description string + subnetName string + isInternal bool + useStandardLB bool + expectedV4 string + expectedV6 string + }{ + { + description: "internal lb should have subnet name on the frontend ip configuration name", + subnetName: "shortsubnet", + isInternal: true, + useStandardLB: true, + expectedV4: "a257b965551374ad2b091ef3f07043ad-shortsubnet", + expectedV6: "a257b965551374ad2b091ef3f07043ad-shortsubnet-IPv6", + }, + } + + for _, c := range cases { + t.Run(c.description, func(t *testing.T) { + if c.useStandardLB { + az.Config.LoadBalancerSku = consts.LoadBalancerSkuStandard + } else { + az.Config.LoadBalancerSku = consts.LoadBalancerSkuBasic + } + svc.Annotations[consts.ServiceAnnotationLoadBalancerInternalSubnet] = c.subnetName + svc.Annotations[consts.ServiceAnnotationLoadBalancerInternal] = strconv.FormatBool(c.isInternal) + + ipconfigNames := az.getFrontendIPConfigNames(svc) + assert.Equal(t, c.expectedV4, ipconfigNames[false]) + assert.Equal(t, c.expectedV6, ipconfigNames[true]) }) } } @@ -597,6 +754,83 @@ func testGetLoadBalancerSubResourceID( } } +func TestGetBackendPoolIDs(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + az := GetTestCloud(ctrl) + + testGetLoadBalancerSubResourceIDs(t, az, az.getBackendPoolIDs, consts.BackendPoolIDTemplate) +} + +func testGetLoadBalancerSubResourceIDs( + t *testing.T, + az *Cloud, + getLoadBalancerSubResourceIDs func(string, string) map[bool]string, + expectedResourceIDTemplate string) { + clusterName := "azure" + rgName := "rgName" + + cases := []struct { + description string + loadBalancerName string + resourceGroupName string + useNetworkResourceInDifferentTenant bool + useNetworkResourceInDifferentSub bool + }{ + { + description: "resource id should contain NetworkResourceSubscriptionID when using network resources in different tenant and subscription", + loadBalancerName: "lbName", + useNetworkResourceInDifferentTenant: true, + useNetworkResourceInDifferentSub: true, + }, + { + description: "resource id should contain NetworkResourceSubscriptionID when using network resources in different subscription", + loadBalancerName: "lbName", + useNetworkResourceInDifferentTenant: false, + useNetworkResourceInDifferentSub: true, + }, + { + description: "resource id should contain SubscriptionID when not using network resources in different subscription", + loadBalancerName: "lbName", + useNetworkResourceInDifferentTenant: false, + useNetworkResourceInDifferentSub: false, + }, + } + + for _, c := range cases { + t.Run(c.description, func(t *testing.T) { + az.ResourceGroup = rgName + subscriptionID := az.SubscriptionID + if c.useNetworkResourceInDifferentTenant { + az.NetworkResourceTenantID = networkResourceTenantID + } else { + az.NetworkResourceTenantID = "" + } + if c.useNetworkResourceInDifferentSub { + az.NetworkResourceSubscriptionID = networkResourceSubscriptionID + subscriptionID = networkResourceSubscriptionID + } else { + az.NetworkResourceSubscriptionID = "" + } + expectedV4 := fmt.Sprintf( + expectedResourceIDTemplate, + subscriptionID, + rgName, + c.loadBalancerName, + clusterName) + expectedV6 := fmt.Sprintf( + expectedResourceIDTemplate, + subscriptionID, + rgName, + c.loadBalancerName, + clusterName) + "-" + v6Suffix + subResourceIDs := getLoadBalancerSubResourceIDs(clusterName, c.loadBalancerName) + assert.Equal(t, expectedV4, subResourceIDs[false]) + assert.Equal(t, expectedV6, subResourceIDs[true]) + }) + } +} + func TestGetProtocolsFromKubernetesProtocol(t *testing.T) { testcases := []struct { Name string @@ -924,6 +1158,26 @@ func TestGetBackendPoolName(t *testing.T) { } } +func TestGetBackendPoolNames(t *testing.T) { + testcases := []struct { + name string + service v1.Service + clusterName string + expectedPoolNames map[bool]string + }{ + { + name: "GetBackendPoolNames should return 2 backend pool names", + service: getTestService("test1", v1.ProtocolTCP, nil, true, 80), + clusterName: "azure", + expectedPoolNames: map[bool]string{false: "azure", true: "azure-IPv6"}, + }, + } + for _, test := range testcases { + backPoolNames := getBackendPoolNames(test.clusterName) + assert.Equal(t, test.expectedPoolNames, backPoolNames) + } +} + func TestIsBackendPoolIPv6(t *testing.T) { testcases := []struct { name string diff --git a/pkg/provider/azure_test.go b/pkg/provider/azure_test.go index f062f7f362..6f02949e88 100644 --- a/pkg/provider/azure_test.go +++ b/pkg/provider/azure_test.go @@ -116,9 +116,7 @@ func TestAddPort(t *testing.T) { mockLBBackendPool.EXPECT().EnsureHostsInPool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() lb, err := az.reconcileLoadBalancer(testClusterName, &svc, clusterResources.nodes, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) // ensure we got a frontend ip configuration if len(*lb.FrontendIPConfigurations) != 1 { @@ -144,6 +142,27 @@ func TestLoadBalancerExternalServiceModeSelection(t *testing.T) { testLoadBalancerServiceAutoModeDeleteSelection(t, false) } +func setMockEnvDualStack(az *Cloud, ctrl *gomock.Controller, expectedInterfaces []network.Interface, expectedVirtualMachines []compute.VirtualMachine, serviceCount int) { + mockInterfacesClient := mockinterfaceclient.NewMockInterface(ctrl) + az.InterfacesClient = mockInterfacesClient + for i := range expectedInterfaces { + mockInterfacesClient.EXPECT().Get(gomock.Any(), az.ResourceGroup, fmt.Sprintf("vm-%d", i), gomock.Any()).Return(expectedInterfaces[i], nil).AnyTimes() + mockInterfacesClient.EXPECT().CreateOrUpdate(gomock.Any(), az.ResourceGroup, fmt.Sprintf("vm-%d", i), gomock.Any()).Return(nil).AnyTimes() + } + + mockVirtualMachinesClient := mockvmclient.NewMockInterface(ctrl) + az.VirtualMachinesClient = mockVirtualMachinesClient + mockVirtualMachinesClient.EXPECT().List(gomock.Any(), az.ResourceGroup).Return(expectedVirtualMachines, nil).AnyTimes() + for i := range expectedVirtualMachines { + mockVirtualMachinesClient.EXPECT().Get(gomock.Any(), az.ResourceGroup, fmt.Sprintf("vm-%d", i), gomock.Any()).Return(expectedVirtualMachines[i], nil).AnyTimes() + } + + setMockPublicIPs(az, ctrl, serviceCount, true, true) + + sg := getTestSecurityGroup(az) + setMockSecurityGroup(az, ctrl, sg) +} + func setMockEnv(az *Cloud, ctrl *gomock.Controller, expectedInterfaces []network.Interface, expectedVirtualMachines []compute.VirtualMachine, serviceCount int, services ...v1.Service) { mockInterfacesClient := mockinterfaceclient.NewMockInterface(ctrl) az.InterfacesClient = mockInterfacesClient @@ -174,12 +193,16 @@ func setMockEnv(az *Cloud, ctrl *gomock.Controller, expectedInterfaces []network func setMockPublicIPs(az *Cloud, ctrl *gomock.Controller, serviceCount int, v4Enabled, v6Enabled bool) { mockPIPsClient := mockpublicipclient.NewMockInterface(ctrl) + expectedPIPsTotal := []network.PublicIPAddress{} if v4Enabled { - setMockPublicIP(az, mockPIPsClient, serviceCount, false) + expectedPIPs := setMockPublicIP(az, mockPIPsClient, serviceCount, false) + expectedPIPsTotal = append(expectedPIPsTotal, expectedPIPs...) } if v6Enabled { - setMockPublicIP(az, mockPIPsClient, serviceCount, true) + expectedPIPs := setMockPublicIP(az, mockPIPsClient, serviceCount, true) + expectedPIPsTotal = append(expectedPIPsTotal, expectedPIPs...) } + mockPIPsClient.EXPECT().List(gomock.Any(), az.ResourceGroup).Return(expectedPIPsTotal, nil).AnyTimes() az.PublicIPAddressesClient = mockPIPsClient mockPIPsClient.EXPECT().CreateOrUpdate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() @@ -187,14 +210,16 @@ func setMockPublicIPs(az *Cloud, ctrl *gomock.Controller, serviceCount int, v4En mockPIPsClient.EXPECT().Get(gomock.Any(), gomock.Not(az.ResourceGroup), gomock.Any(), gomock.Any()).Return(network.PublicIPAddress{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes() } -func setMockPublicIP(az *Cloud, mockPIPsClient *mockpublicipclient.MockInterface, serviceCount int, isIPv6 bool) { +func setMockPublicIP(az *Cloud, mockPIPsClient *mockpublicipclient.MockInterface, serviceCount int, isIPv6 bool) []network.PublicIPAddress { suffix := "" ipVer := network.IPv4 - ipAddr := "1.2.3.4" + ipAddr1 := "1.2.3.4" + ipAddra := "1.2.3.5" if isIPv6 { suffix = "-" + v6Suffix ipVer = network.IPv6 - ipAddr = "fd00::eef0" + ipAddr1 = "fd00::eef0" + ipAddra = "fd00::eef1" } expectedPIP := network.PublicIPAddress{ @@ -203,7 +228,7 @@ func setMockPublicIP(az *Cloud, mockPIPsClient *mockpublicipclient.MockInterface PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ PublicIPAllocationMethod: network.Static, PublicIPAddressVersion: ipVer, - IPAddress: pointer.String(ipAddr), + IPAddress: pointer.String(ipAddr1), }, Tags: map[string]*string{ consts.ServiceTagKey: pointer.String("default/servicea"), @@ -219,18 +244,30 @@ func setMockPublicIP(az *Cloud, mockPIPsClient *mockpublicipclient.MockInterface var expectedPIPs []network.PublicIPAddress for i := 1; i <= serviceCount; i++ { expectedPIP.Name = pointer.String(fmt.Sprintf("testCluster-aservice%d%s", i, suffix)) + expectedPIP.ID = pointer.String(fmt.Sprintf("testCluster-aservice%d%s", i, suffix)) + expectedPIP.PublicIPAddressPropertiesFormat = &network.PublicIPAddressPropertiesFormat{ + PublicIPAllocationMethod: network.Static, + PublicIPAddressVersion: ipVer, + IPAddress: pointer.String(ipAddr1), + } expectedPIP.Tags[consts.ServiceTagKey] = pointer.String(fmt.Sprintf("default/service%d", i)) mockPIPsClient.EXPECT().Get(gomock.Any(), az.ResourceGroup, fmt.Sprintf("testCluster-aservice%d%s", i, suffix), gomock.Any()).Return(expectedPIP, nil).AnyTimes() mockPIPsClient.EXPECT().Delete(gomock.Any(), az.ResourceGroup, fmt.Sprintf("testCluster-aservice%d%s", i, suffix)).Return(nil).AnyTimes() expectedPIPs = append(expectedPIPs, expectedPIP) expectedPIP.Name = pointer.String(fmt.Sprintf("testCluster-aservice%c%s", a, suffix)) + expectedPIP.ID = pointer.String(fmt.Sprintf("testCluster-aservice%c%s", a, suffix)) + expectedPIP.PublicIPAddressPropertiesFormat = &network.PublicIPAddressPropertiesFormat{ + PublicIPAllocationMethod: network.Static, + PublicIPAddressVersion: ipVer, + IPAddress: pointer.String(ipAddra), + } expectedPIP.Tags[consts.ServiceTagKey] = pointer.String(fmt.Sprintf("default/service%c", a)) mockPIPsClient.EXPECT().Get(gomock.Any(), az.ResourceGroup, fmt.Sprintf("testCluster-aservice%c%s", a, suffix), gomock.Any()).Return(expectedPIP, nil).AnyTimes() mockPIPsClient.EXPECT().Delete(gomock.Any(), az.ResourceGroup, fmt.Sprintf("testCluster-aservice%c%s", a, suffix)).Return(nil).AnyTimes() expectedPIPs = append(expectedPIPs, expectedPIP) a++ } - mockPIPsClient.EXPECT().List(gomock.Any(), az.ResourceGroup).Return(expectedPIPs, nil).AnyTimes() + return expectedPIPs } func setMockSecurityGroup(az *Cloud, ctrl *gomock.Controller, sgs ...*network.SecurityGroup) { @@ -242,6 +279,122 @@ func setMockSecurityGroup(az *Cloud, ctrl *gomock.Controller, sgs ...*network.Se mockSGsClient.EXPECT().CreateOrUpdate(gomock.Any(), az.SecurityGroupResourceGroup, az.SecurityGroupName, gomock.Any(), gomock.Any()).Return(nil).AnyTimes() } +func setMockLBsDualStack(az *Cloud, ctrl *gomock.Controller, expectedLBs *[]network.LoadBalancer, svcName string, lbCount, serviceIndex int, isInternal bool) string { + lbIndex := (serviceIndex - 1) % lbCount + expectedLBName := "" + if lbIndex == 0 { + expectedLBName = testClusterName + } else { + expectedLBName = fmt.Sprintf("as-%d", lbIndex) + } + if isInternal { + expectedLBName += "-internal" + } + + fullServiceName := strings.Replace(svcName, "-", "", -1) + + if lbIndex >= len(*expectedLBs) { + lb := network.LoadBalancer{ + Location: &az.Location, + LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{ + BackendAddressPools: &[]network.BackendAddressPool{ + { + Name: pointer.String("testCluster"), + }, + { + Name: pointer.String("testCluster-IPv6"), + }, + }, + }, + } + lb.Name = &expectedLBName + lb.LoadBalancingRules = &[]network.LoadBalancingRule{ + { + Name: pointer.String(fmt.Sprintf("a%s%d-TCP-8081", fullServiceName, serviceIndex)), + }, + { + Name: pointer.String(fmt.Sprintf("a%s%d-TCP-8081-IPv6", fullServiceName, serviceIndex)), + }, + } + fips := []network.FrontendIPConfiguration{ + { + Name: pointer.String(fmt.Sprintf("a%s%d", fullServiceName, serviceIndex)), + ID: pointer.String("fip"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PrivateIPAllocationMethod: "Dynamic", + PrivateIPAddressVersion: network.IPv4, + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String(fmt.Sprintf("testCluster-a%s%d", fullServiceName, serviceIndex))}, + }, + }, + { + Name: pointer.String(fmt.Sprintf("a%s%d-IPv6", fullServiceName, serviceIndex)), + ID: pointer.String("fip-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PrivateIPAllocationMethod: "Dynamic", + PrivateIPAddressVersion: network.IPv6, + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String(fmt.Sprintf("testCluster-a%s%d-IPv6", fullServiceName, serviceIndex))}, + }, + }, + } + if isInternal { + fips[0].Subnet = &network.Subnet{Name: pointer.String("subnet")} + fips[1].Subnet = &network.Subnet{Name: pointer.String("subnet")} + } + lb.FrontendIPConfigurations = &fips + + *expectedLBs = append(*expectedLBs, lb) + } else { + lbRules := []network.LoadBalancingRule{ + { + Name: pointer.String(fmt.Sprintf("a%s%d-TCP-8081", fullServiceName, serviceIndex)), + }, + { + Name: pointer.String(fmt.Sprintf("a%s%d-TCP-8081-IPv6", fullServiceName, serviceIndex)), + }, + } + *(*expectedLBs)[lbIndex].LoadBalancingRules = append(*(*expectedLBs)[lbIndex].LoadBalancingRules, lbRules...) + fips := []network.FrontendIPConfiguration{ + { + Name: pointer.String(fmt.Sprintf("a%s%d", fullServiceName, serviceIndex)), + ID: pointer.String("fip"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PrivateIPAllocationMethod: "Dynamic", + PrivateIPAddressVersion: network.IPv4, + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String(fmt.Sprintf("testCluster-a%s%d", fullServiceName, serviceIndex))}, + }, + }, + { + Name: pointer.String(fmt.Sprintf("a%s%d-IPv6", fullServiceName, serviceIndex)), + ID: pointer.String("fip-IPv6"), + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PrivateIPAllocationMethod: "Dynamic", + PrivateIPAddressVersion: network.IPv6, + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String(fmt.Sprintf("testCluster-a%s%d-IPv6", fullServiceName, serviceIndex))}, + }, + }, + } + if isInternal { + for _, fip := range fips { + fip.Subnet = &network.Subnet{Name: pointer.String("subnet")} + } + } + *(*expectedLBs)[lbIndex].FrontendIPConfigurations = append(*(*expectedLBs)[lbIndex].FrontendIPConfigurations, fips...) + } + + mockLBsClient := mockloadbalancerclient.NewMockInterface(ctrl) + az.LoadBalancerClient = mockLBsClient + mockLBsClient.EXPECT().CreateOrUpdate(gomock.Any(), az.ResourceGroup, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + for _, lb := range *expectedLBs { + mockLBsClient.EXPECT().Get(gomock.Any(), az.ResourceGroup, *lb.Name, gomock.Any()).Return((*expectedLBs)[lbIndex], nil).MaxTimes(2) + } + mockLBsClient.EXPECT().List(gomock.Any(), az.ResourceGroup).Return(*expectedLBs, nil).MaxTimes(4) + mockLBsClient.EXPECT().List(gomock.Any(), gomock.Not(az.ResourceGroup)).Return([]network.LoadBalancer{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes() + mockLBsClient.EXPECT().Get(gomock.Any(), gomock.Not(az.ResourceGroup), gomock.Any(), gomock.Any()).Return(network.LoadBalancer{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes() + mockLBsClient.EXPECT().Delete(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).MaxTimes(1) + + return expectedLBName +} + func setMockLBs(az *Cloud, ctrl *gomock.Controller, expectedLBs *[]network.LoadBalancer, svcName string, lbCount, serviceIndex int, isInternal bool) string { lbIndex := (serviceIndex - 1) % lbCount expectedLBName := "" @@ -713,28 +866,23 @@ func TestReconcileLoadBalancerAddServiceOnInternalSubnet(t *testing.T) { az := GetTestCloud(ctrl) clusterResources, expectedInterfaces, expectedVirtualMachines := getClusterResources(az, 1, 1) - setMockEnv(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) + setMockEnvDualStack(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) - svc := getInternalTestService("service1", 80) + svc := getInternalTestServiceDualStack("service1", 80) validateTestSubnet(t, az, &svc) expectedLBs := make([]network.LoadBalancer, 0) - setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, true) + setMockLBsDualStack(az, ctrl, &expectedLBs, "service", 1, 1, true) mockLBBackendPool := az.LoadBalancerBackendPool.(*MockBackendPool) mockLBBackendPool.EXPECT().ReconcileBackendPools(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, false, nil).AnyTimes() mockLBBackendPool.EXPECT().EnsureHostsInPool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() lb, err := az.reconcileLoadBalancer(testClusterName, &svc, clusterResources.nodes, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } - - // ensure we got a frontend ip configuration - if len(*lb.FrontendIPConfigurations) != 1 { - t.Error("Expected the loadbalancer to have a frontend ip configuration") - } + assert.Nil(t, err) + // ensure we got 2 frontend ip configurations + assert.Equal(t, 2, len(*lb.FrontendIPConfigurations)) validateLoadBalancer(t, lb, svc) } @@ -751,14 +899,10 @@ func TestReconcileSecurityGroupFromAnyDestinationAddressPrefixToLoadBalancerIP(t // Simulate a pre-Kubernetes 1.8 NSG, where we do not specify the destination address prefix _, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(""), nil, true) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) 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) - } + assert.Nil(t, err) validateSecurityGroup(t, sg, svc1) } @@ -775,9 +919,7 @@ func TestReconcileSecurityGroupDynamicLoadBalancerIP(t *testing.T) { dynamicallyAssignedIP := "192.168.0.0" sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, pointer.String(dynamicallyAssignedIP), nil, true) - if err != nil { - t.Errorf("unexpected error: %q", err) - } + assert.Nil(t, err) validateSecurityGroup(t, sg, svc1) } @@ -788,13 +930,13 @@ func TestReconcileLoadBalancerAddServicesOnMultipleSubnets(t *testing.T) { az := GetTestCloud(ctrl) clusterResources, expectedInterfaces, expectedVirtualMachines := getClusterResources(az, 1, 1) - setMockEnv(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) + setMockEnvDualStack(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) - svc1 := getTestService("service1", v1.ProtocolTCP, nil, false, 8081) - svc2 := getInternalTestService("service2", 8081) + svc1 := getTestServiceDualStack("service1", v1.ProtocolTCP, nil, 8081) + svc2 := getInternalTestServiceDualStack("service2", 8081) expectedLBs := make([]network.LoadBalancer, 0) - setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, false) + setMockLBsDualStack(az, ctrl, &expectedLBs, "service", 1, 1, false) mockLBBackendPool := az.LoadBalancerBackendPool.(*MockBackendPool) mockLBBackendPool.EXPECT().ReconcileBackendPools(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, false, nil).AnyTimes() @@ -807,9 +949,7 @@ func TestReconcileLoadBalancerAddServicesOnMultipleSubnets(t *testing.T) { } // ensure we got a frontend ip configuration for each service - if len(*lb.FrontendIPConfigurations) != 1 { - t.Error("Expected the loadbalancer to have 1 frontend ip configurations") - } + assert.Equal(t, 2, len(*lb.FrontendIPConfigurations)) validateLoadBalancer(t, lb, svc1) @@ -817,7 +957,7 @@ func TestReconcileLoadBalancerAddServicesOnMultipleSubnets(t *testing.T) { validateTestSubnet(t, az, &svc2) expectedLBs = make([]network.LoadBalancer, 0) - setMockLBs(az, ctrl, &expectedLBs, "service", 1, 2, true) + setMockLBsDualStack(az, ctrl, &expectedLBs, "service", 1, 2, true) // svc2 is using LB with "-internal" suffix lb, err = az.reconcileLoadBalancer(testClusterName, &svc2, clusterResources.nodes, true /* wantLb */) @@ -826,9 +966,7 @@ func TestReconcileLoadBalancerAddServicesOnMultipleSubnets(t *testing.T) { } // ensure we got a frontend ip configuration for each service - if len(*lb.FrontendIPConfigurations) != 1 { - t.Error("Expected the loadbalancer to have 1 frontend ip configurations") - } + assert.Equal(t, 2, len(*lb.FrontendIPConfigurations)) validateLoadBalancer(t, lb, svc2) } @@ -840,13 +978,13 @@ func TestReconcileLoadBalancerEditServiceSubnet(t *testing.T) { az := GetTestCloud(ctrl) clusterResources, expectedInterfaces, expectedVirtualMachines := getClusterResources(az, 1, 1) - setMockEnv(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) + setMockEnvDualStack(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) - svc := getInternalTestService("service1", 8081) + svc := getInternalTestServiceDualStack("service1", 8081) validateTestSubnet(t, az, &svc) expectedLBs := make([]network.LoadBalancer, 0) - setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, true) + setMockLBsDualStack(az, ctrl, &expectedLBs, "service", 1, 1, true) mockLBBackendPool := az.LoadBalancerBackendPool.(*MockBackendPool) mockLBBackendPool.EXPECT().ReconcileBackendPools(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, false, nil).AnyTimes() @@ -867,7 +1005,7 @@ func TestReconcileLoadBalancerEditServiceSubnet(t *testing.T) { validateTestSubnet(t, az, &svc) expectedLBs = make([]network.LoadBalancer, 0) - setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, true) + setMockLBsDualStack(az, ctrl, &expectedLBs, "service", 1, 1, true) lb, err = az.reconcileLoadBalancer(testClusterName, &svc, clusterResources.nodes, true /* wantLb */) if err != nil { @@ -875,9 +1013,7 @@ func TestReconcileLoadBalancerEditServiceSubnet(t *testing.T) { } // ensure we got a frontend ip configuration for the service - if len(*lb.FrontendIPConfigurations) != 1 { - t.Error("Expected the loadbalancer to have 1 frontend ip configuration") - } + assert.Equal(t, 2, len(*lb.FrontendIPConfigurations)) validateLoadBalancer(t, lb, svc) } @@ -888,28 +1024,24 @@ func TestReconcileLoadBalancerNodeHealth(t *testing.T) { az := GetTestCloud(ctrl) clusterResources, expectedInterfaces, expectedVirtualMachines := getClusterResources(az, 1, 1) - setMockEnv(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) + setMockEnvDualStack(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) - svc := getTestService("service1", v1.ProtocolTCP, nil, false, 80) + svc := getTestServiceDualStack("service1", v1.ProtocolTCP, nil, 80) svc.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeLocal svc.Spec.HealthCheckNodePort = int32(32456) expectedLBs := make([]network.LoadBalancer, 0) - setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, false) + setMockLBsDualStack(az, ctrl, &expectedLBs, "service", 1, 1, false) mockLBBackendPool := az.LoadBalancerBackendPool.(*MockBackendPool) mockLBBackendPool.EXPECT().ReconcileBackendPools(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, false, nil).AnyTimes() mockLBBackendPool.EXPECT().EnsureHostsInPool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() lb, err := az.reconcileLoadBalancer(testClusterName, &svc, clusterResources.nodes, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) // ensure we got a frontend ip configuration - if len(*lb.FrontendIPConfigurations) != 1 { - t.Error("Expected the loadbalancer to have a frontend ip configuration") - } + assert.Equal(t, 2, len(*lb.FrontendIPConfigurations)) validateLoadBalancer(t, lb, svc) } @@ -921,21 +1053,19 @@ func TestReconcileLoadBalancerRemoveService(t *testing.T) { az := GetTestCloud(ctrl) clusterResources, expectedInterfaces, expectedVirtualMachines := getClusterResources(az, 1, 1) - setMockEnv(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) + setMockEnvDualStack(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) - svc := getTestService("service1", v1.ProtocolTCP, nil, false, 80, 443) + svc := getTestServiceDualStack("service1", v1.ProtocolTCP, nil, 80, 443) expectedLBs := make([]network.LoadBalancer, 0) - setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, false) + setMockLBsDualStack(az, ctrl, &expectedLBs, "service", 1, 1, false) mockLBBackendPool := az.LoadBalancerBackendPool.(*MockBackendPool) mockLBBackendPool.EXPECT().ReconcileBackendPools(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, false, nil).AnyTimes() mockLBBackendPool.EXPECT().EnsureHostsInPool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() _, err := az.reconcileLoadBalancer(testClusterName, &svc, clusterResources.nodes, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) expectedLBs[0].FrontendIPConfigurations = &[]network.FrontendIPConfiguration{} mockLBsClient := mockloadbalancerclient.NewMockInterface(ctrl) @@ -946,14 +1076,10 @@ func TestReconcileLoadBalancerRemoveService(t *testing.T) { mockLBsClient.EXPECT().Delete(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) lb, err := az.reconcileLoadBalancer(testClusterName, &svc, clusterResources.nodes, false /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) // ensure we abandoned the frontend ip configuration - if len(*lb.FrontendIPConfigurations) != 0 { - t.Error("Expected the loadbalancer to have no frontend ip configuration") - } + assert.Zero(t, len(*lb.FrontendIPConfigurations)) validateLoadBalancer(t, lb) } @@ -965,24 +1091,22 @@ func TestReconcileLoadBalancerRemoveAllPortsRemovesFrontendConfig(t *testing.T) az := GetTestCloud(ctrl) clusterResources, expectedInterfaces, expectedVirtualMachines := getClusterResources(az, 1, 1) - setMockEnv(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) + setMockEnvDualStack(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) - svc := getTestService("service1", v1.ProtocolTCP, nil, false, 80) + svc := getTestServiceDualStack("service1", v1.ProtocolTCP, nil, 80) expectedLBs := make([]network.LoadBalancer, 0) - setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, false) + setMockLBsDualStack(az, ctrl, &expectedLBs, "service", 1, 1, false) mockLBBackendPool := az.LoadBalancerBackendPool.(*MockBackendPool) mockLBBackendPool.EXPECT().ReconcileBackendPools(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, false, nil).AnyTimes() mockLBBackendPool.EXPECT().EnsureHostsInPool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() lb, err := az.reconcileLoadBalancer(testClusterName, &svc, clusterResources.nodes, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validateLoadBalancer(t, lb, svc) - svcUpdated := getTestService("service1", v1.ProtocolTCP, nil, false) + svcUpdated := getTestServiceDualStack("service1", v1.ProtocolTCP, nil) expectedLBs[0].FrontendIPConfigurations = &[]network.FrontendIPConfiguration{} mockLBsClient := mockloadbalancerclient.NewMockInterface(ctrl) @@ -993,14 +1117,10 @@ func TestReconcileLoadBalancerRemoveAllPortsRemovesFrontendConfig(t *testing.T) mockLBsClient.EXPECT().Delete(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) lb, err = az.reconcileLoadBalancer(testClusterName, &svcUpdated, clusterResources.nodes, false /* wantLb*/) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) // ensure we abandoned the frontend ip configuration - if len(*lb.FrontendIPConfigurations) != 0 { - t.Error("Expected the loadbalancer to have no frontend ip configuration") - } + assert.Zero(t, len(*lb.FrontendIPConfigurations)) validateLoadBalancer(t, lb, svcUpdated) } @@ -1012,27 +1132,23 @@ func TestReconcileLoadBalancerRemovesPort(t *testing.T) { az := GetTestCloud(ctrl) clusterResources, expectedInterfaces, expectedVirtualMachines := getClusterResources(az, 1, 1) - setMockEnv(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) + setMockEnvDualStack(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1) mockLBBackendPool := az.LoadBalancerBackendPool.(*MockBackendPool) mockLBBackendPool.EXPECT().ReconcileBackendPools(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, false, nil).AnyTimes() mockLBBackendPool.EXPECT().EnsureHostsInPool(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() expectedLBs := make([]network.LoadBalancer, 0) - setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, false) - svc := getTestService("service1", v1.ProtocolTCP, nil, false, 80, 443) + setMockLBsDualStack(az, ctrl, &expectedLBs, "service", 1, 1, false) + svc := getTestServiceDualStack("service1", v1.ProtocolTCP, nil, 80, 443) _, err := az.reconcileLoadBalancer(testClusterName, &svc, clusterResources.nodes, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) expectedLBs = make([]network.LoadBalancer, 0) - setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, false) - svcUpdated := getTestService("service1", v1.ProtocolTCP, nil, false, 80) + setMockLBsDualStack(az, ctrl, &expectedLBs, "service", 1, 1, false) + svcUpdated := getTestServiceDualStack("service1", v1.ProtocolTCP, nil, 80) lb, err := az.reconcileLoadBalancer(testClusterName, &svcUpdated, clusterResources.nodes, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validateLoadBalancer(t, lb, svcUpdated) } @@ -1044,13 +1160,13 @@ func TestReconcileLoadBalancerMultipleServices(t *testing.T) { az := GetTestCloud(ctrl) clusterResources, expectedInterfaces, expectedVirtualMachines := getClusterResources(az, 1, 1) - setMockEnv(az, ctrl, expectedInterfaces, expectedVirtualMachines, 2) + setMockEnvDualStack(az, ctrl, expectedInterfaces, expectedVirtualMachines, 2) - svc1 := getTestService("service1", v1.ProtocolTCP, nil, false, 80, 443) - svc2 := getTestService("service2", v1.ProtocolTCP, nil, false, 81) + svc1 := getTestServiceDualStack("service1", v1.ProtocolTCP, nil, 80, 443) + svc2 := getTestServiceDualStack("service2", v1.ProtocolTCP, nil, 81) expectedLBs := make([]network.LoadBalancer, 0) - setMockLBs(az, ctrl, &expectedLBs, "service", 1, 1, false) + setMockLBsDualStack(az, ctrl, &expectedLBs, "service", 1, 1, false) mockLBBackendPool := az.LoadBalancerBackendPool.(*MockBackendPool) mockLBBackendPool.EXPECT().ReconcileBackendPools(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, false, nil).AnyTimes() @@ -1061,16 +1177,12 @@ func TestReconcileLoadBalancerMultipleServices(t *testing.T) { mockPLSClient.EXPECT().List(gomock.Any(), az.Config.ResourceGroup).Return(expectedPLS, nil).MinTimes(1).MaxTimes(1) _, err := az.reconcileLoadBalancer(testClusterName, &svc1, clusterResources.nodes, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) - setMockLBs(az, ctrl, &expectedLBs, "service", 1, 2, false) + setMockLBsDualStack(az, ctrl, &expectedLBs, "service", 1, 2, false) updatedLoadBalancer, err := az.reconcileLoadBalancer(testClusterName, &svc2, clusterResources.nodes, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validateLoadBalancer(t, updatedLoadBalancer, svc1, svc2) } @@ -1277,9 +1389,7 @@ func TestReconcileSecurityGroupNewServiceAddsPort(t *testing.T) { lbStatus, _, _ := az.getServiceLoadBalancerStatus(&svc1, lb) sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, &lbStatus.Ingress[0].IP, nil, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validateSecurityGroup(t, sg, svc1) } @@ -1305,9 +1415,7 @@ func TestReconcileSecurityGroupNewInternalServiceAddsPort(t *testing.T) { lb, _ := az.reconcileLoadBalancer(testClusterName, &svc1, clusterResources.nodes, true) lbStatus, _, _ := az.getServiceLoadBalancerStatus(&svc1, lb) sg, err := az.reconcileSecurityGroup(testClusterName, &svc1, &lbStatus.Ingress[0].IP, nil, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validateSecurityGroup(t, sg, svc1) } @@ -1346,9 +1454,7 @@ func TestReconcileSecurityGroupRemoveService(t *testing.T) { validateSecurityGroup(t, sg, service1, service2) sg, err := az.reconcileSecurityGroup(testClusterName, &service1, &lbStatus.Ingress[0].IP, nil, false /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validateSecurityGroup(t, sg, service2) } @@ -1382,9 +1488,7 @@ func TestReconcileSecurityGroupRemoveServiceRemovesPort(t *testing.T) { lbStatus, _, _ := az.getServiceLoadBalancerStatus(&svc, lb) sg, err := az.reconcileSecurityGroup(testClusterName, &svcUpdated, &lbStatus.Ingress[0].IP, nil, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validateSecurityGroup(t, sg, svcUpdated) } @@ -1413,9 +1517,7 @@ func TestReconcileSecurityWithSourceRanges(t *testing.T) { lbStatus, _, _ := az.getServiceLoadBalancerStatus(&svc, lb) sg, err := az.reconcileSecurityGroup(testClusterName, &svc, &lbStatus.Ingress[0].IP, nil, true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validateSecurityGroup(t, sg, svc) } @@ -1478,15 +1580,11 @@ func TestReconcilePublicIPsWithNewService(t *testing.T) { setMockPublicIPs(az, ctrl, 1, v4Enabled, v6Enabled) pips, err := az.reconcilePublicIPs(testClusterName, &svc, "", true /* wantLb*/) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validatePublicIPs(t, pips, &svc, true) pips2, err := az.reconcilePublicIPs(testClusterName, &svc, "", true /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validatePublicIPs(t, pips2, &svc, true) pipsNames1, pipsNames2 := []string{}, []string{} @@ -1514,17 +1612,13 @@ func TestReconcilePublicIPsRemoveService(t *testing.T) { setMockPublicIPs(az, ctrl, 1, v4Enabled, v6Enabled) pips, err := az.reconcilePublicIPs(testClusterName, &svc, "", true /* wantLb*/) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validatePublicIPs(t, pips, &svc, true) // Remove the service pips, err = az.reconcilePublicIPs(testClusterName, &svc, "", false /* wantLb */) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validatePublicIPs(t, pips, &svc, false) } @@ -1539,9 +1633,7 @@ func TestReconcilePublicIPsWithInternalService(t *testing.T) { setMockPublicIPs(az, ctrl, 1, v4Enabled, v6Enabled) pips, err := az.reconcilePublicIPs(testClusterName, &svc, "", true /* wantLb*/) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validatePublicIPs(t, pips, &svc, true) } @@ -1557,24 +1649,18 @@ func TestReconcilePublicIPsWithExternalAndInternalSwitch(t *testing.T) { setMockPublicIPs(az, ctrl, 1, v4Enabled, v6Enabled) pips, err := az.reconcilePublicIPs(testClusterName, &svc, "", true /* wantLb*/) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validatePublicIPs(t, pips, &svc, true) // Update to external service svcUpdated := getTestService("servicea", v1.ProtocolTCP, nil, false, 80) pips, err = az.reconcilePublicIPs(testClusterName, &svcUpdated, "", true /* wantLb*/) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validatePublicIPs(t, pips, &svcUpdated, true) // Update to internal service again pips, err = az.reconcilePublicIPs(testClusterName, &svc, "", true /* wantLb*/) - if err != nil { - t.Errorf("Unexpected error: %q", err) - } + assert.Nil(t, err) validatePublicIPs(t, pips, &svc, true) } @@ -1839,9 +1925,13 @@ func validateLoadBalancer(t *testing.T, loadBalancer *network.LoadBalancer, serv expectedFrontendIPCount := 0 expectedProbeCount := 0 expectedFrontendIPs := []ExpectedFrontendIPInfo{} + svcIPFamilyCount := 1 + if len(services) > 0 { + svcIPFamilyCount = len(services[0].Spec.IPFamilies) + } for i, svc := range services { if len(svc.Spec.Ports) > 0 { - expectedFrontendIPCount++ + expectedFrontendIPCount += svcIPFamilyCount expectedSubnetName := "" if requiresInternalLoadBalancer(&services[i]) { expectedSubnetName = svc.Annotations[consts.ServiceAnnotationLoadBalancerInternalSubnet] @@ -1854,22 +1944,33 @@ func validateLoadBalancer(t *testing.T, loadBalancer *network.LoadBalancer, serv Subnet: pointer.String(expectedSubnetName), } expectedFrontendIPs = append(expectedFrontendIPs, expectedFrontendIP) + if svcIPFamilyCount == 2 { + expectedFrontendIP := ExpectedFrontendIPInfo{ + Name: az.getDefaultFrontendIPConfigName(&services[i]) + "-" + v6Suffix, + Subnet: pointer.String(expectedSubnetName), + } + expectedFrontendIPs = append(expectedFrontendIPs, expectedFrontendIP) + } } for _, wantedRule := range svc.Spec.Ports { - expectedRuleCount++ - 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) && - *actualRule.FrontendPort == wantedRule.Port && - *actualRule.BackendPort == wantedRule.Port { - foundRule = true - break + expectedRuleCount += svcIPFamilyCount + wantedRuleNameMap := map[bool]string{} + for _, ipFamily := range services[i].Spec.IPFamilies { + isIPv6 := ipFamily == v1.IPv6Protocol + wantedRuleName := az.getLoadBalancerRuleName(&services[i], wantedRule.Protocol, wantedRule.Port, isIPv6) + wantedRuleNameMap[isIPv6] = wantedRuleName + foundRule := false + for _, actualRule := range *loadBalancer.LoadBalancingRules { + if strings.EqualFold(*actualRule.Name, wantedRuleName) && + *actualRule.FrontendPort == wantedRule.Port && + *actualRule.BackendPort == wantedRule.Port { + foundRule = true + break + } + } + if !foundRule { + t.Errorf("Expected load balancer rule but didn't find it: %q", wantedRuleName) } - } - if !foundRule { - t.Errorf("Expected load balancer rule but didn't find it: %q", wantedRuleName) } // if UDP rule, there is no probe @@ -1877,35 +1978,37 @@ func validateLoadBalancer(t *testing.T, loadBalancer *network.LoadBalancer, serv continue } - expectedProbeCount++ - foundProbe := false - if servicehelpers.NeedsHealthCheck(&services[i]) { - path, port := servicehelpers.GetServiceHealthCheckPathPort(&services[i]) - 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 && - *actualProbe.RequestPath == path && - actualProbe.Protocol == network.ProbeProtocolHTTP { - foundProbe = true - break + expectedProbeCount += svcIPFamilyCount + for _, ipFamily := range services[i].Spec.IPFamilies { + isIPv6 := ipFamily == v1.IPv6Protocol + foundProbe := false + if servicehelpers.NeedsHealthCheck(&services[i]) { + path, port := servicehelpers.GetServiceHealthCheckPathPort(&services[i]) + wantedRuleName := az.getLoadBalancerRuleName(&services[i], v1.ProtocolTCP, port, isIPv6) + for _, actualProbe := range *loadBalancer.Probes { + if strings.EqualFold(*actualProbe.Name, wantedRuleName) && + *actualProbe.Port == port && + *actualProbe.RequestPath == path && + actualProbe.Protocol == network.ProbeProtocolHTTP { + foundProbe = true + break + } } - } - } else { - for _, actualProbe := range *loadBalancer.Probes { - if strings.EqualFold(*actualProbe.Name, wantedRuleName) && - *actualProbe.Port == wantedRule.NodePort { - foundProbe = true - break + } else { + for _, actualProbe := range *loadBalancer.Probes { + if strings.EqualFold(*actualProbe.Name, wantedRuleNameMap[isIPv6]) && + *actualProbe.Port == wantedRule.NodePort { + foundProbe = true + break + } } } - } - if !foundProbe { - for _, actualProbe := range *loadBalancer.Probes { - t.Logf("Probe: %s %d", *actualProbe.Name, *actualProbe.Port) + if !foundProbe { + for _, actualProbe := range *loadBalancer.Probes { + t.Logf("Probe: %s %d", *actualProbe.Name, *actualProbe.Port) + } + t.Errorf("Expected loadbalancer probe but didn't find it: %q", wantedRuleNameMap[isIPv6]) } - t.Errorf("Expected loadbalancer probe but didn't find it: %q", wantedRuleName) } } } @@ -2544,9 +2647,7 @@ func TestIfServiceSpecifiesSharedRuleAndRuleDoesNotExistItIsCreated(t *testing.T 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) - } + assert.Nil(t, err) validateSecurityGroup(t, sg, svc) @@ -2604,9 +2705,7 @@ func TestIfServiceSpecifiesSharedRuleAndRuleExistsThenTheServicesPortAndAddressA 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) - } + assert.Nil(t, err) validateSecurityGroup(t, sg, svc) diff --git a/pkg/provider/azure_utils.go b/pkg/provider/azure_utils.go index 4fdc8d4b23..d314fd9957 100644 --- a/pkg/provider/azure_utils.go +++ b/pkg/provider/azure_utils.go @@ -447,27 +447,30 @@ func getResourceByIPFamily(resource string, isDualStack, isIPv6 bool) string { } // isFIPIPv6 checks if the frontend IP configuration is of IPv6. -func (az *Cloud) isFIPIPv6(fip *network.FrontendIPConfiguration, pipResourceGroup string, isInternal bool) (isIPv6 bool, err error) { - pips, err := az.listPIP(pipResourceGroup) - if err != nil { - return false, fmt.Errorf("isFIPIPv6: failed to list pip: %w", err) - } +func (az *Cloud) isFIPIPv6(service *v1.Service, fip *network.FrontendIPConfiguration, isInternal bool) (bool, error) { 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 + if fip.FrontendIPConfigurationPropertiesFormat.PrivateIPAddress != nil { + return net.ParseIP(*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", + klog.Errorf("Checking IP Family of frontend IP configuration %q of internal Service but "+ + "it is not clear. It's considered to be IPv4", pointer.StringDeref(fip.Name, "")) - return + return false, nil } var fipPIPID string if fip.FrontendIPConfigurationPropertiesFormat != nil && fip.FrontendIPConfigurationPropertiesFormat.PublicIPAddress != nil { fipPIPID = pointer.StringDeref(fip.FrontendIPConfigurationPropertiesFormat.PublicIPAddress.ID, "") } + pipResourceGroup := az.getPublicIPAddressResourceGroup(service) + pips, err := az.listPIP(pipResourceGroup) + if err != nil { + return false, err + } for _, pip := range pips { id := pointer.StringDeref(pip.ID, "") if !strings.EqualFold(fipPIPID, id) { @@ -475,15 +478,19 @@ func (az *Cloud) isFIPIPv6(fip *network.FrontendIPConfiguration, pipResourceGrou } 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 + if pip.PublicIPAddressPropertiesFormat.PublicIPAddressVersion != "" { + return pip.PublicIPAddressPropertiesFormat.PublicIPAddressVersion == network.IPv6, nil + } + if pip.PublicIPAddressPropertiesFormat.IPAddress != nil { + return net.ParseIP(pointer.StringDeref(pip.PublicIPAddressPropertiesFormat.IPAddress, "")).To4() == nil, nil } } + klog.Errorf("Checking IP Family of PIP %q of corresponding frontend IP configuration %q of external Service but "+ + "it is not clear. It's considered to be IPv4", + pointer.StringDeref(pip.Name, ""), pointer.StringDeref(fip.Name, "")) break } - return isIPv6, nil + return false, nil } // getResourceIDPrefix returns a substring from the provided one between beginning and the last "/". @@ -494,3 +501,21 @@ func getResourceIDPrefix(id string) string { } return id[:idx] } + +// fillSubnet fills subnet value into the variable. +func (az *Cloud) fillSubnet(subnet *network.Subnet, subnetName string) error { + if subnet == nil { + return fmt.Errorf("subnet is nil, should not happen") + } + if subnet.ID == nil { + curSubnet, existsSubnet, err := az.getSubnet(az.VnetName, subnetName) + if err != nil { + return err + } + if !existsSubnet { + return fmt.Errorf("failed to get subnet: %s/%s", az.VnetName, subnetName) + } + *subnet = curSubnet + } + return nil +} diff --git a/pkg/provider/azure_utils_test.go b/pkg/provider/azure_utils_test.go index 4216fdf445..72267be827 100644 --- a/pkg/provider/azure_utils_test.go +++ b/pkg/provider/azure_utils_test.go @@ -31,6 +31,7 @@ import ( "k8s.io/utils/pointer" "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/publicipclient/mockpublicipclient" + "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/subnetclient/mocksubnetclient" azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache" "sigs.k8s.io/cloud-provider-azure/pkg/consts" ) @@ -899,6 +900,7 @@ func TestGetResourceByIPFamily(t *testing.T) { func TestIsFIPIPv6(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() + svc := v1.Service{} testcases := []struct { desc string @@ -908,7 +910,7 @@ func TestIsFIPIPv6(t *testing.T) { expectedIsIPv6 bool }{ { - "Internal IPv4", + "Internal IPv4 with PrivateIPAddressVersion", &network.FrontendIPConfiguration{ FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ PrivateIPAddressVersion: network.IPv4, @@ -920,7 +922,27 @@ func TestIsFIPIPv6(t *testing.T) { false, }, { - "External IPv6", + "Internal IPv4 with PrivateIPAddress", + &network.FrontendIPConfiguration{ + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PrivateIPAddress: pointer.String("10.0.0.1"), + }, + }, + []network.PublicIPAddress{}, + true, + false, + }, + { + "Internal IPv4 no info so default to IPv4", + &network.FrontendIPConfiguration{ + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{}, + }, + []network.PublicIPAddress{}, + true, + false, + }, + { + "External IPv6 with PublicIPAddressVersion", &network.FrontendIPConfiguration{ FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("pip-id0")}, @@ -947,13 +969,67 @@ func TestIsFIPIPv6(t *testing.T) { false, true, }, + { + "External IPv6 with PIP IPAddress", + &network.FrontendIPConfiguration{ + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("pip-id0")}, + }, + }, + []network.PublicIPAddress{ + { + Name: pointer.String("pip0"), + ID: pointer.String("pip-id0"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + IPAddress: pointer.String("2001::1"), + }, + }, + { + Name: pointer.String("pip1"), + ID: pointer.String("pip-id1"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv4, + IPAddress: pointer.String("10.0.0.1"), + }, + }, + }, + false, + true, + }, + { + "External IPv4 with no info so default to IPv4", + &network.FrontendIPConfiguration{ + FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{ + PublicIPAddress: &network.PublicIPAddress{ID: pointer.String("pip-id0")}, + }, + }, + []network.PublicIPAddress{ + { + Name: pointer.String("pip0"), + ID: pointer.String("pip-id0"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{}, + }, + { + Name: pointer.String("pip1"), + ID: pointer.String("pip-id1"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + PublicIPAddressVersion: network.IPv4, + IPAddress: pointer.String("10.0.0.1"), + }, + }, + }, + false, + false, + }, } for _, tc := range testcases { t.Run(tc.desc, func(t *testing.T) { az := GetTestCloud(ctrl) mockPIPsClient := az.PublicIPAddressesClient.(*mockpublicipclient.MockInterface) - mockPIPsClient.EXPECT().List(gomock.Any(), "rg").Return(tc.pips, nil) - isIPv6, err := az.isFIPIPv6(tc.fip, "rg", tc.isInternal) + if !tc.isInternal { + mockPIPsClient.EXPECT().List(gomock.Any(), "rg").Return(tc.pips, nil) + } + isIPv6, err := az.isFIPIPv6(&svc, tc.fip, tc.isInternal) assert.Nil(t, err) assert.Equal(t, tc.expectedIsIPv6, isIPv6) }) @@ -976,3 +1052,53 @@ func TestGetResourceIDPrefix(t *testing.T) { }) } } + +func TestFillSubnet(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + testcases := []struct { + desc string + subnetName string + subnet *network.Subnet + expectedTimes int + expectedSubnet *network.Subnet + }{ + { + desc: "empty subnet", + subnetName: "subnet0", + subnet: &network.Subnet{}, + expectedTimes: 1, + expectedSubnet: &network.Subnet{ + Name: pointer.String("subnet0"), + ID: pointer.String("subnet-id0"), + }, + }, + { + desc: "filled subnet", + subnetName: "subnet1", + subnet: &network.Subnet{ + Name: pointer.String("subnet1"), + ID: pointer.String("subnet-id1"), + }, + expectedTimes: 0, + expectedSubnet: &network.Subnet{ + Name: pointer.String("subnet1"), + ID: pointer.String("subnet-id1"), + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + az := GetTestCloud(ctrl) + mockSubnetsClient := az.SubnetsClient.(*mocksubnetclient.MockInterface) + mockSubnetsClient.EXPECT().Get(gomock.Any(), az.ResourceGroup, gomock.Any(), tc.subnetName, gomock.Any()). + Return(*tc.expectedSubnet, nil).Times(tc.expectedTimes) + + err := az.fillSubnet(tc.subnet, tc.subnetName) + assert.Nil(t, err) + assert.Equal(t, tc.expectedSubnet, tc.subnet) + }) + } +} diff --git a/pkg/provider/azure_wrap.go b/pkg/provider/azure_wrap.go index 97c8928c7a..2f5a66c44f 100644 --- a/pkg/provider/azure_wrap.go +++ b/pkg/provider/azure_wrap.go @@ -158,9 +158,7 @@ func (az *Cloud) getSubnet(virtualNetworkName string, subnetName string) (networ if !exists { klog.V(2).Infof("Subnet %q not found", subnetName) - return subnet, false, nil } - return subnet, exists, nil }