Revert "Revert #7145 now that #7609 is in, and fix the tests that wer…

…e relying on it"
kubernetes · May 3, 2015 · 83b70c4 · 83b70c4
1 parent b447dc0
commit 83b70c4
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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()