VMware: vm_guest - allow existing vmdk files to be attached to guest #45953

mrmagooey · 2018-09-21T03:40:37Z

SUMMARY

Allows existing vmdk files to be attached to a new guest vm. Adds the filename parameter to the disk dictionary. Still requires the the datastore to be separately specified as there are many dependencies on this value through the code.

Fixes #30041 , although that issue is for the deprecated vsphere_guest.py module.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

cloud/vmware/vmware_guest.py

ANSIBLE VERSION

2.7.0dev
vsphere vcenter 6.7

ADDITIONAL INFORMATION

Currently there is no method of attaching existing vmdk disks to a new guest vm (as far as I'm aware...). This PR will search for the specified vmdk file, get its size, and create the spec for attaching the existing disk to this vm. The guest vm also seems to be happy booting off a vmdk file attached like this.

A working yml disk definition to add a vmdk file looks like:

 ...
 disk:
   - filename: [datastore_name] path/to/my_disk.vmdk
      datastore: datastore_name

This will need some testing as I don't have a clustered vsphere setup to test on, and that might have some different behaviour that I don't know about.

I also note that there is a separate module being introduced at #36165 which this (should it work properly) might be appropriate to be merged into.

ansibot · 2018-09-21T03:42:16Z

cc @Akasurde @dav1x @warthog9
click here for bot help

ansibot · 2018-09-21T03:42:16Z

Hi @mrmagooey,

Thank you for the pullrequest, 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

ansibot · 2018-09-21T03:52:02Z

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test validate-modules [explain] failed with 14 errors:

lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E324 Value for "default" from the argument_spec ('first') for "name_match" does not match the documentation (None)
lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E324 Value for "default" from the argument_spec ('ha-datacenter') for "datacenter" does not match the documentation (None)
lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E324 Value for "default" from the argument_spec ('present') for "state" does not match the documentation (None)
lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E324 Value for "default" from the argument_spec (443) for "port" does not match the documentation (None)
lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E324 Value for "default" from the argument_spec (True) for "validate_certs" does not match the documentation (False)
lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E325 argument_spec for "force" defines type="bool" but documentation does not
lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E325 argument_spec for "is_template" defines type="bool" but documentation does not
lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E325 argument_spec for "linked_clone" defines type="bool" but documentation does not
lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E325 argument_spec for "validate_certs" defines type="bool" but documentation does not
lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E325 argument_spec for "wait_for_ip_address" defines type="bool" but documentation does not
lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E326 Value for "choices" from the argument_spec (['absent', 'poweredoff', 'poweredon', 'present', 'rebootguest', 'restarted', 'shutdownguest', 'suspended']) for "state" does not match the documentation ([])
lib/ansible/modules/cloud/vmware/vmware_guest.py:0:0: E326 Value for "choices" from the argument_spec (['first', 'last']) for "name_match" does not match the documentation ([])
lib/ansible/modules/cloud/vmware/vmware_guest.py:179:9: E302 DOCUMENTATION is not valid YAML
test/sanity/validate-modules/ignore.txt:487:1: A102 Remove since "lib/ansible/modules/cloud/vmware/vmware_guest.py" passes "E322" test

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

lib/ansible/modules/cloud/vmware/vmware_guest.py:179:9: error DOCUMENTATION: syntax error: sequence entries are not allowed here

click here for bot help

jeking3

This breaks unit numbering.

Akasurde · 2019-01-23T03:28:51Z

@pdellaert @mrmagooey @jeking3 Could you please review this ? Thanks in advance.

jeking3 · 2019-01-23T05:09:03Z

lib/ansible/modules/cloud/vmware/vmware_guest.py

+            # index 7 is reserved to SCSI controller
+            if disk_index == 7:
+                disk_index += 1
+


This should also be capped at 15 based on today's ESXi scsi controller rules, unless we want to just let VMware enforce it with a task error...

I am planning to have that in this PR #33338

test/units/module_utils/test_vmware.py

jeking3 · 2019-01-23T05:11:36Z

Could use an integration test.

Akasurde · 2019-01-23T05:22:03Z

@jeking3 Not sure if VCSIM supports, but will try to add integration test for this change.

Akasurde · 2019-01-23T08:49:15Z

@jeking3 VCSIM does not support Datastore.browser.SearchSubFolders API. I have filed an issue against vcsim - vmware/govmomi#1353

pdellaert

LGTM

Akasurde · 2019-01-30T14:35:55Z

@pdellaert @jeking3 Thanks for the review. @mrmagooey Thanks for the contribution.

While creating new VM, don't assume the VMDKs are present, create them as we attache the disk to VM. Possible regression fix for introduced via ansible#45953 Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

While creating new VM, don't assume the VMDKs are present, create them as we attache the disk to VM. Possible regression fix for introduced via #45953 Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

atodorov · 2019-03-04T11:38:16Z

Hello, is this fix available in any of the versions released on PyPI? From what I can tell it is not.

Akasurde · 2019-03-04T12:00:55Z

@atodorov This is merged in 2.8 release which is supposed to be release soon. Meanwhile you can use https://docs.ansible.com/ansible/latest/dev_guide/testing.html#how-to-test-a-pr document to test this PR.

Signed-off-by: James E. King III <jking@apache.org> Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

devdil · 2019-03-30T12:41:48Z

Even after applying the parameters it says "No size mentioned for disk"

ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Sep 21, 2018

mkrizek removed the needs_triage Needs a first human triage before being processed. label Sep 21, 2018

jeking3 suggested changes Nov 5, 2018

View reviewed changes

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Nov 5, 2018

jeking3 mentioned this pull request Nov 5, 2018

Add support for creating vm with multiple scsi controllers and disks. #47028

Closed

Tomorrow9 mentioned this pull request Nov 8, 2018

vmware_guest: create a VM with RDM backed disk #24837

Closed

Akasurde force-pushed the vmware_guest_add_vmdk_files branch from 3dc2f42 to 9fdbe24 Compare January 23, 2019 02:30

ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jan 23, 2019

Added find_vmdk_file

34cffe6

jeking3 approved these changes Jan 23, 2019

View reviewed changes

pdellaert approved these changes Jan 28, 2019

View reviewed changes

ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 28, 2019

Akasurde merged commit 8eff4ca into ansible:devel Jan 30, 2019

Akasurde mentioned this pull request Feb 5, 2019

VMware: do create VMDK for new VMs #51753

Merged

Tomorrow9 mentioned this pull request Feb 14, 2019

VMware: Add support multiple controllers and disks in vmware_guest #49534

Closed

Akasurde pushed a commit to jeking3/ansible that referenced this pull request Mar 13, 2019

Extend changes in ansible#45953 to vmware_guest_disk

b0c1b0c

Signed-off-by: James E. King III <jking@apache.org> Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

ansible locked and limited conversation to collaborators Jul 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VMware: vm_guest - allow existing vmdk files to be attached to guest #45953

VMware: vm_guest - allow existing vmdk files to be attached to guest #45953

mrmagooey commented Sep 21, 2018

ansibot commented Sep 21, 2018

ansibot commented Sep 21, 2018

ansibot commented Sep 21, 2018

jeking3 left a comment

Akasurde commented Jan 23, 2019

jeking3 Jan 23, 2019

Akasurde Jan 23, 2019

jeking3 commented Jan 23, 2019

Akasurde commented Jan 23, 2019

Akasurde commented Jan 23, 2019 •

edited

pdellaert left a comment

Akasurde commented Jan 30, 2019

atodorov commented Mar 4, 2019

Akasurde commented Mar 4, 2019

devdil commented Mar 30, 2019

VMware: vm_guest - allow existing vmdk files to be attached to guest #45953

VMware: vm_guest - allow existing vmdk files to be attached to guest #45953

Conversation

mrmagooey commented Sep 21, 2018

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ADDITIONAL INFORMATION

ansibot commented Sep 21, 2018

ansibot commented Sep 21, 2018

ansibot commented Sep 21, 2018

jeking3 left a comment

Choose a reason for hiding this comment

Akasurde commented Jan 23, 2019

jeking3 Jan 23, 2019

Choose a reason for hiding this comment

Akasurde Jan 23, 2019

Choose a reason for hiding this comment

jeking3 commented Jan 23, 2019

Akasurde commented Jan 23, 2019

Akasurde commented Jan 23, 2019 • edited

pdellaert left a comment

Choose a reason for hiding this comment

Akasurde commented Jan 30, 2019

atodorov commented Mar 4, 2019

Akasurde commented Mar 4, 2019

devdil commented Mar 30, 2019

Akasurde commented Jan 23, 2019 •

edited