Merge remote-tracking branch 'origin/dmw'

- issue #409. Closes #409.

.travis/ansible_tests.py

@@ -63,5 +63,5 @@
 
 
 with ci_lib.Fold('ansible'):
-    run('/usr/bin/time ./run_ansible_playbook.sh all.yml -i "%s" %s',
+    run('/usr/bin/time ./run_ansible_playbook.py all.yml -i "%s" %s',
         HOSTS_DIR, ' '.join(sys.argv[1:]))

ansible_mitogen/connection.py

@@ -142,6 +142,7 @@ def _connect_kubectl(spec):
             'pod': spec['remote_addr'],
             'python_path': spec['python_path'],
             'connect_timeout': spec['ansible_ssh_timeout'] or spec['timeout'],
+            'kubectl_path': spec['mitogen_kubectl_path'],
             'kubectl_args': spec['extra_args'],
         }
     }
@@ -171,6 +172,7 @@ def _connect_lxc(spec):
         'kwargs': {
             'container': spec['remote_addr'],
             'python_path': spec['python_path'],
+            'lxc_attach_path': spec['mitogen_lxc_attach_path'],
             'connect_timeout': spec['ansible_ssh_timeout'] or spec['timeout'],
         }
     }
@@ -185,6 +187,7 @@ def _connect_lxd(spec):
         'kwargs': {
             'container': spec['remote_addr'],
             'python_path': spec['python_path'],
+            'lxc_path': spec['mitogen_lxc_path'],
             'connect_timeout': spec['ansible_ssh_timeout'] or spec['timeout'],
         }
     }
@@ -209,7 +212,7 @@ def _connect_setns(spec):
             'python_path': spec['python_path'],
             'kind': spec['mitogen_kind'],
             'docker_path': spec['mitogen_docker_path'],
-            'kubectl_path': spec['mitogen_kubectl_path'],
+            'lxc_path': spec['mitogen_lxc_path'],
             'lxc_info_path': spec['mitogen_lxc_info_path'],
             'machinectl_path': spec['mitogen_machinectl_path'],
         }
@@ -392,6 +395,10 @@ def config_from_play_context(transport, inventory_name, connection):
             connection.get_task_var('mitogen_docker_path'),
         'mitogen_kubectl_path':
             connection.get_task_var('mitogen_kubectl_path'),
+        'mitogen_lxc_path':
+            connection.get_task_var('mitogen_lxc_path'),
+        'mitogen_lxc_attach_path':
+            connection.get_task_var('mitogen_lxc_attach_path'),
         'mitogen_lxc_info_path':
             connection.get_task_var('mitogen_lxc_info_path'),
         'mitogen_machinectl_path':
@@ -427,6 +434,8 @@ def config_from_hostvars(transport, inventory_name, connection,
         'mitogen_kind': hostvars.get('mitogen_kind'),
         'mitogen_docker_path': hostvars.get('mitogen_docker_path'),
         'mitogen_kubectl_path': hostvars.get('mitogen_kubectl_path'),
+        'mitogen_lxc_path': hostvars.get('mitogen_lxc_path'),
+        'mitogen_lxc_attach_path': hostvars.get('mitogen_lxc_attach_path'),
         'mitogen_lxc_info_path': hostvars.get('mitogen_lxc_info_path'),
         'mitogen_machinectl_path': hostvars.get('mitogen_machinctl_path'),
     })

ansible_mitogen/plugins/connection/mitogen_kubectl.py

@@ -31,7 +31,12 @@
 import os.path
 import sys
 
-import ansible.plugins.connection.kubectl
+try:
+    from ansible.plugins.connection import kubectl
+except ImportError:
+    kubectl = None
+
+from ansible.errors import AnsibleConnectionFailure
 from ansible.module_utils.six import iteritems
 
 try:
@@ -47,9 +52,19 @@
 class Connection(ansible_mitogen.connection.Connection):
     transport = 'kubectl'
 
+    not_supported_msg = (
+        'The "mitogen_kubectl" plug-in requires a version of Ansible '
+        'that ships with the "kubectl" connection plug-in.'
+    )
+
+    def __init__(self, *args, **kwargs):
+        if kubectl is None:
+            raise AnsibleConnectionFailure(self.not_supported_msg)
+        super(Connection, self).__init__(*args, **kwargs)
+
     def get_extra_args(self):
         parameters = []
-        for key, option in iteritems(ansible.plugins.connection.kubectl.CONNECTION_OPTIONS):
+        for key, option in iteritems(kubectl.CONNECTION_OPTIONS):
             if self.get_task_var('ansible_' + key) is not None:
                 parameters += [ option, self.get_task_var('ansible_' + key) ]
 

docs/ansible.rst

