fix: should truncate long subnet name on lb rules

Author: yangl900

staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go

@@ -405,7 +405,7 @@ func (az *Cloud) getServiceLoadBalancerStatus(service *v1.Service, lb *network.L
 		return nil, nil
 	}
 	isInternal := requiresInternalLoadBalancer(service)
-	lbFrontendIPConfigName := az.getFrontendIPConfigName(service, subnet(service))
+	lbFrontendIPConfigName := az.getFrontendIPConfigName(service)
 	serviceName := getServiceName(service)
 	for _, ipConfiguration := range *lb.FrontendIPConfigurations {
 		if lbFrontendIPConfigName == *ipConfiguration.Name {
@@ -693,7 +693,7 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
 	}
 	lbName := *lb.Name
 	klog.V(2).Infof("reconcileLoadBalancer for service(%s): lb(%s) wantLb(%t) resolved load balancer name", serviceName, lbName, wantLb)
-	lbFrontendIPConfigName := az.getFrontendIPConfigName(service, subnet(service))
+	lbFrontendIPConfigName := az.getFrontendIPConfigName(service)
 	lbFrontendIPConfigID := az.getFrontendIPConfigID(lbName, lbFrontendIPConfigName)
 	lbBackendPoolName := getBackendPoolName(az.ipv6DualStackEnabled, clusterName, service)
 	lbBackendPoolID := az.getBackendPoolID(lbName, lbBackendPoolName)
@@ -1026,7 +1026,7 @@ func (az *Cloud) reconcileLoadBalancerRule(
 		}
 
 		for _, protocol := range protocols {
-			lbRuleName := az.getLoadBalancerRuleName(service, protocol, port.Port, subnet(service))
+			lbRuleName := az.getLoadBalancerRuleName(service, protocol, port.Port)
 			klog.V(2).Infof("reconcileLoadBalancerRule lb name (%s) rule name (%s)", lbName, lbRuleName)
 
 			transportProto, _, probeProto, err := getProtocolsFromKubernetesProtocol(protocol)

staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go

@@ -1627,7 +1627,7 @@ func TestGetServiceLoadBalancerStatus(t *testing.T) {
 		},
 		{
 			desc: "getServiceLoadBalancerStatus shall return nil if lb.FrontendIPConfigurations.name != " +
-				"az.getFrontendIPConfigName(service, subnet(service))",
+				"az.getFrontendIPConfigName(service)",
 			service: &internalService,
 			lb:      &lb3,
 		},

staging/src/k8s.io/legacy-cloud-providers/azure/azure_standard.go

@@ -273,12 +273,21 @@ func getBackendPoolName(ipv6DualStackEnabled bool, clusterName string, service *
 	return clusterName
 }
 
-func (az *Cloud) getLoadBalancerRuleName(service *v1.Service, protocol v1.Protocol, port int32, subnetName *string) string {
+func (az *Cloud) getLoadBalancerRuleName(service *v1.Service, protocol v1.Protocol, port int32) string {
 	prefix := az.getRulePrefix(service)
-	if subnetName == nil {
-		return fmt.Sprintf("%s-%s-%d", prefix, protocol, port)
+	ruleName := fmt.Sprintf("%s-%s-%d", prefix, protocol, port)
+	subnet := subnet(service)
+	if subnet == nil {
+		return ruleName
 	}
-	return fmt.Sprintf("%s-%s-%s-%d", prefix, *subnetName, protocol, port)
+
+	// Load balancer rule name must be less or equal to 80 charactors, so excluding the hyphen two segments cannot exceed 79
+	subnetSegment := *subnet
+	if len(ruleName) + len(subnetSegment) > 79 {
+		subnetSegment = subnetSegment[:79 - len(ruleName)]
+	}
+
+	return fmt.Sprintf("%s-%s-%s-%d", prefix, subnetSegment, protocol, port)
 }
 
 func (az *Cloud) getSecurityRuleName(service *v1.Service, port v1.ServicePort, sourceAddrPrefix string) string {
@@ -316,10 +325,17 @@ func (az *Cloud) serviceOwnsFrontendIP(fip network.FrontendIPConfiguration, serv
 	return strings.HasPrefix(*fip.Name, baseName)
 }
 
-func (az *Cloud) getFrontendIPConfigName(service *v1.Service, subnetName *string) string {
+func (az *Cloud) getFrontendIPConfigName(service *v1.Service) string {
 	baseName := az.GetLoadBalancerName(context.TODO(), "", service)
+	subnetName := subnet(service)
 	if subnetName != nil {
-		return fmt.Sprintf("%s-%s", baseName, *subnetName)
+		ipcName := fmt.Sprintf("%s-%s", baseName, *subnetName)
+		
+		// Azure lb front end configuration name must not exceed 80 charactors
+		if len(ipcName) > 80 {
+			ipcName = ipcName[:80]
+		}
+		return ipcName
 	}
 	return baseName
 }

staging/src/k8s.io/legacy-cloud-providers/azure/azure_standard_test.go

@@ -20,6 +20,7 @@ package azure
 
 import (
 	"testing"
+	"strconv"
 
 	"github.com/stretchr/testify/assert"
 
@@ -253,3 +254,87 @@ func TestGetAzureLoadBalancerName(t *testing.T) {
 		assert.Equal(t, c.expected, loadbalancerName, c.description)
 	}
 }
+
+func TestGetLoadBalancingRuleName(t *testing.T) {
+	az := getTestCloud()
+	az.PrimaryAvailabilitySetName = "primary"
+
+	svc := &v1.Service{
+		ObjectMeta: meta.ObjectMeta{
+			Annotations: map[string]string{
+				ServiceAnnotationLoadBalancerInternalSubnet: "subnet",
+				ServiceAnnotationLoadBalancerInternal:       "true",
+			},
+			UID: "257b9655-5137-4ad2-b091-ef3f07043ad3",
+		},
+	}
+
+	cases := []struct {
+		description string
+		subnetName    string
+		isInternal    bool
+		useStandardLB bool
+		protocol      v1.Protocol
+		port          int32
+		expected      string
+	}{
+		{
+			description: "internal lb should have subnet name on the rule name",
+			subnetName:       "shortsubnet",
+			isInternal: true,
+			useStandardLB:    true,
+			protocol: v1.ProtocolTCP,
+			port: 9000,
+			expected: "a257b965551374ad2b091ef3f07043ad-shortsubnet-TCP-9000",
+		},
+		{
+			description: "internal standard lb should have subnet name on the rule name but truncated to 80 charactors",
+			subnetName:       "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet",
+			isInternal: true,
+			useStandardLB:    true,
+			protocol: v1.ProtocolTCP,
+			port: 9000,
+			expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnngg-TCP-9000",
+		},
+		{
+			description: "internal basic lb should have subnet name on the rule name but truncated to 80 charactors",
+			subnetName:       "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet",
+			isInternal: true,
+			useStandardLB:    false,
+			protocol: v1.ProtocolTCP,
+			port: 9000,
+			expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnngg-TCP-9000",
+		},
+		{
+			description: "external standard lb should not have subnet name on the rule name",
+			subnetName:       "shortsubnet",
+			isInternal: false,
+			useStandardLB:    true,
+			protocol: v1.ProtocolTCP,
+			port: 9000,
+			expected: "a257b965551374ad2b091ef3f07043ad-TCP-9000",
+		},
+		{
+			description: "external basic lb should not have subnet name on the rule name",
+			subnetName:       "shortsubnet",
+			isInternal: false,
+			useStandardLB:    false,
+			protocol: v1.ProtocolTCP,
+			port: 9000,
+			expected: "a257b965551374ad2b091ef3f07043ad-TCP-9000",
+		},
+	}
+
+	for _, c := range cases {
+		if c.useStandardLB {
+			az.Config.LoadBalancerSku = loadBalancerSkuStandard
+		} else {
+			az.Config.LoadBalancerSku = loadBalancerSkuBasic
+		}
+		svc.Annotations[ServiceAnnotationLoadBalancerInternalSubnet] = c.subnetName
+		svc.Annotations[ServiceAnnotationLoadBalancerInternal] = strconv.FormatBool(c.isInternal)
+
+		loadbalancerName := az.getLoadBalancerRuleName(svc, c.protocol, c.port)
+		assert.Equal(t, c.expected, loadbalancerName, c.description)
+	}
+}

staging/src/k8s.io/legacy-cloud-providers/azure/azure_test.go

@@ -1238,14 +1238,14 @@ func validateLoadBalancer(t *testing.T, loadBalancer *network.LoadBalancer, serv
 		if len(svc.Spec.Ports) > 0 {
 			expectedFrontendIPCount++
 			expectedFrontendIP := ExpectedFrontendIPInfo{
-				Name:   az.getFrontendIPConfigName(&svc, subnet(&svc)),
+				Name:   az.getFrontendIPConfigName(&svc),
 				Subnet: subnet(&svc),
 			}
 			expectedFrontendIPs = append(expectedFrontendIPs, expectedFrontendIP)
 		}
 		for _, wantedRule := range svc.Spec.Ports {
 			expectedRuleCount++
-			wantedRuleName := az.getLoadBalancerRuleName(&svc, wantedRule.Protocol, wantedRule.Port, subnet(&svc))
+			wantedRuleName := az.getLoadBalancerRuleName(&svc, wantedRule.Protocol, wantedRule.Port)
 			foundRule := false
 			for _, actualRule := range *loadBalancer.LoadBalancingRules {
 				if strings.EqualFold(*actualRule.Name, wantedRuleName) &&