Added Shelve, Shelve-offload and Unshelve API by qhua948 · Pull Request #1799 · gophercloud/gophercloud

qhua948 · 2019-12-10T03:32:43Z

Prior to starting a PR, please make sure you have read our
contributor tutorial.

Prior to a PR being reviewed, there needs to be a Github issue that the PR
addresses. Replace the brackets and text below with that issue number.

For #1798

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

https://docs.openstack.org/api-guide/compute/server_concepts.html
https://docs.openstack.org/api-ref/compute/?expanded=shelve-server-shelve-action-detail,unshelve-restore-shelved-server-unshelve-action-detail,shelf-offload-remove-server-shelveoffload-action-detail

coveralls · 2019-12-10T03:44:45Z

Coverage decreased (-0.03%) to 76.968% when pulling 231b8a5 on qhua948:shelve-unshelve into 7aa2e52 on gophercloud:master.

theopenlab-ci · 2019-12-10T03:50:35Z

Build failed.

gophercloud-unittest : SUCCESS in 3m 19s
gophercloud-acceptance-test : FAILURE in 14m 15s
gophercloud-acceptance-test-ironic : FAILURE in 13m 25s

qhua948 · 2019-12-10T03:53:03Z

recheck

theopenlab-ci · 2019-12-10T04:06:08Z

Build failed.

gophercloud-unittest : SUCCESS in 3m 15s
gophercloud-acceptance-test : FAILURE in 12m 30s
gophercloud-acceptance-test-ironic : FAILURE in 1m 51s

jtopjian

@qhua948 Thank you for working on this! You can ignore the OpenLab failures as there's an ongoing issue right now.

Note that while the API documentation is helpful, we need to see the actual Nova API service code that implements shelving and unshelving. The contributor tutorial (linked in this PR) has examples here.

I've left a few other minor comments. Please let me know if you have any questions.

openstack/compute/v2/extensions/shelveunshelve/requests.go

openstack/compute/v2/extensions/shelveunshelve/testing/doc.go

qhua948 · 2019-12-10T05:05:50Z

@jtopjian Thanks, I'll work on this later.

theopenlab-ci · 2019-12-10T20:33:17Z

Build failed.

gophercloud-unittest : SUCCESS in 2m 30s
gophercloud-acceptance-test : FAILURE in 12m 29s
gophercloud-acceptance-test-ironic : FAILURE in 7m 58s

jtopjian · 2019-12-10T21:18:13Z

@qhua948 Thank you for digging up the API code - it's a pain, but it can sometimes identify areas that aren't reflected in the docs. For this PR, the docs match the API code, so the exercise feels moot, but there's been enough occasions worth the effort.

It looks like the unshelve action can take an optional parameter of availability_zone:

https://github.com/openstack/nova/blob/17b5a1ab85c358a1096788371849f0cccfe84972/nova/api/openstack/compute/shelve.py#L79-L81

We'll then need to add a struct called UnshelveOpts with AvailabilityZone field. Then we'll need to make sure the struct is conditionally sent only if it's specified, and null when not, in order to be backwards compatible.

No doubt that's a bit more work than you anticipated by wanting to add support for basic un/shelving. I can add the AvailabilityZone feature after this PR is merged. Just a heads up that the function signature for Unshelve will be changed shortly after this is merged, in case you wanted to use this feature right away.

theopenlab-ci · 2019-12-10T21:33:05Z

Build failed.

gophercloud-unittest : SUCCESS in 2m 58s
gophercloud-acceptance-test : FAILURE in 8m 46s
gophercloud-acceptance-test-ironic : FAILURE in 8m 38s

qhua948 · 2019-12-10T21:37:40Z

Thanks for pointing it out, for the sake of completeness, I should add that in as well.

jtopjian · 2019-12-10T22:36:26Z

@qhua948 let me know if you need any help. A rough draft of the conditional logic is going to be something like:

func (opts UnshelveOpts) ToShelveUnshelveMap() (map[string]interface{}, error) {
  b, err := gophercloud.BuildRequestBody(opts, "")
  if err != nil {
    return nil, err
  }

  unshelveOpts := make(map[string]interface{})
  if b["availability_zone"] != "" {
    unshelveOpts["availability_zone"] = b["availability_zone"]
  }

  return unshelveOpts
}

func Unshelve(client *gophercloud.ServiceClient, id string, opts ShelveUnshelveOptsBuilder) (r UnshelveResult) {
  b, err := opts.ToShelveUnshelveOpts()
  if err != nil {
    r.Err = err
    return
  }

  _, r.Err = client.Post(extensions.ActionURL(client, id), map[string]interface{}{"unshelve": b}, nil, nil)
  return
}

I might be wrong about make, though - a unit test will confirm if that will end up sending nil or {}.

qhua948 · 2019-12-10T22:54:58Z

Schema

https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/shelve.py

theopenlab-ci · 2019-12-11T00:34:57Z

Build failed.

gophercloud-unittest : SUCCESS in 3m 05s
gophercloud-acceptance-test : FAILURE in 12m 03s
gophercloud-acceptance-test-ironic : FAILURE in 8m 18s

theopenlab-ci · 2019-12-11T00:50:22Z

Build failed.

gophercloud-unittest : SUCCESS in 3m 14s
gophercloud-acceptance-test : FAILURE in 7m 31s
gophercloud-acceptance-test-ironic : FAILURE in 8m 12s

jtopjian

@qhua948 This looks really good - thank you for your work and patience.

Just a few minor fixes and this is good to go.

jtopjian · 2019-12-11T03:55:20Z

openstack/compute/v2/extensions/shelveunshelve/requests.go

+	ToUnshelveMap() (map[string]interface{}, error)
+}
+
+// ShelveOffloadOpts specifies parameters of shelve-offload action.


