Repository navigation
fix(build): bound retries for Bicep downloads - #13291
Conversation
Allow transient GitHub failures to recover beyond the old five-retry budget. Limit each request to 300 seconds and up to ten retries, with connection and transfer timeouts. Stop on permanent HTTP and certificate errors, but keep retries for dropped connections. Add offline regression tests to make test and document the policy. Fixes: #13141 Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>
Use Bash line counts to reduce process starts, and support shasum when sha256sum is not installed. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
github-actions
Bot
added
the
pr:waiting-for-review
nicolejms
added
the
pr:standard
There was a problem hiding this comment.
🟢 Approval recommended
The retry policy is bounded, selectively handles transient failures, and has comprehensive offline regression coverage.
0 open findings
What changed in this PR
Adds bounded, selective retries to make Bicep downloads more resilient without retrying permanent failures.
Changes:
- Implements a shared retry deadline, backoff, and selective error handling.
- Adds offline regression tests and integrates them into
make test. - Documents the retry policy and test command.
| File | Description |
|---|---|
build/scripts/install-bicep.sh |
Implements bounded download retries and preserves checksum safety. |
build/scripts/install-bicep_test.sh |
Tests retry classes, limits, timeouts, and installation safety. |
build/test.mk |
Adds the installer tests to the unit-test target. |
docs/contributing/contributing-code/contributing-code-shell-and-make/README.md |
Documents retry behavior and verification. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Seed a stale installed binary so Linux does not reuse a valid checksum fixture. Assert that the corrupt download is requested and rejected, and that the stale binary is preserved. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Fixed the Linux checksum fixture in a2f61d9. The test previously left a valid installed binary in place, so Linux reused it and never attempted the corrupt download. It now starts with a stale binary and checks that one download occurs, checksum verification fails, and the old binary remains unchanged. Reproduced the failure under Ubuntu before the change. After the change, CI result: run 38608297263 passed the full |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13291 +/- ##
==========================================
- Coverage 60.57% 60.56% -0.01%
==========================================
Files 771 771
Lines 45608 45608
==========================================
- Hits 27625 27624 -1
- Misses 17983 17984 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
github-actions
Bot
added
pr:waiting-for-author
Retry HTTP/2 stream resets and honor Retry-After seconds and HTTP dates within the existing shared deadline. Add coverage for server waits, redirect headers, date parsing, and bounded stream-reset retries. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
pr:waiting-for-review
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
github-actions
Bot
added
pr:review-approved
github-actions
Bot
removed
the
pr:review-approved
Summary
Extend the Bicep installer's download retry budget without retrying permanent errors or whole CI jobs. Add offline regression tests to
make test.Reason for change
Fixes #13141
In run 36507816832, three noncloud jobs exhausted five retries while the Bicep release download returned HTTP 500. That policy allowed about 31 seconds of retry delays. #13189 added
--retry-all-errors, which covers more error types but does not extend the HTTP 500 retry budget.This change gives each request a shared 300-second budget and up to 10 retries. Delays double from 1 second to a 60-second cap. Connection setup has a 15-second timeout. Each transfer has a timeout of at most 60 seconds, reduced to the remaining budget.
Retry HTTP 408, 429, 500, 502, 503, and 504, DNS and connection failures, timeouts, incomplete transfers, and dropped connections. Other failures, including HTTP 403/404, certificate errors, and local write errors, stop at once. The same budget covers a mix of HTTP and transport failures. Checksum validation still occurs before the installed binary is replaced.
A small Bash policy is needed here: curl's built-in
--retryomits dropped connections, while--retry-all-errorsalso retries permanent HTTP and certificate errors. The Go retry package cannot be used by this standalone Bash installer. A verified download cache is a possible later change, but needs separate cache storage and trust rules. Neither approach can guarantee success during a long upstream outage.How to test
bash -n build/scripts/install-bicep.sh build/scripts/install-bicep_test.sh- passed.bash build/scripts/install-bicep_test.sh- passed. Covers all retryable status classes, permanent errors, six HTTP 500 responses followed by success, increasing delays, shared time budget, attempt timeout, retry-count limit, latest-version resolution, partial output replacement, and checksum validation.for ($i = 1; $i -le 20; $i++) { bash build/scripts/install-bicep_test.sh; if ($LASTEXITCODE -ne 0) { throw "Bicep test run $i failed" }; Write-Output "PASS: $i/20" }- passed, all 20 runs.shellcheck --rcfile=.github/linters/.shellcheckrc build/scripts/install-bicep.sh build/scripts/install-bicep_test.sh- passed with ShellCheck v0.11.0.pnpm exec markdownlint-cli2 docs/contributing/contributing-code/contributing-code-shell-and-make/README.md --config .github/linters/.markdownlint-cli2.yaml- passed.pnpm exec markdown-table-formatter docs/contributing/contributing-code/contributing-code-shell-and-make/README.md --check- passed.cspell lint --config .github/linters/.cspell.yml --no-progress docs/contributing/contributing-code/contributing-code-shell-and-make/README.md- passed.git diff --check- passed.Also passed a local HTTP server check with real curl: six HTTP 500 responses then success; HTTP 404 with one attempt; HTTP 500 then HTTP 404 with no further attempt; a connection dropped during transfer then success with complete output; and persistent HTTP 500 stopped at the retry-count limit. The fixture replaces only the URL/protocol and sleep so the test needs no public service or long waits. The committed tests use a fake clock to check the time limits.
make test-install-bicepcould not run because GNU Make is not installed on this Windows host. Its exact Bash test command ran directly, including the 20-run loop. One full run takes about 19 seconds on this host, so the repeat check uses the 20-run minimum, not the 100-run target for fast tests. Earlier 100-run loops were stopped before completion and are not counted as passes. The full Go and cluster-based functional suites were not run; no Go code or cluster behavior changed.File change summary
build/scripts/install-bicep.shbuild/scripts/install-bicep_test.shbuild/test.mktest-install-bicepto the unit-test prerequisites.docs/contributing/contributing-code/contributing-code-shell-and-make/README.md