From bc16d83a51904a17cd6cc11254d2494e4ddf1c14 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Wed, 11 Mar 2015 16:37:11 -0700 Subject: [PATCH] Let CloudProvider return list of NodeAddress, not just one net.IP This lets cloud providers populate the NodeAddress array --- pkg/cloudprovider/aws/aws.go | 7 ++++--- pkg/cloudprovider/aws/aws_test.go | 11 +++++++---- pkg/cloudprovider/cloud.go | 2 +- pkg/cloudprovider/controller/nodecontroller.go | 7 +++---- pkg/cloudprovider/controller/nodecontroller_test.go | 4 ++-- pkg/cloudprovider/fake/fake.go | 10 +++++----- pkg/cloudprovider/gce/gce.go | 6 +++--- pkg/cloudprovider/openstack/openstack.go | 9 +++++---- pkg/cloudprovider/openstack/openstack_test.go | 6 +++--- pkg/cloudprovider/ovirt/ovirt.go | 6 +++--- pkg/cloudprovider/rackspace/rackspace.go | 9 +++++---- pkg/cloudprovider/rackspace/rackspace_test.go | 6 +++--- pkg/cloudprovider/vagrant/vagrant.go | 7 ++++--- pkg/cloudprovider/vagrant/vagrant_test.go | 10 ++++++---- 14 files changed, 54 insertions(+), 46 deletions(-) diff --git a/pkg/cloudprovider/aws/aws.go b/pkg/cloudprovider/aws/aws.go index 5a050083d34..efa5b5ef5ff 100644 --- a/pkg/cloudprovider/aws/aws.go +++ b/pkg/cloudprovider/aws/aws.go @@ -188,8 +188,8 @@ func (aws *AWSCloud) Zones() (cloudprovider.Zones, bool) { return aws, true } -// IPAddress is an implementation of Instances.IPAddress. -func (aws *AWSCloud) IPAddress(name string) (net.IP, error) { +// NodeAddresses is an implementation of Instances.NodeAddresses. +func (aws *AWSCloud) NodeAddresses(name string) ([]api.NodeAddress, error) { inst, err := aws.getInstancesByDnsName(name) if err != nil { return nil, err @@ -198,7 +198,8 @@ func (aws *AWSCloud) IPAddress(name string) (net.IP, error) { if ip == nil { return nil, fmt.Errorf("invalid network IP: %s", inst.PrivateIpAddress) } - return ip, nil + + return []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: ip.String()}}, nil } // ExternalID returns the cloud provider ID of the specified instance. diff --git a/pkg/cloudprovider/aws/aws_test.go b/pkg/cloudprovider/aws/aws_test.go index b5bd78e6ed8..e24ebdc1136 100644 --- a/pkg/cloudprovider/aws/aws_test.go +++ b/pkg/cloudprovider/aws/aws_test.go @@ -168,23 +168,26 @@ func TestIPAddress(t *testing.T) { instances[1].State.Name = "running" aws1 := mockInstancesResp([]ec2.Instance{}) - _, err1 := aws1.IPAddress("instance") + _, err1 := aws1.NodeAddresses("instance") if err1 == nil { t.Errorf("Should error when no instance found") } aws2 := mockInstancesResp(instances) - _, err2 := aws2.IPAddress("instance1") + _, err2 := aws2.NodeAddresses("instance1") if err2 == nil { t.Errorf("Should error when multiple instances found") } aws3 := mockInstancesResp(instances[0:1]) - ip3, err3 := aws3.IPAddress("instance1") + addrs3, err3 := aws3.NodeAddresses("instance1") if err3 != nil { t.Errorf("Should not error when instance found") } - if e, a := instances[0].PrivateIpAddress, ip3.String(); e != a { + if len(addrs3) != 1 { + t.Errorf("Should return exactly one NodeAddress") + } + if e, a := instances[0].PrivateIpAddress, addrs3[0].Address; e != a { t.Errorf("Expected %v, got %v", e, a) } } diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index 7960c00c57e..03132aabb41 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -58,7 +58,7 @@ type TCPLoadBalancer interface { // Instances is an abstract, pluggable interface for sets of instances. type Instances interface { // IPAddress returns an IP address of the specified instance. - IPAddress(name string) (net.IP, error) + NodeAddresses(name string) ([]api.NodeAddress, error) // ExternalID returns the cloud provider ID of the specified instance. ExternalID(name string) (string, error) // List lists instances that match 'filter' which is a regular expression which must match the entire instance name (fqdn) diff --git a/pkg/cloudprovider/controller/nodecontroller.go b/pkg/cloudprovider/controller/nodecontroller.go index f5d4fcf2ace..9a11a1bba81 100644 --- a/pkg/cloudprovider/controller/nodecontroller.go +++ b/pkg/cloudprovider/controller/nodecontroller.go @@ -270,12 +270,11 @@ func (s *NodeController) PopulateAddresses(nodes *api.NodeList) (*api.NodeList, } for i := range nodes.Items { node := &nodes.Items[i] - hostIP, err := instances.IPAddress(node.Name) + nodeAddresses, err := instances.NodeAddresses(node.Name) if err != nil { - glog.Errorf("error getting instance ip address for %s: %v", node.Name, err) + glog.Errorf("error getting instance addresses for %s: %v", node.Name, err) } else { - address := api.NodeAddress{Type: api.NodeLegacyHostIP, Address: hostIP.String()} - api.AddToNodeAddresses(&node.Status.Addresses, address) + api.AddToNodeAddresses(&node.Status.Addresses, nodeAddresses...) } } } else { diff --git a/pkg/cloudprovider/controller/nodecontroller_test.go b/pkg/cloudprovider/controller/nodecontroller_test.go index 05236538810..5a8f8c473e2 100644 --- a/pkg/cloudprovider/controller/nodecontroller_test.go +++ b/pkg/cloudprovider/controller/nodecontroller_test.go @@ -628,7 +628,7 @@ func TestPopulateNodeAddresses(t *testing.T) { }{ { nodes: &api.NodeList{Items: []api.Node{*newNode("node0"), *newNode("node1")}}, - fakeCloud: &fake_cloud.FakeCloud{IP: net.ParseIP("1.2.3.4")}, + fakeCloud: &fake_cloud.FakeCloud{Addresses: []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: "1.2.3.4"}}}, expectedAddresses: []api.NodeAddress{ {Type: api.NodeLegacyHostIP, Address: "1.2.3.4"}, }, @@ -1024,7 +1024,7 @@ func TestSyncNodeStatus(t *testing.T) { Err: nil, }, fakeCloud: &fake_cloud.FakeCloud{ - IP: net.ParseIP("1.2.3.4"), + Addresses: []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: "1.2.3.4"}}, }, expectedNodes: []*api.Node{ { diff --git a/pkg/cloudprovider/fake/fake.go b/pkg/cloudprovider/fake/fake.go index 84c8d9e9ba0..e773041e24b 100644 --- a/pkg/cloudprovider/fake/fake.go +++ b/pkg/cloudprovider/fake/fake.go @@ -38,7 +38,7 @@ type FakeCloud struct { Exists bool Err error Calls []string - IP net.IP + Addresses []api.NodeAddress ExtID map[string]string Machines []string NodeResources *api.NodeResources @@ -115,11 +115,11 @@ func (f *FakeCloud) DeleteTCPLoadBalancer(name, region string) error { return f.Err } -// IPAddress is a test-spy implementation of Instances.IPAddress. -// It adds an entry "ip-address" into the internal method call record. -func (f *FakeCloud) IPAddress(instance string) (net.IP, error) { +// NodeAddresses is a test-spy implementation of Instances.NodeAddresses. +// It adds an entry "node-addresses" into the internal method call record. +func (f *FakeCloud) NodeAddresses(instance string) ([]api.NodeAddress, error) { f.addCall("ip-address") - return f.IP, f.Err + return f.Addresses, f.Err } // ExternalID is a test-spy implementation of Instances.ExternalID. diff --git a/pkg/cloudprovider/gce/gce.go b/pkg/cloudprovider/gce/gce.go index f1e17dfa1d3..31ad7ea9b06 100644 --- a/pkg/cloudprovider/gce/gce.go +++ b/pkg/cloudprovider/gce/gce.go @@ -314,8 +314,8 @@ func (gce *GCECloud) getInstanceByName(name string) (*compute.Instance, error) { return res, nil } -// IPAddress is an implementation of Instances.IPAddress. -func (gce *GCECloud) IPAddress(instance string) (net.IP, error) { +// NodeAddresses is an implementation of Instances.NodeAddresses. +func (gce *GCECloud) NodeAddresses(instance string) ([]api.NodeAddress, error) { inst, err := gce.getInstanceByName(instance) if err != nil { return nil, err @@ -324,7 +324,7 @@ func (gce *GCECloud) IPAddress(instance string) (net.IP, error) { if ip == nil { return nil, fmt.Errorf("invalid network IP: %s", inst.NetworkInterfaces[0].AccessConfigs[0].NatIP) } - return ip, nil + return []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: ip.String()}}, nil } // ExternalID returns the cloud provider ID of the specified instance. diff --git a/pkg/cloudprovider/openstack/openstack.go b/pkg/cloudprovider/openstack/openstack.go index 409495c75d6..fbae5873d3b 100644 --- a/pkg/cloudprovider/openstack/openstack.go +++ b/pkg/cloudprovider/openstack/openstack.go @@ -299,17 +299,18 @@ func getAddressByName(api *gophercloud.ServiceClient, name string) (string, erro return s, nil } -func (i *Instances) IPAddress(name string) (net.IP, error) { - glog.V(2).Infof("IPAddress(%v) called", name) +func (i *Instances) NodeAddresses(name string) ([]api.NodeAddress, error) { + glog.V(2).Infof("NodeAddresses(%v) called", name) ip, err := getAddressByName(i.compute, name) if err != nil { return nil, err } - glog.V(2).Infof("IPAddress(%v) => %v", name, ip) + glog.V(2).Infof("NodeAddresses(%v) => %v", name, ip) - return net.ParseIP(ip), err + // net.ParseIP().String() is to maintain compatibility with the old code + return []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: net.ParseIP(ip).String()}}, nil } // ExternalID returns the cloud provider ID of the specified instance. diff --git a/pkg/cloudprovider/openstack/openstack_test.go b/pkg/cloudprovider/openstack/openstack_test.go index 463d7e20d47..8b3c0850841 100644 --- a/pkg/cloudprovider/openstack/openstack_test.go +++ b/pkg/cloudprovider/openstack/openstack_test.go @@ -144,11 +144,11 @@ func TestInstances(t *testing.T) { } t.Logf("Found servers (%d): %s\n", len(srvs), srvs) - ip, err := i.IPAddress(srvs[0]) + addrs, err := i.NodeAddresses(srvs[0]) if err != nil { - t.Fatalf("Instances.IPAddress(%s) failed: %s", srvs[0], err) + t.Fatalf("Instances.NodeAddresses(%s) failed: %s", srvs[0], err) } - t.Logf("Found IPAddress(%s) = %s\n", srvs[0], ip) + t.Logf("Found NodeAddresses(%s) = %s\n", srvs[0], addrs) rsrcs, err := i.GetNodeResources(srvs[0]) if err != nil { diff --git a/pkg/cloudprovider/ovirt/ovirt.go b/pkg/cloudprovider/ovirt/ovirt.go index c9cdae6e665..f6601017d2f 100644 --- a/pkg/cloudprovider/ovirt/ovirt.go +++ b/pkg/cloudprovider/ovirt/ovirt.go @@ -130,8 +130,8 @@ func (v *OVirtCloud) Zones() (cloudprovider.Zones, bool) { return nil, false } -// IPAddress returns the address of a particular machine instance -func (v *OVirtCloud) IPAddress(name string) (net.IP, error) { +// NodeAddresses returns the NodeAddresses of a particular machine instance +func (v *OVirtCloud) NodeAddresses(name string) ([]api.NodeAddress, error) { instance, err := v.fetchInstance(name) if err != nil { return nil, err @@ -152,7 +152,7 @@ func (v *OVirtCloud) IPAddress(name string) (net.IP, error) { address = resolved[0] } - return address, nil + return []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: address.String()}}, nil } // ExternalID returns the cloud provider ID of the specified instance. diff --git a/pkg/cloudprovider/rackspace/rackspace.go b/pkg/cloudprovider/rackspace/rackspace.go index 95d3ad67e02..3871776e1d6 100644 --- a/pkg/cloudprovider/rackspace/rackspace.go +++ b/pkg/cloudprovider/rackspace/rackspace.go @@ -350,17 +350,18 @@ func getAddressByName(api *gophercloud.ServiceClient, name string) (string, erro return s, nil } -func (i *Instances) IPAddress(name string) (net.IP, error) { - glog.V(2).Infof("IPAddress(%v) called", name) +func (i *Instances) NodeAddresses(name string) ([]api.NodeAddress, error) { + glog.V(2).Infof("NodeAddresses(%v) called", name) ip, err := getAddressByName(i.compute, name) if err != nil { return nil, err } - glog.V(2).Infof("IPAddress(%v) => %v", name, ip) + glog.V(2).Infof("NodeAddresses(%v) => %v", name, ip) - return net.ParseIP(ip), err + // net.ParseIP().String() is to maintain compatibility with the old code + return []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: net.ParseIP(ip).String()}}, nil } // ExternalID returns the cloud provider ID of the specified instance. diff --git a/pkg/cloudprovider/rackspace/rackspace_test.go b/pkg/cloudprovider/rackspace/rackspace_test.go index f72d7cfb449..48b1ec4bfe7 100644 --- a/pkg/cloudprovider/rackspace/rackspace_test.go +++ b/pkg/cloudprovider/rackspace/rackspace_test.go @@ -144,11 +144,11 @@ func TestInstances(t *testing.T) { } t.Logf("Found servers (%d): %s\n", len(srvs), srvs) - ip, err := i.IPAddress(srvs[0]) + addrs, err := i.NodeAddresses(srvs[0]) if err != nil { - t.Fatalf("Instances.IPAddress(%s) failed: %s", srvs[0], err) + t.Fatalf("Instances.NodeAddresses(%s) failed: %s", srvs[0], err) } - t.Logf("Found IPAddress(%s) = %s\n", srvs[0], ip) + t.Logf("Found NodeAddresses(%s) = %s\n", srvs[0], addrs) rsrcs, err := i.GetNodeResources(srvs[0]) if err != nil { diff --git a/pkg/cloudprovider/vagrant/vagrant.go b/pkg/cloudprovider/vagrant/vagrant.go index adb11e498ff..ecd3094b7d0 100644 --- a/pkg/cloudprovider/vagrant/vagrant.go +++ b/pkg/cloudprovider/vagrant/vagrant.go @@ -119,14 +119,15 @@ func (v *VagrantCloud) getInstanceByAddress(address string) (*SaltMinion, error) return nil, fmt.Errorf("unable to find instance for address: %s", address) } -// IPAddress returns the address of a particular machine instance. -func (v *VagrantCloud) IPAddress(instance string) (net.IP, error) { +// NodeAddresses returns the NodeAddress of a particular machine instance. +func (v *VagrantCloud) NodeAddresses(instance string) ([]api.NodeAddress, error) { // Due to vagrant not running with a dedicated DNS setup, we return the IP address of a minion as its hostname at this time minion, err := v.getInstanceByAddress(instance) if err != nil { return nil, err } - return net.ParseIP(minion.IP), nil + ip := net.ParseIP(minion.IP) + return []api.NodeAddress{{Type: api.NodeLegacyHostIP, Address: ip.String()}}, nil } // ExternalID returns the cloud provider ID of the specified instance. diff --git a/pkg/cloudprovider/vagrant/vagrant_test.go b/pkg/cloudprovider/vagrant/vagrant_test.go index 3ed9f0037c2..8012f7db09a 100644 --- a/pkg/cloudprovider/vagrant/vagrant_test.go +++ b/pkg/cloudprovider/vagrant/vagrant_test.go @@ -81,12 +81,14 @@ func TestVagrantCloud(t *testing.T) { t.Fatalf("Invalid instance returned") } - ip, err := vagrantCloud.IPAddress(instances[0]) + addrs, err := vagrantCloud.NodeAddresses(instances[0]) if err != nil { - t.Fatalf("Unexpected error, should have returned a valid IP address: %s", err) + t.Fatalf("Unexpected error, should have returned valid NodeAddresses: %s", err) } - - if ip.String() != expectedInstanceIP { + if len(addrs) != 1 { + t.Fatalf("should have returned exactly one NodeAddress: %v", addrs) + } + if addrs[0].Address != expectedInstanceIP { t.Fatalf("Invalid IP address returned") } }