Add Nova interface-list support by FengyunPan · Pull Request #430 · gophercloud/gophercloud

FengyunPan · 2017-07-22T07:39:56Z

Add support to list interface for a server
through a GET on: /v2.1/{tenant_id}/servers/{server_id}/os-interface

The same operation with OpenStack CLI is done with:
nova interface-list <server_id>

Fix gophercloud issue:
#431

Fix kubernetes issue:

coveralls · 2017-07-22T08:11:06Z

Coverage increased (+0.05%) to 71.252% when pulling 7c33e5b on FengyunPan:add-interface-list into 2804b72 on gophercloud:master.

FengyunPan · 2017-07-22T08:23:03Z

@jtopjian Hi jtopjian, I'd like you to do a code review, thank you.
Currently the openstack cloud provider of kubernetes can't get port by ip, so we need support nova interface-list for it, then openstack cloud provider can get port by interface and ip.

jtopjian · 2017-07-22T14:51:38Z

@FengyunPan Thank you for submitting this. Per the PR template and the STYLEGUIDE you'll need to provide links to the python source code which implements this feature in the OpenStack service. Also, a corresponding Gophercloud issue needs to be opened. Please let me know if you have any questions.

FengyunPan · 2017-07-22T16:01:05Z

@jtopjian I have added it, need other information?

jtopjian · 2017-07-27T04:18:11Z

@FengyunPan I had some time to look into this in more detail.

It looks like the ability to "get" interfaces is part of the os-interface-attachment. This can be seen in the Nova routes here and here. That ends up linking to the attach_interfaces.py file you linked to earlier:

https://github.com/openstack/nova/blob/4f91ed3a547965ed96a22520edcfb783e7936e95/nova/api/openstack/compute/attach_interfaces.py

This is actually utterly confusing because I had no idea this ability existed hidden in the os-attach-interface extension -- I could have used this a long time ago, so thank you for submitting it.

Since it's an extension, it needs to be packaged under the compute/v2/extensions directory, titled attachinterfaces. It's relatively easy to bundle this code as an extension. See the newly added pauseunpause package as a template.

As for the code itself, you'll need to modify the field names to use "camel case" rather than the current format which is using underscores. For example, Fixed_id needs renamed to FixedID, Port_state needs changed to PortState, etc.

Once all of that is done, I can do another review.

Let me know if you have any questions or need any help.

FengyunPan · 2017-07-27T07:49:35Z

@jtopjian Thank for your comment, I will update it soon.

coveralls · 2017-07-27T09:07:55Z

Coverage increased (+0.05%) to 71.252% when pulling 2a59bd5 on FengyunPan:add-interface-list into 2804b72 on gophercloud:master.

jtopjian

@FengyunPan This looks really good. Thank you for both your patience and your work on this.

Please see the request for changes. They're all quite simple -- just renaming and rewording.

jtopjian · 2017-07-28T03:46:31Z

@@ -0,0 +1,3 @@
+// Package attachinterfaces provides the ability to manage attach interfaces through


How about:

"the ability to manage network interfaces"

Yeah, thank you. Done.

jtopjian · 2017-07-28T03:48:21Z

+)
+
+// ListInterfaces makes a request against the nova API to list the servers interfaces.
+func ListInterfaces(client *gophercloud.ServiceClient, id string) pagination.Pager {


The attach-interfaces extension only lists one type of resource (interfaces), so this can be renamed to just List.

In addition, the id is the id of the server, so let's rename id to serverID so it's not mistaken as an "interface id".

jtopjian · 2017-07-28T03:48:56Z

+// Interface represents an interface.
+type FixedID struct {
+	SubnetID  string `json:"subnet_id"`
+	IpAddress string `json:"ip_address"`


IPAddress

jtopjian · 2017-07-28T03:49:36Z

+	"github.com/gophercloud/gophercloud/pagination"
+)
+
+// Interface represents an interface.


// FixedIP represents a Fixed IP Address.

jtopjian · 2017-07-28T03:49:44Z

+)
+
+// Interface represents an interface.
+type FixedID struct {


jtopjian · 2017-07-28T03:50:27Z

+	IpAddress string `json:"ip_address"`
+}
+
+// Interface represents an interface.


How about:

// Interface represents a network interface on an instance.

Just so it's not confused as a golang interface.

jtopjian · 2017-07-28T03:51:03Z

