From 522db8fce8a1e5419b1718c2f849bb4b2c3a0474 Mon Sep 17 00:00:00 2001 From: morrislaw Date: Tue, 14 Aug 2018 18:20:03 -0400 Subject: [PATCH 1/3] Updated comment for DefaultLoadBalancerName to provide further context --- pkg/cloudprovider/cloud.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index a2159a9fc79..cc950d4e4ed 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -62,8 +62,9 @@ type Clusters interface { Master(ctx context.Context, clusterName string) (string, error) } -// TODO(#6812): Use a shorter name that's less likely to be longer than cloud -// providers' name length limits. +// DefaultLoadBalancerName is for getting the load balancer name. It should eventually be removed +// because there is an interface method, LoadBalancer.GetLoadBalancerName, which can be implemented +// per provider and this neutral implementation won't fit every cloud provider's requirements. 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) From 4f8fe9d9229c2cad95a7d629a7c6a315f1708097 Mon Sep 17 00:00:00 2001 From: morrislaw Date: Wed, 15 Aug 2018 20:54:29 -0400 Subject: [PATCH 2/3] Update DefaultLoadBalancerName method and add that its deprecated --- pkg/cloudprovider/cloud.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index cc950d4e4ed..43f37827b21 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -62,9 +62,11 @@ type Clusters interface { Master(ctx context.Context, clusterName string) (string, error) } -// DefaultLoadBalancerName is for getting the load balancer name. It should eventually be removed -// because there is an interface method, LoadBalancer.GetLoadBalancerName, which can be implemented -// per provider and this neutral implementation won't fit every cloud provider's requirements. +// (DEPRECATED) DefaultLoadBalancerName is the default load balancer name that is called from +// LoadBalancer.GetLoadBalancerName. Use this method to maintain backward compatible names for +// LoadBalancers that were created prior to Kubernetes v1.12. In the future, each provider should +// replace this method call in GetLoadBalancerName with a provider-specific implementation that +// is less cryptic than the Service's UUID. 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) From df4f26ef399fae2b56a976a90ee6dd50c2fe7719 Mon Sep 17 00:00:00 2001 From: morrislaw Date: Wed, 15 Aug 2018 21:03:44 -0400 Subject: [PATCH 3/3] Updated cloud providers with todo comment if using DefaultLoadBalancerName --- pkg/cloudprovider/providers/aws/aws.go | 1 + pkg/cloudprovider/providers/azure/azure_loadbalancer.go | 1 + pkg/cloudprovider/providers/fake/fake.go | 1 + pkg/cloudprovider/providers/gce/gce_loadbalancer.go | 1 + pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go | 1 + 5 files changed, 5 insertions(+) diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index a50bf427ca9..d6129b6ca0d 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -3648,6 +3648,7 @@ func (c *Cloud) GetLoadBalancer(ctx context.Context, clusterName string, service // GetLoadBalancerName is an implementation of LoadBalancer.GetLoadBalancerName func (c *Cloud) GetLoadBalancerName(ctx context.Context, clusterName string, service *v1.Service) string { + // TODO: replace DefaultLoadBalancerName to generate more meaningful loadbalancer names. return cloudprovider.DefaultLoadBalancerName(service) } diff --git a/pkg/cloudprovider/providers/azure/azure_loadbalancer.go b/pkg/cloudprovider/providers/azure/azure_loadbalancer.go index 1b767e732ac..b5d260c3cc2 100644 --- a/pkg/cloudprovider/providers/azure/azure_loadbalancer.go +++ b/pkg/cloudprovider/providers/azure/azure_loadbalancer.go @@ -189,6 +189,7 @@ func (az *Cloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName stri // GetLoadBalancerName returns the LoadBalancer name. func (az *Cloud) GetLoadBalancerName(ctx context.Context, clusterName string, service *v1.Service) string { + // TODO: replace DefaultLoadBalancerName to generate more meaningful loadbalancer names. return cloudprovider.DefaultLoadBalancerName(service) } diff --git a/pkg/cloudprovider/providers/fake/fake.go b/pkg/cloudprovider/providers/fake/fake.go index cf999dc4d8e..9077f750690 100644 --- a/pkg/cloudprovider/providers/fake/fake.go +++ b/pkg/cloudprovider/providers/fake/fake.go @@ -156,6 +156,7 @@ func (f *FakeCloud) GetLoadBalancer(ctx context.Context, clusterName string, ser // GetLoadBalancerName is a stub implementation of LoadBalancer.GetLoadBalancerName. func (f *FakeCloud) GetLoadBalancerName(ctx context.Context, clusterName string, service *v1.Service) string { + // TODO: replace DefaultLoadBalancerName to generate more meaningful loadbalancer names. return cloudprovider.DefaultLoadBalancerName(service) } diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer.go index 0258c86eb11..86eea9f87d4 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer.go @@ -105,6 +105,7 @@ func (gce *GCECloud) GetLoadBalancer(ctx context.Context, clusterName string, sv // GetLoadBalancerName is an implementation of LoadBalancer.GetLoadBalancerName. func (gce *GCECloud) GetLoadBalancerName(ctx context.Context, clusterName string, svc *v1.Service) string { + // TODO: replace DefaultLoadBalancerName to generate more meaningful loadbalancer names. return cloudprovider.DefaultLoadBalancerName(svc) } diff --git a/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go b/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go index aa0b0792411..d958948458a 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go +++ b/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go @@ -487,6 +487,7 @@ func (lbaas *LbaasV2) GetLoadBalancer(ctx context.Context, clusterName string, s // GetLoadBalancerName is an implementation of LoadBalancer.GetLoadBalancerName. func (lbaas *LbaasV2) GetLoadBalancerName(ctx context.Context, clusterName string, service *v1.Service) string { + // TODO: replace DefaultLoadBalancerName to generate more meaningful loadbalancer names. return cloudprovider.DefaultLoadBalancerName(service) }