From a8de35e1318cf03b26c7a2a08900d8bec0611a01 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Tue, 6 Jul 2021 10:43:25 -0400 Subject: [PATCH] task_executor - use correct value for ssh connection retries (#75155) Since the task and connection both have the same 'retries' keyword, the task default would override the connection value. Do not pass 'retries' from the task to the connection options. * Set ssh_connection retries default value back to 0 It was 0 before the move to config and was changed to 3 by accident. --- .../fragments/75142-ssh-retries-collision.yml | 7 +++++++ lib/ansible/executor/task_executor.py | 3 +++ lib/ansible/plugins/connection/ssh.py | 6 +++--- test/units/plugins/connection/test_ssh.py | 16 ++++++++-------- 4 files changed, 21 insertions(+), 11 deletions(-) create mode 100644 changelogs/fragments/75142-ssh-retries-collision.yml diff --git a/changelogs/fragments/75142-ssh-retries-collision.yml b/changelogs/fragments/75142-ssh-retries-collision.yml new file mode 100644 index 00000000000..a97895453f3 --- /dev/null +++ b/changelogs/fragments/75142-ssh-retries-collision.yml @@ -0,0 +1,7 @@ +bugfixes: + - >- + task_executor/ssh_connection - use the ``retries`` value from ``ssh_connection`` settings, + not the default from the ``Task`` field attributes (https://github.com/ansible/ansible/issues/75142). + + - ssh_connection - set the default for ``reconnection_retries`` back to ``0`` (https://github.com/ansible/ansible/issues/75142). + - ssh_connection - rename ``retries`` to ``reconnection_retries`` to avoid conflicts with task vars (https://github.com/ansible/ansible/issues/75142). diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index fe54b559a1c..e9f9007f1b8 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -1020,6 +1020,9 @@ class TaskExecutor: # config system instead of directly accessing play_context. task_keys['password'] = self._play_context.password + # Prevent task retries from overriding connection retries + del(task_keys['retries']) + # set options with 'templated vars' specific to this plugin and dependent ones self._connection.set_options(task_keys=task_keys, var_options=options) varnames.extend(self._set_plugin_options('shell', variables, templar, task_keys)) diff --git a/lib/ansible/plugins/connection/ssh.py b/lib/ansible/plugins/connection/ssh.py index e869753259a..da7f38704a8 100644 --- a/lib/ansible/plugins/connection/ssh.py +++ b/lib/ansible/plugins/connection/ssh.py @@ -167,9 +167,9 @@ DOCUMENTATION = ''' version_added: '2.7' cli: - name: ssh_extra_args - retries: + reconnection_retries: description: Number of attempts to connect. - default: 3 + default: 0 type: integer env: - name: ANSIBLE_SSH_RETRIES @@ -453,7 +453,7 @@ def _ssh_retry(func): """ @wraps(func) def wrapped(self, *args, **kwargs): - remaining_tries = int(self.get_option('retries')) + 1 + remaining_tries = int(self.get_option('reconnection_retries')) + 1 cmd_summary = u"%s..." % to_text(args[0]) conn_password = self.get_option('password') or self._play_context.password for attempt in range(remaining_tries): diff --git a/test/units/plugins/connection/test_ssh.py b/test/units/plugins/connection/test_ssh.py index 7c7afa049b6..d693313ff01 100644 --- a/test/units/plugins/connection/test_ssh.py +++ b/test/units/plugins/connection/test_ssh.py @@ -234,7 +234,7 @@ class TestConnectionBaseClass(unittest.TestCase): conn._bare_run.return_value = (0, '', '') conn.host = "some_host" - conn.set_option('retries', 9) + conn.set_option('reconnection_retries', 9) conn.set_option('ssh_transfer_method', None) # unless set to None scp_if_ssh is ignored # Test with SCP_IF_SSH set to smart @@ -292,7 +292,7 @@ class TestConnectionBaseClass(unittest.TestCase): conn._bare_run.return_value = (0, '', '') conn.host = "some_host" - conn.set_option('retries', 9) + conn.set_option('reconnection_retries', 9) conn.set_option('ssh_transfer_method', None) # unless set to None scp_if_ssh is ignored # Test with SCP_IF_SSH set to smart @@ -535,7 +535,7 @@ class TestSSHConnectionRun(object): class TestSSHConnectionRetries(object): def test_incorrect_password(self, monkeypatch): self.conn.set_option('host_key_checking', False) - self.conn.set_option('retries', 5) + self.conn.set_option('reconnection_retries', 5) monkeypatch.setattr('time.sleep', lambda x: None) self.mock_popen_res.stdout.read.side_effect = [b''] @@ -560,7 +560,7 @@ class TestSSHConnectionRetries(object): def test_retry_then_success(self, monkeypatch): self.conn.set_option('host_key_checking', False) - self.conn.set_option('retries', 3) + self.conn.set_option('reconnection_retries', 3) monkeypatch.setattr('time.sleep', lambda x: None) @@ -589,7 +589,7 @@ class TestSSHConnectionRetries(object): def test_multiple_failures(self, monkeypatch): self.conn.set_option('host_key_checking', False) - self.conn.set_option('retries', 9) + self.conn.set_option('reconnection_retries', 9) monkeypatch.setattr('time.sleep', lambda x: None) @@ -612,7 +612,7 @@ class TestSSHConnectionRetries(object): def test_abitrary_exceptions(self, monkeypatch): self.conn.set_option('host_key_checking', False) - self.conn.set_option('retries', 9) + self.conn.set_option('reconnection_retries', 9) monkeypatch.setattr('time.sleep', lambda x: None) @@ -625,7 +625,7 @@ class TestSSHConnectionRetries(object): def test_put_file_retries(self, monkeypatch): self.conn.set_option('host_key_checking', False) - self.conn.set_option('retries', 3) + self.conn.set_option('reconnection_retries', 3) monkeypatch.setattr('time.sleep', lambda x: None) monkeypatch.setattr('ansible.plugins.connection.ssh.os.path.exists', lambda x: True) @@ -656,7 +656,7 @@ class TestSSHConnectionRetries(object): def test_fetch_file_retries(self, monkeypatch): self.conn.set_option('host_key_checking', False) - self.conn.set_option('retries', 3) + self.conn.set_option('reconnection_retries', 3) monkeypatch.setattr('time.sleep', lambda x: None) monkeypatch.setattr('ansible.plugins.connection.ssh.os.path.exists', lambda x: True)