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

Refactor start_clone method and break up powershell functions #14842

Merged
merged 1 commit into from
May 11, 2017
Merged

Refactor start_clone method and break up powershell functions #14842

merged 1 commit into from
May 11, 2017

Conversation

djberg96
Copy link
Contributor

@djberg96 djberg96 commented Apr 21, 2017

This addresses an issue where an invalid provisioning attempt could fail. Instead of a helpful error message, users are getting an obscure JSON parsing error.

What is essentially happening is that the call to New-SCVirtualMachine inside the build_ps_script is failing, with the result that the method returns an empty string because there's no error checking. When JSON then tries to parse an empty string, it bombs.

This PR splits the powershell functions into two separate calls, and we check the results of the first one before moving onto the second.

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

@miq-bot miq-bot added the wip label Apr 21, 2017
@skateman
Copy link
Member

skateman commented May 9, 2017

@djberg96 still having the refresh issue 😞

Ensure methods return a WinRM::Output object.

Explicitly require winrm.

Update spec to reflect returned object change.
@miq-bot
Copy link
Member

miq-bot commented May 10, 2017

Checked commit https://github.com/djberg96/manageiq/commit/8fa3c6bba9db263e335caa7e2cc5e65d4abc706d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. ⭐

@djberg96 djberg96 changed the title [WIP] Refactor start_clone method and break up powershell functions Refactor start_clone method and break up powershell functions May 10, 2017
@djberg96
Copy link
Contributor Author

@jteehan @skateman This should be good to go. You can copy these files over, restart your app and then put it in debug mode to get full output.

For testing I deliberately tried to create a non-HA VM on an HA cluster (which failed with the expected message), tried to create an HA VM on "C:/" (which failed with the expected message) and finally tried to create an HA VM on an HA cluster (which succeeded as expected).

@miq-bot miq-bot removed the wip label May 10, 2017
@blomquisg blomquisg merged commit cb30863 into ManageIQ:master May 11, 2017
@blomquisg blomquisg added this to the Sprint 61 Ending May 22, 2017 milestone May 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants