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

`ec2_group` module doesn't work with Mitogen 0.2.4 #536

Closed
jbscare opened this Issue Feb 11, 2019 · 15 comments

Comments

Projects
None yet
2 participants
@jbscare
Copy link

jbscare commented Feb 11, 2019

With Mitogen 0.2.4, this playbook:

- hosts: localhost
  tasks:
    - ec2_group:
        name: jbstest
        description: JBS testing
        region: "{{ aws_region }}"
        vpc_id: "{{ vpc_id }}"
        rules: []
        rules_egress: []

Produces this output:

TASK [ec2_group] ***************************************************************
fatal: [localhost]: FAILED! => {"changed": false, "msg": "boto3 required for this module"}

It works fine with Mitogen 0.2.2; it fails with 0.2.4, whether the security group already exists or not.

Here's a log with -vvv: ansible.log

@dw

This comment has been minimized.

Copy link
Owner

dw commented Feb 11, 2019

Same as #511

@dw

This comment has been minimized.

Copy link
Owner

dw commented Feb 11, 2019

Somehow explicit localhost's python_path has been broken. To confirm, do you have 'localhost' listed explicitly in your inventory? I see here by removing it (and receiving the default implicit localhost Ansible provides), the problem disappears.

This is almost certainly due to the python_path changes done during the delegate_to fiasco.

@jbscare

This comment has been minimized.

Copy link
Author

jbscare commented Feb 11, 2019

Since you asked on #511, I tried your suggestion there; neither of -e ansible_python_interpreter=python nor -e ansible_python_interpreter=/usr/bin/python change the behavior for me.

We do have localhost explicitly in our inventory, so that we can set three host variables:

  localhost: 
    ansible_connection: local 
    readiness: ready 
    stack: prod 

Does the default localhost go via SSH rather than local?

