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

VMware: Fix vmware_guest cloning bug #58737

Merged
merged 3 commits into from Jul 15, 2019

Conversation

@nagonzalez
Copy link
Contributor

commented Jul 4, 2019

SUMMARY

Fixes #58721, #56861, #55551

VM cloning in 2.8 tries to create new disks even if they already exist in the original template.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

vmware_guest

ADDITIONAL INFORMATION

This PR adds a conditional check that only creates new virtual devices if they aren't based off an existing disk.

@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2019

@nagonzalez, just so you are aware we have a dedicated Working Group for vmware.
You can find other people interested in this in #ansible-vmware on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@Akasurde Akasurde changed the title fix vmware_guest cloning bug VMware: Fix vmware_guest cloning bug Jul 5, 2019

@Akasurde Akasurde requested review from goneri and jillr Jul 5, 2019

@Akasurde Akasurde self-assigned this Jul 5, 2019

@Akasurde Akasurde removed the needs_triage label Jul 5, 2019

@Akasurde

This comment has been minimized.

Copy link
Member

commented Jul 5, 2019

@nagonzalez Thanks for PR. Could you please add a test case for this change here ? Thanks.

@Akasurde

This comment has been minimized.

Copy link
Member

commented Jul 5, 2019

@Tomorrow9 Could you please review this ? Thanks in advance

nagonzalez added some commits Jul 6, 2019

@nagonzalez

This comment has been minimized.

Copy link
Contributor Author

commented Jul 6, 2019

@Akasurde added test case. Tried to follow the existing standard as far as I could tell. Let me know if I need to change or add anything else.

@Akasurde

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

@nagonzalez Thanks for the tests and fix. @Tomorrow9 Could you please review this ? Thanks.

@goneri

goneri approved these changes Jul 12, 2019


- assert:
that:
- l_clone_template_modify_disks.changed | bool

This comment has been minimized.

Copy link
@goneri

goneri Jul 12, 2019

Contributor

You can just do: - l_clone_template_modify_disks is changed

autoselect_datastore: True
template: clone_resize_disks_original
state: poweredoff
register: l_clone_template_modify_disks

This comment has been minimized.

Copy link
@goneri

goneri Jul 12, 2019

Contributor

If you've the opportunity, it would be nice to reduce the length of the variable, this will improve the readability a bit.

@jillr

jillr approved these changes Jul 12, 2019

@Akasurde Akasurde merged commit 3a5d13b into ansible:devel Jul 15, 2019

1 check passed

Shippable Run 131036 status is SUCCESS.
Details
@Tomorrow9

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2019

@nagonzalez Thanks for the tests and fix. @Tomorrow9 Could you please review this ? Thanks.

Hi @Akasurde, sorry for not response in time, I was on vacation last week.

@realged13

This comment has been minimized.

Copy link

commented Jul 18, 2019

I have confirmed that this has been fixed as well.

@Akasurde

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

@realged13 Thanks for confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.