Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for special ansible_python_interpreter values, ansible_python_interpreter discovery, and fixes tests #658

Merged
merged 121 commits into from
Mar 22, 2020

Conversation

s1113950
Copy link
Collaborator

@s1113950 s1113950 commented Oct 29, 2019

Fixes #597 , fixes #680 , fixes #673

adds support for Jinja templating in ansible_python_interpreter, adds support for ansible_python_interpreter discovery, and fixes up some test issues I ran into in order to get this PR passing (tests hadn't passed on a PR since last year).

Additionally I had to update the Mac VM to 10.14; MS said they'll stop supporting 10.13 soon: https://docs.microsoft.com/en-us/azure/devops/pipelines/agents/hosted?view=azure-devops#use-a-microsoft-hosted-agent and also updated away from ubuntu 16.04:
https://dev.azure.com/dw-mitogen/Mitogen/_build/results?buildId=793&view=logs&j=364fd14b-95c5-5834-280a-1629ce194374&t=2241d271-1077-5b70-19c7-218604a5902c

E: Failed to fetch https://packages.microsoft.com/ubuntu/16.04/prod/dists/xenial/main/binary-amd64/Packages.bz2  Hash Sum mismatch
E: Some index files failed to download. They have been ignored, or old ones used instead.
Status: Downloaded newer image for mitogen/debian-test:latest

sometimes this happened... 🤦‍♂

I ported over the tests found here: https://github.com/ansible/ansible/pull/50163/files#diff-2efff5ba3589af60c1010f00a234ef57R1 from ansible where interpreter discovery was first added, and added them to the CI to verify interpreter discovery works.

Also, I leveraged the existing discover_interpreter function from Ansible, which means that interpreter discovery won't work in Ansible < 2.8.0 but it already doesn't work in those Ansible versions so I didn't think this was an issue.

Finally, I disabled a bunch of tests that worked locally but wouldn't pass on CI. I made new github issues out of the fails and labeled them with ci.

@s1113950
Copy link
Collaborator Author

s1113950 commented Oct 29, 2019

If I'm going about this the wrong way please let me know; I ran into an issue where this was broken (but not when using vanilla ansible):
ansible_python_interpreter: source /opt/rh/rh-python36/enable && python
and noticed the created ssh command looked a bit off so thought that'd be a good place for the fix.

@s1113950 s1113950 changed the title Adds support for special python interpreters Adds support for special ansible_python_interpreter values Oct 30, 2019
@s1113950
Copy link
Collaborator Author

I'm gonna run more tests on my end and then add a test for the new functionality + docs.
This so far has passed this command:

- name: pull down docker binaries
  get_url:
    url: https://download.docker.com/linux/static/stable/x86_64/docker-{{ docker_version }}.tgz
    dest: /usr/local/share
    mode: 0644
  vars:
    ansible_python_interpreter: source /opt/rh/rh-python36/enable && python

@s1113950
Copy link
Collaborator Author

Fails on a more complex command:

{"changed": false, "msg": "EOF on stream; last 100 lines received:\nroot@IP's password: \nbash: {%: command not found", "unreachable": true}
- name: download helm and tiller
  get_url:
    url: "https://get.helm.sh/helm-v2.14.1-linux-amd64.tar.gz"
    dest: "/usr/local/src/helm-v2.14.1-linux-amd64.tar.gz"
  vars:
    ansible_python_interpreter: >
      {% if ansible_facts['distribution'] == "CentOS" and ansible_facts['distribution_major_version'] == "6" %}
        source /opt/rh/rh-python36/enable && python
      {% else %}
        python 
      {% endif %}

I'll try and fix that up as well

@s1113950 s1113950 marked this pull request as ready for review October 30, 2019 21:28
@s1113950
Copy link
Collaborator Author

PR ready! Passed all automation on my side, and passes dw.mitogen here. Travis says it's over its data quota which is why it's failing. I'll add some documentation now

@s1113950
Copy link
Collaborator Author

@dw this PR is ready for review now 😄

@s1113950
Copy link
Collaborator Author

Traceback (most recent call last):
  File "/home/vsts/work/1/s/tests/testlib.py", line 520, in tearDownClass
    cls.dockerized_ssh.check_processes()
  File "/home/vsts/work/1/s/tests/testlib.py", line 469, in check_processes
    counts
AssertionError: Docker container 'mitogen-test-9f743adbdb483a88' contained extra running processes after test completed: {'sshd': 1, 'doas <defunct>': 1, 'ps': 1}

I'll rerun tests; commit adding a few sentences to a readme file is unrelated to ^^^

@s1113950
Copy link
Collaborator Author

s1113950 commented Oct 30, 2019

I haven't used azure devops before, not sure how to kick off a rebuild. There's no rebuild button that I can see from https://dev.azure.com/dw-mitogen/Mitogen/_build/results?buildId=696&view=ms.vss-test-web.build-test-results-tab . The docs say that rebuild is available in the latest version of visual studio test task but in the example pic they show from here: https://docs.microsoft.com/en-us/azure/devops/pipelines/test/review-continuous-test-results-after-build?view=azure-devops there is also no rebuild button

@bradh352
Copy link

Any chance you can provide your patch without the formatting cleanups to code you didn't touch. Its actually fairly hard to see your changes.

Also, is this supposed to support the things like ansible_python_interpreter=auto special flags like are documented here:
https://docs.ansible.com/ansible/latest/reference_appendices/interpreter_discovery.html
I didn't immediately see how your changes would accommodate that which I believe is the primary issue at this time.

@ssbarnea
Copy link

This, alongside ansible 2.9 are two critical features that prevent me from attempting to test it as I know I need both.

@s1113950
Copy link
Collaborator Author

For sure! Sorry for the late reply, I recently came back from vacation and have been really busy since so haven't had time to work on this yet.

The formatting changes are due to my editor auto-linting files I save if I made any changes to that file in general. I will disable my auto-linter and revert the pep8 formatting changes though.

Lastly, I don't think this patch will fix special interpreter values of things like auto at the moment. I focused on supporting complex python interpreter commands like sourceRHPythonHere && python as well as Jinja templating in the ansible_python_interpreter but I will also try and tackle things like auto as well.

I will be able to work on changes to this PR soon!

@s1113950
Copy link
Collaborator Author

s1113950 commented Feb 1, 2020

(Epdb) found_interpreters
['/usr/bin/python', '/opt/rh/python27/root/usr/bin/python2.7', '/usr/bin/python2.6', '/usr/bin/python3', '/opt/rh/python27/root/usr/bin/python']

able to detect interpreters now! My test cases taken mostly from https://github.com/ansible/ansible/pull/50163/files#diff-2efff5ba3589af60c1010f00a234ef57 still don't fully pass yet though

@s1113950
Copy link
Collaborator Author

Now the other test failed D: these tests are too flaky...

@s1113950
Copy link
Collaborator Author

travis is under maintenance now. Once it's back up, if tests are solid I'll wait a day or two for more people to try the patch, but I really want to get it in soon so I can start getting ansible 2.9 support working (this PR also fixes tests which is why it should come in first)

@s1113950 s1113950 merged commit a5fe4a9 into mitogen-hq:master Mar 22, 2020
@s1113950 s1113950 deleted the complexAnsiblePythonInterpreterArg branch March 22, 2020 05:27
@cbrum11
Copy link

cbrum11 commented Apr 18, 2020

Hello,

I currently have this same issue using your latest commit. I'm using Ansible 2.9.6

Here's a line of the -vvvv showing it's still trying to use /usr/bin/python

[mux  19549] 09:18:12.658834 D mitogen: <Side of ssh.10.184.2.61 fd 85>: empty read, disconnecting
[mux  19549] 09:18:12.659361 D mitogen.parent: failing connection ssh.10.184.2.61 due to EofError("EOF on stream; last 100 lines received:\namp-admin@10.184.2.61's password: \nbash: /usr/bin/python: No such file or directory",)

Here's my ansible.cfg (as you can see I also tried setting to auto with the same results)

[defaults]
# Set reasonable default command line flags
vault_password_file = vault-sops-client.py
inventory = amp_database_using_serial.yaml
host_key_checking = False
ansible_python_interpreter = /usr/bin/python3
#ansible_python_interpreter = auto

# Use the YAML callback plugin.
stdout_callback = yaml

# Use the stdout_callback when running ad-hoc commands.
bin_ansible_callbacks = True

Here's my pertinent environment variables

export ANSIBLE_INVENTORY_PLUGINS=/home/chase/dev/axon/ansible-playbooks/inventory_plugins

export ANSIBLE_STRATEGY=mitogen_linear
export ANSIBLE_STRATEGY_PLUGINS=/home/chase/.cache/pypoetry/virtualenvs/amp-ansible-lib-aZl7EoP--py3.6/src/mitogen/ansible_mitogen/plugins/strategy

export ANSIBLE_HOST_KEY_CHECKING=False

On the target machine (the machine I'm running Ansible against), there is only a /usr/bin/python3 directory, NOT /usr/bin/python. Hence the error. Please let me know if you need any additional information as I love Mitogen and really want to use it in my current setup. Thanks!

@s1113950
Copy link
Collaborator Author

Can you try with Ansible 2.8.8? Ansible 2.9.6 support isn't fully available yet (some playbooks work and some don't); it might still be broken but it would be good to rule that out first

@cbrum11
Copy link

cbrum11 commented May 17, 2020

@s1113950 - 2.8.8 has always worked wonderfully. That's what I've been using before posting this. I didn't realize this wasn't fully supported in 2.9.6. What's the best way to keep my finger on the pulse of what's supported when regarding 2.9.6?

@s1113950
Copy link
Collaborator Author

@cbrum11 it should be supported in 2.9.6, just wanted to check if there's a case where it doesn't because I originally added the feature using 2.8.8. However, I recently updated the tests to test 2.9.6 as well though and they passed interpreter discovery 🤔

Does interpreter discovery work without mitogen for you on Ansible 2.9.6? Mitogen calls Ansible's discover_interpreter to determine which interpreter to use, and the "possible_pythons" list that Mitogen uses (chicken-and-egg situation where Mitogen needs a python in order to run anything) matches the INTERPRETER_PYTHON_FALLBACK of Ansible, but there's definitely a bug somewhere, as you've shown in your comment.

If interpreter discovery works for you without Mitogen, I'll need your host and target OS in order to better debug your issue :)

As of right now, afaik everything should be supported in Ansible 2.9.6 as of #705 except for collections. There's a big discussion on collections (and Mitogen backwards compat/new releases) here: #715 .

@cbrum11
Copy link

cbrum11 commented May 17, 2020

Thanks for the super speedy response. I believe interpreter discovery does work on 2.9.6 without mitogen, but I'll give it a specific test to make 100% sure this week.

@kpoppel
Copy link

kpoppel commented May 29, 2020

I have just stumbled into this problem. The server is an ubuntu-netinst VM without anything on it. It has only /usr/bin/python3, which ansible -m setup ... also confirms.
Adding the mitogen in the strategy results in ansible -m ping ... to complain about server unreachable.
This is the simplest case I can present.
mitogen is the one from pip3 install: mitogen-0.2.9

Is may not be relevant unless this MR is already published there.

@s1113950
Copy link
Collaborator Author

This PR will be in the next release; 0.2.9 doesn't have it. You can pip install from master of this repo to get the fix for now :) a new release(s) is planned for after Ansible 2.10 comes out

@fourstepper
Copy link

This PR will be in the next release; 0.2.9 doesn't have it. You can pip install from master of this repo to get the fix for now :) a new release(s) is planned for after Ansible 2.10 comes out

Hi, what would the command to install from master be in this case? I am, unfortunately, still not too familiar with pip.

Thank you!

@s1113950
Copy link
Collaborator Author

@fourstepper pip install git+ssh://git@github.com/dw/mitogen.git@a18be5afefd1c8b7a860a994773737f788405a93 will give you the latest commit :)

@tom10271
Copy link

@fourstepper pip install git+ssh://git@github.com/dw/mitogen.git@a18be5afefd1c8b7a860a994773737f788405a93 will give you the latest commit :)

For strategy_plugins in ansible.cfg where should I point to the installed verion? Thank you for replying and your hard work

@tom10271
Copy link

tom10271 commented Nov 10, 2020

@fourstepper pip install git+ssh://git@github.com/dw/mitogen.git@a18be5afefd1c8b7a860a994773737f788405a93 will give you the latest commit :)

For strategy_plugins in ansible.cfg where should I point to the installed verion? Thank you for replying and your hard work

Ok it should be /usr/local/lib/python3.6/site-packages/ansible_mitogen/plugins/strategy for me.

It seems it is working now and it is faster. Thank you very much for fixing it so that Mitrogen can save my time.

Reduced from 2:01 minutes to 1:21, amazing

@lancefriday945
Copy link

I am a little late to the party, but was the issue resolved in regards to the unhandled python interpreter warning message? I am running ansible 2.8.12 via mitogen and receiving that warning:

[WARNING]: Unhandled error in Python interpreter discovery for host <removed>: unexpected output from Python interpreter discovery

Wondering what configuration updates need to be done on my end if any. Thanks in advance for any help.

cc @tom10271 @s1113950

@s1113950
Copy link
Collaborator Author

@lancefriday945
Copy link

@lancefriday945 either of these tags should have the fix in them:
https://github.com/dw/mitogen/releases/tag/v0.2.10-rc.0
https://github.com/dw/mitogen/releases/tag/v0.3.0-rc.0

I got that cleaned up. Were you working on a fix for the unreachable issue?

UNREACHABLE! => {"changed": false, "msg": "Connection timed out.", "unreachable": true}

I am still experiencing that issue. Do I need to put my config somewhere else for ssh retry logic? Right now I have this set in my ansible.cfg file:

[ssh_connection] retries = 5

I also have group_vars folders for our different environments.

@s1113950
Copy link
Collaborator Author

@lancefriday945 I'm not sure about the connection issue, I don't think it relates specifically to the python interpreter patch. Mitogen should use the same logic that Ansible does for setting ssh_connection retries

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants