Skip to content

fix(ci): tolerate stale vcluster webhook readiness - #12278

Merged
saturley-hall merged 2 commits into
mainfrom
fix/vcluster-webhook-readiness
Jul 28, 2026
Merged

fix(ci): tolerate stale vcluster webhook readiness#12278
saturley-hall merged 2 commits into
mainfrom
fix/vcluster-webhook-readiness

Conversation

@lavanyavijayk

@lavanyavijayk lavanyavijayk commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Overview:

Mitigates intermittent deploy/operator CI failures caused by stale Pod readiness synchronization
between vCluster and the host cluster. The operator webhook is healthy, but its endpoint can be
excluded from Service routing until the stale readiness state recovers.

Details:

  • Adds a configurable webhook.service.publishNotReadyAddresses Helm value, defaulting to false.

  • Enables the value only for short-lived vCluster CI environments.

  • Verifies that both the virtual and host webhook Services preserve the override.

  • Keeps real v1alpha1 and v1beta1 admission requests as the final health check.

  • Adds focused diagnostics for Pods, Services, Endpoints, EndpointSlices, operator logs, and
    vCluster syncer logs.

  • Adds Helm tests covering the production default and CI override.

Validation:

  • Helm lint passed.
  • All 33 Helm unit tests passed.
  • Pre-commit checks passed.
  • YAML parsing and composite-action shell syntax checks passed.

Where should the reviewer start?

Start with:

  • .github/actions/setup-dynamo-operator/action.yml for the CI-only override, verification, and
    diagnostics.

  • deploy/helm/charts/platform/components/operator/templates/webhook-service.yaml for the
    rendered Kubernetes Service setting.

  • deploy/helm/charts/platform/tests/webhook_service_test.yaml for default-safety and override
    coverage.

Related Issues

🚫 This PR is NOT linked to an issue:

  • Confirmed — no related issue

Open in Devin Review

Summary by CodeRabbit

  • New Features
    • Added configuration to control whether webhook endpoints are published before pods become ready.
    • Webhook endpoints remain unpublished by default, with an opt-in setting for testing and short-lived validation scenarios.
  • Documentation
    • Documented the new webhook readiness setting and recommended production default.
  • Tests
    • Added coverage confirming both default and enabled behaviors.
  • Bug Fixes
    • Improved operator readiness verification and deployment diagnostics.

Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
@lavanyavijayk
lavanyavijayk requested review from a team as code owners July 28, 2026 19:56
@github-actions github-actions Bot added fix documentation Improvements or additions to documentation deployment::k8s Relates to dynamo deployment in kubernetes actions labels Jul 28, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread .github/actions/setup-dynamo-operator/action.yml
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 36fa44b0-fd21-431a-84ae-15faebed1e10

📥 Commits

Reviewing files that changed from the base of the PR and between a96a2a8 and b09eaf0.

📒 Files selected for processing (6)
  • .github/actions/setup-dynamo-operator/action.yml
  • deploy/helm/charts/platform/README.md
  • deploy/helm/charts/platform/components/operator/templates/webhook-service.yaml
  • deploy/helm/charts/platform/components/operator/values.yaml
  • deploy/helm/charts/platform/tests/webhook_service_test.yaml
  • deploy/helm/charts/platform/values.yaml

Walkthrough

The Helm chart adds configurable webhook Service publication of not-ready endpoints. The setup action enables and verifies this setting, waits for controller-manager readiness, and collects expanded host and vCluster diagnostics on failure.

Changes

Webhook readiness configuration

Layer / File(s) Summary
Helm webhook Service configuration
deploy/helm/charts/platform/values.yaml, deploy/helm/charts/platform/components/operator/values.yaml, deploy/helm/charts/platform/components/operator/templates/webhook-service.yaml, deploy/helm/charts/platform/tests/webhook_service_test.yaml, deploy/helm/charts/platform/README.md
Adds publishNotReadyAddresses, renders it into the webhook Service, tests default and enabled values, and documents the setting.

Operator setup and diagnostics

Layer / File(s) Summary
Operator readiness and webhook verification
.github/actions/setup-dynamo-operator/action.yml
Enables not-ready webhook addresses, waits for controller-manager container readiness, and verifies the vCluster and host webhook Services.
Deployment failure diagnostics
.github/actions/setup-dynamo-operator/action.yml
Collects operator pod, webhook Service, endpoint, vCluster syncer, and vCluster-side diagnostic output on failure.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately describes the CI fix for stale vCluster webhook readiness.
Description check ✅ Passed The description follows the template well with Overview, Details, reviewer guidance, and a completed Related Issues section.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

@datadog-official

datadog-official Bot commented Jul 28, 2026

Copy link
Copy Markdown

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 27.06% (-4.67%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: dc99a8f | Docs | Datadog PR Page | Give us feedback!

@sara4dev sara4dev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are doing to unblock the CI release for now and do a proper fix later.

@saturley-hall
saturley-hall merged commit 329132a into main Jul 28, 2026
71 of 75 checks passed
@saturley-hall
saturley-hall deleted the fix/vcluster-webhook-readiness branch July 28, 2026 21:47
sara4dev added a commit that referenced this pull request Jul 29, 2026
This reverts commit 329132a.

TEMPORARY -- for experimental validation on this branch only, not
intended to merge as-is.

#12278 works around the vCluster guest/host K8s skew rather than fixing
it. Its own comment named the cause: "The vCluster syncer on AWS stalls
pod-Ready propagation due to a NATS qosClass reconciliation loop, so
kubectl wait against the virtual cluster never returns." That is
loft-sh/vcluster#3578, which this branch fixes at the root by aligning
the guest control plane to the host version.

Reverting the workaround here tests whether the root-cause fix makes it
unnecessary -- specifically whether the operator becomes Ready through
the vCluster API without host-namespace polling, and whether the webhook
Service routes without publishNotReadyAddresses=true.

If the deploy tests pass without it, #12278 can be reverted on main as
well. If they fail, the workaround is load-bearing for a reason beyond
the version skew and this commit must be dropped before merge.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Saravana Periyasamy <saperiyasamy@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

actions deployment::k8s Relates to dynamo deployment in kubernetes documentation Improvements or additions to documentation fix size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants