Merge pull request #5131 from brendandburns/service

Place external load balancers in a namespace.

pkg/cloudprovider/fake/fake.go

@@ -24,6 +24,15 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider"
 )
 
+// FakeBalancer is a fake storage of balancer information
+type FakeBalancer struct {
+	Name       string
+	Region     string
+	ExternalIP net.IP
+	Port       int
+	Hosts      []string
+}
+
 // FakeCloud is a test-double implementation of Interface, TCPLoadBalancer and Instances. It is useful for testing.
 type FakeCloud struct {
 	Exists        bool
@@ -36,6 +45,7 @@ type FakeCloud struct {
 	ClusterList   []string
 	MasterName    string
 	ExternalIP    net.IP
+	Balancers     []FakeBalancer
 
 	cloudprovider.Zone
 }
@@ -87,6 +97,7 @@ func (f *FakeCloud) TCPLoadBalancerExists(name, region string) (bool, error) {
 // It adds an entry "create" into the internal method call record.
 func (f *FakeCloud) CreateTCPLoadBalancer(name, region string, externalIP net.IP, port int, hosts []string, affinityType api.AffinityType) (net.IP, error) {
 	f.addCall("create")
+	f.Balancers = append(f.Balancers, FakeBalancer{name, region, externalIP, port, hosts})
 	return f.ExternalIP, f.Err
 }
 

pkg/registry/service/rest.go

@@ -138,7 +138,7 @@ func (rs *REST) Delete(ctx api.Context, id string) (runtime.Object, error) {
 	}
 	rs.portalMgr.Release(net.ParseIP(service.Spec.PortalIP))
 	if service.Spec.CreateExternalLoadBalancer {
-		rs.deleteExternalLoadBalancer(service)
+		rs.deleteExternalLoadBalancer(ctx, service)
 	}
 	return &api.Status{Status: api.StatusSuccess}, rs.registry.DeleteService(ctx, id)
 }
@@ -201,7 +201,7 @@ func (rs *REST) Update(ctx api.Context, obj runtime.Object) (runtime.Object, boo
 	if externalLoadBalancerNeedsUpdate(oldService, service) {
 		// TODO: support updating existing balancers
 		if oldService.Spec.CreateExternalLoadBalancer {
-			err = rs.deleteExternalLoadBalancer(oldService)
+			err = rs.deleteExternalLoadBalancer(ctx, oldService)
 			if err != nil {
 				return nil, false, err
 			}
@@ -232,6 +232,10 @@ func (rs *REST) ResourceLocation(ctx api.Context, id string) (string, error) {
 	return net.JoinHostPort(ep.IP, strconv.Itoa(ep.Port)), nil
 }
 
+func (rs *REST) getLoadbalancerName(ctx api.Context, service *api.Service) string {
+	return rs.clusterName + "-" + api.NamespaceValue(ctx) + "-" + service.Name
+}
+
 func (rs *REST) createExternalLoadBalancer(ctx api.Context, service *api.Service) error {
 	if rs.cloud == nil {
 		return fmt.Errorf("requested an external service, but no cloud provider supplied.")
@@ -256,18 +260,19 @@ func (rs *REST) createExternalLoadBalancer(ctx api.Context, service *api.Service
 	if err != nil {
 		return err
 	}
+	name := rs.getLoadbalancerName(ctx, service)
 	// TODO: We should be able to rely on valid input, and not do defaulting here.
 	var affinityType api.AffinityType = service.Spec.SessionAffinity
 	if len(service.Spec.PublicIPs) > 0 {
 		for _, publicIP := range service.Spec.PublicIPs {
-			_, err = balancer.CreateTCPLoadBalancer(rs.clusterName+"-"+service.Name, zone.Region, net.ParseIP(publicIP), service.Spec.Port, hostsFromMinionList(hosts), affinityType)
+			_, err = balancer.CreateTCPLoadBalancer(name, zone.Region, net.ParseIP(publicIP), service.Spec.Port, hostsFromMinionList(hosts), affinityType)
 			if err != nil {
 				// TODO: have to roll-back any successful calls.
 				return err
 			}
 		}
 	} else {
-		ip, err := balancer.CreateTCPLoadBalancer(rs.clusterName+"-"+service.Name, zone.Region, nil, service.Spec.Port, hostsFromMinionList(hosts), affinityType)
+		ip, err := balancer.CreateTCPLoadBalancer(name, zone.Region, nil, service.Spec.Port, hostsFromMinionList(hosts), affinityType)
 		if err != nil {
 			return err
 		}
@@ -276,7 +281,7 @@ func (rs *REST) createExternalLoadBalancer(ctx api.Context, service *api.Service
 	return nil
 }
 
-func (rs *REST) deleteExternalLoadBalancer(service *api.Service) error {
+func (rs *REST) deleteExternalLoadBalancer(ctx api.Context, service *api.Service) error {
 	if rs.cloud == nil {
 		return fmt.Errorf("requested an external service, but no cloud provider supplied.")
 	}
@@ -296,7 +301,7 @@ func (rs *REST) deleteExternalLoadBalancer(service *api.Service) error {
 	if err != nil {
 		return err
 	}
-	if err := balancer.DeleteTCPLoadBalancer(rs.clusterName+"-"+service.Name, zone.Region); err != nil {
+	if err := balancer.DeleteTCPLoadBalancer(rs.getLoadbalancerName(ctx, service), zone.Region); err != nil {
 		return err
 	}
 	return nil

pkg/registry/service/rest_test.go

@@ -224,6 +224,9 @@ func TestServiceRegistryExternalService(t *testing.T) {
 	if srv == nil {
 		t.Errorf("Failed to find service: %s", svc.Name)
 	}
+	if len(fakeCloud.Balancers) != 1 || fakeCloud.Balancers[0].Name != "kubernetes-default-foo" || fakeCloud.Balancers[0].Port != 6502 {
+		t.Errorf("Unexpected balancer created: %v", fakeCloud.Balancers)
+	}
 }
 
 func TestServiceRegistryExternalServiceError(t *testing.T) {
@@ -627,6 +630,9 @@ func TestServiceRegistryIPExternalLoadBalancer(t *testing.T) {
 	if err != nil {
 		t.Errorf("Unexpected error %v", err)
 	}
+	if len(fakeCloud.Balancers) != 1 || fakeCloud.Balancers[0].Name != "kubernetes-default-foo" || fakeCloud.Balancers[0].Port != 6502 {
+		t.Errorf("Unexpected balancer created: %v", fakeCloud.Balancers)
+	}
 }
 
 func TestServiceRegistryIPReloadFromStorage(t *testing.T) {