vmadm: improvements (#4581)

* improvements on vmadm

* added changelog fragment

* Update plugins/modules/cloud/smartos/vmadm.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/cloud/smartos/vmadm.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/cloud/smartos/vmadm.py

Co-authored-by: Felix Fontein <felix@fontein.de>

changelogs/fragments/4581-vmadm-improvements.yaml

@@ -0,0 +1,2 @@
+minor_changes:
+  - vmadm - minor refactoring and improvement on the module (https://github.com/ansible-collections/community.general/pull/4581).

plugins/modules/cloud/smartos/vmadm.py

@@ -415,7 +415,7 @@ from ansible.module_utils.common.text.converters import to_native
 def get_vm_prop(module, uuid, prop):
     # Lookup a property for the given VM.
     # Returns the property, or None if not found.
-    cmd = '{0} lookup -j -o {1} uuid={2}'.format(module.vmadm, prop, uuid)
+    cmd = [module.vmadm, 'lookup', '-j', '-o', prop, 'uuid={0}'.format(uuid)]
 
     (rc, stdout, stderr) = module.run_command(cmd)
 
@@ -439,7 +439,7 @@ def get_vm_prop(module, uuid, prop):
 def get_vm_uuid(module, alias):
     # Lookup the uuid that goes with the given alias.
     # Returns the uuid or '' if not found.
-    cmd = '{0} lookup -j -o uuid alias={1}'.format(module.vmadm, alias)
+    cmd = [module.vmadm, 'lookup', '-j', '-o', 'uuid', 'alias={1}'.format(alias)]
 
     (rc, stdout, stderr) = module.run_command(cmd)
 
@@ -466,7 +466,7 @@ def get_vm_uuid(module, alias):
 
 def get_all_vm_uuids(module):
     # Retrieve the UUIDs for all VMs.
-    cmd = '{0} lookup -j -o uuid'.format(module.vmadm)
+    cmd = [module.vmadm, 'lookup', '-j', '-o', 'uuid']
 
     (rc, stdout, stderr) = module.run_command(cmd)
 
@@ -520,6 +520,7 @@ def new_vm(module, uuid, vm_state):
 def vmadm_create_vm(module, payload_file):
     # Create a new VM using the provided payload.
     cmd = '{0} create -f {1}'.format(module.vmadm, payload_file)
+    cmd = [module.vmadm, 'create', '-f', payload_file]
 
     return module.run_command(cmd)
 
@@ -541,20 +542,15 @@ def set_vm_state(module, vm_uuid, vm_state):
         'rebooted': ['reboot', False]
     }
 
-    if p['force'] and cmds[vm_state][1]:
+    command, forceable = cmds[vm_state]
-        force = '-F'
+    force = ['-F'] if p['force'] and forceable else []
-    else:
-        force = ''
 
-    cmd = 'vmadm {0} {1} {2}'.format(cmds[vm_state][0], force, vm_uuid)
+    cmd = [module.vmadm, command] + force + [vm_uuid]
 
     (rc, stdout, stderr) = module.run_command(cmd)
 
     match = re.match('^Successfully.*', stderr)
-    if match:
+    return match is not None
-        return True
-    else:
-        return False
 
 
 def create_payload(module, uuid):
@@ -601,23 +597,17 @@ def vm_state_transition(module, uuid, vm_state):
 
 
 def is_valid_uuid(uuid):
-    if re.match('^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$', uuid, re.IGNORECASE):
+    return re.match('^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$', uuid, re.IGNORECASE) is not None
-        return True
-    else:
-        return False
 
 
 def validate_uuids(module):
-    # Perform basic UUID validation.
+    failed = [
-    failed = []
+        name
+        for name, pvalue in [(x, module.params[x]) for x in ['uuid', 'image_uuid']]
+        if pvalue and pvalue != '*' and not is_valid_uuid(pvalue)
+    ]
 
-    for u in [['uuid', module.params['uuid']],
+    if failed:
-              ['image_uuid', module.params['image_uuid']]]:
-        if u[1] and u[1] != '*':
-            if not is_valid_uuid(u[1]):
-                failed.append(u[0])
-
-    if len(failed) > 0:
         module.fail_json(msg='No valid UUID(s) found for: {0}'.format(", ".join(failed)))