Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incompatible with regpg Ansible plugin #554

Closed
fanf2 opened this Issue Feb 27, 2019 · 7 comments

Comments

Projects
None yet
2 participants
@fanf2
Copy link

commented Feb 27, 2019

I am using ansible 2.7.5 from Debian stretch-backports. Host and target are Debian stretch.

I am using the regpg gpg_d action plugin, which you can get from https://dotat.at/prog/regpg/. It's basically a clone-and-hack of Ansible's copy or template modules. Details of a minimal reproduction setup below. It works without Mitogen, but I might be mis-using the API in a way that is revealed by Mitogen...

I have tried Mitogen 0.2.5 (from the git tag) and latest Mitogen master. The error output is below; it occurs (in my code) around https://dotat.at/cgi/git/regpg.git/blob/HEAD:/ansible/action.py#l100

[pid 3135] 13:27:29.905395 D ansible_mitogen.mixins: _make_tmp_path(remote_user=None)
[pid 3135] 13:27:29.906262 D mitogen: unix.connect(path='/tmp/mitogen_unix_VpUUlK.sock')
[pid 3135] 13:27:29.907130 D mitogen: unix.connect(): local ID is 1003, remote is 0
[pid 3031] 13:27:29.907124 D mitogen: mitogen.unix.Listener('/tmp/mitogen_unix_VpUUlK.sock'): accepted mitogen.core.Stream('unix_client.3135')
[pid 3135] 13:27:29.908740 D ansible_mitogen.connection: Temporary directory: u'/home/fanf2/.ansible/tmp/ansible_mitogen_action_0cfadb91dc0a2803'
[pid 3135] 13:27:29.908866 D mitogen: CallChain(Context(2, u'local.3072')).call_no_reply(): posix.mkdir(u'/home/fanf2/.ansible/tmp/ansible_mitogen_action_0cfadb91dc0a2803')
[pid 3135] 13:27:29.909447 D ansible_mitogen.mixins: _remote_expand_user(u'/tmp/foo', sudoable=True)
[pid 3031] 13:27:29.913255 D mitogen.ctx.local.3072: mitogen: _dispatch_one(('grey.csi.cam.ac.uk-3135-7f21bd444700-582e022b1afe0', u'posix', None, u'mkdir', (u'/home/fanf2/.ansible/tmp/ansible_mitogen_action_0cfadb91dc0a2803',), Kwargs({})))
[pid 3031] 13:27:29.913637 D mitogen.ctx.local.3072: mitogen: _dispatch_calls: Message(2, 1003, 0, 101, 0, '\x80\x02(U2grey.csi.cam.ac.uk-3135-7f21bd444700-582e022b'..184) -> None
[pid 3135] 13:27:30.046436 D ansible_mitogen.mixins: _make_tmp_path(remote_user=u'fanf2')
[pid 3135] 13:27:30.047673 D mitogen: CallChain(Context(2, u'local.3072')).call_no_reply(): ansible_mitogen.target.prune_tree(u'/home/fanf2/.ansible/tmp/ansible_mitogen_action_0cfadb91dc0a2803')
[pid 3135] 13:27:30.048189 D mitogen: CallChain(Context(2, u'local.3072')).call_no_reply(): mitogen.core.Dispatcher.forget_chain('grey.csi.cam.ac.uk-3135-7f21bd444700-582e022b1afe0')
[pid 3031] 13:27:30.049134 D ansible_mitogen.services: ContextService().put(Context(2, u'local.3072'))
[pid 3031] 13:27:30.049511 D mitogen.ctx.local.3072: mitogen: _dispatch_one((None, u'ansible_mitogen.target', None, u'prune_tree', (u'/home/fanf2/.ansible/tmp/ansible_mitogen_action_0cfadb91dc0a2803',), Kwargs({})))
[pid 3031] 13:27:30.049903 D mitogen.ctx.local.3072: mitogen: _dispatch_calls: Message(2, 1003, 0, 101, 0, '\x80\x02(NX\x16\x00\x00\x00ansible_mitogen.targetNX\n\x00\x00\x00prune_treeX@\x00'..153) -> None
[pid 3031] 13:27:30.050023 D mitogen.ctx.local.3072: mitogen: _dispatch_one((None, u'mitogen.core', u'Dispatcher', u'forget_chain', ('grey.csi.cam.ac.uk-3135-7f21bd444700-582e022b1afe0',), Kwargs({})))
[pid 3031] 13:27:30.050123 D mitogen.ctx.local.3072: mitogen: _dispatch_calls: Message(2, 1003, 0, 101, 0, '\x80\x02(NX\x0c\x00\x00\x00mitogen.coreX\n\x00\x00\x00Dispatcherq\x01X\x0c\x00\x00\x00forget_'..144) -> None
[pid 3135] 13:27:30.049887 D mitogen: mitogen.core.Stream('unix_listener.3031').on_disconnect()
[pid 3031] 13:27:30.050276 D mitogen: mitogen.core.Stream('unix_client.3135').on_disconnect()
[pid 3135] 13:27:30.050302 D mitogen: Waker(Broker(0x7f21b5ca3cd0) rfd=10, wfd=12).on_disconnect()
[pid 3135] 13:27:30.050618 D mitogen: Router(Broker(0x7f21b5ca3cd0)): stats: 0 module requests in 0 ms, 0 sent (0 ms minify time), 0 negative responses. Sent 0.0 kb total, 0.0 kb avg.
The full traceback is:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/ansible/executor/task_executor.py", line 140, in run
    res = self._execute()
  File "/usr/lib/python2.7/dist-packages/ansible/executor/task_executor.py", line 612, in _execute
    result = self._handler.run(task_vars=variables)
  File "/home/fanf2/gnu/src/mitogen/ansible_mitogen/mixins.py", line 115, in run
    return super(ActionModuleMixin, self).run(tmp, task_vars)
  File "/home/fanf2/work/git/t/testing/plugins/action/gpg_d.py", line 107, in run
    tmp = self._make_tmp_path(remote_user)
  File "/home/fanf2/gnu/src/mitogen/ansible_mitogen/mixins.py", line 182, in _make_tmp_path
    return self._connection._make_tmp_path()
  File "/home/fanf2/gnu/src/mitogen/ansible_mitogen/connection.py", line 715, in _make_tmp_path
    assert getattr(self._shell, 'tmpdir', None) is None
