diff --git a/.github/actions/download-artifact/action.yml b/.github/actions/download-artifact/action.yml new file mode 100644 index 000000000..862030a6a --- /dev/null +++ b/.github/actions/download-artifact/action.yml @@ -0,0 +1,72 @@ +name: Download artifact with a verified retry +description: Re-download once when the download fails or leaves files that fail a check +inputs: + pattern: + description: Glob of artifact names to download + required: true + path: + description: Directory to download into (cleared before the retry) + required: true + merge-multiple: + description: Extract every matching artifact into the same directory + default: 'false' + verify: + description: >- + Bash script, run from the workspace, that exits non-zero when the + downloaded files are incomplete or corrupt (e.g. `zstd -t `) + required: true +runs: + using: composite + steps: + # download-artifact v4 can finish "successfully" with a truncated file: + # run 38059192473 attempt 1 logged "Starting download" and nothing more, + # then `zstd -d` failed with "Read error (39) : premature end" in the + # next step and the leg went red. A merge-queue leg that does that + # evicts the entry, so check the files and fetch them once more. + - name: Download (attempt 1) + id: first + if: ${{ !cancelled() }} + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 + continue-on-error: true + with: + pattern: ${{ inputs.pattern }} + merge-multiple: ${{ inputs.merge-multiple }} + path: ${{ inputs.path }} + + - name: Verify (attempt 1) + id: first-check + if: ${{ !cancelled() && steps.first.outcome == 'success' }} + continue-on-error: true + shell: bash + env: + VERIFY: ${{ inputs.verify }} + run: bash --noprofile --norc -euo pipefail -c "$VERIFY" + + - name: Clear the failed download + if: ${{ !cancelled() && (steps.first.outcome == 'failure' || steps.first-check.outcome == 'failure') }} + shell: bash + env: + DOWNLOAD_PATH: ${{ inputs.path }} + run: | + set -euo pipefail + echo "::warning::Artifact download or check failed; downloading again" + rm -rf -- "$DOWNLOAD_PATH" + sleep 10 + + # Neither step below continues on error: a second bad download is not + # a blip, and the job must fail rather than test corrupt files. + - name: Download (attempt 2) + id: second + if: ${{ !cancelled() && (steps.first.outcome == 'failure' || steps.first-check.outcome == 'failure') }} + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 + with: + pattern: ${{ inputs.pattern }} + merge-multiple: ${{ inputs.merge-multiple }} + path: ${{ inputs.path }} + + - name: Verify (attempt 2) + if: ${{ !cancelled() && steps.second.outcome == 'success' }} + shell: bash + env: + VERIFY: ${{ inputs.verify }} + run: bash --noprofile --norc -euo pipefail -c "$VERIFY" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20caf9438..ffccdf6d8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1288,11 +1288,13 @@ jobs: persist-credentials: false - name: Download the e2e binaries - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 + # Re-downloads once if the archive arrives truncated. + uses: ./.github/actions/download-artifact with: pattern: e2e-bin-${{ matrix.os }}* merge-multiple: true path: target/e2e-archive + verify: zstd -q -t target/e2e-archive/e2e-bin.tar.zst - name: Unpack the e2e binaries shell: bash @@ -2261,11 +2263,13 @@ jobs: with: persist-credentials: false - name: Download the e2e binaries - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 + # Re-downloads once if the archive arrives truncated. + uses: ./.github/actions/download-artifact with: pattern: e2e-bin-${{ matrix.os }}* merge-multiple: true path: target/e2e-archive + verify: zstd -q -t target/e2e-archive/e2e-bin.tar.zst - name: Unpack the e2e binaries shell: bash # Decompress to a file, not into `| tar -xf -`: a tar that exits at diff --git a/scripts/tests/test_ci_e2e_archive.py b/scripts/tests/test_ci_e2e_archive.py index 3ebbbc5d4..ae2f9f6a1 100644 --- a/scripts/tests/test_ci_e2e_archive.py +++ b/scripts/tests/test_ci_e2e_archive.py @@ -119,6 +119,51 @@ def test_corrupt_archive_fails_the_consumer_step(self): (archive_dir / "e2e-bin.tar.zst").write_bytes(b"not a zstd archive") self.assertNotEqual(run(consumer, work).returncode, 0) + def test_download_retries_when_the_archive_arrives_truncated(self): + # download-artifact v4 once left a truncated e2e-bin.tar.zst and + # still passed (run 38059192473): each consumer's verify check must + # reject that file so the wrapper downloads it again. + checks = [] + for job in JOBS.values(): + for step_name, step in rows.steps(job): + if step_name == "Download the e2e binaries": + self.assertIn("uses: ./.github/actions/download-artifact", step) + verify = re.search(r"(?m)^ verify: (.+)$", step) + self.assertIsNotNone(verify, "an e2e-bin download has no verify check") + checks.append(verify[1]) + self.assertTrue(checks) + for check in checks: + with tempfile.TemporaryDirectory(prefix="e2e-truncated-") as directory: + work = Path(directory) + archive_dir = work / "target/e2e-archive" + archive_dir.mkdir(parents=True) + archive = archive_dir / "e2e-bin.tar.zst" + payload = work / "payload" + payload.write_bytes(os.urandom(1 << 20)) + subprocess.run(["zstd", "-q", str(payload), "-o", str(archive)], check=True) + self.assertEqual(run(check, work).returncode, 0, check) + archive.write_bytes(archive.read_bytes()[: archive.stat().st_size // 2]) + self.assertNotEqual(run(check, work).returncode, 0, check) + archive.unlink() + self.assertNotEqual(run(check, work).returncode, 0, "a missing archive passed " + check) + + def test_download_wrapper_retries_a_failed_check_and_fails_on_the_second(self): + action = (ROOT / ".github/actions/download-artifact/action.yml").read_text() + retry = "${{ !cancelled() && (steps.first.outcome == 'failure' || steps.first-check.outcome == 'failure') }}" + steps = action.split(" - name: ")[1:] + gates = {step.split("\n", 1)[0]: re.search(r"(?m)^ if: (.+)$", step)[1] for step in steps} + self.assertEqual(gates, { + "Download (attempt 1)": "${{ !cancelled() }}", + "Verify (attempt 1)": "${{ !cancelled() && steps.first.outcome == 'success' }}", + "Clear the failed download": retry, + "Download (attempt 2)": retry, + "Verify (attempt 2)": "${{ !cancelled() && steps.second.outcome == 'success' }}", + }) + # Only the first attempt may fail softly; the second must fail the job. + for step in steps: + name = step.split("\n", 1)[0] + self.assertEqual("continue-on-error: true" in step, name.endswith("(attempt 1)"), name) + if __name__ == "__main__": unittest.main()