From 895664f5391fbfc00b62fa0057f432c1737cfe7e Mon Sep 17 00:00:00 2001
From: Dag Wieers <dag@wieers.com>
Date: Thu, 16 Mar 2017 15:55:07 +0100
Subject: [PATCH] vmware_guest: Fix "KeyError: 'changed'" (#22564)

Due to how the result-dictionary is being used, in some codepaths it would have an empty dictionary.
This then is problematic if the code expects the 'changed' key later on.

I also improved the docs a bit, by clearly marking the parameter names and URL in the documentation.
---
 .../modules/cloud/vmware/vmware_guest.py      | 108 +++++++++---------
 1 file changed, 56 insertions(+), 52 deletions(-)

diff --git a/lib/ansible/modules/cloud/vmware/vmware_guest.py b/lib/ansible/modules/cloud/vmware/vmware_guest.py
index 2e208bb32e0..48129f1f350 100644
--- a/lib/ansible/modules/cloud/vmware/vmware_guest.py
+++ b/lib/ansible/modules/cloud/vmware/vmware_guest.py
@@ -108,7 +108,6 @@ options:
    datacenter:
         description:
             - Destination datacenter for the deploy operation
-        required: False
         default: ha-datacenter
    cluster:
         description:
@@ -139,23 +138,23 @@ options:
         description:
           - "Parameters to customize template"
           - "Common parameters (Linux/Windows):"
-          - "  dns_servers (list): List of DNS servers to configure"
-          - "  dns_suffix (list): List of domain suffixes, aka DNS search path (default: C(domain) parameter)"
-          - "  domain (string): DNS domain name to use"
-          - "  hostname (string): Computer hostname (default: C(name) parameter)"
+          - "  C(dns_servers) (list): List of DNS servers to configure"
+          - "  C(dns_suffix) (list): List of domain suffixes, aka DNS search path (default: C(domain) parameter)"
+          - "  C(domain) (string): DNS domain name to use"
+          - "  C(hostname) (string): Computer hostname (default: C(name) parameter)"
           - "Parameters related to windows customization:"
-          - "  autologon (bool): Auto logon after VM customization (default: False)"
-          - "  autologoncount (int): Number of autologon after reboot (default: 1)"
-          - "  domainadmin (string): User used to join in AD domain (mandatory with joindomain)"
-          - "  domainadminpassword (string): Password used to join in AD domain (mandatory with joindomain)"
-          - "  fullname (string): Server owner name (default: Administrator)"
-          - "  joindomain (string): AD domain to join (Not compatible with C(joinworkgroup))"
-          - "  joinworkgroup (string): Workgroup to join (Not compatible with C(joindomain), default: WORKGROUP)"
-          - "  orgname (string): Organisation name (default: ACME)"
-          - "  password (string): Local administrator password (mandatory)"
-          - "  productid (string): Product ID"
-          - "  runonce (list): List of commands to run at first user logon"
-          - "  timezone (int): Timezone (default: 85) See https://msdn.microsoft.com/en-us/library/ms912391(v=winembedded.11).aspx"
+          - "  C(autologon) (bool): Auto logon after VM customization (default: False)"
+          - "  C(autologoncount) (int): Number of autologon after reboot (default: 1)"
+          - "  C(domainadmin) (string): User used to join in AD domain (mandatory with joindomain)"
+          - "  C(domainadminpassword) (string): Password used to join in AD domain (mandatory with joindomain)"
+          - "  C(fullname) (string): Server owner name (default: Administrator)"
+          - "  C(joindomain) (string): AD domain to join (Not compatible with C(joinworkgroup))"
+          - "  C(joinworkgroup) (string): Workgroup to join (Not compatible with C(joindomain), default: WORKGROUP)"
+          - "  C(orgname) (string): Organisation name (default: ACME)"
+          - "  C(password) (string): Local administrator password (mandatory)"
+          - "  C(productid) (string): Product ID"
+          - "  C(runonce) (list): List of commands to run at first user logon"
+          - "  C(timezone) (int): Timezone (default: 85) See U(https://msdn.microsoft.com/en-us/library/ms912391(v=winembedded.11).aspx)"
         version_added: "2.3"
 extends_documentation_fragment: vmware.documentation
 '''
@@ -485,17 +484,19 @@ class PyVmomiHelper(object):
         facts = self.gather_facts(vm)
         expected_state = state.replace('_', '').lower()
         current_state = facts['hw_power_status'].lower()
-        result = {}
+        result = dict(
+            changed=False,
+            failed=False,
+        )
 
         # Need Force
         if not force and current_state not in ['poweredon', 'poweredoff']:
-            return "VM is in %s power state. Force is required!" % current_state
+            result['failed'] = True
+            result['msg'] = "VM is in %s power state. Force is required!" % current_state
+            return result
 
-        # State is already true
-        if current_state == expected_state:
-            result['changed'] = False
-            result['failed'] = False
-        else:
+        # State is not already true
+        if current_state != expected_state:
             task = None
             try:
                 if expected_state == 'poweredoff':
@@ -508,15 +509,15 @@ class PyVmomiHelper(object):
                     if current_state in ('poweredon', 'poweringon', 'resetting', 'poweredoff'):
                         task = vm.Reset()
                     else:
-                        result = {'changed': False, 'failed': True,
-                                  'msg': "Cannot restart VM in the current state %s" % current_state}
+                        result['failed'] = True
+                        result['msg'] = "Cannot restart VM in the current state %s" % current_state
 
                 elif expected_state == 'suspended':
                     if current_state in ('poweredon', 'poweringon'):
                         task = vm.Suspend()
                     else:
-                        result = {'changed': False, 'failed': True,
-                                  'msg': 'Cannot suspend VM in the current state %s' % current_state}
+                        result['failed'] = True
+                        result['msg'] = 'Cannot suspend VM in the current state %s' % current_state
 
                 elif expected_state in ['shutdownguest', 'rebootguest']:
                     if current_state == 'poweredon' and vm.guest.toolsRunningStatus == 'guestToolsRunning':
@@ -525,25 +526,28 @@ class PyVmomiHelper(object):
                         else:
                             task = vm.RebootGuest()
                     else:
-                        result = {'changed': False, 'failed': True,
-                                  'msg': "VM %s must be in poweredon state & tools should be installed for guest shutdown/reboot" % vm.name}
+                        result['failed'] = True
+                        result['msg'] = "VM %s must be in poweredon state & tools should be installed for guest shutdown/reboot" % vm.name
 
             except Exception:
                 e = get_exception()
-                result = {'changed': False, 'failed': True, 'msg': e}
+                result['failed'] = True
+                result['msg'] = str(e)
 
             if task:
                 self.wait_for_task(task)
                 if task.info.state == 'error':
-                    result = {'changed': False, 'failed': True, 'msg': task.info.error.msg}
+                    result['failed'] = True
+                    result['msg'] = str(task.info.error.msg)
                 else:
-                    result = {'changed': True, 'failed': False}
+                    result['changed'] = True
 
         # need to get new metadata if changed
         if result['changed']:
             newvm = self.getvm(uuid=vm.config.uuid)
             facts = self.gather_facts(newvm)
             result['instance'] = facts
+
         return result
 
     def gather_facts(self, vm):
@@ -1250,26 +1254,26 @@ def main():
                     'rebootguest'
                 ],
                 default='present'),
-            validate_certs=dict(required=False, type='bool', default=True),
-            template_src=dict(required=False, type='str', aliases=['template'], default=None),
-            is_template=dict(required=False, type='bool', default=False),
-            annotation=dict(required=False, type='str', aliases=['notes']),
-            customvalues=dict(required=False, type='list', default=[]),
+            validate_certs=dict(type='bool', default=True),
+            template_src=dict(type='str', aliases=['template']),
+            is_template=dict(type='bool', default=False),
+            annotation=dict(type='str', aliases=['notes']),
+            customvalues=dict(type='list', default=[]),
             name=dict(required=True, type='str'),
-            name_match=dict(required=False, type='str', default='first'),
-            uuid=dict(required=False, type='str'),
-            folder=dict(required=False, type='str', default='/vm'),
-            guest_id=dict(required=False, type='str', default=None),
-            disk=dict(required=False, type='list', default=[]),
-            hardware=dict(required=False, type='dict', default={}),
-            force=dict(required=False, type='bool', default=False),
-            datacenter=dict(required=False, type='str', default='ha-datacenter'),
-            esxi_hostname=dict(required=False, type='str', default=None),
-            cluster=dict(required=False, type='str', default=None),
-            wait_for_ip_address=dict(required=False, type='bool', default=False),
-            networks=dict(required=False, type='list', default=[]),
-            resource_pool=dict(required=False, type='str', default=None),
-            customization=dict(required=False, type='dict', no_log=True, default={}),
+            name_match=dict(type='str', default='first'),
+            uuid=dict(type='str'),
+            folder=dict(type='str', default='/vm'),
+            guest_id=dict(type='str'),
+            disk=dict(type='list', default=[]),
+            hardware=dict(type='dict', default={}),
+            force=dict(type='bool', default=False),
+            datacenter=dict(type='str', default='ha-datacenter'),
+            esxi_hostname=dict(type='str'),
+            cluster=dict(type='str'),
+            wait_for_ip_address=dict(type='bool', default=False),
+            networks=dict(type='list', default=[]),
+            resource_pool=dict(type='str'),
+            customization=dict(type='dict', no_log=True, default={}),
         ),
         supports_check_mode=True,
         mutually_exclusive=[