From 57869958bc2a2f1c3166bf9c38992b2f1f6445d1 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Tue, 17 Jun 2014 21:58:41 -0700 Subject: [PATCH 1/2] Add IP look up if the Cloud Provider is not null Add Instance info to the Cloud Provider interface --- pkg/cloudprovider/cloud.go | 10 ++++++++++ pkg/cloudprovider/gce.go | 17 +++++++++++++++++ pkg/registry/pod_registry.go | 35 ++++++++++++++++++++++++++++++++++- 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index 1cf38f49ebc..ca45e96a85a 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -16,10 +16,16 @@ limitations under the License. package cloudprovider +import ( + "net" +) + // CloudInterface is an abstract, pluggable interface for cloud providers type Interface interface { // TCPLoadBalancer returns a balancer interface, or nil if none is supported. Returns an error if one occurs. TCPLoadBalancer() (TCPLoadBalancer, error) + // Instances returns an instances interface, or nil if none is supported. Returns an error if one occurs. + Instances() (Instances, error) } type TCPLoadBalancer interface { @@ -29,3 +35,7 @@ type TCPLoadBalancer interface { UpdateTCPLoadBalancer(name, region string, hosts []string) error DeleteTCPLoadBalancer(name, region string) error } + +type Instances interface { + IPAddress(name string) (net.IP, error) +} diff --git a/pkg/cloudprovider/gce.go b/pkg/cloudprovider/gce.go index ecc4e9c24c0..78e1bbc9379 100644 --- a/pkg/cloudprovider/gce.go +++ b/pkg/cloudprovider/gce.go @@ -19,6 +19,7 @@ package cloudprovider import ( "fmt" "io/ioutil" + "net" "net/http" "strconv" "strings" @@ -82,6 +83,10 @@ func (gce *GCECloud) TCPLoadBalancer() (TCPLoadBalancer, error) { return gce, nil } +func (gce *GCECloud) Instances() (Instances, error) { + return gce, nil +} + func makeHostLink(projectID, zone, host string) string { ix := strings.Index(host, ".") if ix != -1 { @@ -162,3 +167,15 @@ func (gce *GCECloud) DeleteTCPLoadBalancer(name, region string) error { _, err = gce.service.TargetPools.Delete(gce.projectID, region, name).Do() return err } + +func (gce *GCECloud) IPAddress(instance string) (net.IP, error) { + res, err := gce.service.Instances.Get(gce.projectID, gce.zone, instance).Do() + if err != nil { + return nil, err + } + ip := net.ParseIP(res.NetworkInterfaces[0].AccessConfigs[0].NatIP) + if ip == nil { + return nil, fmt.Errorf("Invalid network IP: %s", res.NetworkInterfaces[0].AccessConfigs[0].NatIP) + } + return ip, nil +} diff --git a/pkg/registry/pod_registry.go b/pkg/registry/pod_registry.go index c57a7c7f725..2b7f3fede3b 100644 --- a/pkg/registry/pod_registry.go +++ b/pkg/registry/pod_registry.go @@ -18,10 +18,14 @@ package registry import ( "encoding/json" "fmt" + "log" + "net" + "strings" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" + "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) @@ -30,13 +34,15 @@ type PodRegistryStorage struct { registry PodRegistry containerInfo client.ContainerInfo scheduler Scheduler + cloud cloudprovider.Interface } -func MakePodRegistryStorage(registry PodRegistry, containerInfo client.ContainerInfo, scheduler Scheduler) apiserver.RESTStorage { +func MakePodRegistryStorage(registry PodRegistry, containerInfo client.ContainerInfo, scheduler Scheduler, cloud cloudprovider.Interface) apiserver.RESTStorage { return &PodRegistryStorage{ registry: registry, containerInfo: containerInfo, scheduler: scheduler, + cloud: cloud, } } @@ -63,6 +69,31 @@ func makePodStatus(info interface{}) string { return "Pending" } +func getInstanceIP(cloud cloudprovider.Interface, host string) string { + if cloud == nil { + return "" + } + instances, err := cloud.Instances() + if instances == nil { + return "" + } + if err != nil { + log.Printf("Error getting instances: %#v", err) + return "" + } + ix := strings.Index(host, ".") + if ix != -1 { + host = host[:ix] + } + var addr net.IP + addr, err = instances.IPAddress(host) + if err != nil { + log.Printf("Error getting instance IP: %#v", err) + return "" + } + return addr.String() +} + func (storage *PodRegistryStorage) Get(id string) (interface{}, error) { pod, err := storage.registry.GetPod(id) if err != nil { @@ -74,6 +105,8 @@ func (storage *PodRegistryStorage) Get(id string) (interface{}, error) { } pod.CurrentState.Info = info pod.CurrentState.Status = makePodStatus(info) + pod.CurrentState.HostIP = getInstanceIP(storage.cloud, pod.CurrentState.Host) + pod.Kind = "cluster#pod" return pod, err } From 420b2fdd57924b0249d2d433d0d102f38db59644 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Tue, 17 Jun 2014 22:28:44 -0700 Subject: [PATCH 2/2] Add support for populating host ip address. --- cmd/integration/integration.go | 2 +- pkg/cloudprovider/cloud.go | 8 +++--- pkg/cloudprovider/fake_cloud.go | 8 ++++-- pkg/cloudprovider/gce.go | 8 +++--- pkg/master/master.go | 2 +- pkg/registry/pod_registry.go | 27 ++++++++++--------- pkg/registry/pod_registry_test.go | 43 ++++++++++++++++++++++++++++++- pkg/registry/service_registry.go | 17 +++++------- 8 files changed, 77 insertions(+), 38 deletions(-) diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go index 62661145779..8a621dfd78a 100644 --- a/cmd/integration/integration.go +++ b/cmd/integration/integration.go @@ -44,7 +44,7 @@ func main() { reg := registry.MakeEtcdRegistry(etcdClient, machineList) apiserver := apiserver.New(map[string]apiserver.RESTStorage{ - "pods": registry.MakePodRegistryStorage(reg, &client.FakeContainerInfo{}, registry.MakeRoundRobinScheduler(machineList)), + "pods": registry.MakePodRegistryStorage(reg, &client.FakeContainerInfo{}, registry.MakeRoundRobinScheduler(machineList), nil), "replicationControllers": registry.MakeControllerRegistryStorage(reg), }, "/api/v1beta1") server := httptest.NewServer(apiserver) diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index ca45e96a85a..7e516215894 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -22,10 +22,10 @@ import ( // CloudInterface is an abstract, pluggable interface for cloud providers type Interface interface { - // TCPLoadBalancer returns a balancer interface, or nil if none is supported. Returns an error if one occurs. - TCPLoadBalancer() (TCPLoadBalancer, error) - // Instances returns an instances interface, or nil if none is supported. Returns an error if one occurs. - Instances() (Instances, error) + // TCPLoadBalancer returns a balancer interface. Also returns true if the interface is supported, false otherwise. + TCPLoadBalancer() (TCPLoadBalancer, bool) + // Instances returns an instances interface. Also returns true if the interface is supported, false otherwise. + Instances() (Instances, bool) } type TCPLoadBalancer interface { diff --git a/pkg/cloudprovider/fake_cloud.go b/pkg/cloudprovider/fake_cloud.go index b044631e859..e2b06f3aef6 100644 --- a/pkg/cloudprovider/fake_cloud.go +++ b/pkg/cloudprovider/fake_cloud.go @@ -35,8 +35,12 @@ func (f *FakeCloud) ClearCalls() { f.Calls = []string{} } -func (f *FakeCloud) TCPLoadBalancer() (TCPLoadBalancer, error) { - return f, nil +func (f *FakeCloud) TCPLoadBalancer() (TCPLoadBalancer, bool) { + return f, true +} + +func (f *FakeCloud) Instances() (Instances, bool) { + return f, true } func (f *FakeCloud) TCPLoadBalancerExists(name, region string) (bool, error) { diff --git a/pkg/cloudprovider/gce.go b/pkg/cloudprovider/gce.go index 78e1bbc9379..1b1667737d0 100644 --- a/pkg/cloudprovider/gce.go +++ b/pkg/cloudprovider/gce.go @@ -79,12 +79,12 @@ func NewGCECloud() (*GCECloud, error) { }, nil } -func (gce *GCECloud) TCPLoadBalancer() (TCPLoadBalancer, error) { - return gce, nil +func (gce *GCECloud) TCPLoadBalancer() (TCPLoadBalancer, bool) { + return gce, true } -func (gce *GCECloud) Instances() (Instances, error) { - return gce, nil +func (gce *GCECloud) Instances() (Instances, bool) { + return gce, true } func makeHostLink(projectID, zone, host string) string { diff --git a/pkg/master/master.go b/pkg/master/master.go index a14021e2c47..701836ebee8 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -72,7 +72,7 @@ func (m *Master) init(minions []string, cloud cloudprovider.Interface) { m.minions = minions m.random = rand.New(rand.NewSource(int64(time.Now().Nanosecond()))) m.storage = map[string]apiserver.RESTStorage{ - "pods": registry.MakePodRegistryStorage(m.podRegistry, containerInfo, registry.MakeFirstFitScheduler(m.minions, m.podRegistry, m.random)), + "pods": registry.MakePodRegistryStorage(m.podRegistry, containerInfo, registry.MakeFirstFitScheduler(m.minions, m.podRegistry, m.random), cloud), "replicationControllers": registry.MakeControllerRegistryStorage(m.controllerRegistry), "services": registry.MakeServiceRegistryStorage(m.serviceRegistry, cloud, m.minions), } diff --git a/pkg/registry/pod_registry.go b/pkg/registry/pod_registry.go index 2b7f3fede3b..272238c6894 100644 --- a/pkg/registry/pod_registry.go +++ b/pkg/registry/pod_registry.go @@ -19,7 +19,6 @@ import ( "encoding/json" "fmt" "log" - "net" "strings" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -73,20 +72,15 @@ func getInstanceIP(cloud cloudprovider.Interface, host string) string { if cloud == nil { return "" } - instances, err := cloud.Instances() - if instances == nil { - return "" - } - if err != nil { - log.Printf("Error getting instances: %#v", err) + instances, ok := cloud.Instances() + if instances == nil || !ok { return "" } ix := strings.Index(host, ".") if ix != -1 { host = host[:ix] } - var addr net.IP - addr, err = instances.IPAddress(host) + addr, err := instances.IPAddress(host) if err != nil { log.Printf("Error getting instance IP: %#v", err) return "" @@ -99,12 +93,17 @@ func (storage *PodRegistryStorage) Get(id string) (interface{}, error) { if err != nil { return pod, err } - info, err := storage.containerInfo.GetContainerInfo(pod.CurrentState.Host, id) - if err != nil { - return pod, err + if pod == nil { + return pod, nil + } + if storage.containerInfo != nil { + info, err := storage.containerInfo.GetContainerInfo(pod.CurrentState.Host, id) + if err != nil { + return pod, err + } + pod.CurrentState.Info = info + pod.CurrentState.Status = makePodStatus(info) } - pod.CurrentState.Info = info - pod.CurrentState.Status = makePodStatus(info) pod.CurrentState.HostIP = getInstanceIP(storage.cloud, pod.CurrentState.Host) pod.Kind = "cluster#pod" diff --git a/pkg/registry/pod_registry_test.go b/pkg/registry/pod_registry_test.go index ab3223ef938..4c29a322d8d 100644 --- a/pkg/registry/pod_registry_test.go +++ b/pkg/registry/pod_registry_test.go @@ -22,11 +22,13 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) type MockPodRegistry struct { err error + pod *api.Pod pods []api.Pod } @@ -50,7 +52,7 @@ func (registry *MockPodRegistry) ListPods(query labels.Query) ([]api.Pod, error) } func (registry *MockPodRegistry) GetPod(podId string) (*api.Pod, error) { - return &api.Pod{}, registry.err + return registry.pod, registry.err } func (registry *MockPodRegistry) CreatePod(machine string, pod api.Pod) error { @@ -145,6 +147,45 @@ func TestExtractJson(t *testing.T) { } } +func TestGetPod(t *testing.T) { + mockRegistry := MockPodRegistry{ + pod: &api.Pod{ + JSONBase: api.JSONBase{ID: "foo"}, + }, + } + storage := PodRegistryStorage{ + registry: &mockRegistry, + } + obj, err := storage.Get("foo") + pod := obj.(*api.Pod) + expectNoError(t, err) + if !reflect.DeepEqual(*mockRegistry.pod, *pod) { + t.Errorf("Unexpected pod. Expected %#v, Got %#v", *mockRegistry.pod, *pod) + } +} + +func TestGetPodCloud(t *testing.T) { + fakeCloud := &cloudprovider.FakeCloud{} + mockRegistry := MockPodRegistry{ + pod: &api.Pod{ + JSONBase: api.JSONBase{ID: "foo"}, + }, + } + storage := PodRegistryStorage{ + registry: &mockRegistry, + cloud: fakeCloud, + } + obj, err := storage.Get("foo") + pod := obj.(*api.Pod) + expectNoError(t, err) + if !reflect.DeepEqual(*mockRegistry.pod, *pod) { + t.Errorf("Unexpected pod. Expected %#v, Got %#v", *mockRegistry.pod, *pod) + } + if len(fakeCloud.Calls) != 1 || fakeCloud.Calls[0] != "ip-address" { + t.Errorf("Unexpected calls: %#v", fakeCloud.Calls) + } +} + func TestMakePodStatus(t *testing.T) { status := makePodStatus(map[string]interface{}{}) if status != "Pending" { diff --git a/pkg/registry/service_registry.go b/pkg/registry/service_registry.go index f50b63814c7..128e3b571b5 100644 --- a/pkg/registry/service_registry.go +++ b/pkg/registry/service_registry.go @@ -91,13 +91,11 @@ func (sr *ServiceRegistryStorage) Delete(id string) error { } if svc.(*api.Service).CreateExternalLoadBalancer { var balancer cloudprovider.TCPLoadBalancer + var ok bool if sr.cloud != nil { - balancer, err = sr.cloud.TCPLoadBalancer() - if err != nil { - return err - } + balancer, ok = sr.cloud.TCPLoadBalancer() } - if balancer != nil { + if ok && balancer != nil { err = balancer.DeleteTCPLoadBalancer(id, "us-central1") if err != nil { return err @@ -118,14 +116,11 @@ func (sr *ServiceRegistryStorage) Create(obj interface{}) error { srv := obj.(api.Service) if srv.CreateExternalLoadBalancer { var balancer cloudprovider.TCPLoadBalancer + var ok bool if sr.cloud != nil { - var err error - balancer, err = sr.cloud.TCPLoadBalancer() - if err != nil { - return err - } + balancer, ok = sr.cloud.TCPLoadBalancer() } - if balancer != nil { + if ok && balancer != nil { err := balancer.CreateTCPLoadBalancer(srv.ID, "us-central1", srv.Port, sr.hosts) if err != nil { return err