refactor(operator): switch DGD and DCD admission to v1beta1 for 1.5 - #11177
Merged
sttts merged 3 commits intoAug 10, 2026
Conversation
sttts
temporarily deployed
to
external_collaborator
July 2, 2026 13:52 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
sttts
had a problem deploying
to
external_collaborator
July 2, 2026 13:52 — with
GitHub Actions
Failure
GitHub Actions
Failure
github-actions
Bot
added
refactor
deployment::k8s
|
🎯 Code Coverage (details) 🔗 Commit SHA: bdc44ab | Docs | Datadog PR Page | Give us feedback! |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
sttts
temporarily deployed
to
external_collaborator
July 9, 2026 16:35 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
|
/ok to test 7410e0a |
sttts
changed the title
Contributor
WalkthroughThis PR removes v1alpha1 admission webhook support for DynamoGraphDeployment, consolidating defaulting and validation webhooks onto a single v1beta1 endpoint. The Helm chart webhook configuration is updated to reference v1beta1 paths and API versions, and conversion helpers plus related v1alpha1 tests are deleted. Changesv1alpha1 webhook removal and v1beta1 consolidation
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
deploy/operator/internal/webhook/defaulting/dynamographdeployment_handler.go (1)
76-96: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMove nil-
Replicasdefaulting ahead of the admission-request fetch.The replica default is described as preventing a controller nil-pointer panic and must apply "on every operation," yet it now sits after the
RequestFromContextcall, whose error branch (Line 79) returnsnilearly. If the request can't be extracted, the object is admitted withnilReplicas and the panic the comment warns about can still occur downstream. The request is only needed for the CREATE-time annotation stamping, so run the component defaulting first, then fetch the request solely to gate the stamping.🛠️ Proposed reordering
- req, err := admission.RequestFromContext(ctx) - if err != nil { - logger.Error(err, "failed to get admission request from context, skipping defaulting") - return nil - } - // Default nil replicas to 1 for all components. The Replicas field is // *int32 with omitempty, so users can legally omit it. Without this // default the controller panics on a nil pointer dereference in // expandRolesForComponent(). Apply on every operation so that components // added via UPDATE also get the default. grovePathway := d.isGrovePathway(dgd) for i := range dgd.Spec.Components { component := &dgd.Spec.Components[i] if component.Replicas == nil { component.Replicas = ptr.To(int32(1)) } if grovePathway && component.MinAvailable == nil { component.MinAvailable = ptr.To(int32(1)) } } + req, err := admission.RequestFromContext(ctx) + if err != nil { + logger.Error(err, "failed to get admission request from context, skipping origin-version stamping") + return nil + } + if req.Operation == admissionv1.Create {🤖 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 `@deploy/operator/internal/webhook/defaulting/dynamographdeployment_handler.go` around lines 76 - 96, Move the nil-Replicas defaulting in dynamographdeploymentHandler.Default around the RequestFromContext logic so it always runs first, before any early return from admission request lookup. Keep the component loop that initializes Replicas (and grovePathway MinAvailable) unconditional, then call admission.RequestFromContext(ctx) only for the CREATE-time annotation stamping path so a missing request cannot bypass the safety default.deploy/helm/charts/platform/components/operator/templates/webhook-configuration.yaml (1)
44-62: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUpdate the DCD validating webhook to use the new v1beta1 route.
deploy/operator/internal/webhook/validation/dynamocomponentdeployment_handler.gostill registers/validate-nvidia-com-v1alpha1-dynamocomponentdeploymentand gates onnvidiacomv1alpha1.DynamoComponentDeploymentGVK, while the Helm config now sends requests to/validate/nvidia.com/v1beta1/dynamocomponentdeployments. That mismatch will break CREATE/UPDATE admission fordynamocomponentdeployments.🤖 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 `@deploy/helm/charts/platform/components/operator/templates/webhook-configuration.yaml` around lines 44 - 62, The validating webhook route and version gate are still on the old v1alpha1 path, so update `dynamocomponentdeployment_handler.go` to register and handle the new `/validate/nvidia.com/v1beta1/dynamocomponentdeployments` route. Make the handler use the v1beta1 `DynamoComponentDeployment` GVK and ensure the webhook registration, path matching, and admission logic all align with the Helm `webhook-configuration` template so CREATE/UPDATE requests reach the correct validator.
🤖 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.
Outside diff comments:
In
`@deploy/helm/charts/platform/components/operator/templates/webhook-configuration.yaml`:
- Around line 44-62: The validating webhook route and version gate are still on
the old v1alpha1 path, so update `dynamocomponentdeployment_handler.go` to
register and handle the new
`/validate/nvidia.com/v1beta1/dynamocomponentdeployments` route. Make the
handler use the v1beta1 `DynamoComponentDeployment` GVK and ensure the webhook
registration, path matching, and admission logic all align with the Helm
`webhook-configuration` template so CREATE/UPDATE requests reach the correct
validator.
In
`@deploy/operator/internal/webhook/defaulting/dynamographdeployment_handler.go`:
- Around line 76-96: Move the nil-Replicas defaulting in
dynamographdeploymentHandler.Default around the RequestFromContext logic so it
always runs first, before any early return from admission request lookup. Keep
the component loop that initializes Replicas (and grovePathway MinAvailable)
unconditional, then call admission.RequestFromContext(ctx) only for the
CREATE-time annotation stamping path so a missing request cannot bypass the
safety default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a8509ad0-004e-48a5-b913-bfe44f1b016c
📒 Files selected for processing (6)
deploy/helm/charts/platform/components/operator/templates/webhook-configuration.yamldeploy/operator/internal/webhook/defaulting/dynamographdeployment_handler.godeploy/operator/internal/webhook/defaulting/dynamographdeployment_handler_test.godeploy/operator/internal/webhook/dynamographdeployment_conversion.godeploy/operator/internal/webhook/validation/dynamographdeployment_handler.godeploy/operator/internal/webhook/validation/dynamographdeployment_handler_test.go
💤 Files with no reviewable changes (3)
- deploy/operator/internal/webhook/dynamographdeployment_conversion.go
- deploy/operator/internal/webhook/validation/dynamographdeployment_handler_test.go
- deploy/operator/internal/webhook/defaulting/dynamographdeployment_handler_test.go
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
sttts
force-pushed
the
sttts-1.5-remove-v1alpha1-dgd-admission
branch
from
August 10, 2026 08:46
7410e0a to
69b1460
Compare
sttts
temporarily deployed
to
external_collaborator
August 10, 2026 08:46 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
|
/ok to test 69b1460 |
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
sttts
temporarily deployed
to
external_collaborator
August 10, 2026 08:54 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
|
/ok to test bdc44ab |
julienmancuso
approved these changes
Aug 10, 2026
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
3 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
/validate/nvidia.com/v1beta1/dynamocomponentdeployments.This does not remove the v1alpha1 CRD versions or the API conversion webhook.
Dependencies
Validation
GOCACHE=/private/tmp/dynamo-go-cache go test ./internal/webhook/...helm lint . --set discoveryBackend=kuberneteshelm template dcd-validation . --set discoveryBackend=kubernetes --show-only templates/webhook-configuration.yaml/validate/nvidia.com/v1beta1/dynamocomponentdeploymentswithapiVersions: [v1beta1].Summary by CodeRabbit