diff --git a/pkg/cloudprovider/providers/azure/azure.go b/pkg/cloudprovider/providers/azure/azure.go index 38a1aa4426d..f06faf7d323 100644 --- a/pkg/cloudprovider/providers/azure/azure.go +++ b/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{ diff --git a/pkg/cloudprovider/providers/azure/azure_fakes.go b/pkg/cloudprovider/providers/azure/azure_fakes.go index 69be811de3a..35a443021f3 100644 --- a/pkg/cloudprovider/providers/azure/azure_fakes.go +++ b/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") } diff --git a/pkg/cloudprovider/providers/azure/azure_loadbalancer.go b/pkg/cloudprovider/providers/azure/azure_loadbalancer.go index a6c0911e57b..cfbc2710b77 100644 --- a/pkg/cloudprovider/providers/azure/azure_loadbalancer.go +++ b/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 } diff --git a/pkg/cloudprovider/providers/azure/azure_standard.go b/pkg/cloudprovider/providers/azure/azure_standard.go index 1e93b2316e9..41a258df74f 100644 --- a/pkg/cloudprovider/providers/azure/azure_standard.go +++ b/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...) diff --git a/pkg/cloudprovider/providers/azure/azure_standard_test.go b/pkg/cloudprovider/providers/azure/azure_standard_test.go index 5de3e3c4b1b..4d6e7c24572 100644 --- a/pkg/cloudprovider/providers/azure/azure_standard_test.go +++ b/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) + } +} diff --git a/pkg/cloudprovider/providers/azure/azure_vmsets.go b/pkg/cloudprovider/providers/azure/azure_vmsets.go index 4d78b759587..a7d935bee31 100644 --- a/pkg/cloudprovider/providers/azure/azure_vmsets.go +++ b/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 diff --git a/pkg/cloudprovider/providers/azure/azure_vmss.go b/pkg/cloudprovider/providers/azure/azure_vmss.go index bc064bbd2c4..337049f470a 100644 --- a/pkg/cloudprovider/providers/azure/azure_vmss.go +++ b/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) diff --git a/pkg/cloudprovider/providers/azure/azure_wrap.go b/pkg/cloudprovider/providers/azure/azure_wrap.go index 4c295c1a7cb..4b33f03bdc3 100644 --- a/pkg/cloudprovider/providers/azure/azure_wrap.go +++ b/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 +}