From 1555f232153b5904b67e924275e5d39ff506adf2 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 21 Jan 2019 15:25:46 +0000 Subject: [PATCH 01/22] tests: add some more helper function tests. --- tests/call_error_test.py | 35 +++++++++++++++++++++++ tests/error_test.py | 40 ++++++++++++++++++++++++++ tests/policy_function_test.py | 40 ++++++++++++++++++++++++++ tests/types_test.py | 54 +++++++++++++++++++++++++++++++++++ 4 files changed, 169 insertions(+) create mode 100644 tests/error_test.py create mode 100644 tests/policy_function_test.py diff --git a/tests/call_error_test.py b/tests/call_error_test.py index 1480a743..72d3c46e 100644 --- a/tests/call_error_test.py +++ b/tests/call_error_test.py @@ -16,15 +16,18 @@ class ConstructorTest(testlib.TestCase): def test_string_noargs(self): e = self.klass('%s%s') self.assertEquals(e.args[0], '%s%s') + self.assertTrue(isinstance(e.args[0], mitogen.core.UnicodeType)) def test_string_args(self): e = self.klass('%s%s', 1, 1) self.assertEquals(e.args[0], '11') + self.assertTrue(isinstance(e.args[0], mitogen.core.UnicodeType)) def test_from_exc(self): ve = plain_old_module.MyError('eek') e = self.klass(ve) self.assertEquals(e.args[0], 'plain_old_module.MyError: eek') + self.assertTrue(isinstance(e.args[0], mitogen.core.UnicodeType)) def test_form_base_exc(self): ve = SystemExit('eek') @@ -32,6 +35,7 @@ class ConstructorTest(testlib.TestCase): self.assertEquals(e.args[0], # varies across 2/3. '%s.%s: eek' % (type(ve).__module__, type(ve).__name__)) + self.assertTrue(isinstance(e.args[0], mitogen.core.UnicodeType)) def test_from_exc_tb(self): try: @@ -41,8 +45,39 @@ class ConstructorTest(testlib.TestCase): e = self.klass(ve) self.assertTrue(e.args[0].startswith('plain_old_module.MyError: eek')) + self.assertTrue(isinstance(e.args[0], mitogen.core.UnicodeType)) self.assertTrue('test_from_exc_tb' in e.args[0]) + def test_bytestring_conversion(self): + e = self.klass(mitogen.core.b('bytes')) + self.assertEquals(u'bytes', e.args[0]) + self.assertTrue(isinstance(e.args[0], mitogen.core.UnicodeType)) + + def test_reduce(self): + e = self.klass('eek') + func, (arg,) = e.__reduce__() + self.assertTrue(func is mitogen.core._unpickle_call_error) + self.assertEquals(arg, e.args[0]) + + +class UnpickleCallErrorTest(testlib.TestCase): + func = staticmethod(mitogen.core._unpickle_call_error) + + def test_not_unicode(self): + self.assertRaises(TypeError, + lambda: self.func(mitogen.core.b('bad'))) + + def test_oversized(self): + self.assertRaises(TypeError, + lambda: self.func(mitogen.core.b('b'*10001))) + + def test_reify(self): + e = self.func(u'some error') + self.assertEquals(mitogen.core.CallError, type(e)) + self.assertEquals(1, len(e.args)) + self.assertEquals(mitogen.core.UnicodeType, type(e.args[0])) + self.assertEquals(u'some error', e.args[0]) + class PickleTest(testlib.TestCase): klass = mitogen.core.CallError diff --git a/tests/error_test.py b/tests/error_test.py new file mode 100644 index 00000000..7f890875 --- /dev/null +++ b/tests/error_test.py @@ -0,0 +1,40 @@ + +import logging +import subprocess +import time +import zlib + +import unittest2 + +import testlib +import mitogen.master +import mitogen.parent +import mitogen.utils + + +class ConstructorTest(testlib.TestCase): + klass = mitogen.core.Error + + def test_literal_no_format(self): + e = self.klass('error') + self.assertEquals(e.args[0], 'error') + self.assertTrue(isinstance(e.args[0], mitogen.core.UnicodeType)) + + def test_literal_format_chars_present(self): + e = self.klass('error%s') + self.assertEquals(e.args[0], 'error%s') + self.assertTrue(isinstance(e.args[0], mitogen.core.UnicodeType)) + + def test_format(self): + e = self.klass('error%s', 123) + self.assertEquals(e.args[0], 'error123') + self.assertTrue(isinstance(e.args[0], mitogen.core.UnicodeType)) + + def test_bytes_to_unicode(self): + e = self.klass(mitogen.core.b('error')) + self.assertEquals(e.args[0], 'error') + self.assertTrue(isinstance(e.args[0], mitogen.core.UnicodeType)) + + +if __name__ == '__main__': + unittest2.main() diff --git a/tests/policy_function_test.py b/tests/policy_function_test.py new file mode 100644 index 00000000..56e33b89 --- /dev/null +++ b/tests/policy_function_test.py @@ -0,0 +1,40 @@ + +import mock +import unittest2 + +import mitogen.core +import mitogen.parent + +import testlib + + +class HasParentAuthorityTest(testlib.TestCase): + func = staticmethod(mitogen.core.has_parent_authority) + + def call(self, auth_id): + msg = mitogen.core.Message(auth_id=auth_id) + return self.func(msg) + + @mock.patch('mitogen.context_id', 5555) + @mock.patch('mitogen.parent_ids', [111, 222]) + def test_okay(self): + self.assertFalse(self.call(0)) + self.assertTrue(self.call(5555)) + self.assertTrue(self.call(111)) + + +class IsImmediateChildTest(testlib.TestCase): + func = staticmethod(mitogen.core.has_parent_authority) + + def call(self, auth_id, remote_id): + msg = mitogen.core.Message(auth_id=auth_id) + stream = mock.Mock(remote_id=remote_id) + return self.func(msg, stream) + + def test_okay(self): + self.assertFalse(0, 1) + self.assertTrue(1, 1) + + +if __name__ == '__main__': + unittest2.main() diff --git a/tests/types_test.py b/tests/types_test.py index f929c098..2f0b1306 100644 --- a/tests/types_test.py +++ b/tests/types_test.py @@ -76,5 +76,59 @@ class SecretTest(testlib.TestCase): mitogen.core.b(secret2)) +class KwargsTest(testlib.TestCase): + klass = mitogen.core.Kwargs + + def test_empty(self): + kw = self.klass({}) + self.assertEquals({}, kw) + self.assertEquals('Kwargs({})', repr(kw)) + klass, (dct,) = kw.__reduce__() + self.assertTrue(klass is self.klass) + self.assertTrue(type(dct) is dict) + self.assertEquals({}, dct) + + @unittest2.skipIf(condition=lambda: not mitogen.core.PY3, + reason='py3 only') + def test_unicode_conversion(self): + kw = self.klass({mitogen.core.b('key'): 123}) + self.assertEquals({mitogen.core.b('key'): 123}, kw) + self.assertEquals("Kwargs({'key': 123})", repr(kw)) + klass, (dct,) = kw.__reduce__() + self.assertTrue(klass is self.klass) + self.assertTrue(type(dct) is dict) + self.assertEquals({u'key': 123}, dct) + key, = dct + self.assertTrue(type(key) is mitogen.core.UnicodeType) + + +class AdornedUnicode(mitogen.core.UnicodeType): + pass + + +class ToTextTest(testlib.TestCase): + func = staticmethod(mitogen.core.to_text) + + def test_bytes(self): + s = self.func(mitogen.core.b('bytes')) + self.assertEquals(mitogen.core.UnicodeType, type(s)) + self.assertEquals(s, u'bytes') + + def test_unicode(self): + s = self.func(u'text') + self.assertEquals(mitogen.core.UnicodeType, type(s)) + self.assertEquals(s, u'text') + + def test_adorned_unicode(self): + s = self.func(AdornedUnicode(u'text')) + self.assertEquals(mitogen.core.UnicodeType, type(s)) + self.assertEquals(s, u'text') + + def test_integer(self): + s = self.func(123) + self.assertEquals(mitogen.core.UnicodeType, type(s)) + self.assertEquals(s, u'123') + + if __name__ == '__main__': unittest2.main() From 0ba8cc7b61347133f5fa982665740aba78e55abc Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 21 Jan 2019 19:36:57 +0000 Subject: [PATCH 02/22] tests: clean up / deduplicate Ansible inventory. --- tests/ansible/hosts/connection-delegation | 12 ----- ...mmon-hosts => connection_delegation.hosts} | 44 +++++++++---------- tests/ansible/hosts/default.hosts | 8 ++++ tests/ansible/hosts/{k3 => k3.hosts} | 3 ++ tests/ansible/hosts/localhost | 8 ---- tests/ansible/hosts/localhost.hosts | 10 +++++ tests/ansible/hosts/nessy | 10 ----- tests/ansible/hosts/z | 25 ----------- 8 files changed, 41 insertions(+), 79 deletions(-) delete mode 100644 tests/ansible/hosts/connection-delegation rename tests/ansible/hosts/{common-hosts => connection_delegation.hosts} (71%) create mode 100644 tests/ansible/hosts/default.hosts rename tests/ansible/hosts/{k3 => k3.hosts} (79%) delete mode 100644 tests/ansible/hosts/localhost create mode 100644 tests/ansible/hosts/localhost.hosts delete mode 100644 tests/ansible/hosts/nessy delete mode 100644 tests/ansible/hosts/z diff --git a/tests/ansible/hosts/connection-delegation b/tests/ansible/hosts/connection-delegation deleted file mode 100644 index 2fb87455..00000000 --- a/tests/ansible/hosts/connection-delegation +++ /dev/null @@ -1,12 +0,0 @@ -[connection-delegation-test] -cd-bastion -cd-rack11 mitogen_via=ssh-user@cd-bastion -cd-rack11a mitogen_via=root@cd-rack11 -cd-rack11a-docker mitogen_via=docker-admin@cd-rack11a ansible_connection=docker - -[connection-delegation-cycle] -# Create cycle with Docker container. -cdc-bastion mitogen_via=cdc-rack11a-docker -cdc-rack11 mitogen_via=ssh-user@cdc-bastion -cdc-rack11a mitogen_via=root@cdc-rack11 -cdc-rack11a-docker mitogen_via=docker-admin@cdc-rack11a ansible_connection=docker diff --git a/tests/ansible/hosts/common-hosts b/tests/ansible/hosts/connection_delegation.hosts similarity index 71% rename from tests/ansible/hosts/common-hosts rename to tests/ansible/hosts/connection_delegation.hosts index cf84d2d1..a22bd5df 100644 --- a/tests/ansible/hosts/common-hosts +++ b/tests/ansible/hosts/connection_delegation.hosts @@ -1,38 +1,18 @@ # vim: syntax=dosini +# Connection delegation scenarios. It's impossible to connect to them, but their would-be +# config can be inspected using "mitogen_get_stack" action. -# This must be defined explicitly, otherwise _create_implicit_localhost() -# generates its own copy, which includes an ansible_python_interpreter that -# varies according to host machine. -localhost -[connection-delegation-test] -cd-bastion -cd-rack11 mitogen_via=ssh-user@cd-bastion -cd-rack11a mitogen_via=root@cd-rack11 -cd-rack11a-docker mitogen_via=docker-admin@cd-rack11a ansible_connection=docker - -[connection-delegation-cycle] -# Create cycle with Docker container. -cdc-bastion mitogen_via=cdc-rack11a-docker -cdc-rack11 mitogen_via=ssh-user@cdc-bastion -cdc-rack11a mitogen_via=root@cdc-rack11 -cdc-rack11a-docker mitogen_via=docker-admin@cdc-rack11a ansible_connection=docker - -[conn-delegation] -cd-user1 ansible_user=mitogen__user1 ansible_connection=mitogen_sudo mitogen_via=target - - -# Connection delegation scenarios. It's impossible to connection to them, but -# you can inspect the would-be config via "mitogen_get_stack" action. -[cd-no-connect] # Normal inventory host, no aliasing. cd-normal ansible_connection=mitogen_doas ansible_user=normal-user + # Inventory host that is really a different host. cd-alias ansible_connection=ssh ansible_user=alias-user ansible_host=alias-host # Via one normal host. cd-normal-normal mitogen_via=cd-normal + # Via one aliased host. cd-normal-alias mitogen_via=cd-alias @@ -41,3 +21,19 @@ cd-newuser-normal-normal mitogen_via=cd-normal ansible_user=newuser-normal-norma # doas:newuser via host. cd-newuser-doas-normal mitogen_via=cd-normal ansible_connection=mitogen_doas ansible_user=newuser-doas-normal-user + + +[connection-delegation-test] +cd-bastion +cd-rack11 mitogen_via=ssh-user@cd-bastion +cd-rack11a mitogen_via=root@cd-rack11 +cd-rack11a-docker mitogen_via=docker-admin@cd-rack11a ansible_connection=docker + + +[connection-delegation-cycle] +# Create cycle with Docker container. +cdc-bastion mitogen_via=cdc-rack11a-docker +cdc-rack11 mitogen_via=ssh-user@cdc-bastion +cdc-rack11a mitogen_via=root@cdc-rack11 +cdc-rack11a-docker mitogen_via=docker-admin@cdc-rack11a ansible_connection=docker + diff --git a/tests/ansible/hosts/default.hosts b/tests/ansible/hosts/default.hosts new file mode 100644 index 00000000..02f3c614 --- /dev/null +++ b/tests/ansible/hosts/default.hosts @@ -0,0 +1,8 @@ +# vim: syntax=dosini + +# When running the tests outside CI, make a single 'target' host which is the +# local machine. +target ansible_host=localhost + +[test-targets] +target diff --git a/tests/ansible/hosts/k3 b/tests/ansible/hosts/k3.hosts similarity index 79% rename from tests/ansible/hosts/k3 rename to tests/ansible/hosts/k3.hosts index 1a7190d8..34e1ff95 100644 --- a/tests/ansible/hosts/k3 +++ b/tests/ansible/hosts/k3.hosts @@ -1,3 +1,6 @@ +# vim: syntax=dosini + +# Used for manual testing. k3 [k3-x10] diff --git a/tests/ansible/hosts/localhost b/tests/ansible/hosts/localhost deleted file mode 100644 index f4dab2ab..00000000 --- a/tests/ansible/hosts/localhost +++ /dev/null @@ -1,8 +0,0 @@ -localhost -target ansible_host=localhost - -[test-targets] -target - -[localhost-x10] -localhost-[01:10] diff --git a/tests/ansible/hosts/localhost.hosts b/tests/ansible/hosts/localhost.hosts new file mode 100644 index 00000000..ca8716a6 --- /dev/null +++ b/tests/ansible/hosts/localhost.hosts @@ -0,0 +1,10 @@ +# vim: syntax=dosini + +# This must be defined explicitly, otherwise _create_implicit_localhost() +# generates its own copy, which includes an ansible_python_interpreter that +# varies according to host machine. +localhost + +# This is only used for manual testing. +[localhost-x10] +localhost-[01:10] diff --git a/tests/ansible/hosts/nessy b/tests/ansible/hosts/nessy deleted file mode 100644 index 5cdef123..00000000 --- a/tests/ansible/hosts/nessy +++ /dev/null @@ -1,10 +0,0 @@ -nessy - -[nessy-x10] -nessy-[00:10] - -[nessy-x20] -nessy-[00:20] - -[nessy-x50] -nessy-[00:50] diff --git a/tests/ansible/hosts/z b/tests/ansible/hosts/z deleted file mode 100644 index 61d27940..00000000 --- a/tests/ansible/hosts/z +++ /dev/null @@ -1,25 +0,0 @@ -z - -[z-x10] -z-[01:10] - -[z-x20] -z-[01:20] - -[z-x50] -z-[01:50] - -[z-x100] -z-[001:100] - -[z-x200] -z-[001:200] - -[z-x300] -z-[001:300] - -[z-x400] -z-[001:400] - -[z-x500] -z-[001:500] From f5d9af80ef83824658ab812ba41c8bc543644ed8 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 21 Jan 2019 19:38:23 +0000 Subject: [PATCH 03/22] tests: some more utility function tests + flake8. --- tests/call_error_test.py | 1 - tests/error_test.py | 9 +------- tests/router_test.py | 2 -- tests/serialization_test.py | 7 ------ tests/signals_test.py | 45 +++++++++++++++++++++++++++++++++++++ tests/types_test.py | 4 ++-- 6 files changed, 48 insertions(+), 20 deletions(-) create mode 100644 tests/signals_test.py diff --git a/tests/call_error_test.py b/tests/call_error_test.py index 72d3c46e..baebd1eb 100644 --- a/tests/call_error_test.py +++ b/tests/call_error_test.py @@ -1,4 +1,3 @@ -import os import pickle import sys diff --git a/tests/error_test.py b/tests/error_test.py index 7f890875..2eefd567 100644 --- a/tests/error_test.py +++ b/tests/error_test.py @@ -1,15 +1,8 @@ -import logging -import subprocess -import time -import zlib - import unittest2 import testlib -import mitogen.master -import mitogen.parent -import mitogen.utils +import mitogen.core class ConstructorTest(testlib.TestCase): diff --git a/tests/router_test.py b/tests/router_test.py index 351d7ace..51035d54 100644 --- a/tests/router_test.py +++ b/tests/router_test.py @@ -1,5 +1,3 @@ -import logging -import subprocess import time import zlib diff --git a/tests/serialization_test.py b/tests/serialization_test.py index d8c54c59..ffbdf694 100644 --- a/tests/serialization_test.py +++ b/tests/serialization_test.py @@ -1,11 +1,4 @@ -try: - from io import StringIO - from io import BytesIO -except ImportError: - from StringIO import StringIO as StringIO - from StringIO import StringIO as BytesIO - import pickle import unittest2 diff --git a/tests/signals_test.py b/tests/signals_test.py new file mode 100644 index 00000000..5957d3fa --- /dev/null +++ b/tests/signals_test.py @@ -0,0 +1,45 @@ + +import unittest2 + +import testlib +import mitogen.core + + +class Thing(): + pass + + +class ListenFireTest(testlib.TestCase): + def test_no_args(self): + thing = Thing() + latch = mitogen.core.Latch() + mitogen.core.listen(thing, 'event', + lambda: latch.put('event fired')) + + mitogen.core.fire(thing, 'event') + self.assertEquals('event fired', latch.get()) + self.assertTrue(latch.empty()) + + def test_with_args(self): + thing = Thing() + latch = mitogen.core.Latch() + mitogen.core.listen(thing, 'event', latch.put) + mitogen.core.fire(thing, 'event', 'event fired') + self.assertEquals('event fired', latch.get()) + self.assertTrue(latch.empty()) + + def test_two_listeners(self): + thing = Thing() + latch = mitogen.core.Latch() + latch2 = mitogen.core.Latch() + mitogen.core.listen(thing, 'event', latch.put) + mitogen.core.listen(thing, 'event', latch2.put) + mitogen.core.fire(thing, 'event', 'event fired') + self.assertEquals('event fired', latch.get()) + self.assertEquals('event fired', latch2.get()) + self.assertTrue(latch.empty()) + self.assertTrue(latch2.empty()) + + +if __name__ == '__main__': + unittest2.main() diff --git a/tests/types_test.py b/tests/types_test.py index 2f0b1306..bd243949 100644 --- a/tests/types_test.py +++ b/tests/types_test.py @@ -26,14 +26,14 @@ class BlobTest(testlib.TestCase): def test_decays_on_constructor(self): blob = self.make() - self.assertEquals(b('x')*128, mitogen.core.BytesType(blob)) + self.assertEquals(b('x') * 128, mitogen.core.BytesType(blob)) def test_decays_on_write(self): blob = self.make() io = BytesIO() io.write(blob) self.assertEquals(128, io.tell()) - self.assertEquals(b('x')*128, io.getvalue()) + self.assertEquals(b('x') * 128, io.getvalue()) def test_message_roundtrip(self): blob = self.make() From e99b8a8de71dc72627813c3c12dcde1c8f0facba Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 21 Jan 2019 19:38:52 +0000 Subject: [PATCH 04/22] core: replace ancient YOLO loop in fire(). --- mitogen/core.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mitogen/core.py b/mitogen/core.py index 893a1e1e..7aea8704 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -329,7 +329,8 @@ def fire(obj, name, *args, **kwargs): registered for the named signal on `obj`. """ signals = vars(obj).get('_signals', {}) - return [func(*args, **kwargs) for func in signals.get(name, ())] + for func in signals.get(name, ()): + func(*args, **kwargs) def takes_econtext(func): From 630c058a898b9bf51151cc7752cfaf88577b2bca Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 21 Jan 2019 19:40:16 +0000 Subject: [PATCH 05/22] tests: rename 'delegation/' to 'connection_delegation/' --- tests/ansible/integration/all.yml | 2 +- .../{delegation => connection_delegation}/all.yml | 0 .../delegate_to_template.yml | 12 ++++++++++-- .../osa_container_standalone.yml | 2 +- .../osa_delegate_to_self.yml | 2 +- .../stack_construction.yml | 2 +- 6 files changed, 14 insertions(+), 6 deletions(-) rename tests/ansible/integration/{delegation => connection_delegation}/all.yml (100%) rename tests/ansible/integration/{delegation => connection_delegation}/delegate_to_template.yml (81%) rename tests/ansible/integration/{delegation => connection_delegation}/osa_container_standalone.yml (91%) rename tests/ansible/integration/{delegation => connection_delegation}/osa_delegate_to_self.yml (92%) rename tests/ansible/integration/{delegation => connection_delegation}/stack_construction.yml (99%) diff --git a/tests/ansible/integration/all.yml b/tests/ansible/integration/all.yml index c1eaf68a..43c4db96 100644 --- a/tests/ansible/integration/all.yml +++ b/tests/ansible/integration/all.yml @@ -7,9 +7,9 @@ - import_playbook: async/all.yml - import_playbook: become/all.yml - import_playbook: connection/all.yml +- import_playbook: connection_delegation/all.yml - import_playbook: connection_loader/all.yml - import_playbook: context_service/all.yml -- import_playbook: delegation/all.yml - import_playbook: glibc_caches/all.yml - import_playbook: local/all.yml - import_playbook: module_utils/all.yml diff --git a/tests/ansible/integration/delegation/all.yml b/tests/ansible/integration/connection_delegation/all.yml similarity index 100% rename from tests/ansible/integration/delegation/all.yml rename to tests/ansible/integration/connection_delegation/all.yml diff --git a/tests/ansible/integration/delegation/delegate_to_template.yml b/tests/ansible/integration/connection_delegation/delegate_to_template.yml similarity index 81% rename from tests/ansible/integration/delegation/delegate_to_template.yml rename to tests/ansible/integration/connection_delegation/delegate_to_template.yml index 2f0830c4..247adc85 100644 --- a/tests/ansible/integration/delegation/delegate_to_template.yml +++ b/tests/ansible/integration/connection_delegation/delegate_to_template.yml @@ -1,6 +1,14 @@ -# Ensure templated delegate_to field works. +# issue #340: Ensure templated delegate_to field works. +# +# Here we delegate from "test-targets" group to a templated "{{physical_host}}" +# variable, which contains "cd-normal-alias", which has a +# "mitogen_via=cd-alias", which in turn has an "ansible_host="alias-host". +# +# So the full stack should be: +# - First hop: hostname "alias-host", username "alias-user" +# - Second hop: hostname "cd-normal-alias" -- name: integration/delegation/delegate_to_template.yml +- name: integration/connection_delegation/delegate_to_template.yml vars: physical_host: "cd-normal-alias" physical_hosts: ["cd-normal-alias", "cd-normal-normal"] diff --git a/tests/ansible/integration/delegation/osa_container_standalone.yml b/tests/ansible/integration/connection_delegation/osa_container_standalone.yml similarity index 91% rename from tests/ansible/integration/delegation/osa_container_standalone.yml rename to tests/ansible/integration/connection_delegation/osa_container_standalone.yml index b942ef63..5c3699ae 100644 --- a/tests/ansible/integration/delegation/osa_container_standalone.yml +++ b/tests/ansible/integration/connection_delegation/osa_container_standalone.yml @@ -1,6 +1,6 @@ # Verify one OSA-style container has the correct config. -- name: integration/delegation/container_standalone.yml +- name: integration/connection_delegation/container_standalone.yml hosts: dtc-container-1 gather_facts: false tasks: diff --git a/tests/ansible/integration/delegation/osa_delegate_to_self.yml b/tests/ansible/integration/connection_delegation/osa_delegate_to_self.yml similarity index 92% rename from tests/ansible/integration/delegation/osa_delegate_to_self.yml rename to tests/ansible/integration/connection_delegation/osa_delegate_to_self.yml index a9cd6c6e..0c6a62c9 100644 --- a/tests/ansible/integration/delegation/osa_delegate_to_self.yml +++ b/tests/ansible/integration/connection_delegation/osa_delegate_to_self.yml @@ -1,6 +1,6 @@ # OSA: Verify delegating the connection back to the container succeeds. -- name: integration/delegation/osa_delegate_to_self.yml +- name: integration/connection_delegation/osa_delegate_to_self.yml hosts: osa-container-1 vars: target: osa-container-1 diff --git a/tests/ansible/integration/delegation/stack_construction.yml b/tests/ansible/integration/connection_delegation/stack_construction.yml similarity index 99% rename from tests/ansible/integration/delegation/stack_construction.yml rename to tests/ansible/integration/connection_delegation/stack_construction.yml index 4d9c75f4..4241d4e7 100644 --- a/tests/ansible/integration/delegation/stack_construction.yml +++ b/tests/ansible/integration/connection_delegation/stack_construction.yml @@ -16,7 +16,7 @@ # the result list element, it seems to cause assert to silently succeed! -- name: integration/delegation/stack_construction.yml +- name: integration/connection_delegation/stack_construction.yml hosts: cd-normal tasks: - meta: end_play From 91c9aff9fff2c90e1202d9f69ac82463803ed7a3 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 21 Jan 2019 20:19:02 +0000 Subject: [PATCH 06/22] tests: import assert_equal action. --- tests/ansible/lib/action/assert_equal.py | 61 ++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 tests/ansible/lib/action/assert_equal.py diff --git a/tests/ansible/lib/action/assert_equal.py b/tests/ansible/lib/action/assert_equal.py new file mode 100644 index 00000000..b36e6e55 --- /dev/null +++ b/tests/ansible/lib/action/assert_equal.py @@ -0,0 +1,61 @@ +# +# Print data structure diff on assertion failure. +# +# assert_equal: left=some.result right={1:2} +# + +__metaclass__ = type + +import unittest2 + +from ansible.errors import AnsibleError +from ansible.plugins.action import ActionBase +from ansible.module_utils.six import string_types + + +class TestCase(unittest2.TestCase): + def runTest(self): + pass + + +def text_diff(a, b): + tc = TestCase() + tc.maxDiff = None + try: + tc.assertEqual(a, b) + return None + except AssertionError as e: + return str(e) + + +class ActionModule(ActionBase): + ''' Fail with custom message ''' + + TRANSFERS_FILES = False + _VALID_ARGS = frozenset(('left', 'right')) + + def run(self, tmp=None, task_vars=None): + result = super(ActionModule, self).run(tmp, task_vars or {}) + + left = self._templar.template( + self._task.args['left'], + convert_bare=True, + bare_deprecated=False, + ) + right = self._templar.template(self._task.args['right'], + convert_bare=True, + bare_deprecated=False, + ) + + diff = text_diff(left, right) + if diff is None: + return { + 'changed': False + } + + return { + 'changed': False, + 'failed': True, + 'msg': diff, + '_ansible_verbose_always': True, + } From 17eff064b00422863c6ad22254e413a0243861f0 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 21 Jan 2019 20:19:30 +0000 Subject: [PATCH 07/22] tests: use assert_equal in delegate_to_template.yml. --- .../delegate_to_template.yml | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/ansible/integration/connection_delegation/delegate_to_template.yml b/tests/ansible/integration/connection_delegation/delegate_to_template.yml index 247adc85..384de6f3 100644 --- a/tests/ansible/integration/connection_delegation/delegate_to_template.yml +++ b/tests/ansible/integration/connection_delegation/delegate_to_template.yml @@ -23,19 +23,19 @@ delegate_to: "{{ physical_host }}" register: out - - assert: - that: | - out.result == [ + - assert_equal: + left: out.result + right: [ { 'kwargs': { 'check_host_keys': 'ignore', 'connect_timeout': 10, 'hostname': 'alias-host', 'identities_only': False, - 'identity_file': None, - 'password': None, - 'port': None, - 'python_path': None, + 'identity_file': null, + 'password': null, + 'port': null, + 'python_path': null, 'ssh_args': [ '-o', 'ForwardAgent=yes', @@ -44,9 +44,9 @@ '-o', 'ControlPersist=60s', ], - 'ssh_debug_level': None, + 'ssh_debug_level': null, 'ssh_path': 'ssh', - 'username': 'alias-user', + 'username': 'alias-userx', }, 'method': 'ssh', }, @@ -56,10 +56,10 @@ 'connect_timeout': 10, 'hostname': 'cd-normal-alias', 'identities_only': False, - 'identity_file': None, - 'password': None, - 'port': None, - 'python_path': None, + 'identity_file': null, + 'password': null, + 'port': null, + 'python_path': null, 'ssh_args': [ '-o', 'ForwardAgent=yes', @@ -68,9 +68,9 @@ '-o', 'ControlPersist=60s', ], - 'ssh_debug_level': None, + 'ssh_debug_level': null, 'ssh_path': 'ssh', - 'username': None, + 'username': null, }, 'method': 'ssh', } From 4256d2aa4b8eba4eb2fe7246f5d98279f6e5b42b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 21 Jan 2019 20:22:31 +0000 Subject: [PATCH 08/22] tests: make fork_histogram optional --- tests/ansible/lib/callback/fork_histogram.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/ansible/lib/callback/fork_histogram.py b/tests/ansible/lib/callback/fork_histogram.py index 502fb5d5..9ce50e13 100644 --- a/tests/ansible/lib/callback/fork_histogram.py +++ b/tests/ansible/lib/callback/fork_histogram.py @@ -27,7 +27,8 @@ class CallbackModule(ansible.plugins.callback.CallbackBase): self.hist = hdrh.histogram.HdrHistogram(1, int(1e6*60), 3) self.fork_latency_sum_usec = 0.0 - self.install() + if 'FORK_HISTOGRAM' in os.environ: + self.install() def install(self): self.faults_at_start = get_fault_count(resource.RUSAGE_SELF) @@ -53,6 +54,9 @@ class CallbackModule(ansible.plugins.callback.CallbackBase): self.hist.record_value(latency_usec) def playbook_on_stats(self, stats): + if 'FORK_HISTOGRAM' not in os.environ: + return + self_faults = get_fault_count(resource.RUSAGE_SELF) - self.faults_at_start child_faults = get_fault_count() run_duration_sec = time.time() - self.run_start_time From 8891b480806e48979d6c0c8b3ac717c1046b4e03 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 21 Jan 2019 20:29:25 +0000 Subject: [PATCH 09/22] tests: convert stack_construction.yml to assert_equal. --- .../stack_construction.yml | 212 +++++++++--------- 1 file changed, 106 insertions(+), 106 deletions(-) diff --git a/tests/ansible/integration/connection_delegation/stack_construction.yml b/tests/ansible/integration/connection_delegation/stack_construction.yml index 4241d4e7..726ef5e6 100644 --- a/tests/ansible/integration/connection_delegation/stack_construction.yml +++ b/tests/ansible/integration/connection_delegation/stack_construction.yml @@ -4,7 +4,7 @@ # 'connection stack' -- this is just a list of dictionaries specifying a # sequence of proxied Router connection methods and their kwargs used to # establish the connection. That list is passed to ContextService, which loops -# over the stack specifying via=(None or previous entry) for each connection +# over the stack specifying via=(null or previous entry) for each connection # method. # mitogen_get_stack is a magic action that returns the stack, so we can test @@ -35,20 +35,20 @@ - mitogen_get_stack: register: out - - assert: - that: | - out.result == [ - { - "kwargs": { - "connect_timeout": 10, - "doas_path": None, - "password": None, - "python_path": ["/usr/bin/python"], - "username": "normal-user", - }, - "method": "doas", - } - ] + - assert_equal: + left: out.result + right: [ + { + "kwargs": { + "connect_timeout": 10, + "doas_path": null, + "password": null, + "python_path": ["/usr/bin/python"], + "username": "normal-user", + }, + "method": "doas", + } + ] - hosts: cd-normal @@ -59,19 +59,19 @@ - mitogen_get_stack: delegate_to: cd-alias register: out - - assert: - that: | - out.result == [ + - assert_equal: + left: out.result + right: [ { 'kwargs': { 'check_host_keys': 'ignore', 'connect_timeout': 10, 'hostname': 'alias-host', 'identities_only': False, - 'identity_file': None, - 'password': None, - 'port': None, - 'python_path': None, + 'identity_file': null, + 'password': null, + 'port': null, + 'python_path': null, 'ssh_args': [ '-o', 'ForwardAgent=yes', @@ -80,7 +80,7 @@ '-o', 'ControlPersist=60s', ], - 'ssh_debug_level': None, + 'ssh_debug_level': null, 'ssh_path': 'ssh', 'username': 'alias-user', }, @@ -96,18 +96,18 @@ - mitogen_get_stack: register: out - - assert: - that: | - out.result == [ + - assert_equal: + left: out.result + right: [ { 'kwargs': { 'check_host_keys': 'ignore', 'connect_timeout': 10, 'hostname': 'alias-host', 'identities_only': False, - 'identity_file': None, - 'password': None, - 'port': None, + 'identity_file': null, + 'password': null, + 'port': null, 'python_path': ['/usr/bin/python'], 'ssh_args': [ '-o', @@ -117,7 +117,7 @@ '-o', 'ControlPersist=60s', ], - 'ssh_debug_level': None, + 'ssh_debug_level': null, 'ssh_path': 'ssh', 'username': 'alias-user', }, @@ -133,15 +133,15 @@ - mitogen_get_stack: register: out - - assert: - that: | - out.result == [ + - assert_equal: + left: out.result + right: [ { 'kwargs': { 'connect_timeout': 10, - 'doas_path': None, - 'password': None, - 'python_path': None, + 'doas_path': null, + 'password': null, + 'python_path': null, 'username': 'normal-user', }, 'method': 'doas', @@ -152,9 +152,9 @@ 'connect_timeout': 10, 'hostname': 'cd-normal-normal', 'identities_only': False, - 'identity_file': None, - 'password': None, - 'port': None, + 'identity_file': null, + 'password': null, + 'port': null, 'python_path': ['/usr/bin/python'], 'ssh_args': [ '-o', @@ -164,9 +164,9 @@ '-o', 'ControlPersist=60s', ], - 'ssh_debug_level': None, + 'ssh_debug_level': null, 'ssh_path': 'ssh', - 'username': None, + 'username': null, }, 'method': 'ssh', }, @@ -180,19 +180,19 @@ - mitogen_get_stack: register: out - - assert: - that: | - out.result == [ + - assert_equal: + left: out.result + right: [ { 'kwargs': { 'check_host_keys': 'ignore', 'connect_timeout': 10, 'hostname': 'alias-host', 'identities_only': False, - 'identity_file': None, - 'password': None, - 'port': None, - 'python_path': None, + 'identity_file': null, + 'password': null, + 'port': null, + 'python_path': null, 'ssh_args': [ '-o', 'ForwardAgent=yes', @@ -201,7 +201,7 @@ '-o', 'ControlPersist=60s', ], - 'ssh_debug_level': None, + 'ssh_debug_level': null, 'ssh_path': 'ssh', 'username': 'alias-user', }, @@ -213,9 +213,9 @@ 'connect_timeout': 10, 'hostname': 'cd-normal-alias', 'identities_only': False, - 'identity_file': None, - 'password': None, - 'port': None, + 'identity_file': null, + 'password': null, + 'port': null, 'python_path': ['/usr/bin/python'], 'ssh_args': [ '-o', @@ -225,9 +225,9 @@ '-o', 'ControlPersist=60s', ], - 'ssh_debug_level': None, + 'ssh_debug_level': null, 'ssh_path': 'ssh', - 'username': None, + 'username': null, }, 'method': 'ssh', }, @@ -241,15 +241,15 @@ - mitogen_get_stack: register: out - - assert: - that: | - out.result == [ + - assert_equal: + left: out.result + right: [ { 'kwargs': { 'connect_timeout': 10, - 'doas_path': None, - 'password': None, - 'python_path': None, + 'doas_path': null, + 'password': null, + 'python_path': null, 'username': 'normal-user', }, 'method': 'doas', @@ -260,9 +260,9 @@ 'connect_timeout': 10, 'hostname': 'cd-newuser-normal-normal', 'identities_only': False, - 'identity_file': None, - 'password': None, - 'port': None, + 'identity_file': null, + 'password': null, + 'port': null, 'python_path': ['/usr/bin/python'], 'ssh_args': [ '-o', @@ -272,7 +272,7 @@ '-o', 'ControlPersist=60s', ], - 'ssh_debug_level': None, + 'ssh_debug_level': null, 'ssh_path': 'ssh', 'username': 'newuser-normal-normal-user', }, @@ -289,19 +289,19 @@ - mitogen_get_stack: delegate_to: cd-alias register: out - - assert: - that: | - out.result == [ + - assert_equal: + left: out.result + right: [ { 'kwargs': { 'check_host_keys': 'ignore', 'connect_timeout': 10, 'hostname': 'alias-host', 'identities_only': False, - 'identity_file': None, - 'password': None, - 'port': None, - 'python_path': None, + 'identity_file': null, + 'password': null, + 'port': null, + 'python_path': null, 'ssh_args': [ '-o', 'ForwardAgent=yes', @@ -310,7 +310,7 @@ '-o', 'ControlPersist=60s', ], - 'ssh_debug_level': None, + 'ssh_debug_level': null, 'ssh_path': 'ssh', 'username': 'alias-user', }, @@ -326,16 +326,16 @@ - local_action: mitogen_get_stack register: out - - assert: - that: | - out.result == [ - { - 'kwargs': { - 'python_path': None - }, - 'method': 'local', - }, - ] + - assert_equal: + left: out.result + right: [ + { + 'kwargs': { + 'python_path': null + }, + 'method': 'local', + }, + ] - hosts: cd-newuser-doas-normal @@ -345,27 +345,27 @@ - mitogen_get_stack: register: out - - assert: - that: | - out.result == [ - { - 'kwargs': { - 'connect_timeout': 10, - 'doas_path': None, - 'password': None, - 'python_path': None, - 'username': 'normal-user', - }, - 'method': 'doas', - }, - { - 'kwargs': { - 'connect_timeout': 10, - 'doas_path': None, - 'password': None, - 'python_path': ['/usr/bin/python'], - 'username': 'newuser-doas-normal-user', - }, - 'method': 'doas', - }, - ] + - assert_equal: + left: out.result + right: [ + { + 'kwargs': { + 'connect_timeout': 10, + 'doas_path': null, + 'password': null, + 'python_path': null, + 'username': 'normal-user', + }, + 'method': 'doas', + }, + { + 'kwargs': { + 'connect_timeout': 10, + 'doas_path': null, + 'password': null, + 'python_path': ['/usr/bin/python'], + 'username': 'newuser-doas-normal-user', + }, + 'method': 'doas', + }, + ] From d72567b15bc5efabfe5d4600606c5623200f3aec Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 21 Jan 2019 20:29:37 +0000 Subject: [PATCH 10/22] tests: make assert_equal work on newer Ansibles. --- tests/ansible/lib/action/assert_equal.py | 29 ++++++++++++++++-------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/tests/ansible/lib/action/assert_equal.py b/tests/ansible/lib/action/assert_equal.py index b36e6e55..84ec7606 100644 --- a/tests/ansible/lib/action/assert_equal.py +++ b/tests/ansible/lib/action/assert_equal.py @@ -6,13 +6,23 @@ __metaclass__ = type +import inspect import unittest2 +import ansible.template + from ansible.errors import AnsibleError from ansible.plugins.action import ActionBase from ansible.module_utils.six import string_types +TEMPLATE_KWARGS = {} + +_argspec = inspect.getargspec(ansible.template.Templar.template) +if 'bare_deprecated' in _argspec.args: + TEMPLATE_KWARGS['bare_deprecated'] = False + + class TestCase(unittest2.TestCase): def runTest(self): pass @@ -34,19 +44,18 @@ class ActionModule(ActionBase): TRANSFERS_FILES = False _VALID_ARGS = frozenset(('left', 'right')) - def run(self, tmp=None, task_vars=None): - result = super(ActionModule, self).run(tmp, task_vars or {}) - - left = self._templar.template( - self._task.args['left'], - convert_bare=True, - bare_deprecated=False, - ) - right = self._templar.template(self._task.args['right'], + def template(self, obj): + return self._templar.template( + obj, convert_bare=True, - bare_deprecated=False, + **TEMPLATE_KWARGS ) + def run(self, tmp=None, task_vars=None): + result = super(ActionModule, self).run(tmp, task_vars or {}) + left = self.template(self._task.args['left']) + right = self.template(self._task.args['right']) + diff = text_diff(left, right) if diff is None: return { From 1b8748a8d93bcecd51cd8ed14726696e311fcdc8 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 21 Jan 2019 20:35:08 +0000 Subject: [PATCH 11/22] tests: use assert_equal in more places. --- .../delegate_to_template.yml | 96 +++++++++---------- .../osa_container_standalone.yml | 34 +++---- .../osa_delegate_to_self.yml | 34 +++---- 3 files changed, 82 insertions(+), 82 deletions(-) diff --git a/tests/ansible/integration/connection_delegation/delegate_to_template.yml b/tests/ansible/integration/connection_delegation/delegate_to_template.yml index 384de6f3..b6b33355 100644 --- a/tests/ansible/integration/connection_delegation/delegate_to_template.yml +++ b/tests/ansible/integration/connection_delegation/delegate_to_template.yml @@ -26,52 +26,52 @@ - assert_equal: left: out.result right: [ - { - 'kwargs': { - 'check_host_keys': 'ignore', - 'connect_timeout': 10, - 'hostname': 'alias-host', - 'identities_only': False, - 'identity_file': null, - 'password': null, - 'port': null, - 'python_path': null, - 'ssh_args': [ - '-o', - 'ForwardAgent=yes', - '-o', - 'ControlMaster=auto', - '-o', - 'ControlPersist=60s', - ], - 'ssh_debug_level': null, - 'ssh_path': 'ssh', - 'username': 'alias-userx', - }, - 'method': 'ssh', + { + 'kwargs': { + 'check_host_keys': 'ignore', + 'connect_timeout': 10, + 'hostname': 'alias-host', + 'identities_only': False, + 'identity_file': null, + 'password': null, + 'port': null, + 'python_path': null, + 'ssh_args': [ + '-o', + 'ForwardAgent=yes', + '-o', + 'ControlMaster=auto', + '-o', + 'ControlPersist=60s', + ], + 'ssh_debug_level': null, + 'ssh_path': 'ssh', + 'username': 'alias-user', }, - { - 'kwargs': { - 'check_host_keys': 'ignore', - 'connect_timeout': 10, - 'hostname': 'cd-normal-alias', - 'identities_only': False, - 'identity_file': null, - 'password': null, - 'port': null, - 'python_path': null, - 'ssh_args': [ - '-o', - 'ForwardAgent=yes', - '-o', - 'ControlMaster=auto', - '-o', - 'ControlPersist=60s', - ], - 'ssh_debug_level': null, - 'ssh_path': 'ssh', - 'username': null, - }, - 'method': 'ssh', - } - ] + 'method': 'ssh', + }, + { + 'kwargs': { + 'check_host_keys': 'ignore', + 'connect_timeout': 10, + 'hostname': 'cd-normal-alias', + 'identities_only': False, + 'identity_file': null, + 'password': null, + 'port': null, + 'python_path': null, + 'ssh_args': [ + '-o', + 'ForwardAgent=yes', + '-o', + 'ControlMaster=auto', + '-o', + 'ControlPersist=60s', + ], + 'ssh_debug_level': null, + 'ssh_path': 'ssh', + 'username': null, + }, + 'method': 'ssh', + } + ] diff --git a/tests/ansible/integration/connection_delegation/osa_container_standalone.yml b/tests/ansible/integration/connection_delegation/osa_container_standalone.yml index 5c3699ae..d6483bd6 100644 --- a/tests/ansible/integration/connection_delegation/osa_container_standalone.yml +++ b/tests/ansible/integration/connection_delegation/osa_container_standalone.yml @@ -1,6 +1,6 @@ # Verify one OSA-style container has the correct config. -- name: integration/connection_delegation/container_standalone.yml +- name: integration/connection_delegation/osa_container_standalone.yml hosts: dtc-container-1 gather_facts: false tasks: @@ -10,19 +10,19 @@ - mitogen_get_stack: register: out - - assert: - that: | - out.result == [ - { - 'kwargs': { - 'container': 'dtc-container-1', - 'docker_path': None, - 'kind': 'lxc', - 'lxc_info_path': None, - 'machinectl_path': None, - 'python_path': ['/usr/bin/python'], - 'username': None, - }, - 'method': 'setns', - }, - ] + - assert_equal: + left: out.result + right: [ + { + 'kwargs': { + 'container': 'dtc-container-1', + 'docker_path': null, + 'kind': 'lxc', + 'lxc_info_path': null, + 'machinectl_path': null, + 'python_path': ['/usr/bin/python'], + 'username': null, + }, + 'method': 'setns', + }, + ] diff --git a/tests/ansible/integration/connection_delegation/osa_delegate_to_self.yml b/tests/ansible/integration/connection_delegation/osa_delegate_to_self.yml index 0c6a62c9..d8d25f9f 100644 --- a/tests/ansible/integration/connection_delegation/osa_delegate_to_self.yml +++ b/tests/ansible/integration/connection_delegation/osa_delegate_to_self.yml @@ -13,20 +13,20 @@ delegate_to: "{{target}}" register: out - - assert: - that: | - out.result == [ - { - 'kwargs': { - 'container': 'osa-container-1', - 'docker_path': None, - 'kind': 'lxc', - 'lxc_info_path': None, - 'lxc_path': None, - 'machinectl_path': None, - 'python_path': None, - 'username': None, - }, - 'method': 'setns', - }, - ] + - assert_equal: + left: out.result + right: [ + { + 'kwargs': { + 'container': 'osa-container-1', + 'docker_path': null, + 'kind': 'lxc', + 'lxc_info_path': null, + 'lxc_path': null, + 'machinectl_path': null, + 'python_path': null, + 'username': null, + }, + 'method': 'setns', + }, + ] From 6ca2677de5e6f54641d785a1ed54d863e510feaf Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 21 Jan 2019 21:31:37 +0000 Subject: [PATCH 12/22] ansible: fix test failure during process exit. ====================================================================== ERROR: tests.connection_test (unittest2.loader._FailedTest) ---------------------------------------------------------------------- Traceback (most recent call last): ImportError: Failed to import test module: tests.connection_test Traceback (most recent call last): File "/home/dmw/src/mitogen/.venv/local/lib/python2.7/site-packages/unittest2/loader.py", line 456, in _find_test_path module = self._get_module_from_name(name) File "/home/dmw/src/mitogen/.venv/local/lib/python2.7/site-packages/unittest2/loader.py", line 395, in _get_module_from_name __import__(name) RuntimeError: not holding the import lock --- ansible_mitogen/process.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ansible_mitogen/process.py b/ansible_mitogen/process.py index 219d78a5..fd5d5c20 100644 --- a/ansible_mitogen/process.py +++ b/ansible_mitogen/process.py @@ -194,7 +194,10 @@ class MuxProcess(object): cls.worker_sock = None self = cls() self.worker_main() - sys.exit() + # Test frameworks living somewhere higher on the stack of the + # original parent process may try to catch sys.exit(), so do a C + # level exit instead. + os._exit(0) def worker_main(self): """ From 73a0c485cf7480ee528ef49d900b86a236a96631 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 22 Jan 2019 00:24:43 +0000 Subject: [PATCH 13/22] tests: CI should symlink all contents of ansible/hosts/ --- .ci/ansible_tests.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.ci/ansible_tests.py b/.ci/ansible_tests.py index 485dff76..98e45ab8 100755 --- a/.ci/ansible_tests.py +++ b/.ci/ansible_tests.py @@ -1,6 +1,7 @@ #!/usr/bin/env python # Run tests/ansible/all.yml under Ansible and Ansible-Mitogen +import glob import os import sys @@ -30,7 +31,9 @@ with ci_lib.Fold('job_setup'): os.chmod('../data/docker/mitogen__has_sudo_pubkey.key', int('0600', 7)) run("mkdir %s", HOSTS_DIR) - run("ln -s %s/hosts/common-hosts %s", TESTS_DIR, HOSTS_DIR) + for path in glob.glob(TESTS_DIR + '/hosts/*'): + if not path.endswith('default.hosts'): + run("ln -s %s %s", path, HOSTS_DIR) inventory_path = os.path.join(HOSTS_DIR, 'target') with open(inventory_path, 'w') as fp: From 8414ff21ca4e25d021ec391baf54e225981822f2 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 22 Jan 2019 00:36:23 +0000 Subject: [PATCH 14/22] issue #434: tests: set a default remote_user in ansible.cfg. --- tests/ansible/ansible.cfg | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ansible/ansible.cfg b/tests/ansible/ansible.cfg index 1406d2c4..eb45e13c 100644 --- a/tests/ansible/ansible.cfg +++ b/tests/ansible/ansible.cfg @@ -13,6 +13,9 @@ retry_files_enabled = False display_args_to_stdout = True forks = 100 +# issue #434; hosts/delegate_to; integration/delegate_to +remote_user = ansible-cfg-remote-user + # On MacOS, "smart" with a password set causes Ansible to use paramiko. transport = ssh From 2ad05f1238139664c87b4d55a4c797be67793979 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 22 Jan 2019 00:40:32 +0000 Subject: [PATCH 15/22] issue #251, #412, #434: fix connection configuration brainwrong This refactors connection.py to pull the two huge dict-building functions out into new transport_transport_config.PlayContextSpec and MitogenViaSpec classes, leaving a lot more room to breath in both files to figure out exactly how connection configuration should work. The changes made in 1f21a30 / 3d58832 are updated or completely removed, the original change was misguided, in a bid to fix connection delegation taking variables from the wrong place when delegate_to was active. The Python path no longer defaults to '/usr/bin/python', this does not appear to be Ansible's normal behaviour. This has changed several times, so it may have to change again, and it may cause breakage after release. Connection delegation respects the c.DEFAULT_REMOTE_USER whereas the previous version simply tried to fetch whatever was in the 'ansible_user' hostvar. Many more connection delegation variables closer match vanilla's handling, but this still requires more work. Some of the variables need access to the command line, and upstream are in the process of changing all that stuff around. --- ansible_mitogen/connection.py | 347 ++++++------------ ansible_mitogen/transport_config.py | 330 +++++++++++++++++ .../delegate_to_template.yml | 2 +- .../osa_delegate_to_self.yml | 2 +- .../stack_construction.yml | 16 +- .../stub_connections/setns_lxc.yml | 1 + .../stub_connections/setns_lxd.yml | 2 +- 7 files changed, 463 insertions(+), 237 deletions(-) create mode 100644 ansible_mitogen/transport_config.py diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index a8bb74c7..643ecb94 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -52,6 +52,7 @@ import ansible_mitogen.parsing import ansible_mitogen.process import ansible_mitogen.services import ansible_mitogen.target +import ansible_mitogen.transport_config LOG = logging.getLogger(__name__) @@ -77,15 +78,6 @@ def optional_int(value): return None -def parse_python_path(s): - """ - Given the string set for ansible_python_interpeter, parse it using shell - syntax and return an appropriate argument vector. - """ - if s: - return ansible.utils.shlex.shlex_split(s) - - def _connect_local(spec): """ Return ContextService arguments for a local connection. @@ -93,7 +85,7 @@ def _connect_local(spec): return { 'method': 'local', 'kwargs': { - 'python_path': spec['python_path'], + 'python_path': spec.python_path(), } } @@ -109,7 +101,7 @@ def _connect_ssh(spec): # #334: tilde-expand private_key_file to avoid implementation difference # between Python and OpenSSH. - private_key_file = spec['private_key_file'] + private_key_file = spec.private_key_file() if private_key_file is not None: private_key_file = os.path.expanduser(private_key_file) @@ -117,17 +109,17 @@ def _connect_ssh(spec): 'method': 'ssh', 'kwargs': { 'check_host_keys': check_host_keys, - 'hostname': spec['remote_addr'], - 'username': spec['remote_user'], - 'password': optional_secret(spec['password']), - 'port': spec['port'], - 'python_path': spec['python_path'], + 'hostname': spec.remote_addr(), + 'username': spec.remote_user(), + 'password': optional_secret(spec.password()), + 'port': spec.port(), + 'python_path': spec.python_path(), 'identity_file': private_key_file, 'identities_only': False, - 'ssh_path': spec['ssh_executable'], - 'connect_timeout': spec['ansible_ssh_timeout'], - 'ssh_args': spec['ssh_args'], - 'ssh_debug_level': spec['mitogen_ssh_debug_level'], + 'ssh_path': spec.ssh_executable(), + 'connect_timeout': spec.ansible_ssh_timeout(), + 'ssh_args': spec.ssh_args(), + 'ssh_debug_level': spec.mitogen_ssh_debug_level(), } } @@ -139,10 +131,10 @@ def _connect_docker(spec): return { 'method': 'docker', 'kwargs': { - 'username': spec['remote_user'], - 'container': spec['remote_addr'], - 'python_path': spec['python_path'], - 'connect_timeout': spec['ansible_ssh_timeout'] or spec['timeout'], + 'username': spec.remote_user(), + 'container': spec.remote_addr(), + 'python_path': spec.python_path(), + 'connect_timeout': spec.ansible_ssh_timeout() or spec.timeout(), } } @@ -154,11 +146,11 @@ def _connect_kubectl(spec): return { 'method': 'kubectl', 'kwargs': { - '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'], + '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(), } } @@ -170,10 +162,10 @@ def _connect_jail(spec): return { 'method': 'jail', 'kwargs': { - 'username': spec['remote_user'], - 'container': spec['remote_addr'], - 'python_path': spec['python_path'], - 'connect_timeout': spec['ansible_ssh_timeout'] or spec['timeout'], + 'username': spec.remote_user(), + 'container': spec.remote_addr(), + 'python_path': spec.python_path(), + 'connect_timeout': spec.ansible_ssh_timeout() or spec.timeout(), } } @@ -185,10 +177,10 @@ def _connect_lxc(spec): return { 'method': 'lxc', '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'], + '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(), } } @@ -200,10 +192,10 @@ def _connect_lxd(spec): return { 'method': 'lxd', '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'], + 'container': spec.remote_addr(), + 'python_path': spec.python_path(), + 'lxc_path': spec.mitogen_lxc_path(), + 'connect_timeout': spec.ansible_ssh_timeout() or spec.timeout(), } } @@ -212,24 +204,24 @@ def _connect_machinectl(spec): """ Return ContextService arguments for a machinectl connection. """ - return _connect_setns(dict(spec, mitogen_kind='machinectl')) + return _connect_setns(spec, kind='machinectl') -def _connect_setns(spec): +def _connect_setns(spec, kind=None): """ Return ContextService arguments for a mitogen_setns connection. """ return { 'method': 'setns', 'kwargs': { - 'container': spec['remote_addr'], - 'username': spec['remote_user'], - 'python_path': spec['python_path'], - 'kind': spec['mitogen_kind'], - 'docker_path': spec['mitogen_docker_path'], - 'lxc_path': spec['mitogen_lxc_path'], - 'lxc_info_path': spec['mitogen_lxc_info_path'], - 'machinectl_path': spec['mitogen_machinectl_path'], + 'container': spec.remote_addr(), + 'username': spec.remote_user(), + 'python_path': spec.python_path(), + 'kind': kind or spec.mitogen_kind(), + 'docker_path': spec.mitogen_docker_path(), + 'lxc_path': spec.mitogen_lxc_path(), + 'lxc_info_path': spec.mitogen_lxc_info_path(), + 'machinectl_path': spec.mitogen_machinectl_path(), } } @@ -242,11 +234,11 @@ def _connect_su(spec): 'method': 'su', 'enable_lru': True, 'kwargs': { - 'username': spec['become_user'], - 'password': optional_secret(spec['become_pass']), - 'python_path': spec['python_path'], - 'su_path': spec['become_exe'], - 'connect_timeout': spec['timeout'], + 'username': spec.become_user(), + 'password': optional_secret(spec.become_pass()), + 'python_path': spec.python_path(), + 'su_path': spec.become_exe(), + 'connect_timeout': spec.timeout(), } } @@ -259,12 +251,12 @@ def _connect_sudo(spec): 'method': 'sudo', 'enable_lru': True, 'kwargs': { - 'username': spec['become_user'], - 'password': optional_secret(spec['become_pass']), - 'python_path': spec['python_path'], - 'sudo_path': spec['become_exe'], - 'connect_timeout': spec['timeout'], - 'sudo_args': spec['sudo_args'], + 'username': spec.become_user(), + 'password': optional_secret(spec.become_pass()), + 'python_path': spec.python_path(), + 'sudo_path': spec.become_exe(), + 'connect_timeout': spec.timeout(), + 'sudo_args': spec.sudo_args(), } } @@ -277,11 +269,11 @@ def _connect_doas(spec): 'method': 'doas', 'enable_lru': True, 'kwargs': { - 'username': spec['become_user'], - 'password': optional_secret(spec['become_pass']), - 'python_path': spec['python_path'], - 'doas_path': spec['become_exe'], - 'connect_timeout': spec['timeout'], + 'username': spec.become_user(), + 'password': optional_secret(spec.become_pass()), + 'python_path': spec.python_path(), + 'doas_path': spec.become_exe(), + 'connect_timeout': spec.timeout(), } } @@ -293,11 +285,11 @@ def _connect_mitogen_su(spec): return { 'method': 'su', 'kwargs': { - 'username': spec['remote_user'], - 'password': optional_secret(spec['password']), - 'python_path': spec['python_path'], - 'su_path': spec['become_exe'], - 'connect_timeout': spec['timeout'], + 'username': spec.remote_user(), + 'password': optional_secret(spec.password()), + 'python_path': spec.python_path(), + 'su_path': spec.become_exe(), + 'connect_timeout': spec.timeout(), } } @@ -309,12 +301,12 @@ def _connect_mitogen_sudo(spec): return { 'method': 'sudo', 'kwargs': { - 'username': spec['remote_user'], - 'password': optional_secret(spec['password']), - 'python_path': spec['python_path'], - 'sudo_path': spec['become_exe'], - 'connect_timeout': spec['timeout'], - 'sudo_args': spec['sudo_args'], + 'username': spec.remote_user(), + 'password': optional_secret(spec.password()), + 'python_path': spec.python_path(), + 'sudo_path': spec.become_exe(), + 'connect_timeout': spec.timeout(), + 'sudo_args': spec.sudo_args(), } } @@ -326,11 +318,11 @@ def _connect_mitogen_doas(spec): return { 'method': 'doas', 'kwargs': { - 'username': spec['remote_user'], - 'password': optional_secret(spec['password']), - 'python_path': spec['python_path'], - 'doas_path': spec['become_exe'], - 'connect_timeout': spec['timeout'], + 'username': spec.remote_user(), + 'password': optional_secret(spec.password()), + 'python_path': spec.python_path(), + 'doas_path': spec.become_exe(), + 'connect_timeout': spec.timeout(), } } @@ -357,107 +349,6 @@ CONNECTION_METHOD = { } -def config_from_play_context(transport, inventory_name, connection): - """ - Return a dict representing all important connection configuration, allowing - the same functions to work regardless of whether configuration came from - play_context (direct connection) or host vars (mitogen_via=). - """ - return { - 'transport': transport, - 'inventory_name': inventory_name, - 'remote_addr': connection._play_context.remote_addr, - 'remote_user': connection._play_context.remote_user, - 'become': connection._play_context.become, - 'become_method': connection._play_context.become_method, - 'become_user': connection._play_context.become_user, - 'become_pass': connection._play_context.become_pass, - 'password': connection._play_context.password, - 'port': connection._play_context.port, - 'python_path': parse_python_path( - connection.get_task_var('ansible_python_interpreter', - default='/usr/bin/python') - ), - 'private_key_file': connection._play_context.private_key_file, - 'ssh_executable': connection._play_context.ssh_executable, - 'timeout': connection._play_context.timeout, - 'ansible_ssh_timeout': - connection.get_task_var('ansible_ssh_timeout', - default=C.DEFAULT_TIMEOUT), - 'ssh_args': [ - mitogen.core.to_text(term) - for s in ( - getattr(connection._play_context, 'ssh_args', ''), - getattr(connection._play_context, 'ssh_common_args', ''), - getattr(connection._play_context, 'ssh_extra_args', '') - ) - for term in ansible.utils.shlex.shlex_split(s or '') - ], - 'become_exe': connection._play_context.become_exe, - 'sudo_args': [ - mitogen.core.to_text(term) - for s in ( - connection._play_context.sudo_flags, - connection._play_context.become_flags - ) - for term in ansible.utils.shlex.shlex_split(s or '') - ], - 'mitogen_via': - connection.get_task_var('mitogen_via'), - 'mitogen_kind': - connection.get_task_var('mitogen_kind'), - 'mitogen_docker_path': - 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': - connection.get_task_var('mitogen_machinectl_path'), - 'mitogen_ssh_debug_level': - optional_int( - connection.get_task_var('mitogen_ssh_debug_level') - ), - 'extra_args': - connection.get_extra_args(), - } - - -def config_from_hostvars(transport, inventory_name, connection, - hostvars, become_user): - """ - Override config_from_play_context() to take equivalent information from - host vars. - """ - config = config_from_play_context(transport, inventory_name, connection) - hostvars = dict(hostvars) - return dict(config, **{ - 'remote_addr': hostvars.get('ansible_host', inventory_name), - 'become': bool(become_user), - 'become_user': become_user, - 'become_pass': None, - 'remote_user': hostvars.get('ansible_user'), # TODO - 'password': (hostvars.get('ansible_ssh_pass') or - hostvars.get('ansible_password')), - 'port': hostvars.get('ansible_port'), - 'python_path': parse_python_path(hostvars.get('ansible_python_interpreter')), - 'private_key_file': (hostvars.get('ansible_ssh_private_key_file') or - hostvars.get('ansible_private_key_file')), - 'mitogen_via': hostvars.get('mitogen_via'), - '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'), - }) - - class CallChain(mitogen.parent.CallChain): """ Extend :class:`mitogen.parent.CallChain` to additionally cause the @@ -599,8 +490,26 @@ class Connection(ansible.plugins.connection.ConnectionBase): self._mitogen_reset(mode='put') def get_task_var(self, key, default=None): - if self._task_vars and key in self._task_vars: - return self._task_vars[key] + """ + Fetch the value of a task variable related to connection configuration, + or, if delegate_to is active, fetch the same variable via HostVars for + the delegated-to machine. + + When running with delegate_to, Ansible tasks have variables associated + with the original machine, therefore it does not make sense to extract + connection-related configuration information from them. + """ + if self._task_vars: + if self.delegate_to_hostname is None: + if key in self._task_vars: + return self._task_vars[key] + else: + delegated_vars = self._task_vars['ansible_delegated_vars'] + if self.delegate_to_hostname in delegated_vars: + task_vars = delegated_vars[self.delegate_to_hostname] + if key in task_vars: + return task_vars[key] + return default @property @@ -612,12 +521,14 @@ class Connection(ansible.plugins.connection.ConnectionBase): def connected(self): return self.context is not None - def _config_from_via(self, via_spec): + def _spec_from_via(self, via_spec): """ Produce a dict connection specifiction given a string `via_spec`, of the form `[become_user@]inventory_hostname`. """ become_user, _, inventory_name = via_spec.rpartition('@') + become_method, _, become_user = become_user.rpartition(':') + via_vars = self.host_vars[inventory_name] if isinstance(via_vars, jinja2.runtime.Undefined): raise ansible.errors.AnsibleConnectionFailure( @@ -627,39 +538,38 @@ class Connection(ansible.plugins.connection.ConnectionBase): ) ) - return config_from_hostvars( - transport=via_vars.get('ansible_connection', 'ssh'), + return ansible_mitogen.transport_config.MitogenViaSpec( inventory_name=inventory_name, - connection=self, - hostvars=via_vars, + host_vars=dict(via_vars), # TODO: make it lazy + become_method=become_method or None, become_user=become_user or None, ) unknown_via_msg = 'mitogen_via=%s of %s specifies an unknown hostname' via_cycle_msg = 'mitogen_via=%s of %s creates a cycle (%s)' - def _stack_from_config(self, config, stack=(), seen_names=()): - if config['inventory_name'] in seen_names: + def _stack_from_spec(self, spec, stack=(), seen_names=()): + if spec.inventory_name() in seen_names: raise ansible.errors.AnsibleConnectionFailure( self.via_cycle_msg % ( - config['mitogen_via'], - config['inventory_name'], + spec.mitogen_via(), + spec.inventory_name(), ' -> '.join(reversed( - seen_names + (config['inventory_name'],) + seen_names + (spec.inventory_name(),) )), ) ) - if config['mitogen_via']: - stack, seen_names = self._stack_from_config( - self._config_from_via(config['mitogen_via']), + if spec.mitogen_via(): + stack, seen_names = self._stack_from_spec( + self._spec_from_via(spec.mitogen_via()), stack=stack, - seen_names=seen_names + (config['inventory_name'],) + seen_names=seen_names + (spec.inventory_name(),), ) - stack += (CONNECTION_METHOD[config['transport']](config),) - if config['become']: - stack += (CONNECTION_METHOD[config['become_method']](config),) + stack += (CONNECTION_METHOD[spec.transport()](spec),) + if spec.become(): + stack += (CONNECTION_METHOD[spec.become_method()](spec),) return stack, seen_names @@ -675,24 +585,13 @@ class Connection(ansible.plugins.connection.ConnectionBase): broker=self.broker, ) - def _config_from_direct_connection(self): - """ - """ - return config_from_play_context( - transport=self.transport, - inventory_name=self.inventory_hostname, - connection=self - ) - - def _config_from_delegate_to(self): - return config_from_hostvars( - transport=self._play_context.connection, - inventory_name=self.delegate_to_hostname, + def _build_spec(self): + inventory_hostname = self.inventory_hostname + return ansible_mitogen.transport_config.PlayContextSpec( connection=self, - hostvars=self.host_vars[self.delegate_to_hostname], - become_user=(self._play_context.become_user - if self._play_context.become - else None), + play_context=self._play_context, + transport=self.transport, + inventory_name=inventory_hostname, ) def _build_stack(self): @@ -702,12 +601,8 @@ class Connection(ansible.plugins.connection.ConnectionBase): additionally used by the integration tests "mitogen_get_stack" action to fetch the would-be connection configuration. """ - if self.delegate_to_hostname is not None: - target_config = self._config_from_delegate_to() - else: - target_config = self._config_from_direct_connection() - - stack, _ = self._stack_from_config(target_config) + config = self._build_spec() + stack, _ = self._stack_from_spec(config) return stack def _connect_stack(self, stack): diff --git a/ansible_mitogen/transport_config.py b/ansible_mitogen/transport_config.py new file mode 100644 index 00000000..2ad37c33 --- /dev/null +++ b/ansible_mitogen/transport_config.py @@ -0,0 +1,330 @@ + +""" +Mitogen extends Ansible's regular host configuration mechanism in several ways +that require quite a lot of care: + +* Some per-task configurables in Ansible like ansible_python_interpreter are + connection-layer configurables in Mitogen. They must be extracted during each + task execution to form the complete connection-layer configuration. + +* Mitogen has extra configurables not supported by Ansible at all, such as + mitogen_ssh_debug_level. These are extracted the same way as + ansible_python_interpreter. + +* Mitogen allows connections to be delegated to other machines. Ansible has no + internal framework for this, and so Mitogen must figure out a delegated + connection configuration all for itself. This means it cannot reuse much of + the Ansible machinery for building a connection configuration, as that + machinery is deeply spread and out hard-wired to expect Ansible's usual mode + of operation. + +For delegated connections, Ansible's PlayContext information is reused where +possible, but for proxy hops, configurations are built up using the HostVars +magic class to call VariableManager.get_vars() behind the scenes on our behalf. +Where Ansible has multiple sources of a configuration item, for example, +ansible_ssh_extra_args, Mitogen must (ideally perfectly) reproduce how Ansible +arrives at its value, without using mechanisms that are hard-wired or change +across Ansible versions. + +That is what this file is for. It exports two spec classes, one that takes all +information from PlayContext, and another that takes (almost) all information +from HostVars. +""" + +import os +import ansible.utils.shlex +import ansible.constants as C + +import mitogen.core + + +def parse_python_path(s): + """ + Given the string set for ansible_python_interpeter, parse it using shell + syntax and return an appropriate argument vector. + """ + if s: + return ansible.utils.shlex.shlex_split(s) + + +class PlayContextSpec: + """ + Return a dict representing all important connection configuration, allowing + the same functions to work regardless of whether configuration came from + play_context (direct connection) or host vars (mitogen_via=). + """ + def __init__(self, connection, play_context, transport, inventory_name): + self._connection = connection + self._play_context = play_context + self._transport = transport + self._inventory_name = inventory_name + + def transport(self): + return self._transport + + def inventory_name(self): + return self._inventory_name + + def remote_addr(self): + return self._play_context.remote_addr + + def remote_user(self): + return self._play_context.remote_user + + def become(self): + return self._play_context.become + + def become_method(self): + return self._play_context.become_method + + def become_user(self): + return self._play_context.become_user + + def become_pass(self): + return self._play_context.become_pass + + def password(self): + return self._play_context.password + + def port(self): + return self._play_context.port + + def python_path(self): + return parse_python_path( + self._connection.get_task_var('ansible_python_interpreter') + ) + + def private_key_file(self): + return self._play_context.private_key_file + + def ssh_executable(self): + return self._play_context.ssh_executable + + def timeout(self): + return self._play_context.timeout + + def ansible_ssh_timeout(self): + return ( + self._connection.get_task_var('ansible_timeout') or + self._connection.get_task_var('ansible_ssh_timeout') or + self._play_context.timeout + ) + + def ssh_args(self): + return [ + mitogen.core.to_text(term) + for s in ( + getattr(self._play_context, 'ssh_args', ''), + getattr(self._play_context, 'ssh_common_args', ''), + getattr(self._play_context, 'ssh_extra_args', '') + ) + for term in ansible.utils.shlex.shlex_split(s or '') + ] + + def become_exe(self): + return self._play_context.become_exe + + def sudo_args(self): + return [ + mitogen.core.to_text(term) + for s in ( + self._play_context.sudo_flags, + self._play_context.become_flags + ) + for term in ansible.utils.shlex.shlex_split(s or '') + ] + + def mitogen_via(self): + return self._connection.get_task_var('mitogen_via') + + def mitogen_kind(self): + return self._connection.get_task_var('mitogen_kind') + + def mitogen_docker_path(self): + return self._connection.get_task_var('mitogen_docker_path') + + def mitogen_kubectl_path(self): + return self._connection.get_task_var('mitogen_kubectl_path') + + def mitogen_lxc_path(self): + return self._connection.get_task_var('mitogen_lxc_path') + + def mitogen_lxc_attach_path(self): + return self._connection.get_task_var('mitogen_lxc_attach_path') + + def mitogen_lxc_info_path(self): + return self._connection.get_task_var('mitogen_lxc_info_path') + + def mitogen_machinectl_path(self): + return self._connection.get_task_var('mitogen_machinectl_path') + + def mitogen_ssh_debug_level(self): + return self._connection.get_task_var('mitogen_ssh_debug_level') + + def extra_args(self): + return self._connection.get_extra_args() + + +class MitogenViaSpec: + def __init__(self, inventory_name, host_vars, + become_method, become_user): + self._inventory_name = inventory_name + self._host_vars = host_vars + self._become_method = become_method + self._become_user = become_user + + def transport(self): + return ( + self._host_vars.get('ansible_connection') or + C.DEFAULT_TRANSPORT + ) + + def inventory_name(self): + return self._inventory_name + + def remote_addr(self): + return ( + self._host_vars.get('ansible_host') or + self._inventory_name + ) + + def remote_user(self): + return ( + self._host_vars.get('ansible_user') or + self._host_vars.get('ansible_ssh_user') or + C.DEFAULT_REMOTE_USER + ) + + def become(self): + return bool(self._become_user) + + def become_method(self): + return self._become_method or C.DEFAULT_BECOME_METHOD + + def become_user(self): + return self._become_user + + def become_pass(self): + return ( + # TODO: Might have to come from PlayContext. + self._host_vars.get('ansible_become_password') or + self._host_vars.get('ansible_become_pass') + ) + + def password(self): + return ( + # TODO: Might have to come from PlayContext. + self._host_vars.get('ansible_ssh_pass') or + self._host_vars.get('ansible_password') + ) + + def port(self): + return ( + self._host_vars.get('ansible_port') or + C.DEFAULT_REMOTE_PORT + ) + + def python_path(self): + s = parse_python_path( + self._host_vars.get('ansible_python_interpreter') + # This variable has no default for remote hosts. For local hosts it + # is sys.executable. + ) + print('hi ho', self.inventory_name(), s) + return s + + def private_key_file(self): + # TODO: must come from PlayContext too. + return ( + self._host_vars.get('ansible_ssh_private_key_file') or + self._host_vars.get('ansible_private_key_file') or + C.DEFAULT_PRIVATE_KEY_FILE + ) + + def ssh_executable(self): + return ( + self._host_vars.get('ansible_ssh_executable') or + C.ANSIBLE_SSH_EXECUTABLE + ) + + def timeout(self): + # TODO: must come from PlayContext too. + return C.DEFAULT_TIMEOUT + + def ansible_ssh_timeout(self): + return ( + self._host_vars.get('ansible_timeout') or + self._host_vars.get('ansible_ssh_timeout') or + self.timeout() + ) + + def ssh_args(self): + return [ + mitogen.core.to_text(term) + for s in ( + ( + self._host_vars.get('ansible_ssh_args') or + getattr(C, 'ANSIBLE_SSH_ARGS', None) or + os.environ.get('ANSIBLE_SSH_ARGS') + # TODO: ini entry. older versions. + ), + ( + self._host_vars.get('ansible_ssh_common_args') or + os.environ.get('ANSIBLE_SSH_COMMON_ARGS') + # TODO: ini entry. + ), + ( + self._host_vars.get('ansible_ssh_extra_args') or + os.environ.get('ANSIBLE_SSH_EXTRA_ARGS') + # TODO: ini entry. + ), + ) + for term in ansible.utils.shlex.shlex_split(s) + if s + ] + + def become_exe(self): + return ( + self._host_vars.get('ansible_become_exe') or + C.DEFAULT_BECOME_EXE + ) + + def sudo_args(self): + return [ + mitogen.core.to_text(term) + for s in ( + self._host_vars.get('ansible_sudo_flags') or '', + self._host_vars.get('ansible_become_flags') or '', + ) + for term in ansible.utils.shlex.shlex_split(s) + ] + + def mitogen_via(self): + return self._host_vars.get('mitogen_via') + + def mitogen_kind(self): + return self._host_vars.get('mitogen_kind') + + def mitogen_docker_path(self): + return self._host_vars.get('mitogen_docker_path') + + def mitogen_kubectl_path(self): + return self._host_vars.get('mitogen_kubectl_path') + + def mitogen_lxc_path(self): + return self.host_vars.get('mitogen_lxc_path') + + def mitogen_lxc_attach_path(self): + return self._host_vars.get('mitogen_lxc_attach_path') + + def mitogen_lxc_info_path(self): + return self._host_vars.get('mitogen_lxc_info_path') + + def mitogen_machinectl_path(self): + return self._host_vars.get('mitogen_machinectl_path') + + def mitogen_ssh_debug_level(self): + return self._host_vars.get('mitogen_ssh_debug_level') + + def extra_args(self): + return [] # TODO diff --git a/tests/ansible/integration/connection_delegation/delegate_to_template.yml b/tests/ansible/integration/connection_delegation/delegate_to_template.yml index b6b33355..1ffdc714 100644 --- a/tests/ansible/integration/connection_delegation/delegate_to_template.yml +++ b/tests/ansible/integration/connection_delegation/delegate_to_template.yml @@ -70,7 +70,7 @@ ], 'ssh_debug_level': null, 'ssh_path': 'ssh', - 'username': null, + 'username': 'ansible-cfg-remote-user', }, 'method': 'ssh', } diff --git a/tests/ansible/integration/connection_delegation/osa_delegate_to_self.yml b/tests/ansible/integration/connection_delegation/osa_delegate_to_self.yml index d8d25f9f..a761c432 100644 --- a/tests/ansible/integration/connection_delegation/osa_delegate_to_self.yml +++ b/tests/ansible/integration/connection_delegation/osa_delegate_to_self.yml @@ -25,7 +25,7 @@ 'lxc_path': null, 'machinectl_path': null, 'python_path': null, - 'username': null, + 'username': 'ansible-cfg-remote-user', }, 'method': 'setns', }, diff --git a/tests/ansible/integration/connection_delegation/stack_construction.yml b/tests/ansible/integration/connection_delegation/stack_construction.yml index 726ef5e6..1ab14218 100644 --- a/tests/ansible/integration/connection_delegation/stack_construction.yml +++ b/tests/ansible/integration/connection_delegation/stack_construction.yml @@ -43,7 +43,7 @@ "connect_timeout": 10, "doas_path": null, "password": null, - "python_path": ["/usr/bin/python"], + "python_path": null, "username": "normal-user", }, "method": "doas", @@ -108,7 +108,7 @@ 'identity_file': null, 'password': null, 'port': null, - 'python_path': ['/usr/bin/python'], + 'python_path': null, 'ssh_args': [ '-o', 'ForwardAgent=yes', @@ -155,7 +155,7 @@ 'identity_file': null, 'password': null, 'port': null, - 'python_path': ['/usr/bin/python'], + 'python_path': null, 'ssh_args': [ '-o', 'ForwardAgent=yes', @@ -166,7 +166,7 @@ ], 'ssh_debug_level': null, 'ssh_path': 'ssh', - 'username': null, + 'username': 'ansible-cfg-remote-user', }, 'method': 'ssh', }, @@ -216,7 +216,7 @@ 'identity_file': null, 'password': null, 'port': null, - 'python_path': ['/usr/bin/python'], + 'python_path': null, 'ssh_args': [ '-o', 'ForwardAgent=yes', @@ -227,7 +227,7 @@ ], 'ssh_debug_level': null, 'ssh_path': 'ssh', - 'username': null, + 'username': 'ansible-cfg-remote-user', }, 'method': 'ssh', }, @@ -263,7 +263,7 @@ 'identity_file': null, 'password': null, 'port': null, - 'python_path': ['/usr/bin/python'], + 'python_path': null, 'ssh_args': [ '-o', 'ForwardAgent=yes', @@ -363,7 +363,7 @@ 'connect_timeout': 10, 'doas_path': null, 'password': null, - 'python_path': ['/usr/bin/python'], + 'python_path': null, 'username': 'newuser-doas-normal-user', }, 'method': 'doas', diff --git a/tests/ansible/integration/stub_connections/setns_lxc.yml b/tests/ansible/integration/stub_connections/setns_lxc.yml index 4bb21db7..42f6658a 100644 --- a/tests/ansible/integration/stub_connections/setns_lxc.yml +++ b/tests/ansible/integration/stub_connections/setns_lxc.yml @@ -21,6 +21,7 @@ -e mitogen_lxc_info_path={{git_basedir}}/tests/data/stubs/stub-lxc-info.py -m shell -a "echo hi" + -u root localhost args: chdir: ../.. diff --git a/tests/ansible/integration/stub_connections/setns_lxd.yml b/tests/ansible/integration/stub_connections/setns_lxd.yml index e430daa9..b7add672 100644 --- a/tests/ansible/integration/stub_connections/setns_lxd.yml +++ b/tests/ansible/integration/stub_connections/setns_lxd.yml @@ -21,6 +21,7 @@ -e mitogen_lxc_path={{git_basedir}}/tests/data/stubs/stub-lxc.py -m shell -a "echo hi" + -u root localhost args: chdir: ../.. @@ -29,4 +30,3 @@ - assert: that: result.rc == 0 - From 115c3c5657b597f3c1c2ed813e9428784e923101 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 22 Jan 2019 01:25:46 +0000 Subject: [PATCH 16/22] issue #412: add docstrings/boilerplate to transport_config.py. --- ansible_mitogen/connection.py | 23 +-- ansible_mitogen/transport_config.py | 293 +++++++++++++++++++++++++--- 2 files changed, 272 insertions(+), 44 deletions(-) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index 643ecb94..0884da54 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -58,15 +58,6 @@ import ansible_mitogen.transport_config LOG = logging.getLogger(__name__) -def optional_secret(value): - """ - Wrap `value` in :class:`mitogen.core.Secret` if it is not :data:`None`, - otherwise return :data:`None`. - """ - if value is not None: - return mitogen.core.Secret(value) - - def optional_int(value): """ Convert `value` to an integer if it is not :data:`None`, otherwise return @@ -111,7 +102,7 @@ def _connect_ssh(spec): 'check_host_keys': check_host_keys, 'hostname': spec.remote_addr(), 'username': spec.remote_user(), - 'password': optional_secret(spec.password()), + 'password': spec.password(), 'port': spec.port(), 'python_path': spec.python_path(), 'identity_file': private_key_file, @@ -235,7 +226,7 @@ def _connect_su(spec): 'enable_lru': True, 'kwargs': { 'username': spec.become_user(), - 'password': optional_secret(spec.become_pass()), + 'password': spec.become_pass(), 'python_path': spec.python_path(), 'su_path': spec.become_exe(), 'connect_timeout': spec.timeout(), @@ -252,7 +243,7 @@ def _connect_sudo(spec): 'enable_lru': True, 'kwargs': { 'username': spec.become_user(), - 'password': optional_secret(spec.become_pass()), + 'password': spec.become_pass(), 'python_path': spec.python_path(), 'sudo_path': spec.become_exe(), 'connect_timeout': spec.timeout(), @@ -270,7 +261,7 @@ def _connect_doas(spec): 'enable_lru': True, 'kwargs': { 'username': spec.become_user(), - 'password': optional_secret(spec.become_pass()), + 'password': spec.become_pass(), 'python_path': spec.python_path(), 'doas_path': spec.become_exe(), 'connect_timeout': spec.timeout(), @@ -286,7 +277,7 @@ def _connect_mitogen_su(spec): 'method': 'su', 'kwargs': { 'username': spec.remote_user(), - 'password': optional_secret(spec.password()), + 'password': spec.password(), 'python_path': spec.python_path(), 'su_path': spec.become_exe(), 'connect_timeout': spec.timeout(), @@ -302,7 +293,7 @@ def _connect_mitogen_sudo(spec): 'method': 'sudo', 'kwargs': { 'username': spec.remote_user(), - 'password': optional_secret(spec.password()), + 'password': spec.password(), 'python_path': spec.python_path(), 'sudo_path': spec.become_exe(), 'connect_timeout': spec.timeout(), @@ -319,7 +310,7 @@ def _connect_mitogen_doas(spec): 'method': 'doas', 'kwargs': { 'username': spec.remote_user(), - 'password': optional_secret(spec.password()), + 'password': spec.password(), 'python_path': spec.python_path(), 'doas_path': spec.become_exe(), 'connect_timeout': spec.timeout(), diff --git a/ansible_mitogen/transport_config.py b/ansible_mitogen/transport_config.py index 2ad37c33..d46eb565 100644 --- a/ansible_mitogen/transport_config.py +++ b/ansible_mitogen/transport_config.py @@ -1,9 +1,39 @@ +# Copyright 2017, David Wilson +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# 3. Neither the name of the copyright holder nor the names of its contributors +# may be used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +from __future__ import absolute_import +from __future__ import unicode_literals """ -Mitogen extends Ansible's regular host configuration mechanism in several ways -that require quite a lot of care: +Mitogen extends Ansible's target configuration mechanism in several ways that +require some care: -* Some per-task configurables in Ansible like ansible_python_interpreter are +* Per-task configurables in Ansible like ansible_python_interpreter are connection-layer configurables in Mitogen. They must be extracted during each task execution to form the complete connection-layer configuration. @@ -13,28 +43,31 @@ that require quite a lot of care: * Mitogen allows connections to be delegated to other machines. Ansible has no internal framework for this, and so Mitogen must figure out a delegated - connection configuration all for itself. This means it cannot reuse much of - the Ansible machinery for building a connection configuration, as that - machinery is deeply spread and out hard-wired to expect Ansible's usual mode - of operation. - -For delegated connections, Ansible's PlayContext information is reused where -possible, but for proxy hops, configurations are built up using the HostVars -magic class to call VariableManager.get_vars() behind the scenes on our behalf. -Where Ansible has multiple sources of a configuration item, for example, -ansible_ssh_extra_args, Mitogen must (ideally perfectly) reproduce how Ansible -arrives at its value, without using mechanisms that are hard-wired or change -across Ansible versions. + connection configuration all on its own. It cannot reuse much of the Ansible + machinery for building a connection configuration, as that machinery is + deeply spread out and hard-wired to expect Ansible's usual mode of operation. + +For normal and delegate_to connections, Ansible's PlayContext is reused where +possible to maximize compatibility, but for proxy hops, configurations are +built up using the HostVars magic class to call VariableManager.get_vars() +behind the scenes on our behalf. Where Ansible has multiple sources of a +configuration item, for example, ansible_ssh_extra_args, Mitogen must (ideally +perfectly) reproduce how Ansible arrives at its value, without using mechanisms +that are hard-wired or change across Ansible versions. That is what this file is for. It exports two spec classes, one that takes all information from PlayContext, and another that takes (almost) all information from HostVars. """ +import abc import os import ansible.utils.shlex import ansible.constants as C +from ansible.module_utils.six import with_metaclass + + import mitogen.core @@ -47,11 +80,201 @@ def parse_python_path(s): return ansible.utils.shlex.shlex_split(s) -class PlayContextSpec: +def optional_secret(value): + """ + Wrap `value` in :class:`mitogen.core.Secret` if it is not :data:`None`, + otherwise return :data:`None`. + """ + if value is not None: + return mitogen.core.Secret(value) + + +class Spec(with_metaclass(abc.ABCMeta, object)): + """ + A source for variables that comprise a connection configuration. + """ + + @abc.abstractmethod + def transport(self): + """ + The name of the Ansible plug-in implementing the connection. + """ + + @abc.abstractmethod + def inventory_name(self): + """ + The name of the target being connected to as it appears in Ansible's + inventory. + """ + + @abc.abstractmethod + def remote_addr(self): + """ + The network address of the target, or for container and other special + targets, some other unique identifier. + """ + + @abc.abstractmethod + def remote_user(self): + """ + The username of the login account on the target. + """ + + @abc.abstractmethod + def password(self): + """ + The password of the login account on the target. + """ + + @abc.abstractmethod + def become(self): + """ + :data:`True` if privilege escalation should be active. + """ + + @abc.abstractmethod + def become_method(self): + """ + The name of the Ansible become method to use. + """ + + @abc.abstractmethod + def become_user(self): + """ + The username of the target account for become. + """ + + @abc.abstractmethod + def become_pass(self): + """ + The password of the target account for become. + """ + + @abc.abstractmethod + def port(self): + """ + The port of the login service on the target machine. + """ + + @abc.abstractmethod + def python_path(self): + """ + Path to the Python interpreter on the target machine. + """ + + @abc.abstractmethod + def private_key_file(self): + """ + Path to the SSH private key file to use to login. + """ + + @abc.abstractmethod + def ssh_executable(self): + """ + Path to the SSH executable. + """ + + @abc.abstractmethod + def timeout(self): + """ + The generic timeout for all connections. + """ + + @abc.abstractmethod + def ansible_ssh_timeout(self): + """ + The SSH-specific timeout for a connection. + """ + + @abc.abstractmethod + def ssh_args(self): + """ + The list of additional arguments that should be included in an SSH + invocation. + """ + + @abc.abstractmethod + def become_exe(self): + """ + The path to the executable implementing the become method on the remote + machine. + """ + + @abc.abstractmethod + def sudo_args(self): + """ + The list of additional arguments that should be included in a become + invocation. + """ + # TODO: split out into sudo_args/become_args. + + @abc.abstractmethod + def mitogen_via(self): + """ + The value of the mitogen_via= variable for this connection. Indicates + the connection should be established via an intermediary. + """ + + @abc.abstractmethod + def mitogen_kind(self): + """ + The type of container to use with the "setns" transport. + """ + + @abc.abstractmethod + def mitogen_docker_path(self): + """ + The path to the "docker" program for the 'docker' transport. + """ + + @abc.abstractmethod + def mitogen_kubectl_path(self): + """ + The path to the "kubectl" program for the 'docker' transport. + """ + + @abc.abstractmethod + def mitogen_lxc_path(self): + """ + The path to the "lxc" program for the 'lxd' transport. + """ + + @abc.abstractmethod + def mitogen_lxc_attach_path(self): + """ + The path to the "lxc-attach" program for the 'lxc' transport. + """ + + @abc.abstractmethod + def mitogen_lxc_info_path(self): + """ + The path to the "lxc-info" program for the 'lxc' transport. + """ + + @abc.abstractmethod + def mitogen_machinectl_path(self): + """ + The path to the "machinectl" program for the 'setns' transport. + """ + + @abc.abstractmethod + def mitogen_ssh_debug_level(self): + """ + The SSH debug level. + """ + + @abc.abstractmethod + def extra_args(self): + """ + Connection-specific arguments. + """ + + +class PlayContextSpec(Spec): """ - Return a dict representing all important connection configuration, allowing - the same functions to work regardless of whether configuration came from - play_context (direct connection) or host vars (mitogen_via=). + PlayContextSpec takes almost all its information as-is from Ansible's + PlayContext. It is used for normal connections and delegate_to connections, + and should always be accurate. """ def __init__(self, connection, play_context, transport, inventory_name): self._connection = connection @@ -81,10 +304,10 @@ class PlayContextSpec: return self._play_context.become_user def become_pass(self): - return self._play_context.become_pass + return optional_secret(self._play_context.become_pass) def password(self): - return self._play_context.password + return optional_secret(self._play_context.password) def port(self): return self._play_context.port @@ -107,7 +330,7 @@ class PlayContextSpec: return ( self._connection.get_task_var('ansible_timeout') or self._connection.get_task_var('ansible_ssh_timeout') or - self._play_context.timeout + self.timeout() ) def ssh_args(self): @@ -165,7 +388,23 @@ class PlayContextSpec: return self._connection.get_extra_args() -class MitogenViaSpec: +class MitogenViaSpec(Spec): + """ + MitogenViaSpec takes most of its information from the HostVars of the + running task. HostVars is a lightweight wrapper around VariableManager, so + it is better to say that VariableManager.get_vars() is the ultimate source + of MitogenViaSpec's information. + + Due to this, mitogen_via= hosts must have all their configuration + information represented as host and group variables. We cannot use any + per-task configuration, as all that data belongs to the real target host. + + Ansible uses all kinds of strange historical logic for calculating + variables, including making their precedence configurable. MitogenViaSpec + must ultimately reimplement all of that logic. It is likely that if you are + having a configruation problem with connection delegation, the answer to + your problem lies in the method implementations below! + """ def __init__(self, inventory_name, host_vars, become_method, become_user): self._inventory_name = inventory_name @@ -205,14 +444,14 @@ class MitogenViaSpec: return self._become_user def become_pass(self): - return ( + return optional_secret( # TODO: Might have to come from PlayContext. self._host_vars.get('ansible_become_password') or self._host_vars.get('ansible_become_pass') ) def password(self): - return ( + return optional_secret( # TODO: Might have to come from PlayContext. self._host_vars.get('ansible_ssh_pass') or self._host_vars.get('ansible_password') @@ -225,13 +464,11 @@ class MitogenViaSpec: ) def python_path(self): - s = parse_python_path( + return parse_python_path( self._host_vars.get('ansible_python_interpreter') # This variable has no default for remote hosts. For local hosts it # is sys.executable. ) - print('hi ho', self.inventory_name(), s) - return s def private_key_file(self): # TODO: must come from PlayContext too. From 23866084d78f2624287766c5f5b0dd5fd7929857 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 22 Jan 2019 01:42:15 +0000 Subject: [PATCH 17/22] issue #412: promote "mitogen_get_stack" to the main extension. This is to make it easier for users to diagnose their own problems. --- ansible_mitogen/plugins/action/__init__.py | 0 .../plugins/action/mitogen_get_stack.py | 53 +++++++++++++++++++ ansible_mitogen/strategy.py | 11 ++-- tests/ansible/lib/action/mitogen_get_stack.py | 23 +------- 4 files changed, 61 insertions(+), 26 deletions(-) create mode 100644 ansible_mitogen/plugins/action/__init__.py create mode 100644 ansible_mitogen/plugins/action/mitogen_get_stack.py mode change 100644 => 120000 tests/ansible/lib/action/mitogen_get_stack.py diff --git a/ansible_mitogen/plugins/action/__init__.py b/ansible_mitogen/plugins/action/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/ansible_mitogen/plugins/action/mitogen_get_stack.py b/ansible_mitogen/plugins/action/mitogen_get_stack.py new file mode 100644 index 00000000..8e0b310c --- /dev/null +++ b/ansible_mitogen/plugins/action/mitogen_get_stack.py @@ -0,0 +1,53 @@ +# Copyright 2017, David Wilson +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# 3. Neither the name of the copyright holder nor the names of its contributors +# may be used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +from __future__ import absolute_import +from __future__ import unicode_literals + +""" +Fetch the connection configuration stack that would be used to connect to a +target, without actually connecting to it. +""" + +import ansible_mitogen.connection + +from ansible.plugins.action import ActionBase + + +class ActionModule(ActionBase): + def run(self, tmp=None, task_vars=None): + if not isinstance(self._connection, + ansible_mitogen.connection.Connection): + return { + 'skipped': True, + } + + return { + 'changed': True, + 'result': self._connection._build_stack(), + } diff --git a/ansible_mitogen/strategy.py b/ansible_mitogen/strategy.py index dd17f177..e82e6d58 100644 --- a/ansible_mitogen/strategy.py +++ b/ansible_mitogen/strategy.py @@ -174,15 +174,18 @@ class StrategyMixin(object): ansible_mitogen.loaders.action_loader.get = action_loader__get ansible_mitogen.loaders.connection_loader.get = connection_loader__get - def _add_connection_plugin_path(self): + def _add_plugin_paths(self): """ - Add the mitogen connection plug-in directory to the ModuleLoader path, - avoiding the need for manual configuration. + Add the Mitogen plug-in directories to the ModuleLoader path, avoiding + the need for manual configuration. """ base_dir = os.path.join(os.path.dirname(__file__), 'plugins') ansible_mitogen.loaders.connection_loader.add_directory( os.path.join(base_dir, 'connection') ) + ansible_mitogen.loaders.action_loader.add_directory( + os.path.join(base_dir, 'action') + ) def run(self, iterator, play_context, result=0): """ @@ -190,7 +193,7 @@ class StrategyMixin(object): the strategy's real run() method. """ ansible_mitogen.process.MuxProcess.start() - self._add_connection_plugin_path() + self._add_plugin_paths() self._install_wrappers() try: return super(StrategyMixin, self).run(iterator, play_context) diff --git a/tests/ansible/lib/action/mitogen_get_stack.py b/tests/ansible/lib/action/mitogen_get_stack.py deleted file mode 100644 index f1b87f35..00000000 --- a/tests/ansible/lib/action/mitogen_get_stack.py +++ /dev/null @@ -1,22 +0,0 @@ -""" -Fetch the connection configuration stack that would be used to connect to a -target, without actually connecting to it. -""" - -import ansible_mitogen.connection - -from ansible.plugins.action import ActionBase - - -class ActionModule(ActionBase): - def run(self, tmp=None, task_vars=None): - if not isinstance(self._connection, - ansible_mitogen.connection.Connection): - return { - 'skipped': True, - } - - return { - 'changed': True, - 'result': self._connection._build_stack(), - } diff --git a/tests/ansible/lib/action/mitogen_get_stack.py b/tests/ansible/lib/action/mitogen_get_stack.py new file mode 120000 index 00000000..f055f341 --- /dev/null +++ b/tests/ansible/lib/action/mitogen_get_stack.py @@ -0,0 +1 @@ +../../../../ansible_mitogen/plugins/action/mitogen_get_stack.py \ No newline at end of file From e767de3f15e8838fa5d717a386129607e88c2e8d Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 22 Jan 2019 02:26:32 +0000 Subject: [PATCH 18/22] issue #412: force-verbose output for mitogen_get_stack. --- ansible_mitogen/plugins/action/mitogen_get_stack.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ansible_mitogen/plugins/action/mitogen_get_stack.py b/ansible_mitogen/plugins/action/mitogen_get_stack.py index 8e0b310c..ed7520cf 100644 --- a/ansible_mitogen/plugins/action/mitogen_get_stack.py +++ b/ansible_mitogen/plugins/action/mitogen_get_stack.py @@ -50,4 +50,5 @@ class ActionModule(ActionBase): return { 'changed': True, 'result': self._connection._build_stack(), + '_ansible_verbose_always': True, } From 15182bb814dfcd8958f824d8d4403e0438afc9df Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 22 Jan 2019 02:26:32 +0000 Subject: [PATCH 19/22] issue #412: pad out debugging docs, add get_stack to changelog. --- docs/ansible.rst | 148 +++++++++++++++++++++++++++++++++++++++++++++ docs/changelog.rst | 11 ++++ 2 files changed, 159 insertions(+) diff --git a/docs/ansible.rst b/docs/ansible.rst index c8b0d12f..205e5e3c 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -921,6 +921,154 @@ logging is necessary. File-based logging can be enabled by setting enabled, one file per context will be created on the local machine and every target machine, as ``/tmp/mitogen..log``. + +Common Problems +~~~~~~~~~~~~~~~ + +The most common bug reports fall into the following categories, so it is worth +checking whether you can categorize a problem using the tools provided before +reporting it: + +**Missed/Incorrect Configuration Variables** + In some cases Ansible may support a configuration variable that Mitogen + does not yet support, or Mitogen supports, but the support is broken. For + example, Mitogen may pick the wrong username or SSH parameters. + + To detect this, use the special ``mitogen_get_stack`` action described + below to verify all the configuration variables Mitogen has chosen for the + connection make sense. + +**Process Environment Differences** + Mitogen's process model differs significantly to Ansible's in certain + places. In the past, bugs have been reported because Ansible plug-ins + modify an environment variable after Mitogen processes are started + +**Variable Expansion Differences** + To avoid many classes of bugs, Mitogen avoids shell wherever possible. + Ansible however is traditionally built on shell, and it is often difficult + to tell just how many times a configuration parameter will pass through + shell expansion and quoting, and in what context before it is used. + + Due to this, in some circumstances Mitogen may parse some expanded + variables differently, for example, in the wrong user account. Careful + review of ``-vvv`` and ``mitogen_ssh_debug_level`` logs can reveal this. + For example in the past, Mitogen used a different method of expanding + ``~/.ssh/id_rsa``, causing authentication to fail when ``ansible-playbook`` + was run via ``sudo -E``. + +**External Tool Integration Differences** + Mitogen reimplements any aspect of Ansible that involves integrating with + SSH, sudo, Docker, or related tools. For this reason, sometimes its support + for those tools doffers or is less mature than in Ansible. + + In the past Mitogen has had bug reports due to failing to recognize a + particular variation of a login or password prompt on an exotic or + non-English operating system, or confusing a login banner for a password + prompt. Careful review of ``-vvv`` logs help identify these cases, as + Mitogen logs all strings it receives during connection, and how it + interprets them. + + +.. _mitogen-get-stack: + +The `mitogen_get_stack` Action +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When a Mitogen strategy is loaded, a special ``mitogen_get_stack`` action is +available that returns a concise description of the connection configuration as +extracted from Ansible and passed to the core library. Using it, you can learn +whether a problem lies in the Ansible extension or deeper in library code. + +The action may be used in a playbook as ``mitogen_get_stack:`` just like a +regular module, or directly from the command-line:: + + $ ANSIBLE_STRATEGY=mitogen_linear ansible -m mitogen_get_stack -b -k k3 + SSH password: + k3 | SUCCESS => { + "changed": true, + "result": [ + { + "kwargs": { + "check_host_keys": "enforce", + "connect_timeout": 10, + "hostname": "k3", + "identities_only": false, + "identity_file": null, + "password": "mysecretpassword", + "port": null, + "python_path": null, + "ssh_args": [ + "-C", + "-o", + "ControlMaster=auto", + "-o", + "ControlPersist=60s" + ], + "ssh_debug_level": null, + "ssh_path": "ssh", + "username": null + }, + "method": "ssh" + }, + { + "enable_lru": true, + "kwargs": { + "connect_timeout": 10, + "password": null, + "python_path": null, + "sudo_args": [ + "-H", + "-S", + "-n" + ], + "sudo_path": null, + "username": "root" + }, + "method": "sudo" + } + ] + } + +Each object in the list represents a single 'hop' in the connection, from +nearest to furthest. Unlike in Ansible, the core library treats ``become`` +steps and SSH steps identically, so they are represented distinctly in the +output. + +The presence of ``null`` means no explicit value was extracted from Ansible, +and either the Mitogen library or SSH will choose a value for the parameter. In +the example above, Mitogen will choose ``/usr/bin/python`` for ``python_path``, +and SSH will choose ``22`` for ``port``, or whatever ``Port`` it parses from +``~/.ssh/config``. Note the presence of ``null`` may indicate the extension +failed to extract the correct value. + +When using ``mitogen_get_stack`` to diagnose a problem, pay special attention +to ensuring the invocation exactly matches the problematic task. For example, +if the failing task has ``delegate_to:`` or ``become:`` enabled, the +``mitogen_get_stack`` invocation must include those statements in order for the +output to be accurate. + +If a playbook cannot start at all, you may need to temporarily use +``gather_facts: no`` to allow the first task to proceed. This action does not +create connections, so if it is the first task, it is still possible to review +its output. + + +The `mitogen_ssh_debug_level` Variable +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Mitogen has support for capturing SSH diagnostic logs, and integrating them +into the regular debug log output produced when ``-vvv`` is active. This +provides a single audit trail of every component active during SSH +authentication. + +Particularly for authentication failures, setting this variable to 3, in +combination with ``-vvv``, allows review of every parameter passed to SSH, and +review of every action SSH attempted during authentication. + +For example, this method can be used to ascertain whether SSH attempted agent +authentication, or what private key files it was able to access and which it tried. + + .. _diagnosing-hangs: Diagnosing Hangs diff --git a/docs/changelog.rst b/docs/changelog.rst index 3c7350b5..95123a69 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -154,6 +154,11 @@ Enhancements ``mitogen_host_pinned`` strategy wraps the ``host_pinned`` strategy introduced in Ansible 2.7. +* `#412 `_: to simplify diagnosing + issues with connection configuration, Mitogen ships with a + ``mitogen_get_stack`` action that is automatically added to the action + plug-in path. See :ref:`mitogen-get-stack` for more information. + * `#415 `_: the interface employed for in-process queues was changed from `kqueue `_ / @@ -164,6 +169,7 @@ Enhancements a runtime improvement in many-host runs. + Fixes ^^^^^ @@ -210,6 +216,11 @@ Fixes * `#410 `_: the sudo method supports the SELinux ``--type`` and ``--role`` options. +* `#412 `_: connection delegation and + ``delegate_to:`` handling suffered a major regression in 0.2.3. The 0.2.2 + behaviour has been restored, and further work has been made to improve the + compatibility of connection delegation's configuration building methods. + * `#420 `_: if a :class:`Connection` was constructed in the Ansible top-level process, for example while executing ``meta: reset_connection``, resources could become undesirably shared in From d6945443b7a353f1e679c21ecf0c9cb1fe32ec2a Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 22 Jan 2019 02:52:42 +0000 Subject: [PATCH 20/22] tests: add exact test for issue 251; closes #251. --- .../integration/connection_delegation/all.yml | 1 + .../connection_delegation/local_action.yml | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 tests/ansible/integration/connection_delegation/local_action.yml diff --git a/tests/ansible/integration/connection_delegation/all.yml b/tests/ansible/integration/connection_delegation/all.yml index 743ce157..8d5ffe03 100644 --- a/tests/ansible/integration/connection_delegation/all.yml +++ b/tests/ansible/integration/connection_delegation/all.yml @@ -1,4 +1,5 @@ - import_playbook: delegate_to_template.yml +- import_playbook: local_action.yml - import_playbook: osa_container_standalone.yml - import_playbook: osa_delegate_to_self.yml - import_playbook: stack_construction.yml diff --git a/tests/ansible/integration/connection_delegation/local_action.yml b/tests/ansible/integration/connection_delegation/local_action.yml new file mode 100644 index 00000000..d166c0d9 --- /dev/null +++ b/tests/ansible/integration/connection_delegation/local_action.yml @@ -0,0 +1,34 @@ + +# issue #251: local_action with mitogen_via= builds wrong stack. + +- hosts: cd-newuser-normal-normal + tasks: + - meta: end_play + when: not is_mitogen + + - local_action: mitogen_get_stack + become: true + register: out + + - assert_equal: + left: out.result + right: [ + { + 'kwargs': { + 'python_path': null + }, + 'method': 'local', + }, + { + 'enable_lru': true, + 'kwargs': { + 'connect_timeout': 10, + 'python_path': null, + 'password': null, + 'username': 'root', + 'sudo_path': null, + 'sudo_args': ['-H', '-S', '-n'], + }, + 'method': 'sudo', + } + ] From b8e1adf4fae5795126f4ed0698e8e89ec807d21b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 22 Jan 2019 02:54:22 +0000 Subject: [PATCH 21/22] issue #251: readd to Changelog. --- docs/changelog.rst | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 95123a69..087f5e8e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -173,6 +173,12 @@ Enhancements Fixes ^^^^^ +* `#251 `_, + `#412 `_: connection delegation and + ``delegate_to:`` handling suffered a major regression in 0.2.3. The 0.2.2 + behaviour has been restored, and further work has been made to improve the + compatibility of connection delegation's configuration building methods. + * `#323 `_, `#333 `_: work around a Windows Subsystem for Linux bug that caused tracebacks to appear during shutdown. @@ -216,11 +222,6 @@ Fixes * `#410 `_: the sudo method supports the SELinux ``--type`` and ``--role`` options. -* `#412 `_: connection delegation and - ``delegate_to:`` handling suffered a major regression in 0.2.3. The 0.2.2 - behaviour has been restored, and further work has been made to improve the - compatibility of connection delegation's configuration building methods. - * `#420 `_: if a :class:`Connection` was constructed in the Ansible top-level process, for example while executing ``meta: reset_connection``, resources could become undesirably shared in @@ -407,6 +408,7 @@ Thanks! Mitogen would not be possible without the support of users. A huge thanks for bug reports, testing, features and fixes in this release contributed by `Andreas Krüger `_, +`Anton Stroganov `_, `Berend De Schouwer `_, `Brian Candler `_, `dsgnr `_, From c4d0046164b662c692d80c62175eb56fc8135337 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 22 Jan 2019 02:55:35 +0000 Subject: [PATCH 22/22] issue #404: add to Changelog. --- docs/changelog.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 087f5e8e..3a6acd93 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -174,6 +174,7 @@ Fixes ^^^^^ * `#251 `_, + `#404 `_, `#412 `_: connection delegation and ``delegate_to:`` handling suffered a major regression in 0.2.3. The 0.2.2 behaviour has been restored, and further work has been made to improve the @@ -426,10 +427,12 @@ bug reports, testing, features and fixes in this release contributed by `Mohammed Naser `_, `Peter V. Saveliev `_, `Stéphane `_, +`Tom Parker-Shemilt `_, +`Younès HAFRI `_, +`@myssa91 `_, `@syntonym `_, -`@whky `_, -`@yodatak `_, and -`Younès HAFRI `_. +`@whky `_, and +`@yodatak `_. v0.2.3 (2018-10-23)