Repository navigation
fix(ci): cache the Argo CD Redis image for the KWOK deployer lanes - #2959
Conversation
The argocd-* KWOK lanes pull the Argo CD chart's Redis anonymously at job time from ECR Public, which sheds the request under the per-IP throttle the matrix already trips. Six lanes died on `429 Too Many Requests` for ecr-public.aws.com/docker/library/redis:8.2.3-alpine in run 36290149406, and every scheduled run since 2026-09-25 has been red the same way. #2497 removed the two images install-infra.sh preloads from the critical path; this covers the one the deployer stack pulls for itself, out of the same ECR Public namespace the registry image was already cached from. The chart's quay.io and ghcr.io images are left to the kubelet, neither having been observed shedding a pull. Follows the #2483 machinery: prime-images pulls once per run and caches the tarball, each argocd-* lane restores and loads it, and install_argocd side- loads it into the Kind node before Helm's 5m --wait. The chart renders one distinct Redis ref with redis-ha off, and the argocd-redis container is imagePullPolicy: IfNotPresent, so the side-load is what the kubelet uses. The pin carries no renovate annotation on purpose. testing_tools is renovate-managed, so an annotated entry would be bumped to the newest Redis independently of the chart, caching an image nothing pulls and restoring the 429. Nothing gates the pin against the chart, so a failed install renders the chart and reports the mismatch by name instead of leaving the next reader to re-derive it from an ImagePullBackOff. That render is bound by timeout: it re-fetches the chart the install just failed on, and an unbounded stall there would replace exit 30 with a job cancellation. Pinning the ref surfaces ecr-public.aws.com to tools/registry-inventory, which gates structured sources only. This is not new egress: the chart has always pulled that host in-cluster, but as a rendered chart image it sat outside the gate's scope. Allowlisted with that justification, and docs/contributor/registry-egress.md regenerated to match. kwok/README.md described the cache as covering two images; it now covers three, with the Redis pin's provenance and the per-lane restore gating. Fixes: #2501 Signed-off-by: Mark Chmarny <mark@chmarny.com>
mchmarny
added
the
theme/ci-dx
|
🌿 Preview your docs: https://nvidia-preview-fix-2501-cache-argocd-redis-image.docs.buildwithfern.com/aicr |
Coverage Report ✅
Coverage BadgeNo Go source files changed in this PR. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: NVIDIA/aicr/.coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (10)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe change adds an Argo CD Redis image pin and preloads that image before Argo CD installation. It adds Redis to KWOK image-cache priming and configures Argo CD deployers to restore and load the cached image. After a failed Helm install, the script can render the chart and report whether its Redis image matches the configured pin. Registry inventory, allowlist entries, tests, and documentation now include the image or its host. Priority: ⬆️ High Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The cache wiring is consistent and no merge-blocking issue was identified. Confirm that the pinned Redis image matches the chart and that post-merge KWOK runs avoid the reported pull failures. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Comment |
Summary
Cache the Argo CD chart's Redis image for the KWOK
argocd-*deployer lanes, so they stop pulling it anonymously from ECR Public at job time and shedding on the per-IP throttle.Motivation / Context
The
argocd-*lanes install the Argo CD chart, which pullsecr-public.aws.com/docker/library/redis:8.2.3-alpinein-cluster at job time. That is the same throttle bucketregistry_imagealready hit: we cached one image out of that namespace and left another one in it.Six lanes died on this in run 36290149406:
This has escalated since the issue was filed. It was 3 of 127 lanes on 2026-08-31; every scheduled KWOK run since 2026-09-25 has been red the same way.
#2497 removed the two images
install-infra.shpreloads from the job-time critical path. This is the uncovered scope: the image the deployer stack pulls for itself.Fixes: #2501
Related: #2483, #2497, #2496, #843
Type of Change
Component(s) Affected
cmd/aicr,pkg/cli)cmd/aicrd,pkg/server)pkg/recipe)pkg/bundler,pkg/component/*)pkg/collector,pkg/snapshotter)pkg/validator)pkg/errors,pkg/k8s)docs/,examples/) —kwok/README.mdand the regenerateddocs/contributor/registry-egress.mdkwok/scripts,.github/workflows/kwok-recipes.yaml,.github/actions/kwok-test) and the egress allowlist (tools/registry-inventory/registry-allowlist.yaml)Implementation Notes
Option 1 from the issue. Cache the one demonstrated failure, reusing the
#2483machinery as it already exists.prime-imagespulls once per run and saves a tarball,actions/cachecarries it, eachargocd-*lane restores anddocker loads it, andinstall_argocdcallspreload_imagebefore the Helm install sokind loadside-loads it into the node. The kubelet then never contacts ECR Public.The chart's other two images (
quay.io/argoproj/argocd,ghcr.io/dexidp/dex) are deliberately left alone — option 3 in the issue — since neither has been observed shedding a pull.The pin carries no
renovate:annotation, on purpose.testing_toolsis renovate-managed, so an annotatedargocd_redis_imagewould be bumped to the newest Redis independently ofargocd_chart— caching an image the chart never pulls and silently restoring the 429. Its only correct value is whatever the pinned chart renders.Drift is reported, not gated. The issue defers the full freshness gate to option 2, and building it before we know it is needed means paying for it early. Instead, a failed Argo CD install renders the chart and compares, so the next reader sees the cause named rather than re-deriving it from an
ImagePullBackOff:This is not hypothetical: the chart changed Redis tags between 9.0.0 and 9.5.x, and changed the host between 8.x and 9.x.
That check re-fetches the chart the install just failed on, and an unreachable repo is one of the reasons it fails — so it is bounded by
timeoutand skipped outright iftimeoutis unavailable. Unbounded, a stalled fetch would replace the caller'sexit 30with a job cancellation and lose the exit-code taxonomyrun-all-recipes.shbranches on.One trade-off worth naming. The
argocd-gitlane now runs threepreload_imagecalls in series (registry, Redis, Gitea). Each carries its own 180s budget, so a broad registry outage could in principle spend 540s on preload retries inside the 20-minute job ceiling, before Helm's 5m--wait. In practice the budget is only consumed on retry: with a warm cachepreload_imagefinds the image already on the host and returns immediately. Sharing one deadline across a lane's preloads, the wayKWOK_SYNC_DEADLINE_EPOCHalready does for the sync gate, would be the tightening if this ever bites.Adding an image to the cache is three edits (
IMAGE_CACHE_IMAGES, theprime-imagesmatrix, thekwok-testrestore/load steps). Nothing gates that today, so the requirement is now stated at the array.Needs a CODEOWNER look at
registry-allowlist.yaml. Pinning the ref trippedtools/registry-inventory, which gates egress hosts reachable from structured sources. Worth being precise about what that means here: this is not new egress. The Argo CD chart has always pulled this host in-cluster; as a rendered chart image it sat outside the gate's scope. Pinning it in.settings.yamlis what makes it visible, so the allowlist entry records egress that was already happening.It is also a genuinely distinct host from the already-allowlisted
public.ecr.aws— the same AWS ECR Public registry under a second hostname. The pin has to match what the chart renders exactly, or the preloaded image is not the one the kubelet asks for and the cache is inert.docs/contributor/registry-egress.mdis regenerated bymake registry-docsto match (26 → 27 hosts).kwok/README.md's "Public image cache" section described the cache as covering exactly two images; it now enumerates all three in a table, records why the Redis pin is not independently versioned, and notes the per-lane restore gating.Testing
All 158 shell cases pass, shellcheck exits 0, yamllint is clean, and the seven non-Go lint targets pass.
make testpasses, and the pinnedgolangci-lint v2.13.2reports0 issuesacross./....Verified beyond compiling:
argo-cd 9.5.14. It pulls exactly three images; the cached one is the only ref on the throttled host.save→ removed it from the host →load. The restored image ID issha256:08ad0b1d2808..., byte-identical to the digest in the CI 429 error.IMAGE_CACHE_IMAGES, the workflow matrix, and the action's restore/load steps all resolve to the same names and cache keys.9.5.14matches the pin,9.0.0rendersredis:7.2.11-alpine,8.0.0renders a different host.No Go files changed, so the coverage gate is unaffected.
One note for anyone reproducing locally: a Homebrew-upgraded
golangci-lint v2.14.0reports fourgosec G703findings inpkg/ociandtools/k8s-aibom-testthat the pinnedv2.13.2does not (G703ships in 2.14.0's gosec bundle). They are identical on a cleanmaintree and unrelated to this change.make tools-checkrenders golangci-lint as ✓ despite the mismatch, so checkgolangci-lint --versionagainst.settings.yamlbefore chasing them.The issue's second acceptance criterion — several consecutive green Tier 3 runs with no 429 — can only be confirmed after merge.
Risk Assessment
CI-only; no product code changes. The cache is best effort on the consuming side: a miss warns and falls through to
preload_image, then to the kubelet, which is today's behavior. Theprime-imagesgate is the one hard failure, matching the existingregistryandgiteaentries. A first run after this merges primes a cold cache.Rollout notes: N/A
Checklist
make testwith-race)make lint) — Go half verified with the pinnedv2.13.2, not the drifted local binary; see the note abovegit commit -S)