Skip to content

Image Service v2: Add Updating image.Protected field#2221

Merged
jtopjian merged 1 commit intogophercloud:masterfrom
dustymabe:dusty-image-update-protected
Sep 30, 2021
Merged

Image Service v2: Add Updating image.Protected field#2221
jtopjian merged 1 commit intogophercloud:masterfrom
dustymabe:dusty-image-update-protected

Conversation

@dustymabe
Copy link
Copy Markdown
Contributor

Now we should be able to do something like:

updateOpts := images.UpdateOpts{
    images.ReplaceImageProtected{
        NewProtected: false,
    },
}
image, err = images.Update(client, imageID, updateOpts).Extract()

Fixes #2220

@dustymabe
Copy link
Copy Markdown
Contributor Author

I'm confident in this change working but not confident in my changes to the tests, can someone please help me verify those changes are appropriate and fail if tweaked. For some reason locally I couldn't get them to fail even if I put in bad values.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Sep 24, 2021

Build failed.

@dustymabe dustymabe force-pushed the dusty-image-update-protected branch 2 times, most recently from 9e5f6db to a08f681 Compare September 24, 2021 02:35
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Sep 24, 2021

Build succeeded.

@dustymabe
Copy link
Copy Markdown
Contributor Author

dustymabe commented Sep 24, 2021

Build succeeded.

🎉

@kayrus
Copy link
Copy Markdown
Contributor

kayrus commented Sep 24, 2021

@dustymabe can you extend functional test to handle set true -> set false checks?

@dustymabe
Copy link
Copy Markdown
Contributor Author

@dustymabe can you extend functional test to handle set true -> set false checks?

which file is that in?

@jtopjian
Copy link
Copy Markdown
Contributor

@dustymabe @kayrus is suggesting to either modify the current acceptance test or create a new one that flips protection on and off and confirm the value is correct each time.

@jtopjian
Copy link
Copy Markdown
Contributor

I'm confident in this change working but not confident in my changes to the tests, can someone please help me verify those changes are appropriate and fail if tweaked. For some reason locally I couldn't get them to fail even if I put in bad values.

Looks like you've got the hang of it 🙂

I noticed with the first OpenLab failure, it was due to Glance not wanting to delete a protected image, so this change does work. It's just a matter of extending the test a little bit and we should be good to go. Thanks for working on this.

Now we should be able to do something like:

```
updateOpts := images.UpdateOpts{
    images.ReplaceImageProtected{
        NewProtected: false,
    },
}
image, err = images.Update(client, imageID, updateOpts).Extract()
```

Fixes gophercloud#2220
@dustymabe dustymabe force-pushed the dusty-image-update-protected branch from a08f681 to 89d7c3f Compare September 27, 2021 13:34
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Sep 27, 2021

Build succeeded.

@dustymabe
Copy link
Copy Markdown
Contributor Author

Yay. worked this time!

@jtopjian
Copy link
Copy Markdown
Contributor

To fulfill the PR requirement of showing the API service code that allows this change: according to the Glance code, it looks like all Image attributes are able to be updated, so this change is good.

https://github.com/openstack/glance/blob/25e60aea8e2fe11937e4b4d94e89c35cbb43940e/glance/api/v2/images.py#L640-L661

@jtopjian
Copy link
Copy Markdown
Contributor

@dustymabe Let me know if you're ready to have this reviewed/merged.

@dustymabe
Copy link
Copy Markdown
Contributor Author

@dustymabe Let me know if you're ready to have this reviewed/merged.

+1 ready for review/merge

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!

@jtopjian jtopjian merged commit 7749e11 into gophercloud:master Sep 30, 2021
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.

image service v2 should support updating image protection status

3 participants