UnshelveOpts

jtopjian · 2019-12-11T03:55:33Z

openstack/compute/v2/extensions/shelveunshelve/requests.go

+}
+
+// ShelveOffloadOpts specifies parameters of shelve-offload action.
+type UnshelvedOpts struct {


UnshelveOpts

minor nit: Unshelved implies actions after unshelving has happened.

jtopjian · 2019-12-11T03:56:22Z

openstack/compute/v2/extensions/shelveunshelve/doc.go

@@ -0,0 +1,24 @@
+/*
+Package startstop provides functionality to start and stop servers that have


shelveunshelve

jtopjian · 2019-12-11T03:56:41Z

openstack/compute/v2/extensions/shelveunshelve/doc.go

+
+	serverID := "47b6b7b7-568d-40e4-868c-d5c41735532e"
+
+	err := startstop.Shelve(computeClient, serverID).ExtractErr()


err := shelveunshelve.Shelve(computeClient, serverID).ExtractErr()

jtopjian · 2019-12-11T03:57:01Z

openstack/compute/v2/extensions/shelveunshelve/doc.go

+		panic(err)
+	}
+
+	err := startstop.ShelveOffload(computeClient, serverID).ExtractErr()


err := shelveunshelve.ShelveOffload(computeClient, serverID).ExtractErr()

jtopjian · 2019-12-11T03:57:22Z

openstack/compute/v2/extensions/shelveunshelve/doc.go

+		panic(err)
+	}
+
+	err := startstop.Unshelve(computeClient, serverID).ExtractErr()


err := shelveunshelve.Unshelve(computeClient, serverID, nil).ExtractErr()

jtopjian · 2019-12-11T03:58:58Z

openstack/compute/v2/extensions/shelveunshelve/requests.go

+	AvailabilityZone string `json:"availability_zone,omitempty"`
+}
+
+func (opts UnshelvedOpts) ToUnshelveMap() (map[string]interface{}, error) {


Note for posterity if anyone comes across this:

Our usual naming pattern of To-Package-Action-Map doesn't work well here. ToShelveUnshelveUnshelveMap just looks weird.

ToUnshelveMap is fine.

theopenlab-ci · 2019-12-11T19:53:40Z

Build failed.

gophercloud-unittest : SUCCESS in 3m 14s
gophercloud-acceptance-test : FAILURE in 13m 13s
gophercloud-acceptance-test-ironic : FAILURE in 9m 07s

jtopjian

LGTM - thank you!

qhua948 changed the title ~~Added Shelve, Shelve-offload and Unshelve API~~ [WIP] Added Shelve, Shelve-offload and Unshelve API Dec 10, 2019

qhua948 changed the title ~~[WIP] Added Shelve, Shelve-offload and Unshelve API~~ Added Shelve, Shelve-offload and Unshelve API Dec 10, 2019

jtopjian requested changes Dec 10, 2019

View reviewed changes

openstack/compute/v2/extensions/shelveunshelve/requests.go Outdated Show resolved Hide resolved

openstack/compute/v2/extensions/shelveunshelve/testing/doc.go Outdated Show resolved Hide resolved

Added Shelve, Shelve-offload and Unshelve API

3b1cde0

qhua948 force-pushed the shelve-unshelve branch from 39da48e to 3b1cde0 Compare December 10, 2019 20:20

Refractored actionURL and move the function into compute/v2/extensions

97d643d

qhua948 force-pushed the shelve-unshelve branch from c3b988b to ee1d4d1 Compare December 11, 2019 00:22

Added support for optional availability_zone argument for unshelve

3100fb5

qhua948 force-pushed the shelve-unshelve branch from ee1d4d1 to 3100fb5 Compare December 11, 2019 00:41

qhua948 requested a review from jtopjian December 11, 2019 03:25

jtopjian requested changes Dec 11, 2019

View reviewed changes

Fixed up shelveunshelve doc.go and renamed UnshelvedOpts -> UnshelveOpts

231b8a5

qhua948 requested a review from jtopjian December 11, 2019 19:46

jtopjian approved these changes Dec 11, 2019

View reviewed changes

jtopjian merged commit bfa5322 into gophercloud:master Dec 11, 2019

jtopjian mentioned this pull request Dec 11, 2019

Support for SHELVE/UNSHELVE #1798

Closed

		@@ -0,0 +1,24 @@
		/*
		Package startstop provides functionality to start and stop servers that have


		serverID := "47b6b7b7-568d-40e4-868c-d5c41735532e"

		err := startstop.Shelve(computeClient, serverID).ExtractErr()

Conversation

qhua948 commented Dec 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Controllers

APIs

Schema

Uh oh!

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

Uh oh!

Uh oh!

theopenlab-ci bot commented Dec 10, 2019

Uh oh!

qhua948 commented Dec 10, 2019

Uh oh!

theopenlab-ci bot commented Dec 10, 2019

Uh oh!

jtopjian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

qhua948 commented Dec 10, 2019

Uh oh!

theopenlab-ci bot commented Dec 10, 2019

Uh oh!

jtopjian commented Dec 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theopenlab-ci bot commented Dec 10, 2019

Uh oh!

qhua948 commented Dec 10, 2019

Uh oh!

jtopjian commented Dec 10, 2019

Uh oh!

qhua948 commented Dec 10, 2019

Schema

Uh oh!

theopenlab-ci bot commented Dec 11, 2019

Uh oh!

theopenlab-ci bot commented Dec 11, 2019

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!

theopenlab-ci bot commented Dec 11, 2019

Uh oh!

jtopjian left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

qhua948 commented Dec 10, 2019 •

edited

Loading

coveralls commented Dec 10, 2019 •

edited

Loading

jtopjian commented Dec 10, 2019 •

edited

Loading