From c148c869e63054af28ef75eaef5a5f308e6efab1 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 1 Nov 2018 20:04:18 +0000 Subject: [PATCH 1/5] issue #76, #370: add disconnect cleanup test --- ansible_mitogen/services.py | 17 +++++++ .../integration/context_service/all.yml | 1 + .../context_service/disconnect_cleanup.yml | 46 +++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 tests/ansible/integration/context_service/disconnect_cleanup.yml diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index 0ce87e09..a5f31b1c 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -268,6 +268,23 @@ class ContextService(mitogen.service.Service): finally: self._lock.release() + @mitogen.service.expose(mitogen.service.AllowParents()) + def dump(self): + """ + For testing, return a list of dicts describing every currently + connected context. + """ + return [ + { + 'context_name': context.name, + 'via': getattr(self._via_by_context.get(context), + 'name', None), + 'refs': self._refs_by_context.get(context), + } + for context, key in sorted(self._key_by_context.items(), + key=lambda c_k: c_k[0].context_id) + ] + @mitogen.service.expose(mitogen.service.AllowParents()) def shutdown_all(self): """ diff --git a/tests/ansible/integration/context_service/all.yml b/tests/ansible/integration/context_service/all.yml index e70199f8..c10d67cb 100644 --- a/tests/ansible/integration/context_service/all.yml +++ b/tests/ansible/integration/context_service/all.yml @@ -1,2 +1,3 @@ +- import_playbook: disconnect_cleanup.yml - import_playbook: lru_one_target.yml - import_playbook: reconnection.yml diff --git a/tests/ansible/integration/context_service/disconnect_cleanup.yml b/tests/ansible/integration/context_service/disconnect_cleanup.yml new file mode 100644 index 00000000..b657a0dc --- /dev/null +++ b/tests/ansible/integration/context_service/disconnect_cleanup.yml @@ -0,0 +1,46 @@ +# issue #76, #370: ensure context state is forgotten on disconnect, including +# state of dependent contexts (e.g. sudo, connection delegation, ..). + +- name: integration/context_service/disconnect_cleanup.yml + hosts: test-targets + any_errors_fatal: true + tasks: + - meta: end_play + when: not is_mitogen + + # Start with a clean slate. + - mitogen_shutdown_all: + + # Connect a few users. + - shell: "true" + become: true + become_user: "mitogen__user{{item}}" + with_items: [1, 2, 3] + + # Verify current state. + - mitogen_action_script: + script: | + self._connection._connect() + result['dump'] = self._connection.parent.call_service( + service_name='ansible_mitogen.services.ContextService', + method_name='dump' + ) + register: out + + - assert: + that: out.dump|length == 4 # ssh account + 3 sudo accounts + + - meta: reset_connection + + # Verify current state. + - mitogen_action_script: + script: | + self._connection._connect() + result['dump'] = self._connection.parent.call_service( + service_name='ansible_mitogen.services.ContextService', + method_name='dump' + ) + register: out + + - assert: + that: out.dump|length == 1 # just the ssh account From bac28bc5cab23927d10eb8d6fbbde1ac117899bd Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 1 Nov 2018 20:06:09 +0000 Subject: [PATCH 2/5] issue #76, #370: add fix for disconnect cleanup test Simply listen to RouteMonitor's Context "disconnect" and forget contexts according to RouteMonitor's rules, rather than duplicate them (and screw it up). --- ansible_mitogen/services.py | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index a5f31b1c..c97d71bb 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -297,23 +297,19 @@ class ContextService(mitogen.service.Service): finally: self._lock.release() - def _on_stream_disconnect(self, stream): + def _on_context_disconnect(self, context): """ - Respond to Stream disconnection by deleting any record of contexts - reached via that stream. This method runs in the Broker thread and must - not to block. + Respond to Context disconnect event by deleting any record of the no + longer reachable context. This method runs in the Broker thread and + must not to block. """ # TODO: there is a race between creation of a context and disconnection # of its related stream. An error reply should be sent to any message # in _latches_by_key below. self._lock.acquire() try: - routes = self.router.route_monitor.get_routes(stream) - for context in list(self._key_by_context): - if context.context_id in routes: - LOG.info('Dropping %r due to disconnect of %r', - context, stream) - self._forget_context_unlocked(context) + LOG.info('Forgetting %r due to stream disconnect', context) + self._forget_context_unlocked(context) finally: self._lock.release() @@ -379,13 +375,10 @@ class ContextService(mitogen.service.Service): context = method(via=via, unidirectional=True, **spec['kwargs']) if via and spec.get('enable_lru'): self._update_lru(context, spec, via) - else: - # For directly connected contexts, listen to the associated - # Stream's disconnect event and use it to invalidate dependent - # Contexts. - stream = self.router.stream_by_id(context.context_id) - mitogen.core.listen(stream, 'disconnect', - lambda: self._on_stream_disconnect(stream)) + + # Forget the context when its disconnect event fires. + mitogen.core.listen(context, 'disconnect', + lambda: self._on_context_disconnect(context)) self._send_module_forwards(context) init_child_result = context.call( From 5be9a55bf4ba4f51e20ef22d9d04e6bd425b3920 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 1 Nov 2018 20:15:16 +0000 Subject: [PATCH 3/5] core: allow Context to be pickled by non-Mitogen pickler. --- mitogen/core.py | 18 ++++++++++-------- tests/serialization_test.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index f2dece48..faa2d516 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -553,7 +553,7 @@ class Message(object): assert isinstance(self.data, BytesType) def _unpickle_context(self, context_id, name): - return _unpickle_context(self.router, context_id, name) + return _unpickle_context(context_id, name, router=self.router) def _unpickle_sender(self, context_id, dst_handle): return _unpickle_sender(self.router, context_id, dst_handle) @@ -1498,14 +1498,16 @@ class Context(object): return 'Context(%s, %r)' % (self.context_id, self.name) -def _unpickle_context(router, context_id, name): - if not (isinstance(router, Router) and - isinstance(context_id, (int, long)) and context_id >= 0 and ( - (name is None) or - (isinstance(name, UnicodeType) and len(name) < 100)) - ): +def _unpickle_context(context_id, name, router=None): + if not (isinstance(context_id, (int, long)) and context_id >= 0 and ( + (name is None) or + (isinstance(name, UnicodeType) and len(name) < 100)) + ): raise TypeError('cannot unpickle Context: bad input') - return router.context_by_id(context_id, name=name) + + if isinstance(router, Router): + return router.context_by_id(context_id, name=name) + return Context(None, context_id, name) # For plain Jane pickle. class Poller(object): diff --git a/tests/serialization_test.py b/tests/serialization_test.py index 3b56e10a..f108ff37 100644 --- a/tests/serialization_test.py +++ b/tests/serialization_test.py @@ -6,11 +6,14 @@ except ImportError: from StringIO import StringIO as StringIO from StringIO import StringIO as BytesIO +import pickle import unittest2 import mitogen.core from mitogen.core import b +import testlib + def roundtrip(v): msg = mitogen.core.Message.pickled(v) @@ -33,5 +36,30 @@ class BlobTest(unittest2.TestCase): self.assertEquals(b(''), roundtrip(v)) +class ContextTest(testlib.RouterMixin, unittest2.TestCase): + klass = mitogen.core.Context + + # Ensure Context can be round-tripped by regular pickle in addition to + # Mitogen's hacked pickle. Users may try to call pickle on a Context in + # strange circumstances, and it's often used to glue pieces of an app + # together (e.g. Ansible). + + def test_mitogen_roundtrip(self): + c = self.router.fork() + r = mitogen.core.Receiver(self.router) + r.to_sender().send(c) + c2 = r.get().unpickle() + self.assertEquals(None, c2.router) + self.assertEquals(c.context_id, c2.context_id) + self.assertEquals(c.name, c2.name) + + def test_vanilla_roundtrip(self): + c = self.router.fork() + c2 = pickle.loads(pickle.dumps(c)) + self.assertEquals(None, c2.router) + self.assertEquals(c.context_id, c2.context_id) + self.assertEquals(c.name, c2.name) + + if __name__ == '__main__': unittest2.main() From f3f36d6244521a5f5a73207921cb56f3919fa59b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 1 Nov 2018 20:20:50 +0000 Subject: [PATCH 4/5] docs: add connection: "smart" to known issues. --- docs/changelog.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 33c4489a..4fa850bd 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -499,6 +499,12 @@ Mitogen for Ansible **Known Issues** +* On OS X when a SSH password is specified and the default connection type of + ``smart`` is used, Ansible may select the Paramiko plug-in rather than + Mitogen. If you specify a password on OS X, ensure ``connection: ssh`` + appears in your playbook, ``ansible.cfg``, or as ``-c ssh`` on the + command-line. + * The ``raw`` action executes as a regular Mitogen connection, which requires Python on the target, precluding its use for installing Python. This will be addressed in a future 0.2 release. For now, simply mix Mitogen and vanilla From 7fd4549ad16f449bbea9dee72581b3dda9d87067 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 1 Nov 2018 20:25:19 +0000 Subject: [PATCH 5/5] issue #370: update Changelog. --- docs/changelog.rst | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4fa850bd..b7c41808 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -52,6 +52,10 @@ Fixes was invoked using sudo without appropriate flags to cause the ``HOME`` environment variable to be reset to match the target account. +* `#370 `_: the Ansible + `reboot `_ + module is supported. + * `#373 `_: the LXC and LXD methods now print a useful hint when Python fails to start, as no useful error is normally logged to the console by these tools. @@ -110,8 +114,9 @@ bug reports, features and fixes in this release contributed by `Brian Candler `_, `Guy Knights `_, `Jiří Vávra `_, -`Jonathan Rosser `_, and -`Mehdi `_. +`Jonathan Rosser `_, +`Mehdi `_, and +`Mohammed Naser `_. v0.2.3 (2018-10-23)