From d033f7b057f55402613c516b80e31d18c57bc07d Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Tue, 10 Dec 2024 15:28:01 +0000 Subject: [PATCH 1/4] ansible_mitogen: Restore dummy objects in Connection.reset() The previous commit (53b4881628a3f08eb9488b087fbf6ca8d7357674 in PR 1200) was not intended to change these values, but some WIP slipped through. This partially reverts that commit so the two changes (moving the monkey patch, making the monkey patch more capable) exist in distinct commits. --- ansible_mitogen/connection.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index a043ef1df..90ddb7ed7 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -948,11 +948,11 @@ def reset(self): # have an action object, which we need for interpreter_discovery. # Create a temporary action object for this purpose. self._action = ansible_mitogen.mixins.ActionModuleMixin( - task=task, + task=0, connection=self, play_context=self._play_context, - loader=templar._loader, - templar=templar, + loader=0, + templar=0, shared_loader_obj=0, ) self._action_monkey_patched_by_mitogen = True From 941da317357ad343a758a1a1a4a2de10b4ad5826 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Tue, 10 Dec 2024 15:50:57 +0000 Subject: [PATCH 2/4] build: Reduce macOS job timeout to mitigate BlockingIOError This will mitigate the impact of #1185 a little, which causes the job to continue running without making progress, until it hits this timeout. Successful jobs are typically completing in 8 - 12 minutes. refs #1185 --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 24b70c64b..9a275a00c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -162,7 +162,7 @@ jobs: macos: # https://github.com/actions/runner-images/blob/main/images/macos/macos-13-Readme.md runs-on: macos-13 - timeout-minutes: 120 + timeout-minutes: 15 strategy: fail-fast: false From 6900e88dfdf4e77d4800dae8b48bf82e9b7ad419 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Tue, 10 Dec 2024 16:15:06 +0000 Subject: [PATCH 3/4] ansible_mitogen: Fix templated python interpreter with `meta: reset_connection` refs #1079 --- ansible_mitogen/connection.py | 6 +++--- docs/changelog.rst | 2 ++ .../issue_1079__wait_for_connection_timeout.yml | 12 ++++++++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index 90ddb7ed7..a043ef1df 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -948,11 +948,11 @@ def reset(self): # have an action object, which we need for interpreter_discovery. # Create a temporary action object for this purpose. self._action = ansible_mitogen.mixins.ActionModuleMixin( - task=0, + task=task, connection=self, play_context=self._play_context, - loader=0, - templar=0, + loader=templar._loader, + templar=templar, shared_loader_obj=0, ) self._action_monkey_patched_by_mitogen = True diff --git a/docs/changelog.rst b/docs/changelog.rst index 0b83a04cc..9add8ba09 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -23,6 +23,8 @@ In progress (unreleased) * :gh:issue:`1079` :mod:`ansible_mitogen`: Fix :ans:mod:`wait_for_connection` timeout with templated ``ansible_python_interpreter`` +* :gh:issue:`1079` :mod:`ansible_mitogen`: Fix templated python interpreter + with `meta: reset_connection` v0.3.19 (2024-12-02) diff --git a/tests/ansible/regression/issue_1079__wait_for_connection_timeout.yml b/tests/ansible/regression/issue_1079__wait_for_connection_timeout.yml index 2ff1899bd..6ded03ea2 100644 --- a/tests/ansible/regression/issue_1079__wait_for_connection_timeout.yml +++ b/tests/ansible/regression/issue_1079__wait_for_connection_timeout.yml @@ -8,3 +8,15 @@ tags: - issue_1079 - wait_for_connection + +- hosts: issue1079 + gather_facts: false + tasks: + - meta: reset_connection + - name: Wait for connection after reset_connection + wait_for_connection: + timeout: 5 + tags: + - issue_1079 + - reset_connection + - wait_for_connection From 5e6d7bf4fb348b01889e111323c4c18211a684e9 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Tue, 10 Dec 2024 17:28:46 +0000 Subject: [PATCH 4/4] ansible_mitogen: Templated connection timeout Ansible >= 4 (ansible-core >= 2.11) the SSH plugin has a `timeout` option and with variable `ansible_ssh_timeout`, but not a `ansible_timeout` variable. The local plugin has no such option or variable(s). However `ansible_timeout` is backfilled for all conection plugins, by legacy mechanisms that populate the play context attribute: - `ansible.constants.COMMON_CONNECTION_VARS` - `ansible.constants.MAGIC_VARIABLE_MAPPING` The `timeout` keyword is for task completion timeout, not connection timeout. --- ansible_mitogen/connection.py | 16 ++++++------ ansible_mitogen/strategy.py | 26 ++++++++++++++++--- ansible_mitogen/transport_config.py | 8 ++---- docs/changelog.rst | 2 ++ tests/ansible/hosts/default.hosts | 1 + .../ssh/templated_by_play_taskvar.yml | 2 ++ ..._109__target_has_old_ansible_installed.yml | 1 + tests/ansible/templates/test-targets.j2 | 1 + 8 files changed, 40 insertions(+), 17 deletions(-) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index a043ef1df..5053a5f5b 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -145,7 +145,7 @@ def _connect_ssh(spec): 'identity_file': private_key_file, 'identities_only': False, 'ssh_path': spec.ssh_executable(), - 'connect_timeout': spec.ansible_ssh_timeout(), + 'connect_timeout': spec.timeout(), 'ssh_args': spec.ssh_args(), 'ssh_debug_level': spec.mitogen_ssh_debug_level(), 'remote_name': get_remote_name(spec), @@ -169,7 +169,7 @@ def _connect_buildah(spec): 'username': spec.remote_user(), 'container': spec.remote_addr(), 'python_path': spec.python_path(), - 'connect_timeout': spec.ansible_ssh_timeout() or spec.timeout(), + 'connect_timeout': spec.timeout(), 'remote_name': get_remote_name(spec), } } @@ -185,7 +185,7 @@ def _connect_docker(spec): 'username': spec.remote_user(), 'container': spec.remote_addr(), 'python_path': spec.python_path(rediscover_python=True), - 'connect_timeout': spec.ansible_ssh_timeout() or spec.timeout(), + 'connect_timeout': spec.timeout(), 'remote_name': get_remote_name(spec), } } @@ -200,7 +200,7 @@ def _connect_kubectl(spec): 'kwargs': { 'pod': spec.remote_addr(), 'python_path': spec.python_path(), - 'connect_timeout': spec.ansible_ssh_timeout() or spec.timeout(), + 'connect_timeout': spec.timeout(), 'kubectl_path': spec.mitogen_kubectl_path(), 'kubectl_args': spec.extra_args(), 'remote_name': get_remote_name(spec), @@ -218,7 +218,7 @@ def _connect_jail(spec): 'username': spec.remote_user(), 'container': spec.remote_addr(), 'python_path': spec.python_path(), - 'connect_timeout': spec.ansible_ssh_timeout() or spec.timeout(), + 'connect_timeout': spec.timeout(), 'remote_name': get_remote_name(spec), } } @@ -234,7 +234,7 @@ def _connect_lxc(spec): 'container': spec.remote_addr(), 'python_path': spec.python_path(), 'lxc_attach_path': spec.mitogen_lxc_attach_path(), - 'connect_timeout': spec.ansible_ssh_timeout() or spec.timeout(), + 'connect_timeout': spec.timeout(), 'remote_name': get_remote_name(spec), } } @@ -250,7 +250,7 @@ def _connect_lxd(spec): 'container': spec.remote_addr(), 'python_path': spec.python_path(), 'lxc_path': spec.mitogen_lxc_path(), - 'connect_timeout': spec.ansible_ssh_timeout() or spec.timeout(), + 'connect_timeout': spec.timeout(), 'remote_name': get_remote_name(spec), } } @@ -273,7 +273,7 @@ def _connect_podman(spec): 'username': spec.remote_user(), 'container': spec.remote_addr(), 'python_path': spec.python_path(rediscover_python=True), - 'connect_timeout': spec.ansible_ssh_timeout() or spec.timeout(), + 'connect_timeout': spec.timeout(), 'remote_name': get_remote_name(spec), } } diff --git a/ansible_mitogen/strategy.py b/ansible_mitogen/strategy.py index c319f3e11..440e58112 100644 --- a/ansible_mitogen/strategy.py +++ b/ansible_mitogen/strategy.py @@ -328,8 +328,12 @@ def run(self, iterator, play_context, result=0): finally: ansible_mitogen.process.set_worker_model(None) - def _smuggle_to_connction_reset(self, task, play_context, iterator, target_host): - # Workaround for https://github.com/ansible/ansible/issues/84238 + def _smuggle_to_connection_reset(self, task, play_context, iterator, target_host): + """ + Create a templar and make it available for use in Connection.reset(). + This allows templated connection variables to be used when Mitogen + reconstructs its connection stack. + """ variables = self._variable_manager.get_vars( play=iterator._play, host=target_host, task=task, _hosts=self._hosts_cache, _hosts_all=self._hosts_cache_all, @@ -337,13 +341,29 @@ def _smuggle_to_connction_reset(self, task, play_context, iterator, target_host) templar = ansible.template.Templar( loader=self._loader, variables=variables, ) + + # Required for remote_user option set by variable (e.g. ansible_user). + # Without it remote_user in ansible.cfg gets used. + play_context = play_context.set_task_and_variable_override( + task=task, variables=variables, templar=templar, + ) + play_context.post_validate(templar=templar) + + # Required for timeout option set by variable (e.g. ansible_timeout). + # Without it the task timeout keyword (default: 0) gets used. + play_context.update_vars(variables) + + # Stash the task and templar somewhere Connection.reset() can find it play_context.vars.update({ '_mitogen.smuggled.reset_connection': (task, templar), }) + return play_context def _execute_meta(self, task, play_context, iterator, target_host): if task.args['_raw_params'] == 'reset_connection': - self._smuggle_to_connction_reset(task, play_context, iterator, target_host) + play_context = self._smuggle_to_connection_reset( + task, play_context, iterator, target_host, + ) return super(StrategyMixin, self)._execute_meta( task, play_context, iterator, target_host, diff --git a/ansible_mitogen/transport_config.py b/ansible_mitogen/transport_config.py index 2218a7fa7..937755528 100644 --- a/ansible_mitogen/transport_config.py +++ b/ansible_mitogen/transport_config.py @@ -513,14 +513,10 @@ def ssh_executable(self): return self._connection_option('ssh_executable') def timeout(self): - return self._play_context.timeout + return self._connection_option('timeout') def ansible_ssh_timeout(self): - return ( - self._connection.get_task_var('ansible_timeout') or - self._connection.get_task_var('ansible_ssh_timeout') or - self.timeout() - ) + return self.timeout() def ssh_args(self): return [ diff --git a/docs/changelog.rst b/docs/changelog.rst index 9add8ba09..23d3b327d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -25,6 +25,8 @@ In progress (unreleased) timeout with templated ``ansible_python_interpreter`` * :gh:issue:`1079` :mod:`ansible_mitogen`: Fix templated python interpreter with `meta: reset_connection` +* :gh:issue:`1083` :mod:`ansible_mitogen`: Templated connection timeout + (e.g. ``ansible_timeout``). v0.3.19 (2024-12-02) diff --git a/tests/ansible/hosts/default.hosts b/tests/ansible/hosts/default.hosts index a232c6ab0..3d20e73df 100644 --- a/tests/ansible/hosts/default.hosts +++ b/tests/ansible/hosts/default.hosts @@ -52,3 +52,4 @@ tt-port ansible_host=localhost ansible_password=has_sudo_ tt-private-key-file ansible_host=localhost ansible_private_key_file="{{ git_basedir }}/tests/data/docker/mitogen__has_sudo_pubkey.key" ansible_user=mitogen__has_sudo_pubkey tt-remote-user ansible_host=localhost ansible_password=has_sudo_nopw_password ansible_user="{{ 'mitogen__has_sudo_nopw' | trim }}" tt-ssh-executable ansible_host=localhost ansible_password=has_sudo_nopw_password ansible_ssh_executable="{{ 'ssh' | trim }}" ansible_user=mitogen__has_sudo_nopw +tt-timeout ansible_host=localhost ansible_password=has_sudo_nopw_password ansible_timeout="{{ 5 | int }}" ansible_user=mitogen__has_sudo_nopw diff --git a/tests/ansible/integration/ssh/templated_by_play_taskvar.yml b/tests/ansible/integration/ssh/templated_by_play_taskvar.yml index 4d7e318e9..c5c2e5443 100644 --- a/tests/ansible/integration/ssh/templated_by_play_taskvar.yml +++ b/tests/ansible/integration/ssh/templated_by_play_taskvar.yml @@ -7,6 +7,7 @@ ansible_password: "{{ 'has_sudo_nopw_password' | trim }}" ansible_port: "{{ hostvars[groups['test-targets'][0]].ansible_port | default(22) }}" ansible_ssh_executable: "{{ 'ssh' | trim }}" + ansible_timeout: "{{ 5 | int }}" ansible_user: "{{ 'mitogen__has_sudo_nopw' | trim }}" tasks: @@ -23,6 +24,7 @@ ansible_private_key_file: "{{ git_basedir }}/tests/data/docker/mitogen__has_sudo_pubkey.key" ansible_port: "{{ hostvars[groups['test-targets'][0]].ansible_port | default(22) }}" ansible_ssh_executable: "{{ 'ssh' | trim }}" + ansible_timeout: "{{ 5 | int }}" ansible_user: "{{ 'mitogen__has_sudo_pubkey' | trim }}" tasks: diff --git a/tests/ansible/regression/issue_109__target_has_old_ansible_installed.yml b/tests/ansible/regression/issue_109__target_has_old_ansible_installed.yml index 064832ece..a7ae0908c 100644 --- a/tests/ansible/regression/issue_109__target_has_old_ansible_installed.yml +++ b/tests/ansible/regression/issue_109__target_has_old_ansible_installed.yml @@ -26,6 +26,7 @@ - env.cwd == ansible_user_dir - (not env.mitogen_loaded) or (env.python_path.count("") == 1) fail_msg: | + ansible_user_dir={{ ansible_user_dir }} env={{ env }} - name: Run some new-style from ansible.module_utils... modules diff --git a/tests/ansible/templates/test-targets.j2 b/tests/ansible/templates/test-targets.j2 index 87b0e7c90..65f2fd7d9 100644 --- a/tests/ansible/templates/test-targets.j2 +++ b/tests/ansible/templates/test-targets.j2 @@ -86,3 +86,4 @@ tt-port ansible_host={{ tt.hostname }} ansible_password=h tt-private-key-file ansible_host={{ tt.hostname }} ansible_port={{ tt.port }} ansible_private_key_file="{{ '{{' }} git_basedir {{ '}}' }}/tests/data/docker/mitogen__has_sudo_pubkey.key" ansible_python_interpreter={{ tt.python_path }} ansible_user=mitogen__has_sudo_pubkey tt-remote-user ansible_host={{ tt.hostname }} ansible_password=has_sudo_nopw_password ansible_port={{ tt.port }} ansible_python_interpreter={{ tt.python_path }} ansible_user="{{ '{{' }} 'mitogen__has_sudo_nopw' | trim {{ '}}' }}" tt-ssh-executable ansible_host={{ tt.hostname }} ansible_password=has_sudo_nopw_password ansible_port={{ tt.port }} ansible_python_interpreter={{ tt.python_path }} ansible_ssh_executable="{{ '{{' }} 'ssh' | trim {{ '}}' }}" ansible_user=mitogen__has_sudo_nopw +tt-timeout ansible_host={{ tt.hostname }} ansible_password=has_sudo_nopw_password ansible_port={{ tt.port }} ansible_python_interpreter={{ tt.python_path }} ansible_timeout="{{ '{{' }} 5 | int {{ '}}' }}" ansible_user=mitogen__has_sudo_nopw