AssertionError

I ran regpg init ansible; echo foo | regpg encrypt foo.asc to install the Ansible plugins, set up the regpg keyring, and make a file to be decrypted. I added the Mitogen lines to ansible.cfg. No other setup except for the playbook, which is:

---
- hosts: localhost
  tasks:
    - gpg_d:
         src: foo.asc
         dest: /tmp/foo

ansible-config dump --only-changed:

DEFAULT_ACTION_PLUGIN_PATH(/home/fanf2/work/git/t/testing/ansible.cfg) = [u'/home/fanf2/work/git/t/testing/plugins/action', u'/home/fanf2/work/git/t/testing/plugins/action']
DEFAULT_FILTER_PLUGIN_PATH(/home/fanf2/work/git/t/testing/ansible.cfg) = [u'/home/fanf2/work/git/t/testing/plugins/filter', u'/home/fanf2/work/git/t/testing/plugins/filter']
DEFAULT_STRATEGY(/home/fanf2/work/git/t/testing/ansible.cfg) = mitogen_linear
DEFAULT_STRATEGY_PLUGIN_PATH(/home/fanf2/work/git/t/testing/ansible.cfg) = [u'/home/fanf2/gnu/src/mitogen/ansible_mitogen/plugins/strategy']
@dw

This comment has been minimized.

Copy link
Owner

commented Feb 27, 2019

It's due to the double call to _make_tmp_path, the extension assumes there can only be one temp directory.

I think the correct behaviour will be to return the existing directory in the second call: Ansible internally only tracks one directory, so having multiple would cause others not to be deleted.

This is a real easy fix, I just need to double check the logic above :)

Thanks for reporting this!

@fanf2

This comment has been minimized.

Copy link
Author

commented Feb 27, 2019

Thanks!

My understanding of Ansible's internals is rather murky, but the impression I got from all the tmp juggling is that my plugin would be passed a tmp arg if there already was one. (But all of that logic seems rather ridiculous for not being part of the shared plugin code...)

@dw

This comment has been minimized.

Copy link
Owner

commented Feb 27, 2019

The default _make_tmp_path keeps a note of the last generated directory name in the 'shell' plugin, and some magical handling is attached to that particular name. This stuff continues to change rapidly across versions (2.7 featured more work to it IIRC). Regardless, your plugin worked previously and it needs to work again, as there will be more unreported cases just like this.

Looks like returning the same path twice is definitely incorrect, if it wants to keep pipelining temp directory removal it needs to try harder, or disable the pipelining behaviour for subsequent make_tmp_path calls

