VMware: Add support in VMWare modules for BIOS and instance UUID's

[naphta]

SUMMARY

Adding support for specifying the type of UUID to match against, by default it will continue to use the bios UUID which originally was the only one which worked.

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

lib/ansible/module_utils/vmware.py
lib/ansible/modules/cloud/vmware/vmware_guest_boot_facts.py
lib/ansible/modules/cloud/vmware/vmware_guest_boot_manager.py
lib/ansible/modules/cloud/vmware/vmware_guest_custom_attributes.py
lib/ansible/modules/cloud/vmware/vmware_guest_disk_facts.py
lib/ansible/modules/cloud/vmware/vmware_guest_facts.py
lib/ansible/modules/cloud/vmware/vmware_guest_file_operation.py
lib/ansible/modules/cloud/vmware/vmware_guest_find.py
lib/ansible/modules/cloud/vmware/vmware_guest_move.py
lib/ansible/modules/cloud/vmware/vmware_guest_powerstate.py
lib/ansible/modules/cloud/vmware/vmware_guest_snapshot.py
lib/ansible/modules/cloud/vmware/vmware_guest_snapshot_facts.py
lib/ansible/modules/cloud/vmware/vmware_guest_tools_wait.py
lib/ansible/modules/cloud/vmware/vmware_vm_shell.py
lib/ansible/modules/cloud/vmware/vmware_vmotion.py
test/integration/targets/vmware_guest_facts/tasks/main.yml

ANSIBLE VERSION

ansible 2.7.0.dev0
  config file = None
  configured module search path = [u'/Users/jhill/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python2.7/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 2.7.15 (default, Jun 17 2018, 12:46:58) [GCC 4.2.1 Compatible Apple LLVM 9.1.0 (clang-902.0.39.2)]

ADDITIONAL INFORMATION

I've put this in (and to every referenced module) to increase the options of how you can refer to a VM. In-house where I work we created a lot of VMWare modules before they were available directly from Ansible so in doing this we personally can migrate to Ansible code more easily. Broadly speaking it seemed logical to support either kind of UUID as in PyVmomi it boils down to a true/false to determine which one it searched on.

[ansibot]

cc @Akasurde @bedecarroll @chrrrles @dav1x @imjoseangel @nerzhul @oboukili @pdellaert @ritzk @stravassac @warthog9
click here for bot help

[ansibot]

Hi @naphta,

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

[pdellaert]

Bunch of changes required. I'm also missing test cases for all the modules you have changed (because it would have identified a few issues, i think).

[pdellaert]

Also think that this could be achieved with a boolean instead of a choice attribute, is_instance_uuid for those modules where a separate attribute makes sense (there's a bunch that already have vm_id_type, which should be reused)

cc @Akasurde

[naphta]

Boolean probably makes sense in hindsight (I'm sure vmware are unlikely to add another UUID).
Reusing vm_id_type definitely makes sense. Perhaps something like:

instance_uuid: yes/no

I could also wrap this functionality to be invisible so that it tries the BIOS UUID first then if nothing returns it continues with the instance UUID; I didn't do that originally just because it seemed a bit heavy, but asides from speed concerns it would make the user interface pretty simple. The UUID could also just be prefixed instance-.

My intention is to flesh out all of the test cases I just got the pull request open to start working on any of the already existing issues.

[pdellaert]

@naphta, doing it transparent seems a bit dangerous. It's better, in my opinion, to give the choice. I do like the boolean approach more. I'd suggest use_instance_uuid, as it is more clear that it is a boolean and the purpose. Default false, of course.

Thanks!

[naphta]

Had a quick run at converting it to use a boolean, I'll look at fleshing out the tests in the coming days. I'm not especially familar with the project but are there any tests beyond the integration ones I need to concern myself with?

[naphta]

I couldn't see any other integration tests which refer to the UUID other than the one I already included.

[naphta]

ready_for_review

[naphta]

LGTM, except for some cosmetic/docs changes.

I would also like to see some of the integration tests using this option.

For what it's worth I did add a test in for this using the VMWare mock API included.

[pdellaert]

LGTM

Cool improvement :)

[ansibot]

cc @lparkes
click here for bot help

[Akasurde]

Closing and re-opening for CI trigger.

[ansibot]

cc @Tomorrow9
click here for bot help

[Akasurde]

@naphta Thanks for the contribution. @pdellaert @dagwieers Thanks for the reviews.