Merge pull request #5346 from justinsb/cloudprovider_multiple_addresses

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.

pkg/cloudprovider/aws/aws_test.go

@@ -156,7 +156,7 @@ func TestList(t *testing.T) {
 	}
 }
 
-func TestIPAddress(t *testing.T) {
+func TestNodeAddresses(t *testing.T) {
 	// Note these instances have the same name
 	// (we test that this produces an error)
 	instances := make([]ec2.Instance, 2)
@@ -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)
 	}
 }

pkg/cloudprovider/cloud.go

@@ -57,8 +57,8 @@ 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 returns the addresses of the specified instance.
+	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)

pkg/cloudprovider/controller/nodecontroller.go

@@ -274,12 +274,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)
+				node.Status.Addresses = nodeAddresses
 			}
 		}
 	} else {
@@ -288,7 +287,7 @@ func (s *NodeController) PopulateAddresses(nodes *api.NodeList) (*api.NodeList,
 			addr := net.ParseIP(node.Name)
 			if addr != nil {
 				address := api.NodeAddress{Type: api.NodeLegacyHostIP, Address: addr.String()}
-				api.AddToNodeAddresses(&node.Status.Addresses, address)
+				node.Status.Addresses = []api.NodeAddress{address}
 			} else {
 				addrs, err := s.lookupIP(node.Name)
 				if err != nil {
@@ -297,7 +296,7 @@ func (s *NodeController) PopulateAddresses(nodes *api.NodeList) (*api.NodeList,
 					glog.Errorf("No ip address for node %v", node.Name)
 				} else {
 					address := api.NodeAddress{Type: api.NodeLegacyHostIP, Address: addrs[0].String()}
-					api.AddToNodeAddresses(&node.Status.Addresses, address)
+					node.Status.Addresses = []api.NodeAddress{address}
 				}
 			}
 		}

pkg/cloudprovider/controller/nodecontroller_test.go

@@ -649,7 +649,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"},
 			},
@@ -1061,7 +1061,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{
 				{

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) {
-	f.addCall("ip-address")
-	return f.IP, f.Err
+// 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("node-addresses")
+	return f.Addresses, f.Err
 }
 
 // ExternalID is a test-spy implementation of Instances.ExternalID.

pkg/cloudprovider/gce/gce.go

@@ -315,8 +315,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
@@ -325,7 +325,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.

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.

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 {

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.

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.

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 {

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 NodeAddresses 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.

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")
 	}
 }