diff --git a/components/src/dynamo/profiler/utils/dgdr_v1beta1_types.py b/components/src/dynamo/profiler/utils/dgdr_v1beta1_types.py index a6d4d5f18f7f..12c3619f0b50 100644 --- a/components/src/dynamo/profiler/utils/dgdr_v1beta1_types.py +++ b/components/src/dynamo/profiler/utils/dgdr_v1beta1_types.py @@ -358,6 +358,10 @@ class DynamoGraphDeploymentRequestStatus(BaseModel): default=None, description="ObservedGeneration is the most recent generation observed by the controller.", ) + observedSpecFingerprint: Optional[str] = Field( + default=None, + description="ObservedSpecFingerprint identifies the spec associated with ObservedGeneration. The controller uses it to verify runtimeVersionOverride-only repairs.", + ) class DynamoGraphDeploymentRequest(BaseModel): diff --git a/deploy/helm/charts/platform/README.md b/deploy/helm/charts/platform/README.md index 08a266769641..e063b3acd50b 100644 --- a/deploy/helm/charts/platform/README.md +++ b/deploy/helm/charts/platform/README.md @@ -69,11 +69,12 @@ Admission now requires every component's main-container image. In `v1beta1`, set required by Dynamo admission, but were effectively required: Kubernetes rejects the rendered Pod specification when its main container has no image. -After upgrading the CRDs and operator, admission denies a new DGD, or an update to a pre-existing -DGD, when a component's main-image tag is not a semantic version and `runtimeVersionOverride` is -unset. This includes custom and SHA-tagged images. Set `runtimeVersionOverride` to the Dynamo -runtime compatibility version for that image before creating or updating the DGD. Existing resources -are not changed or revalidated solely by the upgrade. +After upgrading the CRDs and operator, admission denies a new DGD, DCD, or DGDR when a component's +main-image tag is not a semantic version and `runtimeVersionOverride` is unset. This includes custom +and SHA-tagged images. On updates, this requirement is ratcheted: a pre-existing resource with an +unchanged non-semantic-version image remains admissible without backfilling the override. Changing +the image to a non-semantic-version tag requires setting `runtimeVersionOverride` to that image's +Dynamo runtime compatibility version in the same update. ### Bundled NATS is now disabled by default diff --git a/deploy/helm/charts/platform/README.md.gotmpl b/deploy/helm/charts/platform/README.md.gotmpl index 6df9e59c3e24..fd00b0ba36a4 100644 --- a/deploy/helm/charts/platform/README.md.gotmpl +++ b/deploy/helm/charts/platform/README.md.gotmpl @@ -69,11 +69,12 @@ Admission now requires every component's main-container image. In `v1beta1`, set required by Dynamo admission, but were effectively required: Kubernetes rejects the rendered Pod specification when its main container has no image. -After upgrading the CRDs and operator, admission denies a new DGD, or an update to a pre-existing -DGD, when a component's main-image tag is not a semantic version and `runtimeVersionOverride` is -unset. This includes custom and SHA-tagged images. Set `runtimeVersionOverride` to the Dynamo -runtime compatibility version for that image before creating or updating the DGD. Existing resources -are not changed or revalidated solely by the upgrade. +After upgrading the CRDs and operator, admission denies a new DGD, DCD, or DGDR when a component's +main-image tag is not a semantic version and `runtimeVersionOverride` is unset. This includes custom +and SHA-tagged images. On updates, this requirement is ratcheted: a pre-existing resource with an +unchanged non-semantic-version image remains admissible without backfilling the override. Changing +the image to a non-semantic-version tag requires setting `runtimeVersionOverride` to that image's +Dynamo runtime compatibility version in the same update. ### Bundled NATS is now disabled by default diff --git a/deploy/operator/AGENTS.md b/deploy/operator/AGENTS.md index 1016c6cd806c..0f9a88c9738d 100644 --- a/deploy/operator/AGENTS.md +++ b/deploy/operator/AGENTS.md @@ -9,6 +9,14 @@ SPDX-License-Identifier: Apache-2.0 - Keep chart-only grants in the manual section of the platform chart's `../helm/charts/platform/components/operator/templates/manager-rbac.yaml`. +## Go Code Style + +- Put a one-line story comment above every multi-line block of logically + connected code. +- Separate multi-line semantic blocks from surrounding code with one blank + line. Do not add trailing blank lines before a closing delimiter or between + a block-leading comment and its code. + ## Go Test Style - Use `t.Log` to tell the test's story, with one heading before each block that diff --git a/deploy/operator/api/v1alpha1/dynamographdeploymentrequest_conversion.go b/deploy/operator/api/v1alpha1/dynamographdeploymentrequest_conversion.go index 6d2eb9fcf663..7e513b640740 100644 --- a/deploy/operator/api/v1alpha1/dynamographdeploymentrequest_conversion.go +++ b/deploy/operator/api/v1alpha1/dynamographdeploymentrequest_conversion.go @@ -1013,6 +1013,7 @@ func saveDGDRHubOnlyStatus(src *v1beta1.DynamoGraphDeploymentRequestStatus, dst if src == nil || save == nil { return } + save.ObservedSpecFingerprint = src.ObservedSpecFingerprint if src.Phase == v1beta1.DGDRPhaseDeployed && dgdrStateToPhase(string(dst.State), dst.Deployment) != src.Phase { save.Phase = src.Phase save.DGDName = src.DGDName @@ -1035,6 +1036,7 @@ func restoreDGDRHubOnlyStatus(restored *v1beta1.DynamoGraphDeploymentRequestStat if restored == nil || dst == nil { return } + dst.ObservedSpecFingerprint = restored.ObservedSpecFingerprint if restored.Phase == v1beta1.DGDRPhaseDeployed && dst.Phase == v1beta1.DGDRPhaseReady && src != nil && diff --git a/deploy/operator/api/v1alpha1/dynamographdeploymentrequest_conversion_test.go b/deploy/operator/api/v1alpha1/dynamographdeploymentrequest_conversion_test.go index bcc9a8c09381..26911d7070fb 100644 --- a/deploy/operator/api/v1alpha1/dynamographdeploymentrequest_conversion_test.go +++ b/deploy/operator/api/v1alpha1/dynamographdeploymentrequest_conversion_test.go @@ -137,11 +137,12 @@ func newV1beta1DGDR() *v1beta1.DynamoGraphDeploymentRequest { }, }, Status: v1beta1.DynamoGraphDeploymentRequestStatus{ - Phase: v1beta1.DGDRPhaseProfiling, - ObservedGeneration: 2, - DGDName: "hub-dgd", - ProfilingPhase: v1beta1.ProfilingPhaseSweepingDecode, - ProfilingJobName: "profiling-job-1", + Phase: v1beta1.DGDRPhaseProfiling, + ObservedGeneration: 2, + ObservedSpecFingerprint: "test-spec-fingerprint", + DGDName: "hub-dgd", + ProfilingPhase: v1beta1.ProfilingPhaseSweepingDecode, + ProfilingJobName: "profiling-job-1", ProfilingResults: &v1beta1.ProfilingResultsStatus{ SelectedConfig: &runtime.RawExtension{Raw: rawDGD}, }, diff --git a/deploy/operator/api/v1beta1/dynamographdeploymentrequest_types.go b/deploy/operator/api/v1beta1/dynamographdeploymentrequest_types.go index f13ad82aaccf..c2e0f6217f4b 100644 --- a/deploy/operator/api/v1beta1/dynamographdeploymentrequest_types.go +++ b/deploy/operator/api/v1beta1/dynamographdeploymentrequest_types.go @@ -584,6 +584,11 @@ type DynamoGraphDeploymentRequestStatus struct { // ObservedGeneration is the most recent generation observed by the controller. // +optional ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // ObservedSpecFingerprint identifies the spec associated with ObservedGeneration. + // The controller uses it to verify runtimeVersionOverride-only repairs. + // +optional + ObservedSpecFingerprint string `json:"observedSpecFingerprint,omitempty"` } // DynamoGraphDeploymentRequest is the Schema for the dynamographdeploymentrequests API. diff --git a/deploy/operator/config/crd/bases/nvidia.com_dynamographdeploymentrequests.yaml b/deploy/operator/config/crd/bases/nvidia.com_dynamographdeploymentrequests.yaml index 23320d1b85e4..d814c6b5ffcc 100644 --- a/deploy/operator/config/crd/bases/nvidia.com_dynamographdeploymentrequests.yaml +++ b/deploy/operator/config/crd/bases/nvidia.com_dynamographdeploymentrequests.yaml @@ -9392,6 +9392,11 @@ spec: description: ObservedGeneration is the most recent generation observed by the controller. format: int64 type: integer + observedSpecFingerprint: + description: |- + ObservedSpecFingerprint identifies the spec associated with ObservedGeneration. + The controller uses it to verify runtimeVersionOverride-only repairs. + type: string phase: description: Phase is the high-level lifecycle phase of the deployment request. enum: diff --git a/deploy/operator/internal/controller/dynamographdeploymentrequest_controller.go b/deploy/operator/internal/controller/dynamographdeploymentrequest_controller.go index 9dd7c224d4c8..fd8cc6fc57e5 100644 --- a/deploy/operator/internal/controller/dynamographdeploymentrequest_controller.go +++ b/deploy/operator/internal/controller/dynamographdeploymentrequest_controller.go @@ -54,6 +54,7 @@ import ( nvidiacomv1beta1 "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" "github.com/ai-dynamo/dynamo/deploy/operator/internal/consts" commonController "github.com/ai-dynamo/dynamo/deploy/operator/internal/controller_common" + "github.com/ai-dynamo/dynamo/deploy/operator/internal/dgdrutil" "github.com/ai-dynamo/dynamo/deploy/operator/internal/dynamo" "github.com/ai-dynamo/dynamo/deploy/operator/internal/features" "github.com/ai-dynamo/dynamo/deploy/operator/internal/gpu" @@ -462,21 +463,55 @@ func (r *DynamoGraphDeploymentRequestReconciler) Reconcile(ctx context.Context, return ctrl.Result{}, nil } - // Check for spec changes (immutability enforcement) + currentSpecFingerprint, err := dgdrutil.SpecFingerprint(&dgdr.Spec) + if err != nil { + return ctrl.Result{}, err + } + + // Record the fingerprint for the spec associated with the observed generation. + if dgdr.Status.ObservedGeneration > 0 && + dgdr.Status.ObservedGeneration == dgdr.Generation && + dgdr.Status.ObservedSpecFingerprint != currentSpecFingerprint { + dgdr.Status.ObservedSpecFingerprint = currentSpecFingerprint + if err := r.Status().Update(ctx, dgdr); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to record observed DGDR spec fingerprint: %w", err) + } + return ctrl.Result{Requeue: true}, nil + } + + // Verify immutable-phase repairs against the last observed spec before resuming the phase machine. if dgdr.Status.ObservedGeneration > 0 && dgdr.Status.ObservedGeneration != dgdr.Generation { - // Spec changed after initial processing if dgdr.Status.Phase == nvidiacomv1beta1.DGDRPhaseProfiling || dgdr.Status.Phase == nvidiacomv1beta1.DGDRPhaseDeploying || dgdr.Status.Phase == nvidiacomv1beta1.DGDRPhaseReady || dgdr.Status.Phase == nvidiacomv1beta1.DGDRPhaseDeployed { + repair, err := dgdrutil.IsRuntimeVersionOverrideRepair(&dgdr.Spec, dgdr.Status.ObservedSpecFingerprint) + if err != nil { + return ctrl.Result{}, err + } + if repair { + logger.Info("Observing verified runtime version override repair in immutable phase", + "phase", dgdr.Status.Phase, + "observedGeneration", dgdr.Status.ObservedGeneration, + "currentGeneration", dgdr.Generation) + + // Repair generated manifests before acknowledging the new generation. + if err := r.repairGeneratedDGDArtifacts(ctx, dgdr); err != nil { + return ctrl.Result{}, err + } + + dgdr.Status.ObservedGeneration = dgdr.Generation + dgdr.Status.ObservedSpecFingerprint = currentSpecFingerprint + if err := r.Status().Update(ctx, dgdr); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to observe DGDR runtime version override repair: %w", err) + } + return ctrl.Result{Requeue: true}, nil + } + logger.Info("Spec change detected in immutable phase", "phase", dgdr.Status.Phase, "observedGeneration", dgdr.Status.ObservedGeneration, "currentGeneration", dgdr.Generation) - r.Recorder.Event(dgdr, corev1.EventTypeWarning, nvidiacomv1beta1.EventReasonSpecChangeRejected, fmt.Sprintf(MessageSpecChangeRejected, dgdr.Status.Phase)) - - // Keep the old observedGeneration to continue rejecting changes - // No phase transition - stay in current phase with old spec return ctrl.Result{}, nil } } @@ -516,9 +551,6 @@ func (r *DynamoGraphDeploymentRequestReconciler) handlePendingPhase(ctx context. return r.updatePhaseWithCondition(ctx, dgdr, nvidiacomv1beta1.DGDRPhaseFailed, nvidiacomv1beta1.ConditionTypeValidation, metav1.ConditionFalse, nvidiacomv1beta1.EventReasonValidationFailed, err.Error()) } - // Set observedGeneration to track the spec we're processing - dgdr.Status.ObservedGeneration = dgdr.Generation - dgdr.AddStatusCondition(metav1.Condition{ Type: nvidiacomv1beta1.ConditionTypeValidation, Status: metav1.ConditionTrue, @@ -955,6 +987,7 @@ func (r *DynamoGraphDeploymentRequestReconciler) createDGD(ctx context.Context, if err != nil { return ctrl.Result{}, fmt.Errorf("failed to unmarshal generated deployment from annotation: %w", err) } + applyDGDRRuntimeVersionOverride(dgdr, generatedDGD) // Determine DGD name and namespace from generated deployment dgdName := generatedDGD.Name @@ -2052,6 +2085,7 @@ func (r *DynamoGraphDeploymentRequestReconciler) generateDGDSpec(ctx context.Con if err != nil { return nil, "", fmt.Errorf("failed to extract DGD from %s: %w", outputFile, err) } + applyDGDRRuntimeVersionOverride(dgdr, dgd) // Override the profiler-generated name with a DGDR-scoped unique name. // The profiler emits a static topology-derived name (e.g. "vllm-agg") which @@ -2105,6 +2139,79 @@ func (r *DynamoGraphDeploymentRequestReconciler) generateDGDSpec(ctx context.Con return profilingResults, dgd.Name, nil } +// applyDGDRRuntimeVersionOverride makes the DGDR authoritative across profiler versions. +func applyDGDRRuntimeVersionOverride( + dgdr *nvidiacomv1beta1.DynamoGraphDeploymentRequest, + dgd *nvidiacomv1beta1.DynamoGraphDeployment, +) { + if dgdr.Spec.RuntimeVersionOverride == "" { + return + } + for i := range dgd.Spec.Components { + dgd.Spec.Components[i].RuntimeVersionOverride = dgdr.Spec.RuntimeVersionOverride + } +} + +// repairGeneratedDGDArtifacts applies a repaired override to every persisted generated manifest. +func (r *DynamoGraphDeploymentRequestReconciler) repairGeneratedDGDArtifacts( + ctx context.Context, + dgdr *nvidiacomv1beta1.DynamoGraphDeploymentRequest, +) error { + // Repair the manifest exposed for manual application. + if dgdr.Status.ProfilingResults != nil && + dgdr.Status.ProfilingResults.SelectedConfig != nil && + len(dgdr.Status.ProfilingResults.SelectedConfig.Raw) > 0 { + dgd, err := r.extractDGDFromYAML(dgdr.Status.ProfilingResults.SelectedConfig.Raw) + if err != nil { + return fmt.Errorf("failed to decode selected DGD config for runtime version repair: %w", err) + } + applyDGDRRuntimeVersionOverride(dgdr, dgd) + dgdJSON, _, err := r.encodeBetaDGDManifest(dgd) + if err != nil { + return fmt.Errorf("failed to encode selected DGD config after runtime version repair: %w", err) + } + dgdr.Status.ProfilingResults.SelectedConfig.Raw = dgdJSON + } + + // Repair the manifest retained for automatic DGD creation. + generatedDGDYAML := dgdr.Annotations[AnnotationGeneratedDGDSpec] + if generatedDGDYAML == "" { + return nil + } + + dgd, err := r.extractDGDFromYAML([]byte(generatedDGDYAML)) + if err != nil { + return fmt.Errorf("failed to decode generated DGD annotation for runtime version repair: %w", err) + } + applyDGDRRuntimeVersionOverride(dgdr, dgd) + _, dgdYAML, err := r.encodeBetaDGDManifest(dgd) + if err != nil { + return fmt.Errorf("failed to encode generated DGD annotation after runtime version repair: %w", err) + } + + // Persist the repaired annotation without taking ownership of unrelated metadata. + annotations := map[string]any{AnnotationGeneratedDGDSpec: string(dgdYAML)} + if additionalResources := dgdr.Annotations[AnnotationAdditionalResources]; additionalResources != "" { + annotations[AnnotationAdditionalResources] = additionalResources + } + apply := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": nvidiacomv1beta1.GroupVersion.String(), + "kind": "DynamoGraphDeploymentRequest", + "metadata": map[string]any{ + "name": dgdr.Name, + "namespace": dgdr.Namespace, + "resourceVersion": dgdr.ResourceVersion, + "annotations": annotations, + }, + }} + if err := r.Apply(ctx, client.ApplyConfigurationFromUnstructured(apply), client.FieldOwner("dynamo-operator-dgdr"), client.ForceOwnership); err != nil { + return fmt.Errorf("failed to persist repaired generated DGD annotation: %w", err) + } + dgdr.Annotations[AnnotationGeneratedDGDSpec] = string(dgdYAML) + dgdr.ResourceVersion = apply.GetResourceVersion() + return nil +} + // encodeBetaDGDManifest returns JSON/YAML manifest bytes for a beta DGD. // The Kubernetes versioning encoder temporarily supplies apiVersion/kind from // the scheme during serialization and restores the typed object's TypeMeta after. @@ -2323,8 +2430,10 @@ func setSucceededCondition(dgdr *nvidiacomv1beta1.DynamoGraphDeploymentRequest, func (r *DynamoGraphDeploymentRequestReconciler) updatePhase(ctx context.Context, dgdr *nvidiacomv1beta1.DynamoGraphDeploymentRequest, phase nvidiacomv1beta1.DGDRPhase, message string) (ctrl.Result, error) { logger := log.FromContext(ctx) logger.Info("Updating DGDR phase", "name", dgdr.Name, "phase", phase, "message", message) + if err := observeCurrentDGDRSpec(dgdr); err != nil { + return ctrl.Result{}, err + } dgdr.Status.Phase = phase - dgdr.Status.ObservedGeneration = dgdr.Generation setSucceededCondition(dgdr, phase) if err := r.Status().Update(ctx, dgdr); err != nil { return ctrl.Result{}, err @@ -2342,8 +2451,10 @@ func (r *DynamoGraphDeploymentRequestReconciler) updatePhaseWithCondition( reason string, message string, ) (ctrl.Result, error) { + if err := observeCurrentDGDRSpec(dgdr); err != nil { + return ctrl.Result{}, err + } dgdr.Status.Phase = phase - dgdr.Status.ObservedGeneration = dgdr.Generation // Set the specific condition first so setSucceededCondition can surface it. dgdr.AddStatusCondition(metav1.Condition{ @@ -2363,6 +2474,16 @@ func (r *DynamoGraphDeploymentRequestReconciler) updatePhaseWithCondition( return ctrl.Result{}, nil } +func observeCurrentDGDRSpec(dgdr *nvidiacomv1beta1.DynamoGraphDeploymentRequest) error { + specFingerprint, err := dgdrutil.SpecFingerprint(&dgdr.Spec) + if err != nil { + return err + } + dgdr.Status.ObservedGeneration = dgdr.Generation + dgdr.Status.ObservedSpecFingerprint = specFingerprint + return nil +} + // SetupWithManager sets up the controller with the Manager func (r *DynamoGraphDeploymentRequestReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/deploy/operator/internal/controller/dynamographdeploymentrequest_controller_envtest_test.go b/deploy/operator/internal/controller/dynamographdeploymentrequest_controller_envtest_test.go index 6cbbf94772b6..d492e2be3337 100644 --- a/deploy/operator/internal/controller/dynamographdeploymentrequest_controller_envtest_test.go +++ b/deploy/operator/internal/controller/dynamographdeploymentrequest_controller_envtest_test.go @@ -963,7 +963,6 @@ spec: services: Frontend: replicas: 1 - runtimeVersionOverride: 1.1.0 extraPodSpec: mainContainer: image: registry.example/runtime:custom` @@ -995,6 +994,8 @@ spec: var updated nvidiacomv1beta1.DynamoGraphDeploymentRequest Expect(k8sClient.Get(ctx, types.NamespacedName{Name: dgdrName, Namespace: namespace}, &updated)).Should(Succeed()) Expect(updated.Status.Phase).Should(Equal(nvidiacomv1beta1.DGDRPhaseDeploying)) + Expect(updated.Annotations[AnnotationGeneratedDGDSpec]).Should(ContainSubstring("runtimeVersionOverride: 1.1.0")) + Expect(string(updated.Status.ProfilingResults.SelectedConfig.Raw)).Should(ContainSubstring(`"runtimeVersionOverride":"1.1.0"`)) // Reconcile again to create DGD _, err = reconciler.Reconcile(ctx, reconcile.Request{ @@ -1018,6 +1019,44 @@ spec: _ = k8sClient.Delete(ctx, dgd) }) + It("Should apply the DGDR runtime version to a persisted legacy profiler result", func() { + ctx := context.Background() + dgdName := "legacy-profiler-result-dgd" + dgdr := &nvidiacomv1beta1.DynamoGraphDeploymentRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: "legacy-profiler-result", + Namespace: envtestNamespace, + Annotations: map[string]string{ + AnnotationGeneratedDGDSpec: `apiVersion: nvidia.com/v1alpha1 +kind: DynamoGraphDeployment +metadata: + name: legacy-profiler-result-dgd +spec: + services: + worker: + extraPodSpec: + mainContainer: + image: registry.example/runtime:custom`, + }, + }, + Spec: nvidiacomv1beta1.DynamoGraphDeploymentRequestSpec{ + RuntimeVersionOverride: "1.2.3", + }, + Status: nvidiacomv1beta1.DynamoGraphDeploymentRequestStatus{ + DGDName: dgdName, + }, + } + + _, err := reconciler.createDGD(ctx, dgdr) + Expect(err).NotTo(HaveOccurred()) + + dgd := &nvidiacomv1beta1.DynamoGraphDeployment{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: dgdName, Namespace: envtestNamespace}, dgd)).Should(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, dgd) }() + Expect(dgd.Spec.Components).Should(HaveLen(1)) + Expect(dgd.Spec.Components[0].RuntimeVersionOverride).Should(Equal("1.2.3")) + }) + It("Should create additional ConfigMaps without DGDR ownership and adopt them after DGD creation", func() { ctx := context.Background() dgdrName := "test-dgdr-additional-cm-owner" @@ -1328,11 +1367,11 @@ spec: }) }) - Context("When enforcing spec immutability", func() { - It("Should reject spec changes after profiling starts", func() { + Context("When observing admitted spec repairs", func() { + It("Should continue reconciliation after a runtime version override repair", func() { t := GinkgoT() ctx := context.Background() - dgdrName := "test-dgdr-immutable" + dgdrName := "test-dgdr-runtime-version-repair" namespace := envtestNamespace dgdr := &nvidiacomv1beta1.DynamoGraphDeploymentRequest{ @@ -1343,7 +1382,7 @@ spec: Spec: nvidiacomv1beta1.DynamoGraphDeploymentRequestSpec{ Model: "test-model", Backend: "vllm", - Image: "test-profiler:1.1.0", + Image: "test-profiler:custom", Hardware: &nvidiacomv1beta1.HardwareSpec{ NumGPUsPerNode: ptr.To[int32](8), GPUSKU: nvidiacomv1beta1.GPUSKUTypeH100SXM, @@ -1357,50 +1396,102 @@ spec: }, } - t.Log("Create and reconcile the initial request") - Expect(k8sClient.Create(ctx, dgdr)).Should(Succeed()) + t.Log("Seed and reconcile the legacy request") + Expect(admissionBypassClient.Create(ctx, dgdr)).Should(Succeed()) defer func() { _ = k8sClient.Delete(ctx, dgdr) }() _, err := reconciler.Reconcile(ctx, reconcile.Request{ NamespacedName: types.NamespacedName{Name: dgdrName, Namespace: namespace}, }) Expect(err).NotTo(HaveOccurred()) - t.Log("Read the initialized generation") + t.Log("Continue reconciliation after initialization") + result, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{Name: dgdrName, Namespace: namespace}, + }) + Expect(err).NotTo(HaveOccurred()) + Expect(result.IsZero()).Should(BeTrue()) + + t.Log("Read the initialized generation and fingerprint") var current nvidiacomv1beta1.DynamoGraphDeploymentRequest Expect(k8sClient.Get(ctx, types.NamespacedName{Name: dgdrName, Namespace: namespace}, ¤t)).Should(Succeed()) initialGeneration := current.Generation - observedGeneration := current.Status.ObservedGeneration + Expect(current.Status.ObservedSpecFingerprint).ShouldNot(BeEmpty()) - t.Log("Move the request into the profiling phase") - current.Status.Phase = nvidiacomv1beta1.DGDRPhaseProfiling + t.Log("Store legacy generated manifests and move the request into the ready phase") + generatedDGD := &nvidiacomv1beta1.DynamoGraphDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: dgdrName + "-generated", + Namespace: namespace, + }, + Spec: nvidiacomv1beta1.DynamoGraphDeploymentSpec{ + BackendFramework: "vllm", + Components: []nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec{{ + ComponentName: "worker", + ComponentType: nvidiacomv1beta1.ComponentTypeWorker, + Replicas: ptr.To[int32](1), + PodTemplate: &corev1.PodTemplateSpec{Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: consts.MainContainerName, Image: "registry.example/runtime:custom"}}, + }}, + }}, + }, + } + dgdJSON, dgdYAML, err := reconciler.encodeBetaDGDManifest(generatedDGD) + Expect(err).NotTo(HaveOccurred()) + current.Annotations = map[string]string{AnnotationGeneratedDGDSpec: string(dgdYAML)} + Expect(k8sClient.Update(ctx, ¤t)).Should(Succeed()) + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: dgdrName, Namespace: namespace}, ¤t)).Should(Succeed()) + current.Status.Phase = nvidiacomv1beta1.DGDRPhaseReady + current.Status.ProfilingResults = &nvidiacomv1beta1.ProfilingResultsStatus{ + SelectedConfig: &runtime.RawExtension{Raw: dgdJSON}, + } Expect(k8sClient.Status().Update(ctx, ¤t)).Should(Succeed()) - t.Log("Seed a spec change that validating admission normally rejects") + t.Log("Repair the missing runtime version override") Expect(k8sClient.Get(ctx, types.NamespacedName{Name: dgdrName, Namespace: namespace}, ¤t)).Should(Succeed()) - current.Spec.Model = "modified-model" - Expect(admissionBypassClient.Update(ctx, ¤t)).Should(Succeed()) + current.Spec.RuntimeVersionOverride = "1.1.0" + Expect(k8sClient.Update(ctx, ¤t)).Should(Succeed()) - t.Log("Reconcile the legacy invalid state") - _, err = reconciler.Reconcile(ctx, reconcile.Request{ + t.Log("Observe the admitted repair") + result, err = reconciler.Reconcile(ctx, reconcile.Request{ NamespacedName: types.NamespacedName{Name: dgdrName, Namespace: namespace}, }) Expect(err).NotTo(HaveOccurred()) + Expect(result.IsZero()).Should(BeFalse()) - t.Log("Verify that reconciliation preserves the previously observed state") + t.Log("Verify that reconciliation advances the observed generation") Expect(k8sClient.Get(ctx, types.NamespacedName{Name: dgdrName, Namespace: namespace}, ¤t)).Should(Succeed()) Expect(current.Generation).Should(BeNumerically(">", initialGeneration)) - Expect(current.Status.ObservedGeneration).Should(Equal(observedGeneration)) - Expect(current.Status.Phase).Should(Equal(nvidiacomv1beta1.DGDRPhaseProfiling)) + Expect(current.Status.ObservedGeneration).Should(Equal(current.Generation)) + Expect(current.Status.Phase).Should(Equal(nvidiacomv1beta1.DGDRPhaseReady)) - t.Log("Verify that reconciliation reports the rejected change") - Eventually(func() bool { - select { - case event := <-recorder.Events: - return strings.Contains(event, "DynamoGraphDeploymentRequest is immutable once profiling starts") - default: - return false - } - }, timeout, interval).Should(BeTrue()) + t.Log("Verify that both persisted generated manifests contain the repaired override") + selectedDGD, err := reconciler.extractDGDFromYAML(current.Status.ProfilingResults.SelectedConfig.Raw) + Expect(err).NotTo(HaveOccurred()) + Expect(selectedDGD.Spec.Components).Should(HaveLen(1)) + Expect(selectedDGD.Spec.Components[0].RuntimeVersionOverride).Should(Equal("1.1.0")) + annotatedDGD, err := reconciler.extractDGDFromYAML([]byte(current.Annotations[AnnotationGeneratedDGDSpec])) + Expect(err).NotTo(HaveOccurred()) + Expect(annotatedDGD.Spec.Components).Should(HaveLen(1)) + Expect(annotatedDGD.Spec.Components[0].RuntimeVersionOverride).Should(Equal("1.1.0")) + + t.Log("Apply the repaired selected config through admission") + Expect(k8sClient.Create(ctx, selectedDGD)).Should(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, selectedDGD) }() + + t.Log("Bypass admission with an unrelated spec change") + repairedGeneration := current.Status.ObservedGeneration + current.Spec.Model = "modified-model" + Expect(admissionBypassClient.Update(ctx, ¤t)).Should(Succeed()) + + t.Log("Verify that the fingerprint prevents observing the unrelated change") + result, err = reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{Name: dgdrName, Namespace: namespace}, + }) + Expect(err).NotTo(HaveOccurred()) + Expect(result.IsZero()).Should(BeTrue()) + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: dgdrName, Namespace: namespace}, ¤t)).Should(Succeed()) + Expect(current.Status.ObservedGeneration).Should(Equal(repairedGeneration)) + Expect(current.Generation).Should(BeNumerically(">", repairedGeneration)) }) }) @@ -2757,7 +2848,7 @@ spec: // Transition to Profiling dgdr.Status.Phase = nvidiacomv1beta1.DGDRPhaseProfiling - dgdr.Status.ObservedGeneration = dgdr.Generation + Expect(observeCurrentDGDRSpec(dgdr)).Should(Succeed()) Expect(k8sClient.Status().Update(ctx, dgdr)).Should(Succeed()) // Create completed job diff --git a/deploy/operator/internal/dgdrutil/spec_fingerprint.go b/deploy/operator/internal/dgdrutil/spec_fingerprint.go new file mode 100644 index 000000000000..99c5354074c1 --- /dev/null +++ b/deploy/operator/internal/dgdrutil/spec_fingerprint.go @@ -0,0 +1,53 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package dgdrutil + +import ( + "crypto/sha256" + "encoding/json" + "fmt" + + nvidiacomv1beta1 "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" +) + +// SpecFingerprint returns a stable SHA-256 fingerprint of a DGDR spec. +func SpecFingerprint(spec *nvidiacomv1beta1.DynamoGraphDeploymentRequestSpec) (string, error) { + data, err := json.Marshal(spec) + if err != nil { + return "", fmt.Errorf("marshal DGDR spec: %w", err) + } + return fmt.Sprintf("%x", sha256.Sum256(data)), nil +} + +// IsRuntimeVersionOverrideRepair reports whether current only adds an override to the observed spec. +func IsRuntimeVersionOverrideRepair( + current *nvidiacomv1beta1.DynamoGraphDeploymentRequestSpec, + observedFingerprint string, +) (bool, error) { + if observedFingerprint == "" || current.RuntimeVersionOverride == "" { + return false, nil + } + + observedCandidate := current.DeepCopy() + observedCandidate.RuntimeVersionOverride = "" + fingerprint, err := SpecFingerprint(observedCandidate) + if err != nil { + return false, err + } + return fingerprint == observedFingerprint, nil +} diff --git a/deploy/operator/internal/webhook/validation/dynamocomponentdeployment.go b/deploy/operator/internal/webhook/validation/dynamocomponentdeployment.go index 67ec12426d60..b68e4c76d002 100644 --- a/deploy/operator/internal/webhook/validation/dynamocomponentdeployment.go +++ b/deploy/operator/internal/webhook/validation/dynamocomponentdeployment.go @@ -70,31 +70,38 @@ func (v *DynamoComponentDeploymentValidator) validate( // ValidateUpdate performs complete validation of an updated v1beta1 DCD and // compares its state with the previous object. -// ctx, oldDCD, and newDCD must not be nil. +// ctx, oldDCD, and newDCD must not be nil. runtimeVersionSource identifies the request's source API. func (v *DynamoComponentDeploymentValidator) ValidateUpdate( ctx context.Context, oldDCD *nvidiacomv1beta1.DynamoComponentDeployment, newDCD *nvidiacomv1beta1.DynamoComponentDeployment, -) (admission.Warnings, error) { - return v.validateUpdate(ctx, oldDCD, newDCD, runtimeVersionSourceV1Beta1) -} - -func (v *DynamoComponentDeploymentValidator) validateUpdate( - ctx context.Context, - oldDCD *nvidiacomv1beta1.DynamoComponentDeployment, - newDCD *nvidiacomv1beta1.DynamoComponentDeployment, runtimeVersionSource runtimeVersionValidationSource, ) (admission.Warnings, error) { validation := &dynamoComponentDeploymentValidation{ - sharedValidation: sharedValidation{ctx: ctx, runtimeVersionSource: runtimeVersionSource}, + sharedValidation: sharedValidation{ctx: ctx, runtimeVersionSource: runtimeVersionSourceDisabled}, } allErrs := validation.validateDynamoComponentDeployment(newDCD) - alpha, err := alphaDynamoComponentDeploymentForValidation(newDCD) + newAlpha, err := alphaDynamoComponentDeploymentForValidation(newDCD) if err != nil { return nil, fmt.Errorf("cannot validate preserved v1alpha1 DynamoComponentDeployment fields: %w", err) } - allErrs = append(allErrs, validation.validateDynamoComponentDeploymentV1alpha1(alpha)...) + allErrs = append(allErrs, validation.validateDynamoComponentDeploymentV1alpha1(newAlpha)...) + + // Re-enable source-version runtime validation for the old/new ratchet. + validation.runtimeVersionSource = runtimeVersionSource + if validation.validatesRuntimeVersionFor(runtimeVersionSourceV1Alpha1) { + oldAlpha, err := alphaDynamoComponentDeploymentForValidation(oldDCD) + if err != nil { + return nil, fmt.Errorf("cannot validate old preserved v1alpha1 DynamoComponentDeployment fields: %w", err) + } + allErrs = append(allErrs, validation.validateDynamoComponentDeploymentSharedSpecUpdateV1alpha1( + &newAlpha.Spec.DynamoComponentDeploymentSharedSpec, + &oldAlpha.Spec.DynamoComponentDeploymentSharedSpec, + field.NewPath("spec"), + )...) + } + allErrs = append(allErrs, validation.validateDynamoComponentDeploymentUpdate(newDCD, oldDCD)...) return validation.warnings, invalidDynamoComponentDeploymentError(newDCD, allErrs) } diff --git a/deploy/operator/internal/webhook/validation/dynamocomponentdeployment_handler.go b/deploy/operator/internal/webhook/validation/dynamocomponentdeployment_handler.go index 07e02ae40009..c099225336c4 100644 --- a/deploy/operator/internal/webhook/validation/dynamocomponentdeployment_handler.go +++ b/deploy/operator/internal/webhook/validation/dynamocomponentdeployment_handler.go @@ -118,7 +118,7 @@ func (h *DynamoComponentDeploymentHandler) validateUpdate( } validator := NewDynamoComponentDeploymentValidator() - return validator.validateUpdate(ctx, oldDeployment, newDeployment, runtimeVersionValidationSourceForRequest(ctx, expectedGVK)) + return validator.ValidateUpdate(ctx, oldDeployment, newDeployment, runtimeVersionValidationSourceForRequest(ctx, expectedGVK)) } // ValidateDelete validates a DynamoComponentDeployment delete request. diff --git a/deploy/operator/internal/webhook/validation/dynamocomponentdeployment_validation_envtest_test.go b/deploy/operator/internal/webhook/validation/dynamocomponentdeployment_validation_envtest_test.go index 9f63baf404b6..01ccce0919f4 100644 --- a/deploy/operator/internal/webhook/validation/dynamocomponentdeployment_validation_envtest_test.go +++ b/deploy/operator/internal/webhook/validation/dynamocomponentdeployment_validation_envtest_test.go @@ -91,16 +91,66 @@ func TestDynamoComponentDeploymentValidator_Validate(t *testing.T) { deployment: alphaDCDForAdmission(func(dcd *nvidiacomv1alpha1.DynamoComponentDeployment) { dcd.Spec.RuntimeVersionOverride = "" dcd.Spec.ExtraPodSpec = &nvidiacomv1alpha1.ExtraPodSpec{ - MainContainer: &corev1.Container{Image: "registry.example/runtime:custom"}, + MainContainer: &corev1.Container{Image: customRuntimeImage}, } }), wantWebhookErrs: []string{"spec.runtimeVersionOverride: Required value: is required when the specified main container image has no parseable semantic-version tag"}, }, + { + name: "unchanged legacy v1beta1 runtime version is ratcheted on update", + seedWithoutWebhook: true, + oldDeployment: betaDCDForAdmission(func(dcd *nvidiacomv1beta1.DynamoComponentDeployment) { + dcd.Spec.RuntimeVersionOverride = "" + dcd.Spec.PodTemplate.Spec.Containers[0].Image = customRuntimeImage + }), + deployment: betaDCDForAdmission(func(dcd *nvidiacomv1beta1.DynamoComponentDeployment) { + dcd.Spec.RuntimeVersionOverride = "" + dcd.Spec.PodTemplate.Spec.Containers[0].Image = customRuntimeImage + dcd.Labels = map[string]string{"updated": "true"} + }), + }, + { + name: "unchanged legacy v1alpha1 runtime version is ratcheted on update", + seedWithoutWebhook: true, + oldDeployment: alphaDCDForAdmission(func(dcd *nvidiacomv1alpha1.DynamoComponentDeployment) { + dcd.Spec.RuntimeVersionOverride = "" + dcd.Spec.ExtraPodSpec.MainContainer.Image = customRuntimeImage + }), + deployment: alphaDCDForAdmission(func(dcd *nvidiacomv1alpha1.DynamoComponentDeployment) { + dcd.Spec.RuntimeVersionOverride = "" + dcd.Spec.ExtraPodSpec.MainContainer.Image = customRuntimeImage + dcd.Labels = map[string]string{"updated": "true"} + }), + }, + { + name: "v1beta1 image change to custom requires runtime version override", + oldDeployment: betaDCDForAdmission(func(dcd *nvidiacomv1beta1.DynamoComponentDeployment) { + dcd.Spec.RuntimeVersionOverride = "" + }), + deployment: betaDCDForAdmission(func(dcd *nvidiacomv1beta1.DynamoComponentDeployment) { + dcd.Spec.RuntimeVersionOverride = "" + dcd.Spec.PodTemplate.Spec.Containers[0].Image = customRuntimeImage + }), + wantWebhookErrs: []string{"spec.runtimeVersionOverride: Required value: is required when the specified main container image has no parseable semantic-version tag"}, + }, + { + name: "changing a legacy v1alpha1 custom image requires runtime version override", + seedWithoutWebhook: true, + oldDeployment: alphaDCDForAdmission(func(dcd *nvidiacomv1alpha1.DynamoComponentDeployment) { + dcd.Spec.RuntimeVersionOverride = "" + dcd.Spec.ExtraPodSpec.MainContainer.Image = customRuntimeImage + }), + deployment: alphaDCDForAdmission(func(dcd *nvidiacomv1alpha1.DynamoComponentDeployment) { + dcd.Spec.RuntimeVersionOverride = "" + dcd.Spec.ExtraPodSpec.MainContainer.Image = "registry.example/runtime:other-custom" + }), + wantWebhookErrs: []string{"spec.runtimeVersionOverride: Required value: is required when the specified main container image has no parseable semantic-version tag"}, + }, { name: "v1alpha1 compatibility validation does not duplicate runtime version errors", deployment: alphaDCDForAdmission(func(dcd *nvidiacomv1alpha1.DynamoComponentDeployment) { dcd.Spec.RuntimeVersionOverride = "" - dcd.Spec.ExtraPodSpec.MainContainer.Image = "registry.example/runtime:custom" + dcd.Spec.ExtraPodSpec.MainContainer.Image = customRuntimeImage dcd.Spec.Ingress = &nvidiacomv1alpha1.IngressSpec{Enabled: true} }), wantWebhookErrs: []string{ diff --git a/deploy/operator/internal/webhook/validation/dynamographdeployment.go b/deploy/operator/internal/webhook/validation/dynamographdeployment.go index e83a40d3ff5f..9ef204e43ede 100644 --- a/deploy/operator/internal/webhook/validation/dynamographdeployment.go +++ b/deploy/operator/internal/webhook/validation/dynamographdeployment.go @@ -99,7 +99,7 @@ func (v *DynamoGraphDeploymentValidator) validate( } // ValidateUpdate performs stateful validation comparing old and new v1beta1 DGD objects. -// ctx, oldDGD, and newDGD must not be nil. +// ctx, oldDGD, and newDGD must not be nil. runtimeVersionSource identifies the request's source API. // If userInfo is nil, replica changes for DGDSA-enabled components fail closed. func (v *DynamoGraphDeploymentValidator) ValidateUpdate( ctx context.Context, @@ -107,14 +107,30 @@ func (v *DynamoGraphDeploymentValidator) ValidateUpdate( newDGD *nvidiacomv1beta1.DynamoGraphDeployment, userInfo *authenticationv1.UserInfo, operatorPrincipal string, + runtimeVersionSource runtimeVersionValidationSource, ) (admission.Warnings, error) { validation := &dynamoGraphDeploymentValidation{ - sharedValidation: sharedValidation{ctx: ctx, mgr: v.mgr, runtimeVersionSource: runtimeVersionSourceV1Beta1}, + sharedValidation: sharedValidation{ctx: ctx, mgr: v.mgr, runtimeVersionSource: runtimeVersionSource}, userInfo: userInfo, operatorPrincipal: operatorPrincipal, } allErrs := validation.validateDynamoGraphDeploymentUpdate(newDGD, oldDGD) + if validation.validatesRuntimeVersionFor(runtimeVersionSourceV1Alpha1) { + newAlpha, err := alphaDynamoGraphDeploymentForValidation(newDGD) + if err != nil { + return nil, fmt.Errorf("cannot validate preserved v1alpha1 DynamoGraphDeployment fields: %w", err) + } + oldAlpha, err := alphaDynamoGraphDeploymentForValidation(oldDGD) + if err != nil { + return nil, fmt.Errorf("cannot validate old preserved v1alpha1 DynamoGraphDeployment fields: %w", err) + } + allErrs = append(allErrs, validation.validateDynamoGraphDeploymentSpecUpdateV1alpha1( + &newAlpha.Spec, + &oldAlpha.Spec, + field.NewPath("spec"), + )...) + } return validation.warnings, invalidDynamoGraphDeploymentError(newDGD, allErrs) } diff --git a/deploy/operator/internal/webhook/validation/dynamographdeployment_handler.go b/deploy/operator/internal/webhook/validation/dynamographdeployment_handler.go index 7217c7181315..d9196139b412 100644 --- a/deploy/operator/internal/webhook/validation/dynamographdeployment_handler.go +++ b/deploy/operator/internal/webhook/validation/dynamographdeployment_handler.go @@ -132,7 +132,8 @@ func (h *DynamoGraphDeploymentHandler) validateUpdate( // Create validator with manager for API group detection and perform validation. validator := NewDynamoGraphDeploymentValidator(h.mgr) - warnings, err := validator.validate(ctx, newDeployment, runtimeVersionValidationSourceForRequest(ctx, expectedGVK)) + runtimeVersionSource := runtimeVersionValidationSourceForRequest(ctx, expectedGVK) + warnings, err := validator.validate(ctx, newDeployment, runtimeVersionSourceDisabled) if err != nil { return warnings, err } @@ -148,7 +149,14 @@ func (h *DynamoGraphDeploymentHandler) validateUpdate( } // Validate stateful rules (immutability + replicas protection) - updateWarnings, err := validator.ValidateUpdate(ctx, oldDeployment, newDeployment, userInfo, h.operatorPrincipal) + updateWarnings, err := validator.ValidateUpdate( + ctx, + oldDeployment, + newDeployment, + userInfo, + h.operatorPrincipal, + runtimeVersionSource, + ) if err != nil { username := "" if userInfo != nil { diff --git a/deploy/operator/internal/webhook/validation/dynamographdeployment_v1alpha1.go b/deploy/operator/internal/webhook/validation/dynamographdeployment_v1alpha1.go index 7e2a48ccf849..ac6a588c683d 100644 --- a/deploy/operator/internal/webhook/validation/dynamographdeployment_v1alpha1.go +++ b/deploy/operator/internal/webhook/validation/dynamographdeployment_v1alpha1.go @@ -65,6 +65,30 @@ func (v *dynamoGraphDeploymentValidation) validateDynamoGraphDeploymentSpecV1alp return allErrs } +// validateDynamoGraphDeploymentSpecUpdateV1alpha1 validates source-version runtime fields on update. +// newSpec, oldSpec, and fldPath must not be nil. +func (v *dynamoGraphDeploymentValidation) validateDynamoGraphDeploymentSpecUpdateV1alpha1( + newSpec *nvidiacomv1alpha1.DynamoGraphDeploymentSpec, + oldSpec *nvidiacomv1alpha1.DynamoGraphDeploymentSpec, + fldPath *field.Path, +) field.ErrorList { + allErrs := field.ErrorList{} + servicesPath := fldPath.Child("services") + for _, serviceName := range sortedV1Alpha1ServiceNames(newSpec.Services) { + newService := newSpec.Services[serviceName] + oldService, exists := oldSpec.Services[serviceName] + if !exists { + continue + } + allErrs = append(allErrs, v.validateDynamoComponentDeploymentSharedSpecUpdateV1alpha1( + newService, + oldService, + servicesPath.Key(serviceName), + )...) + } + return allErrs +} + // validatePVCV1alpha1 validates pvc. pvc and fldPath must not be nil. func (v *dynamoGraphDeploymentValidation) validatePVCV1alpha1( pvc *nvidiacomv1alpha1.PVC, diff --git a/deploy/operator/internal/webhook/validation/dynamographdeployment_validation_envtest_test.go b/deploy/operator/internal/webhook/validation/dynamographdeployment_validation_envtest_test.go index 62e37f6af306..bb777853810f 100644 --- a/deploy/operator/internal/webhook/validation/dynamographdeployment_validation_envtest_test.go +++ b/deploy/operator/internal/webhook/validation/dynamographdeployment_validation_envtest_test.go @@ -47,14 +47,15 @@ func TestDynamoGraphDeploymentValidator_Validate(t *testing.T) { tooLongComponentName := boundaryComponentName + "x" tests := []struct { - name string - deployment runtime.Object - oldDeployment runtime.Object - mutateRequest func(*testing.T, map[string]any) // mutates the source-version request map - withoutTopology bool // omits the default cluster topology fixture - groveDisabled bool // disables the configured Grove pathway - checkpointOff bool // disables checkpoint creation and restore - username string // supplies the admission request identity + name string + deployment runtime.Object + oldDeployment runtime.Object + mutateRequest func(*testing.T, map[string]any) // mutates the source-version request map + withoutTopology bool // omits the default cluster topology fixture + groveDisabled bool // disables the configured Grove pathway + checkpointOff bool // disables checkpoint creation and restore + seedWithoutWebhook bool // seeds oldDeployment without validating it + username string // supplies the admission request identity wantSchemaErr string wantCELErr string @@ -74,7 +75,7 @@ func TestDynamoGraphDeploymentValidator_Validate(t *testing.T) { worker := betaWorkerComponent(dgd) worker.RuntimeVersionOverride = "" worker.PodTemplate = &corev1.PodTemplateSpec{Spec: corev1.PodSpec{ - Containers: []corev1.Container{{Name: consts.MainContainerName, Image: "registry.example/runtime:custom"}}, + Containers: []corev1.Container{{Name: consts.MainContainerName, Image: customRuntimeImage}}, }} }), wantWebhookErrs: []string{"spec.components[1].runtimeVersionOverride: Required value: is required when the specified main container image has no parseable semantic-version tag"}, @@ -84,7 +85,65 @@ func TestDynamoGraphDeploymentValidator_Validate(t *testing.T) { deployment: alphaDGDForAdmission(func(dgd *nvidiacomv1alpha1.DynamoGraphDeployment) { worker := dgd.Spec.Services["worker"] worker.RuntimeVersionOverride = "" - worker.ExtraPodSpec.MainContainer.Image = "registry.example/runtime:custom" + worker.ExtraPodSpec.MainContainer.Image = customRuntimeImage + }), + wantWebhookErrs: []string{"spec.services[worker].runtimeVersionOverride: Required value: is required when the specified main container image has no parseable semantic-version tag"}, + }, + { + name: "unchanged legacy beta component runtime version is ratcheted on update", + seedWithoutWebhook: true, + oldDeployment: betaDGDForAdmission(func(dgd *nvidiacomv1beta1.DynamoGraphDeployment) { + worker := betaWorkerComponent(dgd) + worker.RuntimeVersionOverride = "" + worker.PodTemplate.Spec.Containers[0].Image = customRuntimeImage + }), + deployment: betaDGDForAdmission(func(dgd *nvidiacomv1beta1.DynamoGraphDeployment) { + worker := betaWorkerComponent(dgd) + worker.RuntimeVersionOverride = "" + worker.PodTemplate.Spec.Containers[0].Image = customRuntimeImage + dgd.Labels = map[string]string{"updated": "true"} + }), + }, + { + name: "unchanged legacy alpha service runtime version is ratcheted on update", + seedWithoutWebhook: true, + oldDeployment: alphaDGDForAdmission(func(dgd *nvidiacomv1alpha1.DynamoGraphDeployment) { + worker := dgd.Spec.Services["worker"] + worker.RuntimeVersionOverride = "" + worker.ExtraPodSpec.MainContainer.Image = customRuntimeImage + }), + deployment: alphaDGDForAdmission(func(dgd *nvidiacomv1alpha1.DynamoGraphDeployment) { + worker := dgd.Spec.Services["worker"] + worker.RuntimeVersionOverride = "" + worker.ExtraPodSpec.MainContainer.Image = customRuntimeImage + dgd.Labels = map[string]string{"updated": "true"} + }), + }, + { + name: "beta component image change to custom requires runtime version override", + oldDeployment: betaDGDForAdmission(func(dgd *nvidiacomv1beta1.DynamoGraphDeployment) { + worker := betaWorkerComponent(dgd) + worker.RuntimeVersionOverride = "" + }), + deployment: betaDGDForAdmission(func(dgd *nvidiacomv1beta1.DynamoGraphDeployment) { + worker := betaWorkerComponent(dgd) + worker.RuntimeVersionOverride = "" + worker.PodTemplate.Spec.Containers[0].Image = customRuntimeImage + }), + wantWebhookErrs: []string{"spec.components[1].runtimeVersionOverride: Required value: is required when the specified main container image has no parseable semantic-version tag"}, + }, + { + name: "changing a legacy alpha custom image requires runtime version override", + seedWithoutWebhook: true, + oldDeployment: alphaDGDForAdmission(func(dgd *nvidiacomv1alpha1.DynamoGraphDeployment) { + worker := dgd.Spec.Services["worker"] + worker.RuntimeVersionOverride = "" + worker.ExtraPodSpec.MainContainer.Image = customRuntimeImage + }), + deployment: alphaDGDForAdmission(func(dgd *nvidiacomv1alpha1.DynamoGraphDeployment) { + worker := dgd.Spec.Services["worker"] + worker.RuntimeVersionOverride = "" + worker.ExtraPodSpec.MainContainer.Image = "registry.example/runtime:other-custom" }), wantWebhookErrs: []string{"spec.services[worker].runtimeVersionOverride: Required value: is required when the specified main container image has no parseable semantic-version tag"}, }, @@ -1666,18 +1725,19 @@ func TestDynamoGraphDeploymentValidator_Validate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { gates := features.Gates{Checkpoint: !tt.checkpointOff, Grove: !tt.groveDisabled} test := admissionTestCase{ - object: tt.deployment, - oldObject: tt.oldDeployment, - mutateObject: tt.mutateRequest, - gates: gates, - withoutTopology: tt.withoutTopology, - username: tt.username, - wantSchemaError: tt.wantSchemaErr, - wantCELError: tt.wantCELErr, - wantAdmissionErrs: tt.wantAdmissionErrs, - wantWebhookErrors: tt.wantWebhookErrs, - wantWarnings: tt.wantWarnings, - notWantError: tt.notWantErr, + object: tt.deployment, + oldObject: tt.oldDeployment, + mutateObject: tt.mutateRequest, + gates: gates, + withoutTopology: tt.withoutTopology, + seedWithoutWebhook: tt.seedWithoutWebhook, + username: tt.username, + wantSchemaError: tt.wantSchemaErr, + wantCELError: tt.wantCELErr, + wantAdmissionErrs: tt.wantAdmissionErrs, + wantWebhookErrors: tt.wantWebhookErrs, + wantWarnings: tt.wantWarnings, + notWantError: tt.notWantErr, } if tt.oldDeployment != nil { test.oldBeforeUpdate = dgdBeforeRestart(t, tt.oldDeployment) diff --git a/deploy/operator/internal/webhook/validation/dynamographdeploymentrequest.go b/deploy/operator/internal/webhook/validation/dynamographdeploymentrequest.go index 63f61a4ba9f5..5fbd9793eb54 100644 --- a/deploy/operator/internal/webhook/validation/dynamographdeploymentrequest.go +++ b/deploy/operator/internal/webhook/validation/dynamographdeploymentrequest.go @@ -118,7 +118,7 @@ func (v *dynamoGraphDeploymentRequestValidation) validateDynamoGraphDeploymentRe } // validateDynamoGraphDeploymentRequestSpecUpdate validates a spec update. -// newSpec, oldSpec, and fldPath must not be nil; oldPhase comes from the owning old resource status. +// newSpec, oldSpec, and fldPath must not be nil; oldPhase comes from the owning old resource. func (v *dynamoGraphDeploymentRequestValidation) validateDynamoGraphDeploymentRequestSpecUpdate( newSpec *nvidiacomv1beta1.DynamoGraphDeploymentRequestSpec, oldSpec *nvidiacomv1beta1.DynamoGraphDeploymentRequestSpec, @@ -149,14 +149,17 @@ func (v *dynamoGraphDeploymentRequestValidation) validateDynamoGraphDeploymentRe )) } - if dgdrRuntimeVersionOverrideRequired(newSpec) { + if dgdrRuntimeVersionOverrideRequired(newSpec) && + (newSpec.Image != oldSpec.Image || newSpec.RuntimeVersionOverride != oldSpec.RuntimeVersionOverride) { allErrs = append(allErrs, field.Required( fldPath.Child("runtimeVersionOverride"), "is required when spec.image has no parseable semantic-version tag", )) } - if isImmutableDGDRPhase(oldPhase) && !apiequality.Semantic.DeepEqual(newSpec, oldSpec) { + if isImmutableDGDRPhase(oldPhase) && + !apiequality.Semantic.DeepEqual(newSpec, oldSpec) && + !isDGDRRuntimeVersionOverrideRepair(newSpec, oldSpec) { allErrs = append(allErrs, field.Forbidden( fldPath, fmt.Sprintf("updates are forbidden while the resource is in phase %q; delete and recreate the resource to change its spec", oldPhase), @@ -173,3 +176,19 @@ func dgdrRuntimeVersionOverrideRequired(spec *nvidiacomv1beta1.DynamoGraphDeploy _, err := runtimeversion.ParseImageVersion(spec.Image) return err != nil } + +// isDGDRRuntimeVersionOverrideRepair reports whether an update only repairs a missing required override. +func isDGDRRuntimeVersionOverrideRepair( + newSpec *nvidiacomv1beta1.DynamoGraphDeploymentRequestSpec, + oldSpec *nvidiacomv1beta1.DynamoGraphDeploymentRequestSpec, +) bool { + if !dgdrRuntimeVersionOverrideRequired(oldSpec) || + newSpec.RuntimeVersionOverride == oldSpec.RuntimeVersionOverride || + dgdrRuntimeVersionOverrideRequired(newSpec) { + return false + } + + newWithoutRepair := newSpec.DeepCopy() + newWithoutRepair.RuntimeVersionOverride = oldSpec.RuntimeVersionOverride + return apiequality.Semantic.DeepEqual(newWithoutRepair, oldSpec) +} diff --git a/deploy/operator/internal/webhook/validation/dynamographdeploymentrequest_helpers.go b/deploy/operator/internal/webhook/validation/dynamographdeploymentrequest_helpers.go index 1e80974f99ff..ba82cd86712a 100644 --- a/deploy/operator/internal/webhook/validation/dynamographdeploymentrequest_helpers.go +++ b/deploy/operator/internal/webhook/validation/dynamographdeploymentrequest_helpers.go @@ -32,6 +32,7 @@ func isImmutableDGDRPhase(phase nvidiacomv1beta1.DGDRPhase) bool { switch phase { case nvidiacomv1beta1.DGDRPhaseProfiling, nvidiacomv1beta1.DGDRPhaseDeploying, + nvidiacomv1beta1.DGDRPhaseReady, nvidiacomv1beta1.DGDRPhaseDeployed: return true default: diff --git a/deploy/operator/internal/webhook/validation/dynamographdeploymentrequest_validation_envtest_test.go b/deploy/operator/internal/webhook/validation/dynamographdeploymentrequest_validation_envtest_test.go index 37c101f5280c..b50855757756 100644 --- a/deploy/operator/internal/webhook/validation/dynamographdeploymentrequest_validation_envtest_test.go +++ b/deploy/operator/internal/webhook/validation/dynamographdeploymentrequest_validation_envtest_test.go @@ -23,6 +23,7 @@ import ( nvidiacomv1alpha1 "github.com/ai-dynamo/dynamo/deploy/operator/api/v1alpha1" nvidiacomv1beta1 "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" "github.com/ai-dynamo/dynamo/deploy/operator/internal/consts" + "github.com/ai-dynamo/dynamo/deploy/operator/internal/dgdrutil" "github.com/ai-dynamo/dynamo/deploy/operator/internal/features" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -225,31 +226,44 @@ func TestDynamoGraphDeploymentRequestValidator_Validate(t *testing.T) { gpuDiscovery: true, }, { - name: "legacy custom image without override must be repaired on update", + name: "unchanged legacy custom image without override is ratcheted on update", seedWithoutWebhook: true, oldRequest: betaDGDRForAdmission(func(request *nvidiacomv1beta1.DynamoGraphDeploymentRequest) { request.Spec.RuntimeVersionOverride = "" + request.Status.Phase = nvidiacomv1beta1.DGDRPhaseDeployed }), request: betaDGDRForAdmission(func(request *nvidiacomv1beta1.DynamoGraphDeploymentRequest) { request.Spec.RuntimeVersionOverride = "" + request.Status.Phase = nvidiacomv1beta1.DGDRPhaseDeployed request.Labels = map[string]string{"updated": "true"} }), gpuDiscovery: true, - wantWebhook: []string{ - "spec.runtimeVersionOverride: Required value: is required when spec.image has no parseable semantic-version tag", - }, }, { - name: "adding runtime version override repairs legacy custom image", + name: "adding runtime version override repairs legacy custom image in immutable phase", seedWithoutWebhook: true, oldRequest: betaDGDRForAdmission(func(request *nvidiacomv1beta1.DynamoGraphDeploymentRequest) { request.Spec.RuntimeVersionOverride = "" + request.Status.Phase = nvidiacomv1beta1.DGDRPhaseDeployed }), request: betaDGDRForAdmission(func(request *nvidiacomv1beta1.DynamoGraphDeploymentRequest) { + request.Status.Phase = nvidiacomv1beta1.DGDRPhaseDeployed request.Labels = map[string]string{"updated": "true"} }), gpuDiscovery: true, }, + { + name: "runtime version override repair does not require observed fingerprint", + seedWithoutWebhook: true, + oldRequest: betaDGDRForAdmissionWithoutFingerprint(func(request *nvidiacomv1beta1.DynamoGraphDeploymentRequest) { + request.Spec.RuntimeVersionOverride = "" + request.Status.Phase = nvidiacomv1beta1.DGDRPhaseDeployed + }), + request: betaDGDRForAdmission(func(request *nvidiacomv1beta1.DynamoGraphDeploymentRequest) { + request.Status.Phase = nvidiacomv1beta1.DGDRPhaseDeployed + }), + gpuDiscovery: true, + }, { name: "newly introduced custom image without override is rejected on update", oldRequest: betaDGDRForAdmission(func(request *nvidiacomv1beta1.DynamoGraphDeploymentRequest) { @@ -264,6 +278,21 @@ func TestDynamoGraphDeploymentRequestValidator_Validate(t *testing.T) { "spec.runtimeVersionOverride: Required value: is required when spec.image has no parseable semantic-version tag", }, }, + { + name: "changing a legacy custom image without override is rejected on update", + seedWithoutWebhook: true, + oldRequest: betaDGDRForAdmission(func(request *nvidiacomv1beta1.DynamoGraphDeploymentRequest) { + request.Spec.RuntimeVersionOverride = "" + }), + request: betaDGDRForAdmission(func(request *nvidiacomv1beta1.DynamoGraphDeploymentRequest) { + request.Spec.Image = "test-profiler:other-custom" + request.Spec.RuntimeVersionOverride = "" + }), + gpuDiscovery: true, + wantWebhook: []string{ + "spec.runtimeVersionOverride: Required value: is required when spec.image has no parseable semantic-version tag", + }, + }, { name: "missing hardware is ratcheted when GPU discovery becomes disabled", oldRequest: betaDGDRForAdmission(nil), @@ -348,6 +377,15 @@ func betaDGDRForAdmission( if mutate != nil { mutate(request) } + request.Status.ObservedSpecFingerprint, _ = dgdrutil.SpecFingerprint(&request.Spec) + return request +} + +func betaDGDRForAdmissionWithoutFingerprint( + mutate func(*nvidiacomv1beta1.DynamoGraphDeploymentRequest), +) *nvidiacomv1beta1.DynamoGraphDeploymentRequest { + request := betaDGDRForAdmission(mutate) + request.Status.ObservedSpecFingerprint = "" return request } diff --git a/deploy/operator/internal/webhook/validation/shared_helpers.go b/deploy/operator/internal/webhook/validation/shared_helpers.go index 1e3c272057df..69d224a317aa 100644 --- a/deploy/operator/internal/webhook/validation/shared_helpers.go +++ b/deploy/operator/internal/webhook/validation/shared_helpers.go @@ -27,8 +27,10 @@ import ( "github.com/ai-dynamo/dynamo/deploy/operator/internal/consts" "github.com/ai-dynamo/dynamo/deploy/operator/internal/dynamo/epp" "github.com/ai-dynamo/dynamo/deploy/operator/internal/features" + "github.com/ai-dynamo/dynamo/deploy/operator/internal/runtimeversion" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) @@ -38,6 +40,8 @@ const ( vllmDistributedExecutorBackendMP = "mp" vllmDistributedExecutorBackendRay = "ray" + + runtimeVersionOverrideRequiredMessage = "is required when the specified main container image has no parseable semantic-version tag" ) // runtimeVersionValidationSource identifies the API representation whose field @@ -47,6 +51,7 @@ type runtimeVersionValidationSource uint8 const ( runtimeVersionSourceV1Beta1 runtimeVersionValidationSource = iota runtimeVersionSourceV1Alpha1 + runtimeVersionSourceDisabled ) // runtimeVersionValidationSourceForRequest uses RequestKind because it preserves @@ -76,6 +81,46 @@ func (v *sharedValidation) validatesRuntimeVersionFor(source runtimeVersionValid return v.runtimeVersionSource == source } +// runtimeVersionImageAndPath returns the main image and its v1beta1 field path. +// spec and fldPath must not be nil. +func runtimeVersionImageAndPath( + spec *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec, + fldPath *field.Path, +) (string, *field.Path) { + imagePath := fldPath.Child("podTemplate", "spec", "containers") + + // Resolve the exact container path when the named main container exists. + if spec.PodTemplate != nil { + if index := containerIndexByName(spec.PodTemplate.Spec.Containers, consts.MainContainerName); index >= 0 { + imagePath = imagePath.Index(index).Child("image") + return spec.PodTemplate.Spec.Containers[index].Image, imagePath + } + } + return "", imagePath +} + +// runtimeVersionImageAndPathV1Alpha1 returns the main image and its v1alpha1 field path. +// spec and fldPath must not be nil. +func runtimeVersionImageAndPathV1Alpha1( + spec *nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec, + fldPath *field.Path, +) (string, *field.Path) { + imagePath := fldPath.Child("extraPodSpec", "mainContainer", "image") + if spec.ExtraPodSpec != nil && spec.ExtraPodSpec.MainContainer != nil { + return spec.ExtraPodSpec.MainContainer.Image, imagePath + } + return "", imagePath +} + +// runtimeVersionOverrideRequired reports whether image cannot provide a version and override is absent. +func runtimeVersionOverrideRequired(image, override string) bool { + if override != "" { + return false + } + _, err := runtimeversion.ParseImageVersion(image) + return err != nil +} + func hasContainerNamed(containers []corev1.Container, name string) bool { for i := range containers { if containers[i].Name == name { diff --git a/deploy/operator/internal/webhook/validation/shared_v1alpha1.go b/deploy/operator/internal/webhook/validation/shared_v1alpha1.go index 2c67bd746199..27a65d4003a0 100644 --- a/deploy/operator/internal/webhook/validation/shared_v1alpha1.go +++ b/deploy/operator/internal/webhook/validation/shared_v1alpha1.go @@ -22,7 +22,6 @@ import ( nvidiacomv1alpha1 "github.com/ai-dynamo/dynamo/deploy/operator/api/v1alpha1" "github.com/ai-dynamo/dynamo/deploy/operator/internal/consts" - "github.com/ai-dynamo/dynamo/deploy/operator/internal/runtimeversion" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -77,9 +76,44 @@ func (v *sharedValidation) validateDynamoComponentDeploymentSharedSpecV1alpha1( if spec.Failover != nil { allErrs = append(allErrs, v.validateFailoverSpecV1alpha1(spec.Failover, fldPath.Child("failover"))...) } + + // Validate runtime compatibility against the source-version fields. if v.validatesRuntimeVersionFor(runtimeVersionSourceV1Alpha1) { - if err := runtimeVersionOverrideErrorV1Alpha1(spec, fldPath); err != nil { - allErrs = append(allErrs, err) + image, imagePath := runtimeVersionImageAndPathV1Alpha1(spec, fldPath) + if image == "" { + allErrs = append(allErrs, field.Required(imagePath, "is required")) + } else if runtimeVersionOverrideRequired(image, spec.RuntimeVersionOverride) { + allErrs = append(allErrs, field.Required( + fldPath.Child("runtimeVersionOverride"), + runtimeVersionOverrideRequiredMessage, + )) + } + } + + return allErrs +} + +// validateDynamoComponentDeploymentSharedSpecUpdateV1alpha1 validates a preserved v1alpha1 shared spec update. +// newSpec, oldSpec, and fldPath must not be nil. +func (v *sharedValidation) validateDynamoComponentDeploymentSharedSpecUpdateV1alpha1( + newSpec *nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec, + oldSpec *nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec, + fldPath *field.Path, +) field.ErrorList { + allErrs := field.ErrorList{} + + // Ratchet an unchanged legacy tuple but reject a missing image or newly invalid tuple. + if v.validatesRuntimeVersionFor(runtimeVersionSourceV1Alpha1) { + newImage, imagePath := runtimeVersionImageAndPathV1Alpha1(newSpec, fldPath) + oldImage, _ := runtimeVersionImageAndPathV1Alpha1(oldSpec, fldPath) + if newImage == "" { + allErrs = append(allErrs, field.Required(imagePath, "is required")) + } else if runtimeVersionOverrideRequired(newImage, newSpec.RuntimeVersionOverride) && + (newImage != oldImage || newSpec.RuntimeVersionOverride != oldSpec.RuntimeVersionOverride) { + allErrs = append(allErrs, field.Required( + fldPath.Child("runtimeVersionOverride"), + runtimeVersionOverrideRequiredMessage, + )) } } @@ -140,24 +174,3 @@ func (v *sharedValidation) validateFailoverSpecV1alpha1( fmt.Sprintf("is invalid for mode=%q: intraPod uses a fixed 1 primary + 1 shadow sidecar; use failover.mode=%q to configure numShadows", nvidiacomv1alpha1.GMSModeIntraPod, nvidiacomv1alpha1.GMSModeInterPod), )} } - -func runtimeVersionOverrideErrorV1Alpha1( - spec *nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec, - fldPath *field.Path, -) *field.Error { - overridePath := fldPath.Child("runtimeVersionOverride") - image := "" - if spec.ExtraPodSpec != nil && spec.ExtraPodSpec.MainContainer != nil { - image = spec.ExtraPodSpec.MainContainer.Image - } - if image == "" { - return field.Required(fldPath.Child("extraPodSpec", "mainContainer", "image"), "is required") - } - if spec.RuntimeVersionOverride != "" { - return nil - } - if _, err := runtimeversion.ParseImageVersion(image); err != nil { - return field.Required(overridePath, "is required when the specified main container image has no parseable semantic-version tag") - } - return nil -} diff --git a/deploy/operator/internal/webhook/validation/shared_v1beta1.go b/deploy/operator/internal/webhook/validation/shared_v1beta1.go index 03db5c94db1f..6d840fb6c309 100644 --- a/deploy/operator/internal/webhook/validation/shared_v1beta1.go +++ b/deploy/operator/internal/webhook/validation/shared_v1beta1.go @@ -22,11 +22,9 @@ import ( "fmt" nvidiacomv1beta1 "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" - "github.com/ai-dynamo/dynamo/deploy/operator/internal/consts" "github.com/ai-dynamo/dynamo/deploy/operator/internal/dra" "github.com/ai-dynamo/dynamo/deploy/operator/internal/dynamo" "github.com/ai-dynamo/dynamo/deploy/operator/internal/features" - "github.com/ai-dynamo/dynamo/deploy/operator/internal/runtimeversion" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" @@ -127,9 +125,16 @@ func (v *sharedValidation) validateDynamoComponentDeploymentSharedSpec( )...) } + // Validate runtime compatibility against the source-version fields. if v.validatesRuntimeVersionFor(runtimeVersionSourceV1Beta1) { - if err := runtimeVersionOverrideError(spec, fldPath); err != nil { - allErrs = append(allErrs, err) + image, imagePath := runtimeVersionImageAndPath(spec, fldPath) + if image == "" { + allErrs = append(allErrs, field.Required(imagePath, "is required")) + } else if runtimeVersionOverrideRequired(image, spec.RuntimeVersionOverride) { + allErrs = append(allErrs, field.Required( + fldPath.Child("runtimeVersionOverride"), + runtimeVersionOverrideRequiredMessage, + )) } } @@ -398,6 +403,21 @@ func (v *sharedValidation) validateDynamoComponentDeploymentSharedSpecUpdate( )) } } + + // Ratchet an unchanged legacy tuple but reject a missing image or newly invalid tuple. + if v.validatesRuntimeVersionFor(runtimeVersionSourceV1Beta1) { + newImage, imagePath := runtimeVersionImageAndPath(newComponent, fldPath) + oldImage, _ := runtimeVersionImageAndPath(oldComponent, fldPath) + if newImage == "" { + allErrs = append(allErrs, field.Required(imagePath, "is required")) + } else if runtimeVersionOverrideRequired(newImage, newComponent.RuntimeVersionOverride) && + (newImage != oldImage || newComponent.RuntimeVersionOverride != oldComponent.RuntimeVersionOverride) { + allErrs = append(allErrs, field.Required( + fldPath.Child("runtimeVersionOverride"), + runtimeVersionOverrideRequiredMessage, + )) + } + } return allErrs } @@ -457,28 +477,3 @@ func (v *sharedValidation) validateExperimentalSpecUpdate( } return allErrs } - -func runtimeVersionOverrideError( - spec *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec, - fldPath *field.Path, -) *field.Error { - overridePath := fldPath.Child("runtimeVersionOverride") - image := "" - imagePath := fldPath.Child("podTemplate", "spec", "containers") - if spec.PodTemplate != nil { - if index := containerIndexByName(spec.PodTemplate.Spec.Containers, consts.MainContainerName); index >= 0 { - image = spec.PodTemplate.Spec.Containers[index].Image - imagePath = imagePath.Index(index).Child("image") - } - } - if image == "" { - return field.Required(imagePath, "is required") - } - if spec.RuntimeVersionOverride != "" { - return nil - } - if _, err := runtimeversion.ParseImageVersion(image); err != nil { - return field.Required(overridePath, "is required when the specified main container image has no parseable semantic-version tag") - } - return nil -} diff --git a/deploy/operator/internal/webhook/validation/suite_envtest_test.go b/deploy/operator/internal/webhook/validation/suite_envtest_test.go index 2424a60299da..4c71c2d32bb3 100644 --- a/deploy/operator/internal/webhook/validation/suite_envtest_test.go +++ b/deploy/operator/internal/webhook/validation/suite_envtest_test.go @@ -31,6 +31,7 @@ import ( const ( admissionOperatorPrincipal = "system:serviceaccount:dynamo-system:dynamo-operator" + customRuntimeImage = "registry.example/runtime:custom" legacySeedUsername = "operatorenv-legacy-seeder" ) diff --git a/docs/fern/components/profiler/profiler-guide.md b/docs/fern/components/profiler/profiler-guide.md index d277e348d469..122b1d97cd67 100644 --- a/docs/fern/components/profiler/profiler-guide.md +++ b/docs/fern/components/profiler/profiler-guide.md @@ -208,12 +208,16 @@ spec: image: "nvcr.io/nvidia/ai-dynamo/dynamo-planner:1.2.1" # dynamo-frontend for Dynamo < 1.1.0 ``` -> [!WARNING] -> With Dynamo Operator 1.4.0 or later, an explicitly selected profiler based on -> Dynamo 1.3.0 or earlier must use a `spec.image` tag that accurately identifies -> its runtime with semantic versioning, such as `:1.3.0`. Older profilers -> silently discard the DGDR-level `spec.runtimeVersionOverride` field and -> therefore cannot propagate it into the generated DGD. +> [!NOTE] +> The DGDR-level `spec.runtimeVersionOverride` is authoritative for every +> component in the generated DGD. The operator applies it after processing +> profiler output and DGD overrides, including when a profiler based on Dynamo +> 1.3.0 or earlier discards the field while parsing the DGDR. Set the override +> when the effective generated runtime images do not use tags that identify +> their Dynamo runtime versions. +> The profiler also applies the field when its output is consumed directly, +> outside the operator-managed DGDR workflow. The operator remains authoritative +> for DGDR-managed deployments and reapplies the value after all DGD overrides. > > [Profiler Image Version Compatibility](../../kubernetes/dgdr-reference.mdx#profiler-image-version-compatibility) > for details. diff --git a/docs/fern/kubernetes/api-reference.md b/docs/fern/kubernetes/api-reference.md index e22ddeefc752..0d8b4824a3af 100644 --- a/docs/fern/kubernetes/api-reference.md +++ b/docs/fern/kubernetes/api-reference.md @@ -2313,6 +2313,7 @@ _Appears in:_ | `profilingResults` _[ProfilingResultsStatus](#profilingresultsstatus)_ | ProfilingResults contains the selected deployment configuration produced by profiling.
Deprecated compatibility fields may remain on objects created by older releases. | | Optional: \{\}
| | `deploymentInfo` _[DeploymentInfoStatus](#deploymentinfostatus)_ | DeploymentInfo tracks the state of the deployed DynamoGraphDeployment.
Populated when a DGD has been created (either via autoApply or manually). | | Optional: \{\}
| | `observedGeneration` _integer_ | ObservedGeneration is the most recent generation observed by the controller. | | Optional: \{\}
| +| `observedSpecFingerprint` _string_ | ObservedSpecFingerprint identifies the spec associated with ObservedGeneration.
The controller uses it to verify runtimeVersionOverride-only repairs. | | Optional: \{\}
| #### DynamoGraphDeploymentScalingAdapter diff --git a/docs/fern/kubernetes/dgdr-reference.mdx b/docs/fern/kubernetes/dgdr-reference.mdx index 43d901907bc7..6c94ec16e459 100644 --- a/docs/fern/kubernetes/dgdr-reference.mdx +++ b/docs/fern/kubernetes/dgdr-reference.mdx @@ -50,36 +50,30 @@ When `spec.image` is omitted, the operator defaults it on creation to requires `operatorVersion` to be valid semantic versioning, so the default image has a parseable version tag. - - When using Dynamo Operator 1.4.0 or later, explicitly setting `spec.image` to - a profiler based on Dynamo 1.3.0 or earlier requires a semantic-version tag - that accurately identifies the generated runtime version, such as - `dynamo-planner:1.3.0`. - - Profilers through 1.3.0 do not recognize the DGDR-level - `spec.runtimeVersionOverride` field and silently discard it while parsing the - DGDR configuration. These profilers derive generated DGD component images - from `spec.image` but cannot propagate the DGDR-level override into those - components. The DGD admission webhook must therefore infer compatibility from - the generated component image tags. Do not use tags such as `:latest`, - `:custom`, or `:sha-abc123` for this version combination. - - -Images supplied through `spec.overrides.dgd` follow the DGD component rules -independently. An overridden worker image with no parseable semantic-version tag -requires `runtimeVersionOverride` in the effective DGD component. - -Profilers from Dynamo 1.4.0 or later propagate the DGDR-level override to every -generated component, so `spec.runtimeVersionOverride` also covers a worker image -replaced through `overrides.dgd`. - -With a profiler based on Dynamo 1.3.0 or earlier, the DGDR-level field is -ignored. Those profilers also discard the embedded override's `apiVersion` and -`kind`, then directly merge the remaining dictionary into a generated +The DGDR-level `spec.runtimeVersionOverride` is authoritative for every +component in the generated DGD. After the profiler materializes the DGD and +applies `spec.overrides.dgd`, the operator copies the DGDR value to every +generated component before DGD admission. This behavior applies across profiler +versions. Profilers through Dynamo 1.3.0 may discard the field while parsing the +DGDR, but the operator reapplies it from the stored DGDR. + +Set `spec.runtimeVersionOverride` when an effective generated component image +uses a non-semantic-version tag or digest, or when its tag does not identify the +intended Dynamo runtime version. The DGDR value also covers images replaced +through `spec.overrides.dgd` and takes precedence over component-level values in +that override. + +When the DGDR-level field is unset, each effective generated DGD component +follows the DGD component rules independently. An overridden image without a +parseable semantic-version tag requires `runtimeVersionOverride` on that +component. + +Profilers through Dynamo 1.3.0 discard the embedded DGD override's `apiVersion` +and `kind`, then directly merge the remaining dictionary into a generated `v1alpha1` DGD. Raw storage preserves the override fields but does not convert -between DGD schemas. Use a `v1alpha1`-shaped override under `spec.services` and -set `runtimeVersionOverride` on the affected service. A `v1beta1` -`spec.components` override is not translated into the generated service. +between DGD schemas. Use a `v1alpha1`-shaped override under `spec.services`; a +`v1beta1` `spec.components` override is not translated into the generated +service. Expected workload characteristics for SLA-based profiling.