VMware: New module: vmware_guest_disk_facts

[Akasurde]

SUMMARY

Signed-off-by: Abhijeet Kasurde akasurde@redhat.com

ISSUE TYPE

• New Module Pull Request

COMPONENT NAME

lib/ansible/modules/cloud/vmware/vmware_guest_disk_facts.py
test/integration/targets/vmware_guest_disk_facts/aliases
test/integration/targets/vmware_guest_disk_facts/tasks/main.yml

ANSIBLE VERSION

2.6devel

[ansibot]

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

test/integration/targets/vmware_guest_disk_facts/tasks/main.yml:55:1: empty-lines too many blank lines (1 > 0)

click here for bot help

[ansibot]

cc @Akasurde @dav1x @jctanner @nerzhul
click here for bot help

[ansibot]

@Akasurde @bedecarroll @chrrrles @dav1x @garbled1 @jjahns @kamsz @lrivallain @nafpliot-ibm @nerzhul @pdellaert @rhoop @ritzk @stravassac @tchernomax @woshihaoren

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

[tchernomax]

backing_eagerlyscrub=bool(disk.backing.eagerlyScrub),
?

[Akasurde]

👍
This will happen when you overthink.

[tchernomax]

+    # VM already exists
+    if vm:
+        …

→

+    if vm:
+        # VM exists
+        …

But that's a matter of taste.

[Akasurde]

ok. You know just a copy-paste.

[tchernomax]

ping @Akasurde

[tchernomax]

LGTM

[Akasurde]

@tchernomax Thanks for your valuable review comments. 👍

[phemmer]

datastore.name requires access to the datastore itself, and thus might throw a permission error. This value should be wrapped in a try/except.

[Akasurde]

OK. I will added try/except for this.

[phemmer]

This should be a list, not a dictionary. The dictionary key is just an incrementing index value, which is what a list is for. Having it as a dictionary just makes it harder to use (having to deal with an index generator, and dictionaries don't guarantee order).

[Akasurde]

Sorry, but I forgot the reason why I implemented dict instead of list for returning facts. If you feel that list is more appropriate than dict, then we can always refactor this code again. PR welcomed.

[pdellaert]

@phemmer , We've discussed this in the weekly Ansible VMware meeting.
In my opinion, having a dict is useful, IF the key is useful:

• The reason to have facts is to use them in a task later on
• With a list, to find the proper disk, it is harder to find the item you want to match on ( {{ (disks | selectattr('label', 'search', 'Hard disk 1') | list | first) }} for instance)
• With a dict, if the key is useful, it doesn't require that. If you want to match on another criteria, the loop is still a possibility.

Proposal is to have the label as the dict key as it is what is visible in vCenter and is orderable, does that make sense to you? Any other criteria you might find easier? (disk.key is another possibility, but less 'visible' in vCenter)

[Akasurde]

@phemmer @pdellaert With current implementation, I am able to add new disk to an existing virtual machine -

---
- name: Add a disk to already existing VM
  gather_facts: no
  vars_files:
    - vcenter_vars.yml
  hosts: localhost
  tasks:
    - set_fact:
        vm_name: "VM_2262"

    - name: "Getting information about VM {{ vm_name }}"
      vmware_guest_disk_facts:
        hostname: "{{ vcenter_server }}"
        username: "{{ vcenter_user }}"
        password: "{{ vcenter_pass }}"
        validate_certs: no
        datacenter: "{{ datacenter }}"
        name: "{{ vm_name }}"
      register: no_vm_result

    - set_fact:
        disk: "{{ disk | default([]) + [{ 'datastore' : item.value.backing_datastore, 'size_kb': item.value.capacity_in_kb }] }}"
      with_dict: "{{ no_vm_result.guest_disk_facts }}"

    - debug:
        var: disk

    - set_fact:
        new_disk: "{{ disk + [{ 'datastore': 'datastore2', 'size_kb': '21504'}] }}"

    - debug:
        var: new_disk

    - name: "Adding new disk to existing VM {{ vm_name }}"
      vmware_guest:
        hostname: "{{ vcenter_server }}"
        username: "{{ vcenter_user }}"
        password: "{{ vcenter_pass }}"
        validate_certs: no
        datacenter: "{{ datacenter }}"
        esxi_hostname: "{{ esxi_server }}"
        state: present
        folder: /Site2/DC2/vm
        name: "{{ vm_name }}"
        disk: "{{ new_disk }}"

[Akasurde]

For posterity,

https://github.com/Akasurde/ansible-reproducers/tree/master/36026