Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle attempts to delete volumes that have already been deleted #147

Merged
merged 3 commits into from
Nov 29, 2017

Conversation

mansam
Copy link
Contributor

@mansam mansam commented Nov 7, 2017

This handles an exception that would occur when attempting to delete
a volume that was already deleted from the provider, but the deletion
was not yet reflected in ManageIQ. The exception is now caught and
a nice error message is shown.

https://bugzilla.redhat.com/show_bug.cgi?id=1487234

@mansam mansam force-pushed the handle-volume-double-deletion-error branch from 222af39 to 9081b6b Compare November 7, 2017 21:15
# NoMethodError here usually means the above block got a nil while checking the volume
# status, probably because the user already deleted the volume. Assume that's the case
# and show a reassuring message.
rescue NoMethodError
Copy link
Member

Choose a reason for hiding this comment

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

Hi @mansam , thanks for the PR!

I think there can be other cases when NoMethodError is raised than Volume already queued for deletion.

We can merge this in case of emergency, but I'd like discuss changing the line with_provider_object(&:status) == "in-use" to stop connecting to OpenStack and reading the status information from MIQ database record instead. Do you know if this would be possible? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably possible, I'll try it and see if I run into any problems.

This handles an exception that would occur when attempting to delete
a volume that was already deleted from the provider, but the deletion
was not yet reflected in ManageIQ. The exception is now caught and
a nice error message is shown.
@mansam mansam force-pushed the handle-volume-double-deletion-error branch from effb3d4 to 4bbbe1c Compare November 17, 2017 16:53
@miq-bot
Copy link
Member

miq-bot commented Nov 17, 2017

Checked commits mansam/manageiq-providers-openstack@d4229d7~...4bbbe1c with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@aufi
Copy link
Member

aufi commented Nov 29, 2017

Looks good to me, thanks!

@aufi aufi merged commit 14fd2f9 into ManageIQ:master Nov 29, 2017
@aufi aufi added the bug label Nov 29, 2017
@aufi aufi added this to the Sprint 75 Ending Dec 11, 2017 milestone Nov 29, 2017
@aufi
Copy link
Member

aufi commented Nov 29, 2017

It looks that backport is not needed from the BZ, but feel free to update it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants