ansible: Respect interpreter_python config variable

[extmind]

Use the global configuration variable interpreter_python when ansible_python_interpreter is unset.

Fixes #740

[stefanor]

@moreati: Thanks for the more frequent releases.

This is one of the patches that Debian has been carrying for a while. As we don't have a /usr/bin/python in the distro any more, it's common to have interpreter_python configured. I'd appreciate it if you would re-review this PR.

[moreati]

Sorry for the long wait. I've been avoiding the interpreter discovery code for far too long. I've now tackled most of templating connection options/variables and connection timeout, and I have a mcuh better understanding of the ... intricacies.

This patch looks broadly correct. I'm going to update for master, rejig a couple of details, and add some tests.

[moreati]

The failure in https://github.com/mitogen-hq/mitogen/actions/runs/12412205309/job/34651500318?pr=833

TASK [assert_equal left=out.result[0].kwargs.python_path, right=['{{out.discovered_interpreter}}']] ***
Thursday 19 December 2024  12:07:55 +0000 (0:00:00.056)       0:07:56.582 ***** 
fatal: [tc-python-path-hostvar]: FAILED! => changed=false 
  msg: |-
    Lists differ: ['/hostvar/path/to/python'] != [False]
  
    First differing element 0:
    '/hostvar/path/to/python'
    False
  
    - ['/hostvar/path/to/python']
    + [False]

is perplexng. The test is expecting ansible_mitogen.plugins.action.mitogen_get_stack.ActionModule()._connection._action._discovered_interpreter == u'/hostvar/path/to/python'. However that value could only come from https://github.com/mitogen-hq/mitogen/blob/e8005ece3ab39dacefdae81517610a9cd1ed6312/tests/ansible/hosts/transport_config.hosts#L36-L38 which by definition makes it not a discovered python interpreter.

I need to confirm the semantics of ansible.plugins.ActionBase._discovered_python in vanilla Ansible.

[moreati]

Going by https://github.com/ansible/ansible/commit/4d3a6123d5b84ed8cdaba4e945e6f1d55c35c4b4#diff-cbe4262a246b2475f3884118713f2ae5a6c49a443d9a942b6c63445bed9354dcR32

• ansible.plugins.action.ActionBase._discovered_interpreter starts False.
• It's assigned the path of an interpreter on the targert if and only if interpreter discvoery is run.
• Assignment to ActionBase._discovered_interpreter is closely tied to populating the variable discovered_interpreter_python.

[moreati]

@extmind I've arrived at a slightly different approach in #1206. I'd welcome any comments you have, here or as code review. It's based on your work, so I've credited you as co-author.

@stefanor does #1206 still fix the Debian package? If so I'll merge it and make a release.

[stefanor]

@stefanor does https://github.com/mitogen-hq/mitogen/pull/1206 still fix the Debian package? If so I'll merge it and make a release.

I tested the branch, it behaves identically to the current Debian package, and to master, from what I can see. And I don't think this means the feature is working at all in any of them. I get the notice:

[WARNING]: Platform linux on host $HOST is using the discovered Python interpreter at /usr/bin/python3.11, but future installation of another Python interpreter could change the meaning of that path. See https://docs.ansible.com/ansible-core/2.18/reference_appendices/interpreter_discovery.html for more information.

It doesn't seem to matter what I set interpreter_python to in my ~/.ansible.cfg. I can leave it unset and see the same thing.

So, at least, the issue seems to be defused, now that ansible has better autodetection. But I can't say whether that MR is a step in the right direction or not.

[moreati]

I tested the branch, it behaves identically to the current Debian package, and to master, from what I can see. And I don't think this means the feature is working at all in any of them. I get the notice:

Thanks, do I understand correctly

1. Mitogen master doesn't respect ansible.cfg/default/interpreter_python in your testing
2. Ditto with a Debian package patched using #1206
3. Ditto with with the existing Debian package (which has been already been patched using this PR)

I expected 1. I'm slightly surprised if 2 is true, but in retrospect it could make sense. I'm most surprised if 3 is true, I thought you'd previously said the Debian was carrying this PR as a patch to allow ansible.cfg/default/interpreter_python.

