diff options
author | Kate Case <ncase@redhat.com> | 2022-08-31 13:21:59 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-08-31 12:21:59 -0500 |
commit | 252c6551ff3e63a83eec45e4b78924a250b4244d (patch) | |
tree | e289fdca860a49c15fec5fcb8ce75e034e9da334 | |
parent | ee7f9b79361b73add27662ca9e455f1a2c12f5e1 (diff) | |
download | ansible-252c6551ff3e63a83eec45e4b78924a250b4244d.tar.gz |
Replace get_persistent_connection_options in task_executor with get_options (#74446) (#78591)
Replace get_persistent_connection_options with get_options
Remove special case for network sub_plugin in _set_plugin_options
Try to avoid mock connection pretending to be persistent
Rename variables->options to reflect what they actually are
Gather options for ssh_type_conn on network_cli
Drop reliance on sub_plugin["type"]
(cherry picked from commit bf1ef5a1f3562c9a59168adbc78750304c3e4309)
-rw-r--r-- | changelogs/fragments/74446-network-conn-options.yaml | 3 | ||||
-rwxr-xr-x | lib/ansible/cli/scripts/ansible_connection_cli_stub.py | 20 | ||||
-rw-r--r-- | lib/ansible/executor/task_executor.py | 53 | ||||
-rw-r--r-- | test/units/executor/test_task_executor.py | 2 |
4 files changed, 42 insertions, 36 deletions
diff --git a/changelogs/fragments/74446-network-conn-options.yaml b/changelogs/fragments/74446-network-conn-options.yaml new file mode 100644 index 0000000000..c862c43553 --- /dev/null +++ b/changelogs/fragments/74446-network-conn-options.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: + - Fix for network_cli not getting all relevant connection options diff --git a/lib/ansible/cli/scripts/ansible_connection_cli_stub.py b/lib/ansible/cli/scripts/ansible_connection_cli_stub.py index 31047d96c5..0afca0cc5f 100755 --- a/lib/ansible/cli/scripts/ansible_connection_cli_stub.py +++ b/lib/ansible/cli/scripts/ansible_connection_cli_stub.py @@ -85,11 +85,11 @@ class ConnectionProcess(object): self.connection = None self._ansible_playbook_pid = ansible_playbook_pid - def start(self, variables): - try: - messages = list() - result = {} + def start(self, options): + messages = list() + result = {} + try: messages.append(('vvvv', 'control socket path is %s' % self.socket_path)) # If this is a relative path (~ gets expanded later) then plug the @@ -100,7 +100,7 @@ class ConnectionProcess(object): self.connection = connection_loader.get(self.play_context.connection, self.play_context, '/dev/null', task_uuid=self._task_uuid, ansible_playbook_pid=self._ansible_playbook_pid) try: - self.connection.set_options(var_options=variables) + self.connection.set_options(direct=options) except ConnectionError as exc: messages.append(('debug', to_text(exc))) raise ConnectionError('Unable to decode JSON from response set_options. See the debug log for more information.') @@ -237,15 +237,15 @@ def main(): try: # read the play context data via stdin, which means depickling it - vars_data = read_stream(stdin) + opts_data = read_stream(stdin) init_data = read_stream(stdin) if PY3: pc_data = cPickle.loads(init_data, encoding='bytes') - variables = cPickle.loads(vars_data, encoding='bytes') + options = cPickle.loads(opts_data, encoding='bytes') else: pc_data = cPickle.loads(init_data) - variables = cPickle.loads(vars_data) + options = cPickle.loads(opts_data) play_context = PlayContext() play_context.deserialize(pc_data) @@ -283,7 +283,7 @@ def main(): os.close(r) wfd = os.fdopen(w, 'w') process = ConnectionProcess(wfd, play_context, socket_path, original_path, task_uuid, ansible_playbook_pid) - process.start(variables) + process.start(options) except Exception: messages.append(('error', traceback.format_exc())) rc = 1 @@ -306,7 +306,7 @@ def main(): messages.append(('vvvv', 'found existing local domain socket, using it!')) conn = Connection(socket_path) try: - conn.set_options(var_options=variables) + conn.set_options(direct=options) except ConnectionError as exc: messages.append(('debug', to_text(exc))) raise ConnectionError('Unable to decode JSON from response set_options. See the debug log for more information.') diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index 9b0ef23760..e1827c9e93 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -25,6 +25,7 @@ from ansible.module_utils._text import to_text, to_native from ansible.module_utils.connection import write_to_file_descriptor from ansible.playbook.conditional import Conditional from ansible.playbook.task import Task +from ansible.plugins import get_plugin_class from ansible.plugins.loader import become_loader, cliconf_loader, connection_loader, httpapi_loader, netconf_loader, terminal_loader from ansible.template import Templar from ansible.utils.collection_loader import AnsibleCollectionConfig, AnsibleCollectionRef @@ -549,6 +550,17 @@ class TaskExecutor: plugin_vars = self._set_connection_options(cvars, templar) templar.available_variables = orig_vars + # for persistent connections, initialize socket path and start connection manager + if any(((self._connection.supports_persistence and C.USE_PERSISTENT_CONNECTIONS), self._connection.force_persistence)): + self._play_context.timeout = self._connection.get_option('persistent_command_timeout') + display.vvvv('attempting to start connection', host=self._play_context.remote_addr) + display.vvvv('using connection plugin %s' % self._connection.transport, host=self._play_context.remote_addr) + + options = self._connection.get_options() + socket_path = start_connection(self._play_context, options, self._task._uuid) + display.vvvv('local domain socket path is %s' % socket_path, host=self._play_context.remote_addr) + setattr(self._connection, '_socket_path', socket_path) + # TODO: eventually remove this block as this should be a 'consequence' of 'forced_local' modules # special handling for python interpreter for network_os, default to ansible python unless overriden if 'ansible_network_os' in cvars and 'ansible_python_interpreter' not in cvars: @@ -966,32 +978,8 @@ class TaskExecutor: # Also backwards compat call for those still using play_context self._play_context.set_attributes_from_plugin(connection) - if any(((connection.supports_persistence and C.USE_PERSISTENT_CONNECTIONS), connection.force_persistence)): - self._play_context.timeout = connection.get_option('persistent_command_timeout') - display.vvvv('attempting to start connection', host=self._play_context.remote_addr) - display.vvvv('using connection plugin %s' % connection.transport, host=self._play_context.remote_addr) - - options = self._get_persistent_connection_options(connection, cvars, templar) - socket_path = start_connection(self._play_context, options, self._task._uuid) - display.vvvv('local domain socket path is %s' % socket_path, host=self._play_context.remote_addr) - setattr(connection, '_socket_path', socket_path) - return connection - def _get_persistent_connection_options(self, connection, final_vars, templar): - - option_vars = C.config.get_plugin_vars('connection', connection._load_name) - plugin = connection._sub_plugin - if plugin.get('type'): - option_vars.extend(C.config.get_plugin_vars(plugin['type'], plugin['name'])) - - options = {} - for k in option_vars: - if k in final_vars: - options[k] = templar.template(final_vars[k]) - - return options - def _set_plugin_options(self, plugin_type, variables, templar, task_keys): try: plugin = getattr(self._connection, '_%s' % plugin_type) @@ -999,6 +987,10 @@ class TaskExecutor: # Some plugins are assigned to private attrs, ``become`` is not plugin = getattr(self._connection, plugin_type) + # network_cli's "real" connection plugin is not named connection + # to avoid the confusion of having connection.connection + if plugin_type == "ssh_type_conn": + plugin_type = "connection" option_vars = C.config.get_plugin_vars(plugin_type, plugin._load_name) options = {} for k in option_vars: @@ -1068,6 +1060,15 @@ class TaskExecutor: pass # some plugins don't support all base flags self._play_context.prompt = self._connection.become.prompt + # deals with networking sub_plugins (network_cli/httpapi/netconf) + sub = getattr(self._connection, '_sub_plugin', None) + if sub is not None and sub.get('type') != 'external': + plugin_type = get_plugin_class(sub.get("obj")) + varnames.extend(self._set_plugin_options(plugin_type, variables, templar, task_keys)) + sub_conn = getattr(self._connection, 'ssh_type_conn', None) + if sub_conn is not None: + varnames.extend(self._set_plugin_options("ssh_type_conn", variables, templar, task_keys)) + return varnames def _get_action_handler(self, connection, templar): @@ -1130,7 +1131,7 @@ class TaskExecutor: return handler, module -def start_connection(play_context, variables, task_uuid): +def start_connection(play_context, options, task_uuid): ''' Starts the persistent connection ''' @@ -1176,7 +1177,7 @@ def start_connection(play_context, variables, task_uuid): try: termios.tcsetattr(master, termios.TCSANOW, new) - write_to_file_descriptor(master, variables) + write_to_file_descriptor(master, options) write_to_file_descriptor(master, play_context.serialize()) (stdout, stderr) = p.communicate() diff --git a/test/units/executor/test_task_executor.py b/test/units/executor/test_task_executor.py index 7352774a4a..dd8e20a733 100644 --- a/test/units/executor/test_task_executor.py +++ b/test/units/executor/test_task_executor.py @@ -334,6 +334,8 @@ class TestTaskExecutor(unittest.TestCase): mock_play_context.update_vars.return_value = None mock_connection = MagicMock() + mock_connection.force_persistence = False + mock_connection.supports_persistence = False mock_connection.set_host_overrides.return_value = None mock_connection._connect.return_value = None |