Skip to content

Commit

Permalink
Implement GetLoadBalancerName per provider and add DefaultLoadBalance…
Browse files Browse the repository at this point in the history 
…rName.
  • Loading branch information
@MorrisLaw
MorrisLaw committed Aug 4, 2018
1 parent 0ffee49 commit 6ecec23
Show file tree
Hide file tree
Showing 20 changed files with 138 additions and 101 deletions.
5 changes: 4 additions & 1 deletion pkg/cloudprovider/cloud.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ type Clusters interface {

// TODO(#6812): Use a shorter name that's less likely to be longer than cloud
// providers' name length limits.
func GetLoadBalancerName(service *v1.Service) string {
func DefaultLoadBalancerName(service *v1.Service) string {
//GCE requires that the name of a load balancer starts with a lower case letter.
ret := "a" + string(service.UID)
ret = strings.Replace(ret, "-", "", -1)
Expand Down Expand Up @@ -96,6 +96,9 @@ type LoadBalancer interface {
// Implementations must treat the *v1.Service parameter as read-only and not modify it.
// Parameter 'clusterName' is the name of the cluster as presented to kube-controller-manager
GetLoadBalancer(ctx context.Context, clusterName string, service *v1.Service) (status *v1.LoadBalancerStatus, exists bool, err error)
// GetLoadBalancerName returns the name of the load balancer. Implementations must treat the
// *v1.Service parameter as read-only and not modify it.
GetLoadBalancerName(ctx context.Context, clusterName string, service *v1.Service) string
// EnsureLoadBalancer creates a new load balancer 'name', or updates the existing one. Returns the status of the balancer
// Implementations must treat the *v1.Service and *v1.Node
// parameters as read-only and not modify them.
Expand Down
15 changes: 10 additions & 5 deletions pkg/cloudprovider/providers/aws/aws.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3361,7 +3361,7 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS
return nil, fmt.Errorf("could not find any suitable subnets for creating the ELB")
}

loadBalancerName := cloudprovider.GetLoadBalancerName(apiService)
loadBalancerName := c.GetLoadBalancerName(ctx, clusterName, apiService)
serviceName := types.NamespacedName{Namespace: apiService.Namespace, Name: apiService.Name}

instanceIDs := []string{}
Expand Down Expand Up @@ -3523,7 +3523,7 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS
return nil, fmt.Errorf("could not find any suitable subnets for creating the ELB")
}

loadBalancerName := cloudprovider.GetLoadBalancerName(apiService)
loadBalancerName := c.GetLoadBalancerName(ctx, clusterName, apiService)
serviceName := types.NamespacedName{Namespace: apiService.Namespace, Name: apiService.Name}
securityGroupIDs, err := c.buildELBSecurityGroupList(serviceName, loadBalancerName, annotations)
if err != nil {
Expand Down Expand Up @@ -3646,7 +3646,7 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS

// GetLoadBalancer is an implementation of LoadBalancer.GetLoadBalancer
func (c *Cloud) GetLoadBalancer(ctx context.Context, clusterName string, service *v1.Service) (*v1.LoadBalancerStatus, bool, error) {
loadBalancerName := cloudprovider.GetLoadBalancerName(service)
loadBalancerName := c.GetLoadBalancerName(ctx, clusterName, service)

if isNLB(service.Annotations) {
lb, err := c.describeLoadBalancerv2(loadBalancerName)
Expand All @@ -3672,6 +3672,11 @@ func (c *Cloud) GetLoadBalancer(ctx context.Context, clusterName string, service
return status, true, nil
}

// GetLoadBalancerName is an implementation of LoadBalancer.GetLoadBalancerName
func (c *Cloud) GetLoadBalancerName(ctx context.Context, clusterName string, service *v1.Service) string {
return cloudprovider.DefaultLoadBalancerName(service)
}

func toStatus(lb *elb.LoadBalancerDescription) *v1.LoadBalancerStatus {
status := &v1.LoadBalancerStatus{}

Expand Down Expand Up @@ -3910,7 +3915,7 @@ func (c *Cloud) updateInstanceSecurityGroupsForLoadBalancer(lb *elb.LoadBalancer

// EnsureLoadBalancerDeleted implements LoadBalancer.EnsureLoadBalancerDeleted.
func (c *Cloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName string, service *v1.Service) error {
loadBalancerName := cloudprovider.GetLoadBalancerName(service)
loadBalancerName := c.GetLoadBalancerName(ctx, clusterName, service)

if isNLB(service.Annotations) {
lb, err := c.describeLoadBalancerv2(loadBalancerName)
Expand Down Expand Up @@ -4158,7 +4163,7 @@ func (c *Cloud) UpdateLoadBalancer(ctx context.Context, clusterName string, serv
return err
}

loadBalancerName := cloudprovider.GetLoadBalancerName(service)
loadBalancerName := c.GetLoadBalancerName(ctx, clusterName, service)
if isNLB(service.Annotations) {
lb, err := c.describeLoadBalancerv2(loadBalancerName)
if err != nil {
Expand Down
32 changes: 19 additions & 13 deletions pkg/cloudprovider/providers/azure/azure_loadbalancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
serviceapi "k8s.io/kubernetes/pkg/api/v1/service"
"k8s.io/kubernetes/pkg/cloudprovider"

"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2017-09-01/network"
"github.com/Azure/go-autorest/autorest/to"
Expand Down Expand Up @@ -186,6 +187,11 @@ func (az *Cloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName stri
return nil
}

// GetLoadBalancerName returns the LoadBalancer name.
func (az *Cloud) GetLoadBalancerName(ctx context.Context, clusterName string, service *v1.Service) string {
return cloudprovider.DefaultLoadBalancerName(service)
}

// getServiceLoadBalancer gets the loadbalancer for the service if it already exists.
// If wantLb is TRUE then -it selects a new load balancer.
// In case the selected load balancer does not exist it returns network.LoadBalancer struct
Expand All @@ -195,7 +201,7 @@ func (az *Cloud) getServiceLoadBalancer(service *v1.Service, clusterName string,
isInternal := requiresInternalLoadBalancer(service)
var defaultLB *network.LoadBalancer
primaryVMSetName := az.vmSet.GetPrimaryVMSetName()
defaultLBName := az.getLoadBalancerName(clusterName, primaryVMSetName, isInternal)
defaultLBName := az.getAzureLoadBalancerName(clusterName, primaryVMSetName, isInternal)

existingLBs, err := az.ListLBWithRetry()
if err != nil {
Expand Down Expand Up @@ -280,7 +286,7 @@ func (az *Cloud) selectLoadBalancer(clusterName string, service *v1.Service, exi
}
selectedLBRuleCount := math.MaxInt32
for _, currASName := range *vmSetNames {
currLBName := az.getLoadBalancerName(clusterName, currASName, isInternal)
currLBName := az.getAzureLoadBalancerName(clusterName, currASName, isInternal)
lb, exists := mapExistingLBs[currLBName]
if !exists {
// select this LB as this is a new LB and will have minimum rules
Expand Down Expand Up @@ -330,7 +336,7 @@ func (az *Cloud) getServiceLoadBalancerStatus(service *v1.Service, lb *network.L
return nil, nil
}
isInternal := requiresInternalLoadBalancer(service)
lbFrontendIPConfigName := getFrontendIPConfigName(service, subnet(service))
lbFrontendIPConfigName := az.getFrontendIPConfigName(service, subnet(service))
serviceName := getServiceName(service)
for _, ipConfiguration := range *lb.FrontendIPConfigurations {
if lbFrontendIPConfigName == *ipConfiguration.Name {
Expand Down Expand Up @@ -369,7 +375,7 @@ func (az *Cloud) getServiceLoadBalancerStatus(service *v1.Service, lb *network.L
func (az *Cloud) determinePublicIPName(clusterName string, service *v1.Service) (string, error) {
loadBalancerIP := service.Spec.LoadBalancerIP
if len(loadBalancerIP) == 0 {
return getPublicIPName(clusterName, service), nil
return az.getPublicIPName(clusterName, service), nil
}

pipResourceGroup := az.getPublicIPAddressResourceGroup(service)
Expand Down Expand Up @@ -511,7 +517,7 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
}
lbName := *lb.Name
glog.V(2).Infof("reconcileLoadBalancer for service(%s): lb(%s) wantLb(%t) resolved load balancer name", serviceName, lbName, wantLb)
lbFrontendIPConfigName := getFrontendIPConfigName(service, subnet(service))
lbFrontendIPConfigName := az.getFrontendIPConfigName(service, subnet(service))
lbFrontendIPConfigID := az.getFrontendIPConfigID(lbName, lbFrontendIPConfigName)
lbBackendPoolName := getBackendPoolName(clusterName)
lbBackendPoolID := az.getBackendPoolID(lbName, lbBackendPoolName)
Expand Down Expand Up @@ -561,7 +567,7 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
if !wantLb {
for i := len(newConfigs) - 1; i >= 0; i-- {
config := newConfigs[i]
if serviceOwnsFrontendIP(config, service) {
if az.serviceOwnsFrontendIP(config, service) {
glog.V(2).Infof("reconcileLoadBalancer for service (%s)(%t): lb frontendconfig(%s) - dropping", serviceName, wantLb, lbFrontendIPConfigName)
newConfigs = append(newConfigs[:i], newConfigs[i+1:]...)
dirtyConfigs = true
Expand All @@ -571,7 +577,7 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
if isInternal {
for i := len(newConfigs) - 1; i >= 0; i-- {
config := newConfigs[i]
if serviceOwnsFrontendIP(config, service) && !strings.EqualFold(*config.Name, lbFrontendIPConfigName) {
if az.serviceOwnsFrontendIP(config, service) && !strings.EqualFold(*config.Name, lbFrontendIPConfigName) {
glog.V(2).Infof("reconcileLoadBalancer for service (%s)(%t): lb frontendconfig(%s) - dropping", serviceName, wantLb, *config.Name)
newConfigs = append(newConfigs[:i], newConfigs[i+1:]...)
dirtyConfigs = true
Expand Down Expand Up @@ -656,7 +662,7 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
var expectedProbes []network.Probe
var expectedRules []network.LoadBalancingRule
for _, port := range ports {
lbRuleName := getLoadBalancerRuleName(service, port, subnet(service))
lbRuleName := az.getLoadBalancerRuleName(service, port, subnet(service))

transportProto, _, probeProto, err := getProtocolsFromKubernetesProtocol(port.Protocol)
if err != nil {
Expand Down Expand Up @@ -739,7 +745,7 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
}
for i := len(updatedProbes) - 1; i >= 0; i-- {
existingProbe := updatedProbes[i]
if serviceOwnsRule(service, *existingProbe.Name) {
if az.serviceOwnsRule(service, *existingProbe.Name) {
glog.V(10).Infof("reconcileLoadBalancer for service (%s)(%t): lb probe(%s) - considering evicting", serviceName, wantLb, *existingProbe.Name)
keepProbe := false
if findProbe(expectedProbes, existingProbe) {
Expand Down Expand Up @@ -780,7 +786,7 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
// update rules: remove unwanted
for i := len(updatedRules) - 1; i >= 0; i-- {
existingRule := updatedRules[i]
if serviceOwnsRule(service, *existingRule.Name) {
if az.serviceOwnsRule(service, *existingRule.Name) {
keepRule := false
glog.V(10).Infof("reconcileLoadBalancer for service (%s)(%t): lb rule(%s) - considering evicting", serviceName, wantLb, *existingRule.Name)
if findRule(expectedRules, existingRule) {
Expand Down Expand Up @@ -939,7 +945,7 @@ func (az *Cloud) reconcileSecurityGroup(clusterName string, service *v1.Service,
}
for j := range sourceAddressPrefixes {
ix := i*len(sourceAddressPrefixes) + j
securityRuleName := getSecurityRuleName(service, port, sourceAddressPrefixes[j])
securityRuleName := az.getSecurityRuleName(service, port, sourceAddressPrefixes[j])
expectedSecurityRules[ix] = network.SecurityRule{
Name: to.StringPtr(securityRuleName),
SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{
Expand Down Expand Up @@ -975,7 +981,7 @@ func (az *Cloud) reconcileSecurityGroup(clusterName string, service *v1.Service,
// to this service
for i := len(updatedRules) - 1; i >= 0; i-- {
existingRule := updatedRules[i]
if serviceOwnsRule(service, *existingRule.Name) {
if az.serviceOwnsRule(service, *existingRule.Name) {
glog.V(10).Infof("reconcile(%s)(%t): sg rule(%s) - considering evicting", serviceName, wantLb, *existingRule.Name)
keepRule := false
if findSecurityRule(expectedSecurityRules, existingRule) {
Expand All @@ -994,7 +1000,7 @@ func (az *Cloud) reconcileSecurityGroup(clusterName string, service *v1.Service,
if useSharedSecurityRule(service) && !wantLb {
for _, port := range ports {
for _, sourceAddressPrefix := range sourceAddressPrefixes {
sharedRuleName := getSecurityRuleName(service, port, sourceAddressPrefix)
sharedRuleName := az.getSecurityRuleName(service, port, sourceAddressPrefix)
sharedIndex, sharedRule, sharedRuleFound := findSecurityRuleByName(updatedRules, sharedRuleName)
if !sharedRuleFound {
glog.V(4).Infof("Expected to find shared rule %s for service %s being deleted, but did not", sharedRuleName, service.Name)
Expand Down
35 changes: 19 additions & 16 deletions pkg/cloudprovider/providers/azure/azure_standard.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package azure

import (
"context"
"errors"
"fmt"
"hash/crc32"
Expand Down Expand Up @@ -123,7 +124,7 @@ func (az *Cloud) mapLoadBalancerNameToVMSet(lbName string, clusterName string) (
// Thus Azure do not allow mixed type (public and internal) load balancer.
// So we'd have a separate name for internal load balancer.
// This would be the name for Azure LoadBalancer resource.
func (az *Cloud) getLoadBalancerName(clusterName string, vmSetName string, isInternal bool) string {
func (az *Cloud) getAzureLoadBalancerName(clusterName string, vmSetName string, isInternal bool) string {
lbNamePrefix := vmSetName
if strings.EqualFold(vmSetName, az.vmSet.GetPrimaryVMSetName()) || az.useStandardLoadBalancer() {
lbNamePrefix = clusterName
Expand Down Expand Up @@ -220,20 +221,22 @@ func getBackendPoolName(clusterName string) string {
return clusterName
}

func getLoadBalancerRuleName(service *v1.Service, port v1.ServicePort, subnetName *string) string {
func (az *Cloud) getLoadBalancerRuleName(service *v1.Service, port v1.ServicePort, subnetName *string) string {
prefix := az.getRulePrefix(service)
if subnetName == nil {
return fmt.Sprintf("%s-%s-%d", getRulePrefix(service), port.Protocol, port.Port)
return fmt.Sprintf("%s-%s-%d", prefix, port.Protocol, port.Port)
}
return fmt.Sprintf("%s-%s-%s-%d", getRulePrefix(service), *subnetName, port.Protocol, port.Port)
return fmt.Sprintf("%s-%s-%s-%d", prefix, *subnetName, port.Protocol, port.Port)
}

func getSecurityRuleName(service *v1.Service, port v1.ServicePort, sourceAddrPrefix string) string {
func (az *Cloud) getSecurityRuleName(service *v1.Service, port v1.ServicePort, sourceAddrPrefix string) string {
if useSharedSecurityRule(service) {
safePrefix := strings.Replace(sourceAddrPrefix, "/", "_", -1)
return fmt.Sprintf("shared-%s-%d-%s", port.Protocol, port.Port, safePrefix)
}
safePrefix := strings.Replace(sourceAddrPrefix, "/", "_", -1)
return fmt.Sprintf("%s-%s-%d-%s", getRulePrefix(service), port.Protocol, port.Port, safePrefix)
rulePrefix := az.getRulePrefix(service)
return fmt.Sprintf("%s-%s-%d-%s", rulePrefix, port.Protocol, port.Port, safePrefix)
}

// This returns a human-readable version of the Service used to tag some resources.
Expand All @@ -243,26 +246,26 @@ func getServiceName(service *v1.Service) string {
}

// This returns a prefix for loadbalancer/security rules.
func getRulePrefix(service *v1.Service) string {
return cloudprovider.GetLoadBalancerName(service)
func (az *Cloud) getRulePrefix(service *v1.Service) string {
return az.GetLoadBalancerName(context.TODO(), "", service)
}

func getPublicIPName(clusterName string, service *v1.Service) string {
return fmt.Sprintf("%s-%s", clusterName, cloudprovider.GetLoadBalancerName(service))
func (az *Cloud) getPublicIPName(clusterName string, service *v1.Service) string {
return fmt.Sprintf("%s-%s", clusterName, az.GetLoadBalancerName(context.TODO(), clusterName, service))
}

func serviceOwnsRule(service *v1.Service, rule string) bool {
prefix := getRulePrefix(service)
func (az *Cloud) serviceOwnsRule(service *v1.Service, rule string) bool {
prefix := az.getRulePrefix(service)
return strings.HasPrefix(strings.ToUpper(rule), strings.ToUpper(prefix))
}

func serviceOwnsFrontendIP(fip network.FrontendIPConfiguration, service *v1.Service) bool {
baseName := cloudprovider.GetLoadBalancerName(service)
func (az *Cloud) serviceOwnsFrontendIP(fip network.FrontendIPConfiguration, service *v1.Service) bool {
baseName := az.GetLoadBalancerName(context.TODO(), "", service)
return strings.HasPrefix(*fip.Name, baseName)
}

func getFrontendIPConfigName(service *v1.Service, subnetName *string) string {
baseName := cloudprovider.GetLoadBalancerName(service)
func (az *Cloud) getFrontendIPConfigName(service *v1.Service, subnetName *string) string {
baseName := az.GetLoadBalancerName(context.TODO(), "", service)
if subnetName != nil {
return fmt.Sprintf("%s-%s", baseName, *subnetName)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/cloudprovider/providers/azure/azure_standard_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func TestMapLoadBalancerNameToVMSet(t *testing.T) {
}
}

func TestGetLoadBalancerName(t *testing.T) {
func TestGetAzureLoadBalancerName(t *testing.T) {
az := getTestCloud()
az.PrimaryAvailabilitySetName = "primary"

Expand Down Expand Up @@ -247,7 +247,7 @@ func TestGetLoadBalancerName(t *testing.T) {
} else {
az.Config.LoadBalancerSku = loadBalancerSkuBasic
}
loadbalancerName := az.getLoadBalancerName(c.clusterName, c.vmSet, c.isInternal)
loadbalancerName := az.getAzureLoadBalancerName(c.clusterName, c.vmSet, c.isInternal)
assert.Equal(t, c.expected, loadbalancerName, c.description)
}
}
14 changes: 8 additions & 6 deletions pkg/cloudprovider/providers/azure/azure_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1160,7 +1160,7 @@ func getTestSecurityGroup(az *Cloud, services ...v1.Service) *network.SecurityGr
for _, port := range service.Spec.Ports {
sources := getServiceSourceRanges(&service)
for _, src := range sources {
ruleName := getSecurityRuleName(&service, port, src)
ruleName := az.getSecurityRuleName(&service, port, src)
rules = append(rules, network.SecurityRule{
Name: to.StringPtr(ruleName),
SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{
Expand Down Expand Up @@ -1191,6 +1191,7 @@ func getTestSecurityGroup(az *Cloud, services ...v1.Service) *network.SecurityGr
}

func validateLoadBalancer(t *testing.T, loadBalancer *network.LoadBalancer, services ...v1.Service) {
az := getTestCloud()
expectedRuleCount := 0
expectedFrontendIPCount := 0
expectedProbeCount := 0
Expand All @@ -1199,14 +1200,14 @@ func validateLoadBalancer(t *testing.T, loadBalancer *network.LoadBalancer, serv
if len(svc.Spec.Ports) > 0 {
expectedFrontendIPCount++
expectedFrontendIP := ExpectedFrontendIPInfo{
Name: getFrontendIPConfigName(&svc, subnet(&svc)),
Name: az.getFrontendIPConfigName(&svc, subnet(&svc)),
Subnet: subnet(&svc),
}
expectedFrontendIPs = append(expectedFrontendIPs, expectedFrontendIP)
}
for _, wantedRule := range svc.Spec.Ports {
expectedRuleCount++
wantedRuleName := getLoadBalancerRuleName(&svc, wantedRule, subnet(&svc))
wantedRuleName := az.getLoadBalancerRuleName(&svc, wantedRule, subnet(&svc))
foundRule := false
for _, actualRule := range *loadBalancer.LoadBalancingRules {
if strings.EqualFold(*actualRule.Name, wantedRuleName) &&
Expand Down Expand Up @@ -1397,12 +1398,13 @@ func securityRuleMatches(serviceSourceRange string, servicePort v1.ServicePort,
}

func validateSecurityGroup(t *testing.T, securityGroup *network.SecurityGroup, services ...v1.Service) {
az := getTestCloud()
seenRules := make(map[string]string)
for _, svc := range services {
for _, wantedRule := range svc.Spec.Ports {
sources := getServiceSourceRanges(&svc)
for _, source := range sources {
wantedRuleName := getSecurityRuleName(&svc, wantedRule, source)
wantedRuleName := az.getSecurityRuleName(&svc, wantedRule, source)
seenRules[wantedRuleName] = wantedRuleName
foundRule := false
for _, actualRule := range *securityGroup.SecurityRules {
Expand Down Expand Up @@ -2568,8 +2570,8 @@ func TestCanCombineSharedAndPrivateRulesInSameGroup(t *testing.T) {

expectedRuleName13 := "shared-TCP-4444-Internet"
expectedRuleName2 := "shared-TCP-8888-Internet"
expectedRuleName4 := getSecurityRuleName(&svc4, v1.ServicePort{Port: 4444, Protocol: v1.ProtocolTCP}, "Internet")
expectedRuleName5 := getSecurityRuleName(&svc5, v1.ServicePort{Port: 8888, Protocol: v1.ProtocolTCP}, "Internet")
expectedRuleName4 := az.getSecurityRuleName(&svc4, v1.ServicePort{Port: 4444, Protocol: v1.ProtocolTCP}, "Internet")
expectedRuleName5 := az.getSecurityRuleName(&svc5, v1.ServicePort{Port: 8888, Protocol: v1.ProtocolTCP}, "Internet")

sg := getTestSecurityGroup(az)

Expand Down
Loading

0 comments on commit 6ecec23

Please sign in to comment.