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

Completely stop/suspend VM, not just partially #206

Merged
merged 1 commit into from
Mar 8, 2018

Conversation

sasoc
Copy link
Contributor

@sasoc sasoc commented Mar 5, 2018

If you stop/suspend VM in vCloud Director UI, it stops/suspends completely. But when calling same action using vCloud API it only stops/suspends VM partially, that is because it stays in deployed state. To reach the same effect as in vCloud Director UI we now use post_undeploy_vapp with 'UndeployPowerAction' set to 'powerOff'/'suspend'

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

@sasoc
Copy link
Contributor Author

sasoc commented Mar 5, 2018

cc @miha-plesko

@sasoc
Copy link
Contributor Author

sasoc commented Mar 5, 2018

before:
partially_powered_off
partially_suspended
after:
powered_off
suspended

Copy link
Contributor

@miha-plesko miha-plesko left a comment

Choose a reason for hiding this comment

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

@sasoc thanks for this awesome PR, I only have two cosmetic comments, otherwise looks really good.

context '.raw_stop' do
it 'stops the virtual machine' do
expect(connection).to receive(:post_undeploy_vapp).with('id', :UndeployPowerAction => 'powerOff').and_return(response)
expect(connection).to receive(:process_task).and_return(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't really need the .and_return(true) part, please feel free to remove it (also below, for suspend).

let(:ems) { FactoryGirl.create(:ems_vmware_cloud) }
let(:vm) { FactoryGirl.create(:vm_vcloud, :ext_management_system => ems, :ems_ref => 'id') }
let(:connection) { double('connection') }
let(:response) { double('response', :body => nil) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpicking, but can you please vertically align {. I know that Rubocop says nothing, but still it's prettier.

@miha-plesko
Copy link
Contributor

@agrare this was a problem for some time now and we didn't know why it happens. Until we recently found this blog that explains the problem behind.

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

@miq-bot add_label enhancement,gaprindashvili/yes
@miq-bot assign @agrare

If you stop/suspend VM in vCloud Director UI, it stops/suspends completely.
But when calling same action using vCloud API it only stops/suspends VM partially,
that is because it stays in `deployed` state. To reach the same effect as in
vCloud Director UI we now use`post_undeploy_vapp` with `UndeployPowerAction'`
set to `'powerOff'`/`'suspend'`

Signed-off-by: sasoc <saso.cvitkovic@gmail.com>
@sasoc sasoc force-pushed the completely-stop-or-suspend-vm branch from acde81f to 4c60f14 Compare March 5, 2018 11:30
@miq-bot
Copy link
Member

miq-bot commented Mar 5, 2018

Checked commit sasoc@4c60f14 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

Copy link
Contributor

@miha-plesko miha-plesko left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

👍 nice @sasoc

@agrare agrare merged commit 30e5ad2 into ManageIQ:master Mar 8, 2018
@agrare agrare added this to the Sprint 81 Ending Mar 12, 2018 milestone Mar 8, 2018
simaishi pushed a commit that referenced this pull request Mar 8, 2018
Completely stop/suspend VM, not just partially
(cherry picked from commit 30e5ad2)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1553389
@simaishi
Copy link
Contributor

simaishi commented Mar 8, 2018

Gaprindashvili backport details:

$ git log -1
commit 47d3dc1ba37f29c5ad120a1bf8269ce41aabeef7
Author: Adam Grare <agrare@redhat.com>
Date:   Thu Mar 8 08:05:46 2018 -0500

    Merge pull request #206 from sasoc/completely-stop-or-suspend-vm
    
    Completely stop/suspend VM, not just partially
    (cherry picked from commit 30e5ad296e526b2a04098d7dda9ec2ce423e8910)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1553389

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