[bazel] Improve job reliability and ensure jobs are time-bound - #5925
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The changes are consistent across CI configuration and documentation, and the enforcement/timeouts directly address the stated reliability goals without introducing apparent functional risk.
Pull request overview
Improves Bazel CI reliability by enforcing gRPC remote cache usage, adding explicit job-level timeouts to keep failures time-bounded, and aligning tooling with a Bazel 8.8.0 upgrade.
Changes:
- Enforce
grpcs://remote-cache URLs in CI setup and document the rationale/tradeoffs of gRPC vs HTTP remote caching. - Add configurable per-job
timeout-minutesto Bazel CI workflows (with longer limits for scheduled runs). - Upgrade Bazel to 8.8.0 and refresh the Bazel module lockfile accordingly.
File summaries
| File | Description |
|---|---|
MODULE.bazel.lock |
Updates the bzlmod lockfile to the newer format/version and refreshed dependency/module metadata for the Bazel upgrade. |
docs/bazel.md |
Adds documentation explaining why CI uses the gRPC remote cache protocol and how timeouts/retries behave. |
.github/workflows/bazel-ci.yml |
Introduces workflow-call inputs for job timeouts and applies timeout-minutes to all Bazel CI jobs. |
.github/workflows/bazel-ci-smoke.yml |
Adds explicit timeout-minutes to smoke-test jobs to keep them bounded. |
.github/workflows/bazel-ci-scheduled.yml |
Configures longer timeout inputs for scheduled runs and disables the remote cache for broader scheduled coverage. |
.github/actions/setup-bazel/configure-bazelrc.sh |
Enforces grpcs:// for remote-cache URLs when remote cache is enabled in CI. |
.bazelversion |
Upgrades the pinned Bazel version to 8.8.0. |
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| default: 5 | ||
|
|
||
| env: | ||
| FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true |
| BAZEL_REMOTE_CACHE_URL="${BAZEL_REMOTE_CACHE_URL:-}" | ||
| BAZEL_REMOTE_CACHE_AUTH_HEADER="${BAZEL_REMOTE_CACHE_AUTH_HEADER:-}" | ||
|
|
||
| if [[ "$BAZEL_REMOTE_CACHE_ENABLED" == "true" && -n "$BAZEL_REMOTE_CACHE_URL" && -n "$BAZEL_REMOTE_CACHE_AUTH_HEADER" ]]; then |
There was a problem hiding this comment.
If BAZEL_REMOTE_CACHE_ENABLED but the AUTH is empty, we run cache-less without warning. Is that intended? It was true before this PR therefore no change required.
There was a problem hiding this comment.
Added a GitHub Actions warning when remote cache is enabled but auth is absent.
| command can make multiple RPCs. Retry delays and other build work also add time. | ||
| Use command and job timeouts to set a limit for the complete CI operation. | ||
|
|
||
| Pre-merge jobs use these job-level limits: |
There was a problem hiding this comment.
This is true today but the limits for smoke are specified in a separate place, might diverge. Perhaps point that out, either here or as inline comments? No change required.
There was a problem hiding this comment.
Removed the duplicated timeout values from this document. The workflow files are now the source of truth for their configured values.
| steps: | ||
| - uses: actions/checkout@v5 | ||
| - name: Run E2E smoke test with Bazel-built binary | ||
| uses: ./.github/actions/run-monitored-tmpnet-cmd |
There was a problem hiding this comment.
The "Run" step inside run-monitored-tmpnet-cmd sets continue-on-error, so the logs etc are uploaded on failure. If the job is canceled instead by the timeout set here, the logs etc will have to upload within the grace period, which might be tight.
Perhaps the timeout should go in the "Run" step, but that requires an additional parameter etc, don't like it.
Perhaps ok to leave as is: when the failure is due to this timeout, maybe we don't care about the logs from the tmpnet run.
No change required.
There was a problem hiding this comment.
I'm not yet convinced we're going to see enough timeouts where we want to see log output, and we can revisit this should it become a pressing concern.
There was a problem hiding this comment.
Perhaps add the new grpcs:// requirement?
Recent job failures caused by cache stalls were corrected by switching from HTTP to gRPC transport for cache requests. Effecting this transition did not involve a PR, but suggested the following follow-on changes: - Enforce the use of gRPC in CI to prevent accidental misconfiguration - Document the rationale for gRPC over HTTP - Add timeouts to Bazel CI jobs to ensure the impact of future failures is time-bound - Upgrade to Bazel 8.8.0, the latest in the 8.x series Change-Id: I175ca05d6bdd1faa35457e1e8a1b30f8518f4c86
Change-Id: I1a3b71bfbd768c84bdfd6911acbcd129d839d3bb
c3f5d81 to
be8c692
Compare
Why this should be merged
Recent job failures caused by cache stalls were corrected by switching from HTTP to gRPC transport for cache requests. Effecting this transition did not involve a PR, but suggested the following follow-on changes: