Skip to content

Commit e680ad7

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#86276 from yangl900/fix-subnet-toolong
Fix internal loadbalancer configuration failure when subnet name too long
2 parents 0a61fd9 + fbe3521 commit e680ad7

File tree

5 files changed

+185
-13
lines changed

5 files changed

+185
-13
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ func (az *Cloud) getServiceLoadBalancerStatus(service *v1.Service, lb *network.L
405405
return nil, nil
406406
}
407407
isInternal := requiresInternalLoadBalancer(service)
408-
lbFrontendIPConfigName := az.getFrontendIPConfigName(service, subnet(service))
408+
lbFrontendIPConfigName := az.getFrontendIPConfigName(service)
409409
serviceName := getServiceName(service)
410410
for _, ipConfiguration := range *lb.FrontendIPConfigurations {
411411
if lbFrontendIPConfigName == *ipConfiguration.Name {
@@ -693,7 +693,7 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
693693
}
694694
lbName := *lb.Name
695695
klog.V(2).Infof("reconcileLoadBalancer for service(%s): lb(%s) wantLb(%t) resolved load balancer name", serviceName, lbName, wantLb)
696-
lbFrontendIPConfigName := az.getFrontendIPConfigName(service, subnet(service))
696+
lbFrontendIPConfigName := az.getFrontendIPConfigName(service)
697697
lbFrontendIPConfigID := az.getFrontendIPConfigID(lbName, lbFrontendIPConfigName)
698698
lbBackendPoolName := getBackendPoolName(az.ipv6DualStackEnabled, clusterName, service)
699699
lbBackendPoolID := az.getBackendPoolID(lbName, lbBackendPoolName)
@@ -1026,7 +1026,7 @@ func (az *Cloud) reconcileLoadBalancerRule(
10261026
}
10271027

10281028
for _, protocol := range protocols {
1029-
lbRuleName := az.getLoadBalancerRuleName(service, protocol, port.Port, subnet(service))
1029+
lbRuleName := az.getLoadBalancerRuleName(service, protocol, port.Port)
10301030
klog.V(2).Infof("reconcileLoadBalancerRule lb name (%s) rule name (%s)", lbName, lbRuleName)
10311031

10321032
transportProto, _, probeProto, err := getProtocolsFromKubernetesProtocol(protocol)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1627,7 +1627,7 @@ func TestGetServiceLoadBalancerStatus(t *testing.T) {
16271627
},
16281628
{
16291629
desc: "getServiceLoadBalancerStatus shall return nil if lb.FrontendIPConfigurations.name != " +
1630-
"az.getFrontendIPConfigName(service, subnet(service))",
1630+
"az.getFrontendIPConfigName(service)",
16311631
service: &internalService,
16321632
lb: &lb3,
16331633
},

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

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ const (
6565
nodeLabelRole = "kubernetes.io/role"
6666
nicFailedState = "Failed"
6767

68-
storageAccountNameMaxLength = 24
68+
storageAccountNameMaxLength = 24
69+
frontendIPConfigNameMaxLength = 80
70+
loadBalancerRuleNameMaxLength = 80
6971
)
7072

7173
var errNotInVMSet = errors.New("vm is not in the vmset")
@@ -273,12 +275,21 @@ func getBackendPoolName(ipv6DualStackEnabled bool, clusterName string, service *
273275
return clusterName
274276
}
275277

276-
func (az *Cloud) getLoadBalancerRuleName(service *v1.Service, protocol v1.Protocol, port int32, subnetName *string) string {
278+
func (az *Cloud) getLoadBalancerRuleName(service *v1.Service, protocol v1.Protocol, port int32) string {
277279
prefix := az.getRulePrefix(service)
278-
if subnetName == nil {
279-
return fmt.Sprintf("%s-%s-%d", prefix, protocol, port)
280+
ruleName := fmt.Sprintf("%s-%s-%d", prefix, protocol, port)
281+
subnet := subnet(service)
282+
if subnet == nil {
283+
return ruleName
280284
}
281-
return fmt.Sprintf("%s-%s-%s-%d", prefix, *subnetName, protocol, port)
285+
286+
// Load balancer rule name must be less or equal to 80 characters, so excluding the hyphen two segments cannot exceed 79
287+
subnetSegment := *subnet
288+
if len(ruleName)+len(subnetSegment)+1 > loadBalancerRuleNameMaxLength {
289+
subnetSegment = subnetSegment[:loadBalancerRuleNameMaxLength-len(ruleName)-1]
290+
}
291+
292+
return fmt.Sprintf("%s-%s-%s-%d", prefix, subnetSegment, protocol, port)
282293
}
283294

284295
func (az *Cloud) getSecurityRuleName(service *v1.Service, port v1.ServicePort, sourceAddrPrefix string) string {
@@ -316,10 +327,17 @@ func (az *Cloud) serviceOwnsFrontendIP(fip network.FrontendIPConfiguration, serv
316327
return strings.HasPrefix(*fip.Name, baseName)
317328
}
318329

319-
func (az *Cloud) getFrontendIPConfigName(service *v1.Service, subnetName *string) string {
330+
func (az *Cloud) getFrontendIPConfigName(service *v1.Service) string {
320331
baseName := az.GetLoadBalancerName(context.TODO(), "", service)
332+
subnetName := subnet(service)
321333
if subnetName != nil {
322-
return fmt.Sprintf("%s-%s", baseName, *subnetName)
334+
ipcName := fmt.Sprintf("%s-%s", baseName, *subnetName)
335+
336+
// Azure lb front end configuration name must not exceed 80 characters
337+
if len(ipcName) > frontendIPConfigNameMaxLength {
338+
ipcName = ipcName[:frontendIPConfigNameMaxLength]
339+
}
340+
return ipcName
323341
}
324342
return baseName
325343
}

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

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ limitations under the License.
1919
package azure
2020

2121
import (
22+
"strconv"
2223
"testing"
2324

2425
"github.com/stretchr/testify/assert"
@@ -253,3 +254,156 @@ func TestGetAzureLoadBalancerName(t *testing.T) {
253254
assert.Equal(t, c.expected, loadbalancerName, c.description)
254255
}
255256
}
257+
258+
func TestGetLoadBalancingRuleName(t *testing.T) {
259+
az := getTestCloud()
260+
az.PrimaryAvailabilitySetName = "primary"
261+
262+
svc := &v1.Service{
263+
ObjectMeta: meta.ObjectMeta{
264+
Annotations: map[string]string{},
265+
UID: "257b9655-5137-4ad2-b091-ef3f07043ad3",
266+
},
267+
}
268+
269+
cases := []struct {
270+
description string
271+
subnetName string
272+
isInternal bool
273+
useStandardLB bool
274+
protocol v1.Protocol
275+
port int32
276+
expected string
277+
}{
278+
{
279+
description: "internal lb should have subnet name on the rule name",
280+
subnetName: "shortsubnet",
281+
isInternal: true,
282+
useStandardLB: true,
283+
protocol: v1.ProtocolTCP,
284+
port: 9000,
285+
expected: "a257b965551374ad2b091ef3f07043ad-shortsubnet-TCP-9000",
286+
},
287+
{
288+
description: "internal standard lb should have subnet name on the rule name but truncated to 80 characters",
289+
subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet",
290+
isInternal: true,
291+
useStandardLB: true,
292+
protocol: v1.ProtocolTCP,
293+
port: 9000,
294+
expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnngg-TCP-9000",
295+
},
296+
{
297+
description: "internal basic lb should have subnet name on the rule name but truncated to 80 characters",
298+
subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet",
299+
isInternal: true,
300+
useStandardLB: false,
301+
protocol: v1.ProtocolTCP,
302+
port: 9000,
303+
expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnngg-TCP-9000",
304+
},
305+
{
306+
description: "external standard lb should not have subnet name on the rule name",
307+
subnetName: "shortsubnet",
308+
isInternal: false,
309+
useStandardLB: true,
310+
protocol: v1.ProtocolTCP,
311+
port: 9000,
312+
expected: "a257b965551374ad2b091ef3f07043ad-TCP-9000",
313+
},
314+
{
315+
description: "external basic lb should not have subnet name on the rule name",
316+
subnetName: "shortsubnet",
317+
isInternal: false,
318+
useStandardLB: false,
319+
protocol: v1.ProtocolTCP,
320+
port: 9000,
321+
expected: "a257b965551374ad2b091ef3f07043ad-TCP-9000",
322+
},
323+
}
324+
325+
for _, c := range cases {
326+
if c.useStandardLB {
327+
az.Config.LoadBalancerSku = loadBalancerSkuStandard
328+
} else {
329+
az.Config.LoadBalancerSku = loadBalancerSkuBasic
330+
}
331+
svc.Annotations[ServiceAnnotationLoadBalancerInternalSubnet] = c.subnetName
332+
svc.Annotations[ServiceAnnotationLoadBalancerInternal] = strconv.FormatBool(c.isInternal)
333+
334+
loadbalancerRuleName := az.getLoadBalancerRuleName(svc, c.protocol, c.port)
335+
assert.Equal(t, c.expected, loadbalancerRuleName, c.description)
336+
}
337+
}
338+
339+
func TestGetFrontendIPConfigName(t *testing.T) {
340+
az := getTestCloud()
341+
az.PrimaryAvailabilitySetName = "primary"
342+
343+
svc := &v1.Service{
344+
ObjectMeta: meta.ObjectMeta{
345+
Annotations: map[string]string{
346+
ServiceAnnotationLoadBalancerInternalSubnet: "subnet",
347+
ServiceAnnotationLoadBalancerInternal: "true",
348+
},
349+
UID: "257b9655-5137-4ad2-b091-ef3f07043ad3",
350+
},
351+
}
352+
353+
cases := []struct {
354+
description string
355+
subnetName string
356+
isInternal bool
357+
useStandardLB bool
358+
expected string
359+
}{
360+
{
361+
description: "internal lb should have subnet name on the frontend ip configuration name",
362+
subnetName: "shortsubnet",
363+
isInternal: true,
364+
useStandardLB: true,
365+
expected: "a257b965551374ad2b091ef3f07043ad-shortsubnet",
366+
},
367+
{
368+
description: "internal standard lb should have subnet name on the frontend ip configuration name but truncated to 80 characters",
369+
subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet",
370+
isInternal: true,
371+
useStandardLB: true,
372+
expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnnggggggggggg",
373+
},
374+
{
375+
description: "internal basic lb should have subnet name on the frontend ip configuration name but truncated to 80 characters",
376+
subnetName: "averylonnnngggnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggggggggggsubet",
377+
isInternal: true,
378+
useStandardLB: false,
379+
expected: "a257b965551374ad2b091ef3f07043ad-averylonnnngggnnnnnnnnnnnnnnnnnnnnnnggggggggggg",
380+
},
381+
{
382+
description: "external standard lb should not have subnet name on the frontend ip configuration name",
383+
subnetName: "shortsubnet",
384+
isInternal: false,
385+
useStandardLB: true,
386+
expected: "a257b965551374ad2b091ef3f07043ad",
387+
},
388+
{
389+
description: "external basic lb should not have subnet name on the frontend ip configuration name",
390+
subnetName: "shortsubnet",
391+
isInternal: false,
392+
useStandardLB: false,
393+
expected: "a257b965551374ad2b091ef3f07043ad",
394+
},
395+
}
396+
397+
for _, c := range cases {
398+
if c.useStandardLB {
399+
az.Config.LoadBalancerSku = loadBalancerSkuStandard
400+
} else {
401+
az.Config.LoadBalancerSku = loadBalancerSkuBasic
402+
}
403+
svc.Annotations[ServiceAnnotationLoadBalancerInternalSubnet] = c.subnetName
404+
svc.Annotations[ServiceAnnotationLoadBalancerInternal] = strconv.FormatBool(c.isInternal)
405+
406+
ipconfigName := az.getFrontendIPConfigName(svc)
407+
assert.Equal(t, c.expected, ipconfigName, c.description)
408+
}
409+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1238,14 +1238,14 @@ func validateLoadBalancer(t *testing.T, loadBalancer *network.LoadBalancer, serv
12381238
if len(svc.Spec.Ports) > 0 {
12391239
expectedFrontendIPCount++
12401240
expectedFrontendIP := ExpectedFrontendIPInfo{
1241-
Name: az.getFrontendIPConfigName(&svc, subnet(&svc)),
1241+
Name: az.getFrontendIPConfigName(&svc),
12421242
Subnet: subnet(&svc),
12431243
}
12441244
expectedFrontendIPs = append(expectedFrontendIPs, expectedFrontendIP)
12451245
}
12461246
for _, wantedRule := range svc.Spec.Ports {
12471247
expectedRuleCount++
1248-
wantedRuleName := az.getLoadBalancerRuleName(&svc, wantedRule.Protocol, wantedRule.Port, subnet(&svc))
1248+
wantedRuleName := az.getLoadBalancerRuleName(&svc, wantedRule.Protocol, wantedRule.Port)
12491249
foundRule := false
12501250
for _, actualRule := range *loadBalancer.LoadBalancingRules {
12511251
if strings.EqualFold(*actualRule.Name, wantedRuleName) &&

0 commit comments

Comments
 (0)