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

Support update VM snapshot #205

Merged
merged 1 commit into from
Mar 14, 2018
Merged

Conversation

sasoc
Copy link
Contributor

@sasoc sasoc commented Mar 2, 2018

Create new snapshot was disabled when VM already had an existing snapshot. VCloud only supports having one snapshot per VM, but you can still create new snapshot, which updates the existing one. So now VM supports snapshot_create even when it already has a snapshot.

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

@sasoc
Copy link
Contributor Author

sasoc commented Mar 2, 2018

cc @miha-plesko

@miha-plesko
Copy link
Contributor

@sasoc can you please add a simple unit test for this? Otherwise it looks good to me - was really strange that snapshoting got disabled after user created the first snapshot.

@sasoc sasoc force-pushed the support-update-vm-snapshot branch from ca71cda to 0651d7a Compare March 5, 2018 10:39
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 one spec case more please, then my thumb is up.


it 'supports snapshot create' do
expect(vm.supports_snapshot_create?).to be_truthy
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add another test case where you first manually add one snapshot to the vm prior checking for supports_snapshot_create?. Just to make sure your supports did override the one from core.

@sasoc sasoc force-pushed the support-update-vm-snapshot branch from 0651d7a to 6e2cb29 Compare March 5, 2018 12:11
expect(vm.supports_snapshot_create?).to be_truthy
end

it 'supports snapshot update' do
Copy link
Contributor

Choose a reason for hiding this comment

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

it 'supports snapshot create (for second snapshot)' do

Create new snapshot was disabled when VM already had an existing snapshot.
VCloud only supports having one snapshot per VM, but you can still create
new snapshot, which updates the existing one. So now VM supports
`snapshot_create` even when it already has a snapshot.

Signed-off-by: sasoc <[email protected]>
@sasoc sasoc force-pushed the support-update-vm-snapshot branch from 6e2cb29 to 3073ae5 Compare March 5, 2018 12:24
@miq-bot
Copy link
Member

miq-bot commented Mar 5, 2018

Checked commit sasoc@3073ae5 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

@agrare agrare self-assigned this Mar 8, 2018
@agrare agrare merged commit ba01933 into ManageIQ:master Mar 14, 2018
@agrare agrare added this to the Sprint 82 Ending Mar 26, 2018 milestone Mar 14, 2018
@miha-plesko
Copy link
Contributor

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

@miq-bot add_label gaprindashvili/yes

simaishi pushed a commit that referenced this pull request Apr 16, 2018
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 41282ebcc27b2bac02f6034a5d2211365415a01e
Author: Adam Grare <[email protected]>
Date:   Wed Mar 14 13:36:11 2018 -0400

    Merge pull request #205 from sasoc/support-update-vm-snapshot
    
    Support update VM snapshot
    (cherry picked from commit ba019333f0222d2570c89b342cea65c1f4ff7407)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1567962

agrare pushed a commit to agrare/manageiq-providers-vmware that referenced this pull request Apr 15, 2019
…approval2

Auto approval for instance resize operation.
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