From 5b6a78fc778ce68473fd54472c1c01e970c5f223 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Fri, 6 Mar 2015 17:16:27 +0100 Subject: [PATCH] Place external load balancers in a namespace, and add a test to validate the behavior. --- pkg/cloudprovider/fake/fake.go | 11 +++++++++++ pkg/registry/service/rest.go | 17 +++++++++++------ pkg/registry/service/rest_test.go | 6 ++++++ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/pkg/cloudprovider/fake/fake.go b/pkg/cloudprovider/fake/fake.go index 653ab3c947..84c8d9e9ba 100644 --- a/pkg/cloudprovider/fake/fake.go +++ b/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 } diff --git a/pkg/registry/service/rest.go b/pkg/registry/service/rest.go index 9d48e96a23..aa761e50b7 100644 --- a/pkg/registry/service/rest.go +++ b/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 diff --git a/pkg/registry/service/rest_test.go b/pkg/registry/service/rest_test.go index 361e626716..56b734a3e0 100644 --- a/pkg/registry/service/rest_test.go +++ b/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) {