acc: deflake dashboard read-after-deploy retry (#5895)

The `bundle/resources/dashboards` acceptance test flaked on cloud: after
deploy, reads of a just-renamed/updated dashboard hit eventual
consistency, and `retry.py`'s default ~2s window (5 × 500ms) wasn't
enough.

- **`retry.py`**: On exhaustion, the old loop passed the last attempt's
stdout through as if valid even when a `--until`/`--until-not` condition
never held — masking the flake as a downstream output diff. Now, an
unmet content condition writes stale output to **stderr** and exits
non-zero (`retry: condition not met after N attempts`); success and the
no-condition case pass output through on stdout unchanged.
- **`test.toml`**: Widen the window to `RETRY_MAX_ATTEMPTS=20` /
`RETRY_INTERVAL_MS=1000` (~20s). `retry.py`'s loop isn't scaled by
`TimeoutCIMultiplier`, and real cloud eventual consistency regularly
exceeds the ~2s default.

This pull request and its description were written by Isaac.

Author: pietern

acceptance/bin/retry.py

@@ -8,6 +8,13 @@
 An attempt is considered successful when the command exits with code 0 and:
   --until SUBSTR     SUBSTR appears in stdout
   --until-not SUBSTR SUBSTR does not appear in stdout
+
+The condition is checked on every attempt, including the last. If a
+--until/--until-not condition was given but never satisfied within
+RETRY_MAX_ATTEMPTS, retry.py writes the last (stale) output to stderr and exits
+non-zero, rather than passing the stale output through on stdout. When no
+condition is given, the final attempt's output and exit code pass through
+unchanged.
 """
 
 import argparse
@@ -32,18 +39,37 @@ def main():
     interval = float(os.environ.get("RETRY_INTERVAL_MS", "500")) / 1000.0
     max_attempts = int(os.environ.get("RETRY_MAX_ATTEMPTS", "5"))
 
-    result = subprocess.run(argv, capture_output=True)
-    for _ in range(1, max_attempts):
-        success = (
+    def succeeded(result):
+        return (
             result.returncode == 0
             and (until is None or until.encode() in result.stdout)
             and (until_not is None or until_not.encode() not in result.stdout)
         )
-        if success:
-            break
-        time.sleep(interval)
+
+    ok = False
+    result = None
+    for attempt in range(max_attempts):
+        if attempt > 0:
+            time.sleep(interval)
         result = subprocess.run(argv, capture_output=True)
+        if succeeded(result):
+            ok = True
+            break
+
+    if not ok and (until is not None or until_not is not None):
+        # A content condition was requested but never held within max_attempts. Emit the
+        # stale output to stderr (never stdout) so callers capturing stdout don't silently
+        # ingest stale data, and fail loudly with a non-zero exit so the flake is
+        # attributed here rather than surfacing later as an unexplained output diff.
+        sys.stderr.buffer.write(result.stdout)
+        sys.stderr.buffer.write(result.stderr)
+        sys.stderr.buffer.flush()
+        print(f"retry: condition not met after {max_attempts} attempts", file=sys.stderr)
+        sys.exit(result.returncode or 1)
 
+    # Success, or no content condition was requested (retries only guarded the exit code).
+    # Either way callers capture stdout (e.g. DASHBOARD=$(retry --until ...)), so pass the
+    # final attempt's output and exit code through unchanged.
     sys.stdout.buffer.write(result.stdout)
     sys.stderr.buffer.write(result.stderr)
     sys.exit(result.returncode)

acceptance/bundle/resources/dashboards/test.toml

@@ -15,3 +15,10 @@ MSYS_NO_PATHCONV = "1"
 # returns 404, so reads right after deploy must be retried (retry.py). Skipped on
 # terraform, whose provider reads the dashboard back after create and fails on the 404.
 INJECT_STALE_ON_DIRECT = "1"
+
+# Widen retry.py's window well beyond its ~2s default (5 attempts x 500ms). retry.py's
+# loop is not scaled by TimeoutCIMultiplier (that only scales the overall test timeout),
+# and real dashboard rename/update eventual consistency on cloud regularly exceeds 2s,
+# which flaked these read-after-deploy checks. 20 attempts x 1000ms gives a ~20s window.
+RETRY_MAX_ATTEMPTS = "20"
+RETRY_INTERVAL_MS = "1000"