refactor(review): Additional code review items

This commit is contained in:
munchtoast 2025-07-02 16:25:09 -04:00
commit 00b87134f8
2 changed files with 129 additions and 141 deletions

View file

@ -27,7 +27,7 @@ options:
state: state:
description: description:
- Indicate desired state for cluster STONITH. - Indicate desired state for cluster STONITH.
choices: [ present, absent, enabled, disabled ] choices: [present, absent, enabled, disabled]
default: present default: present
type: str type: str
name: name:
@ -75,7 +75,7 @@ options:
description: description:
- Action to apply to STONITH. - Action to apply to STONITH.
type: str type: str
choices: [ group, before, after ] choices: [group, before, after]
argument_options: argument_options:
description: description:
- Options to associate with STONITH action. - Options to associate with STONITH action.
@ -109,10 +109,10 @@ EXAMPLES = '''
RETURN = ''' RETURN = '''
cluster_stonith: cluster_stonith:
description: The cluster STONITH output message. description: The cluster STONITH output message.
type: str type: str
sample: "" sample: ""
returned: always returned: always
''' '''
from ansible_collections.community.general.plugins.module_utils.module_helper import StateModuleHelper from ansible_collections.community.general.plugins.module_utils.module_helper import StateModuleHelper
@ -143,13 +143,14 @@ class PacemakerStonith(StateModuleHelper):
supports_check_mode=True supports_check_mode=True
) )
default_state = "present"
def __init_module__(self): def __init_module__(self):
self.runner = pacemaker_runner(self.module, cli_action='stonith') self.runner = pacemaker_runner(self.module, cli_action='stonith')
self.vars.set('previous_value', self._get()) self.vars.set('previous_value', self._get()['out'])
self.vars.set('value', self.vars.previous_value, change=True, diff=True) self.vars.set('value', self.vars.previous_value, change=True, diff=True)
def __quit_module__(self):
self.vars.set('value', self._get()['out'])
def _process_command_output(self, fail_on_err, ignore_err_msg=""): def _process_command_output(self, fail_on_err, ignore_err_msg=""):
def process(rc, out, err): def process(rc, out, err):
if fail_on_err and rc != 0 and err and ignore_err_msg not in err: if fail_on_err and rc != 0 and err and ignore_err_msg not in err:
@ -159,14 +160,17 @@ class PacemakerStonith(StateModuleHelper):
return process return process
def _get(self): def _get(self):
with self.runner('state name', output_process=self._process_command_output(False)) as ctx: with self.runner('state name') as ctx:
return ctx.run(state='status') result = ctx.run(state='status')
return dict([('rc', result[0]),
('out', result[1] if result[1] != "" else None),
('err', result[2])])
def _fmt_stonith_resource(self): def fmt_stonith_resource(self):
return dict([("resource_name", self.vars.stonith_type)]) return dict([("resource_name", self.vars.stonith_type)])
# TODO: Pluralize operation_options in separate PR and remove this helper fmt function # TODO: Pluralize operation_options in separate PR and remove this helper fmt function
def _fmt_stonith_operations(self): def fmt_stonith_operations(self):
modified_stonith_operations = [] modified_stonith_operations = []
for stonith_operation in self.vars.stonith_operations: for stonith_operation in self.vars.stonith_operations:
modified_stonith_operations.append(dict([("operation_action", stonith_operation.get('operation_action')), modified_stonith_operations.append(dict([("operation_action", stonith_operation.get('operation_action')),
@ -176,41 +180,25 @@ class PacemakerStonith(StateModuleHelper):
def state_absent(self): def state_absent(self):
with self.runner('state name', output_process=self._process_command_output(True, "does not exist"), check_mode_skip=True) as ctx: with self.runner('state name', output_process=self._process_command_output(True, "does not exist"), check_mode_skip=True) as ctx:
ctx.run() ctx.run()
self.vars.set('value', self._get())
self.vars.stdout = ctx.results_out
self.vars.stderr = ctx.results_err
self.vars.cmd = ctx.cmd
def state_present(self): def state_present(self):
with self.runner( with self.runner(
'state name resource_type resource_option resource_operation resource_meta resource_argument agent_validation wait', 'state name resource_type resource_option resource_operation resource_meta resource_argument agent_validation wait',
output_process=self._process_command_output(True, "already exists"), output_process=self._process_command_output(True, "already exists"),
check_mode_skip=True) as ctx: check_mode_skip=True) as ctx:
ctx.run(resource_type=self._fmt_stonith_resource(), ctx.run(resource_type=self.fmt_stonith_resource(),
resource_option=self.vars.stonith_options, resource_option=self.vars.stonith_options,
resource_operation=self._fmt_stonith_operations(), resource_operation=self.fmt_stonith_operations(),
resource_meta=self.vars.stonith_metas, resource_meta=self.vars.stonith_metas,
resource_argument=self.vars.stonith_argument) resource_argument=self.vars.stonith_argument)
self.vars.set('value', self._get())
self.vars.stdout = ctx.results_out
self.vars.stderr = ctx.results_err
self.vars.cmd = ctx.cmd
def state_enabled(self): def state_enabled(self):
with self.runner('state name', output_process=self._process_command_output(True, "Starting"), check_mode_skip=True) as ctx: with self.runner('state name', output_process=self._process_command_output(True, "Starting"), check_mode_skip=True) as ctx:
ctx.run() ctx.run()
self.vars.set('value', self._get())
self.vars.stdout = ctx.results_out
self.vars.stderr = ctx.results_err
self.vars.cmd = ctx.cmd
def state_disabled(self): def state_disabled(self):
with self.runner('state name', output_process=self._process_command_output(True, "Stopped"), check_mode_skip=True) as ctx: with self.runner('state name', output_process=self._process_command_output(True, "Stopped"), check_mode_skip=True) as ctx:
ctx.run() ctx.run()
self.vars.set('value', self._get())
self.vars.stdout = ctx.results_out
self.vars.stderr = ctx.results_err
self.vars.cmd = ctx.cmd
def main(): def main():

View file

@ -29,21 +29,21 @@ test_cases:
value: " * virtual-stonith\t(stonith:fence_virt):\t Started" value: " * virtual-stonith\t(stonith:fence_virt):\t Started"
mocks: mocks:
run_command: run_command:
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 1 rc: 1
out: "" out: ""
err: "" err: ""
- command: ["/testbin/pcs", stonith, create, virtual-stonith, fence_virt, "pcmk_host_list=f1", "op", "monitor", "interval=30s", "--wait=300"] - command: ["/testbin/pcs", stonith, create, virtual-stonith, fence_virt, "pcmk_host_list=f1", "op", "monitor", "interval=30s", "--wait=300"]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: "" out: ""
err: "" err: ""
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: " * virtual-stonith\t(stonith:fence_virt):\t Started" out: " * virtual-stonith\t(stonith:fence_virt):\t Started"
err: "" err: ""
- id: test_present_minimal_input_stonith_exists - id: test_present_minimal_input_stonith_exists
input: input:
state: present state: present
@ -61,21 +61,21 @@ test_cases:
value: " * virtual-stonith\t(stonith:fence_virt):\t Started" value: " * virtual-stonith\t(stonith:fence_virt):\t Started"
mocks: mocks:
run_command: run_command:
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: " * virtual-stonith\t(stonith:fence_virt):\t Started" out: " * virtual-stonith\t(stonith:fence_virt):\t Started"
err: "" err: ""
- command: ["/testbin/pcs", stonith, create, virtual-stonith, fence_virt, "pcmk_host_list=f1", "op", "monitor", "interval=30s", "--wait=300"] - command: ["/testbin/pcs", stonith, create, virtual-stonith, fence_virt, "pcmk_host_list=f1", "op", "monitor", "interval=30s", "--wait=300"]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: "" out: ""
err: "Error: 'virtual-stonith' already exists.\n" err: "Error: 'virtual-stonith' already exists.\n"
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: " * virtual-stonith\t(stonith:fence_virt):\t Started" out: " * virtual-stonith\t(stonith:fence_virt):\t Started"
err: "" err: ""
- id: test_absent_minimal_input_stonith_not_exists - id: test_absent_minimal_input_stonith_not_exists
input: input:
state: absent state: absent
@ -86,21 +86,21 @@ test_cases:
value: null value: null
mocks: mocks:
run_command: run_command:
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 1 rc: 1
out: "" out: ""
err: "" err: ""
- command: ["/testbin/pcs", stonith, remove, virtual-stonith] - command: ["/testbin/pcs", stonith, remove, virtual-stonith]
environ: *env-def environ: *env-def
rc: 1 rc: 1
out: "" out: ""
err: "Error: Resource 'virtual-stonith' does not exist.\n" err: "Error: Resource 'virtual-stonith' does not exist.\n"
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 1 rc: 1
out: "" out: ""
err: "" err: ""
- id: test_absent_minimal_input_stonith_exists - id: test_absent_minimal_input_stonith_exists
input: input:
state: absent state: absent
@ -111,21 +111,21 @@ test_cases:
value: null value: null
mocks: mocks:
run_command: run_command:
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: " * virtual-stonith\t(stonith:fence_virt):\t Started" out: " * virtual-stonith\t(stonith:fence_virt):\t Started"
err: "" err: ""
- command: ["/testbin/pcs", stonith, remove, virtual-stonith] - command: ["/testbin/pcs", stonith, remove, virtual-stonith]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: "" out: ""
err: "Attempting to stop: virtual-ip... Stopped\n" err: "Attempting to stop: virtual-ip... Stopped\n"
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 1 rc: 1
out: "" out: ""
err: "" err: ""
- id: test_enabled_minimal_input_stonith_not_exists - id: test_enabled_minimal_input_stonith_not_exists
input: input:
state: enabled state: enabled
@ -135,16 +135,16 @@ test_cases:
msg: "pcs failed with error (rc=1): Error: Resource 'virtual-stonith' does not exist." msg: "pcs failed with error (rc=1): Error: Resource 'virtual-stonith' does not exist."
mocks: mocks:
run_command: run_command:
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 1 rc: 1
out: "" out: ""
err: "" err: ""
- command: ["/testbin/pcs", stonith, enable, virtual-stonith] - command: ["/testbin/pcs", stonith, enable, virtual-stonith]
environ: *env-def environ: *env-def
rc: 1 rc: 1
out: "" out: ""
err: "Error: Resource 'virtual-stonith' does not exist." err: "Error: Resource 'virtual-stonith' does not exist."
- id: test_enabled_minimal_input_stonith_exists - id: test_enabled_minimal_input_stonith_exists
input: input:
state: enabled state: enabled
@ -155,21 +155,21 @@ test_cases:
value: " * virtual-stonith\t(stonith:fence_virt):\t Starting" value: " * virtual-stonith\t(stonith:fence_virt):\t Starting"
mocks: mocks:
run_command: run_command:
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: " * virtual-stonith\t(stonith:fence_virt):\t Stopped (disabled)" out: " * virtual-stonith\t(stonith:fence_virt):\t Stopped (disabled)"
err: "" err: ""
- command: ["/testbin/pcs", stonith, enable, virtual-stonith] - command: ["/testbin/pcs", stonith, enable, virtual-stonith]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: "" out: ""
err: "" err: ""
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: " * virtual-stonith\t(stonith:fence_virt):\t Starting" out: " * virtual-stonith\t(stonith:fence_virt):\t Starting"
err: "" err: ""
- id: test_disable_minimal_input_stonith_not_exists - id: test_disable_minimal_input_stonith_not_exists
input: input:
state: disabled state: disabled
@ -179,16 +179,16 @@ test_cases:
msg: "pcs failed with error (rc=1): Error: Resource 'virtual-stonith' does not exist." msg: "pcs failed with error (rc=1): Error: Resource 'virtual-stonith' does not exist."
mocks: mocks:
run_command: run_command:
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 1 rc: 1
out: "" out: ""
err: "" err: ""
- command: ["/testbin/pcs", stonith, disable, virtual-stonith] - command: ["/testbin/pcs", stonith, disable, virtual-stonith]
environ: *env-def environ: *env-def
rc: 1 rc: 1
out: "" out: ""
err: "Error: Resource 'virtual-stonith' does not exist." err: "Error: Resource 'virtual-stonith' does not exist."
- id: test_disable_minimal_input_stonith_exists - id: test_disable_minimal_input_stonith_exists
input: input:
state: disabled state: disabled
@ -199,18 +199,18 @@ test_cases:
value: " * virtual-stonith\t(stonith:fence_virt):\t Stopped (disabled)" value: " * virtual-stonith\t(stonith:fence_virt):\t Stopped (disabled)"
mocks: mocks:
run_command: run_command:
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: " * virtual-stonith\t(stonith:fence_virt):\t Started" out: " * virtual-stonith\t(stonith:fence_virt):\t Started"
err: "" err: ""
- command: ["/testbin/pcs", stonith, disable, virtual-stonith] - command: ["/testbin/pcs", stonith, disable, virtual-stonith]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: "" out: ""
err: "" err: ""
- command: ["/testbin/pcs", stonith, status, virtual-stonith] - command: ["/testbin/pcs", stonith, status, virtual-stonith]
environ: *env-def environ: *env-def
rc: 0 rc: 0
out: " * virtual-stonith\t(stonith:fence_virt):\t Stopped (disabled)" out: " * virtual-stonith\t(stonith:fence_virt):\t Stopped (disabled)"
err: "" err: ""