+// Interface represents an interface.
+type Interface struct {
+	PortState string    `json:"port_state"`
+	FixedIPs  []FixedID `json:"fixed_ips"`


FixedIPs []FixedIP `json:"fixed_ips"`

jtopjian · 2017-07-28T03:51:33Z

+	MacAddr   string    `json:"mac_addr"`
+}
+
+// InterfacePage abstracts the raw results of making a ListInterfaces() request against the API.


jtopjian · 2017-07-28T03:58:33Z

 		t.Logf("Addresses on %s: %+v", network, address)
 	}

+	allInterfacePages, err := attachinterfaces.ListInterfaces(client, server.ID).AllPages()


coveralls · 2017-07-28T04:31:55Z

Coverage increased (+0.05%) to 71.281% when pulling 8c294ef on FengyunPan:add-interface-list into 574cf4f on gophercloud:master.

jtopjian

one more change. I apologize.

jtopjian · 2017-07-28T04:48:14Z

+	FixedIPs  []FixedIP `json:"fixed_ips"`
+	PortID    string    `json:"port_id"`
+	NetID     string    `json:"net_id"`
+	MacAddr   string    `json:"mac_addr"`


@FengyunPan I'm sorry, I just noticed this.

This should be MACAddr. There are several other areas in Gophercloud where MAC is used, so this will be consistent. It's better to change this now than later.

Yeah, my mistake, I update it soon, thank you very much.

Add support to list interface for a server through a GET on: /v2.1/{tenant_id}/servers/{server_id}/os-interface The same operation with OpenStack CLI is done with: nova interface-list <server_id>

coveralls · 2017-07-28T04:58:57Z

Coverage increased (+0.05%) to 71.281% when pulling d1f42e8 on FengyunPan:add-interface-list into 574cf4f on gophercloud:master.

jtopjian · 2017-07-28T04:59:56Z

Looks good to me! Thank you again for your patience and for contributing this!

This commit increases the Gomago Eventuall timeout to 30 seconds in all tests cases to avoid tests failures by timeout [1]. [1]: https://github.com/Wind-River/cloud-platform-deployment-manager/pull/429/checks?check_run_id=33619644827 Test Case: PASS - make test Signed-off-by: Wallysson Silva <[email protected]>

FengyunPan force-pushed the add-interface-list branch from 52223c6 to 7c33e5b Compare July 22, 2017 08:04

jtopjian mentioned this pull request Jul 24, 2017

Can't Get correct port by ip for server #431

Closed

FengyunPan force-pushed the add-interface-list branch 2 times, most recently from c991229 to 2a59bd5 Compare July 27, 2017 09:01

jtopjian requested changes Jul 28, 2017

View reviewed changes

jtopjian mentioned this pull request Jul 28, 2017

Compute v2: Add Suspend Acceptance Test #440

Merged

FengyunPan force-pushed the add-interface-list branch from 2a59bd5 to 8c294ef Compare July 28, 2017 04:25

jtopjian approved these changes Jul 28, 2017

View reviewed changes

jtopjian requested changes Jul 28, 2017

View reviewed changes

Add Nova interface-list support

d1f42e8

Add support to list interface for a server through a GET on: /v2.1/{tenant_id}/servers/{server_id}/os-interface The same operation with OpenStack CLI is done with: nova interface-list <server_id>

FengyunPan force-pushed the add-interface-list branch from 8c294ef to d1f42e8 Compare July 28, 2017 04:52

jtopjian approved these changes Jul 28, 2017

View reviewed changes

jtopjian merged commit ec29b8a into gophercloud:master Jul 28, 2017

		@@ -0,0 +1,3 @@
		// Package attachinterfaces provides the ability to manage attach interfaces through

Conversation

FengyunPan commented Jul 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jul 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FengyunPan commented Jul 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtopjian commented Jul 22, 2017

Uh oh!

FengyunPan commented Jul 22, 2017

Uh oh!

jtopjian commented Jul 27, 2017

Uh oh!

FengyunPan commented Jul 27, 2017

Uh oh!

coveralls commented Jul 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtopjian left a comment

Choose a reason for hiding this comment

Uh oh!

jtopjian Jul 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jul 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtopjian left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jul 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtopjian commented Jul 28, 2017

Uh oh!

Reviewers

Assignees

Labels

FengyunPan commented Jul 22, 2017 •

edited

Loading

coveralls commented Jul 22, 2017 •

edited

Loading

FengyunPan commented Jul 22, 2017 •

edited

Loading

coveralls commented Jul 27, 2017 •

edited

Loading

jtopjian Jul 28, 2017 •

edited

Loading

coveralls commented Jul 28, 2017 •

edited

Loading

coveralls commented Jul 28, 2017 •

edited

Loading