fix(ci): pass image_tag to nightly deploy tests - #12637
Conversation
The nightly deploy-test jobs added in #12090 call shared-deploy-test.yml with an `image_suffix` input, which that workflow does not define. Its required input is `image_tag`. GitHub validates reusable-workflow inputs when it instantiates the caller, so the entire nightly workflow was rejected at startup and no jobs ran (run 30894125332, startup_failure). Nothing on the PR could catch it: nightly-ci.yml never runs on a PR. The value was wrong as well. `format('{0}-nightly', <fw>-build.outputs. target_tag_plain)` omits the `<version>-ci-<sha>-` prefix that shared-copy.yml prepends when it publishes to ACR, so the deploy tests would have failed to pull the image even with the input named correctly. Consume `<fw>-copy-to-acr.outputs.image_tag` instead, which is the tag actually pushed to ACR and what post-merge-ci.yml already passes. `<fw>-build` drops out of `needs` since `<fw>-copy-to-acr` requires it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Tushar Sharma <tusharma@nvidia.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| framework: vllm | ||
| profiles: '["agg"]' | ||
| image_suffix: ${{ format('{0}-nightly', needs.vllm-build.outputs.target_tag_plain) }} | ||
| image_tag: ${{ needs.vllm-copy-to-acr.outputs.image_tag }} |
There was a problem hiding this comment.
🔍 Nightly copy job derives source tag from github.sha, not resolve-source-sha
The deploy tests now consume vllm-copy-to-acr.outputs.image_tag, which shared-copy.yml builds as <version>-ci-${{ github.sha }}-<target_tag_plain> (.github/workflows/shared-copy.yml:71, .github/workflows/shared-copy.yml:90). The nightly builds, however, check out needs.resolve-source-sha.outputs.source_sha rather than github.sha. If resolve-source-sha ever resolves to a commit other than the scheduled run's github.sha (e.g. pinning the last green commit), the copy step will look for an ECR tag that was never pushed and the copy — and therefore the deploy test — will fail. Worth confirming that the nightly build image tag really is keyed on github.sha; this is pre-existing but now sits directly on the deploy-test path.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughThe nightly workflow now makes vLLM, SGLang, and TensorRT-LLM deployment tests depend on their copy-to-ACR jobs. Each test receives the copied job’s ChangesNightly deployment image handoff
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
|
🎯 Code Coverage (details) 🔗 Commit SHA: 20e5c50 | Docs | Datadog PR Page | Give us feedback! |
|
resolved via: #12642 |
1 participant
Summary
The nightly pipeline has not run since #12090 merged. Run 30894125332 ended in
startup_failure— GitHub rejected the workflow file, so no jobs ran at all, not just the deploy tests:Two bugs, one hiding behind the other:
Wrong input name. The three
deploy-test-*jobs passimage_suffixtoshared-deploy-test.yml, which defines no such input — its required input isimage_tag. GitHub validates reusable-workflow inputs when it instantiates the caller, which is why this takes down the whole file. All three jobs have the defect; GitHub only reports the first.Wrong value.
format('{0}-nightly', <fw>-build.outputs.target_tag_plain)omits the<version>-ci-<sha>-prefix thatshared-copy.ymlprepends when it publishes to ACR. Even with the input named correctly, the deploy tests would have failed on image pull.Both are fixed by consuming
<fw>-copy-to-acr.outputs.image_tag— the tag actually pushed to ACR — which is whatpost-merge-ci.yml:877already does.<fw>-builddrops out of eachneedslist because<fw>-copy-to-acralready depends on it.Nothing on the original PR could have caught this: reusable-workflow input contracts are only validated when the calling workflow is instantiated, and
nightly-ci.ymlnever runs on a PR. Adding anactionlintpre-commit hook would catch this class statically — it reproduces the exact GitHub error offline — and will follow in a separate PR.Validation
actionlinton the branch reports 0workflow-callerrors innightly-ci.yml; onmainit reports 6 (theimage_tag-required andimage_suffix-undefined pair for each of the vllm/sglang/trtllm jobs), matching GitHub's startup failure verbatim.post-merge-ci.ymldeploy-test callers.startup_failure.🤖 Generated with Claude Code
Summary by CodeRabbit