Optional ansible-connection execute_module verb

[dw]

[WIP! But it's fleshed out enough that good feedback may already be possible. This could be further split into multiple proposals? Or just one cohesive proposal split out into multiple PRs? It may also not include every detail from the meeting yet, or include too much]

Proposal: Optional ansible-connection execute_module verb

Author: David Wilson <@dw>

Date: 2019-02-06

• Status: New
• Proposal type: core design
• Targeted release: TBD
• Associated PR: TBD
• Estimated time to implement: ~2 weeks

Motivation

An extension to Ansible delivered as part of the Mitogen project demonstrated through changes to networking and the process model on targets, it is possible to recover significant speed improvements in a variety of real-world scenarios.

In order to achieve its goals, that project rewrote significant portions of the existing module executor in order to integrate with the newly available networking APIs, and to add the ability cache module source code ephemerally for the duration of a playbook run, yielding a large reduction in bandwidth consumption.

Problems

While the long term goal should probably be to merge away the duplicate module executor, as an initial step towards upstreaming it is desirable to reuse the implementation that has already been designed for this new scenario.

Simultaneous to the needs of the Mitogen project, Ansible already supports diverse target environments, where the existing executor has required heavy special casing for use on Windows, or in the case of networking, only sees vestigial use targeting the local machine, as the majority of networking functionality is implemented in terms of action modules. It is therefore desirable to provide a general mechanism to allow selection of the executor.

Solution proposal

This proposal covers a series of hopefully small pull requests, that pave the way for:

• refactor a postprocess_response() method out of _execute_module(),
• introduce of a type separate from ActionBase to describe the active configuration leading to a module invocation,
• the choice of module executor to be placed under the control of the active connection plug-in, by adding a short-circuit path to the ActionBase.execute_module() function.
• updating the ConnectionBase class with documentation of the new method.
• the ansible-connection program to be extended to support new execute module and file transfer commands, and
• for the persistent connection plug-in to be extended if necessary to drive selection of the actual connection plug-in that is loaded by ansible-connection.

Refactor postprocess_response()

To emulate the existing executor, the Mitogen ActionModule mix-in presently cut-pastes a large chunk of code from the bottom of execute_module(), responsible for normalizing stdout_lines and similar response fields.

This behaviour is an integral part of module response formatting, rather than a feature of the Ansiballz executor, and should therefore be separated from it.

Module invocation type

Since module execution will become a public part of the Connection API and must remain stable over time, it is desirable to introduce a compound type to describe interface parameters, that can be extended without changing the prototype of API methods over time, instead permitting old plug-ins to continue operating even though the compound type has grown new attributes they do not understand.

The alternative of passing parameter lists permits no future backwards-compatible extension of the interface, since it is not possible to know which style of parameter list any particular Connection.execute_module() implementation understands.

The minimum necessary information to execute a module includes:

• module_name: the module name (command)
• module_args: the final set of template-expanded, processed arguments ({"_raw_params": "hostname"})
• module_env: the final set of template-expanded, processed module environment variables
• wrap_async: True if the invocation is to start an asynchronous task
• timeout_secs: If >0, the limit in seconds a wrap_async may execute for

Additional information, such as the currently active PluginLoader search paths, are available globally and do not need to be passed to the connection plug-in.

The Mitogen extension has a closely related type which includes some attributes that are definitely unneeded for a generic implementation, but exists for the same reason: to delineate an interface boundary.

TBD that type presently includes access to a Templar and task_vars to permit templating of ansible_*_interpreter variables. It is unclear whether a generic interface should include either of these attributes, and if they should not be included, what a replacement should look like.

ActionBase short-circuit

The intention of the ActionBase modification is to have a redirect similar to the following:

    def _execute_module(self, ...):
        # Gather up module invocation context
        invocation = ModuleInvocation(...)

        conn_execute_module = getattr(self._connection, 'execute_module', None)
        try:
            return conn_execute_module(invocation)
        except NotImplementedError:
            pass

        return self._ansiballz_execute_module(invocation)

    def _ansiballz_execute_module(self, invocation):
        # Optional: avoid diff churn by simply passing raw args straight-through.

This avoids assuming that all connection plug-ins would feature an execute_module() method, which may be true for custom plug-ins that may not inherit from ConnectionBase.

Update ConnectionBase

ConnectionBase is updated with a new documentary stub method similar to:

    def execute_module(self, invocation):
        '''
        Arrange for the module described by `invocation` to be invoked on the
        target.
        '''
        raise NotImplementedError()

Update ansible-connection

The ansible-connection program implements a JSON-RPC server over a UNIX socket. Presently it has no support for execute_module() or file transfer.

• A new execute_module JSON-RPC method will be added, implemented by a new forwarder function in (AFAIK) lib/ansible/module_utils/connection.py

• New functions will be added in a separate PR to add forwarders for file transfer functionality.

Update persistent

TBD.

Dependencies (optional)

The changes described may need to occur in set order.

Testing (optional)

Tests to ensure that:

• ActionBase triggers an alternative module executor where the active connection plug-in supplies one.
• ActionBase uses Ansiballz given a plug-in entirely missing the execute_module() method.
• ActionBase yields a ModuleInvocation instance containing the correct information.
• ActionBase._execute_module correctly post-processes responses that require it.
• persistent + ansible-connection forwarders correctly implement fetch_file()
• persistent + ansible-connection forwarders correctly implement put_file()
• persistent + ansible-connection forwarders correctly implement execute_module()

TBD:

• persistent correctly determines hosted plug-in does/does not support execute_module().

Documentation (optional)

Relevant docstrings should suffice.

Anything else?

This proposal is currently missing:

• a mechanism to avoid any round-trip from the persistent plug-in to ansible-connection in order to determine if file transfer or execute_module() are supported. It may not be needed, simply turn the relevant JSON-RPC error into NotImplementedError or similar.

• a mechanism to select the actual connection plug-in hosted by ansible-connection. It looks like it may be hard-wired for ssh/paramiko at present?

[dw]

I've had doubts overnight whether this should be the first step -- it may make sense to work from the other direction, first landing an SSH-like connection method only, then extending it with the module executor parts, as it (seemingly) requires less structural change and 'forces' the Mitogen-specific pieces out into the open.

Coming from the other direction, we're forced to address the issue of a varying per-task ansible_python_interpreter variable that somehow needs to make it into -connection.

Is it okay to discuss it here, or put an item on the agenda for the next meeting? It is still possible to work from this direction, there is no lost work, but I suspect this per-task variables issue might be trickier business that it would be better to get out of the way upfront.

[dagwieers]

@dw Maybe put it on the agenda so that it can be discussed with core developers? Next meeting is again on Tuesday.

[bcoca]

Some thoughts i had while away:

• persistent: this should not be a connection plugin but a property of connections. Depending on the connection plugin it could have built in support or require ansible-connection helper (class property?). Keeping ansible-connection 'dumb' allows for the actual connection plugins to implement things differently as needed.

• execute module: I would still keep the action base and split this into the 'remote executor' in the background, then have result processing in the general action function instead of having to copy/refactor elsewhere. I was thinking of adding a 'shell' plugin that would cover mitogen as the 'executor' on the remote side, but unsure this is the best path, i really don't want to make module execution 'pluggable' but it is already 1/2 way there, the current construction already goes over several diff ways of executing modules (ansiballz , old style, replacer, non-python, etc). This also adds the posibility of moving ansible_x_intepreter (python is just most common one) logic into the shell plugins, allowing for 'signature' changes at the plugin level to disambiguate.

[dw]

Re: ansible_*_interpreter, that's an incredibly good point. You're right, the Mitogen executor already needs access to those. Actually it is also using mitogen_task_isolation, but ideally that wouldn't need to be user-configurable at all in the long term, so no need to preserve it

[dw]

Just making a note here that ansible-connection lifetime does not match that of existing connection multiplexer -- ansible-connections are torn down in StrategyBase.cleanup() at the end of each play. Mitogen mux exists for the duration of the run