(The other two variables are things that normally get set via the ec2.py inventory plugin, but localhost doesn't come from EC2, of course.)

@dw

This comment has been minimized.

Copy link
Owner

dw commented Feb 11, 2019

The implicit localhost gets ansible_connection=local and ansible_python_interpreter=[sys.executable], if you define it in inventory, it's just another host -- so ansible.cfg's transport setting by default, and IIRC just python or /usr/bin/python for ansible_python_interpreter.

I've found the problem and it's the opposite to what was expected -- somehow it is using my virtualenv Python interpreter path over SSH, which is completely wrong

@dw

This comment has been minimized.

Copy link
Owner

dw commented Feb 12, 2019

Kicking myself. I filed a bug to verify this stuff before cutting release, and didn't even make it through all the tickets. Fatigue :/ #476

dw added a commit that referenced this issue Feb 12, 2019

issue #536: stop defining explicit localhost in inventory.
This was needed at some point in the past, but the tests don't seem to
care about it any more. We'll fix any CI breakage by changing the tests,
since verifying implicit localhost behaviour is important.

dw added a commit that referenced this issue Feb 12, 2019

issue #536: connection_delegation/ tests were erroneously broken
While fixing delegate_to, this un-hardwiring of /usr/bin/python
happened. It was always incorrect.

dw added a commit that referenced this issue Feb 12, 2019

dw added a commit that referenced this issue Feb 12, 2019

@dw dw closed this in 9040183 Feb 12, 2019

dw added a commit that referenced this issue Feb 12, 2019

Merge remote-tracking branch 'origin/dmw'
* origin/dmw:
  docs: update Changelog; closes #511, closes #536.
  docs: update Changelog release date.
  issue #536: disable transport_config tests on vanilla
  issue #536: restore correct Python interpreter selection behaviour.
  issue #536: connection_delegation/ tests were erroneously broken
  tests: define MITOGEN_INVENTORY_FILE even if -i unspecified.
  issue #536: add tests for each ansible_python_interpreter case.
  issue #536: stop defining explicit localhost in inventory.
  tests: allow running Ansible tests locally without -udmw again.
  stable: fix preamble_size on stable docs.
  issue #481: add test.
@dw

This comment has been minimized.

Copy link
Owner

dw commented Feb 12, 2019

0.2.5 will be out before end of the week, sorry for the trouble

This is now on the master branch and will make it into the next release. To be updated when a new release is made, subscribe to https://www.freelists.org/list/mitogen-announce

Thanks for reporting this!

dw added a commit that referenced this issue Feb 12, 2019

@jbscare

This comment has been minimized.

Copy link
Author

jbscare commented Feb 12, 2019

Hm, I tried this with master just now (5c5e9fb), and my playbook above still fails, same error.

@dw dw reopened this Feb 12, 2019

@dw

This comment has been minimized.

Copy link
Owner

dw commented Feb 12, 2019

Can you please check/paste output of:

ANSIBLE_STRATEGY=mitogen_linear ansible localhost -m mitogen_get_stack

This diagnosis method is new in 0.2.4, so it won't work with 0.2.2 or 0.2.3.

It will list a "method" and "python_path". For implicit localhost, it will be method "local" and the same Python interpreter as was used to run Ansible -- maybe from a virtualenv.

For explicit localhost, it would have "python_path" == "/usr/bin/python" by default, and "method" "ssh" by default.

I have reproduced your config locally and confirmed "a" fix for 0.2.3, but apparently your case still differs somehow

@dw

This comment has been minimized.

Copy link
Owner

dw commented Feb 12, 2019

In my case, using tests that have no explicit localhost in the inventory, I get:

screenshot from 2019-02-12 17-30-24

If I have boto/boto3 installed in the virtualenv, things work as expected. But if I add "-i localhost," to the command line with an EC2 module, I get the failure you're seeing

@jbscare

This comment has been minimized.

Copy link
Author

jbscare commented Feb 12, 2019

I think the -l before localhost is wrong, but here's output without that, from master:

Executing ansible localhost -b -m mitogen_get_stack
localhost | SUCCESS => {
    "changed": true, 
    "result": [
        {
            "kwargs": {
                "python_path": [
                    "/usr/bin/python"
                ]
            }, 
            "method": "local"
        }, 
        {
            "enable_lru": true, 
            "kwargs": {
                "connect_timeout": 10, 
                "password": null, 
                "python_path": [
                    "/usr/bin/python"
                ], 
                "sudo_args": [
                    "-H", 
                    "-S", 
                    "-n"
                ], 
                "sudo_path": null, 
                "username": "root"
            }, 
            "method": "sudo"
        }
    ]
}
@dw

This comment has been minimized.

Copy link
Owner

dw commented Feb 12, 2019

Putting instructions here as they're lengthy and reusable :) Per IRC,

The output of this module will contain a sys_executable key, and also a python_path key.

You can compare the output of those keys with the same module run under vanilla -- just drop the ANSIBLE_STRATEGY from the command line.

Any difference in the run impacting import will be visible from these two variables

@jbscare

This comment has been minimized.

Copy link
Author

jbscare commented Feb 12, 2019

With ANSIBLE_STRATEGY=mitogen_linear:

     "python_path": [
        "", 
        "/usr/lib/python27.zip", 
        "/usr/lib64/python2.7", 
        "/usr/lib64/python2.7/plat-linux2", 
        "/usr/lib64/python2.7/lib-tk", 
        "/usr/lib64/python2.7/lib-old", 
        "/usr/lib64/python2.7/lib-dynload", 
        "/usr/local/lib64/python2.7/site-packages", 
        "/usr/local/lib/python2.7/site-packages", 
        "/usr/lib64/python2.7/site-packages", 
        "/usr/lib/python2.7/site-packages", 
        "/usr/lib64/python2.7/dist-packages", 
        "/usr/lib64/python2.7/dist-packages/PIL", 
        "/usr/lib/python2.7/dist-packages"
    ], 
    "python_version": "2.7", 
    "sys_executable": "/usr/bin/python", 

With ANSIBLE_STRATEGY=linear:

    "python_path": [
        "/tmp/ansible_pVXC8g", 
        "/tmp/ansible_pVXC8g/ansible_modlib.zip", 
        "/tmp/ansible_pVXC8g/ansible_modlib.zip", 
        "/usr/lib/python27.zip", 
        "/usr/lib64/python2.7", 
        "/usr/lib64/python2.7/plat-linux2", 
        "/usr/lib64/python2.7/lib-tk", 
        "/usr/lib64/python2.7/lib-old", 
        "/usr/lib64/python2.7/lib-dynload", 
        "/usr/local/lib64/python2.7/site-packages", 
        "/usr/local/lib/python2.7/site-packages", 
        "/usr/lib64/python2.7/site-packages", 
        "/usr/lib/python2.7/site-packages", 
        "/usr/lib64/python2.7/dist-packages", 
        "/usr/lib64/python2.7/dist-packages/PIL", 
        "/usr/lib/python2.7/dist-packages"
    ], 
    "python_version": "2.7", 
    "sys_executable": "/usr/bin/python", 
@jbscare

This comment has been minimized.

Copy link
Author

jbscare commented Feb 12, 2019

With Mitogen 0.2.2, for comparison:

    "python_path": [
        "", 
        "/usr/lib/python27.zip", 
        "/usr/lib64/python2.7", 
        "/usr/lib64/python2.7/plat-linux2", 
        "/usr/lib64/python2.7/lib-tk", 
        "/usr/lib64/python2.7/lib-old", 
        "/usr/lib64/python2.7/lib-dynload", 
        "/usr/local/lib64/python2.7/site-packages", 
        "/usr/local/lib/python2.7/site-packages", 
        "/usr/lib64/python2.7/site-packages", 
        "/usr/lib/python2.7/site-packages", 
        "/usr/lib64/python2.7/dist-packages", 
        "/usr/lib64/python2.7/dist-packages/PIL", 
        "/usr/lib/python2.7/dist-packages"
    ], 
    "python_version": "2.7", 
    "sys_executable": "/usr/bin/python", 
@jbscare

This comment has been minimized.

Copy link
Author

jbscare commented Feb 12, 2019

As discussed on IRC, I made this change:

+$ git diff -U2
diff --git a/lib/ansible/module_utils/ec2.py b/lib/ansible/module_utils/ec2.py
index 71f64447e5..50cc58f00c 100644
--- a/lib/ansible/module_utils/ec2.py
+++ b/lib/ansible/module_utils/ec2.py
@@ -52,4 +52,5 @@ try:
 except:
     HAS_BOTO3 = False
+    raise

 try:

Here's the log from running my playbook above: ansible.log

dw added a commit that referenced this issue Feb 13, 2019

issue #536: rework how 2.3-compatible simplejson is served
Regardless of the version of simplejson loaded in the master, load up
the ModuleResponder cache with our 2.4-compatible version.

To cope with simplejson being loaded due to modules like ec2_group that
try to import it before importing 'json', also update target.py to
remove it from the whitelist if a local 'json' module import succeeds.

dw added a commit that referenced this issue Feb 13, 2019

Merge remote-tracking branch 'origin/dmw'
* origin/dmw:
  docs: update Changelog.
  issue #536: rework how 2.3-compatible simplejson is served
  .github: add some more questions to issue template
  docs: duplicate word
  docs: update Changelog.
  tests/ansible: Spec.become_method() test & mitogen_via= fix.
  setup.py: include LICENSE; closes #538.
  tests/ansible: Spec.become() test
  tests/ansible: Spec.password() test, document interactive pw limitation.
  tests/ansible: Spec.remote_user() test & mitogen_via= fix.
  tests/ansible: Spec.remote_addr() test & mitogen_via= fix.
  tests/ansible: Spec.transport() test.
  docs: lighter pink
  docs: add 'Fixes' heading
  docs: more margin tweaks for changelog
  docs: tighter <p> margins, even less shouting, red headings
  docs: tidy up footer and GitHub link
  docs: enable fixed_sidebar
  docs: sans-serif fonts, reduce shouty headings
  issue #536: add mitogen_via= tests too.
  ansible: fix a crash on 2.3 when mitogen_via= host is missing.
  tests: for 2.3 compatibility, disable gcloud.py for now
@jbscare

This comment has been minimized.

Copy link
Author

jbscare commented Feb 13, 2019

My reproducer now works fine with master. Thanks!

@jbscare jbscare closed this Feb 13, 2019

dw added a commit that referenced this issue Feb 14, 2019

Merge remote-tracking branch 'origin/dmw' into stable
* origin/dmw:
  issue #537: disable just the trivial LinuxPolicyTest on Travis.
  docs: update Changelog; closes #537.
  ansible: refactor affinity class and add abstract tests.
  Bump version for release.
  docs: update Changelog.
  core: serialize calls to _service_stub_main().
  docs: update Changelog; closes #532.
  issue #532: PushFileService race.
  docs: more concise Changelog.
  issue #541: changelog typos.
  ansible: quiesce boto logger; closes #541.
  docs: update Changelog.
  tests/ansible: Spec.port() test & mitogen_via= fix.
  Update copyright year everywhere.
  tests/ansible: Spec.become_pass() test.
  docs: remove top "Table of Contents" link
  docs: remove a little more top margin wastage
  tests/ansible: Spec.become_user() test.
  docs: update Changelog; closes #539.
  issue #539: disable logger propagation.
  ansible: capture stderr stream of async tasks. Closes #540.
  docs: update Changelog.
  issue #536: rework how 2.3-compatible simplejson is served
  .github: add some more questions to issue template
  docs: duplicate word
  docs: update Changelog.
  tests/ansible: Spec.become_method() test & mitogen_via= fix.
  setup.py: include LICENSE; closes #538.
  tests/ansible: Spec.become() test
  tests/ansible: Spec.password() test, document interactive pw limitation.
  tests/ansible: Spec.remote_user() test & mitogen_via= fix.
  tests/ansible: Spec.remote_addr() test & mitogen_via= fix.
  tests/ansible: Spec.transport() test.
  docs: lighter pink
  docs: add 'Fixes' heading
  docs: more margin tweaks for changelog
  docs: tighter <p> margins, even less shouting, red headings
  docs: tidy up footer and GitHub link
  docs: enable fixed_sidebar
  docs: sans-serif fonts, reduce shouty headings
  issue #536: add mitogen_via= tests too.
  ansible: fix a crash on 2.3 when mitogen_via= host is missing.
  tests: for 2.3 compatibility, disable gcloud.py for now
  docs: update Changelog; closes #511, closes #536.
  docs: update Changelog release date.
  issue #536: disable transport_config tests on vanilla
  issue #536: restore correct Python interpreter selection behaviour.
  issue #536: connection_delegation/ tests were erroneously broken
  tests: define MITOGEN_INVENTORY_FILE even if -i unspecified.
  issue #536: add tests for each ansible_python_interpreter case.
  issue #536: stop defining explicit localhost in inventory.
  tests: allow running Ansible tests locally without -udmw again.
  stable: fix preamble_size on stable docs.
  issue #481: add test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.