From ca8444f2389556d8a746cc4128f855f760bfa20f Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Sat, 28 Mar 2026 03:50:59 +1000 Subject: [PATCH] Simplify pipelining logic for Windows ci_complete (#86734) * Simplify pipelining logic for Windows ci_complete Has the Windows connection plugins override is_pipelining_enabled to return True rather than use the special connection plugin attributes. These attributes should be removed in the future but when is still dependent on when we can expect Ansible 2.19 is the minimum version supported in collections. * Make CI green Signed-off-by: Abhijeet Kasurde --------- Signed-off-by: Abhijeet Kasurde Co-authored-by: Abhijeet Kasurde --- changelogs/fragments/connection-pipeline.yml | 6 ++++++ lib/ansible/plugins/action/command.py | 2 +- lib/ansible/plugins/action/normal.py | 2 +- lib/ansible/plugins/connection/__init__.py | 16 +++++++++++++--- lib/ansible/plugins/connection/psrp.py | 6 +++--- lib/ansible/plugins/connection/ssh.py | 7 ++++--- lib/ansible/plugins/connection/winrm.py | 6 +++--- .../targets/async_fail/action_plugins/normal.py | 2 +- 8 files changed, 32 insertions(+), 15 deletions(-) create mode 100644 changelogs/fragments/connection-pipeline.yml diff --git a/changelogs/fragments/connection-pipeline.yml b/changelogs/fragments/connection-pipeline.yml new file mode 100644 index 00000000000..8736a18e703 --- /dev/null +++ b/changelogs/fragments/connection-pipeline.yml @@ -0,0 +1,6 @@ +deprecated_features: + - >- + connection plugins - Added a soft deprecation on the connection attributes ``has_native_async`` and + ``always_pipeline_modules``. Connection plugins that wish to apply custom behaviour around pipelining should + instead override the method ``is_pipelining_enabled(self, wrap_async=False)`` added in Ansible 2.19. For backwards + compatibility no runtime deprecation warning is emitted but will be in the future. diff --git a/lib/ansible/plugins/action/command.py b/lib/ansible/plugins/action/command.py index df4dbe9f7c6..e0f60b42f53 100644 --- a/lib/ansible/plugins/action/command.py +++ b/lib/ansible/plugins/action/command.py @@ -14,7 +14,7 @@ class ActionModule(ActionBase): results = super(ActionModule, self).run(tmp, task_vars) del tmp # tmp no longer has any effect - wrap_async = self._task.async_val and not self._connection.has_native_async + wrap_async = self._task.async_val # explicitly call `ansible.legacy.command` for backcompat to allow library/ override of `command` while not allowing # collections search for an unqualified `command` module results = merge_hash(results, self._execute_module(module_name='ansible.legacy.command', task_vars=task_vars, wrap_async=wrap_async)) diff --git a/lib/ansible/plugins/action/normal.py b/lib/ansible/plugins/action/normal.py index 0476f9aacdf..96664288283 100644 --- a/lib/ansible/plugins/action/normal.py +++ b/lib/ansible/plugins/action/normal.py @@ -32,7 +32,7 @@ class ActionModule(ActionBase): result = super(ActionModule, self).run(tmp, task_vars) del tmp # tmp no longer has any effect - wrap_async = self._task.async_val and not self._connection.has_native_async + wrap_async = self._task.async_val # do work! result = merge_hash(result, self._execute_module(task_vars=task_vars, wrap_async=wrap_async)) diff --git a/lib/ansible/plugins/connection/__init__.py b/lib/ansible/plugins/connection/__init__.py index c592ed1d037..16a359e387d 100644 --- a/lib/ansible/plugins/connection/__init__.py +++ b/lib/ansible/plugins/connection/__init__.py @@ -55,8 +55,13 @@ class ConnectionBase(AnsiblePlugin): """ has_pipelining = False - has_native_async = False # eg, winrm - always_pipeline_modules = False # eg, winrm + + # The next 2 options are obsolete and should not be set to True. Plugins should instead override + # is_pipelining_enabled if they need to change the pipelining behaviour. We cannot mark as deprecated yet as some + # plugins may still need to set these for compatibility with older Ansible versions still in use. + has_native_async = False + always_pipeline_modules = False + has_tty = True # for interacting with become plugins # When running over this connection type, prefer modules written in a certain language # as discovered by the specified file extension. An empty string as the @@ -308,7 +313,12 @@ class ConnectionBase(AnsiblePlugin): except KeyError: is_enabled = getattr(self._play_context, 'pipelining', False) - # TODO: deprecate always_pipeline_modules and has_native_async in favor for each plugin overriding this function + # We should deprecate always_pipeline_modules and has_native_async in favor of plugins just overriding this + # method. We cannot add it now as plugins will need to be compatible with older Ansible versions that still need + # to set this to be true. This function was added in 2.19 and when we expect 2.19 being a baseline for plugins + # we can add a runtime deprecation that checks whether these are True and if so warn the function should be + # overridden instead. + # deprecate: description='Revisit adding a deprecation or bump the core_version' core_version='2.24' conditions = [ is_enabled or self.always_pipeline_modules, # enabled via config or forced via connection (eg winrm) not wrap_async or self.has_native_async, # async does not normally support pipelining unless it does (eg winrm) diff --git a/lib/ansible/plugins/connection/psrp.py b/lib/ansible/plugins/connection/psrp.py index 67911649cac..6ec36728fb6 100644 --- a/lib/ansible/plugins/connection/psrp.py +++ b/lib/ansible/plugins/connection/psrp.py @@ -369,9 +369,6 @@ class Connection(ConnectionBase): _shell: PowerShellPlugin def __init__(self, *args: t.Any, **kwargs: t.Any) -> None: - self.always_pipeline_modules = True - self.has_native_async = True - self.runspace: RunspacePool | None = None self.host: PSHost | None = None self._last_pipeline: PowerShell | None = None @@ -790,3 +787,6 @@ class Connection(ConnectionBase): self.host.ui.stderr = [] return rc, to_bytes(stdout, encoding='utf-8'), to_bytes(stderr, encoding='utf-8') + + def is_pipelining_enabled(self, wrap_async: bool = False) -> bool: + return True diff --git a/lib/ansible/plugins/connection/ssh.py b/lib/ansible/plugins/connection/ssh.py index b6ae4994a83..3fa2a1b2357 100644 --- a/lib/ansible/plugins/connection/ssh.py +++ b/lib/ansible/plugins/connection/ssh.py @@ -660,8 +660,6 @@ class Connection(ConnectionBase): # we need to set various properties to ensure SSH on Windows continues # to work if getattr(self._shell, "_IS_WINDOWS", False): - self.has_native_async = True - self.always_pipeline_modules = True self.module_implementation_preferences = ('.ps1', '.exe', '') self.allow_executable = False @@ -1618,7 +1616,10 @@ class Connection(ConnectionBase): def is_pipelining_enabled(self, wrap_async=False): """ override parent method and ensure we don't request a tty """ - if self._is_tty_requested(): + if getattr(self._shell, "_IS_WINDOWS", False): + # pipelining is always used on Windows + return True + elif self._is_tty_requested(): return False else: return super(Connection, self).is_pipelining_enabled(wrap_async) diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py index a46ff8e6830..df89bdcd1bc 100644 --- a/lib/ansible/plugins/connection/winrm.py +++ b/lib/ansible/plugins/connection/winrm.py @@ -245,9 +245,6 @@ class Connection(ConnectionBase): def __init__(self, *args: t.Any, **kwargs: t.Any) -> None: - self.always_pipeline_modules = True - self.has_native_async = True - self.protocol: winrm.Protocol | None = None self.shell_id: str | None = None self.delegate = None @@ -879,3 +876,6 @@ class Connection(ConnectionBase): self.shell_id = None self.protocol = None self._connected = False + + def is_pipelining_enabled(self, wrap_async: bool = False) -> bool: + return True diff --git a/test/integration/targets/async_fail/action_plugins/normal.py b/test/integration/targets/async_fail/action_plugins/normal.py index 22e2a3faeea..5d70d3f5a55 100644 --- a/test/integration/targets/async_fail/action_plugins/normal.py +++ b/test/integration/targets/async_fail/action_plugins/normal.py @@ -40,7 +40,7 @@ class ActionModule(ActionBase): del result['invocation']['module_args'] # FUTURE: better to let _execute_module calculate this internally? - wrap_async = self._task.async_val and not self._connection.has_native_async + wrap_async = self._task.async_val # do work! result = merge_hash(result, self._execute_module(task_vars=task_vars, wrap_async=wrap_async))