From 6ecec23690afaebecb88bd2bebb666ab0665c39e Mon Sep 17 00:00:00 2001 From: morrislaw Date: Sat, 4 Aug 2018 00:36:48 -0400 Subject: [PATCH] Implement GetLoadBalancerName per provider and add DefaultLoadBalancerName. --- pkg/cloudprovider/cloud.go | 5 ++- pkg/cloudprovider/providers/aws/aws.go | 15 +++++--- .../providers/azure/azure_loadbalancer.go | 32 ++++++++++------- .../providers/azure/azure_standard.go | 35 ++++++++++--------- .../providers/azure/azure_standard_test.go | 4 +-- .../providers/azure/azure_test.go | 14 ++++---- .../cloudstack/cloudstack_loadbalancer.go | 8 +++-- pkg/cloudprovider/providers/fake/fake.go | 7 +++- .../providers/gce/gce_loadbalancer.go | 13 ++++--- .../gce/gce_loadbalancer_external.go | 10 +++--- .../gce/gce_loadbalancer_external_test.go | 17 +++++---- .../gce/gce_loadbalancer_internal.go | 8 ++--- .../gce/gce_loadbalancer_internal_test.go | 22 ++++++------ .../gce/gce_loadbalancer_utils_test.go | 10 +++--- .../openstack/openstack_loadbalancer.go | 13 ++++--- pkg/controller/service/service_controller.go | 4 +-- test/e2e/framework/firewall_util.go | 4 +-- test/e2e/network/firewall.go | 2 +- test/e2e/network/network_tiers.go | 2 +- test/e2e/network/service.go | 14 ++++---- 20 files changed, 138 insertions(+), 101 deletions(-) diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index 0358890bcd8..34e464da462 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -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) @@ -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. diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 308bb5c6f73..45f89cc306f 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -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{} @@ -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 { @@ -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) @@ -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{} @@ -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) @@ -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 { diff --git a/pkg/cloudprovider/providers/azure/azure_loadbalancer.go b/pkg/cloudprovider/providers/azure/azure_loadbalancer.go index cc2a6c1b204..1b767e732ac 100644 --- a/pkg/cloudprovider/providers/azure/azure_loadbalancer.go +++ b/pkg/cloudprovider/providers/azure/azure_loadbalancer.go @@ -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" @@ -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 @@ -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 { @@ -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 @@ -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 { @@ -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) @@ -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) @@ -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 @@ -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 @@ -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 { @@ -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) { @@ -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) { @@ -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{ @@ -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) { @@ -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) diff --git a/pkg/cloudprovider/providers/azure/azure_standard.go b/pkg/cloudprovider/providers/azure/azure_standard.go index c7e3a91cf6a..ef0efcbdd5c 100644 --- a/pkg/cloudprovider/providers/azure/azure_standard.go +++ b/pkg/cloudprovider/providers/azure/azure_standard.go @@ -17,6 +17,7 @@ limitations under the License. package azure import ( + "context" "errors" "fmt" "hash/crc32" @@ -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 @@ -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. @@ -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) } diff --git a/pkg/cloudprovider/providers/azure/azure_standard_test.go b/pkg/cloudprovider/providers/azure/azure_standard_test.go index d5071192fbe..aa457fe96bf 100644 --- a/pkg/cloudprovider/providers/azure/azure_standard_test.go +++ b/pkg/cloudprovider/providers/azure/azure_standard_test.go @@ -171,7 +171,7 @@ func TestMapLoadBalancerNameToVMSet(t *testing.T) { } } -func TestGetLoadBalancerName(t *testing.T) { +func TestGetAzureLoadBalancerName(t *testing.T) { az := getTestCloud() az.PrimaryAvailabilitySetName = "primary" @@ -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) } } diff --git a/pkg/cloudprovider/providers/azure/azure_test.go b/pkg/cloudprovider/providers/azure/azure_test.go index 979e6a2bdd6..4ea3e43d267 100644 --- a/pkg/cloudprovider/providers/azure/azure_test.go +++ b/pkg/cloudprovider/providers/azure/azure_test.go @@ -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{ @@ -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 @@ -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) && @@ -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 { @@ -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) diff --git a/pkg/cloudprovider/providers/cloudstack/cloudstack_loadbalancer.go b/pkg/cloudprovider/providers/cloudstack/cloudstack_loadbalancer.go index c4431d41cc5..89912402502 100644 --- a/pkg/cloudprovider/providers/cloudstack/cloudstack_loadbalancer.go +++ b/pkg/cloudprovider/providers/cloudstack/cloudstack_loadbalancer.go @@ -24,7 +24,6 @@ import ( "github.com/golang/glog" "github.com/xanzy/go-cloudstack/cloudstack" "k8s.io/api/core/v1" - "k8s.io/kubernetes/pkg/cloudprovider" ) type loadBalancer struct { @@ -238,11 +237,16 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st return nil } +// GetLoadBalancerName retrieves the name of the LoadBalancer. +func (cs *CSCloud) GetLoadBalancerName(ctx context.Context, clusterName string, service *v1.Service) string { + return cs.GetLoadBalancerName(ctx, clusterName, service) +} + // getLoadBalancer retrieves the IP address and ID and all the existing rules it can find. func (cs *CSCloud) getLoadBalancer(service *v1.Service) (*loadBalancer, error) { lb := &loadBalancer{ CloudStackClient: cs.client, - name: cloudprovider.GetLoadBalancerName(service), + name: cs.GetLoadBalancerName(context.TODO(), "", service), projectID: cs.projectID, rules: make(map[string]*cloudstack.LoadBalancerRule), } diff --git a/pkg/cloudprovider/providers/fake/fake.go b/pkg/cloudprovider/providers/fake/fake.go index 4fcb642d9d2..cf999dc4d8e 100644 --- a/pkg/cloudprovider/providers/fake/fake.go +++ b/pkg/cloudprovider/providers/fake/fake.go @@ -154,6 +154,11 @@ func (f *FakeCloud) GetLoadBalancer(ctx context.Context, clusterName string, ser return status, f.Exists, f.Err } +// GetLoadBalancerName is a stub implementation of LoadBalancer.GetLoadBalancerName. +func (f *FakeCloud) GetLoadBalancerName(ctx context.Context, clusterName string, service *v1.Service) string { + return cloudprovider.DefaultLoadBalancerName(service) +} + // EnsureLoadBalancer is a test-spy implementation of LoadBalancer.EnsureLoadBalancer. // It adds an entry "create" into the internal method call record. func (f *FakeCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, service *v1.Service, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) { @@ -162,7 +167,7 @@ func (f *FakeCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, f.Balancers = make(map[string]FakeBalancer) } - name := cloudprovider.GetLoadBalancerName(service) + name := f.GetLoadBalancerName(ctx, clusterName, service) spec := service.Spec zone, err := f.GetZone(context.TODO()) diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer.go index 4cb66234661..0258c86eb11 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer.go @@ -92,7 +92,7 @@ func LoadBalancerSrcRanges() []string { // GetLoadBalancer is an implementation of LoadBalancer.GetLoadBalancer func (gce *GCECloud) GetLoadBalancer(ctx context.Context, clusterName string, svc *v1.Service) (*v1.LoadBalancerStatus, bool, error) { - loadBalancerName := cloudprovider.GetLoadBalancerName(svc) + loadBalancerName := gce.GetLoadBalancerName(ctx, clusterName, svc) fwd, err := gce.GetRegionForwardingRule(loadBalancerName, gce.region) if err == nil { status := &v1.LoadBalancerStatus{} @@ -103,9 +103,14 @@ func (gce *GCECloud) GetLoadBalancer(ctx context.Context, clusterName string, sv return nil, false, ignoreNotFound(err) } +// GetLoadBalancerName is an implementation of LoadBalancer.GetLoadBalancerName. +func (gce *GCECloud) GetLoadBalancerName(ctx context.Context, clusterName string, svc *v1.Service) string { + return cloudprovider.DefaultLoadBalancerName(svc) +} + // EnsureLoadBalancer is an implementation of LoadBalancer.EnsureLoadBalancer. func (gce *GCECloud) EnsureLoadBalancer(ctx context.Context, clusterName string, svc *v1.Service, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) { - loadBalancerName := cloudprovider.GetLoadBalancerName(svc) + loadBalancerName := gce.GetLoadBalancerName(ctx, clusterName, svc) desiredScheme := getSvcScheme(svc) clusterID, err := gce.ClusterID.GetID() if err != nil { @@ -154,7 +159,7 @@ func (gce *GCECloud) EnsureLoadBalancer(ctx context.Context, clusterName string, // UpdateLoadBalancer is an implementation of LoadBalancer.UpdateLoadBalancer. func (gce *GCECloud) UpdateLoadBalancer(ctx context.Context, clusterName string, svc *v1.Service, nodes []*v1.Node) error { - loadBalancerName := cloudprovider.GetLoadBalancerName(svc) + loadBalancerName := gce.GetLoadBalancerName(ctx, clusterName, svc) scheme := getSvcScheme(svc) clusterID, err := gce.ClusterID.GetID() if err != nil { @@ -175,7 +180,7 @@ func (gce *GCECloud) UpdateLoadBalancer(ctx context.Context, clusterName string, // EnsureLoadBalancerDeleted is an implementation of LoadBalancer.EnsureLoadBalancerDeleted. func (gce *GCECloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName string, svc *v1.Service) error { - loadBalancerName := cloudprovider.GetLoadBalancerName(svc) + loadBalancerName := gce.GetLoadBalancerName(ctx, clusterName, svc) scheme := getSvcScheme(svc) clusterID, err := gce.ClusterID.GetID() if err != nil { diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go index 4fd60b969da..9687393b453 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go @@ -17,6 +17,7 @@ limitations under the License. package gce import ( + "context" "fmt" "net/http" "strconv" @@ -27,7 +28,6 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" apiservice "k8s.io/kubernetes/pkg/api/v1/service" - "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud" netsets "k8s.io/kubernetes/pkg/util/net/sets" @@ -44,7 +44,7 @@ import ( // Due to an interesting series of design decisions, this handles both creating // new load balancers and updating existing load balancers, recognizing when // each is needed. -func (gce *GCECloud) ensureExternalLoadBalancer(clusterName, clusterID string, apiService *v1.Service, existingFwdRule *compute.ForwardingRule, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) { +func (gce *GCECloud) ensureExternalLoadBalancer(clusterName string, clusterID string, apiService *v1.Service, existingFwdRule *compute.ForwardingRule, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) { if len(nodes) == 0 { return nil, fmt.Errorf("Cannot EnsureLoadBalancer() with no hosts") } @@ -56,7 +56,7 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName, clusterID string, a return nil, err } - loadBalancerName := cloudprovider.GetLoadBalancerName(apiService) + loadBalancerName := gce.GetLoadBalancerName(context.TODO(), clusterName, apiService) requestedIP := apiService.Spec.LoadBalancerIP ports := apiService.Spec.Ports portStr := []string{} @@ -281,13 +281,13 @@ func (gce *GCECloud) updateExternalLoadBalancer(clusterName string, service *v1. return err } - loadBalancerName := cloudprovider.GetLoadBalancerName(service) + loadBalancerName := gce.GetLoadBalancerName(context.TODO(), clusterName, service) return gce.updateTargetPool(loadBalancerName, hosts) } // ensureExternalLoadBalancerDeleted is the external implementation of LoadBalancer.EnsureLoadBalancerDeleted func (gce *GCECloud) ensureExternalLoadBalancerDeleted(clusterName, clusterID string, service *v1.Service) error { - loadBalancerName := cloudprovider.GetLoadBalancerName(service) + loadBalancerName := gce.GetLoadBalancerName(context.TODO(), clusterName, service) serviceName := types.NamespacedName{Namespace: service.Namespace, Name: service.Name} lbRefStr := fmt.Sprintf("%v(%v)", loadBalancerName, serviceName) diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go index ac53fa01582..18312857144 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external_test.go @@ -31,7 +31,6 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" - "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud/meta" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud/mock" @@ -330,7 +329,7 @@ func TestUpdateExternalLoadBalancer(t *testing.T) { err = gce.updateExternalLoadBalancer("", svc, newNodes) assert.NoError(t, err) - lbName := cloudprovider.GetLoadBalancerName(svc) + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) pool, err := gce.GetTargetPool(lbName, gce.region) require.NoError(t, err) @@ -401,7 +400,7 @@ func TestLoadBalancerWrongTierResourceDeletion(t *testing.T) { require.NoError(t, err) assert.Equal(t, cloud.NetworkTierPremium, desiredTier) - lbName := cloudprovider.GetLoadBalancerName(svc) + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) serviceName := types.NamespacedName{Namespace: svc.Namespace, Name: svc.Name} // create ForwardingRule and Address with the wrong tier @@ -484,7 +483,7 @@ func TestForwardingRuleNeedsUpdate(t *testing.T) { require.NoError(t, err) svc := fakeLoadbalancerService("") - lbName := cloudprovider.GetLoadBalancerName(svc) + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) ipAddr := status.Ingress[0].IP lbIP := svc.Spec.LoadBalancerIP @@ -566,7 +565,7 @@ func TestTargetPoolNeedsRecreation(t *testing.T) { svc := fakeLoadbalancerService("") serviceName := svc.ObjectMeta.Name - lbName := cloudprovider.GetLoadBalancerName(svc) + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) require.NoError(t, err) hostNames := nodeNames(nodes) @@ -619,7 +618,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { region := vals.Region ipAddr := status.Ingress[0].IP - lbName := cloudprovider.GetLoadBalancerName(svc) + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) ipnet, err := netsets.ParseIPNets("0.0.0.0/0") require.NoError(t, err) @@ -804,7 +803,7 @@ func TestEnsureTargetPoolAndHealthCheck(t *testing.T) { clusterID := vals.ClusterID ipAddr := status.Ingress[0].IP - lbName := cloudprovider.GetLoadBalancerName(svc) + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) region := vals.Region hcToCreate := makeHttpHealthCheck(MakeNodesHealthCheckName(clusterID), GetNodesHealthCheckPath(), GetNodesHealthCheckPort()) @@ -869,7 +868,7 @@ func TestCreateAndUpdateFirewallSucceedsOnXPN(t *testing.T) { require.NoError(t, err) gce.createFirewall( svc, - cloudprovider.GetLoadBalancerName(svc), + gce.GetLoadBalancerName(context.TODO(), "", svc), gce.region, "A sad little firewall", ipnet, @@ -882,7 +881,7 @@ func TestCreateAndUpdateFirewallSucceedsOnXPN(t *testing.T) { gce.updateFirewall( svc, - cloudprovider.GetLoadBalancerName(svc), + gce.GetLoadBalancerName(context.TODO(), "", svc), gce.region, "A sad little firewall", ipnet, diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal.go index ab4fc965705..e4401b21ab1 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal.go @@ -17,6 +17,7 @@ limitations under the License. package gce import ( + "context" "fmt" "strconv" "strings" @@ -27,7 +28,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" v1_service "k8s.io/kubernetes/pkg/api/v1/service" - "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud" ) @@ -39,7 +39,7 @@ func (gce *GCECloud) ensureInternalLoadBalancer(clusterName, clusterID string, s nm := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} ports, protocol := getPortsAndProtocol(svc.Spec.Ports) scheme := cloud.SchemeInternal - loadBalancerName := cloudprovider.GetLoadBalancerName(svc) + loadBalancerName := gce.GetLoadBalancerName(context.TODO(), clusterName, svc) sharedBackend := shareBackendService(svc) backendServiceName := makeBackendServiceName(loadBalancerName, clusterID, sharedBackend, scheme, protocol, svc.Spec.SessionAffinity) backendServiceLink := gce.getBackendServiceLink(backendServiceName) @@ -210,14 +210,14 @@ func (gce *GCECloud) updateInternalLoadBalancer(clusterName, clusterID string, s // Generate the backend service name _, protocol := getPortsAndProtocol(svc.Spec.Ports) scheme := cloud.SchemeInternal - loadBalancerName := cloudprovider.GetLoadBalancerName(svc) + loadBalancerName := gce.GetLoadBalancerName(context.TODO(), clusterName, svc) backendServiceName := makeBackendServiceName(loadBalancerName, clusterID, shareBackendService(svc), scheme, protocol, svc.Spec.SessionAffinity) // Ensure the backend service has the proper backend/instance-group links return gce.ensureInternalBackendServiceGroups(backendServiceName, igLinks) } func (gce *GCECloud) ensureInternalLoadBalancerDeleted(clusterName, clusterID string, svc *v1.Service) error { - loadBalancerName := cloudprovider.GetLoadBalancerName(svc) + loadBalancerName := gce.GetLoadBalancerName(context.TODO(), clusterName, svc) _, protocol := getPortsAndProtocol(svc.Spec.Ports) scheme := cloud.SchemeInternal sharedBackend := shareBackendService(svc) diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go index d24148f20fb..32808409102 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal_test.go @@ -17,6 +17,7 @@ limitations under the License. package gce import ( + "context" "fmt" "strings" "testing" @@ -29,7 +30,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" v1_service "k8s.io/kubernetes/pkg/api/v1/service" - "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud/mock" ) @@ -59,7 +59,7 @@ func TestEnsureInternalBackendServiceUpdates(t *testing.T) { require.NoError(t, err) svc := fakeLoadbalancerService(string(LBTypeInternal)) - lbName := cloudprovider.GetLoadBalancerName(svc) + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) igName := makeInstanceGroupName(vals.ClusterID) igLinks, err := gce.ensureInternalInstanceGroups(igName, nodes) @@ -105,7 +105,7 @@ func TestEnsureInternalBackendServiceGroups(t *testing.T) { require.NoError(t, err) svc := fakeLoadbalancerService(string(LBTypeInternal)) - lbName := cloudprovider.GetLoadBalancerName(svc) + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName) igName := makeInstanceGroupName(vals.ClusterID) igLinks, err := gce.ensureInternalInstanceGroups(igName, nodes) @@ -167,7 +167,7 @@ func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) { // Create the expected resources necessary for an Internal Load Balancer nm := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} - lbName := cloudprovider.GetLoadBalancerName(svc) + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(svc) hcName := makeHealthCheckName(lbName, vals.ClusterID, sharedHealthCheck) @@ -199,7 +199,7 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { require.NoError(t, err) svc := fakeLoadbalancerService(string(LBTypeInternal)) - lbName := cloudprovider.GetLoadBalancerName(svc) + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) // Create a ForwardingRule that's missing an IP address existingFwdRule := &compute.ForwardingRule{ @@ -285,7 +285,7 @@ func TestUpdateInternalLoadBalancerBackendServices(t *testing.T) { // incorrect (missing) attributes. // ensureInternalBackendServiceGroups is called and creates the correct // BackendService - lbName := cloudprovider.GetLoadBalancerName(svc) + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) sharedBackend := shareBackendService(svc) backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", svc.Spec.SessionAffinity) existingBS := &compute.BackendService{ @@ -349,7 +349,7 @@ func TestUpdateInternalLoadBalancerNodes(t *testing.T) { err = gce.updateInternalLoadBalancer(vals.ClusterName, vals.ClusterID, svc, nodes) assert.NoError(t, err) - lbName := cloudprovider.GetLoadBalancerName(svc) + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) sharedBackend := shareBackendService(svc) backendServiceName := makeBackendServiceName(lbName, vals.ClusterID, sharedBackend, cloud.SchemeInternal, "TCP", svc.Spec.SessionAffinity) bs, err := gce.GetRegionBackendService(backendServiceName, gce.region) @@ -442,7 +442,7 @@ func TestEnsureInternalLoadBalancerWithSpecialHealthCheck(t *testing.T) { assert.NoError(t, err) assert.NotEmpty(t, status.Ingress) - loadBalancerName := cloudprovider.GetLoadBalancerName(svc) + loadBalancerName := gce.GetLoadBalancerName(context.TODO(), "", svc) hc, err := gce.GetHealthCheck(loadBalancerName) assert.NoError(t, err) assert.NotNil(t, hc) @@ -453,9 +453,9 @@ func TestClearPreviousInternalResources(t *testing.T) { // Configure testing environment. vals := DefaultTestClusterValues() svc := fakeLoadbalancerService(string(LBTypeInternal)) - loadBalancerName := cloudprovider.GetLoadBalancerName(svc) - nm := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} gce, err := fakeGCECloud(vals) + loadBalancerName := gce.GetLoadBalancerName(context.TODO(), "", svc) + nm := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} c := gce.c.(*cloud.MockGCE) require.NoError(t, err) @@ -516,7 +516,7 @@ func TestEnsureInternalFirewallSucceedsOnXPN(t *testing.T) { require.NoError(t, err) vals := DefaultTestClusterValues() svc := fakeLoadbalancerService(string(LBTypeInternal)) - fwName := cloudprovider.GetLoadBalancerName(svc) + fwName := gce.GetLoadBalancerName(context.TODO(), "", svc) c := gce.c.(*cloud.MockGCE) c.MockFirewalls.InsertHook = mock.InsertFirewallsUnauthorizedErrHook diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go index 7a5dbc715c9..c21cb2e79f2 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_utils_test.go @@ -21,6 +21,7 @@ limitations under the License. package gce import ( + "context" "fmt" "net/http" "strings" @@ -37,7 +38,6 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" v1_service "k8s.io/kubernetes/pkg/api/v1/service" - "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud/meta" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud/mock" @@ -220,7 +220,7 @@ func fakeClusterID(clusterID string) ClusterID { } func assertExternalLbResources(t *testing.T, gce *GCECloud, apiService *v1.Service, vals TestClusterValues, nodeNames []string) { - lbName := cloudprovider.GetLoadBalancerName(apiService) + lbName := gce.GetLoadBalancerName(context.TODO(), "", apiService) hcName := MakeNodesHealthCheckName(vals.ClusterID) // Check that Firewalls are created for the LoadBalancer and the HealthCheck @@ -257,7 +257,7 @@ func assertExternalLbResources(t *testing.T, gce *GCECloud, apiService *v1.Servi } func assertExternalLbResourcesDeleted(t *testing.T, gce *GCECloud, apiService *v1.Service, vals TestClusterValues, firewallsDeleted bool) { - lbName := cloudprovider.GetLoadBalancerName(apiService) + lbName := gce.GetLoadBalancerName(context.TODO(), "", apiService) hcName := MakeNodesHealthCheckName(vals.ClusterID) if firewallsDeleted { @@ -292,7 +292,7 @@ func assertExternalLbResourcesDeleted(t *testing.T, gce *GCECloud, apiService *v } func assertInternalLbResources(t *testing.T, gce *GCECloud, apiService *v1.Service, vals TestClusterValues, nodeNames []string) { - lbName := cloudprovider.GetLoadBalancerName(apiService) + lbName := gce.GetLoadBalancerName(context.TODO(), "", apiService) // Check that Instance Group is created igName := makeInstanceGroupName(vals.ClusterID) @@ -345,7 +345,7 @@ func assertInternalLbResources(t *testing.T, gce *GCECloud, apiService *v1.Servi } func assertInternalLbResourcesDeleted(t *testing.T, gce *GCECloud, apiService *v1.Service, vals TestClusterValues, firewallsDeleted bool) { - lbName := cloudprovider.GetLoadBalancerName(apiService) + lbName := gce.GetLoadBalancerName(context.TODO(), "", apiService) sharedHealthCheck := !v1_service.RequestsOnlyLocalTraffic(apiService) hcName := makeHealthCheckName(lbName, vals.ClusterID, sharedHealthCheck) diff --git a/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go b/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go index f0dad3d5589..aa0b0792411 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go +++ b/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go @@ -460,7 +460,7 @@ func (lbaas *LbaasV2) createLoadBalancer(service *v1.Service, name string, inter // GetLoadBalancer returns whether the specified load balancer exists and its status func (lbaas *LbaasV2) GetLoadBalancer(ctx context.Context, clusterName string, service *v1.Service) (*v1.LoadBalancerStatus, bool, error) { - loadBalancerName := cloudprovider.GetLoadBalancerName(service) + loadBalancerName := lbaas.GetLoadBalancerName(ctx, clusterName, service) loadbalancer, err := getLoadbalancerByName(lbaas.lb, loadBalancerName) if err == ErrNotFound { return nil, false, nil @@ -485,6 +485,11 @@ func (lbaas *LbaasV2) GetLoadBalancer(ctx context.Context, clusterName string, s return status, true, err } +// GetLoadBalancerName is an implementation of LoadBalancer.GetLoadBalancerName. +func (lbaas *LbaasV2) GetLoadBalancerName(ctx context.Context, clusterName string, service *v1.Service) string { + return cloudprovider.DefaultLoadBalancerName(service) +} + // The LB needs to be configured with instance addresses on the same // subnet as the LB (aka opts.SubnetID). Currently we're just // guessing that the node's InternalIP is the right address. @@ -731,7 +736,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(ctx context.Context, clusterName string return nil, fmt.Errorf("unsupported load balancer affinity: %v", affinity) } - name := cloudprovider.GetLoadBalancerName(apiService) + name := lbaas.GetLoadBalancerName(ctx, clusterName, apiService) loadbalancer, err := getLoadbalancerByName(lbaas.lb, name) if err != nil { if err != ErrNotFound { @@ -1173,7 +1178,7 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser // UpdateLoadBalancer updates hosts under the specified load balancer. func (lbaas *LbaasV2) UpdateLoadBalancer(ctx context.Context, clusterName string, service *v1.Service, nodes []*v1.Node) error { - loadBalancerName := cloudprovider.GetLoadBalancerName(service) + loadBalancerName := lbaas.GetLoadBalancerName(ctx, clusterName, service) glog.V(4).Infof("UpdateLoadBalancer(%v, %v, %v)", clusterName, loadBalancerName, nodes) lbaas.opts.SubnetID = getStringFromServiceAnnotation(service, ServiceAnnotationLoadBalancerSubnetID, lbaas.opts.SubnetID) @@ -1396,7 +1401,7 @@ func (lbaas *LbaasV2) updateSecurityGroup(clusterName string, apiService *v1.Ser // EnsureLoadBalancerDeleted deletes the specified load balancer func (lbaas *LbaasV2) EnsureLoadBalancerDeleted(ctx context.Context, clusterName string, service *v1.Service) error { - loadBalancerName := cloudprovider.GetLoadBalancerName(service) + loadBalancerName := lbaas.GetLoadBalancerName(ctx, clusterName, service) glog.V(4).Infof("EnsureLoadBalancerDeleted(%v, %v)", clusterName, loadBalancerName) loadbalancer, err := getLoadbalancerByName(lbaas.lb, loadBalancerName) diff --git a/pkg/controller/service/service_controller.go b/pkg/controller/service/service_controller.go index 48c4ca9bdd3..21d56029e92 100644 --- a/pkg/controller/service/service_controller.go +++ b/pkg/controller/service/service_controller.go @@ -497,7 +497,7 @@ func (s *ServiceController) needsUpdate(oldService *v1.Service, newService *v1.S } func (s *ServiceController) loadBalancerName(service *v1.Service) string { - return cloudprovider.GetLoadBalancerName(service) + return s.balancer.GetLoadBalancerName(context.TODO(), "", service) } func getPortsForLB(service *v1.Service) ([]*v1.ServicePort, error) { @@ -686,7 +686,7 @@ func (s *ServiceController) lockedUpdateLoadBalancerHosts(service *v1.Service, h // It's only an actual error if the load balancer still exists. if _, exists, err := s.balancer.GetLoadBalancer(context.TODO(), s.clusterName, service); err != nil { - glog.Errorf("External error while checking if load balancer %q exists: name, %v", cloudprovider.GetLoadBalancerName(service), err) + glog.Errorf("External error while checking if load balancer %q exists: name, %v", s.balancer.GetLoadBalancerName(context.TODO(), s.clusterName, service), err) } else if !exists { return nil } diff --git a/test/e2e/framework/firewall_util.go b/test/e2e/framework/firewall_util.go index 62b14ed5363..6eff9ca7198 100644 --- a/test/e2e/framework/firewall_util.go +++ b/test/e2e/framework/firewall_util.go @@ -54,7 +54,7 @@ func ConstructFirewallForLBService(svc *v1.Service, nodeTag string) *compute.Fir Failf("can not construct firewall rule for non-loadbalancer type service") } fw := compute.Firewall{} - fw.Name = MakeFirewallNameForLBService(cloudprovider.GetLoadBalancerName(svc)) + fw.Name = MakeFirewallNameForLBService(cloudprovider.DefaultLoadBalancerName(svc)) fw.TargetTags = []string{nodeTag} if svc.Spec.LoadBalancerSourceRanges == nil { fw.SourceRanges = []string{"0.0.0.0/0"} @@ -80,7 +80,7 @@ func ConstructHealthCheckFirewallForLBService(clusterID string, svc *v1.Service, Failf("can not construct firewall rule for non-loadbalancer type service") } fw := compute.Firewall{} - fw.Name = MakeHealthCheckFirewallNameForLBService(clusterID, cloudprovider.GetLoadBalancerName(svc), isNodesHealthCheck) + fw.Name = MakeHealthCheckFirewallNameForLBService(clusterID, cloudprovider.DefaultLoadBalancerName(svc), isNodesHealthCheck) fw.TargetTags = []string{nodeTag} fw.SourceRanges = gcecloud.LoadBalancerSrcRanges() healthCheckPort := gcecloud.GetNodesHealthCheckPort() diff --git a/test/e2e/network/firewall.go b/test/e2e/network/firewall.go index 5324a280274..01616295d78 100644 --- a/test/e2e/network/firewall.go +++ b/test/e2e/network/firewall.go @@ -80,7 +80,7 @@ var _ = SIGDescribe("Firewall rule", func() { }) Expect(cs.CoreV1().Services(svc.Namespace).Delete(svc.Name, nil)).NotTo(HaveOccurred()) By("Waiting for the local traffic health check firewall rule to be deleted") - localHCFwName := framework.MakeHealthCheckFirewallNameForLBService(clusterID, cloudprovider.GetLoadBalancerName(svc), false) + localHCFwName := framework.MakeHealthCheckFirewallNameForLBService(clusterID, cloudprovider.DefaultLoadBalancerName(svc), false) _, err := framework.WaitForFirewallRule(gceCloud, localHCFwName, false, framework.LoadBalancerCleanupTimeout) Expect(err).NotTo(HaveOccurred()) }() diff --git a/test/e2e/network/network_tiers.go b/test/e2e/network/network_tiers.go index 6410c517a84..d8ea9766656 100644 --- a/test/e2e/network/network_tiers.go +++ b/test/e2e/network/network_tiers.go @@ -80,7 +80,7 @@ var _ = SIGDescribe("Services [Feature:GCEAlphaFeature][Slow]", func() { Expect(err).NotTo(HaveOccurred()) Expect(svcTier).To(Equal(cloud.NetworkTierStandard)) // Record the LB name for test cleanup. - serviceLBNames = append(serviceLBNames, cloudprovider.GetLoadBalancerName(svc)) + serviceLBNames = append(serviceLBNames, cloudprovider.DefaultLoadBalancerName(svc)) // Wait and verify the LB. ingressIP := waitAndVerifyLBWithTier(jig, ns, svcName, "", createTimeout, lagTimeout) diff --git a/test/e2e/network/service.go b/test/e2e/network/service.go index 8bda7438af1..aac76a6bdf8 100644 --- a/test/e2e/network/service.go +++ b/test/e2e/network/service.go @@ -620,9 +620,9 @@ var _ = SIGDescribe("Services", func() { s.Spec.Type = v1.ServiceTypeLoadBalancer }) } - serviceLBNames = append(serviceLBNames, cloudprovider.GetLoadBalancerName(tcpService)) + serviceLBNames = append(serviceLBNames, cloudprovider.DefaultLoadBalancerName(tcpService)) if loadBalancerSupportsUDP { - serviceLBNames = append(serviceLBNames, cloudprovider.GetLoadBalancerName(udpService)) + serviceLBNames = append(serviceLBNames, cloudprovider.DefaultLoadBalancerName(udpService)) } By("waiting for the TCP service to have a load balancer") @@ -1637,7 +1637,7 @@ var _ = SIGDescribe("ESIPP [Slow] [DisabledForLargeClusters]", func() { jig := framework.NewServiceTestJig(cs, serviceName) svc := jig.CreateOnlyLocalLoadBalancerService(namespace, serviceName, loadBalancerCreateTimeout, true, nil) - serviceLBNames = append(serviceLBNames, cloudprovider.GetLoadBalancerName(svc)) + serviceLBNames = append(serviceLBNames, cloudprovider.DefaultLoadBalancerName(svc)) healthCheckNodePort := int(svc.Spec.HealthCheckNodePort) if healthCheckNodePort == 0 { framework.Failf("Service HealthCheck NodePort was not allocated") @@ -1709,7 +1709,7 @@ var _ = SIGDescribe("ESIPP [Slow] [DisabledForLargeClusters]", func() { } }) - serviceLBNames = append(serviceLBNames, cloudprovider.GetLoadBalancerName(svc)) + serviceLBNames = append(serviceLBNames, cloudprovider.DefaultLoadBalancerName(svc)) defer func() { jig.ChangeServiceType(svc.Namespace, svc.Name, v1.ServiceTypeClusterIP, loadBalancerCreateTimeout) Expect(cs.CoreV1().Services(svc.Namespace).Delete(svc.Name, nil)).NotTo(HaveOccurred()) @@ -1764,7 +1764,7 @@ var _ = SIGDescribe("ESIPP [Slow] [DisabledForLargeClusters]", func() { nodes := jig.GetNodes(framework.MaxNodesForEndpointsTests) svc := jig.CreateOnlyLocalLoadBalancerService(namespace, serviceName, loadBalancerCreateTimeout, true, nil) - serviceLBNames = append(serviceLBNames, cloudprovider.GetLoadBalancerName(svc)) + serviceLBNames = append(serviceLBNames, cloudprovider.DefaultLoadBalancerName(svc)) defer func() { jig.ChangeServiceType(svc.Namespace, svc.Name, v1.ServiceTypeClusterIP, loadBalancerCreateTimeout) Expect(cs.CoreV1().Services(svc.Namespace).Delete(svc.Name, nil)).NotTo(HaveOccurred()) @@ -1817,7 +1817,7 @@ var _ = SIGDescribe("ESIPP [Slow] [DisabledForLargeClusters]", func() { } svc := jig.CreateOnlyLocalLoadBalancerService(namespace, serviceName, loadBalancerCreateTimeout, true, nil) - serviceLBNames = append(serviceLBNames, cloudprovider.GetLoadBalancerName(svc)) + serviceLBNames = append(serviceLBNames, cloudprovider.DefaultLoadBalancerName(svc)) defer func() { jig.ChangeServiceType(svc.Namespace, svc.Name, v1.ServiceTypeClusterIP, loadBalancerCreateTimeout) Expect(cs.CoreV1().Services(svc.Namespace).Delete(svc.Name, nil)).NotTo(HaveOccurred()) @@ -2024,7 +2024,7 @@ func execAffinityTestForLBService(f *framework.Framework, cs clientset.Interface jig.SanityCheckService(svc, v1.ServiceTypeLoadBalancer) defer func() { framework.StopServeHostnameService(cs, ns, serviceName) - lb := cloudprovider.GetLoadBalancerName(svc) + lb := cloudprovider.DefaultLoadBalancerName(svc) framework.Logf("cleaning load balancer resource for %s", lb) framework.CleanupServiceResources(cs, lb, framework.TestContext.CloudConfig.Region, framework.TestContext.CloudConfig.Zone) }()