[moreati]

AFAICT Debian's python-mitogen source package does include this PR as a patch https://sources.debian.org/patches/python-mitogen/0.3.19-1/interpreter_python/ and it works - ansible.cfg is respected

alex@d13:~/src/mitogen$ cat ansible.cfg 
[defaults]
interpreter_python = /usr/bin/python3.12
alex@d13:~/src/mitogen$ ANSIBLE_STRATEGY_PLUGINS=/usr/lib/python3/dist-packages/ansible_mitogen/plugins/strategy/ ANSIBLE_STRATEGY=mitogen_linear ansible -i hosts.ini all -m ping
d13 | SUCCESS => {
    "changed": false,
    "ping": "pong"
}

Mitogen master doesn't respect ansible.cfg

alex@d13:~/src/mitogen$ git checkout master
Already on 'master'
Your branch is up to date with 'origin/master'.
alex@d13:~/src/mitogen$ ANSIBLE_STRATEGY_PLUGINS=ansible_mitogen/plugins/strategy/ ANSIBLE_STRATEGY=mitogen_linear ansible -i hosts.ini all -m ping
[WARNING]: Platform linux on host d13 is using the discovered Python interpreter at /usr/bin/python3.12, but future
installation of another Python interpreter could change the meaning of that path. See https://docs.ansible.com/ansible-
core/2.18/reference_appendices/interpreter_discovery.html for more information.
d13 | SUCCESS => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python3.12"
    },
    "changed": false,
    "ping": "pong"
}

Branch issue1083-python_path2 (PR #1206) does

alex@d13:~/src/mitogen$ git checkout issue1083-python_path2 
Switched to branch 'issue1083-python_path2'
Your branch is up to date with 'origin/issue1083-python_path2'.
alex@d13:~/src/mitogen$ ANSIBLE_STRATEGY_PLUGINS=ansible_mitogen/plugins/strategy/ ANSIBLE_STRATEGY=mitogen_linear ansible -i hosts.ini all -m ping
d13 | SUCCESS => {
    "changed": false,
    "ping": "pong"
}

[stefanor]

I tested:

1. Your branch with ansible 11.1.0
2. The master branch with ansbile 11.1.0
3. The Debian package in trixie (0.3.19-1 + this patch) with Debian ansible 11.1.0+dfsg-1

With: 1.

[defaults]
interpreter_python = /usr/bin/python3

[defaults]
interpreter_python = auto_legacy

3. No config (same as = auto)

Every time I get that banner, and see:

$HOST | SUCCESS => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python3.11"
    },
    "changed": false,
    "ping": "pong"
}

[stefanor]

I'm most surprised if 3 is true, I thought you'd previously said the Debian was carrying this PR as a patch to

We carry this patch, because it was necessary at the time. But I think advancements in ansible have allowed it to find our python3 binaries.

[moreati]

Something is different between our test setups. We're getting different results. I'll try to work out what over the weekend.

EDIT: added timescale

[moreati]

Every time I get that banner, and see:

$HOST | SUCCESS => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python3.11"
    },
    "changed": false,
    "ping": "pong"
}

I cannot reproduce this result.

I used tox on Debian trixie to automate setup and testing of the 9 combinations. I do not see the discovery banner or discovered_interpreter_python when 1. I provide a .cfg file containing interpreter_python = python3, and 2. I use Mitogen from Debian's patched mitogen-0.3.19-1 or from PR #1206. This is as intended.

In all other cases I do see the banner and discovered_interpreter_python. Also as intended.

The full details and setup are in https://gist.github.com/moreati/95132e5a348654f76fc567f9a609877d. If you'd like to try it yourself, then I think the following commands will suffice

sudo apt install ansible ansible-mitogen git tox
git clone https://gist.github.com/95132e5a348654f76fc567f9a609877d.git scratch
cd scratch
tox

[stefanor]

Argh, trying with a different playbook and host and I see things working. It's possible I had some magic in there.

[moreati]

extmid, stefanor. Thank you both for your effort and patience. I've merged #1206, fixing #740. Release 0.3.20 should be out in the next few hours.