@fanf2

This comment has been minimized.

Copy link
Author

commented Feb 27, 2019

Ah, I see that the copy and template actions have changed a fair bit in this area since I last looked. I've tried gpg_d with Mitogen with Ansible 2.3 and 2.4 and it works OK. 2.5 is the first version that explodes.

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

@dw

This comment has been minimized.

Copy link
Owner

commented Feb 28, 2019

A first pass at this naturally this broke something :) This may take over the weekend, I won't be around so much for the coming 2 days

@fanf2

This comment has been minimized.

Copy link
Author

commented Feb 28, 2019

OK, thanks for looking in to it :-) I have some acceptable workarounds for the time being.

dw added a commit that referenced this issue Mar 6, 2019

dw added a commit that referenced this issue Mar 6, 2019

issue #554: don't rely on tmp_path autoremoval in test.
Ansible doesn't do this, so we shouldn't either.

dw added a commit that referenced this issue Mar 6, 2019

dw added a commit that referenced this issue Mar 6, 2019

dw added a commit that referenced this issue Mar 6, 2019

dw added a commit that referenced this issue Mar 6, 2019

dw added a commit that referenced this issue Mar 6, 2019

dw added a commit that referenced this issue Mar 6, 2019

Merge remote-tracking branch 'origin/dmw'
* origin/dmw:
  issue #554: mitogen_action_script fix
  issue #554: fix Ansible 2.4 compatibility
  issue #554: don't rely on tmp_path autoremoval in test.
  issue #554: track and remove multiple make_tmp_path() calls.
  docs: update Changelog.
  docs: drastically simplify install/changelog.
  issue #552: include process identity in log messages.
  issue #550: update Changelog.
@dw

This comment has been minimized.

Copy link
Owner

commented Mar 6, 2019

This is on master and will make it into the next release -- coming real soon! Thanks again for reporting this, and of course, if you're still having issues please reopen

@dw dw closed this Mar 6, 2019

dw added a commit that referenced this issue Mar 6, 2019

Merge remote-tracking branch 'origin/026' into stable
* origin/026:
  docs: update Changelog for release.
  Bump version for release.
  issue #555: ansible: workaround ancient reload(sys) hack.
  issue #554: mitogen_action_script fix
  issue #554: fix Ansible 2.4 compatibility
  issue #554: don't rely on tmp_path autoremoval in test.
  issue #554: track and remove multiple make_tmp_path() calls.
  docs: update Changelog.
  docs: drastically simplify install/changelog.
  issue #552: include process identity in log messages.
  issue #550: update Changelog.
  issue #550: parent: add explanatory comment.
  issue #550: fix up TTY ioctls on WSL 2016 Anniversary Update
  docs: update Changelog.
  service: make service list optional.
  docs: update Changelog; closes #548.
  issue #548: always treat transport=smart as 'ssh' for mitogen_via=.
  docs: better intro paragraph.
  .ci: copy private key file to tempdir.
  os_fork: more doc tweaks
  os_fork: more doc tweaks
  os_fork: yet more doc tidyup
  os_fork: more doc tweaks
  os_fork: clean up docs
  .ci: import soak scripts.
  .ci: allow containers for different jobs to run simultaneously
  os_fork: python 3 fixes and tests.
  issue #535: activate Corker on 2.4 in master too.
  issue #535: update Changelog.
  issue #535: wire mitogen.os_fork into Broker and Pool.
  issue #535: parent: add create_socketpair(size=..) parameter.
  issue #535: introduce mitogen.os_fork module and Corker class.
  issue #535: docs: update Changelog
  issue #535: service: support Pool.defer() like Broker.defer()
  issue #535: core: unicode.encode() may take importer lock on 2.x
  issue #535: docs: fix up Select doc
  issue #535: docs: update Changelog.
  issue #535: core/select: support selecting from Latches.
  core: increase cookie field lengths to 64-bit; closes #545.
  tests: ensure serialization restrictions are in effect
  tests/bench: set process affinity in throughput.py.
  docs: update copyright year.
  docs: update Changelog.
  core: Make Latch.put(obj=) optional.
  docs: change 'unreleased' Changelog format and add a hint.
  docs: update Changelog; closes #542.
  issue #542: return of select poller, new selection logic
  issue #542: .ci: move some tests to Azure and enable Mac job.
  ansible: create stub __init__.py for sdist.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.