Skip to content

Commit b51f348

Browse files
mdboothmandre
mdbooth
authored and
mandre
committed
Replace call to Nova os-interfaces with direct Neutron call (kubernetes#2250)
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 3b70141 commit b51f348

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
@@ -680,7 +680,7 @@ func getBoolFromServiceAnnotation(service *corev1.Service, annotationKey string,
680680
}
681681

682682
// getSubnetIDForLB returns subnet-id for a specific node
683-
func getSubnetIDForLB(compute *gophercloud.ServiceClient, node corev1.Node, preferredIPFamily corev1.IPFamily) (string, error) {
683+
func getSubnetIDForLB(network *gophercloud.ServiceClient, node corev1.Node, preferredIPFamily corev1.IPFamily) (string, error) {
684684
ipAddress, err := nodeAddressForLB(&node, preferredIPFamily)
685685
if err != nil {
686686
return "", err
@@ -691,13 +691,13 @@ func getSubnetIDForLB(compute *gophercloud.ServiceClient, node corev1.Node, pref
691691
instanceID = instanceID[(ind + 1):]
692692
}
693693

694-
interfaces, err := getAttachedInterfacesByID(compute, instanceID)
694+
ports, err := getAttachedPorts(network, instanceID)
695695
if err != nil {
696696
return "", err
697697
}
698698

699-
for _, intf := range interfaces {
700-
for _, fixedIP := range intf.FixedIPs {
699+
for _, port := range ports {
700+
for _, fixedIP := range port.FixedIPs {
701701
if fixedIP.IPAddress == ipAddress {
702702
return fixedIP.SubnetID, nil
703703
}
@@ -1546,7 +1546,7 @@ func (lbaas *LbaasV2) checkServiceUpdate(service *corev1.Service, nodes []*corev
15461546
} else {
15471547
svcConf.lbMemberSubnetID = getStringFromServiceAnnotation(service, ServiceAnnotationLoadBalancerSubnetID, lbaas.opts.SubnetID)
15481548
if len(svcConf.lbMemberSubnetID) == 0 && len(nodes) > 0 {
1549-
subnetID, err := getSubnetIDForLB(lbaas.compute, *nodes[0], svcConf.preferredIPFamily)
1549+
subnetID, err := getSubnetIDForLB(lbaas.network, *nodes[0], svcConf.preferredIPFamily)
15501550
if err != nil {
15511551
return fmt.Errorf("no subnet-id found for service %s: %v", serviceName, err)
15521552
}
@@ -1675,7 +1675,7 @@ func (lbaas *LbaasV2) checkService(service *corev1.Service, nodes []*corev1.Node
16751675
svcConf.lbMemberSubnetID = svcConf.lbSubnetID
16761676
}
16771677
if len(svcConf.lbNetworkID) == 0 && len(svcConf.lbSubnetID) == 0 {
1678-
subnetID, err := getSubnetIDForLB(lbaas.compute, *nodes[0], svcConf.preferredIPFamily)
1678+
subnetID, err := getSubnetIDForLB(lbaas.network, *nodes[0], svcConf.preferredIPFamily)
16791679
if err != nil {
16801680
return fmt.Errorf("failed to get subnet to create load balancer for service %s: %v", serviceName, err)
16811681
}

‎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"
@@ -373,10 +373,10 @@ func TestNodeAddresses(t *testing.T) {
373373
PublicNetworkName: []string{"public"},
374374
}
375375

376-
interfaces := []attachinterfaces.Interface{
376+
ports := []ports.Port{
377377
{
378-
PortState: "ACTIVE",
379-
FixedIPs: []attachinterfaces.FixedIP{
378+
Status: "ACTIVE",
379+
FixedIPs: []ports.IP{
380380
{
381381
IPAddress: "10.0.0.32",
382382
},
@@ -387,7 +387,7 @@ func TestNodeAddresses(t *testing.T) {
387387
},
388388
}
389389

390-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
390+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
391391
if err != nil {
392392
t.Fatalf("nodeAddresses returned error: %v", err)
393393
}
@@ -452,10 +452,10 @@ func TestNodeAddressesCustomPublicNetwork(t *testing.T) {
452452
PublicNetworkName: []string{"pub-net"},
453453
}
454454

455-
interfaces := []attachinterfaces.Interface{
455+
ports := []ports.Port{
456456
{
457-
PortState: "ACTIVE",
458-
FixedIPs: []attachinterfaces.FixedIP{
457+
Status: "ACTIVE",
458+
FixedIPs: []ports.IP{
459459
{
460460
IPAddress: "10.0.0.32",
461461
},
@@ -466,7 +466,7 @@ func TestNodeAddressesCustomPublicNetwork(t *testing.T) {
466466
},
467467
}
468468

469-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
469+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
470470
if err != nil {
471471
t.Fatalf("nodeAddresses returned error: %v", err)
472472
}
@@ -525,10 +525,10 @@ func TestNodeAddressesCustomPublicNetworkWithIntersectingFixedIP(t *testing.T) {
525525
PublicNetworkName: []string{"pub-net"},
526526
}
527527

528-
interfaces := []attachinterfaces.Interface{
528+
ports := []ports.Port{
529529
{
530-
PortState: "ACTIVE",
531-
FixedIPs: []attachinterfaces.FixedIP{
530+
Status: "ACTIVE",
531+
FixedIPs: []ports.IP{
532532
{
533533
IPAddress: "10.0.0.32",
534534
},
@@ -543,7 +543,7 @@ func TestNodeAddressesCustomPublicNetworkWithIntersectingFixedIP(t *testing.T) {
543543
},
544544
}
545545

546-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
546+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
547547
if err != nil {
548548
t.Fatalf("nodeAddresses returned error: %v", err)
549549
}
@@ -613,10 +613,10 @@ func TestNodeAddressesMultipleCustomInternalNetworks(t *testing.T) {
613613
InternalNetworkName: []string{"private", "also-private"},
614614
}
615615

616-
interfaces := []attachinterfaces.Interface{
616+
ports := []ports.Port{
617617
{
618-
PortState: "ACTIVE",
619-
FixedIPs: []attachinterfaces.FixedIP{
618+
Status: "ACTIVE",
619+
FixedIPs: []ports.IP{
620620
{
621621
IPAddress: "10.0.0.32",
622622
},
@@ -627,7 +627,7 @@ func TestNodeAddressesMultipleCustomInternalNetworks(t *testing.T) {
627627
},
628628
}
629629

630-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
630+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
631631
if err != nil {
632632
t.Fatalf("nodeAddresses returned error: %v", err)
633633
}
@@ -697,10 +697,10 @@ func TestNodeAddressesOneInternalNetwork(t *testing.T) {
697697
InternalNetworkName: []string{"also-private"},
698698
}
699699

700-
interfaces := []attachinterfaces.Interface{
700+
ports := []ports.Port{
701701
{
702-
PortState: "ACTIVE",
703-
FixedIPs: []attachinterfaces.FixedIP{
702+
Status: "ACTIVE",
703+
FixedIPs: []ports.IP{
704704
{
705705
IPAddress: "10.0.0.32",
706706
},
@@ -711,7 +711,7 @@ func TestNodeAddressesOneInternalNetwork(t *testing.T) {
711711
},
712712
}
713713

714-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
714+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
715715
if err != nil {
716716
t.Fatalf("nodeAddresses returned error: %v", err)
717717
}
@@ -773,10 +773,10 @@ func TestNodeAddressesIPv6Disabled(t *testing.T) {
773773
IPv6SupportDisabled: true,
774774
}
775775

776-
interfaces := []attachinterfaces.Interface{
776+
ports := []ports.Port{
777777
{
778-
PortState: "ACTIVE",
779-
FixedIPs: []attachinterfaces.FixedIP{
778+
Status: "ACTIVE",
779+
FixedIPs: []ports.IP{
780780
{
781781
IPAddress: "10.0.0.32",
782782
},
@@ -787,7 +787,7 @@ func TestNodeAddressesIPv6Disabled(t *testing.T) {
787787
},
788788
}
789789

790-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
790+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
791791
if err != nil {
792792
t.Fatalf("nodeAddresses returned error: %v", err)
793793
}
@@ -854,10 +854,10 @@ func TestNodeAddressesWithAddressSortOrderOptions(t *testing.T) {
854854
AddressSortOrder: "10.0.0.0/8, 50.56.176.0/24, 2001:4800::/32",
855855
}
856856

857-
interfaces := []attachinterfaces.Interface{
857+
ports := []ports.Port{
858858
{
859-
PortState: "ACTIVE",
860-
FixedIPs: []attachinterfaces.FixedIP{
859+
Status: "ACTIVE",
860+
FixedIPs: []ports.IP{
861861
{
862862
IPAddress: "10.0.0.32",
863863
},
@@ -868,7 +868,7 @@ func TestNodeAddressesWithAddressSortOrderOptions(t *testing.T) {
868868
},
869869
}
870870

871-
addrs, err := nodeAddresses(&srv, interfaces, networkingOpts)
871+
addrs, err := nodeAddresses(&srv, ports, networkingOpts)
872872
if err != nil {
873873
t.Fatalf("nodeAddresses returned error: %v", err)
874874
}

0 commit comments

Comments
 (0)