Skip to content

Commit

Permalink
fix: Only check internal lbs for internal services when using multi-slb
Browse files Browse the repository at this point in the history
  • Loading branch information
nilo19 authored and k8s-infra-cherrypick-robot committed May 8, 2024
1 parent 94a9b06 commit 46670ca
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 9 deletions.
21 changes: 14 additions & 7 deletions pkg/provider/azure_loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -3899,7 +3899,7 @@ func (az *Cloud) getAzureLoadBalancerName(
}

currentLBName := az.getServiceCurrentLoadBalancerName(service)
lbNamePrefix = getMostEligibleLBForService(currentLBName, eligibleLBs, existingLBs)
lbNamePrefix = getMostEligibleLBForService(currentLBName, eligibleLBs, existingLBs, requiresInternalLoadBalancer(service))
}

if isInternal {
Expand All @@ -3912,6 +3912,7 @@ func getMostEligibleLBForService(
currentLBName string,
eligibleLBs []string,
existingLBs *[]network.LoadBalancer,
isInternal bool,
) string {
// 1. If the LB is eligible and being used, choose it.
if StringInSlice(currentLBName, eligibleLBs) {
Expand All @@ -3923,8 +3924,10 @@ func getMostEligibleLBForService(
for _, eligibleLB := range eligibleLBs {
var found bool
if existingLBs != nil {
for _, existingLB := range *existingLBs {
if strings.EqualFold(trimSuffixIgnoreCase(pointer.StringDeref(existingLB.Name, ""), consts.InternalLoadBalancerNameSuffix), eligibleLB) {
for i := range *existingLBs {
existingLB := (*existingLBs)[i]
if strings.EqualFold(trimSuffixIgnoreCase(pointer.StringDeref(existingLB.Name, ""), consts.InternalLoadBalancerNameSuffix), eligibleLB) &&
isInternalLoadBalancer(&existingLB) == isInternal {
found = true
break
}
Expand All @@ -3940,8 +3943,10 @@ func getMostEligibleLBForService(
var expectedLBName string
ruleCount := 301
if existingLBs != nil {
for _, existingLB := range *existingLBs {
if StringInSlice(trimSuffixIgnoreCase(pointer.StringDeref(existingLB.Name, ""), consts.InternalLoadBalancerNameSuffix), eligibleLBs) {
for i := range *existingLBs {
existingLB := (*existingLBs)[i]
if StringInSlice(trimSuffixIgnoreCase(pointer.StringDeref(existingLB.Name, ""), consts.InternalLoadBalancerNameSuffix), eligibleLBs) &&
isInternalLoadBalancer(&existingLB) == isInternal {
if existingLB.LoadBalancerPropertiesFormat != nil &&
existingLB.LoadBalancingRules != nil {
if len(*existingLB.LoadBalancingRules) < ruleCount {
Expand Down Expand Up @@ -3996,7 +4001,8 @@ func (az *Cloud) getEligibleLoadBalancersForService(service *v1.Service) ([]stri
lbsFromAnnotation := consts.GetLoadBalancerConfigurationsNames(service)
if len(lbsFromAnnotation) > 0 {
lbNamesSet := utilsets.NewString(lbsFromAnnotation...)
for _, multiSLBConfig := range az.MultipleStandardLoadBalancerConfigurations {
for i := range az.MultipleStandardLoadBalancerConfigurations {
multiSLBConfig := az.MultipleStandardLoadBalancerConfigurations[i]
if lbNamesSet.Has(multiSLBConfig.Name) {
logger.V(4).Info("selects the load balancer by annotation",
"load balancer configuration name", multiSLBConfig.Name)
Expand Down Expand Up @@ -4121,7 +4127,8 @@ func (az *Cloud) getEligibleLoadBalancersForService(service *v1.Service) ([]stri
logger.V(4).Info("no load balancer that has label/namespace selector matches the service, so the service can be placed on the load balancers that do not have label/namespace selector")
}

for _, eligibleLB := range eligibleLBs {
for i := range eligibleLBs {
eligibleLB := eligibleLBs[i]
eligibleLBNames = append(eligibleLBNames, eligibleLB.Name)
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/provider/azure_loadbalancer_backendpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,9 @@ func (bi *backendPoolTypeNodeIP) EnsureHostsInPool(service *v1.Service, nodes []
}

privateIP := getNodePrivateIPAddress(node, isIPv6)
nodePrivateIPsSet.Insert(privateIP)
if privateIP != "" {
nodePrivateIPsSet.Insert(privateIP)
}

if bi.useMultipleStandardLoadBalancers() {
if !activeNodes.Has(node.Name) {
Expand Down
4 changes: 3 additions & 1 deletion pkg/provider/azure_loadbalancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7400,6 +7400,7 @@ func TestGetMostEligibleLBName(t *testing.T) {
currentLBName string
eligibleLBs []string
existingLBs *[]network.LoadBalancer
isInternal bool
expectedLBName string
}{
{
Expand Down Expand Up @@ -7487,10 +7488,11 @@ func TestGetMostEligibleLBName(t *testing.T) {
},
},
expectedLBName: "lb2",
isInternal: true,
},
} {
t.Run(tc.description, func(t *testing.T) {
lbName := getMostEligibleLBForService(tc.currentLBName, tc.eligibleLBs, tc.existingLBs)
lbName := getMostEligibleLBForService(tc.currentLBName, tc.eligibleLBs, tc.existingLBs, tc.isInternal)
assert.Equal(t, tc.expectedLBName, lbName)
})
}
Expand Down

0 comments on commit 46670ca

Please sign in to comment.