ansible_mitogen: Fix usage of connection_loader__get.

[Nihlus]

This PR revives #976 and updates the proposed fix in both the SSH and kubectl connection methods.

As a short recap, get_with_context returns a named tuple as of Ansible 2.10, which necessitates a small change in how it's accessed in Mitogen.

The original PR states that this fixes #766, #662, and #342. I have verified that it resolves an issue with delegation of set_facts, but I haven't been able to get network_cli working yet.

[moreati]

Thank you for looking at this. The PR needs work to fix one or more failing tests and to include a changelog entry.

PLAY [integration/stub_connections/kubectl.yml] ********************************

TASK [include_tasks _raw_params=../_mitogen_only.yml] **************************
Saturday 11 January 2025  20:28:10 +0000 (0:00:03.327)       0:03:11.667 ****** 
included: /home/runner/work/mitogen/mitogen/tests/ansible/integration/_mitogen_only.yml for target-debian11-1, target-ubuntu2004-2

TASK [meta _raw_params=end_play] ***********************************************
Saturday 11 January 2025  20:28:10 +0000 (0:00:00.037)       0:03:11.704 ****** 
skipping: [target-debian11-1]

TASK [meta _raw_params=end_play] ***********************************************
Saturday 11 January 2025  20:28:10 +0000 (0:00:00.008)       0:03:11.712 ****** 
skipping: [target-debian11-1]

TASK [custom_python_detect_environment ] ***************************************
Saturday 11 January 2025  20:28:10 +0000 (0:00:00.007)       0:03:11.720 ****** 
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: NameError: name 'vanilla_class' is not defined. Did you mean: 'self.vanilla_class'?
fatal: [target-debian11-1]: FAILED! => 
  msg: 'Unexpected failure during module execution: name ''vanilla_class'' is not defined'
  stdout: ''

[moreati]

I have an alternative approach that may eliminate the need for ansible_mitogen.loaders.connection_loader__get. I haven't had a chance to try it out/test it

@@ -33,6 +33,7 @@ import os.path
 import sys
 
 from ansible.plugins.connection.ssh import (
+    Connection as _ansible_ssh_Connection,
     DOCUMENTATION as _ansible_ssh_DOCUMENTATION,
 )
 
@@ -55,18 +56,13 @@ except ImportError:
     del base_dir
 
 import ansible_mitogen.connection
-import ansible_mitogen.loaders
 
 
 class Connection(ansible_mitogen.connection.Connection):
     transport = 'ssh'
-    vanilla_class = ansible_mitogen.loaders.connection_loader__get(
-        'ssh',
-        class_only=True,
-    )
 
     @staticmethod
     def _create_control_path(*args, **kwargs):
         """Forward _create_control_path() to the implementation in ssh.py."""
         # https://github.com/dw/mitogen/issues/342
-        return Connection.vanilla_class._create_control_path(*args, **kwargs)
+        return _ansible_ssh_Connection._create_control_path(*args, **kwargs)

Does it work for you?

[Nihlus]

That works, though I don't think it'd work as well for the kubectl connection plugin. That one checks for a valid return from connection_loader__get as a way to detect kubectl support, and I'm not sure if pushing the error up to the import level would be the right choice.

[Nihlus]

I noticed another issue with how get_with_context is handled - Ansible's get function on the loader is actually a wrapper over get_with_context these days, so if something were to access an action via get_with_context directly, Mitogen's mixin would not be applied. I added a fix to that as well.

[Nihlus]

I'm currently working on tracking down the "Could not recover task_vars" issue - from what I've gathered thus far, it happens when the network_cli connection (which is not Mitogen) drops to the local connection (which is Mitogen) to run some raw commands. At that point, the action on the stack (gather_facts) has determined that it is not running under Mitogen and transmuted itself back to the non-mixin variant. However, we've not discovered the task vars yet, and we don't have a fallback to handle this like we do with _execute_meta.

