diff --git a/changelogs/fragments/20802-until-default.yml b/changelogs/fragments/20802-until-default.yml new file mode 100644 index 00000000000..52062df9143 --- /dev/null +++ b/changelogs/fragments/20802-until-default.yml @@ -0,0 +1,2 @@ +minor_changes: + - tasks - the ``retries`` keyword can be specified without ``until`` in which case the task is retried until it succeeds but at most ``retries`` times (https://github.com/ansible/ansible/issues/20802) diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index 47141f06984..59c9eae40e7 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -620,17 +620,11 @@ class TaskExecutor: if omit_token is not None: self._task.args = remove_omit(self._task.args, omit_token) - # Read some values from the task, so that we can modify them if need be - if self._task.until: - retries = self._task.retries - if retries is None: - retries = 3 - elif retries <= 0: - retries = 1 - else: - retries += 1 - else: - retries = 1 + retries = 1 # includes the default actual run + retries set by user/default + if self._task.retries is not None: + retries += max(0, self._task.retries) + elif self._task.until: + retries += 3 # the default is not set in FA because we need to differentiate "unset" value delay = self._task.delay if delay < 0: @@ -736,7 +730,7 @@ class TaskExecutor: result['failed'] = False # Make attempts and retries available early to allow their use in changed/failed_when - if self._task.until: + if retries > 1: result['attempts'] = attempt # set the changed property if it was missing. @@ -768,7 +762,7 @@ class TaskExecutor: if retries > 1: cond = Conditional(loader=self._loader) - cond.when = self._task.until + cond.when = self._task.until or [not result['failed']] if cond.evaluate_conditional(templar, vars_copy): break else: diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 599abf2b9d9..8c80fdfe633 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -79,7 +79,7 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl loop_control = NonInheritableFieldAttribute(isa='class', class_type=LoopControl, default=LoopControl) poll = NonInheritableFieldAttribute(isa='int', default=C.DEFAULT_POLL_INTERVAL) register = NonInheritableFieldAttribute(isa='string', static=True) - retries = NonInheritableFieldAttribute(isa='int', default=3) + retries = NonInheritableFieldAttribute(isa='int') # default is set in TaskExecutor until = NonInheritableFieldAttribute(isa='list', default=list) # deprecated, used to be loop and loop_args but loop has been repurposed diff --git a/test/integration/targets/until/tasks/main.yml b/test/integration/targets/until/tasks/main.yml index 2b2ac94e57a..42ce9c8fb95 100644 --- a/test/integration/targets/until/tasks/main.yml +++ b/test/integration/targets/until/tasks/main.yml @@ -82,3 +82,37 @@ register: counter delay: 0.5 until: counter.rc == 0 + +- name: test retries without explicit until, defaults to "until task succeeds" + block: + - name: EXPECTED FAILURE + fail: + retries: 3 + delay: 0.1 + register: r + ignore_errors: true + + - assert: + that: + - r.attempts == 3 + + - vars: + test_file: "{{ lookup('env', 'OUTPUT_DIR') }}/until_success_test_file" + block: + - file: + name: "{{ test_file }}" + state: absent + + - name: fail on the first invocation, succeed on the second + shell: "[ -f {{ test_file }} ] || (touch {{ test_file }} && false)" + retries: 5 + delay: 0.1 + register: r + always: + - file: + name: "{{ test_file }}" + state: absent + + - assert: + that: + - r.attempts == 2