diff --git a/changelogs/fragments/9318-fstr-actionplugins.yml b/changelogs/fragments/9318-fstr-actionplugins.yml new file mode 100644 index 0000000000..7df54f3c19 --- /dev/null +++ b/changelogs/fragments/9318-fstr-actionplugins.yml @@ -0,0 +1,3 @@ +minor_changes: + - iptables_state action plugin - use f-strings instead of interpolations or ``format`` (https://github.com/ansible-collections/community.general/pull/9318). + - shutdown action plugin - use f-strings instead of interpolations or ``format`` (https://github.com/ansible-collections/community.general/pull/9318). diff --git a/plugins/action/iptables_state.py b/plugins/action/iptables_state.py index 5ea55af58c..39ee85d778 100644 --- a/plugins/action/iptables_state.py +++ b/plugins/action/iptables_state.py @@ -22,25 +22,33 @@ class ActionModule(ActionBase): _VALID_ARGS = frozenset(('path', 'state', 'table', 'noflush', 'counters', 'modprobe', 'ip_version', 'wait')) DEFAULT_SUDOABLE = True - MSG_ERROR__ASYNC_AND_POLL_NOT_ZERO = ( - "This module doesn't support async>0 and poll>0 when its 'state' param " - "is set to 'restored'. To enable its rollback feature (that needs the " - "module to run asynchronously on the remote), please set task attribute " - "'poll' (=%s) to 0, and 'async' (=%s) to a value >2 and not greater than " - "'ansible_timeout' (=%s) (recommended).") - MSG_WARNING__NO_ASYNC_IS_NO_ROLLBACK = ( - "Attempts to restore iptables state without rollback in case of mistake " - "may lead the ansible controller to loose access to the hosts and never " - "regain it before fixing firewall rules through a serial console, or any " - "other way except SSH. Please set task attribute 'poll' (=%s) to 0, and " - "'async' (=%s) to a value >2 and not greater than 'ansible_timeout' (=%s) " - "(recommended).") - MSG_WARNING__ASYNC_GREATER_THAN_TIMEOUT = ( - "You attempt to restore iptables state with rollback in case of mistake, " - "but with settings that will lead this rollback to happen AFTER that the " - "controller will reach its own timeout. Please set task attribute 'poll' " - "(=%s) to 0, and 'async' (=%s) to a value >2 and not greater than " - "'ansible_timeout' (=%s) (recommended).") + @staticmethod + def msg_error__async_and_poll_not_zero(task_poll, task_async, max_timeout): + return ( + "This module doesn't support async>0 and poll>0 when its 'state' param " + "is set to 'restored'. To enable its rollback feature (that needs the " + "module to run asynchronously on the remote), please set task attribute " + f"'poll' (={task_poll}) to 0, and 'async' (={task_async}) to a value >2 and not greater than " + f"'ansible_timeout' (={max_timeout}) (recommended).") + + @staticmethod + def msg_warning__no_async_is_no_rollback(task_poll, task_async, max_timeout): + return ( + "Attempts to restore iptables state without rollback in case of mistake " + "may lead the ansible controller to loose access to the hosts and never " + "regain it before fixing firewall rules through a serial console, or any " + f"other way except SSH. Please set task attribute 'poll' (={task_poll}) to 0, and " + f"'async' (={task_async}) to a value >2 and not greater than 'ansible_timeout' (={max_timeout}) " + "(recommended).") + + @staticmethod + def msg_warning__async_greater_than_timeout(task_poll, task_async, max_timeout): + return ( + "You attempt to restore iptables state with rollback in case of mistake, " + "but with settings that will lead this rollback to happen AFTER that the " + "controller will reach its own timeout. Please set task attribute 'poll' " + f"(={task_poll}) to 0, and 'async' (={task_async}) to a value >2 and not greater than " + f"'ansible_timeout' (={max_timeout}) (recommended).") def _async_result(self, async_status_args, task_vars, timeout): ''' @@ -95,18 +103,18 @@ class ActionModule(ActionBase): if module_args.get('state', None) == 'restored': if not wrap_async: if not check_mode: - display.warning(self.MSG_WARNING__NO_ASYNC_IS_NO_ROLLBACK % ( + display.warning(self.msg_error__async_and_poll_not_zero( task_poll, task_async, max_timeout)) elif task_poll: - raise AnsibleActionFail(self.MSG_ERROR__ASYNC_AND_POLL_NOT_ZERO % ( + raise AnsibleActionFail(self.msg_warning__no_async_is_no_rollback( task_poll, task_async, max_timeout)) else: if task_async > max_timeout and not check_mode: - display.warning(self.MSG_WARNING__ASYNC_GREATER_THAN_TIMEOUT % ( + display.warning(self.msg_warning__async_greater_than_timeout( task_poll, task_async, max_timeout)) @@ -119,10 +127,10 @@ class ActionModule(ActionBase): # remote and local sides (if not the same, make the loop # longer on the controller); and set a backup file path. module_args['_timeout'] = task_async - module_args['_back'] = '%s/iptables.state' % async_dir + module_args['_back'] = f'{async_dir}/iptables.state' async_status_args = dict(mode='status') - confirm_cmd = 'rm -f %s' % module_args['_back'] - starter_cmd = 'touch %s.starter' % module_args['_back'] + confirm_cmd = f"rm -f {module_args['_back']}" + starter_cmd = f"touch {module_args['_back']}.starter" remaining_time = max(task_async, max_timeout) # do work! diff --git a/plugins/action/shutdown.py b/plugins/action/shutdown.py index 01201a6405..9505cc2155 100644 --- a/plugins/action/shutdown.py +++ b/plugins/action/shutdown.py @@ -18,6 +18,10 @@ from ansible.utils.display import Display display = Display() +def fmt(mapping, key): + return to_native(mapping[key]).strip() + + class TimedOutException(Exception): pass @@ -84,31 +88,26 @@ class ActionModule(ActionBase): def get_distribution(self, task_vars): # FIXME: only execute the module if we don't already have the facts we need distribution = {} - display.debug('{action}: running setup module to get distribution'.format(action=self._task.action)) + display.debug(f'{self._task.action}: running setup module to get distribution') module_output = self._execute_module( task_vars=task_vars, module_name='ansible.legacy.setup', module_args={'gather_subset': 'min'}) try: if module_output.get('failed', False): - raise AnsibleError('Failed to determine system distribution. {0}, {1}'.format( - to_native(module_output['module_stdout']).strip(), - to_native(module_output['module_stderr']).strip())) + raise AnsibleError(f"Failed to determine system distribution. {fmt(module_output, 'module_stdout')}, {fmt(module_output, 'module_stderr')}") distribution['name'] = module_output['ansible_facts']['ansible_distribution'].lower() distribution['version'] = to_text( module_output['ansible_facts']['ansible_distribution_version'].split('.')[0]) distribution['family'] = to_text(module_output['ansible_facts']['ansible_os_family'].lower()) - display.debug("{action}: distribution: {dist}".format(action=self._task.action, dist=distribution)) + display.debug(f"{self._task.action}: distribution: {distribution}") return distribution except KeyError as ke: - raise AnsibleError('Failed to get distribution information. Missing "{0}" in output.'.format(ke.args[0])) + raise AnsibleError(f'Failed to get distribution information. Missing "{ke.args[0]}" in output.') def get_shutdown_command(self, task_vars, distribution): def find_command(command, find_search_paths): - display.debug('{action}: running find module looking in {paths} to get path for "{command}"'.format( - action=self._task.action, - command=command, - paths=find_search_paths)) + display.debug(f'{self._task.action}: running find module looking in {find_search_paths} to get path for "{command}"') find_result = self._execute_module( task_vars=task_vars, # prevent collection search by calling with ansible.legacy (still allows library/ override of find) @@ -130,42 +129,37 @@ class ActionModule(ActionBase): if is_string(search_paths): search_paths = [search_paths] - # Error if we didn't get a list - err_msg = "'search_paths' must be a string or flat list of strings, got {0}" try: incorrect_type = any(not is_string(x) for x in search_paths) if not isinstance(search_paths, list) or incorrect_type: raise TypeError except TypeError: - raise AnsibleError(err_msg.format(search_paths)) + # Error if we didn't get a list + err_msg = f"'search_paths' must be a string or flat list of strings, got {search_paths}" + raise AnsibleError(err_msg) full_path = find_command(shutdown_bin, search_paths) # find the path to the shutdown command if not full_path: # if we could not find the shutdown command - display.vvv('Unable to find command "{0}" in search paths: {1}, will attempt a shutdown using systemd ' - 'directly.'.format(shutdown_bin, search_paths)) # tell the user we will try with systemd + + # tell the user we will try with systemd + display.vvv(f'Unable to find command "{shutdown_bin}" in search paths: {search_paths}, will attempt a shutdown using systemd directly.') systemctl_search_paths = ['/bin', '/usr/bin'] full_path = find_command('systemctl', systemctl_search_paths) # find the path to the systemctl command if not full_path: # if we couldn't find systemctl raise AnsibleError( - 'Could not find command "{0}" in search paths: {1} or systemctl command in search paths: {2}, unable to shutdown.'. - format(shutdown_bin, search_paths, systemctl_search_paths)) # we give up here + f'Could not find command "{shutdown_bin}" in search paths: {search_paths} or systemctl' + f' command in search paths: {systemctl_search_paths}, unable to shutdown.') # we give up here else: - return "{0} poweroff".format(full_path[0]) # done, since we cannot use args with systemd shutdown + return f"{full_path[0]} poweroff" # done, since we cannot use args with systemd shutdown # systemd case taken care of, here we add args to the command args = self._get_value_from_facts('SHUTDOWN_COMMAND_ARGS', distribution, 'DEFAULT_SHUTDOWN_COMMAND_ARGS') # Convert seconds to minutes. If less that 60, set it to 0. delay_sec = self.delay shutdown_message = self._task.args.get('msg', self.DEFAULT_SHUTDOWN_MESSAGE) - return '{0} {1}'. \ - format( - full_path[0], - args.format( - delay_sec=delay_sec, - delay_min=delay_sec // 60, - message=shutdown_message - ) - ) + + af = args.format(delay_sec=delay_sec, delay_min=delay_sec // 60, message=shutdown_message) + return f'{full_path[0]} {af}' def perform_shutdown(self, task_vars, distribution): result = {} @@ -174,9 +168,8 @@ class ActionModule(ActionBase): self.cleanup(force=True) try: - display.vvv("{action}: shutting down server...".format(action=self._task.action)) - display.debug("{action}: shutting down server with command '{command}'". - format(action=self._task.action, command=shutdown_command_exec)) + display.vvv(f"{self._task.action}: shutting down server...") + display.debug(f"{self._task.action}: shutting down server with command '{shutdown_command_exec}'") if self._play_context.check_mode: shutdown_result['rc'] = 0 else: @@ -184,16 +177,13 @@ class ActionModule(ActionBase): except AnsibleConnectionFailure as e: # If the connection is closed too quickly due to the system being shutdown, carry on display.debug( - '{action}: AnsibleConnectionFailure caught and handled: {error}'.format(action=self._task.action, - error=to_text(e))) + f'{self._task.action}: AnsibleConnectionFailure caught and handled: {to_text(e)}') shutdown_result['rc'] = 0 if shutdown_result['rc'] != 0: result['failed'] = True result['shutdown'] = False - result['msg'] = "Shutdown command failed. Error was {stdout}, {stderr}".format( - stdout=to_native(shutdown_result['stdout'].strip()), - stderr=to_native(shutdown_result['stderr'].strip())) + result['msg'] = f"Shutdown command failed. Error was {fmt(shutdown_result, 'stdout')}, {fmt(shutdown_result, 'stderr')}" return result result['failed'] = False @@ -206,7 +196,7 @@ class ActionModule(ActionBase): # If running with local connection, fail so we don't shutdown ourself if self._connection.transport == 'local' and (not self._play_context.check_mode): - msg = 'Running {0} with local connection would shutdown the control node.'.format(self._task.action) + msg = f'Running {self._task.action} with local connection would shutdown the control node.' return {'changed': False, 'elapsed': 0, 'shutdown': False, 'failed': True, 'msg': msg} if task_vars is None: