Add list resource providers API for placement service by dkt26111 · Pull Request #1815 · gophercloud/gophercloud

dkt26111 · 2020-01-10T00:34:09Z

Links to the line numbers/files in the OpenStack source code that support the
code in this PR:

API doc:
https://docs.openstack.org/api-ref/placement/?expanded=#list-resource-providers

API code:
https://github.com/openstack/placement/blob/master/placement/handlers/resource_provider.py#L197

coveralls · 2020-01-10T00:43:02Z

Coverage decreased (-0.005%) to 77.074% when pulling f948e68 on dkt26111:placement_resource_providers into 20a03f8 on gophercloud:master.

teosum · 2020-01-10T00:47:04Z

openstack/placement/resourceproviders/results.go

+	ParentProviderUuid string                  `json:"parent_provider_uuid"`
+	RootProviderUuid   string                  `json:"root_provider_uuid"`


UUID capitalization for variable names.

teosum · 2020-01-10T00:47:32Z

openstack/placement/resourceproviders/testing/fixtures.go

+	ParentProviderUuid: "542df8ed-9be2-49b9-b4db-6d3183ff8ec8",
+	RootProviderUuid:   "542df8ed-9be2-49b9-b4db-6d3183ff8ec8",


UUID capitalization for variable names.

teosum · 2020-01-10T00:47:43Z

openstack/placement/resourceproviders/testing/fixtures.go

+	ParentProviderUuid: "",
+	RootProviderUuid:   "d0b381e9-8761-42de-8e6c-bba99a96d5f5",


UUID capitalization for variable names.

theopenlab-ci · 2020-01-10T02:10:36Z

Build succeeded.

gophercloud-unittest : SUCCESS in 3m 12s
gophercloud-acceptance-test : SUCCESS in 1h 05m 37s
gophercloud-acceptance-test-ironic : SUCCESS in 23m 13s

dkt26111 · 2020-01-10T17:42:42Z

@jtopjian This is ready for review. Thanks.

theopenlab-ci · 2020-01-10T18:30:00Z

Build succeeded.

gophercloud-unittest : SUCCESS in 3m 21s
gophercloud-acceptance-test : SUCCESS in 1h 05m 06s
gophercloud-acceptance-test-ironic : SUCCESS in 44m 07s

jtopjian

@dkt26111 Thank you for working on this 🙂

This all looks good. I've left a few minor comments. In addition, the files need slightly reorganized:

acceptance/openstack/placement/v1
openstack/placement/v1/resourceproviders

Please let me know if you have any questions.

jtopjian · 2020-01-11T04:43:22Z

acceptance/clients/clients.go

+// NewPlacementClient returns a *ServiceClient for making calls
+// to the OpenStack Placement API. An error will be returned
+// if authentication or client creation was not possible.
+func NewPlacementClient() (*gophercloud.ServiceClient, error) {


NewPlacementV1Client

jtopjian · 2020-01-11T04:43:35Z

openstack/client.go

 }
+
+// NewPlacement creates a ServiceClient that may be used with the placement package.
+func NewPlacement(client *gophercloud.ProviderClient, eo gophercloud.EndpointOpts) (*gophercloud.ServiceClient, error) {


NewPlacementV1

jtopjian · 2020-01-11T04:44:30Z

openstack/placement/resourceproviders/doc.go

+
+	for _, r := range allResourceProviders {
+        fmt.Printf("%+v\n", r)
+	}


nit: the formatting of the above example code looks a little off. godoc depends on correct formatting or else it wouldn't be a big deal.

You are right. I'll fix that.

jtopjian · 2020-01-11T04:45:05Z

openstack/placement/resourceproviders/requests.go

+	UUID string `q:"uuid"`
+
+	// Member_of is a string representing aggregate uuids to filter or exclude from the list
+	Member_of string `q:"member_of"`


jtopjian · 2020-01-11T04:45:14Z

openstack/placement/resourceproviders/requests.go

+
+	// In_tree is a string that represents a resource provider UUID.  The returned resource
+	// providers will be in the same provider tree as the specified provider.
+	In_tree string `q:"in_tree"`


jtopjian · 2020-01-11T04:52:47Z

openstack/placement/resourceproviders/results.go

+	Links              []ResourceProviderLinks `json:"links"`
+	Name               string                  `json:"name"`
+	ParentProviderUUID string                  `json:"parent_provider_uuid"`
+	RootProviderUUID   string                  `json:"root_provider_uuid"`


It looks like the above two fields are part of a microversion?

https://github.com/openstack/placement/blob/master/placement/handlers/resource_provider.py#L56-L58

According to our microversion rules, these should have their own Extract methods. However, Placement is still a relatively new service and I think 1.14 should be an early enough release that we can consider that part of "base".

It might be worthwhile to add a comment about this, though:

// ParentProviderUUID contains the UUID of the parent provider. // Requires microversion 1.14 or above. ParentProviderUUID string `json:"parent_provider_uuid"` // RootProviderUUID contains the UUID of the root provider. // Requires microversion 1.14 or above. ParentProviderUUID string `json:"parent_provider_uuid"`

Sure, I'll add the comments.

theopenlab-ci · 2020-01-13T19:29:52Z

Build failed.

gophercloud-unittest : SUCCESS in 2m 27s
gophercloud-acceptance-test : FAILURE in 1h 37m 22s
gophercloud-acceptance-test-ironic : SUCCESS in 40m 53s

dkt26111 · 2020-01-13T19:34:38Z

@jtopjian Ready for review again.
The acceptance test failure is unrelated to placement.

jtopjian

LGTM - thank you!

Please see the comment about the acceptance test, just as a heads up.

jtopjian · 2020-01-14T00:55:30Z

acceptance/openstack/placement/v1/resourceproviders_test.go

+	th "github.com/gophercloud/gophercloud/testhelper"
+)
+
+func TestResourceProviderList(t *testing.T) {


I'm getting a 403 error when I try to run this test as a normal user -- I think this requires admin access?

If so, then we need to add:

clients.RequireAdmin(t)

at the very beginning of this test function.

I'll make this change right after I merge this PR.

Good catch. In fact all placement APIs appear to be restricted to admin by default:
https://docs.openstack.org/placement/latest/configuration/policy.html

I usually use admin user when running the acceptance tests so I didn't notice it.

Add list resource providers API for placement service

b590d76

teosum reviewed Jan 10, 2020

View reviewed changes

fix variable capitalization

cda2c1b

remove change in go.mod

23c6972

jtopjian requested changes Jan 11, 2020

View reviewed changes

Fix review comments

f948e68

jtopjian approved these changes Jan 14, 2020

View reviewed changes

jtopjian merged commit 75e9c9c into gophercloud:master Jan 14, 2020

		ParentProviderUuid string `json:"parent_provider_uuid"`
		RootProviderUuid string `json:"root_provider_uuid"`

		ParentProviderUuid: "542df8ed-9be2-49b9-b4db-6d3183ff8ec8",
		RootProviderUuid: "542df8ed-9be2-49b9-b4db-6d3183ff8ec8",

		ParentProviderUuid: "",
		RootProviderUuid: "d0b381e9-8761-42de-8e6c-bba99a96d5f5",

Conversation

dkt26111 commented Jan 10, 2020

Uh oh!

coveralls commented Jan 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

theopenlab-ci bot commented Jan 10, 2020

Uh oh!

dkt26111 commented Jan 10, 2020

Uh oh!

theopenlab-ci bot commented Jan 10, 2020

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!

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!

theopenlab-ci bot commented Jan 13, 2020

Uh oh!

dkt26111 commented Jan 13, 2020

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Jan 10, 2020 •

edited

Loading