From 1d8407cf7d617b845cfe2d97998cc5143d0ae6cf Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Thu, 31 Jul 2014 17:35:54 -0700 Subject: [PATCH 1/2] Don't hard code load balancer zone. --- pkg/cloudprovider/gce_test.go | 38 +++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 pkg/cloudprovider/gce_test.go diff --git a/pkg/cloudprovider/gce_test.go b/pkg/cloudprovider/gce_test.go new file mode 100644 index 00000000000..c3d8b64d9c9 --- /dev/null +++ b/pkg/cloudprovider/gce_test.go @@ -0,0 +1,38 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cloudprovider + +import ( + "testing" +) + +func TestGetRegion(t *testing.T) { + gce := &GCECloud{ + zone: "us-central1-b", + } + zones, ok := gce.Zones() + if !ok { + t.Errorf("Unexpected missing zones impl") + } + region, err := zones.GetRegion() + if err != nil { + t.Errorf("unexpected error %v (%#v)", err) + } + if region != "us-central1" { + t.Errorf("Unexpected region: %s", region) + } +} From c27ab184814d0f979283eb2f88230f337dca7786 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Mon, 4 Aug 2014 09:58:10 -0700 Subject: [PATCH 2/2] Don't use zone for regional load balancers. --- pkg/cloudprovider/cloud.go | 10 ++++++++-- pkg/cloudprovider/fake_cloud.go | 4 ++-- pkg/cloudprovider/gce.go | 22 ++++++++++++++++++++-- pkg/cloudprovider/gce_test.go | 10 +++++----- pkg/registry/servicestorage.go | 4 ++-- 5 files changed, 37 insertions(+), 13 deletions(-) diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index b2935357913..09e0d25d7e5 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -51,8 +51,14 @@ type Instances interface { List(filter string) ([]string, error) } +// Zone represents the location of a particular machine +type Zone struct { + FailureDomain string + Region string +} + // Zones is an abstract, pluggable interface for zone enumeration. type Zones interface { - // GetZone returns the name of the current failure zone that the program is running in - GetZone() (string, error) + // GetZone returns the Zone containing the current failure zone and locality region that the program is running in + GetZone() (Zone, error) } diff --git a/pkg/cloudprovider/fake_cloud.go b/pkg/cloudprovider/fake_cloud.go index b0cf1dd9bd1..da16f336fdd 100644 --- a/pkg/cloudprovider/fake_cloud.go +++ b/pkg/cloudprovider/fake_cloud.go @@ -28,7 +28,7 @@ type FakeCloud struct { Calls []string IP net.IP Machines []string - Zone string + Zone } func (f *FakeCloud) addCall(desc string) { @@ -104,7 +104,7 @@ func (f *FakeCloud) List(filter string) ([]string, error) { return result, f.Err } -func (f *FakeCloud) GetZone() (string, error) { +func (f *FakeCloud) GetZone() (Zone, error) { f.addCall("get-zone") return f.Zone, f.Err } diff --git a/pkg/cloudprovider/gce.go b/pkg/cloudprovider/gce.go index c753f00697a..f657f7b3c11 100644 --- a/pkg/cloudprovider/gce.go +++ b/pkg/cloudprovider/gce.go @@ -237,6 +237,24 @@ func (gce *GCECloud) List(filter string) ([]string, error) { return instances, nil } -func (gce *GCECloud) GetZone() (string, error) { - return gce.zone, nil +func (gce *GCECloud) GetZone() (Zone, error) { + region, err := getGceRegion(gce.zone) + if err != nil { + return Zone{}, err + } + return Zone{ + FailureDomain: gce.zone, + Region: region, + }, nil +} + +// gce zone names are of the form: ${region-name}-${ix}. +// For example "us-central1-b" has a region of "us-central1". +// So we look for the last '-' and trim to just before that. +func getGceRegion(zone string) (string, error) { + ix := strings.LastIndex(zone, "-") + if ix == -1 { + return "", fmt.Errorf("unexpected zone: %s", zone) + } + return zone[:ix], nil } diff --git a/pkg/cloudprovider/gce_test.go b/pkg/cloudprovider/gce_test.go index c3d8b64d9c9..b542d1d32da 100644 --- a/pkg/cloudprovider/gce_test.go +++ b/pkg/cloudprovider/gce_test.go @@ -26,13 +26,13 @@ func TestGetRegion(t *testing.T) { } zones, ok := gce.Zones() if !ok { - t.Errorf("Unexpected missing zones impl") + t.Fatalf("Unexpected missing zones impl") } - region, err := zones.GetRegion() + zone, err := zones.GetZone() if err != nil { - t.Errorf("unexpected error %v (%#v)", err) + t.Fatalf("unexpected error %v", err) } - if region != "us-central1" { - t.Errorf("Unexpected region: %s", region) + if zone.Region != "us-central1" { + t.Errorf("Unexpected region: %s", zone.Region) } } diff --git a/pkg/registry/servicestorage.go b/pkg/registry/servicestorage.go index 8d1e1de6163..f059f4f2720 100644 --- a/pkg/registry/servicestorage.go +++ b/pkg/registry/servicestorage.go @@ -142,7 +142,7 @@ func (sr *ServiceRegistryStorage) deleteExternalLoadBalancer(service *api.Servic return err } - if err := balancer.DeleteTCPLoadBalancer(service.JSONBase.ID, zone); err != nil { + if err := balancer.DeleteTCPLoadBalancer(service.JSONBase.ID, zone.Region); err != nil { return err } @@ -194,7 +194,7 @@ func (sr *ServiceRegistryStorage) Create(obj interface{}) (<-chan interface{}, e if err != nil { return nil, err } - err = balancer.CreateTCPLoadBalancer(srv.ID, zone, srv.Port, hosts) + err = balancer.CreateTCPLoadBalancer(srv.ID, zone.Region, srv.Port, hosts) if err != nil { return nil, err }