From 27332be0772f204474ae4a7fb193a618afdd8777 Mon Sep 17 00:00:00 2001 From: kayrus Date: Thu, 19 Dec 2019 19:01:47 +0100 Subject: [PATCH 1/2] CCM: set predicted and proper server ip addresses order --- go.mod | 1 - .../providers/openstack/openstack.go | 86 ++++++++++++------- .../openstack/openstack_instances.go | 7 +- .../providers/openstack/openstack_routes.go | 7 +- .../providers/openstack/openstack_test.go | 62 ++++++++++--- 5 files changed, 119 insertions(+), 44 deletions(-) diff --git a/go.mod b/go.mod index fa4b006bc4..71cba7dd7f 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,6 @@ require ( github.com/coreos/go-systemd v0.0.0-20190620071333-e64a0ec8b42a // indirect github.com/emicklei/go-restful v2.9.6+incompatible // indirect github.com/evanphx/json-patch v4.5.0+incompatible // indirect - github.com/go-logfmt/logfmt v0.4.0 // indirect github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6 // indirect github.com/golang/protobuf v1.3.2 github.com/google/go-cmp v0.3.1 // indirect diff --git a/pkg/cloudprovider/providers/openstack/openstack.go b/pkg/cloudprovider/providers/openstack/openstack.go index 525e7f5ae7..0cbd22ccb8 100644 --- a/pkg/cloudprovider/providers/openstack/openstack.go +++ b/pkg/cloudprovider/providers/openstack/openstack.go @@ -671,9 +671,58 @@ func getServerByName(client *gophercloud.ServiceClient, name types.NodeName) (*S return &serverList[0], nil } -func nodeAddresses(srv *servers.Server, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) { +// IP addresses order: +// * interfaces private IPs +// * access IPs +// * metadata hostname +// * server object Addresses +func nodeAddresses(srv *servers.Server, interfaces []attachinterfaces.Interface, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) { addrs := []v1.NodeAddress{} + // parse private IP addresses first in an ordered manner + for _, iface := range interfaces { + for _, fixedIP := range iface.FixedIPs { + isIPv6 := net.ParseIP(fixedIP.IPAddress).To4() == nil + if !(isIPv6 && networkingOpts.IPv6SupportDisabled) { + v1helper.AddToNodeAddresses(&addrs, + v1.NodeAddress{ + Type: v1.NodeInternalIP, + Address: fixedIP.IPAddress, + }, + ) + } + } + } + + // process public IP addresses + if srv.AccessIPv4 != "" { + v1helper.AddToNodeAddresses(&addrs, + v1.NodeAddress{ + Type: v1.NodeExternalIP, + Address: srv.AccessIPv4, + }, + ) + } + + if srv.AccessIPv6 != "" && !networkingOpts.IPv6SupportDisabled { + v1helper.AddToNodeAddresses(&addrs, + v1.NodeAddress{ + Type: v1.NodeExternalIP, + Address: srv.AccessIPv6, + }, + ) + } + + if srv.Metadata[TypeHostName] != "" { + v1helper.AddToNodeAddresses(&addrs, + v1.NodeAddress{ + Type: v1.NodeHostName, + Address: srv.Metadata[TypeHostName], + }, + ) + } + + // process the rest type Address struct { IPType string `mapstructure:"OS-EXT-IPS:type"` Addr string @@ -717,34 +766,6 @@ func nodeAddresses(srv *servers.Server, networkingOpts NetworkingOpts) ([]v1.Nod } } - // AccessIPs are usually duplicates of "public" addresses. - if srv.AccessIPv4 != "" { - v1helper.AddToNodeAddresses(&addrs, - v1.NodeAddress{ - Type: v1.NodeExternalIP, - Address: srv.AccessIPv4, - }, - ) - } - - if srv.AccessIPv6 != "" && !networkingOpts.IPv6SupportDisabled { - v1helper.AddToNodeAddresses(&addrs, - v1.NodeAddress{ - Type: v1.NodeExternalIP, - Address: srv.AccessIPv6, - }, - ) - } - - if srv.Metadata[TypeHostName] != "" { - v1helper.AddToNodeAddresses(&addrs, - v1.NodeAddress{ - Type: v1.NodeHostName, - Address: srv.Metadata[TypeHostName], - }, - ) - } - return addrs, nil } @@ -754,7 +775,12 @@ func getAddressesByName(client *gophercloud.ServiceClient, name types.NodeName, return nil, err } - return nodeAddresses(&srv.Server, networkingOpts) + interfaces, err := getAttachedInterfacesByID(client, srv.ID) + if err != nil { + return nil, err + } + + return nodeAddresses(&srv.Server, interfaces, networkingOpts) } func getAddressByName(client *gophercloud.ServiceClient, name types.NodeName, needIPv6 bool, networkingOpts NetworkingOpts) (string, error) { diff --git a/pkg/cloudprovider/providers/openstack/openstack_instances.go b/pkg/cloudprovider/providers/openstack/openstack_instances.go index caf7b50735..0422d27b7f 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_instances.go +++ b/pkg/cloudprovider/providers/openstack/openstack_instances.go @@ -107,7 +107,12 @@ func (i *Instances) NodeAddressesByProviderID(ctx context.Context, providerID st return []v1.NodeAddress{}, err } - addresses, err := nodeAddresses(server, i.networkingOpts) + interfaces, err := getAttachedInterfacesByID(i.compute, server.ID) + if err != nil { + return []v1.NodeAddress{}, err + } + + addresses, err := nodeAddresses(server, interfaces, i.networkingOpts) if err != nil { return []v1.NodeAddress{}, err } diff --git a/pkg/cloudprovider/providers/openstack/openstack_routes.go b/pkg/cloudprovider/providers/openstack/openstack_routes.go index ff36973ace..965f4d0e6e 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_routes.go +++ b/pkg/cloudprovider/providers/openstack/openstack_routes.go @@ -61,7 +61,12 @@ func (r *Routes) ListRoutes(ctx context.Context, clusterName string) ([]*cloudpr nodeNamesByAddr := make(map[string]types.NodeName) err := foreachServer(r.compute, servers.ListOpts{}, func(srv *servers.Server) (bool, error) { - addrs, err := nodeAddresses(srv, r.networkingOpts) + interfaces, err := getAttachedInterfacesByID(r.compute, srv.ID) + if err != nil { + return false, err + } + + addrs, err := nodeAddresses(srv, interfaces, r.networkingOpts) if err != nil { return false, err } diff --git a/pkg/cloudprovider/providers/openstack/openstack_test.go b/pkg/cloudprovider/providers/openstack/openstack_test.go index 174bff118c..bd7fa0769a 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_test.go +++ b/pkg/cloudprovider/providers/openstack/openstack_test.go @@ -29,6 +29,7 @@ import ( "time" "github.com/gophercloud/gophercloud" + "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/attachinterfaces" "github.com/gophercloud/gophercloud/openstack/compute/v2/servers" "github.com/spf13/pflag" @@ -587,7 +588,20 @@ func TestNodeAddresses(t *testing.T) { PublicNetworkName: "public", } - addrs, err := nodeAddresses(&srv, networkingOpts) + interfaces := []attachinterfaces.Interface{ + { + FixedIPs: []attachinterfaces.FixedIP{ + { + IPAddress: "10.0.0.32", + }, + { + IPAddress: "10.0.0.31", + }, + }, + }, + } + + addrs, err := nodeAddresses(&srv, interfaces, networkingOpts) if err != nil { t.Fatalf("nodeAddresses returned error: %v", err) } @@ -596,13 +610,13 @@ func TestNodeAddresses(t *testing.T) { want := []v1.NodeAddress{ {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, - {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, {Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"}, {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.novalocal"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, + {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, } if !reflect.DeepEqual(want, addrs) { @@ -652,7 +666,20 @@ func TestNodeAddressesCustomPublicNetwork(t *testing.T) { PublicNetworkName: "pub-net", } - addrs, err := nodeAddresses(&srv, networkingOpts) + interfaces := []attachinterfaces.Interface{ + { + FixedIPs: []attachinterfaces.FixedIP{ + { + IPAddress: "10.0.0.32", + }, + { + IPAddress: "10.0.0.31", + }, + }, + }, + } + + addrs, err := nodeAddresses(&srv, interfaces, networkingOpts) if err != nil { t.Fatalf("nodeAddresses returned error: %v", err) } @@ -661,12 +688,12 @@ func TestNodeAddressesCustomPublicNetwork(t *testing.T) { want := []v1.NodeAddress{ {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, - {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, {Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, + {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, } if !reflect.DeepEqual(want, addrs) { @@ -717,7 +744,20 @@ func TestNodeAddressesIPv6Disabled(t *testing.T) { IPv6SupportDisabled: true, } - addrs, err := nodeAddresses(&srv, networkingOpts) + interfaces := []attachinterfaces.Interface{ + { + FixedIPs: []attachinterfaces.FixedIP{ + { + IPAddress: "10.0.0.32", + }, + { + IPAddress: "10.0.0.31", + }, + }, + }, + } + + addrs, err := nodeAddresses(&srv, interfaces, networkingOpts) if err != nil { t.Fatalf("nodeAddresses returned error: %v", err) } @@ -726,10 +766,10 @@ func TestNodeAddressesIPv6Disabled(t *testing.T) { want := []v1.NodeAddress{ {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, } if !reflect.DeepEqual(want, addrs) { From 9256f0224d8cfb1a0f434c8450b47a7a8b5409cd Mon Sep 17 00:00:00 2001 From: kayrus Date: Wed, 8 Jan 2020 07:12:30 +0100 Subject: [PATCH 2/2] code review --- .../providers/openstack/openstack.go | 20 ++++++++++--------- .../providers/openstack/openstack_test.go | 3 +++ 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/pkg/cloudprovider/providers/openstack/openstack.go b/pkg/cloudprovider/providers/openstack/openstack.go index 0cbd22ccb8..096cbc7b21 100644 --- a/pkg/cloudprovider/providers/openstack/openstack.go +++ b/pkg/cloudprovider/providers/openstack/openstack.go @@ -675,21 +675,23 @@ func getServerByName(client *gophercloud.ServiceClient, name types.NodeName) (*S // * interfaces private IPs // * access IPs // * metadata hostname -// * server object Addresses +// * server object Addresses (floating type) func nodeAddresses(srv *servers.Server, interfaces []attachinterfaces.Interface, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) { addrs := []v1.NodeAddress{} // parse private IP addresses first in an ordered manner for _, iface := range interfaces { for _, fixedIP := range iface.FixedIPs { - isIPv6 := net.ParseIP(fixedIP.IPAddress).To4() == nil - if !(isIPv6 && networkingOpts.IPv6SupportDisabled) { - v1helper.AddToNodeAddresses(&addrs, - v1.NodeAddress{ - Type: v1.NodeInternalIP, - Address: fixedIP.IPAddress, - }, - ) + if iface.PortState == "ACTIVE" { + isIPv6 := net.ParseIP(fixedIP.IPAddress).To4() == nil + if !(isIPv6 && networkingOpts.IPv6SupportDisabled) { + v1helper.AddToNodeAddresses(&addrs, + v1.NodeAddress{ + Type: v1.NodeInternalIP, + Address: fixedIP.IPAddress, + }, + ) + } } } } diff --git a/pkg/cloudprovider/providers/openstack/openstack_test.go b/pkg/cloudprovider/providers/openstack/openstack_test.go index bd7fa0769a..605cb5cf3c 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_test.go +++ b/pkg/cloudprovider/providers/openstack/openstack_test.go @@ -590,6 +590,7 @@ func TestNodeAddresses(t *testing.T) { interfaces := []attachinterfaces.Interface{ { + PortState: "ACTIVE", FixedIPs: []attachinterfaces.FixedIP{ { IPAddress: "10.0.0.32", @@ -668,6 +669,7 @@ func TestNodeAddressesCustomPublicNetwork(t *testing.T) { interfaces := []attachinterfaces.Interface{ { + PortState: "ACTIVE", FixedIPs: []attachinterfaces.FixedIP{ { IPAddress: "10.0.0.32", @@ -746,6 +748,7 @@ func TestNodeAddressesIPv6Disabled(t *testing.T) { interfaces := []attachinterfaces.Interface{ { + PortState: "ACTIVE", FixedIPs: []attachinterfaces.FixedIP{ { IPAddress: "10.0.0.32",