Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions components/src/dynamo/profiler/utils/dgdr_v1beta1_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
11 changes: 6 additions & 5 deletions deploy/helm/charts/platform/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 6 additions & 5 deletions deploy/helm/charts/platform/README.md.gotmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 8 additions & 0 deletions deploy/operator/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Comment thread
sttts marked this conversation as resolved.
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
}
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -2105,6 +2139,79 @@ func (r *DynamoGraphDeploymentRequestReconciler) generateDGDSpec(ctx context.Con
return profilingResults, dgd.Name, nil
}

// applyDGDRRuntimeVersionOverride makes the DGDR authoritative across profiler versions.
Comment thread
sttts marked this conversation as resolved.
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
Comment thread
sttts marked this conversation as resolved.
}
}

// 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.
Expand Down Expand Up @@ -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
Expand All @@ -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{
Expand All @@ -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).
Expand Down
Loading
Loading