"retries" without "until": retry until success (#81531)

Fixes #20802

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)

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:

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

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