@@ -768,10 +768,10 @@ Connect to classic LXC containers, like `lxc
 connection delegation is supported, and ``lxc-attach`` is always used rather
 than the LXC Python bindings, as is usual with ``lxc``.
 
-The ``lxc-attach`` command must be available on the host machine.
-
 * ``ansible_python_interpreter``
 * ``ansible_host``: Name of LXC container (default: inventory hostname).
+* ``mitogen_lxc_attach_path``: path to ``lxc-attach`` command if not available
+    on the system path.
 
 
 .. _method-lxd:
@@ -786,6 +786,8 @@ the host machine.
 
 * ``ansible_python_interpreter``
 * ``ansible_host``: Name of LXC container (default: inventory hostname).
+* ``mitogen_lxc_path``: path to ``lxc`` command if not available on the system
+  path.
 
 
 .. _machinectl:

docs/changelog.rst

@@ -50,6 +50,14 @@ Fixes
   now print a useful hint when Python fails to start, as no useful error is
   normally logged to the console by these tools.
 
+* `#409 <https://github.com/dw/mitogen/issues/409>`_: the setns method was
+  silently broken due to missing tests. Basic coverage was added to prevent a
+  recurrence.
+
+* `#409 <https://github.com/dw/mitogen/issues/409>`_: the LXC and LXD methods
+  support ``mitogen_lxc_path`` and ``mitogen_lxc_attach`` variables to control
+  the location of third pary utilities.
+
 
 Core Library
 ~~~~~~~~~~~~
@@ -87,8 +95,9 @@ Thanks!
 
 Mitogen would not be possible without the support of users. A huge thanks for
 bug reports, features and fixes in this release contributed by
-`Brian Candler <https://github.com/candlerb>`_, and
-`Guy Knights <https://github.com/knightsg>`_.
+`Brian Candler <https://github.com/candlerb>`_,
+`Guy Knights <https://github.com/knightsg>`_, and
+`Jonathan Rosser <https://github.com/jrosser>`_.
 
 
 v0.2.3 (2018-10-23)

tests/ansible/README.md

@@ -13,7 +13,7 @@ demonstrator for what does and doesn't work.
 See `../image_prep/README.md`.
 
 
-## `run_ansible_playbook.sh`
+## `run_ansible_playbook.py`
 
 This is necessary to set some environment variables used by future tests, as
 there appears to be no better way to inject them into the top-level process
@@ -22,7 +22,7 @@ environment before the Mitogen connection process forks.
 
 ## Running Everything
 
-`ANSIBLE_STRATEGY=mitogen_linear ./run_ansible_playbook.sh all.yml`
+`ANSIBLE_STRATEGY=mitogen_linear ./run_ansible_playbook.py all.yml`
 
 
 ## `hosts/` and `common-hosts`

tests/ansible/integration/all.yml

@@ -7,6 +7,7 @@
 - import_playbook: async/all.yml
 - import_playbook: become/all.yml
 - import_playbook: connection/all.yml
+- import_playbook: stub_connections/all.yml
 - import_playbook: connection_loader/all.yml
 - import_playbook: context_service/all.yml
 - import_playbook: delegation/all.yml

tests/ansible/integration/stub_connections/README.md

@@ -0,0 +1,9 @@
+
+# `stub_connections/`
+
+The playbooks in this directory use stub implementations of various third party
+tools (kubectl etc.) to verify arguments passed by Ansible to Mitogen and
+subsequently onward to the tool result in something that looks sane.
+
+These are bare minimum tests just to ensure sporadically tested connection
+methods haven't broken in embarrasingly obvious ways.

tests/ansible/integration/stub_connections/_end_play_if_not_sudo_linux.yml

@@ -0,0 +1,17 @@
+# End the play if we're not on Linux and a raw 'sudo' command isn't available.
+# Expects connection:local
+
+- shell: uname -s
+  register: out
+
+- meta: end_play
+  when: out.stdout != 'Linux'
+
+- command: sudo -n whoami
+  args:
+    warn: false
+  ignore_errors: true
+  register: sudo_available
+
+- meta: end_play
+  when: sudo_available.rc != 0

tests/ansible/integration/stub_connections/all.yml

@@ -0,0 +1,5 @@
+- import_playbook: kubectl.yml
+- import_playbook: lxc.yml
+- import_playbook: lxd.yml
+- import_playbook: setns_lxc.yml
+- import_playbook: setns_lxd.yml

tests/ansible/integration/stub_connections/kubectl.yml

@@ -0,0 +1,21 @@
+
+- name: integration/stub_connections/kubectl.yml
+  hosts: test-targets
+  gather_facts: false
+  any_errors_fatal: true
+  tasks:
+  - meta: end_play
+    when: not is_mitogen
+
+  - meta: end_play
+    when: ansible_version.full < '2.5'
+
+  - custom_python_detect_environment:
+    vars:
+      ansible_connection: kubectl
+      mitogen_kubectl_path: stub-kubectl.py
+    register: out
+
+  - assert:
+      that:
+      - out.env.THIS_IS_STUB_KUBECTL == '1'

