Skip to content

Adds support for setting image metadata for a volume#1621

Merged
jtopjian merged 6 commits intogophercloud:masterfrom
psinghal20:issue-1610
Jun 15, 2019
Merged

Adds support for setting image metadata for a volume#1621
jtopjian merged 6 commits intogophercloud:masterfrom
psinghal20:issue-1610

Conversation

@psinghal20
Copy link
Copy Markdown
Contributor

For #1610
Adds support for setting image metadata for a volume in volumeactions.

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

Signed-off-by: Pratyush singhal <psinghal20@gmail.com>
Signed-off-by: Pratyush singhal <psinghal20@gmail.com>
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 12, 2019

Coverage Status

Coverage increased (+0.006%) to 76.844% when pulling 1e0d7f1 on psinghal20:issue-1610 into 4e17bac on gophercloud:master.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Jun 12, 2019

Build succeeded.

@psinghal20
Copy link
Copy Markdown
Contributor Author

@jtopjian I think this PR is ready. Can you take a look? Thanks!

Copy link
Copy Markdown
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@psinghal20 Just one change with regard to the opts name. Otherwise, this is good to go - nice work.

I created some small code to verify this works, and it does. If you'd like to include an acceptance test for this, I'd be happy to help.

}

// SetImageMetadataOpts contains options for setting image metadata to a volume.
type SetImageMetadataOpts struct {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change this to ImageMetadataOpts.


// SetImageMetdataOptsBuilder allows extensions to add additional parameters to the
// SetImageMetadataRequest request.
type SetImageMetdataOptsBuilder interface {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ImageMetadataOptsBuilder

// SetImageMetdataOptsBuilder allows extensions to add additional parameters to the
// SetImageMetadataRequest request.
type SetImageMetdataOptsBuilder interface {
ToSetImageMetadataMap() (map[string]interface{}, error)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ToImageMetadataMap


// ToSetImageMetadataMap assembles a request body based on the contents of a
// SetImageMetadataOpts.
func (opts SetImageMetadataOpts) ToSetImageMetadataMap() (map[string]interface{}, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

func (opts ImageMetadataOpts) ToImageMetadataMap() (map[string]interface{}, error) {

}

// SetImageMetadata will set image metadata on a volume based on the values in UploadImageOptsBuilder.
func SetImageMetadata(client *gophercloud.ServiceClient, id string, opts SetImageMetdataOptsBuilder) (r SetImageMetadataResult) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

func SetImageMetadata(client *gophercloud.ServiceClient, id string, opts ImageMetdataOptsBuilder) (r SetImageMetadataResult) {

Signed-off-by: Pratyush singhal <psinghal20@gmail.com>
@psinghal20
Copy link
Copy Markdown
Contributor Author

@jtopjian Done with the name changes. I will try working on the acceptance test and ping you if I have any doubts! 🙂

Signed-off-by: Pratyush singhal <psinghal20@gmail.com>
Signed-off-by: Pratyush singhal <psinghal20@gmail.com>
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Jun 13, 2019

Build failed.

@psinghal20
Copy link
Copy Markdown
Contributor Author

@jtopjian I tried adding an acceptance test but it looks like gophercloud-acceptance-test-ironic is failing. Can you help me out with this? Thanks!

Copy link
Copy Markdown
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@psinghal20 I apologize - I wasn't clear with the request for the name change. Please see below and let me know if you have any questions.

As for the acceptance test, it's fine that the ironic test failed. The ironic test only runs ironic/baremetal tests, not the block storage tests. The cause for the failure was probably a temporary issue with OpenLab.

You can see that your new test passed in the gophercloud-acceptance-test suite: https://logs.openlabtesting.org/logs/21/1621/c031459a0aee3895e9b8277203e07cd62d56ff6c/check/gophercloud-acceptance-test/34342c3/test_results.html (search for TestVolumeActionsImageMetadata). Nice work :)

}

// ImageMetadata will set image metadata on a volume based on the values in ImageMetadataOptsBuilder.
func ImageMetadata(client *gophercloud.ServiceClient, id string, opts ImageMetadataOptsBuilder) (r ImageMetadataResult) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I had meant for this to stay SetImageMetadata (#1621 (comment))

The idea is that the opts structure is a "thing" and the function is an "action". We're setting image metadata with the options defined in the image metadata struct, if that makes sense.

Same with the result - that can stay as SetImageMetadataResult because it's the result of the action.

Signed-off-by: Pratyush singhal <psinghal20@gmail.com>
@psinghal20
Copy link
Copy Markdown
Contributor Author

Hey @jtopjian, Thanks for clarification. Referred the styleguide and understood what you meant. Hopefully, this final change closes the deal!

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Jun 14, 2019

Build failed.

Copy link
Copy Markdown
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thank you for working on this and for your patience.

@jtopjian jtopjian merged commit be7c1c0 into gophercloud:master Jun 15, 2019
@psinghal20 psinghal20 deleted the issue-1610 branch June 25, 2019 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants