Merge pull request kubernetes#76691 from feiskyer/slb-multi-backendpool

Fix Azure SLB support for multiple backend pools

Author: k8s-ci-robot

pkg/cloudprovider/providers/azure/azure_standard.go

@@ -676,17 +676,20 @@ func (as *availabilitySet) ensureHostInPool(service *v1.Service, nodeName types.
 			// sets, the same network interface couldn't be added to more than one load balancer of
 			// the same type. Omit those nodes (e.g. masters) so Azure ARM won't complain
 			// about this.
+			newBackendPoolsIDs := make([]string, 0, len(newBackendPools))
 			for _, pool := range newBackendPools {
-				backendPool := *pool.ID
-				matches := backendPoolIDRE.FindStringSubmatch(backendPool)
-				if len(matches) == 2 {
-					lbName := matches[1]
-					if strings.HasSuffix(lbName, InternalLoadBalancerNameSuffix) == isInternal {
-						klog.V(4).Infof("Node %q has already been added to LB %q, omit adding it to a new one", nodeName, lbName)
-						return nil
-					}
+				if pool.ID != nil {
+					newBackendPoolsIDs = append(newBackendPoolsIDs, *pool.ID)
 				}
 			}
+			isSameLB, oldLBName, err := isBackendPoolOnSameLB(backendPoolID, newBackendPoolsIDs)
+			if err != nil {
+				return err
+			}
+			if !isSameLB {
+				klog.V(4).Infof("Node %q has already been added to LB %q, omit adding it to a new one", nodeName, oldLBName)
+				return nil
+			}
 		}
 
 		newBackendPools = append(newBackendPools,

pkg/cloudprovider/providers/azure/azure_vmss.go

@@ -785,17 +785,20 @@ func (ss *scaleSet) ensureHostsInVMSetPool(service *v1.Service, backendPoolID st
 			// the same network interface couldn't be added to more than one load balancer of
 			// the same type. Omit those nodes (e.g. masters) so Azure ARM won't complain
 			// about this.
+			newBackendPoolsIDs := make([]string, 0, len(newBackendPools))
 			for _, pool := range newBackendPools {
-				backendPool := *pool.ID
-				matches := backendPoolIDRE.FindStringSubmatch(backendPool)
-				if len(matches) == 2 {
-					lbName := matches[1]
-					if strings.HasSuffix(lbName, InternalLoadBalancerNameSuffix) == isInternal {
-						klog.V(4).Infof("vmss %q has already been added to LB %q, omit adding it to a new one", vmSetName, lbName)
-						return nil
-					}
+				if pool.ID != nil {
+					newBackendPoolsIDs = append(newBackendPoolsIDs, *pool.ID)
 				}
 			}
+			isSameLB, oldLBName, err := isBackendPoolOnSameLB(backendPoolID, newBackendPoolsIDs)
+			if err != nil {
+				return err
+			}
+			if !isSameLB {
+				klog.V(4).Infof("VMSS %q has already been added to LB %q, omit adding it to a new one", vmSetName, oldLBName)
+				return nil
+			}
 		}
 
 		newBackendPools = append(newBackendPools,

pkg/cloudprovider/providers/azure/azure_wrap.go

@@ -336,3 +336,29 @@ func convertResourceGroupNameToLower(resourceID string) (string, error) {
 	resourceGroup := matches[1]
 	return strings.Replace(resourceID, resourceGroup, strings.ToLower(resourceGroup), 1), nil
 }
+
+// isBackendPoolOnSameLB checks whether newBackendPoolID is on the same load balancer as existingBackendPools.
+// Since both public and internal LBs are supported, lbName and lbName-internal are treated as same.
+// If not same, the lbName for existingBackendPools would also be returned.
+func isBackendPoolOnSameLB(newBackendPoolID string, existingBackendPools []string) (bool, string, error) {
+	matches := backendPoolIDRE.FindStringSubmatch(newBackendPoolID)
+	if len(matches) != 2 {
+		return false, "", fmt.Errorf("new backendPoolID %q is in wrong format", newBackendPoolID)
+	}
+
+	newLBName := matches[1]
+	newLBNameTrimmed := strings.TrimRight(newLBName, InternalLoadBalancerNameSuffix)
+	for _, backendPool := range existingBackendPools {
+		matches := backendPoolIDRE.FindStringSubmatch(backendPool)
+		if len(matches) != 2 {
+			return false, "", fmt.Errorf("existing backendPoolID %q is in wrong format", backendPool)
+		}
+
+		lbName := matches[1]
+		if !strings.EqualFold(strings.TrimRight(lbName, InternalLoadBalancerNameSuffix), newLBNameTrimmed) {
+			return false, lbName, nil
+		}
+	}
+
+	return true, "", nil
+}

pkg/cloudprovider/providers/azure/azure_wrap_test.go

@@ -198,3 +198,78 @@ func TestConvertResourceGroupNameToLower(t *testing.T) {
 		assert.Equal(t, test.expected, real, test.desc)
 	}
 }
+
+func TestIsBackendPoolOnSameLB(t *testing.T) {
+	tests := []struct {
+		backendPoolID        string
+		existingBackendPools []string
+		expected             bool
+		expectedLBName       string
+		expectError          bool
+	}{
+		{
+			backendPoolID: "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lb1/backendAddressPools/pool1",
+			existingBackendPools: []string{
+				"/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lb1/backendAddressPools/pool2",
+			},
+			expected:       true,
+			expectedLBName: "",
+		},
+		{
+			backendPoolID: "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lb1-internal/backendAddressPools/pool1",
+			existingBackendPools: []string{
+				"/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lb1/backendAddressPools/pool2",
+			},
+			expected:       true,
+			expectedLBName: "",
+		},
+		{
+			backendPoolID: "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lb1/backendAddressPools/pool1",
+			existingBackendPools: []string{
+				"/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lb1-internal/backendAddressPools/pool2",
+			},
+			expected:       true,
+			expectedLBName: "",
+		},
+		{
+			backendPoolID: "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lb1/backendAddressPools/pool1",
+			existingBackendPools: []string{
+				"/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lb2/backendAddressPools/pool2",
+			},
+			expected:       false,
+			expectedLBName: "lb2",
+		},
+		{
+			backendPoolID: "wrong-backendpool-id",
+			existingBackendPools: []string{
+				"/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lb1/backendAddressPools/pool2",
+			},
+			expectError: true,
+		},
+		{
+			backendPoolID: "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/loadBalancers/lb1/backendAddressPools/pool1",
+			existingBackendPools: []string{
+				"wrong-existing-backendpool-id",
+			},
+			expectError: true,
+		},
+		{
+			backendPoolID: "wrong-backendpool-id",
+			existingBackendPools: []string{
+				"wrong-existing-backendpool-id",
+			},
+			expectError: true,
+		},
+	}
+
+	for _, test := range tests {
+		isSameLB, lbName, err := isBackendPoolOnSameLB(test.backendPoolID, test.existingBackendPools)
+		if test.expectError {
+			assert.Error(t, err)
+			continue
+		}
+
+		assert.Equal(t, test.expected, isSameLB)
+		assert.Equal(t, test.expectedLBName, lbName)
+	}
+}