chore(efa-device-plugin): automate image tag bumps - #1367
Draft
Pavani-Panakanti wants to merge 1 commit into
Draft
chore(efa-device-plugin): automate image tag bumps#1367Pavani-Panakanti wants to merge 1 commit into
Pavani-Panakanti wants to merge 1 commit into
Conversation
Pavani-Panakanti
force-pushed
the
efa-chart-image-tag-automation
branch
9 times, most recently
from
September 11, 2026 01:28
07713b5 to
2691001
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings affect workflow execution and image validation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Automates EFA device-plugin image tag updates alongside instance-type refreshes, validating replicated multi-architecture tags before updating chart metadata.
Changes:
- Adds ECR tag discovery and readiness validation.
- Updates chart values, metadata, README, and version.
- Integrates both refresh scripts into the scheduled workflow with linting and version checks.
File summaries
| File | Findings |
|---|---|
hack/update-efa-image-tag.sh |
Moderate (3 votes): validate each region’s manifest index. Moderate (1 vote): surface registry evaluation errors. Moderate (1 vote): inspect all tags newer than the current tag rather than only the newest 10. |
.github/workflows/update-eks-chart.yaml |
Critical (1 vote): fix non-root installation to /usr/local/bin. Moderate (2 votes): prevent partial changes after instance refresh failures. Moderate (1 vote): install and invoke the pinned Helm version. |
Review details
Suppressed comments (3)
.github/workflows/update-eks-chart.yaml:63
- This newly added step invokes Helm, but this workflow only installs yq. The repository's Helm-using CI and release workflows first run
make install-toolchain, which installs the pinned Helm binary underbuild/tools; without equivalent setup here, the scheduled job depends on whatever happens to be preinstalled onubuntu-latestand can fail or use an unpinned version. Install and invoke the repository's pinned Helm before linting.
- name: Lint the modified chart
run: helm lint stable/aws-efa-k8s-device-plugin
hack/update-efa-image-tag.sh:246
- The
EVALUATION_ERRORScheck only runs when no candidate was accepted. If a newer candidate's registry call fails, the loop can then accept the current or an older ready tag and either update to a non-newest tag or reach the no-op path and report success, hiding the failed read. Treat evaluation errors as a failed run before accepting a candidate, or at least before the no-op path, so the scheduled job retries instead of masking a registry failure.
if [ "$EVALUATION_ERRORS" -gt 0 ]; then
log_error "could not evaluate any candidate tag: ${EVALUATION_ERRORS} registry call(s) failed"
exit 1
hack/update-efa-image-tag.sh:139
- Truncating the candidates to the newest 10 can skip an adoptable tag that is still newer than the chart. For example, if 11 tags were published after the pinned tag and the newest 10 are single-arch or still replicating, a ready 11th tag is omitted and the script exits without updating. Walk all tags newer than
CURRENT_TAG(or continue back to the pinned tag) so readiness failures cannot hide a valid upgrade.
mapfile -t CANDIDATE_TAGS < <(printf '%s\n' "${ALL_TAGS[@]}" | tail -n "$MAX_CANDIDATES")
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
33
to
34
| wget https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/yq_${YQ_PLATFORM} -O /usr/local/bin/yq &&\ | ||
| chmod +x /usr/local/bin/yq |
|
|
||
| - name: Run ./hack/update-efa-instance-types.sh | ||
| id: instance-types | ||
| continue-on-error: true |
Comment on lines
+217
to
+221
| fetch_manifest "$region" "imageTag=${tag}" "${MANIFEST_INDEX_TYPES[@]}" || rc=$? | ||
| if [ "$rc" -ne 0 ]; then | ||
| [ "$rc" -eq 2 ] && echo " ${tag}: index has not replicated to ${region} yet" | ||
| return 1 | ||
| fi |
The EFA device plugin ships to customers through this chart only, so a new eksbuild image is not actually consumable until image.tag is bumped here. That bump is manual today and gets missed, which leaves the chart pointing at an image with CVEs that were already patched upstream. Adds hack/update-efa-image-tag.sh and calls it from the existing EFA workflow, so one PR carries both the instance type refresh and the tag bump and there is a single writer for the chart version field. It runs before the instance type refresh, and that refresh is non-fatal, so a failure in the cosmetic update cannot discard a patch-currency bump. Accepted tag shapes are vX.Y.Z and vX.Y.Z-eksbuild.N, both of which this chart has pinned. Only rebuilds of the pinned plugin version are adopted; a newer base version is an upstream upgrade, so it is reported for a human instead. Before adopting a tag the script checks, per sampled region, that the tag resolves to an index advertising amd64 and arm64 and that every platform manifest the index references also resolves there. The build publishes to the home region well before replication finishes, and replication is per image with no ordering guarantee between an index and its children, so an index alone is not evidence the image is pullable. Checking child manifests is sufficient because ECR rejects a manifest whose layers are absent. Failure modes are loud. A registry read failure, an unrecognised chart tag, a README that has drifted from values.yaml, and a run where no candidate could be evaluated because calls were failing all exit non-zero with an annotation, rather than reporting that there is nothing to update. Genuine replication lag stays quiet and self-heals. Since PRs opened with GITHUB_TOKEN do not trigger ci.yaml, the job now lints the chart it rewrote. The script reads the registry with ecr:ListImages and ecr:BatchGetImage. The EKS registry's repository policy already permits both cross-account, so this needs no change in the registry account; the role behind UPDATE_CHARTS_AWS_ROLE_ARN just needs those two actions in its own identity policy.
Pavani-Panakanti
force-pushed
the
efa-chart-image-tag-automation
branch
from
September 11, 2026 02:09
2691001 to
c073bfe
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.
This PR adds automation for efa device plugin image tag updates. We have existing EFA workflow that updates any new efa supported instance types in the charts. PR adds hack/update-efa-image-tag.sh and calls it from the existing EFA workflow, so one PR carries both the instance type refresh and the tag bump and there is a single writer for the chart version field.
The script only adopts a tag once it is a multi-arch manifest list resolving in every sampled region. It never moves the tag backwards, and re-running with nothing new to pick up leaves the tree untouched.
Ran the script against the live registry. It selected
v0.5.21-eksbuild.14, which is published and replicated while master is onv0.5.21-eksbuild.13, and produced exactly the expected diff:values.yamlimage.tag,Chart.yamlappVersion, the README config table, and a chart version bumpv0.5.32tov0.5.33. This PR has automation only changesChart is already up to dateand leaves the chart version alone.update-efa-instance-types.shhaving already bumped the version plus changed an instance type, then ran the tag script. It applied the tag change and did not double bump the version.amd64andarm64in all four sampled regions.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.