[CI] Remove the unused ECR cache lookup from pipeline generation - #433
Open
Hoiyeuhng wants to merge 1 commit into
Open
[CI] Remove the unused ECR cache lookup from pipeline generation#433Hoiyeuhng wants to merge 1 commit into
Hoiyeuhng wants to merge 1 commit into
Conversation
Hoiyeuhng
marked this pull request as draft
July 22, 2026 20:57
Hoiyeuhng
force-pushed
the
fix/remove-unused-ecr-cache-resolution
branch
from
July 22, 2026 21:29
a8ecf47 to
c37b2af
Compare
Hoiyeuhng
marked this pull request as ready for review
July 22, 2026 21:40
Hoiyeuhng
force-pushed
the
fix/remove-unused-ecr-cache-resolution
branch
2 times, most recently
from
August 29, 2026 04:28
6544601 to
cd77146
Compare
Hoiyeuhng
force-pushed
the
fix/remove-unused-ecr-cache-resolution
branch
from
August 29, 2026 04:55
cd77146 to
fe577eb
Compare
`_get_variables_to_inject` injected `$CACHE_FROM` and `$CACHE_TO` into step commands, resolving them through `get_ecr_cache_registry()`. No step command consumes either placeholder: vLLM's image builds moved into `.buildkite/image_build/` (vllm-project/vllm#29343) and the scripts there compute and export their own BuildKit cache variables. The helper ran `aws ecr get-login-password`, `docker login` and up to two `docker manifest inspect` calls on every pipeline generation, and raised `RuntimeError` on authentication failure, so the dead path also carried a failure mode. Registry caching itself is untouched. Signed-off-by: Haiyang Luo <41023868+Hoiyeuhng@users.noreply.github.com>
Hoiyeuhng
force-pushed
the
fix/remove-unused-ecr-cache-resolution
branch
from
August 29, 2026 05:03
fe577eb to
1593a75
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
$CACHE_FROMand$CACHE_TOinto step commandsget_ecr_cache_registry()and the two helpers only it usedNet
+9 / -97. Registry caching behavior is unchanged.Why
_get_variables_to_inject()resolves the two placeholders throughget_ecr_cache_registry(), which runsaws ecr get-login-password,docker loginand up to twodocker manifest inspectcalls, then raisesRuntimeErrorif the login fails.Nothing consumes the result. In vLLM
main's.buildkite/tree the placeholders occur only insideimage_build/image_build.sh,image_build/image_build_cpu.shandscripts/ci-bake-rocm.sh, each of which computes its own values just before the build — true since vllm-project/vllm#29343 moved image builds under.buildkite/image_build/. Release branchesv0.28.0,v0.27.0andv0.9.2have no occurrences either.The other injected variables are still live, which is what isolates these two.
image_build.yamlconsumes five of the seven as positional arguments:So the cost today is less the round trip than that
_get_variables_to_inject()reads as though the generator still owns cache-tag resolution.Scope
bootstrap-intel.shhas its own bashresolve_ecr_cache_varsthat also looks unused, but it sits on a live bootstrap path whose caller is configured in Buildkite rather than in this repo, so I left it alone. Happy to follow up if you confirm it.Duplicate-work check
#394 changes
ci.hcland terraform rather than the generator, and #413 is explicitly foundation-only, so neither overlaps. #321 edits the same function, so whichever lands second needs a trivial conflict fix.Validation
pytest buildkite/tests— 111 passed on Python 3.12 and on 3.9 (the pipeline agents run 3.9), installed the way thePipeline generator testsworkflow does.mainbefore passing here.pre-commitclean on the changed files exceptruff-format, which reformatsbuildkite_step.pyandtest_step.pyidentically atmain— pre-existing, so left out of this diff.AI assistance
AI assistance was used to research, implement and test this change. The submitter has reviewed every changed line.