Skip to content

fix(ci): authenticate nightly vLLM image pulls - #357

Open
wenxind-nvidia wants to merge 3 commits into
mainfrom
fix/nightly-ngc-auth
Open

fix(ci): authenticate nightly vLLM image pulls#357
wenxind-nvidia wants to merge 3 commits into
mainfrom
fix/nightly-ngc-auth

Conversation

@wenxind-nvidia

Copy link
Copy Markdown
Collaborator

Summary

  • authenticate the nightly GPU job to nvcr.io with the existing NGC_API_KEY repository secret
  • isolate Docker credentials in a per-run DOCKER_CONFIG and remove them during always-run cleanup
  • fail before dependency setup when the key is absent or rejected instead of failing three model tests later
  • document why nightly CI must not depend on cached vLLM images or runner-global Docker state

Root cause

The nightly workflow did not map secrets.NGC_API_KEY into any step. The vLLM tests launch model wrappers directly, so the launcher credential cache is not involved. Run 31667652128 reached Docker with neither a local nvcr.io/nvidia/vllm:26.04-py3 image nor valid registry authentication, and all three vLLM-backed tests failed with Access Denied.

Closes #346

Validation

  • actionlint (excluding the existing custom-runner-label warning)
  • workflow YAML parse
  • valid-key login/cleanup shell-path harness with a fake Docker client
  • missing-key fail-fast/cleanup shell-path harness
  • pre-commit SPDX checks
  • git diff --check

@blongs-nv

Copy link
Copy Markdown
Contributor

Reviewed at 0778bf2c1042830ce3955f8ecdda6d3bb135d2e7 by a bot.

Blockers

  1. The docker login runs before the DOCKER_CONFIG export takes effect, so the PR does not fix the auth failure and leaks the NGC key onto the runner.

    .github/workflows/nightly-xr-ai-test.yml:95-103
    GITHUB_ENV only applies to subsequent steps, so the login in this step writes credentials to the runner-global ~/.docker/config.json, while every later step (including the pytest step whose subprocesses run docker pull/docker run) reads the empty per-run directory and pulls unauthenticated; the cleanup step then deletes an empty directory and the key stays on the shared self-hosted runner. I verified this in this PR's own green GPU run (job 94361740159): the auth step's env group has no DOCKER_CONFIG while every later step shows it set, so the run passed only because the vLLM image was already cached, the exact dependency the change claims to remove. Adding export DOCKER_CONFIG="$docker_config" before the login fixes both halves; update the changelog entry at the same time, since as written it describes the intended behavior, not the actual one.

Suggestions

  1. The broken login has already written the NGC key into the runner's global ~/.docker/config.json (this PR's pre-merge runs did it). Add docker logout nvcr.io || true to the cleanup step so the leaked entry gets removed on the next run.
  2. The hard exit 1 on an empty NGC_API_KEY (.github/workflows/nightly-xr-ai-test.yml:98-101) will fail every fork PR that edits this workflow, since secrets are not exposed to fork pull_request runs. Gate the step on non-fork events, or downgrade to a warning on pull_request.
  3. Even with the export fixed, a green pytest (gpu) run cannot distinguish working auth from a cached image. Consider asserting auth right after login, for example docker manifest inspect nvcr.io/nvidia/vllm:26.04-py3, so the login step fails loudly when the key is rejected.
  4. Move the empty-key guard above the directory creation and GITHUB_ENV write so a missing secret has no side effects, and add set -u to the cleanup step to match the sibling always-run purge step.
  5. The new required repository secret is undocumented. Add it to the workflow header's "Setup note" and to the NGC_API_KEY docs, and note in the changelog entry that the CI login deliberately bypasses the existing _maybe_ngc_login() path in xr_ai_vllm/_docker.py (whose _already_logged_in() reads ~/.docker/config.json and ignores DOCKER_CONFIG), so a future reader doesn't reconcile the two mechanisms the wrong way.

The diagnosis in the PR is right and the shape of the fix (per-run Docker config, secret scoped to one step, always-run cleanup) is the correct instinct. The blocker above should be resolved before merge.

@yanziz-nvidia

Copy link
Copy Markdown
Collaborator

Can you make it so that it won't skip the test when you touch .github/workflows/nightly-xr-ai-test.yml

Signed-off-by: Wenxin Ding <wenxind@nvidia.com>
Signed-off-by: Wenxin Ding <wenxind@nvidia.com>
github-actions Bot added a commit that referenced this pull request Aug 14, 2026
Signed-off-by: Wenxin Ding <wenxind@nvidia.com>
github-actions Bot added a commit that referenced this pull request Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nightly XR AI test is failing

3 participants