Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated cherry pick of #86276: fix: should truncate long subnet name on lb rules #91429

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -407,7 +407,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 {
Expand Down Expand Up @@ -697,7 +697,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(clusterName, service)
lbBackendPoolID := az.getBackendPoolID(lbName, lbBackendPoolName)
Expand Down Expand Up @@ -1030,7 +1030,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)
Expand Down
Expand Up @@ -1628,7 +1628,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,
},
Expand Down
32 changes: 25 additions & 7 deletions staging/src/k8s.io/legacy-cloud-providers/azure/azure_standard.go
Expand Up @@ -66,7 +66,9 @@ const (
nodeLabelRole = "kubernetes.io/role"
nicFailedState = "Failed"

storageAccountNameMaxLength = 24
storageAccountNameMaxLength = 24
frontendIPConfigNameMaxLength = 80
loadBalancerRuleNameMaxLength = 80
)

var errNotInVMSet = errors.New("vm is not in the vmset")
Expand Down Expand Up @@ -275,12 +277,21 @@ func getBackendPoolName(clusterName string, service *v1.Service) string {
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 characters, so excluding the hyphen two segments cannot exceed 79
subnetSegment := *subnet
if len(ruleName)+len(subnetSegment)+1 > loadBalancerRuleNameMaxLength {
subnetSegment = subnetSegment[:loadBalancerRuleNameMaxLength-len(ruleName)-1]
}

return fmt.Sprintf("%s-%s-%s-%d", prefix, subnetSegment, protocol, port)
}

func (az *Cloud) getSecurityRuleName(service *v1.Service, port v1.ServicePort, sourceAddrPrefix string) string {
Expand Down Expand Up @@ -318,10 +329,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 characters
if len(ipcName) > frontendIPConfigNameMaxLength {
ipcName = ipcName[:frontendIPConfigNameMaxLength]
}
return ipcName
}
return baseName
}
Expand Down
154 changes: 154 additions & 0 deletions staging/src/k8s.io/legacy-cloud-providers/azure/azure_standard_test.go
Expand Up @@ -19,6 +19,7 @@ limitations under the License.
package azure

import (
"strconv"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -264,3 +265,156 @@ 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{},
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 characters",
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 characters",
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)

loadbalancerRuleName := az.getLoadBalancerRuleName(svc, c.protocol, c.port)
assert.Equal(t, c.expected, loadbalancerRuleName, c.description)
}
}

func TestGetFrontendIPConfigName(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
expected string
}{
{
description: "internal lb should have subnet name on the frontend ip configuration name",
subnetName: "shortsubnet",
isInternal: true,
useStandardLB: true,
expected: "a257b965551374ad2b091ef3f07043ad-shortsubnet",
},
{
description: "internal standard lb should have subnet name on the frontend ip configuration name but truncated to 80 characters",
subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet",
isInternal: true,
useStandardLB: true,
expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnnggggggggggg",
},
{
description: "internal basic lb should have subnet name on the frontend ip configuration name but truncated to 80 characters",
subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet",
isInternal: true,
useStandardLB: false,
expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnnggggggggggg",
},
{
description: "external standard lb should not have subnet name on the frontend ip configuration name",
subnetName: "shortsubnet",
isInternal: false,
useStandardLB: true,
expected: "a257b965551374ad2b091ef3f07043ad",
},
{
description: "external basic lb should not have subnet name on the frontend ip configuration name",
subnetName: "shortsubnet",
isInternal: false,
useStandardLB: false,
expected: "a257b965551374ad2b091ef3f07043ad",
},
}

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)

ipconfigName := az.getFrontendIPConfigName(svc)
assert.Equal(t, c.expected, ipconfigName, c.description)
}
}
4 changes: 2 additions & 2 deletions staging/src/k8s.io/legacy-cloud-providers/azure/azure_test.go
Expand Up @@ -1240,14 +1240,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) &&
Expand Down