openstack: Implement Routes API

This change implements the Routes API using Neutron's "extraroute"
extension.

To use, this requires all the nodes to be on the same Neutron network
and the UUID of the Neutron router on that network.

Required cloud provider config section:
  [Route]
  router-id = <UUID of Neutron router>

Ensure kube-controllermanager is started with (non-default)
`--allocate-node-cidrs=true` and set `--cluster-cidr` to the POD
super-subnet (a private /16 would be reasonable).

Based on an earlier version by @timbyr (#19473)

pkg/cloudprovider/providers/openstack/BUILD

@@ -17,6 +17,7 @@ go_library(
         "openstack.go",
         "openstack_instances.go",
         "openstack_loadbalancer.go",
+        "openstack_routes.go",
         "openstack_volumes.go",
     ],
     tags = ["automanaged"],
@@ -30,6 +31,7 @@ go_library(
         "//pkg/util/mount:go_default_library",
         "//pkg/volume:go_default_library",
         "//vendor:github.com/golang/glog",
+        "//vendor:github.com/mitchellh/mapstructure",
         "//vendor:github.com/rackspace/gophercloud",
         "//vendor:github.com/rackspace/gophercloud/openstack",
         "//vendor:github.com/rackspace/gophercloud/openstack/blockstorage/v1/volumes",
@@ -40,6 +42,7 @@ go_library(
         "//vendor:github.com/rackspace/gophercloud/openstack/identity/v3/tokens",
         "//vendor:github.com/rackspace/gophercloud/openstack/networking/v2/extensions",
         "//vendor:github.com/rackspace/gophercloud/openstack/networking/v2/extensions/layer3/floatingips",
+        "//vendor:github.com/rackspace/gophercloud/openstack/networking/v2/extensions/layer3/routers",
         "//vendor:github.com/rackspace/gophercloud/openstack/networking/v2/extensions/lbaas/members",
         "//vendor:github.com/rackspace/gophercloud/openstack/networking/v2/extensions/lbaas/monitors",
         "//vendor:github.com/rackspace/gophercloud/openstack/networking/v2/extensions/lbaas/pools",
@@ -60,13 +63,17 @@ go_test(
     name = "go_default_test",
     srcs = [
         "metadata_test.go",
+        "openstack_routes_test.go",
         "openstack_test.go",
     ],
     library = "go_default_library",
     tags = ["automanaged"],
     deps = [
         "//pkg/api/v1:go_default_library",
+        "//pkg/cloudprovider:go_default_library",
+        "//pkg/types:go_default_library",
         "//pkg/util/rand:go_default_library",
         "//vendor:github.com/rackspace/gophercloud",
+        "//vendor:github.com/rackspace/gophercloud/openstack/compute/v2/servers",
     ],
 )

pkg/cloudprovider/providers/openstack/openstack.go

@@ -26,14 +26,14 @@ import (
 	"strings"
 	"time"
 
-	"gopkg.in/gcfg.v1"
-
+	"github.com/mitchellh/mapstructure"
 	"github.com/rackspace/gophercloud"
 	"github.com/rackspace/gophercloud/openstack"
 	"github.com/rackspace/gophercloud/openstack/compute/v2/servers"
 	"github.com/rackspace/gophercloud/openstack/identity/v3/extensions/trust"
 	token3 "github.com/rackspace/gophercloud/openstack/identity/v3/tokens"
 	"github.com/rackspace/gophercloud/pagination"
+	"gopkg.in/gcfg.v1"
 
 	"github.com/golang/glog"
 	"k8s.io/kubernetes/pkg/api/v1"
@@ -90,12 +90,17 @@ type BlockStorageOpts struct {
 	TrustDevicePath bool `gcfg:"trust-device-path"` // See Issue #33128
 }
 
+type RouterOpts struct {
+	RouterId string `gcfg:"router-id"` // required
+}
+
 // OpenStack is an implementation of cloud provider Interface for OpenStack.
 type OpenStack struct {
-	provider *gophercloud.ProviderClient
-	region   string
-	lbOpts   LoadBalancerOpts
-	bsOpts   BlockStorageOpts
+	provider  *gophercloud.ProviderClient
+	region    string
+	lbOpts    LoadBalancerOpts
+	bsOpts    BlockStorageOpts
+	routeOpts RouterOpts
 	// InstanceID of the server where this OpenStack object is instantiated.
 	localInstanceID string
 }
@@ -116,6 +121,7 @@ type Config struct {
 	}
 	LoadBalancer LoadBalancerOpts
 	BlockStorage BlockStorageOpts
+	Route        RouterOpts
 }
 
 func init() {
@@ -160,6 +166,18 @@ func readConfig(config io.Reader) (Config, error) {
 	return cfg, err
 }
 
+// Tiny helper for conditional unwind logic
+type Caller bool
+
+func NewCaller() Caller   { return Caller(true) }
+func (c *Caller) Disarm() { *c = false }
+
+func (c *Caller) Call(f func()) {
+	if *c {
+		f()
+	}
+}
+
 func readInstanceID() (string, error) {
 	// Try to find instance ID on the local filesystem (created by cloud-init)
 	const instanceIDFile = "/var/lib/cloud/data/instance-id"
@@ -211,6 +229,7 @@ func newOpenStack(cfg Config) (*OpenStack, error) {
 		region:          cfg.Global.Region,
 		lbOpts:          cfg.LoadBalancer,
 		bsOpts:          cfg.BlockStorage,
+		routeOpts:       cfg.Route,
 		localInstanceID: id,
 	}
 
@@ -225,7 +244,29 @@ func mapNodeNameToServerName(nodeName types.NodeName) string {
 
 // mapServerToNodeName maps an OpenStack Server to a k8s NodeName
 func mapServerToNodeName(server *servers.Server) types.NodeName {
-	return types.NodeName(server.Name)
+	// Node names are always lowercase, and (at least)
+	// routecontroller does case-sensitive string comparisons
+	// assuming this
+	return types.NodeName(strings.ToLower(server.Name))
+}
+
+func foreachServer(client *gophercloud.ServiceClient, opts servers.ListOptsBuilder, handler func(*servers.Server) (bool, error)) error {
+	pager := servers.List(client, opts)
+
+	err := pager.EachPage(func(page pagination.Page) (bool, error) {
+		s, err := servers.ExtractServers(page)
+		if err != nil {
+			return false, err
+		}
+		for _, server := range s {
+			ok, err := handler(&server)
+			if !ok || err != nil {
+				return false, err
+			}
+		}
+		return true, nil
+	})
+	return err
 }
 
 func getServerByName(client *gophercloud.ServiceClient, name types.NodeName) (*servers.Server, error) {
@@ -261,48 +302,33 @@ func getServerByName(client *gophercloud.ServiceClient, name types.NodeName) (*s
 	return &serverList[0], nil
 }
 
-func getAddressesByName(client *gophercloud.ServiceClient, name types.NodeName) ([]v1.NodeAddress, error) {
-	srv, err := getServerByName(client, name)
+func nodeAddresses(srv *servers.Server) ([]v1.NodeAddress, error) {
+	addrs := []v1.NodeAddress{}
+
+	type Address struct {
+		IpType string `mapstructure:"OS-EXT-IPS:type"`
+		Addr   string
+	}
+
+	var addresses map[string][]Address
+	err := mapstructure.Decode(srv.Addresses, &addresses)
 	if err != nil {
 		return nil, err
 	}
 
-	addrs := []v1.NodeAddress{}
-
-	for network, netblob := range srv.Addresses {
-		list, ok := netblob.([]interface{})
-		if !ok {
-			continue
-		}
-
-		for _, item := range list {
+	for network, addrlist := range addresses {
+		for _, props := range addrlist {
 			var addressType v1.NodeAddressType
-
-			props, ok := item.(map[string]interface{})
-			if !ok {
-				continue
-			}
-
-			extIPType, ok := props["OS-EXT-IPS:type"]
-			if (ok && extIPType == "floating") || (!ok && network == "public") {
+			if props.IpType == "floating" || network == "public" {
 				addressType = v1.NodeExternalIP
 			} else {
 				addressType = v1.NodeInternalIP
 			}
 
-			tmp, ok := props["addr"]
-			if !ok {
-				continue
-			}
-			addr, ok := tmp.(string)
-			if !ok {
-				continue
-			}
-
 			v1.AddToNodeAddresses(&addrs,
 				v1.NodeAddress{
 					Type:    addressType,
-					Address: addr,
+					Address: props.Addr,
 				},
 			)
 		}
@@ -330,6 +356,15 @@ func getAddressesByName(client *gophercloud.ServiceClient, name types.NodeName)
 	return addrs, nil
 }
 
+func getAddressesByName(client *gophercloud.ServiceClient, name types.NodeName) ([]v1.NodeAddress, error) {
+	srv, err := getServerByName(client, name)
+	if err != nil {
+		return nil, err
+	}
+
+	return nodeAddresses(srv)
+}
+
 func getAddressByName(client *gophercloud.ServiceClient, name types.NodeName) (string, error) {
 	addrs, err := getAddressesByName(client, name)
 	if err != nil {
@@ -369,7 +404,7 @@ func (os *OpenStack) LoadBalancer() (cloudprovider.LoadBalancer, bool) {
 		Region: os.region,
 	})
 	if err != nil {
-		glog.Warningf("Failed to find neutron endpoint: %v", err)
+		glog.Warningf("Failed to find network endpoint: %v", err)
 		return nil, false
 	}
 
@@ -439,5 +474,42 @@ func (os *OpenStack) GetZone() (cloudprovider.Zone, error) {
 }
 
 func (os *OpenStack) Routes() (cloudprovider.Routes, bool) {
-	return nil, false
+	glog.V(4).Info("openstack.Routes() called")
+
+	network, err := openstack.NewNetworkV2(os.provider, gophercloud.EndpointOpts{
+		Region: os.region,
+	})
+	if err != nil {
+		glog.Warningf("Failed to find network endpoint: %v", err)
+		return nil, false
+	}
+
+	netExts, err := networkExtensions(network)
+	if err != nil {
+		glog.Warningf("Failed to list neutron extensions: %v", err)
+		return nil, false
+	}
+
+	if !netExts["extraroute"] {
+		glog.V(3).Infof("Neutron extraroute extension not found, required for Routes support")
+		return nil, false
+	}
+
+	compute, err := openstack.NewComputeV2(os.provider, gophercloud.EndpointOpts{
+		Region: os.region,
+	})
+	if err != nil {
+		glog.Warningf("Failed to find compute endpoint: %v", err)
+		return nil, false
+	}
+
+	r, err := NewRoutes(compute, network, os.routeOpts)
+	if err != nil {
+		glog.Warningf("Error initialising Routes support: %v", err)
+		return nil, false
+	}
+
+	glog.V(1).Info("Claiming to support Routes")
+
+	return r, true
 }

pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go

@@ -108,14 +108,6 @@ func getPortByIP(client *gophercloud.ServiceClient, ipAddress string) (neutronpo
 	return targetPort, err
 }
 
-func getPortIDByIP(client *gophercloud.ServiceClient, ipAddress string) (string, error) {
-	targetPort, err := getPortByIP(client, ipAddress)
-	if err != nil {
-		return targetPort.ID, err
-	}
-	return targetPort.ID, nil
-}
-
 func getFloatingIPByPortID(client *gophercloud.ServiceClient, portID string) (*floatingips.FloatingIP, error) {
 	opts := floatingips.ListOpts{
 		PortID: portID,
@@ -1099,12 +1091,12 @@ func (lbaas *LbaasV2) EnsureLoadBalancerDeleted(clusterName string, service *v1.
 	}
 
 	if lbaas.opts.FloatingNetworkId != "" && loadbalancer != nil {
-		portID, err := getPortIDByIP(lbaas.network, loadbalancer.VipAddress)
+		port, err := getPortByIP(lbaas.network, loadbalancer.VipAddress)
 		if err != nil {
 			return err
 		}
 
-		floatingIP, err := getFloatingIPByPortID(lbaas.network, portID)
+		floatingIP, err := getFloatingIPByPortID(lbaas.network, port.ID)
 		if err != nil && err != ErrNotFound {
 			return err
 		}

pkg/cloudprovider/providers/openstack/openstack_routes.go

@@ -0,0 +1,278 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+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 openstack
+
+import (
+	"errors"
+
+	"github.com/rackspace/gophercloud"
+	"github.com/rackspace/gophercloud/openstack/compute/v2/servers"
+	"github.com/rackspace/gophercloud/openstack/networking/v2/extensions/layer3/routers"
+	neutronports "github.com/rackspace/gophercloud/openstack/networking/v2/ports"
+
+	"github.com/golang/glog"
+	"k8s.io/kubernetes/pkg/cloudprovider"
+	"k8s.io/kubernetes/pkg/types"
+)
+
+var ErrNoRouterId = errors.New("router-id not set in cloud provider config")
+
+type Routes struct {
+	compute *gophercloud.ServiceClient
+	network *gophercloud.ServiceClient
+	opts    RouterOpts
+}
+
+func NewRoutes(compute *gophercloud.ServiceClient, network *gophercloud.ServiceClient, opts RouterOpts) (cloudprovider.Routes, error) {
+	if opts.RouterId == "" {
+		return nil, ErrNoRouterId
+	}
+
+	return &Routes{
+		compute: compute,
+		network: network,
+		opts:    opts,
+	}, nil
+}
+
+func (r *Routes) ListRoutes(clusterName string) ([]*cloudprovider.Route, error) {
+	glog.V(4).Infof("ListRoutes(%v)", clusterName)
+
+	nodeNamesByAddr := make(map[string]types.NodeName)
+	err := foreachServer(r.compute, servers.ListOpts{Status: "ACTIVE"}, func(srv *servers.Server) (bool, error) {
+		addrs, err := nodeAddresses(srv)
+		if err != nil {
+			return false, err
+		}
+
+		name := mapServerToNodeName(srv)
+		for _, addr := range addrs {
+			nodeNamesByAddr[addr.Address] = name
+		}
+
+		return true, nil
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	router, err := routers.Get(r.network, r.opts.RouterId).Extract()
+	if err != nil {
+		return nil, err
+	}
+
+	var routes []*cloudprovider.Route
+	for _, item := range router.Routes {
+		nodeName, ok := nodeNamesByAddr[item.NextHop]
+		if !ok {
+			// Not one of our routes?
+			glog.V(4).Infof("Skipping route with unknown nexthop %v", item.NextHop)
+			continue
+		}
+		route := cloudprovider.Route{
+			Name:            item.DestinationCIDR,
+			TargetNode:      nodeName,
+			DestinationCIDR: item.DestinationCIDR,
+		}
+		routes = append(routes, &route)
+	}
+
+	return routes, nil
+}
+
+func updateRoutes(network *gophercloud.ServiceClient, router *routers.Router, newRoutes []routers.Route) (func(), error) {
+	origRoutes := router.Routes // shallow copy
+
+	_, err := routers.Update(network, router.ID, routers.UpdateOpts{
+		Routes: newRoutes,
+	}).Extract()
+	if err != nil {
+		return nil, err
+	}
+
+	unwinder := func() {
+		glog.V(4).Info("Reverting routes change to router ", router.ID)
+		_, err := routers.Update(network, router.ID, routers.UpdateOpts{
+			Routes: origRoutes,
+		}).Extract()
+		if err != nil {
+			glog.Warning("Unable to reset routes during error unwind: ", err)
+		}
+	}
+
+	return unwinder, nil
+}
+
+func updateAllowedAddressPairs(network *gophercloud.ServiceClient, port *neutronports.Port, newPairs []neutronports.AddressPair) (func(), error) {
+	origPairs := port.AllowedAddressPairs // shallow copy
+
+	_, err := neutronports.Update(network, port.ID, neutronports.UpdateOpts{
+		AllowedAddressPairs: newPairs,
+	}).Extract()
+	if err != nil {
+		return nil, err
+	}
+
+	unwinder := func() {
+		glog.V(4).Info("Reverting allowed-address-pairs change to port ", port.ID)
+		_, err := neutronports.Update(network, port.ID, neutronports.UpdateOpts{
+			AllowedAddressPairs: origPairs,
+		}).Extract()
+		if err != nil {
+			glog.Warning("Unable to reset allowed-address-pairs during error unwind: ", err)
+		}
+	}
+
+	return unwinder, nil
+}
+
+func (r *Routes) CreateRoute(clusterName string, nameHint string, route *cloudprovider.Route) error {
+	glog.V(4).Infof("CreateRoute(%v, %v, %v)", clusterName, nameHint, route)
+
+	onFailure := NewCaller()
+
+	addr, err := getAddressByName(r.compute, route.TargetNode)
+	if err != nil {
+		return err
+	}
+
+	glog.V(4).Infof("Using nexthop %v for node %v", addr, route.TargetNode)
+
+	router, err := routers.Get(r.network, r.opts.RouterId).Extract()
+	if err != nil {
+		return err
+	}
+
+	routes := router.Routes
+
+	for _, item := range routes {
+		if item.DestinationCIDR == route.DestinationCIDR && item.NextHop == addr {
+			glog.V(4).Infof("Skipping existing route: %v", route)
+			return nil
+		}
+	}
+
+	routes = append(routes, routers.Route{
+		DestinationCIDR: route.DestinationCIDR,
+		NextHop:         addr,
+	})
+
+	unwind, err := updateRoutes(r.network, router, routes)
+	if err != nil {
+		return err
+	}
+	defer onFailure.Call(unwind)
+
+	port, err := getPortByIP(r.network, addr)
+	if err != nil {
+		return err
+	}
+
+	found := false
+	for _, item := range port.AllowedAddressPairs {
+		if item.IPAddress == route.DestinationCIDR {
+			glog.V(4).Info("Found existing allowed-address-pair: ", item)
+			found = true
+			break
+		}
+	}
+
+	if !found {
+		newPairs := append(port.AllowedAddressPairs, neutronports.AddressPair{
+			IPAddress: route.DestinationCIDR,
+		})
+		unwind, err := updateAllowedAddressPairs(r.network, &port, newPairs)
+		if err != nil {
+			return err
+		}
+		defer onFailure.Call(unwind)
+	}
+
+	glog.V(4).Infof("Route created: %v", route)
+	onFailure.Disarm()
+	return nil
+}
+
+func (r *Routes) DeleteRoute(clusterName string, route *cloudprovider.Route) error {
+	glog.V(4).Infof("DeleteRoute(%v, %v)", clusterName, route)
+
+	onFailure := NewCaller()
+
+	addr, err := getAddressByName(r.compute, route.TargetNode)
+	if err != nil {
+		return err
+	}
+
+	router, err := routers.Get(r.network, r.opts.RouterId).Extract()
+	if err != nil {
+		return err
+	}
+
+	routes := router.Routes
+	index := -1
+	for i, item := range routes {
+		if item.DestinationCIDR == route.DestinationCIDR && item.NextHop == addr {
+			index = i
+			break
+		}
+	}
+
+	if index == -1 {
+		glog.V(4).Infof("Skipping non-existent route: %v", route)
+		return nil
+	}
+
+	// Delete element `index`
+	routes[index] = routes[len(routes)-1]
+	routes = routes[:len(routes)-1]
+
+	unwind, err := updateRoutes(r.network, router, routes)
+	if err != nil {
+		return err
+	}
+	defer onFailure.Call(unwind)
+
+	port, err := getPortByIP(r.network, addr)
+	if err != nil {
+		return err
+	}
+
+	addr_pairs := port.AllowedAddressPairs
+	index = -1
+	for i, item := range addr_pairs {
+		if item.IPAddress == route.DestinationCIDR {
+			index = i
+			break
+		}
+	}
+
+	if index != -1 {
+		// Delete element `index`
+		addr_pairs[index] = addr_pairs[len(routes)-1]
+		addr_pairs = addr_pairs[:len(routes)-1]
+
+		unwind, err := updateAllowedAddressPairs(r.network, &port, addr_pairs)
+		if err != nil {
+			return err
+		}
+		defer onFailure.Call(unwind)
+	}
+
+	glog.V(4).Infof("Route deleted: %v", route)
+	onFailure.Disarm()
+	return nil
+}

pkg/cloudprovider/providers/openstack/openstack_routes_test.go

@@ -0,0 +1,71 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+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 openstack
+
+import (
+	"net"
+	"testing"
+
+	"k8s.io/kubernetes/pkg/cloudprovider"
+	"k8s.io/kubernetes/pkg/types"
+)
+
+func TestRoutes(t *testing.T) {
+	const clusterName = "ignored"
+
+	cfg, ok := configFromEnv()
+	if !ok {
+		t.Skipf("No config found in environment")
+	}
+
+	os, err := newOpenStack(cfg)
+	if err != nil {
+		t.Fatalf("Failed to construct/authenticate OpenStack: %s", err)
+	}
+
+	r, ok := os.Routes()
+	if !ok {
+		t.Fatalf("Routes() returned false - perhaps your stack doens't support Neutron?")
+	}
+
+	newroute := cloudprovider.Route{
+		DestinationCIDR: "10.164.2.0/24",
+		TargetNode:      types.NodeName("testinstance"),
+	}
+	err = r.CreateRoute(clusterName, "myhint", &newroute)
+	if err != nil {
+		t.Fatalf("CreateRoute error: %v", err)
+	}
+
+	routelist, err := r.ListRoutes(clusterName)
+	if err != nil {
+		t.Fatalf("ListRoutes() error: %v", err)
+	}
+	for _, route := range routelist {
+		_, cidr, err := net.ParseCIDR(route.DestinationCIDR)
+		if err != nil {
+			t.Logf("Ignoring route %s, unparsable CIDR: %v", route.Name, err)
+			continue
+		}
+		t.Logf("%s via %s", cidr, route.TargetNode)
+	}
+
+	err = r.DeleteRoute(clusterName, &newroute)
+	if err != nil {
+		t.Fatalf("DeleteRoute error: %v", err)
+	}
+}

pkg/cloudprovider/providers/openstack/openstack_test.go

@@ -18,14 +18,17 @@ package openstack
 
 import (
 	"os"
+	"reflect"
+	"sort"
 	"strings"
 	"testing"
 	"time"
 
-	"k8s.io/kubernetes/pkg/util/rand"
-
 	"github.com/rackspace/gophercloud"
+	"github.com/rackspace/gophercloud/openstack/compute/v2/servers"
+
 	"k8s.io/kubernetes/pkg/api/v1"
+	"k8s.io/kubernetes/pkg/util/rand"
 )
 
 const volumeAvailableStatus = "available"
@@ -118,6 +121,115 @@ func TestToAuthOptions(t *testing.T) {
 	}
 }
 
+func TestCaller(t *testing.T) {
+	called := false
+	myFunc := func() { called = true }
+
+	c := NewCaller()
+	c.Call(myFunc)
+
+	if !called {
+		t.Errorf("Caller failed to call function in default case")
+	}
+
+	c.Disarm()
+	called = false
+	c.Call(myFunc)
+
+	if called {
+		t.Error("Caller still called function when disarmed")
+	}
+
+	// Confirm the "usual" deferred Caller pattern works as expected
+
+	called = false
+	success_case := func() {
+		c := NewCaller()
+		defer c.Call(func() { called = true })
+		c.Disarm()
+	}
+	if success_case(); called {
+		t.Error("Deferred success case still invoked unwind")
+	}
+
+	called = false
+	failure_case := func() {
+		c := NewCaller()
+		defer c.Call(func() { called = true })
+	}
+	if failure_case(); !called {
+		t.Error("Deferred failure case failed to invoke unwind")
+	}
+}
+
+// An arbitrary sort.Interface, just for easier comparison
+type AddressSlice []v1.NodeAddress
+
+func (a AddressSlice) Len() int           { return len(a) }
+func (a AddressSlice) Less(i, j int) bool { return a[i].Address < a[j].Address }
+func (a AddressSlice) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
+
+func TestNodeAddresses(t *testing.T) {
+	srv := servers.Server{
+		Status:     "ACTIVE",
+		HostID:     "29d3c8c896a45aa4c34e52247875d7fefc3d94bbcc9f622b5d204362",
+		AccessIPv4: "50.56.176.99",
+		AccessIPv6: "2001:4800:790e:510:be76:4eff:fe04:82a8",
+		Addresses: map[string]interface{}{
+			"private": []interface{}{
+				map[string]interface{}{
+					"OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:7c:1b:2b",
+					"version":                 float64(4),
+					"addr":                    "10.0.0.32",
+					"OS-EXT-IPS:type":         "fixed",
+				},
+				map[string]interface{}{
+					"version":         float64(4),
+					"addr":            "50.56.176.36",
+					"OS-EXT-IPS:type": "floating",
+				},
+				map[string]interface{}{
+					"version": float64(4),
+					"addr":    "10.0.0.31",
+					// No OS-EXT-IPS:type
+				},
+			},
+			"public": []interface{}{
+				map[string]interface{}{
+					"version": float64(4),
+					"addr":    "50.56.176.35",
+				},
+				map[string]interface{}{
+					"version": float64(6),
+					"addr":    "2001:4800:780e:510:be76:4eff:fe04:84a8",
+				},
+			},
+		},
+	}
+
+	addrs, err := nodeAddresses(&srv)
+	if err != nil {
+		t.Fatalf("nodeAddresses returned error: %v", err)
+	}
+
+	sort.Sort(AddressSlice(addrs))
+	t.Logf("addresses is %v", addrs)
+
+	want := []v1.NodeAddress{
+		{Type: v1.NodeInternalIP, Address: "10.0.0.31"},
+		{Type: v1.NodeInternalIP, Address: "10.0.0.32"},
+		{Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"},
+		{Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"},
+		{Type: v1.NodeExternalIP, Address: "50.56.176.35"},
+		{Type: v1.NodeExternalIP, Address: "50.56.176.36"},
+		{Type: v1.NodeExternalIP, Address: "50.56.176.99"},
+	}
+
+	if !reflect.DeepEqual(want, addrs) {
+		t.Errorf("nodeAddresses returned incorrect value %v", addrs)
+	}
+}
+
 // This allows acceptance testing against an existing OpenStack
 // install, using the standard OS_* OpenStack client environment
 // variables.