Skip to content

Commit 08efe87

Browse files
mdboothmdbooth
committed
Replace call to Nova os-interfaces with direct Neutron call
os-interfaces is a proxy API for neutron's ports list. This has 2 significant disadvantages to us: * It adds the overhead of an extra API call every time it is used * It doesn't contain all the information returned by Neutron
1 parent 98ba5d7 commit 08efe87

File tree

4 files changed

+71
-65
lines changed

4 files changed

+71
-65
lines changed

‎pkg/openstack/instances.go‎

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ import (
2727
"strings"
2828

2929
"github.com/gophercloud/gophercloud"
30-
"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/attachinterfaces"
3130
"github.com/gophercloud/gophercloud/openstack/compute/v2/flavors"
3231
"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
32+
"github.com/gophercloud/gophercloud/openstack/networking/v2/ports"
3333
"github.com/gophercloud/gophercloud/pagination"
3434
"github.com/mitchellh/mapstructure"
3535
v1 "k8s.io/api/core/v1"
@@ -43,11 +43,13 @@ import (
4343
"k8s.io/cloud-provider-openstack/pkg/util"
4444
"k8s.io/cloud-provider-openstack/pkg/util/errors"
4545
"k8s.io/cloud-provider-openstack/pkg/util/metadata"
46+
"k8s.io/cloud-provider-openstack/pkg/util/openstack"
4647
)
4748

4849
// Instances encapsulates an implementation of Instances for OpenStack.
4950
type Instances struct {
5051
compute *gophercloud.ServiceClient
52+
network *gophercloud.ServiceClient
5153
region string
5254
regionProviderID bool
5355
opts metadata.Opts
@@ -148,13 +150,20 @@ func (os *OpenStack) instances() (*Instances, bool) {
148150
return nil, false
149151
}
150152

153+
network, err := client.NewNetworkV2(os.provider, os.epOpts)
154+
if err != nil {
155+
klog.Errorf("unable to access network v2 API : %v", err)
156+
return nil, false
157+
}
158+
151159
regionalProviderID := false
152160
if isRegionalProviderID := sysos.Getenv(RegionalProviderIDEnv); isRegionalProviderID == "true" {
153161
regionalProviderID = true
154162
}
155163

156164
return &Instances{
157165
compute: compute,
166+
network: network,
158167
region: os.epOpts.Region,
159168
regionProviderID: regionalProviderID,
160169
opts: os.metadataOpts,
@@ -226,12 +235,12 @@ func (i *Instances) NodeAddressesByProviderID(ctx context.Context, providerID st
226235
return []v1.NodeAddress{}, err
227236
}
228237

229-
interfaces, err := getAttachedInterfacesByID(i.compute, server.ID)
238+
ports, err := getAttachedPorts(i.network, server.ID)
230239
if err != nil {
231240
return []v1.NodeAddress{}, err
232241
}
233242

234-
addresses, err := nodeAddresses(server, interfaces, i.networkingOpts)
243+
addresses, err := nodeAddresses(server, ports, i.networkingOpts)
235244
if err != nil {
236245
return []v1.NodeAddress{}, err
237246
}
@@ -332,11 +341,11 @@ func (i *Instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud
332341
return nil, err
333342
}
334343

335-
interfaces, err := getAttachedInterfacesByID(i.compute, srv.ID)
344+
ports, err := getAttachedPorts(i.network, srv.ID)
336345
if err != nil {
337346
return nil, err
338347
}
339-
addresses, err := nodeAddresses(srv, interfaces, i.networkingOpts)
348+
addresses, err := nodeAddresses(srv, ports, i.networkingOpts)
340349
if err != nil {
341350
return nil, err
342351
}
@@ -564,13 +573,13 @@ func getServerByName(client *gophercloud.ServiceClient, name types.NodeName) (*S
564573
// * access IPs
565574
// * metadata hostname
566575
// * server object Addresses (floating type)
567-
func nodeAddresses(srv *servers.Server, interfaces []attachinterfaces.Interface, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) {
576+
func nodeAddresses(srv *servers.Server, ports []ports.Port, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) {
568577
addrs := []v1.NodeAddress{}
569578

570579
// parse private IP addresses first in an ordered manner
571-
for _, iface := range interfaces {
572-
for _, fixedIP := range iface.FixedIPs {
573-
if iface.PortState == "ACTIVE" {
580+
for _, port := range ports {
581+
for _, fixedIP := range port.FixedIPs {
582+
if port.Status == "ACTIVE" {
574583
isIPv6 := net.ParseIP(fixedIP.IPAddress).To4() == nil
575584
if !(isIPv6 && networkingOpts.IPv6SupportDisabled) {
576585
AddToNodeAddresses(&addrs,
@@ -683,31 +692,20 @@ func getAddressesByName(client *gophercloud.ServiceClient, name types.NodeName,
683692
return nil, err
684693
}
685694

686-
interfaces, err := getAttachedInterfacesByID(client, srv.ID)
695+
ports, err := getAttachedPorts(client, srv.ID)
687696
if err != nil {
688697
return nil, err
689698
}
690699

691-
return nodeAddresses(&srv.Server, interfaces, networkingOpts)
700+
return nodeAddresses(&srv.Server, ports, networkingOpts)
692701
}
693702

694-
// getAttachedInterfacesByID returns the node interfaces of the specified instance.
695-
func getAttachedInterfacesByID(client *gophercloud.ServiceClient, serviceID string) ([]attachinterfaces.Interface, error) {
696-
var interfaces []attachinterfaces.Interface
697-
698-
mc := metrics.NewMetricContext("server_os_interface", "list")
699-
pager := attachinterfaces.List(client, serviceID)
700-
err := pager.EachPage(func(page pagination.Page) (bool, error) {
701-
s, err := attachinterfaces.ExtractInterfaces(page)
702-
if err != nil {
703-
return false, err
704-
}
705-
interfaces = append(interfaces, s...)
706-
return true, nil
707-
})
708-
if mc.ObserveRequest(err) != nil {
709-
return interfaces, err
703+
// getAttachedPorts returns a list of ports attached to a server.
704+
func getAttachedPorts(client *gophercloud.ServiceClient, serverID string) ([]ports.Port, error) {
705+
listOpts := ports.ListOpts{
706+
DeviceID: serverID,
707+
DeviceOwner: "compute:nova",
710708
}
711709

712-
return interfaces, nil
710+
return openstack.GetPorts(client, listOpts)
713711
}

‎pkg/openstack/instancesv2.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
// InstancesV2 encapsulates an implementation of InstancesV2 for OpenStack.
3535
type InstancesV2 struct {
3636
compute *gophercloud.ServiceClient
37+
network *gophercloud.ServiceClient
3738
region string
3839
regionProviderID bool
3940
networkingOpts NetworkingOpts
@@ -56,13 +57,20 @@ func (os *OpenStack) instancesv2() (*InstancesV2, bool) {
5657
return nil, false
5758
}
5859

60+
network, err := client.NewNetworkV2(os.provider, os.epOpts)
61+
if err != nil {
62+
klog.Errorf("unable to access network v2 API : %v", err)
63+
return nil, false
64+
}
65+
5966
regionalProviderID := false
6067
if isRegionalProviderID := sysos.Getenv(RegionalProviderIDEnv); isRegionalProviderID == "true" {
6168
regionalProviderID = true
6269
}
6370

6471
return &InstancesV2{
6572
compute: compute,
73+
network: network,
6674
region: os.epOpts.Region,
6775
regionProviderID: regionalProviderID,
6876
networkingOpts: os.networkingOpts,
@@ -115,12 +123,12 @@ func (i *InstancesV2) InstanceMetadata(ctx context.Context, node *v1.Node) (*clo
115123
return nil, err
116124
}
117125

118-
interfaces, err := getAttachedInterfacesByID(i.compute, server.ID)
126+
ports, err := getAttachedPorts(i.network, server.ID)
119127
if err != nil {
120128
return nil, err
121129
}
122130

123-
addresses, err := nodeAddresses(&server.Server, interfaces, i.networkingOpts)
131+
addresses, err := nodeAddresses(&server.Server, ports, i.networkingOpts)
124132
if err != nil {
125133
return nil, err
126134
}

‎pkg/openstack/loadbalancer.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -707,7 +707,7 @@ func getBoolFromServiceAnnotation(service *corev1.Service, annotationKey string,
707707
}
708708

709709
// getSubnetIDForLB returns subnet-id for a specific node
710-
func getSubnetIDForLB(compute *gophercloud.ServiceClient, node corev1.Node, preferredIPFamily corev1.IPFamily) (string, error) {
710+
func getSubnetIDForLB(network *gophercloud.ServiceClient, node corev1.Node, preferredIPFamily corev1.IPFamily) (string, error) {
711711
ipAddress, err := nodeAddressForLB(&node, preferredIPFamily)
712712
if err != nil {
713713
return "", err
@@ -718,13 +718,13 @@ func getSubnetIDForLB(compute *gophercloud.ServiceClient, node corev1.Node, pref
718718
instanceID = instanceID[(ind + 1):]
719719
}
720720

721-
interfaces, err := getAttachedInterfacesByID(compute, instanceID)
721+
ports, err := getAttachedPorts(network, instanceID)
722722
if err != nil {
723723
return "", err
724724
}
725725

726-
for _, intf := range interfaces {
727-
for _, fixedIP := range intf.FixedIPs {
726+
for _, port := range ports {
727+
for _, fixedIP := range port.FixedIPs {
728728
if fixedIP.IPAddress == ipAddress {
729729
return fixedIP.SubnetID, nil
730730
}
@@ -1554,7 +1554,7 @@ func (lbaas *LbaasV2) checkServiceUpdate(service *corev1.Service, nodes []*corev
15541554
} else {
15551555
svcConf.lbMemberSubnetID = getStringFromServiceAnnotation(service, ServiceAnnotationLoadBalancerSubnetID, lbaas.opts.SubnetID)
15561556
if len(svcConf.lbMemberSubnetID) == 0 && len(nodes) > 0 {
1557-
subnetID, err := getSubnetIDForLB(lbaas.compute, *nodes[0], svcConf.preferredIPFamily)
1557+
subnetID, err := getSubnetIDForLB(lbaas.network, *nodes[0], svcConf.preferredIPFamily)
15581558
if err != nil {
15591559
return fmt.Errorf("no subnet-id found for service %s: %v", serviceName, err)
15601560
}
@@ -1668,7 +1668,7 @@ func (lbaas *LbaasV2) checkService(service *corev1.Service, nodes []*corev1.Node
16681668
svcConf.lbMemberSubnetID = svcConf.lbSubnetID
16691669
}
16701670
if len(svcConf.lbNetworkID) == 0 && len(svcConf.lbSubnetID) == 0 {
1671-
subnetID, err := getSubnetIDForLB(lbaas.compute, *nodes[0], svcConf.preferredIPFamily)
1671+
subnetID, err := getSubnetIDForLB(lbaas.network, *nodes[0], svcConf.preferredIPFamily)
16721672
if err != nil {
16731673
return fmt.Errorf("failed to get subnet to create load balancer for service %s: %v", serviceName, err)
16741674
}

‎pkg/openstack/openstack_test.go‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ import (
2727
"time"
2828

2929
"github.com/gophercloud/gophercloud"
30-
"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/attachinterfaces"
3130
"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
31+
"github.com/gophercloud/gophercloud/openstack/networking/v2/ports"
3232
"github.com/spf13/pflag"
3333
v1 "k8s.io/api/core/v1"
3434
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -360,10 +360,10 @@ func TestNodeAddresses(t *testing.T) {
360360
PublicNetworkName: []string{"public"},
361361
}
362362

363-
interfaces := []attachinterfaces.Interface{
363+
ports := []ports.Port{
364364
{
365-
PortState: "ACTIVE",
366-
FixedIPs: []attachinterfaces.FixedIP{
365+
Status: "ACTIVE",
366+
FixedIPs: []ports.IP{
367367
{
368368
IPAddress: "10.0.0.32",
369369
},
@@ -374,7 +374,7 @@ func TestNodeAddresses(t *testing.T) {
374374
},
375375
}
376376

377-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
377+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
378378
if err != nil {
379379
t.Fatalf("nodeAddresses returned error: %v", err)
380380
}
@@ -439,10 +439,10 @@ func TestNodeAddressesCustomPublicNetwork(t *testing.T) {
439439
PublicNetworkName: []string{"pub-net"},
440440
}
441441

442-
interfaces := []attachinterfaces.Interface{
442+
ports := []ports.Port{
443443
{
444-
PortState: "ACTIVE",
445-
FixedIPs: []attachinterfaces.FixedIP{
444+
Status: "ACTIVE",
445+
FixedIPs: []ports.IP{
446446
{
447447
IPAddress: "10.0.0.32",
448448
},
@@ -453,7 +453,7 @@ func TestNodeAddressesCustomPublicNetwork(t *testing.T) {
453453
},
454454
}
455455

456-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
456+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
457457
if err != nil {
458458
t.Fatalf("nodeAddresses returned error: %v", err)
459459
}
@@ -512,10 +512,10 @@ func TestNodeAddressesCustomPublicNetworkWithIntersectingFixedIP(t *testing.T) {
512512
PublicNetworkName: []string{"pub-net"},
513513
}
514514

515-
interfaces := []attachinterfaces.Interface{
515+
ports := []ports.Port{
516516
{
517-
PortState: "ACTIVE",
518-
FixedIPs: []attachinterfaces.FixedIP{
517+
Status: "ACTIVE",
518+
FixedIPs: []ports.IP{
519519
{
520520
IPAddress: "10.0.0.32",
521521
},
@@ -530,7 +530,7 @@ func TestNodeAddressesCustomPublicNetworkWithIntersectingFixedIP(t *testing.T) {
530530
},
531531
}
532532

533-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
533+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
534534
if err != nil {
535535
t.Fatalf("nodeAddresses returned error: %v", err)
536536
}
@@ -600,10 +600,10 @@ func TestNodeAddressesMultipleCustomInternalNetworks(t *testing.T) {
600600
InternalNetworkName: []string{"private", "also-private"},
601601
}
602602

603-
interfaces := []attachinterfaces.Interface{
603+
ports := []ports.Port{
604604
{
605-
PortState: "ACTIVE",
606-
FixedIPs: []attachinterfaces.FixedIP{
605+
Status: "ACTIVE",
606+
FixedIPs: []ports.IP{
607607
{
608608
IPAddress: "10.0.0.32",
609609
},
@@ -614,7 +614,7 @@ func TestNodeAddressesMultipleCustomInternalNetworks(t *testing.T) {
614614
},
615615
}
616616

617-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
617+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
618618
if err != nil {
619619
t.Fatalf("nodeAddresses returned error: %v", err)
620620
}
@@ -684,10 +684,10 @@ func TestNodeAddressesOneInternalNetwork(t *testing.T) {
684684
InternalNetworkName: []string{"also-private"},
685685
}
686686

687-
interfaces := []attachinterfaces.Interface{
687+
ports := []ports.Port{
688688
{
689-
PortState: "ACTIVE",
690-
FixedIPs: []attachinterfaces.FixedIP{
689+
Status: "ACTIVE",
690+
FixedIPs: []ports.IP{
691691
{
692692
IPAddress: "10.0.0.32",
693693
},
@@ -698,7 +698,7 @@ func TestNodeAddressesOneInternalNetwork(t *testing.T) {
698698
},
699699
}
700700

701-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
701+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
702702
if err != nil {
703703
t.Fatalf("nodeAddresses returned error: %v", err)
704704
}
@@ -760,10 +760,10 @@ func TestNodeAddressesIPv6Disabled(t *testing.T) {
760760
IPv6SupportDisabled: true,
761761
}
762762

763-
interfaces := []attachinterfaces.Interface{
763+
ports := []ports.Port{
764764
{
765-
PortState: "ACTIVE",
766-
FixedIPs: []attachinterfaces.FixedIP{
765+
Status: "ACTIVE",
766+
FixedIPs: []ports.IP{
767767
{
768768
IPAddress: "10.0.0.32",
769769
},
@@ -774,7 +774,7 @@ func TestNodeAddressesIPv6Disabled(t *testing.T) {
774774
},
775775
}
776776

777-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
777+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
778778
if err != nil {
779779
t.Fatalf("nodeAddresses returned error: %v", err)
780780
}
@@ -841,10 +841,10 @@ func TestNodeAddressesWithAddressSortOrderOptions(t *testing.T) {
841841
AddressSortOrder: "10.0.0.0/8, 50.56.176.0/24, 2001:4800::/32",
842842
}
843843

844-
interfaces := []attachinterfaces.Interface{
844+
ports := []ports.Port{
845845
{
846-
PortState: "ACTIVE",
847-
FixedIPs: []attachinterfaces.FixedIP{
846+
Status: "ACTIVE",
847+
FixedIPs: []ports.IP{
848848
{
849849
IPAddress: "10.0.0.32",
850850
},
@@ -855,7 +855,7 @@ func TestNodeAddressesWithAddressSortOrderOptions(t *testing.T) {
855855
},
856856
}
857857

858-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
858+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
859859
if err != nil {
860860
t.Fatalf("nodeAddresses returned error: %v", err)
861861
}

0 commit comments

Comments
 (0)