fix(kube): Update kube scripts to pull from ghcr - #513
Merged
matthewgrossman merged 8 commits intoJun 30, 2026
Merged
Conversation
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Changesk8s Secret Helper and Helm Wiring
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/k8s/scripts/create_secrets.sh`:
- Around line 26-35: Stop passing secret values through kubectl command
arguments in create_secrets.sh, since the ngc_key is currently exposed via
create secret generic and create secret docker-registry. Update the secret
creation flow to generate Secret manifests without embedding tokens in
argv—either pipe YAML/JSON to kubectl apply via stdin or write to a 0600 temp
file and apply it. Apply the same pattern to the other token-based secret blocks
referenced in the script, and use the existing helpers like log_info and
kubectl_ns only for non-sensitive command parts.
- Around line 39-42: The GHCR pull secret in create_secrets.sh is using the
wrong docker username for a GITHUB_TOKEN. Update the create_secrets.sh secret
creation command to use the actual GitHub account name (the Actions actor)
instead of x-access-token, and keep the existing ghcr-pull secret setup
consistent with the kubectl_ns/create secret docker-registry flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1df2534d-6675-408b-9b66-ccf8b261ee8d
📒 Files selected for processing (4)
e2e/k8s/scripts/create_secrets.she2e/k8s/scripts/setup_local_kind_cpu.she2e/k8s/scripts/setup_local_minikube_cpu.she2e/k8s/scripts/setup_local_minikube_gpu.sh
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/k8s/scripts/install_helm_e2e.sh (1)
227-232: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSplit
imagePullSecretsby backing credential.
create_platform_secretsonly createsnvcrimagepullsecretwhenNGC_API_KEYis set, but this block adds it wheneverGITHUB_TOKENis present. That can render pods with a nonexistent pull secret in GHCR-only setups. Gate each secret independently.Suggested fix
+pull_secret_index=0 + if [ -n "${GITHUB_TOKEN:-}" ]; then HELM_ARGS+=( - --set "imagePullSecrets[0].name=ghcr-pull" - --set "imagePullSecrets[1].name=nvcrimagepullsecret" + --set "imagePullSecrets[${pull_secret_index}].name=ghcr-pull" ) + pull_secret_index=$((pull_secret_index + 1)) +fi + +if [ -n "${NGC_API_KEY:-}" ]; then + HELM_ARGS+=( + --set "imagePullSecrets[${pull_secret_index}].name=nvcrimagepullsecret" + ) fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/k8s/scripts/install_helm_e2e.sh` around lines 227 - 232, The Helm argument setup in install_helm_e2e.sh is coupling both imagePullSecrets to GITHUB_TOKEN, which can add nvcrimagepullsecret even when create_platform_secrets never creates it. Update the HELM_ARGS block so each secret is gated independently: keep ghcr-pull tied to GITHUB_TOKEN, and only append nvcrimagepullsecret when the same condition used by create_platform_secrets for NGC_API_KEY is satisfied. Refer to the HELM_ARGS array construction and create_platform_secrets behavior to align the secret names with their backing credentials.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/k8s/scripts/lib.sh`:
- Around line 27-49: The helper in log/create secret flow creates ghcr-pull when
GITHUB_TOKEN is set, but nvcrimagepullsecret is only created inside the
NGC_API_KEY branch, so imagePullSecrets can reference a secret that does not
exist. Update the secret-creation logic in the script’s NGC/GHCR handling so
nvcrimagepullsecret is only wired when NGC_API_KEY is present, or otherwise
ensure that create secret docker-registry nvcrimagepullsecret is always executed
before any code paths that add it to imagePullSecrets.
---
Outside diff comments:
In `@e2e/k8s/scripts/install_helm_e2e.sh`:
- Around line 227-232: The Helm argument setup in install_helm_e2e.sh is
coupling both imagePullSecrets to GITHUB_TOKEN, which can add
nvcrimagepullsecret even when create_platform_secrets never creates it. Update
the HELM_ARGS block so each secret is gated independently: keep ghcr-pull tied
to GITHUB_TOKEN, and only append nvcrimagepullsecret when the same condition
used by create_platform_secrets for NGC_API_KEY is satisfied. Refer to the
HELM_ARGS array construction and create_platform_secrets behavior to align the
secret names with their backing credentials.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3818af6b-194b-4877-bdac-57d593e082cc
📒 Files selected for processing (5)
e2e/k8s/scripts/install_helm_e2e.she2e/k8s/scripts/lib.she2e/k8s/scripts/setup_local_kind_cpu.she2e/k8s/scripts/setup_local_minikube_cpu.she2e/k8s/scripts/setup_local_minikube_gpu.sh
…e-via-minikube Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
svvarom
self-requested a review
June 30, 2026 17:01
svvarom
reviewed
Jun 30, 2026
svvarom
reviewed
Jun 30, 2026
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
matthewgrossman
deleted the
mgrossman/aircore-857-gpu-testing-on-dev-blue-via-minikube
branch
June 30, 2026 18:34
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
2 participants
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
Add GHCR image pull support to the K8s e2e scripts, and consolidate duplicated code across setup scripts into a shared
lib.sh.Changes
New:
e2e/k8s/scripts/lib.shlog_info,log_warn,log_error) — previously duplicated in every setup scriptcreate_platform_secrets()— creates NGC, GHCR, and HuggingFace secrets when their corresponding env vars are sete2e/k8s/scripts/install_helm_e2e.shlib.shinstead of defining its own log functionsimagePullSecretsto the helm chart whenNGC_API_KEY(for nvcr.io) and/orGITHUB_TOKEN(for ghcr.io) are sete2e/k8s/scripts/setup_local_minikube_gpu.shlib.sh, removes duplicated log functions and secret creationcreate_platform_secretswhich now handles GHCR pull secret creatione2e/k8s/scripts/setup_local_minikube_cpu.she2e/k8s/scripts/setup_local_kind_cpu.shMotivation
When deploying NeMo Platform on dev-blue (or any environment using GHCR pre-built images), the scripts needed to:
ghcr-pullK8s secret for kubelet to authenticate to ghcr.ioimagePullSecretsto the helm chart so pods reference the secretnvcrimagepullsecretfor NIM container images from nvcr.ioThis logic was missing from the OSS scripts (it existed in Platform-Deploy for GitLab but was stripped during the port). The secret creation was also duplicated across 3 setup scripts, requiring identical changes in all 3 files.
Test plan
create_platform_secretscreates correct secrets when env vars are setNGC_API_KEYplaceholder)