diff --git a/scripts/docker/shared-image-artifact.sh b/scripts/docker/shared-image-artifact.sh index 95aebccaab37..09e5ca3153e7 100755 --- a/scripts/docker/shared-image-artifact.sh +++ b/scripts/docker/shared-image-artifact.sh @@ -53,12 +53,17 @@ configure_image_artifact_inputs() { is_transient_gh_api_get_error() { local error_text="$1" - if [[ "$error_text" =~ (^|[^0-9])(401|403|404|422)([^0-9]|$) || + local not_found_policy="$2" + if [[ "$error_text" =~ (^|[^0-9])(401|403|422)([^0-9]|$) || "$error_text" =~ [Bb]ad[[:space:]]+[Cc]redentials || "$error_text" =~ [Cc]redential || "$error_text" =~ [Aa]uthentication ]]; then return 1 fi + if [[ "$error_text" =~ (^|[^0-9])404([^0-9]|$) ]]; then + [[ "$not_found_policy" == "retry-fresh-artifact" ]] + return + fi [[ "$error_text" == *"i/o timeout"* || "$error_text" =~ [Cc]ontext[[:space:]]+deadline[[:space:]]+exceeded || @@ -76,7 +81,21 @@ is_transient_gh_api_get_error() { gh_api_get_with_retry() { local label="$1" local endpoint="$2" + local not_found_policy="$3" local attempt error_file response_file retry_delay retry_dir + case "$not_found_policy" in + fail-fast) ;; + retry-fresh-artifact) + # Artifact metadata can briefly lag upload completion. Keep 404 retries confined + # to that fresh-object read so producer tuple and authentication failures stay immediate. + if [[ ! "$endpoint" =~ ^repos/[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+/actions/artifacts/[1-9][0-9]*$ ]]; then + fail "$label cannot retry 404 responses for this GitHub API endpoint." + fi + ;; + *) + fail "$label has an invalid GitHub API 404 retry policy." + ;; + esac retry_dir="$(mktemp -d)" response_file="${retry_dir}/response" error_file="${retry_dir}/error" @@ -90,7 +109,8 @@ gh_api_get_with_retry() { return 0 fi - if [[ "$attempt" -lt 3 ]] && is_transient_gh_api_get_error "$(cat "$error_file")"; then + if [[ "$attempt" -lt 3 ]] && + is_transient_gh_api_get_error "$(cat "$error_file")" "$not_found_policy"; then retry_delay=$((attempt * 2)) printf \ 'warning: %s GitHub API GET failed transiently on attempt %d/3; retrying in %ss.\n' \ @@ -142,7 +162,8 @@ verify_uploaded_artifact() { artifact_json="$( gh_api_get_with_retry \ "$artifact_label artifact metadata" \ - "repos/${GITHUB_REPOSITORY}/actions/artifacts/${artifact_id}" + "repos/${GITHUB_REPOSITORY}/actions/artifacts/${artifact_id}" \ + "retry-fresh-artifact" )" jq -e \ --arg digest "sha256:${artifact_digest}" \ @@ -161,7 +182,8 @@ verify_uploaded_artifact() { attempt_json="$( gh_api_get_with_retry \ "$artifact_label producer run attempt metadata" \ - "repos/${GITHUB_REPOSITORY}/actions/runs/${artifact_run_id}/attempts/${artifact_run_attempt}" + "repos/${GITHUB_REPOSITORY}/actions/runs/${artifact_run_id}/attempts/${artifact_run_attempt}" \ + "fail-fast" )" jq -e \ --arg attempt "$artifact_run_attempt" \ diff --git a/test/scripts/shared-image-artifact.test.ts b/test/scripts/shared-image-artifact.test.ts index 8cdec2a8c304..ba6b457fc087 100644 --- a/test/scripts/shared-image-artifact.test.ts +++ b/test/scripts/shared-image-artifact.test.ts @@ -235,6 +235,11 @@ case "$path" in count_file="$FAKE_GH_STATE/attempt" count="$(( $(cat "$count_file" 2>/dev/null || printf 0) + 1 ))" printf '%s\\n' "$count" > "$count_file" + if [[ "$count" -le "\${FAKE_GH_ATTEMPT_FAILURES:-0}" ]]; then + printf 'partial attempt response\\n' + printf '%s\\n' "\${FAKE_GH_ATTEMPT_ERROR:-gh: request failed}" >&2 + exit 1 + fi printf '{"id":%s,"run_attempt":%s}\\n' \ "$FAKE_ATTEMPT_RUN_ID" "$FAKE_ARTIFACT_RUN_ATTEMPT" ;; @@ -363,6 +368,69 @@ describe("shared Docker image artifacts", () => { } }); + it("retries a fresh artifact metadata 404 and then succeeds", () => { + const fixture = createFixture(); + try { + const verified = verifyUploadedArtifact(fixture, { + env: { + FAKE_GH_ARTIFACT_ERROR: "gh: Not Found (HTTP 404)", + FAKE_GH_ARTIFACT_FAILURES: "1", + }, + }); + expect(verified.status, `${verified.stdout}\n${verified.stderr}`).toBe(0); + expect(verified.stderr).toContain( + "artifact metadata GitHub API GET failed transiently on attempt 1/3; retrying in 2s", + ); + expect(readFileSync(fixture.sleepLog, "utf8")).toBe("2\n"); + const calls = readFileSync(fixture.ghLog, "utf8"); + expect(calls.match(/actions\/artifacts/g)).toHaveLength(2); + expect(calls.match(/actions\/runs/g)).toHaveLength(1); + } finally { + rmSync(fixture.root, { force: true, recursive: true }); + } + }); + + it("fails after three fresh artifact metadata 404 responses", () => { + const fixture = createFixture(); + try { + const failed = verifyUploadedArtifact(fixture, { + env: { + FAKE_GH_ARTIFACT_ERROR: "gh: Not Found (HTTP 404)", + FAKE_GH_ARTIFACT_FAILURES: "3", + }, + }); + expect(failed.status).not.toBe(0); + expect(failed.stderr).toContain("GitHub API GET failed after 3 attempt(s)"); + expect(readFileSync(fixture.sleepLog, "utf8")).toBe("2\n4\n"); + const calls = readFileSync(fixture.ghLog, "utf8"); + expect(calls.match(/actions\/artifacts/g)).toHaveLength(3); + expect(calls).not.toContain("actions/runs"); + } finally { + rmSync(fixture.root, { force: true, recursive: true }); + } + }); + + it("fails immediately when producer run-attempt metadata returns 404", () => { + const fixture = createFixture(); + try { + const failed = verifyUploadedArtifact(fixture, { + env: { + FAKE_GH_ATTEMPT_ERROR: "gh: Not Found (HTTP 404)", + FAKE_GH_ATTEMPT_FAILURES: "3", + }, + }); + expect(failed.status).not.toBe(0); + expect(failed.stderr).toContain("GitHub API GET failed after 1 attempt(s)"); + expect(failed.stderr).not.toContain("retrying"); + expect(readFileSync(fixture.sleepLog, "utf8")).toBe(""); + const calls = readFileSync(fixture.ghLog, "utf8"); + expect(calls.match(/actions\/artifacts/g)).toHaveLength(1); + expect(calls.match(/actions\/runs/g)).toHaveLength(1); + } finally { + rmSync(fixture.root, { force: true, recursive: true }); + } + }); + it("recovers on the third attempt and fails cleanly when all attempts are exhausted", () => { const recoveredFixture = createFixture(); try { @@ -402,7 +470,7 @@ describe("shared Docker image artifacts", () => { } }); - it("retries explicit rate limiting but not permanent HTTP or credential failures", () => { + it("retries explicit rate limiting but fails fast on permanent HTTP and credential errors", () => { const rateLimitFixture = createFixture(); try { const rateLimited = verifyUploadedArtifact(rateLimitFixture, { @@ -417,7 +485,13 @@ describe("shared Docker image artifacts", () => { rmSync(rateLimitFixture.root, { force: true, recursive: true }); } - for (const error of ["gh: Not Found (HTTP 404)", "gh: Bad credentials (HTTP 500)"]) { + for (const error of [ + "gh: Unauthorized (HTTP 401)", + "gh: Forbidden (HTTP 403)", + "gh: Unprocessable Entity (HTTP 422)", + "gh: Bad credentials (HTTP 500)", + "gh: authentication failed (HTTP 500)", + ]) { const fixture = createFixture(); try { const failed = verifyUploadedArtifact(fixture, {