From 83b70c4411695de89bca18733c9c23c39cf694a2 Mon Sep 17 00:00:00 2001 From: Quinton Hoole Date: Sat, 2 May 2015 23:32:21 -0700 Subject: [PATCH] Revert "Revert #7145 now that #7609 is in, and fix the tests that were relying on it" --- pkg/cloudprovider/cloud.go | 14 +++++++++-- .../servicecontroller/servicecontroller.go | 4 ++-- .../servicecontroller_test.go | 23 ++++++++----------- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index 66f47889ac4..a56a33c6a29 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -18,6 +18,7 @@ package cloudprovider import ( "net" + "strings" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" ) @@ -42,8 +43,17 @@ type Clusters interface { Master(clusterName string) (string, error) } -func GetLoadBalancerName(clusterName, serviceNamespace, serviceName string) string { - return clusterName + "-" + serviceNamespace + "-" + serviceName +// TODO(#6812): Use a shorter name that's less likely to be longer than cloud +// providers' name length limits. +func GetLoadBalancerName(service *api.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) + //AWS requires that the name of a load balancer is shorter than 32 bytes. + if len(ret) > 32 { + ret = ret[:32] + } + return ret } // TCPLoadBalancer is an abstract, pluggable interface for TCP load balancers. diff --git a/pkg/cloudprovider/servicecontroller/servicecontroller.go b/pkg/cloudprovider/servicecontroller/servicecontroller.go index 6072e83937a..80f81993246 100644 --- a/pkg/cloudprovider/servicecontroller/servicecontroller.go +++ b/pkg/cloudprovider/servicecontroller/servicecontroller.go @@ -438,7 +438,7 @@ func needsUpdate(oldService *api.Service, newService *api.Service) bool { } func (s *ServiceController) loadBalancerName(service *api.Service) string { - return cloudprovider.GetLoadBalancerName(s.clusterName, service.Namespace, service.Name) + return cloudprovider.GetLoadBalancerName(service) } func getTCPPorts(service *api.Service) ([]int, error) { @@ -571,7 +571,7 @@ func (s *ServiceController) lockedUpdateLoadBalancerHosts(service *api.Service, return nil } - name := s.loadBalancerName(service) + name := cloudprovider.GetLoadBalancerName(service) err := s.balancer.UpdateTCPLoadBalancer(name, s.zone.Region, hosts) if err == nil { return nil diff --git a/pkg/cloudprovider/servicecontroller/servicecontroller_test.go b/pkg/cloudprovider/servicecontroller/servicecontroller_test.go index a9b268b38a4..ed3c0176699 100644 --- a/pkg/cloudprovider/servicecontroller/servicecontroller_test.go +++ b/pkg/cloudprovider/servicecontroller/servicecontroller_test.go @@ -27,14 +27,9 @@ import ( ) const region = "us-central" -const clusterName = "test-cluster" -const namespace = "namespace" func newService(name string, uid types.UID, external bool) *api.Service { - return &api.Service{ObjectMeta: api.ObjectMeta{Name: name, Namespace: namespace, UID: uid}, Spec: api.ServiceSpec{CreateExternalLoadBalancer: external}} -} -func lbName(serviceName string) string { - return clusterName + "-" + namespace + "-" + serviceName + return &api.Service{ObjectMeta: api.ObjectMeta{Name: name, Namespace: "namespace", UID: uid}, Spec: api.ServiceSpec{CreateExternalLoadBalancer: external}} } func TestCreateExternalLoadBalancer(t *testing.T) { @@ -96,7 +91,7 @@ func TestCreateExternalLoadBalancer(t *testing.T) { cloud := &fake_cloud.FakeCloud{} cloud.Region = region client := &testclient.Fake{} - controller := New(cloud, client, clusterName) + controller := New(cloud, client, "test-cluster") controller.init() cloud.Calls = nil // ignore any cloud calls made in init() client.Actions = nil // ignore any client calls made in init() @@ -160,7 +155,7 @@ func TestUpdateNodesInExternalLoadBalancer(t *testing.T) { newService("s0", "333", true), }, expectedUpdateCalls: []fake_cloud.FakeUpdateBalancerCall{ - {Name: lbName("s0"), Region: region, Hosts: []string{"node0", "node1", "node73"}}, + {Name: "a333", Region: region, Hosts: []string{"node0", "node1", "node73"}}, }, }, { @@ -171,9 +166,9 @@ func TestUpdateNodesInExternalLoadBalancer(t *testing.T) { newService("s2", "666", true), }, expectedUpdateCalls: []fake_cloud.FakeUpdateBalancerCall{ - {Name: lbName("s0"), Region: region, Hosts: []string{"node0", "node1", "node73"}}, - {Name: lbName("s1"), Region: region, Hosts: []string{"node0", "node1", "node73"}}, - {Name: lbName("s2"), Region: region, Hosts: []string{"node0", "node1", "node73"}}, + {Name: "a444", Region: region, Hosts: []string{"node0", "node1", "node73"}}, + {Name: "a555", Region: region, Hosts: []string{"node0", "node1", "node73"}}, + {Name: "a666", Region: region, Hosts: []string{"node0", "node1", "node73"}}, }, }, { @@ -185,8 +180,8 @@ func TestUpdateNodesInExternalLoadBalancer(t *testing.T) { newService("s4", "123", false), }, expectedUpdateCalls: []fake_cloud.FakeUpdateBalancerCall{ - {Name: lbName("s1"), Region: region, Hosts: []string{"node0", "node1", "node73"}}, - {Name: lbName("s3"), Region: region, Hosts: []string{"node0", "node1", "node73"}}, + {Name: "a888", Region: region, Hosts: []string{"node0", "node1", "node73"}}, + {Name: "a999", Region: region, Hosts: []string{"node0", "node1", "node73"}}, }, }, } @@ -195,7 +190,7 @@ func TestUpdateNodesInExternalLoadBalancer(t *testing.T) { cloud.Region = region client := &testclient.Fake{} - controller := New(cloud, client, clusterName) + controller := New(cloud, client, "test-cluster2") controller.init() cloud.Calls = nil // ignore any cloud calls made in init()