tests/ansible/integration/stub_connections/lxc.yml

@@ -0,0 +1,18 @@
+
+- name: integration/stub_connections/lxc.yml
+  hosts: test-targets
+  gather_facts: false
+  any_errors_fatal: true
+  tasks:
+  - meta: end_play
+    when: not is_mitogen
+
+  - custom_python_detect_environment:
+    vars:
+      ansible_connection: lxc
+      mitogen_lxc_attach_path: stub-lxc-attach.py
+    register: out
+
+  - assert:
+      that:
+      - out.env.THIS_IS_STUB_LXC_ATTACH == '1'

tests/ansible/integration/stub_connections/lxd.yml

@@ -0,0 +1,18 @@
+
+- name: integration/stub_connections/lxd.yml
+  hosts: test-targets
+  gather_facts: false
+  any_errors_fatal: true
+  tasks:
+  - meta: end_play
+    when: not is_mitogen
+
+  - custom_python_detect_environment:
+    vars:
+      ansible_connection: lxd
+      mitogen_lxc_path: stub-lxc.py
+    register: out
+
+  - assert:
+      that:
+      - out.env.THIS_IS_STUB_LXC == '1'

tests/ansible/integration/stub_connections/setns_lxc.yml

@@ -0,0 +1,31 @@
+# issue #409.
+# setns is hard -- it wants to do superuser syscalls, so we must run it in a
+# child Ansible via sudo. But that only works if sudo works.
+
+- name: integration/stub_connections/setns_lxc.yml
+  hosts: test-targets
+  gather_facts: false
+  any_errors_fatal: false
+  connection: local
+  tasks:
+    - meta: end_play
+      when: not is_mitogen
+
+    - include_tasks: _end_play_if_not_sudo_linux.yml
+
+    - command: |
+        sudo -nE ansible
+        -i localhost,
+        -c setns
+        -e mitogen_kind=lxc
+        -e mitogen_lxc_info_path=stub-lxc-info.py
+        -m shell
+        -a "echo hi"
+        localhost
+      args:
+        chdir: ../..
+        warn: false
+      register: result
+
+    - assert:
+        that: result.rc == 0

tests/ansible/integration/stub_connections/setns_lxd.yml

@@ -0,0 +1,32 @@
+# issue #409.
+# setns is hard -- it wants to do superuser syscalls, so we must run it in a
+# child Ansible via sudo. But that only works if sudo works.
+
+- name: integration/stub_connections/setns_lxd.yml
+  hosts: test-targets
+  gather_facts: false
+  any_errors_fatal: false
+  connection: local
+  tasks:
+    - meta: end_play
+      when: not is_mitogen
+
+    - include_tasks: _end_play_if_not_sudo_linux.yml
+
+    - command: |
+        sudo -nE ansible
+        -i localhost,
+        -c setns
+        -e mitogen_kind=lxd
+        -e mitogen_lxc_path=stub-lxc.py
+        -m shell
+        -a "echo hi"
+        localhost
+      args:
+        chdir: ../..
+        warn: false
+      register: result
+
+    - assert:
+        that: result.rc == 0
+

tests/ansible/mitogen_ansible_playbook.sh

@@ -1,3 +1,3 @@
 #!/bin/bash
 export ANSIBLE_STRATEGY=mitogen_linear
-exec ./run_ansible_playbook.sh "$@"
+exec ./run_ansible_playbook.py "$@"

tests/ansible/run_ansible_playbook.py

@@ -0,0 +1,37 @@
+#!/usr/bin/env python
+# Wrap ansible-playbook, setting up some test of the test environment.
+
+import json
+import os
+import sys
+
+
+GIT_BASEDIR = os.path.dirname(
+    os.path.abspath(
+        os.path.join(__file__, '..', '..')
+    )
+)
+
+
+# Used by delegate_to.yml to ensure "sudo -E" preserves environment.
+os.environ['I_WAS_PRESERVED'] = '1'
+
+# Used by LRU tests.
+os.environ['MITOGEN_MAX_INTERPRETERS'] = '3'
+
+# Add test stubs to path.
+os.environ['PATH'] = '%s%s%s' % (
+    os.path.join(GIT_BASEDIR, 'tests', 'data', 'stubs'),
+    os.pathsep,
+    os.environ['PATH'],
+)
+
+extra = {
+    'is_mitogen': os.environ.get('ANSIBLE_STRATEGY', '').startswith('mitogen'),
+    'git_basedir': GIT_BASEDIR,
+}
+
+args = ['ansible-playbook']
+args += ['-e', json.dumps(extra)]
+args += sys.argv[1:]
+os.execvp(args[0], args)