Merge pull request #969 from moreati/transport_config_python_undiscover

tests: Speed up transport config tests by avoiding interpreter discovery

.ci/ci_lib.py

@@ -300,6 +300,9 @@ def proc_is_docker(pid):
 
 
 def get_interesting_procs(container_name=None):
+    """
+    Return a list of (pid, line) tuples for processes considered interesting.
+    """
     args = ['ps', 'ax', '-oppid=', '-opid=', '-ocomm=', '-ocommand=']
     if container_name is not None:
         args = ['docker', 'exec', container_name] + args
@@ -312,6 +315,9 @@ def get_interesting_procs(container_name=None):
                 any(comm.startswith(s) for s in INTERESTING_COMMS) or
                 'mitogen:' in rest
             ) and
+            (
+                'WALinuxAgent' not in rest
+            ) and
             (
                 container_name is not None or
                 (not proc_is_docker(pid))

ansible_mitogen/connection.py

@@ -484,6 +484,7 @@ class Connection(ansible.plugins.connection.ConnectionBase):
     login_context = None
 
     #: Only sudo, su, and doas are supported for now.
+    # Ansible ConnectionBase attribute, removed in Ansible >= 2.8
     become_methods = ['sudo', 'su', 'doas']
 
     #: Dict containing init_child() return value as recorded at startup by
@@ -521,15 +522,6 @@ class Connection(ansible.plugins.connection.ConnectionBase):
     # set by `_get_task_vars()` for interpreter discovery
     _action = None
 
-    def __del__(self):
-        """
-        Ansible cannot be trusted to always call close() e.g. the synchronize
-        action constructs a local connection like this. So provide a destructor
-        in the hopes of catching these cases.
-        """
-        # https://github.com/dw/mitogen/issues/140
-        self.close()
-
     def on_action_run(self, task_vars, delegate_to_hostname, loader_basedir):
         """
         Invoked by ActionModuleMixin to indicate a new task is about to start
@@ -684,6 +676,9 @@ class Connection(ansible.plugins.connection.ConnectionBase):
 
     @property
     def connected(self):
+        """
+        Ansible connection plugin property. Used by ansible-connection command.
+        """
         return self.context is not None
 
     def _spec_from_via(self, proxied_inventory_name, via_spec):
@@ -842,7 +837,11 @@ class Connection(ansible.plugins.connection.ConnectionBase):
         the _connect_*() service calls defined above to cause the master
         process to establish the real connection on our behalf, or return a
         reference to the existing one.
+
+        Ansible connection plugin method.
         """
+        # In some Ansible connection plugins this method returns self.
+        # However nothing I've found uses it, it's not even assigned.
         if self.connected:
             return
 
@@ -880,6 +879,8 @@ class Connection(ansible.plugins.connection.ConnectionBase):
         Arrange for the mitogen.master.Router running in the worker to
         gracefully shut down, and wait for shutdown to complete. Safe to call
         multiple times.
+
+        Ansible connection plugin method.
         """
         self._put_connection()
         if self.binding:
@@ -896,6 +897,8 @@ class Connection(ansible.plugins.connection.ConnectionBase):
         any local state we hold for the connection, returns the Connection to
         the 'disconnected' state, and informs ContextService the connection is
         bad somehow, and should be shut down and discarded.
+
+        Ansible connection plugin method.
         """
         if self._play_context.remote_addr is None:
             # <2.5.6 incorrectly populate PlayContext for reset_connection
@@ -1002,6 +1005,8 @@ class Connection(ansible.plugins.connection.ConnectionBase):
             Data to supply on ``stdin`` of the process.
         :returns:
             (return code, stdout bytes, stderr bytes)
+
+        Ansible connection plugin method.
         """
         emulate_tty = (not in_data and sudoable)
         rc, stdout, stderr = self.get_chain().call(
@@ -1027,6 +1032,8 @@ class Connection(ansible.plugins.connection.ConnectionBase):
             Remote filesystem path to read.
         :param str out_path:
             Local filesystem path to write.
+
+        Ansible connection plugin method.
         """
         self._connect()
         ansible_mitogen.target.transfer_file(
@@ -1076,6 +1083,8 @@ class Connection(ansible.plugins.connection.ConnectionBase):
             Local filesystem path to read.
         :param str out_path:
             Remote filesystem path to write.
+
+        Ansible connection plugin method.
         """
         try:
             st = os.stat(in_path)

docs/changelog.rst

@@ -22,6 +22,11 @@ v0.3.4.dev0
 
 * :gh:issue:`929` Support Ansible 6 and ansible-core 2.13
 * :gh:issue:`832` Fix runtime error when using the ansible.builtin.dnf module multiple times
+* :gh:issue:`925` :class:`ansible_mitogen.connection.Connection` no longer tries to close the 
+  connection on destruction. This is expected to reduce cases of `mitogen.core.Error: An attempt
+  was made to enqueue a message with a Broker that has already exitted`. However it may result in
+  resource leaks.
+
 
 v0.3.3 (2022-06-03)
 -------------------

tests/ansible/hosts/transport_config.hosts

@@ -1,55 +1,76 @@
 # integration/transport_config
 # Hosts with twiddled configs that need to be checked somehow.
 
+[transport_config:children]
+transport_config_undiscover
+tc_python_path
 
-# tansport()
+[transport_config_undiscover:children]
+tc_become
+tc_become_method
+tc_become_pass
+tc_become_user
+tc_password
+tc_port
+tc_remote_addr
+tc_remote_user
+tc_transport
+
+[transport_config_undiscover:vars]
+# If python interpreter path is unset, Ansible tries to connect & discover it.
+# That causes approx 10 seconds timeout per task - there's no host to connect to.
+# This optimisation should not be relied in any test.
+# Note: tc-python-path-* are intentionally not included.
+ansible_python_interpreter = python3000  # Not expected to exist
+
+[tc_transport]
 tc-transport-unset
 tc-transport-local ansible_connection=local
 tc-transport-smart ansible_connection=smart
 
-# python_path()
+[tc_python_path]
 tc-python-path-unset
 tc-python-path-hostvar  ansible_python_interpreter=/hostvar/path/to/python
 tc-python-path-local-unset ansible_connection=local
 tc-python-path-local-explicit ansible_connection=local ansible_python_interpreter=/a/b/c
 
-# remote_addr()
+[tc_remote_addr]
 tc-remote-addr-unset  # defaults to inventory_hostname
 tc-remote-addr-explicit-ssh ansible_ssh_host=ansi.ssh.host
 tc-remote-addr-explicit-host ansible_host=ansi.host
 tc-remote-addr-explicit-both ansible_ssh_host=a.b.c ansible_host=b.c.d
 
-# password()
+[tc_password]
 tc-password-unset
 tc-password-explicit-ssh  ansible_ssh_pass=ansi-ssh-pass
 tc-password-explicit-pass ansible_password=ansi-pass
 tc-password-explicit-both ansible_password=a.b.c ansible_ssh_pass=c.b.a
 
-# remote_user()
+[tc_remote_user]
 tc-remote-user-unset  # defaults to C.DEFAULT_REMOTE_USER
 tc-remote-user-explicit-ssh  ansible_ssh_user=ansi-ssh-user
 tc-remote-user-explicit-user ansible_user=ansi-user
 tc-remote-user-explicit-both ansible_user=a.b.c ansible_ssh_user=c.b.a
 
-# become()
+[tc_become]
 tc-become-unset
 tc-become-set
 
-# become_method()
+[tc_become_method]
 tc-become-method-unset
 tc-become-method-su ansible_become_method=su
 
-# become_user()
+[tc_become_user]
 tc-become-user-unset
 tc-become-user-set ansible_become_user=ansi-become-user
 
-# become_pass()
+[tc_become_pass]
 tc-become-pass-unset
 tc-become-pass-password ansible_become_password=apassword
 tc-become-pass-pass ansible_become_pass=apass
 tc-become-pass-both ansible_become_pass=bpass ansible_become_password=bpassword
 
-# port()
+[tc_port]
 tc-port-unset
 tc-port-explicit-port ansible_port=1234
 tc-port-explicit-ssh ansible_ssh_port=4321