Merge remote-tracking branch 'origin/dmw'

- issue #409. Closes #409.

.travis/ansible_tests.py

@@ -63,5 +63,5 @@ with ci_lib.Fold('job_setup'):
 
 
 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 @@ from __future__ import absolute_import
 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 @@ import ansible_mitogen.connection
 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
+`Brian Candler <https://github.com/candlerb>`_,
-`Guy Knights <https://github.com/knightsg>`_.
+`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/debug_run_ansible_playbook.sh

@@ -1,15 +0,0 @@
-#!/bin/bash
-# Wrap ansible-playbook, setting up some test of the test environment.
-
-# Used by delegate_to.yml to ensure "sudo -E" preserves environment.
-export I_WAS_PRESERVED=1
-export MITOGEN_MAX_INTERPRETERS=3
-
-if [ "${ANSIBLE_STRATEGY:0:7}" = "mitogen" ]
-then
-    EXTRA='{"is_mitogen": true}'
-else
-    EXTRA='{"is_mitogen": false}'
-fi
-
-exec ~/src/cpython/venv/bin/ansible-playbook -e "$EXTRA" -e ansible_python_interpreter=/Users/dmw/src/cpython/venv/bin/python2.7 "$@"

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)

tests/ansible/run_ansible_playbook.sh

@@ -1,15 +0,0 @@
-#!/bin/bash
-# Wrap ansible-playbook, setting up some test of the test environment.
-
-# Used by delegate_to.yml to ensure "sudo -E" preserves environment.
-export I_WAS_PRESERVED=1
-export MITOGEN_MAX_INTERPRETERS=3
-
-if [ "${ANSIBLE_STRATEGY:0:7}" = "mitogen" ]
-then
-    EXTRA='{"is_mitogen": true}'
-else
-    EXTRA='{"is_mitogen": false}'
-fi
-
-exec ansible-playbook -e "$EXTRA" "$@"

tests/data/iter_read_generator.py

@@ -0,0 +1,13 @@
+#!/usr/bin/env python
+# I produce text every 100ms, for testing mitogen.core.iter_read()
+
+import sys
+import time
+
+
+i = 0
+while True:
+    i += 1
+    sys.stdout.write(str(i))
+    sys.stdout.flush()
+    time.sleep(0.1)

tests/data/iter_read_generator.sh

@@ -1,10 +0,0 @@
-#!/bin/bash
-# I produce text every 100ms, for testing mitogen.core.iter_read()
-
-i=0
-
-while :; do
-    i=$(($i + 1))
-    echo "$i"
-    sleep 0.1
-done

tests/data/python_never_responds.py

@@ -1,3 +1,7 @@
-#!/bin/bash
+#!/usr/bin/python
 # I am a Python interpreter that sits idle until the connection times out.
-exec -a mitogen-tests-python-never-responds.sh sleep 86400
+
+import time
+
+while True:
+    time.sleep(86400)

tests/data/stubs/stub-kubectl.py

@@ -4,4 +4,5 @@ import sys
 import os
 
 os.environ['ORIGINAL_ARGV'] = repr(sys.argv)
+os.environ['THIS_IS_STUB_KUBECTL'] = '1'
 os.execv(sys.executable, sys.argv[sys.argv.index('--') + 1:])

tests/data/stubs/stub-lxc-attach.py

@@ -4,4 +4,5 @@ import sys
 import os
 
 os.environ['ORIGINAL_ARGV'] = repr(sys.argv)
+os.environ['THIS_IS_STUB_LXC_ATTACH'] = '1'
 os.execv(sys.executable, sys.argv[sys.argv.index('--') + 1:])

tests/data/stubs/stub-lxc-info.py

@@ -0,0 +1,4 @@
+#!/usr/bin/env python
+# Mainly for use in stubconnections/kubectl.yml
+
+print 'PID: 1'

tests/data/stubs/stub-lxc.py

@@ -0,0 +1,13 @@
+#!/usr/bin/env python
+
+import sys
+import os
+
+# setns.py fetching leader PID.
+if sys.argv[1] == 'info':
+    print 'Pid: 1'
+    sys.exit(0)
+
+os.environ['ORIGINAL_ARGV'] = repr(sys.argv)
+os.environ['THIS_IS_STUB_LXC'] = '1'
+os.execv(sys.executable, sys.argv[sys.argv.index('--') + 1:])

tests/data/write_all_consumer.py

@@ -0,0 +1,9 @@
+#!/usr/bin/env python
+# I consume 65535 bytes every 10ms, for testing mitogen.core.write_all()
+
+import os
+import time
+
+while True:
+    os.read(0, 65535)
+    time.sleep(0.01)

tests/data/write_all_consumer.sh

@@ -1,7 +0,0 @@
-#!/bin/bash
-# I consume 65535 bytes every 10ms, for testing mitogen.core.write_all()
-
-while :; do
-    read -n 65535
-    sleep 0.01
-done

tests/docker_test.py

@@ -9,7 +9,7 @@ import testlib
 
 class ConstructorTest(testlib.RouterMixin, unittest2.TestCase):
     def test_okay(self):
-        docker_path = testlib.data_path('stubs/docker.py')
+        docker_path = testlib.data_path('stubs/stub-docker.py')
         context = self.router.docker(
             container='container_name',
             docker_path=docker_path,

tests/kubectl_test.py

@@ -0,0 +1,29 @@
+
+import os
+
+import mitogen
+import mitogen.parent
+
+import unittest2
+
+import testlib
+
+
+class ConstructorTest(testlib.RouterMixin, testlib.TestCase):
+    kubectl_path = testlib.data_path('stubs/stub-kubectl.py')
+
+    def test_okay(self):
+        context = self.router.kubectl(
+            pod='pod_name',
+            kubectl_path=self.kubectl_path
+        )
+
+        argv = eval(context.call(os.getenv, 'ORIGINAL_ARGV'))
+        self.assertEquals(argv[0], self.kubectl_path)
+        self.assertEquals(argv[1], 'exec')
+        self.assertEquals(argv[2], '-it')
+        self.assertEquals(argv[3], 'pod_name')
+
+
+if __name__ == '__main__':
+    unittest2.main()

tests/lxc_test.py

@@ -13,7 +13,7 @@ def has_subseq(seq, subseq):
 
 
 class ConstructorTest(testlib.RouterMixin, testlib.TestCase):
-    lxc_attach_path = testlib.data_path('stubs/lxc-attach.py')
+    lxc_attach_path = testlib.data_path('stubs/stub-lxc-attach.py')
 
     def test_okay(self):
         context = self.router.lxc(

tests/lxd_test.py

@@ -11,7 +11,7 @@ import testlib
 
 class ConstructorTest(testlib.RouterMixin, testlib.TestCase):
     def test_okay(self):
-        lxc_path = testlib.data_path('stubs/lxc.py')
+        lxc_path = testlib.data_path('stubs/stub-lxc.py')
         context = self.router.lxd(
             container='container_name',
             lxc_path=lxc_path,

tests/parent_test.py

@@ -93,7 +93,7 @@ class ReapChildTest(testlib.RouterMixin, testlib.TestCase):
             remote_id=1234,
             old_router=self.router,
             max_message_size=self.router.max_message_size,
-            python_path=testlib.data_path('python_never_responds.sh'),
+            python_path=testlib.data_path('python_never_responds.py'),
             connect_timeout=0.5,
         )
         self.assertRaises(mitogen.core.TimeoutError,
@@ -215,7 +215,7 @@ class IterReadTest(unittest2.TestCase):
     func = staticmethod(mitogen.parent.iter_read)
 
     def make_proc(self):
-        args = [testlib.data_path('iter_read_generator.sh')]
+        args = [testlib.data_path('iter_read_generator.py')]
         proc = subprocess.Popen(args, stdout=subprocess.PIPE)
         mitogen.core.set_nonblock(proc.stdout.fileno())
         return proc
@@ -267,7 +267,7 @@ class WriteAllTest(unittest2.TestCase):
     func = staticmethod(mitogen.parent.write_all)
 
     def make_proc(self):
-        args = [testlib.data_path('write_all_consumer.sh')]
+        args = [testlib.data_path('write_all_consumer.py')]
         proc = subprocess.Popen(args, stdin=subprocess.PIPE)
         mitogen.core.set_nonblock(proc.stdin.fileno())
         return proc

tests/ssh_test.py

@@ -22,7 +22,7 @@ class StubSshMixin(testlib.RouterMixin):
             return self.router.ssh(
                 hostname='hostname',
                 username='mitogen__has_sudo',
-                ssh_path=testlib.data_path('stubs/ssh.py'),
+                ssh_path=testlib.data_path('stubs/stub-ssh.py'),
                 **kwargs
             )
         finally:
@@ -34,7 +34,7 @@ class ConstructorTest(testlib.RouterMixin, unittest2.TestCase):
         context = self.router.ssh(
             hostname='hostname',
             username='mitogen__has_sudo',
-            ssh_path=testlib.data_path('stubs/ssh.py'),
+            ssh_path=testlib.data_path('stubs/stub-ssh.py'),
         )
         #context.call(mitogen.utils.log_to_file, '/tmp/log')
         #context.call(mitogen.utils.disable_site_packages)