minor refactors on plugins/modules/cloud/misc (#2557)

* minor refactors on plugins/modules/cloud/misc

* added changelog fragment

* removed unreachable statement

* Update plugins/modules/cloud/misc/terraform.py

Co-authored-by: Felix Fontein <[email protected]>

* Update plugins/modules/cloud/misc/rhevm.py

Co-authored-by: Felix Fontein <[email protected]>

* adjusted per PR comment

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 3afcf7e)

changelogs/fragments/2557-cloud-misc-refactor.yml

@@ -0,0 +1,7 @@
+minor_changes:
+  - cloud_init_data_facts - minor refactor (https://github.com/ansible-collections/community.general/pull/2557).
+  - proxmox_group_info - minor refactor (https://github.com/ansible-collections/community.general/pull/2557).
+  - proxmox_kvm - minor refactor (https://github.com/ansible-collections/community.general/pull/2557).
+  - rhevm - minor refactor (https://github.com/ansible-collections/community.general/pull/2557).
+  - serverless - minor refactor (https://github.com/ansible-collections/community.general/pull/2557).
+  - terraform - minor refactor (https://github.com/ansible-collections/community.general/pull/2557).

plugins/modules/cloud/misc/cloud_init_data_facts.py

@@ -88,7 +88,7 @@
 from ansible.module_utils._text import to_text
 
 
-CLOUD_INIT_PATH = "/var/lib/cloud/data/"
+CLOUD_INIT_PATH = "/var/lib/cloud/data"
 
 
 def gather_cloud_init_data_facts(module):
@@ -100,7 +100,7 @@ def gather_cloud_init_data_facts(module):
         filter = module.params.get('filter')
         if filter is None or filter == i:
             res['cloud_init_data_facts'][i] = dict()
-            json_file = CLOUD_INIT_PATH + i + '.json'
+            json_file = os.path.join(CLOUD_INIT_PATH, i + '.json')
 
             if os.path.exists(json_file):
                 f = open(json_file, 'rb')

plugins/modules/cloud/misc/proxmox_group_info.py

@@ -95,7 +95,7 @@ def __init__(self, group):
         self.group = dict()
         # Data representation is not the same depending on API calls
         for k, v in group.items():
-            if k == 'users' and type(v) == str:
+            if k == 'users' and isinstance(v, str):
                 self.group['users'] = v.split(',')
             elif k == 'members':
                 self.group['users'] = group['members']

plugins/modules/cloud/misc/proxmox_kvm.py

@@ -808,23 +808,23 @@ def get_vminfo(module, proxmox, node, vmid, **kwargs):
     # Sanitize kwargs. Remove not defined args and ensure True and False converted to int.
     kwargs = dict((k, v) for k, v in kwargs.items() if v is not None)
 
-    # Convert all dict in kwargs to elements. For hostpci[n], ide[n], net[n], numa[n], parallel[n], sata[n], scsi[n], serial[n], virtio[n]
+    # Convert all dict in kwargs to elements.
+    # For hostpci[n], ide[n], net[n], numa[n], parallel[n], sata[n], scsi[n], serial[n], virtio[n]
     for k in list(kwargs.keys()):
         if isinstance(kwargs[k], dict):
             kwargs.update(kwargs[k])
             del kwargs[k]
 
     # Split information by type
+    re_net = re.compile(r'net[0-9]')
+    re_dev = re.compile(r'(virtio|ide|scsi|sata)[0-9]')
     for k, v in kwargs.items():
-        if re.match(r'net[0-9]', k) is not None:
+        if re_net.match(k):
             interface = k
             k = vm[k]
             k = re.search('=(.*?),', k).group(1)
             mac[interface] = k
-        if (re.match(r'virtio[0-9]', k) is not None or
-                re.match(r'ide[0-9]', k) is not None or
-                re.match(r'scsi[0-9]', k) is not None or
-                re.match(r'sata[0-9]', k) is not None):
+        elif re_dev.match(k):
             device = k
             k = vm[k]
             k = re.search('(.*?),', k).group(1)
@@ -835,16 +835,13 @@ def get_vminfo(module, proxmox, node, vmid, **kwargs):
     results['vmid'] = int(vmid)
 
 
-def settings(module, proxmox, vmid, node, name, **kwargs):
+def settings(proxmox, vmid, node, **kwargs):
     proxmox_node = proxmox.nodes(node)
 
     # Sanitize kwargs. Remove not defined args and ensure True and False converted to int.
     kwargs = dict((k, v) for k, v in kwargs.items() if v is not None)
 
-    if proxmox_node.qemu(vmid).config.set(**kwargs) is None:
-        return True
-    else:
-        return False
+    return proxmox_node.qemu(vmid).config.set(**kwargs) is None
 
 
 def wait_for_task(module, proxmox, node, taskid):
@@ -915,7 +912,8 @@ def create_vm(module, proxmox, vmid, newid, node, name, memory, cpu, cores, sock
         if 'pool' in kwargs:
             del kwargs['pool']
 
-    # Convert all dict in kwargs to elements. For hostpci[n], ide[n], net[n], numa[n], parallel[n], sata[n], scsi[n], serial[n], virtio[n], ipconfig[n]
+    # Convert all dict in kwargs to elements.
+    # For hostpci[n], ide[n], net[n], numa[n], parallel[n], sata[n], scsi[n], serial[n], virtio[n], ipconfig[n]
     for k in list(kwargs.keys()):
         if isinstance(kwargs[k], dict):
             kwargs.update(kwargs[k])
@@ -938,8 +936,9 @@ def create_vm(module, proxmox, vmid, newid, node, name, memory, cpu, cores, sock
 
     # VM tags are expected to be valid and presented as a comma/semi-colon delimited string
     if 'tags' in kwargs:
+        re_tag = re.compile(r'^[a-z0-9_][a-z0-9_\-\+\.]*$')
         for tag in kwargs['tags']:
-            if not re.match(r'^[a-z0-9_][a-z0-9_\-\+\.]*$', tag):
+            if not re_tag.match(tag):
                 module.fail_json(msg='%s is not a valid tag' % tag)
         kwargs['tags'] = ",".join(kwargs['tags'])
 
@@ -971,7 +970,7 @@ def create_vm(module, proxmox, vmid, newid, node, name, memory, cpu, cores, sock
 
     if not wait_for_task(module, proxmox, node, taskid):
         module.fail_json(msg='Reached timeout while waiting for creating VM. Last line in task before timeout: %s' %
-                         proxmox_node.tasks(taskid).log.get()[:1])
+                             proxmox_node.tasks(taskid).log.get()[:1])
         return False
     return True
 
@@ -1209,14 +1208,14 @@ def main():
 
     if delete is not None:
         try:
-            settings(module, proxmox, vmid, node, name, delete=delete)
+            settings(proxmox, vmid, node, delete=delete)
             module.exit_json(changed=True, vmid=vmid, msg="Settings has deleted on VM {0} with vmid {1}".format(name, vmid))
         except Exception as e:
             module.fail_json(vmid=vmid, msg='Unable to delete settings on VM {0} with vmid {1}: '.format(name, vmid) + str(e))
 
     if revert is not None:
         try:
-            settings(module, proxmox, vmid, node, name, revert=revert)
+            settings(proxmox, vmid, node, revert=revert)
             module.exit_json(changed=True, vmid=vmid, msg="Settings has reverted on VM {0} with vmid {1}".format(name, vmid))
         except Exception as e:
             module.fail_json(vmid=vmid, msg='Unable to revert settings on VM {0} with vmid {1}: Maybe is not a pending task...   '.format(name, vmid) + str(e))

plugins/modules/cloud/misc/rhevm.py

@@ -547,7 +547,7 @@ def set_Memory(self, name, memory):
 
     def set_Memory_Policy(self, name, memory_policy):
         VM = self.get_VM(name)
-        VM.memory_policy.guaranteed = int(int(memory_policy) * 1024 * 1024 * 1024)
+        VM.memory_policy.guaranteed = int(memory_policy) * 1024 * 1024 * 1024
         try:
             VM.update()
             setMsg("The memory policy has been updated.")
@@ -1260,7 +1260,7 @@ def core(module):
 
     r = RHEV(module)
 
-    state = module.params.get('state', 'present')
+    state = module.params.get('state')
 
     if state == 'ping':
         r.test()

plugins/modules/cloud/misc/serverless.py

@@ -139,16 +139,14 @@
 
 def read_serverless_config(module):
     path = module.params.get('service_path')
+    full_path = os.path.join(path, 'serverless.yml')
 
     try:
-        with open(os.path.join(path, 'serverless.yml')) as sls_config:
+        with open(full_path) as sls_config:
             config = yaml.safe_load(sls_config.read())
             return config
     except IOError as e:
-        module.fail_json(msg="Could not open serverless.yml in {0}. err: {1}".format(path, str(e)))
-
-    module.fail_json(msg="Failed to open serverless config at {0}".format(
-        os.path.join(path, 'serverless.yml')))
+        module.fail_json(msg="Could not open serverless.yml in {0}. err: {1}".format(full_path, str(e)))
 
 
 def get_service_name(module, stage):
@@ -182,7 +180,6 @@ def main():
 
     service_path = module.params.get('service_path')
     state = module.params.get('state')
-    functions = module.params.get('functions')
     region = module.params.get('region')
     stage = module.params.get('stage')
     deploy = module.params.get('deploy', True)
@@ -193,7 +190,7 @@ def main():
     if serverless_bin_path is not None:
         command = serverless_bin_path + " "
     else:
-        command = "serverless "
+        command = module.get_bin_path("serverless") + " "
 
     if state == 'present':
         command += 'deploy '

plugins/modules/cloud/misc/terraform.py

@@ -227,7 +227,7 @@ def get_version(bin_path):
 
 
 def preflight_validation(bin_path, project_path, version, variables_args=None, plan_file=None):
-    if project_path in [None, ''] or '/' not in project_path:
+    if project_path is None or '/' not in project_path:
         module.fail_json(msg="Path for Terraform project can not be None or ''.")
     if not os.path.exists(bin_path):
         module.fail_json(msg="Path for Terraform binary '{0}' doesn't exist on this host - check the path and try again please.".format(bin_path))