Merge pull request #61884 from feiskyer/standard

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

 Add support of Azure standard load balancer and public IP

**What this PR does / why we need it**:

This PR adds a new configure `loadBalancerSku` and `excludeMasterFromStandardLB` to support Azure standard load balancer and public IP.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
First step of #60485

**Special notes for your reviewer**:

This is the first step of #60485, and only VMAS is supported yet. VMSS support will be added in a following PR.

**Release note**:

```release-note
Azure cloud provider now supports standard SKU load balancer and public IP. To use it, set cloud provider config with
{
  "loadBalancerSku": "standard",
  "excludeMasterFromStandardLB": true,
}

If excludeMasterFromStandardLB is not set, master nodes are also added to the backend of standard LB.
```

pkg/cloudprovider/providers/azure/azure.go

@@ -49,6 +49,14 @@ const (
 
 	vmTypeVMSS     = "vmss"
 	vmTypeStandard = "standard"
+
+	loadBalancerSkuBasic    = "basic"
+	loadBalancerSkuStandard = "standard"
+)
+
+var (
+	// Master nodes are not added to standard load balancer by default.
+	defaultExcludeMasterFromStandardLB = true
 )
 
 // Config holds the configuration parsed from the --cloud-config flag
@@ -109,6 +117,13 @@ type Config struct {
 	// Use instance metadata service where possible
 	UseInstanceMetadata bool `json:"useInstanceMetadata" yaml:"useInstanceMetadata"`
 
+	// Sku of Load Balancer and Public IP. Candidate values are: basic and standard.
+	// If not set, it will be default to basic.
+	LoadBalancerSku string `json:"loadBalancerSku" yaml:"loadBalancerSku"`
+	// ExcludeMasterFromStandardLB excludes master nodes from standard load balancer.
+	// If not set, it will be default to true.
+	ExcludeMasterFromStandardLB *bool `json:"excludeMasterFromStandardLB" yaml:"excludeMasterFromStandardLB"`
+
 	// Maximum allowed LoadBalancer Rule Count is the limit enforced by Azure Load balancer
 	MaximumLoadBalancerRuleCount int `json:"maximumLoadBalancerRuleCount" yaml:"maximumLoadBalancerRuleCount"`
 }
@@ -208,7 +223,11 @@ func NewCloud(configReader io.Reader) (cloudprovider.Interface, error) {
 		glog.V(2).Infof("Azure cloudprovider (write ops) using rate limit config: QPS=%g, bucket=%d",
 			config.CloudProviderRateLimitQPSWrite,
 			config.CloudProviderRateLimitBucketWrite)
+	}
 
