From e18b5f623002d27d91f9021480138af61697fadb Mon Sep 17 00:00:00 2001 From: Niels Huylebroeck Date: Fri, 13 Dec 2019 16:02:56 +0100 Subject: [PATCH 1/3] DRY return value and check_mode implementation --- library/insights_register.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/library/insights_register.py b/library/insights_register.py index 89de8ec..cc02457 100644 --- a/library/insights_register.py +++ b/library/insights_register.py @@ -106,9 +106,6 @@ def run_module(): supports_check_mode=True ) - if module.check_mode: - return result - state = module.params['state'] insights_name = module.params['insights_name'] display_name = module.params['display_name'] @@ -121,29 +118,28 @@ def run_module(): if reg_status == 0 and not force_reregister: result['changed'] = False result['message'] = 'The Insights API has determined that this machine is already registered' - module.exit_json(**result) elif reg_status == 0 and force_reregister: - subprocess.call([insights_name, '--force-reregister']) + if not module.check_mode: + subprocess.call([insights_name, '--force-reregister']) result['changed'] = True result['message'] = 'New machine-id created - ' + insights_name + ' has been registered' - module.exit_json(**result) else: - subprocess.call([insights_name, '--register']) + if not module.check_mode: + subprocess.call([insights_name, '--register']) result['changed'] = True result['message'] = insights_name + ' has been registered' - module.exit_json(**result) if state == 'absent': result['original_message'] = 'Attempting to unregister ' + insights_name if reg_status is not 0: result['changed'] = False result['message'] = insights_name + ' is already unregistered' - module.exit_json(**result) else: subprocess.call([insights_name, '--unregister']) result['changed'] = True result['message'] = insights_name + ' has been unregistered' - module.exit_json(**result) + + module.exit_json(**result) def main(): run_module() From e49f19b58c9812991b190e0c61902e9b2d794ab5 Mon Sep 17 00:00:00 2001 From: Niels Huylebroeck Date: Fri, 13 Dec 2019 16:15:24 +0100 Subject: [PATCH 2/3] Missed one subprocess call during check_mode --- library/insights_register.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/insights_register.py b/library/insights_register.py index cc02457..c0cc687 100644 --- a/library/insights_register.py +++ b/library/insights_register.py @@ -135,7 +135,8 @@ def run_module(): result['changed'] = False result['message'] = insights_name + ' is already unregistered' else: - subprocess.call([insights_name, '--unregister']) + if not module.check_mode: + subprocess.call([insights_name, '--unregister']) result['changed'] = True result['message'] = insights_name + ' has been unregistered' From ccd1518dd119fd442b1063f6e2bf2e87eabdc85b Mon Sep 17 00:00:00 2001 From: Niels Huylebroeck Date: Fri, 13 Dec 2019 16:16:27 +0100 Subject: [PATCH 3/3] Replace raw subprocess call with module.run_command --- library/insights_register.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/insights_register.py b/library/insights_register.py index c0cc687..aebf686 100644 --- a/library/insights_register.py +++ b/library/insights_register.py @@ -111,7 +111,7 @@ def run_module(): display_name = module.params['display_name'] force_reregister = module.params['force_reregister'] - reg_status = subprocess.call([insights_name, '--status']) + (reg_status, _, _) = module.run_command([insights_name, '--status']) if state == 'present': result['original_message'] = 'Attempting to register ' + insights_name @@ -120,12 +120,12 @@ def run_module(): result['message'] = 'The Insights API has determined that this machine is already registered' elif reg_status == 0 and force_reregister: if not module.check_mode: - subprocess.call([insights_name, '--force-reregister']) + module.run_command([insights_name, '--force-reregister']) result['changed'] = True result['message'] = 'New machine-id created - ' + insights_name + ' has been registered' else: if not module.check_mode: - subprocess.call([insights_name, '--register']) + module.run_command([insights_name, '--register']) result['changed'] = True result['message'] = insights_name + ' has been registered' @@ -136,7 +136,7 @@ def run_module(): result['message'] = insights_name + ' is already unregistered' else: if not module.check_mode: - subprocess.call([insights_name, '--unregister']) + module.run_command([insights_name, '--unregister']) result['changed'] = True result['message'] = insights_name + ' has been unregistered'