ci(deploy-test): align vCluster Kubernetes with host and remove readiness workaround - #12279
Conversation
WalkthroughThe setup action now defaults vCluster Kubernetes versions to ChangesvCluster Version Alignment
Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/actions/setup-dynamo-operator/action.yml:
- Around line 50-52: Remove the default value from the Kubernetes version input
in the setup-dynamo-operator action, and update every caller to pass an explicit
supported version aligned with its host cluster. Keep the input optional only if
the action’s existing validation permits omission; otherwise make it required so
no call site silently inherits a version.
🪄 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: 42f6b3a2-b3d7-4b69-ac65-763bbcf86841
📒 Files selected for processing (1)
.github/actions/setup-dynamo-operator/action.yml
|
🎯 Code Coverage (details) 🔗 Commit SHA: 187aacb | Docs | Datadog PR Page | Give us feedback! |
Signed-off-by: Saravana Periyasamy <saperiyasamy@nvidia.com>
d1a8c05 to
187aacb
Compare
dagil-nvidia
left a comment
There was a problem hiding this comment.
Approving. Green across 81 checks, and this removes the CI workaround rather than adding one.
The chart-surface removal is safe: dynamo-operator.webhook.service.publishNotReadyAddresses was introduced yesterday in #12278 and is not in release/1.3.0, so it never shipped and nothing can depend on it. Better to take this before the v1.4.0 cut than to ship a production chart value that exists only to paper over a CI environment bug.
One follow-up, not a blocker: the previous code carried an explicit constraint that the vCluster guest version must be supported by the kr8s Python library, deliberately independent of the host. Deriving it from the host couples them, so a future host upgrade past kr8s support will surface as confusing deploy-test failures rather than an unsupported-version error. Worth confirming kr8s covers 1.34, and considering an upper bound or supported-version allowlist.
Summary
vMAJOR.MINOR.PATCHimage tag.publishNotReadyAddressesoverrides for the Dynamo and Grove webhook Services, including the chart value and its dedicated unit test.Fixes #12266.
Root cause
The deploy-test vCluster guest was pinned to Kubernetes
v1.32.13while the host had moved tov1.34.9. That skew triggers loft-sh/vcluster#3578: the syncer can fail to write Pod status becausestatus.qosClassis immutable, leaving guest Pods stale even when their host Pods are running.The old static pin could drift again on the next host upgrade. This change reads the host's
serverVersion.gitVersionand uses its upstream semantic version for the vCluster control-plane image.Validation
4830f0c7ad, which includes fix(ci): remove Grove mutation assertion #12399.vcluster_k8s_versioninput.publishNotReadyAddresses.187aacbf02:deploy-operator, both operator jobs, and all eight vLLM/SGLang/TRT-LLM deploy variants are green.The first full-CI attempt hit the self-hosted runner's 30-minute cutoff in an unrelated TRT-LLM parallel GPU test; the same job failed concurrently on PR #12363. A failed-jobs rerun passed without code changes.
The full platform Helm dependency build could not complete locally because the external NATS chart download stalled; the component chart used by this change was linted and rendered directly.
Out of scope