+	// Do not add master nodes to standard LB by default.
+	if config.ExcludeMasterFromStandardLB == nil {
+		config.ExcludeMasterFromStandardLB = &defaultExcludeMasterFromStandardLB
 	}
 
 	azClientConfig := &azClientConfig{

pkg/cloudprovider/providers/azure/azure_fakes.go

@@ -1158,7 +1158,7 @@ func (f *fakeVMSet) GetVMSetNames(service *v1.Service, nodes []*v1.Node) (availa
 	return nil, fmt.Errorf("unimplemented")
 }
 
-func (f *fakeVMSet) EnsureHostsInPool(serviceName string, nodes []*v1.Node, backendPoolID string, vmSetName string) error {
+func (f *fakeVMSet) EnsureHostsInPool(serviceName string, nodes []*v1.Node, backendPoolID string, vmSetName string, isInternal bool) error {
 	return fmt.Errorf("unimplemented")
 }
 

pkg/cloudprovider/providers/azure/azure_loadbalancer.go

@@ -219,8 +219,14 @@ func (az *Cloud) getServiceLoadBalancer(service *v1.Service, clusterName string,
 		}
 	}
 
-	// service does not have a load balancer, select one
-	if wantLb {
+	hasMode, _, _ := getServiceLoadBalancerMode(service)
+	if az.useStandardLoadBalancer() && hasMode {
+		return nil, nil, false, fmt.Errorf("standard load balancer doesn't work with annotation %q", ServiceAnnotationLoadBalancerMode)
+	}
+
+	// service does not have a basic load balancer, select one.
+	// Standard load balancer doesn't need this because all backends nodes should be added to same LB.
+	if wantLb && !az.useStandardLoadBalancer() {
 		// select new load balancer for service
 		selectedLB, exists, err := az.selectLoadBalancer(clusterName, service, &existingLBs, nodes)
 		if err != nil {
@@ -237,6 +243,11 @@ func (az *Cloud) getServiceLoadBalancer(service *v1.Service, clusterName string,
 			Location:                     &az.Location,
 			LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{},
 		}
+		if az.useStandardLoadBalancer() {
+			defaultLB.Sku = &network.LoadBalancerSku{
+				Name: network.LoadBalancerSkuNameStandard,
+			}
+		}
 	}
 
 	return defaultLB, nil, false, nil
@@ -427,6 +438,12 @@ func (az *Cloud) ensurePublicIPExists(service *v1.Service, pipName string, domai
 		}
 	}
 	pip.Tags = &map[string]*string{"service": &serviceName}
+	if az.useStandardLoadBalancer() {
+		pip.Sku = &network.PublicIPAddressSku{
+			Name: network.PublicIPAddressSkuNameStandard,
+		}
+	}
+
 	glog.V(3).Infof("ensure(%s): pip(%s) - creating", serviceName, *pip.Name)
 	glog.V(10).Infof("CreateOrUpdatePIPWithRetry(%s, %q): start", pipResourceGroup, *pip.Name)
 	err = az.CreateOrUpdatePIPWithRetry(pipResourceGroup, pip)
@@ -804,7 +821,7 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
 	if wantLb && nodes != nil {
 		// Add the machines to the backend pool if they're not already
 		vmSetName := az.mapLoadBalancerNameToVMSet(lbName, clusterName)
-		err := az.vmSet.EnsureHostsInPool(serviceName, nodes, lbBackendPoolID, vmSetName)
+		err := az.vmSet.EnsureHostsInPool(serviceName, nodes, lbBackendPoolID, vmSetName, isInternal)
 		if err != nil {
 			return nil, err
 		}

pkg/cloudprovider/providers/azure/azure_standard.go

@@ -59,6 +59,7 @@ const (
 
 var errNotInVMSet = errors.New("vm is not in the vmset")
 var providerIDRE = regexp.MustCompile(`^` + CloudProviderName + `://(?:.*)/Microsoft.Compute/virtualMachines/(.+)$`)
+var backendPoolIDRE = regexp.MustCompile(`^/subscriptions/(?:.*)/resourceGroups/(?:.*)/providers/Microsoft.Network/loadBalancers/(.+)/backendAddressPools/(?:.*)`)
 
 // getStandardMachineID returns the full identifier of a virtual machine.
 func (az *Cloud) getStandardMachineID(machineName string) string {
@@ -109,6 +110,12 @@ func (az *Cloud) getLoadBalancerProbeID(lbName, lbRuleName string) string {
 }
 
 func (az *Cloud) mapLoadBalancerNameToVMSet(lbName string, clusterName string) (vmSetName string) {
+	// Backends of Standard load balancer could belong to multiple VMAS or VMSS.
+	// Return "" to indicate so that following logic won't check this.
+	if az.useStandardLoadBalancer() {
+		return ""
+	}
+
 	vmSetName = strings.TrimSuffix(lbName, InternalLoadBalancerNameSuffix)
 	if strings.EqualFold(clusterName, vmSetName) {
 		vmSetName = az.vmSet.GetPrimaryVMSetName()
@@ -123,7 +130,7 @@ func (az *Cloud) mapLoadBalancerNameToVMSet(lbName string, clusterName string) (
 // This would be the name for Azure LoadBalancer resource.
 func (az *Cloud) getLoadBalancerName(clusterName string, vmSetName string, isInternal bool) string {
 	lbNamePrefix := vmSetName
-	if strings.EqualFold(vmSetName, az.vmSet.GetPrimaryVMSetName()) {
+	if strings.EqualFold(vmSetName, az.vmSet.GetPrimaryVMSetName()) || az.useStandardLoadBalancer() {
 		lbNamePrefix = clusterName
 	}
 	if isInternal {
@@ -592,7 +599,7 @@ func (as *availabilitySet) GetPrimaryInterface(nodeName, vmSetName string) (netw
 
 // ensureHostInPool ensures the given VM's Primary NIC's Primary IP Configuration is
 // participating in the specified LoadBalancer Backend Pool.
-func (as *availabilitySet) ensureHostInPool(serviceName string, nodeName types.NodeName, backendPoolID string, vmSetName string) error {
+func (as *availabilitySet) ensureHostInPool(serviceName string, nodeName types.NodeName, backendPoolID string, vmSetName string, isInternal bool) error {
 	vmName := mapNodeNameToVMName(nodeName)
 	nic, err := as.GetPrimaryInterface(vmName, vmSetName)
 	if err != nil {
@@ -623,6 +630,22 @@ func (as *availabilitySet) ensureHostInPool(serviceName string, nodeName types.N
 		}
 	}
 	if !foundPool {
+		if as.useStandardLoadBalancer() && len(newBackendPools) > 0 {
+			// Although standard load balancer supports backends from multiple availability
+			// 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.
+			backendPool := *newBackendPools[0].ID
+			matches := backendPoolIDRE.FindStringSubmatch(backendPool)
+			if len(matches) == 2 {
+				lbName := matches[1]
+				if strings.HasSuffix(lbName, InternalLoadBalancerNameSuffix) == isInternal {
+					glog.V(4).Infof("Node %q has already been added to LB %q, omit adding it to a new one", nodeName, lbName)
+					return nil
+				}
+			}
+		}
+
 		newBackendPools = append(newBackendPools,
 			network.BackendAddressPool{
 				ID: to.StringPtr(backendPoolID),
@@ -653,18 +676,23 @@ func (as *availabilitySet) ensureHostInPool(serviceName string, nodeName types.N
 
 // EnsureHostsInPool ensures the given Node's primary IP configurations are
 // participating in the specified LoadBalancer Backend Pool.
-func (as *availabilitySet) EnsureHostsInPool(serviceName string, nodes []*v1.Node, backendPoolID string, vmSetName string) error {
-	hostUpdates := make([]func() error, len(nodes))
-	for i, node := range nodes {
+func (as *availabilitySet) EnsureHostsInPool(serviceName string, nodes []*v1.Node, backendPoolID string, vmSetName string, isInternal bool) error {
+	hostUpdates := make([]func() error, 0, len(nodes))
+	for _, node := range nodes {
 		localNodeName := node.Name
+		if as.useStandardLoadBalancer() && as.excludeMasterNodesFromStandardLB() && isMasterNode(node) {
+			glog.V(4).Infof("Excluding master node %q from load balancer backendpool %q", localNodeName, backendPoolID)
+			continue
+		}
+
 		f := func() error {
-			err := as.ensureHostInPool(serviceName, types.NodeName(localNodeName), backendPoolID, vmSetName)
+			err := as.ensureHostInPool(serviceName, types.NodeName(localNodeName), backendPoolID, vmSetName, isInternal)
 			if err != nil {
 				return fmt.Errorf("ensure(%s): backendPoolID(%s) - failed to ensure host in pool: %q", serviceName, backendPoolID, err)
 			}
 			return nil
 		}
-		hostUpdates[i] = f
+		hostUpdates = append(hostUpdates, f)
 	}
 
 	errs := utilerrors.AggregateGoroutines(hostUpdates...)

pkg/cloudprovider/providers/azure/azure_standard_test.go

@@ -130,6 +130,7 @@ func TestMapLoadBalancerNameToVMSet(t *testing.T) {
 	cases := []struct {
 		description   string
 		lbName        string
+		useStandardLB bool
 		clusterName   string
 		expectedVMSet string
 	}{
@@ -157,10 +158,124 @@ func TestMapLoadBalancerNameToVMSet(t *testing.T) {
 			clusterName:   "azure",
 			expectedVMSet: "azuretest",
 		},
+		{
+			description:   "default standard external LB should map to empty string",
+			lbName:        "azure",
+			useStandardLB: true,
+			clusterName:   "azure",
+			expectedVMSet: "",
+		},
+		{
+			description:   "default standard internal LB should map to empty string",
+			lbName:        "azure-internal",
+			useStandardLB: true,
+			clusterName:   "azure",
+			expectedVMSet: "",
+		},
+		{
+			description:   "non-default standard external LB should map to empty string",
+			lbName:        "azuretest",
+			useStandardLB: true,
+			clusterName:   "azure",
+			expectedVMSet: "",
+		},
+		{
+			description:   "non-default standard internal LB should map to empty string",
+			lbName:        "azuretest-internal",
+			useStandardLB: true,
+			clusterName:   "azure",
+			expectedVMSet: "",
+		},
 	}
 
 	for _, c := range cases {
+		if c.useStandardLB {
+			az.Config.LoadBalancerSku = loadBalancerSkuStandard
+		} else {
+			az.Config.LoadBalancerSku = loadBalancerSkuBasic
+		}
 		vmset := az.mapLoadBalancerNameToVMSet(c.lbName, c.clusterName)
 		assert.Equal(t, c.expectedVMSet, vmset, c.description)
 	}
 }
+
+func TestGetLoadBalancerName(t *testing.T) {
+	az := getTestCloud()
+	az.PrimaryAvailabilitySetName = "primary"
+
+	cases := []struct {
+		description   string
+		vmSet         string
+		isInternal    bool
+		useStandardLB bool
+		clusterName   string
+		expected      string
+	}{
+		{
+			description: "default external LB should get primary vmset",
+			vmSet:       "primary",
+			clusterName: "azure",
+			expected:    "azure",
+		},
+		{
+			description: "default internal LB should get primary vmset",
+			vmSet:       "primary",
+			clusterName: "azure",
+			isInternal:  true,
+			expected:    "azure-internal",
+		},
+		{
+			description: "non-default external LB should get its own vmset",
+			vmSet:       "as",
+			clusterName: "azure",
+			expected:    "as",
+		},
+		{
+			description: "non-default internal LB should get its own vmset",
+			vmSet:       "as",
+			clusterName: "azure",
+			isInternal:  true,
+			expected:    "as-internal",
+		},
+		{
+			description:   "default standard external LB should get cluster name",
+			vmSet:         "primary",
+			useStandardLB: true,
+			clusterName:   "azure",
+			expected:      "azure",
+		},
+		{
+			description:   "default standard internal LB should get cluster name",
+			vmSet:         "primary",
+			useStandardLB: true,
+			isInternal:    true,
+			clusterName:   "azure",
+			expected:      "azure-internal",
+		},
+		{
+			description:   "non-default standard external LB should get cluster-name",
+			vmSet:         "as",
+			useStandardLB: true,
+			clusterName:   "azure",
+			expected:      "azure",
+		},
+		{
+			description:   "non-default standard internal LB should get cluster-name",
+			vmSet:         "as",
+			useStandardLB: true,
+			isInternal:    true,
+			clusterName:   "azure",
+			expected:      "azure-internal",
+		},
+	}
+
+	for _, c := range cases {
+		if c.useStandardLB {
+			az.Config.LoadBalancerSku = loadBalancerSkuStandard
+		} else {
+			az.Config.LoadBalancerSku = loadBalancerSkuBasic
+		}
+		loadbalancerName := az.getLoadBalancerName(c.clusterName, c.vmSet, c.isInternal)
+		assert.Equal(t, c.expected, loadbalancerName, c.description)
+	}
+}

pkg/cloudprovider/providers/azure/azure_vmsets.go

@@ -54,7 +54,7 @@ type VMSet interface {
 	GetVMSetNames(service *v1.Service, nodes []*v1.Node) (availabilitySetNames *[]string, err error)
 	// EnsureHostsInPool ensures the given Node's primary IP configurations are
 	// participating in the specified LoadBalancer Backend Pool.
-	EnsureHostsInPool(serviceName string, nodes []*v1.Node, backendPoolID string, vmSetName string) error
+	EnsureHostsInPool(serviceName string, nodes []*v1.Node, backendPoolID string, vmSetName string, isInternal bool) error
 	// EnsureBackendPoolDeleted ensures the loadBalancer backendAddressPools deleted from the specified vmSet.
 	EnsureBackendPoolDeleted(poolID, vmSetName string) error
 

pkg/cloudprovider/providers/azure/azure_vmss.go

@@ -547,7 +547,7 @@ func (ss *scaleSet) updateVMSSInstancesWithRetry(scaleSetName string, vmInstance
 
 // EnsureHostsInPool ensures the given Node's primary IP configurations are
 // participating in the specified LoadBalancer Backend Pool.
-func (ss *scaleSet) EnsureHostsInPool(serviceName string, nodes []*v1.Node, backendPoolID string, vmSetName string) error {
+func (ss *scaleSet) EnsureHostsInPool(serviceName string, nodes []*v1.Node, backendPoolID string, vmSetName string, isInternal bool) error {
 	virtualMachineScaleSet, exists, err := ss.getScaleSetWithRetry(vmSetName)
 	if err != nil {
 		glog.Errorf("ss.getScaleSetWithRetry(%s) for service %q failed: %v", vmSetName, serviceName, err)

pkg/cloudprovider/providers/azure/azure_wrap.go

@@ -19,6 +19,7 @@ package azure
 import (
 	"fmt"
 	"net/http"
+	"strings"
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/arm/compute"
@@ -242,3 +243,11 @@ func (az *Cloud) newRouteTableCache() (*timedCache, error) {
 
 	return newTimedcache(rtCacheTTL, getter)
 }
+
+func (az *Cloud) useStandardLoadBalancer() bool {
+	return strings.EqualFold(az.LoadBalancerSku, loadBalancerSkuStandard)
+}
+
+func (az *Cloud) excludeMasterNodesFromStandardLB() bool {
+	return az.ExcludeMasterFromStandardLB != nil && *az.ExcludeMasterFromStandardLB
+}