Merge pull request #1201 from moreati/issue1083-timeout

ansible_mitogen: Templated connection timeout
mitogen-hq · Dec 18, 2024 · e8005ec · e8005ec
2 parents 7a828e7 + 5e6d7bf
commit e8005ec
Show file tree

Hide file tree

Showing 10 changed files with 55 additions and 18 deletions.
diff --git 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

diff --git 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
@@ -328,22 +328,42 @@ 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,
         )
         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
@@ -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
@@ -23,6 +23,10 @@ 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`
+* :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
@@ -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
@@ -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_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
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
@@ -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
@@ -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