From 3afcf7e75db37d4c6e24bb4ef25999d95013d4e3 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky <103110+russoz@users.noreply.github.com> Date: Fri, 28 May 2021 05:13:21 +1200 Subject: [PATCH] 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 * Update plugins/modules/cloud/misc/rhevm.py Co-authored-by: Felix Fontein * adjusted per PR comment Co-authored-by: Felix Fontein --- .../fragments/2557-cloud-misc-refactor.yml | 7 +++++ .../cloud/misc/cloud_init_data_facts.py | 4 +-- .../modules/cloud/misc/proxmox_group_info.py | 2 +- plugins/modules/cloud/misc/proxmox_kvm.py | 31 +++++++++---------- plugins/modules/cloud/misc/rhevm.py | 4 +-- plugins/modules/cloud/misc/serverless.py | 11 +++---- plugins/modules/cloud/misc/terraform.py | 2 +- 7 files changed, 32 insertions(+), 29 deletions(-) create mode 100644 changelogs/fragments/2557-cloud-misc-refactor.yml diff --git a/changelogs/fragments/2557-cloud-misc-refactor.yml b/changelogs/fragments/2557-cloud-misc-refactor.yml new file mode 100644 index 0000000000..82e56dc942 --- /dev/null +++ b/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). diff --git a/plugins/modules/cloud/misc/cloud_init_data_facts.py b/plugins/modules/cloud/misc/cloud_init_data_facts.py index 2efb90cfeb..5774fa6f39 100644 --- a/plugins/modules/cloud/misc/cloud_init_data_facts.py +++ b/plugins/modules/cloud/misc/cloud_init_data_facts.py @@ -88,7 +88,7 @@ from ansible.module_utils.basic import AnsibleModule 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') diff --git a/plugins/modules/cloud/misc/proxmox_group_info.py b/plugins/modules/cloud/misc/proxmox_group_info.py index bf88659656..3d60e7e214 100644 --- a/plugins/modules/cloud/misc/proxmox_group_info.py +++ b/plugins/modules/cloud/misc/proxmox_group_info.py @@ -95,7 +95,7 @@ class ProxmoxGroup: 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'] diff --git a/plugins/modules/cloud/misc/proxmox_kvm.py b/plugins/modules/cloud/misc/proxmox_kvm.py index 2dcb1ab573..0ad75a45bd 100644 --- a/plugins/modules/cloud/misc/proxmox_kvm.py +++ b/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)) diff --git a/plugins/modules/cloud/misc/rhevm.py b/plugins/modules/cloud/misc/rhevm.py index cc6c1252bf..77b40248b3 100644 --- a/plugins/modules/cloud/misc/rhevm.py +++ b/plugins/modules/cloud/misc/rhevm.py @@ -547,7 +547,7 @@ class RHEVConn(object): 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() diff --git a/plugins/modules/cloud/misc/serverless.py b/plugins/modules/cloud/misc/serverless.py index 912d4226a8..1b2f8b62a6 100644 --- a/plugins/modules/cloud/misc/serverless.py +++ b/plugins/modules/cloud/misc/serverless.py @@ -139,16 +139,14 @@ from ansible.module_utils.basic import AnsibleModule 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 ' diff --git a/plugins/modules/cloud/misc/terraform.py b/plugins/modules/cloud/misc/terraform.py index 9bf36c8c81..8a34f9699b 100644 --- a/plugins/modules/cloud/misc/terraform.py +++ b/plugins/modules/cloud/misc/terraform.py @@ -233,7 +233,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))