I've tried forcing it through in various ways without any luck, and it all either boils down to the action on the stack not being a Mitogen action with a mixin while we're operating on the local Mitogen connection, or the python interpreter not having been discovered. Any input on that would be appreciated :)

[moreati]

Notes to self

• Only ansible_mitogen.plugins.connections.mitogen_ssh has the from ... import DOCUMENTATION as ... kludge. Others probably need it, or something better.
• `ansible_mitogen.plugins.connections.mitogen_kubectl.Connection.get_extra_args() does task var shenanigans. Probably related to above.
• Mitogen's test coverage of connection plugins is very sparse outside ssh and local.
• There's Ansible < 2.10 code in ansible_mitogen.plugins.connection.mitogen_kubectl.Connection.get_extra_args(). It should probably be deleted.
• mitogen_kubectl and mitogen_ssh are the only connection plugins that import ansible_mitogen.loaders
• Importing ansible_mitogen.loaders runs assert_supported_release() as a side effect. This is arguably also a kludge. ansible_mitogen.loaders was chosen for this purpose because it is very early in the order of imports when Ansible is used with Mitogen. Removing ansible_mitogen.loaders from connection plugins is probably fine, because it is also imported by the strategy plugins. Otherwise the check could be moved elsewhere, or eliminated (RFC in draft).

$ ag kubectl tests/ansible                  
tests/ansible/integration/transport/all.yml
2:- include_playbook: kubectl.yml
4:    - kubectl

tests/ansible/integration/transport/kubectl.yml
45:        ansible_connection: "kubectl"
50:- name: "Test kubectl connection (default strategy)"
73:            ansible_kubectl_container: python3
94:- name: "Test kubectl connection (mitogen strategy)"
117:            ansible_kubectl_container: python3

tests/ansible/integration/stub_connections/all.yml
1:- import_playbook: kubectl.yml

tests/ansible/integration/stub_connections/kubectl.yml
2:- name: integration/stub_connections/kubectl.yml
14:      ansible_connection: kubectl
15:      mitogen_kubectl_path: stub-kubectl.py
20:      - out.env.THIS_IS_STUB_KUBECTL == '1'
24:    - kubectl

tests/ansible/integration/stub_connections/README.md
5:tools (kubectl etc.) to verify arguments passed by Ansible to Mitogen and

[moreati]

That works, though I don't think it'd work as well for the kubectl connection plugin

Agreed. I like from ansible.plugins.connection... import Connection as ... for SSH, but kubectl will need something else.

if something were to access an action via get_with_context directly, Mitogen's mixin would not be applied. I added a fix to that as well

Good spot.

Any input on that would be appreciated

Looking, but no promises. We're in deep, at the intersection of many tight/accidental couplings. I've only recently got my head around this bit of the code.

[moreati]

+1 changing the names. ...__get = ...get_with_context has long bugged me.

[Nihlus]

Yeah, I saw that and thought like hell I'm leaving it like that hah

[moreati]

Personal preference, use absolute imports when possible. It makes it easier to distinguish what belongs to ansibler.* and what belongs to ansible_mitogen.*, or mitogen.*

[Nihlus]

Gotcha - I had some issues getting an absolute import working in this file due to other imports masking things. Tried a few more ways just now but didn't get anywhere.

[moreati]

I tried some variations in moreati@25da0f7. We could eliminate this import entirely by using

result = ansible_mitogen.loaders.action_loader__get_with_context(...)
...
return result._replace(object=adorned_klass)

It loses a little clarity. I'm in favour of the tradeoff, but ready to be convinced otherwise.

[moreati]

I'm currently working on tracking down the "Could not recover task_vars" issue

Which issue is this? Do you have a playbook or other way to reproduce it?

[moreati]

I'm currently working on tracking down the "Could not recover task_vars" issue

Which issue is this? Do you have a playbook or other way to reproduce it?

Probably a reference to one or more of the issues (#766, #662, #342) that this PR (derived from #976 by @markafarrell) may solve. There are repro instructions in https://github.com/markafarrell/mitogen-issue-766-repro, mentioned in #976 (comment).