VMware: refactor configure disk logic (#36617)

This fix corrects logic related to disk parameters.

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

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

@@ -1369,28 +1369,45 @@ class PyVmomiHelper(PyVmomi):
     def get_configured_disk_size(self, expected_disk_spec):
         # what size is it?
         if [x for x in expected_disk_spec.keys() if x.startswith('size_') or x == 'size']:
-            # size_tb, size_gb, size_mb, size_kb, size_b ...?
+            # size, size_tb, size_gb, size_mb, size_kb
             if 'size' in expected_disk_spec:
-                expected = ''.join(c for c in expected_disk_spec['size'] if c.isdigit())
+                size_regex = re.compile(r'(\d+(?:\.\d+)?)([tgmkTGMK][bB])')
-                unit = expected_disk_spec['size'].replace(expected, '').lower()
+                disk_size_m = size_regex.match(expected_disk_spec['size'])
+                try:
+                    if disk_size_m:
+                        expected = disk_size_m.group(1)
+                        unit = disk_size_m.group(2)
+                    else:
+                        raise ValueError
+
+                    if re.match(r'\d+\.\d+', expected):
+                        # We found float value in string, let's typecast it
+                        expected = float(expected)
+                    else:
+                        # We found int value in string, let's typecast it
                         expected = int(expected)
+
+                    if not expected or not unit:
+                        raise ValueError
+
+                except (TypeError, ValueError, NameError):
+                    # Common failure
+                    self.module.fail_json(msg="Failed to parse disk size please review value"
+                                              " provided using documentation.")
             else:
                 param = [x for x in expected_disk_spec.keys() if x.startswith('size_')][0]
                 unit = param.split('_')[-1].lower()
                 expected = [x[1] for x in expected_disk_spec.items() if x[0].startswith('size_')][0]
                 expected = int(expected)
 
-            if unit == 'tb':
+            disk_units = dict(tb=3, gb=2, mb=1, kb=0)
-                return expected * 1024 * 1024 * 1024
+            if unit in disk_units:
-            elif unit == 'gb':
+                unit = unit.lower()
-                return expected * 1024 * 1024
+                return expected * (1024 ** disk_units[unit])
-            elif unit == 'mb':
+            else:
-                return expected * 1024
+                self.module.fail_json(msg="%s is not a supported unit for disk size."
-            elif unit == 'kb':
+                                          " Supported units are ['%s']." % (unit,
-                return expected
+                                                                            "', '".join(disk_units.keys())))
-
-            self.module.fail_json(
-                msg='%s is not a supported unit for disk size. Supported units are kb, mb, gb or tb' % unit)
 
         # No size found but disk, fail
         self.module.fail_json(

test/integration/targets/vmware_guest/tasks/clone_d1_c1_f0.yml

@@ -40,7 +40,7 @@
     template: "{{ item|basename }}"
     datacenter: "{{ (item|basename).split('_')[0] }}"
     disk:
-        - size: 0gb
+        - size: 1gb
           type: thin
           autoselect_datastore: True
     state: poweredoff

test/integration/targets/vmware_guest/tasks/create_d1_c1_f0.yml

@@ -58,7 +58,7 @@
         # cpu_reservation: 4096
         max_connections: 10
     disk:
-        - size: 0gb
+        - size: 1gb
           type: thin
           autoselect_datastore: True
     state: poweredoff
@@ -88,7 +88,7 @@
         num_cpu_cores_per_socket: 2
         memory_mb: 512
     disk:
-        - size: 0gb
+        - size: 1gb
           type: thin
           autoselect_datastore: True
     state: poweredoff

test/integration/targets/vmware_guest/tasks/create_nw_d1_c1_f0.yml

@@ -43,7 +43,7 @@
         num_cpus: 4
         memory_mb: 512
     disk:
-        - size: 0gb
+        - size: 1gb
           type: thin
           autoselect_datastore: True
     networks:

test/integration/targets/vmware_guest/tasks/create_rp_d1_c1_f0.yml

@@ -59,7 +59,7 @@
         num_cpus: 1
         memory_mb: 512
     disk:
-        - size: 0gb
+        - size: 1gb
           type: thin
           autoselect_datastore: True
     state: poweredoff
@@ -112,7 +112,7 @@
         num_cpus: 1
         memory_mb: 512
     disk:
-        - size: 0gb
+        - size: 1gb
           type: thin
           autoselect_datastore: True
     state: poweredoff
@@ -167,7 +167,7 @@
         num_cpus: 1
         memory_mb: 512
     disk:
-        - size: 0gb
+        - size: 1gb
           type: thin
           autoselect_datastore: True
     state: poweredoff
@@ -221,7 +221,7 @@
         num_cpus: 1
         memory_mb: 512
     disk:
-        - size: 0gb
+        - size: 1gb
           type: thin
           autoselect_datastore: True
     state: poweredoff

test/integration/targets/vmware_guest/tasks/disk_size_d1_c1_f0.yml

@@ -0,0 +1,60 @@
+# Test code for the vmware_guest module.
+# Copyright: (c) 2018, Abhijeet Kasurde <akasurde@redhat.com>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+- name: Wait for Flask controller to come up online
+  wait_for:
+    host: "{{ vcsim }}"
+    port: 5000
+    state: started
+
+- name: kill vcsim
+  uri:
+    url: http://{{ vcsim }}:5000/killall
+- name: start vcsim with no folders
+  uri:
+    url: http://{{ vcsim }}:5000/spawn?datacenter=1&cluster=1&folder=0
+  register: vcsim_instance
+
+- name: Wait for Flask controller to come up online
+  wait_for:
+    host: "{{ vcsim }}"
+    port: 443
+    state: started
+
+- name: get a list of VMS from vcsim
+  uri:
+    url: http://{{ vcsim }}:5000/govc_find?filter=VM
+  register: vmlist
+
+- debug: var=vcsim_instance
+- debug: var=vmlist
+
+- name: create new VMs with invalid disk size
+  vmware_guest:
+    validate_certs: False
+    hostname: "{{ vcsim }}"
+    username: "{{ vcsim_instance['json']['username'] }}"
+    password: "{{ vcsim_instance['json']['password'] }}"
+    name: "{{ 'newvm_' + item|basename }}"
+    guest_id: centos64Guest
+    datacenter: "{{ (item|basename).split('_')[0] }}"
+    hardware:
+        num_cpus: 1
+        memory_mb: 512
+    disk:
+        - size: 0gb
+          type: eagerzeroedthick
+          autoselect_datastore: True
+    state: poweredoff
+    folder: "{{ item|dirname }}"
+  with_items: "{{ vmlist['json'] }}"
+  register: disk_size_d1_c1_f0
+  ignore_errors: True
+
+- debug: var=disk_size_d1_c1_f0
+
+- name: assert that changes were made
+  assert:
+    that:
+        - "disk_size_d1_c1_f0.results|map(attribute='changed')|unique|list == [false]"

test/integration/targets/vmware_guest/tasks/disk_type_d1_c1_f0.yml

@@ -43,10 +43,10 @@
         num_cpus: 1
         memory_mb: 512
     disk:
-        - size: 0gb
+        - size: 1gb
           type: eagerzeroedthick
           autoselect_datastore: True
-        - size: 0gb
+        - size: 1gb
           type: thin
           autoselect_datastore: True
     state: poweredoff

test/integration/targets/vmware_guest/tasks/mac_address_d1_c1_f0.yml

@@ -37,7 +37,7 @@
         num_cpus: 1
         memory_mb: 512
     disk:
-        - size: 0gb
+        - size: 1gb
           type: thin
           autoselect_datastore: True
     networks:

test/integration/targets/vmware_guest/tasks/main.yml

@@ -31,3 +31,4 @@
 #- include: network_with_dvpg.yml
 #- include: template_d1_c1_f0.yml
 - include: vapp_d1_c1_f0.yml
+- include: disk_size_d1_c1_f0.yml