bug 1641011: delete snapshot after smartstate analysis

[sseago]

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

For instances launched from volumes, the volume snapshot was
not being deleted after smartstate analysis.

With this PR, it gets deleted if present.

[mansam]

@roliveri Would you mind taking a look at this as soon as you can? It's for a blocker bug.

[JPrause]

@miq-bot add_label blocker

[roliveri]

@sseago Can you address the codeclimate issues?

[sseago]

@roliveri I talked to @mansam about this yesterday. I hadn't done so yet since I was just following the existing style for logging errors here. Should I add the parentheses to the logging calls, even though this doesn't seem to be done anywhere else in the manageiq-smartstate repo? Also, the rescue on error code is exactly what's being done elsewhere too. I don't want this commit to potentially introduce a new bug if the new fog call ends up raising an unexpected exception, causing the operation to blow up. Rescuing StandardError would leave functionality unchanged but would probably satisfy codeclimate -- is that the right answer for cases like this?

[sseago]

Oh, wait, nevermind. I was thinking about the rubocop issues. Looking at codeclimate now...

[sseago]

@roliveri OK, so that mostly fixes it. I still see "Method delete_evm_snapshot has 26 lines of code (exceeds 25 allowed). Consider refactoring."

Basically I'm adding a single "if " to an existing method, and that puts it one line over the limit. While I can make codeclimate happy by moving the if condition into the new method, that actually obscures the logic of the conditional method call, making it harder to see what's going on from looking at the code. Is this OK as-is, or should I move the conditional into the method?

[roliveri]

@sseago This is fine. I'll merge when travis completes.

[sseago]

@roliveri Actually, hold off on merging. I need to confirm that this fix works properly. I thought I tested it before pushing, but it looks like I jumped the gun on the push.

[roliveri]

@sseago OK, ping me when it's ready.

[sseago]

@roliveri another update coming shortly

[sseago]

@roliveri OK, should be good to go now. The refactored method had an error in the logging code, and I forgot to check this before pushing the refactor to the PR. I just confirmed that the delete snapshot is working as expected (and logging as expected) now.

[miq-bot]

Checked commit sseago@c99b013 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 7 offenses detected

lib/OpenStackExtract/MiqOpenStackVm/MiqOpenStackInstance.rb

• ❗ - Line 180, Col 5 - Style/RescueStandardError - Avoid rescuing without specifying an error class.
• ❗ - Line 181, Col 7 - Style/MethodCallWithArgsParentheses - Use parentheses for method calls with arguments.
• ❗ - Line 182, Col 7 - Style/MethodCallWithArgsParentheses - Use parentheses for method calls with arguments.
• ❗ - Line 187, Col 9 - Style/MethodCallWithArgsParentheses - Use parentheses for method calls with arguments.
• ❗ - Line 189, Col 7 - Style/RescueStandardError - Avoid rescuing without specifying an error class.
• ❗ - Line 190, Col 9 - Style/MethodCallWithArgsParentheses - Use parentheses for method calls with arguments.
• ❗ - Line 191, Col 9 - Style/MethodCallWithArgsParentheses - Use parentheses for method calls with arguments.

[aufi]

Thanks for merging!

[aufi]

@roliveri Can we ask for a release including this fix?

[roliveri]

@sseago Done, version 0.2.18