From d8ff9e841d97f611d80b07e1fbf8ad2cc7cbaa7c Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 Jul 2026 12:54:03 +0200 Subject: [PATCH 01/10] operator/worker-hash: converge generations lazily to v2 Signed-off-by: Dr. Stefan Schimanski --- .../api/v1alpha1/legacy_worker_hash.go | 71 --- .../api/v1alpha1/legacy_worker_hash_test.go | 522 ------------------ .../shared_spec_conversion_bugs_test.go | 24 - deploy/operator/internal/consts/consts.go | 35 +- .../dynamographdeployment_controller.go | 31 +- .../dynamographdeployment_controller_test.go | 40 +- .../dynamographdeployment_rollingupdate.go | 260 ++++----- ...ynamographdeployment_rollingupdate_test.go | 329 +++++------ .../controller/test_beta_helpers_test.go | 9 - deploy/operator/internal/dynamo/hash.go | 12 - deploy/operator/internal/dynamo/hash_test.go | 66 --- 11 files changed, 283 insertions(+), 1116 deletions(-) delete mode 100644 deploy/operator/api/v1alpha1/legacy_worker_hash.go delete mode 100644 deploy/operator/api/v1alpha1/legacy_worker_hash_test.go diff --git a/deploy/operator/api/v1alpha1/legacy_worker_hash.go b/deploy/operator/api/v1alpha1/legacy_worker_hash.go deleted file mode 100644 index c1116679fe9b..000000000000 --- a/deploy/operator/api/v1alpha1/legacy_worker_hash.go +++ /dev/null @@ -1,71 +0,0 @@ -/* - * SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -package v1alpha1 - -import ( - "crypto/sha256" - "encoding/hex" - "encoding/json" - "fmt" - "sort" -) - -// ComputeDGDWorkersSpecHash computes the worker hash used by the -// v1alpha1 DGD controller. Controller reconciliation may use this to compare -// v1/v2 worker generations, but API conversion must not derive controller -// rollout state from it. -// -// Keep this in the api/v1alpha1 package so internal controller helpers can -// reproduce the legacy hash without duplicating the old algorithm. -func ComputeDGDWorkersSpecHash(dgd *DynamoGraphDeployment) (string, error) { - if dgd == nil { - return "", fmt.Errorf("nil DynamoGraphDeployment") - } - - var workerNames []string - for name, spec := range dgd.Spec.Services { - if spec != nil && isV1alpha1WorkerComponent(spec.ComponentType) { - workerNames = append(workerNames, name) - } - } - sort.Strings(workerNames) - - hashInputs := make(map[string]DynamoComponentDeploymentSharedSpec) - for _, name := range workerNames { - hashInputs[name] = stripV1alpha1NonPodTemplateFields(dgd.Spec.Services[name]) - } - - data, err := json.Marshal(hashInputs) - if err != nil { - return "", err - } - - hash := sha256.Sum256(data) - return hex.EncodeToString(hash[:])[:8], nil -} - -func isV1alpha1WorkerComponent(componentType string) bool { - return componentType == "worker" || componentType == "prefill" || componentType == "decode" -} - -func stripV1alpha1NonPodTemplateFields(spec *DynamoComponentDeploymentSharedSpec) DynamoComponentDeploymentSharedSpec { - stripped := *spec - - stripped.Annotations = nil - stripped.Labels = nil - stripped.ServiceName = "" - stripped.ComponentType = "" - stripped.SubComponentType = "" - stripped.DynamoNamespace = nil - stripped.Replicas = nil - stripped.Autoscaling = nil //nolint:staticcheck // SA1019: intentionally matching the old v1alpha1 worker hash - stripped.ScalingAdapter = nil - stripped.Ingress = nil - stripped.ModelRef = nil - stripped.EPPConfig = nil - - return stripped -} diff --git a/deploy/operator/api/v1alpha1/legacy_worker_hash_test.go b/deploy/operator/api/v1alpha1/legacy_worker_hash_test.go deleted file mode 100644 index d79c987311e6..000000000000 --- a/deploy/operator/api/v1alpha1/legacy_worker_hash_test.go +++ /dev/null @@ -1,522 +0,0 @@ -/* - * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -package v1alpha1 - -import ( - "testing" - "unicode/utf8" - - "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" - commonconsts "github.com/ai-dynamo/dynamo/deploy/operator/internal/consts" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/utils/ptr" -) - -const currentWorkerHashAnnotation = "nvidia.com/current-worker-hash" - -func TestComputeDGDWorkersSpecHashGolden(t *testing.T) { - for _, tt := range legacyWorkerHashGoldenCases() { - t.Run(tt.name, func(t *testing.T) { - got1, err := ComputeDGDWorkersSpecHash(tt.dgd) - if err != nil { - t.Fatalf("ComputeDGDWorkersSpecHash: %v", err) - } - got2, err := ComputeDGDWorkersSpecHash(tt.dgd) - if err != nil { - t.Fatalf("second ComputeDGDWorkersSpecHash: %v", err) - } - if got1 != got2 { - t.Fatalf("hash is not stable: first %q, second %q", got1, got2) - } - if got1 != tt.want { - t.Fatalf("ComputeDGDWorkersSpecHash() = %q, want golden %q", got1, tt.want) - } - }) - } -} - -type legacyWorkerHashGoldenCase struct { - name string - dgd *DynamoGraphDeployment - want string -} - -// legacyWorkerHashGoldenCases are golden values from the v1.1.x worker-hash -// algorithm in deploy/operator/internal/dynamo/hash.go. Changing any value here -// is a rollout-compatibility change and needs an explicit migration plan. -func legacyWorkerHashGoldenCases() []legacyWorkerHashGoldenCase { - return []legacyWorkerHashGoldenCase{ - { - name: "worker", - dgd: legacyWorkerHashDGD(), - want: "9b66accc", - }, - { - name: "resource metadata and non-workers ignored", - dgd: legacyWorkerHashDGDWithResourceMetadataAndNonWorkerChanges(), - want: "9b66accc", - }, - { - name: "pod metadata included", - dgd: legacyWorkerHashDGDWithPodMetadataChanges(), - want: "af8a6c60", - }, - { - name: "no workers", - dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "frontend": {ComponentType: commonconsts.ComponentTypeFrontend}, - }), - want: "44136fa3", - }, - { - name: "nil services", - dgd: legacyWorkerHashDGDFromServices(nil), - want: "44136fa3", - }, - { - name: "worker ordering", - dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "z-worker": {ComponentType: commonconsts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "Z", Value: "1"}}}, - "a-worker": {ComponentType: commonconsts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "A", Value: "1"}}}, - }), - want: "59cae8b3", - }, - { - name: "all worker component types", - dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "decode": {ComponentType: commonconsts.ComponentTypeDecode, Envs: []corev1.EnvVar{{Name: "ROLE", Value: "decode"}}}, - "prefill": {ComponentType: commonconsts.ComponentTypePrefill, Envs: []corev1.EnvVar{{Name: "ROLE", Value: "prefill"}}}, - "worker": {ComponentType: commonconsts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "ROLE", Value: "worker"}}}, - }), - want: "b175ee30", - }, - { - name: "main container name only", - dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - ExtraPodSpec: &ExtraPodSpec{ - MainContainer: &corev1.Container{Name: commonconsts.MainContainerName}, - }, - }, - }), - want: "0c322ce0", - }, - { - name: "main container rich", - dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - ExtraPodSpec: &ExtraPodSpec{ - MainContainer: &corev1.Container{ - Name: commonconsts.MainContainerName, - Image: "worker:1", - Command: []string{"python", "-m", "server"}, - Args: []string{"--model", "qwen"}, - Env: []corev1.EnvVar{{Name: "EXTRA", Value: "true"}}, - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("2")}, - }, - }, - }, - }, - }), - want: "9e64367a", - }, - { - name: "pod spec rich", - dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - ExtraPodSpec: &ExtraPodSpec{ - PodSpec: &corev1.PodSpec{ - NodeSelector: map[string]string{"gpu": "true"}, - Tolerations: []corev1.Toleration{{ - Key: "nvidia.com/gpu", - Operator: corev1.TolerationOpExists, - }}, - Volumes: []corev1.Volume{{ - Name: "cache", - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, - }, - }}, - }, - }, - }, - }), - want: "1fcd5d7c", - }, - { - name: "resources requests limits claims", - dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - Resources: &Resources{ - Requests: &ResourceItem{CPU: "1", Memory: "4Gi", GPU: "1", GPUType: "nvidia.com/gpu"}, - Limits: &ResourceItem{CPU: "2", Memory: "8Gi", GPU: "1"}, - Claims: []corev1.ResourceClaim{{Name: "gpu-claim"}}, - }, - }, - }), - want: "a4172e4e", - }, - { - name: "envs volume mounts secret", - dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - Envs: []corev1.EnvVar{{Name: "MODEL", Value: "llama"}}, - EnvFromSecret: ptr.To("worker-secret"), - VolumeMounts: []VolumeMount{{Name: "models", MountPoint: "/models", UseAsCompilationCache: true}}, - SharedMemory: &SharedMemorySpec{Size: resource.MustParse("2Gi")}, - LivenessProbe: &corev1.Probe{InitialDelaySeconds: 5}, - ReadinessProbe: &corev1.Probe{TimeoutSeconds: 3}, - }, - }), - want: "a0ceefd2", - }, - { - name: "multiple compilation cache volume mounts", - dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - VolumeMounts: []VolumeMount{ - {Name: "model-cache", MountPoint: "/models", UseAsCompilationCache: true}, - {Name: "compile-cache", MountPoint: "/compile", UseAsCompilationCache: true}, - }, - }, - }), - want: "5a3c0f65", - }, - { - name: "ignored scaling ingress model", - dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - Annotations: map[string]string{"ignored": "true"}, - Labels: map[string]string{"ignored": "true"}, - Replicas: ptr.To(int32(3)), - Autoscaling: &Autoscaling{Enabled: true, MinReplicas: 1, MaxReplicas: 10}, - Ingress: &IngressSpec{Enabled: true, Host: "example.com"}, - ModelRef: &ModelReference{Name: "model"}, - ScalingAdapter: &ScalingAdapter{Enabled: true}, - }, - }), - want: "769fa7c7", - }, - { - name: "multinode sidecar checkpoint", - dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - Multinode: &MultinodeSpec{NodeCount: 2}, - FrontendSidecar: &FrontendSidecarSpec{ - Image: "frontend:1", - Args: []string{"--router-mode", "direct"}, - }, - Checkpoint: &ServiceCheckpointConfig{Enabled: true}, - }, - }), - want: "bf66a8e3", - }, - { - name: "probe handler", - dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - ReadinessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/health", - Port: intstr.FromString("http"), - }, - }, - }, - }, - }), - want: "8ddbbadc", - }, - } -} - -func TestComputeDGDWorkersSpecHashNil(t *testing.T) { - if _, err := ComputeDGDWorkersSpecHash(nil); err == nil { - t.Fatal("ComputeDGDWorkersSpecHash(nil) error = nil, want error") - } -} - -func TestDGDConvertToPreservesWorkerHashAnnotationOpaquely(t *testing.T) { - src := legacyWorkerHashDGD() - src.Annotations = map[string]string{ - currentWorkerHashAnnotation: "controller-owned-hash", - "user": "kept", - } - - hub := &v1beta1.DynamoGraphDeployment{} - if err := src.ConvertTo(hub); err != nil { - t.Fatalf("ConvertTo: %v", err) - } - - if got := hub.Annotations[currentWorkerHashAnnotation]; got != "controller-owned-hash" { - t.Fatalf("%s = %q, want controller-owned-hash", currentWorkerHashAnnotation, got) - } - if got := hub.Annotations["user"]; got != "kept" { - t.Fatalf("user annotation = %q, want kept", got) - } -} - -func TestComputeDGDWorkersSpecHashConversionRoundTripExact(t *testing.T) { - alpha := legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - ExtraPodSpec: &ExtraPodSpec{ - MainContainer: &corev1.Container{Name: commonconsts.MainContainerName}, - }, - }, - }) - - directHash, err := ComputeDGDWorkersSpecHash(alpha) - if err != nil { - t.Fatalf("ComputeDGDWorkersSpecHash(alpha): %v", err) - } - roundTripHash, err := legacyWorkerHashAfterBetaRoundTrip(alpha) - if err != nil { - t.Fatalf("legacyWorkerHashAfterBetaRoundTrip: %v", err) - } - - if directHash != "0c322ce0" { - t.Fatalf("direct alpha hash = %q, want 0c322ce0", directHash) - } - if roundTripHash != directHash { - t.Fatalf("round-tripped alpha hash = %q, want direct alpha hash %q", roundTripHash, directHash) - } -} - -func TestComputeDGDWorkersSpecHashConversionRoundTripStableSubset(t *testing.T) { - for _, tt := range legacyWorkerHashGoldenCases() { - t.Run(tt.name, func(t *testing.T) { - roundTripHash, err := legacyWorkerHashAfterBetaRoundTrip(tt.dgd) - if err != nil { - t.Fatalf("legacyWorkerHashAfterBetaRoundTrip: %v", err) - } - if roundTripHash != tt.want { - t.Fatalf("round-tripped alpha hash = %q, want direct golden %q", roundTripHash, tt.want) - } - }) - } -} - -func TestComputeDGDWorkersSpecHashTracksPodMetadata(t *testing.T) { - baseHash, err := ComputeDGDWorkersSpecHash(legacyWorkerHashDGD()) - if err != nil { - t.Fatalf("ComputeDGDWorkersSpecHash: %v", err) - } - - mutated := legacyWorkerHashDGDWithPodMetadataChanges() - - got, err := ComputeDGDWorkersSpecHash(mutated) - if err != nil { - t.Fatalf("ComputeDGDWorkersSpecHash: %v", err) - } - if got == baseHash { - t.Fatalf("pod metadata change did not change preserved legacy worker hash: %q", got) - } -} - -func TestComputeDGDWorkersSpecHashIgnoresResourceMetadataAndNonWorkers(t *testing.T) { - baseHash, err := ComputeDGDWorkersSpecHash(legacyWorkerHashDGD()) - if err != nil { - t.Fatalf("ComputeDGDWorkersSpecHash: %v", err) - } - - mutated := legacyWorkerHashDGDWithResourceMetadataAndNonWorkerChanges() - - got, err := ComputeDGDWorkersSpecHash(mutated) - if err != nil { - t.Fatalf("ComputeDGDWorkersSpecHash: %v", err) - } - if got != baseHash { - t.Fatalf("non-pod-template metadata/non-worker changes changed preserved legacy worker hash: got %q, want %q", got, baseHash) - } -} - -func FuzzComputeDGDWorkersSpecHashDeterministic(f *testing.F) { - f.Add("worker", "MODEL", "llama", "checksum/config", "base", false) - f.Add("prefill", "ROLE", "prefill", "rollout", "v1", true) - f.Add("decode", "", "", "", "", false) - - f.Fuzz(func(t *testing.T, componentType, envName, envValue, metadataKey, metadataValue string, includeMainContainer bool) { - switch componentType { - case commonconsts.ComponentTypeWorker, commonconsts.ComponentTypePrefill, commonconsts.ComponentTypeDecode: - default: - componentType = commonconsts.ComponentTypeWorker - } - - spec := &DynamoComponentDeploymentSharedSpec{ - ComponentType: componentType, - Envs: []corev1.EnvVar{{Name: envName, Value: envValue}}, - ExtraPodMetadata: &ExtraPodMetadata{ - Labels: map[string]string{metadataKey: metadataValue}, - }, - } - if includeMainContainer { - spec.ExtraPodSpec = &ExtraPodSpec{ - MainContainer: &corev1.Container{Name: commonconsts.MainContainerName, Image: envValue}, - } - } - - dgd := legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "worker": spec, - }) - got1, err := ComputeDGDWorkersSpecHash(dgd) - if err != nil { - t.Fatalf("ComputeDGDWorkersSpecHash: %v", err) - } - got2, err := ComputeDGDWorkersSpecHash(dgd) - if err != nil { - t.Fatalf("second ComputeDGDWorkersSpecHash: %v", err) - } - if got1 != got2 { - t.Fatalf("hash is not deterministic: first %q, second %q", got1, got2) - } - if len(got1) != 8 { - t.Fatalf("hash length = %d, want 8: %q", len(got1), got1) - } - }) -} - -func FuzzComputeDGDWorkersSpecHashConversionRoundTripStableSubset(f *testing.F) { - f.Add("worker", "MODEL", "llama", "rollout", "base", "1", "4Gi") - f.Add("prefill", "ROLE", "prefill", "checksum", "v1", "", "") - f.Add("decode", "", "", "", "", "250m", "1Gi") - - f.Fuzz(func(t *testing.T, componentType, envName, envValue, metadataKey, metadataValue, cpu, memory string) { - for _, value := range []string{componentType, envName, envValue, metadataKey, metadataValue, cpu, memory} { - if !utf8.ValidString(value) { - t.Skip("Kubernetes JSON strings are valid UTF-8") - } - } - if cpu != "" { - if _, err := resource.ParseQuantity(cpu); err != nil { - t.Skip("invalid CPU quantity") - } - } - if memory != "" { - if _, err := resource.ParseQuantity(memory); err != nil { - t.Skip("invalid memory quantity") - } - } - switch componentType { - case commonconsts.ComponentTypeWorker, commonconsts.ComponentTypePrefill, commonconsts.ComponentTypeDecode: - default: - componentType = commonconsts.ComponentTypeWorker - } - - alpha := legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: componentType, - Envs: []corev1.EnvVar{{Name: envName, Value: envValue}}, - ExtraPodMetadata: &ExtraPodMetadata{ - Labels: map[string]string{metadataKey: metadataValue}, - }, - Resources: &Resources{ - Requests: &ResourceItem{CPU: cpu, Memory: memory}, - }, - }, - }) - - directHash, err := ComputeDGDWorkersSpecHash(alpha) - if err != nil { - t.Fatalf("ComputeDGDWorkersSpecHash(alpha): %v", err) - } - roundTripHash, err := legacyWorkerHashAfterBetaRoundTrip(alpha) - if err != nil { - t.Fatalf("legacyWorkerHashAfterBetaRoundTrip: %v", err) - } - if directHash != roundTripHash { - t.Fatalf("round-trip changed stable-subset hash: direct %q, round-trip %q", directHash, roundTripHash) - } - }) -} - -func legacyWorkerHashAfterBetaRoundTrip(src *DynamoGraphDeployment) (string, error) { - hub := &v1beta1.DynamoGraphDeployment{} - if err := src.ConvertTo(hub); err != nil { - return "", err - } - - roundTripped := &DynamoGraphDeployment{} - if err := roundTripped.ConvertFrom(hub); err != nil { - return "", err - } - return ComputeDGDWorkersSpecHash(roundTripped) -} - -func legacyWorkerHashDGDFromServices(services map[string]*DynamoComponentDeploymentSharedSpec) *DynamoGraphDeployment { - return &DynamoGraphDeployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "legacy-worker-hash", - Namespace: "ns", - }, - Spec: DynamoGraphDeploymentSpec{ - Services: services, - }, - } -} - -func legacyWorkerHashDGD() *DynamoGraphDeployment { - return &DynamoGraphDeployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "legacy-worker-hash", - Namespace: "ns", - }, - Spec: DynamoGraphDeploymentSpec{ - Services: map[string]*DynamoComponentDeploymentSharedSpec{ - "frontend": { - ComponentType: commonconsts.ComponentTypeFrontend, - Envs: []corev1.EnvVar{{Name: "FRONTEND_ONLY", Value: "ignored"}}, - }, - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - Annotations: map[string]string{"resource": "base"}, - Labels: map[string]string{"resource": "base"}, - Replicas: ptr.To(int32(2)), - Envs: []corev1.EnvVar{{Name: "MODEL", Value: "llama"}}, - ExtraPodMetadata: &ExtraPodMetadata{ - Labels: map[string]string{"rollout": "base"}, - Annotations: map[string]string{"checksum/config": "base"}, - }, - }, - }, - }, - } -} - -func legacyWorkerHashDGDWithPodMetadataChanges() *DynamoGraphDeployment { - mutated := legacyWorkerHashDGD() - mutated.Spec.Services["worker"].ExtraPodMetadata = &ExtraPodMetadata{ - Labels: map[string]string{"rollout": "changed"}, - Annotations: map[string]string{"checksum/config": "changed"}, - } - return mutated -} - -func legacyWorkerHashDGDWithResourceMetadataAndNonWorkerChanges() *DynamoGraphDeployment { - mutated := legacyWorkerHashDGD() - mutated.Spec.Services["worker"].Annotations = map[string]string{"resource": "changed"} - mutated.Spec.Services["worker"].Labels = map[string]string{"resource": "changed"} - mutated.Spec.Services["worker"].Replicas = ptr.To(int32(99)) - mutated.Spec.Services["worker"].Ingress = &IngressSpec{ - Enabled: true, - Host: "changed.example.com", - } - mutated.Spec.Services["frontend"].Envs = []corev1.EnvVar{{Name: "FRONTEND_ONLY", Value: "changed"}} - return mutated -} diff --git a/deploy/operator/api/v1alpha1/shared_spec_conversion_bugs_test.go b/deploy/operator/api/v1alpha1/shared_spec_conversion_bugs_test.go index 7d8a7b7ed2d2..c62d3e6c5377 100644 --- a/deploy/operator/api/v1alpha1/shared_spec_conversion_bugs_test.go +++ b/deploy/operator/api/v1alpha1/shared_spec_conversion_bugs_test.go @@ -247,11 +247,6 @@ func TestBugDGD_SpokeMainContainerNameOnlyRoundTrips(t *testing.T) { }, }, } - wantHash, err := ComputeDGDWorkersSpecHash(in) - if err != nil { - t.Fatalf("ComputeDGDWorkersSpecHash(in) error = %v", err) - } - hub := &v1beta1.DynamoGraphDeployment{} if err := in.ConvertTo(hub); err != nil { t.Fatalf("ConvertTo() error = %v", err) @@ -268,13 +263,6 @@ func TestBugDGD_SpokeMainContainerNameOnlyRoundTrips(t *testing.T) { if got.ExtraPodSpec.MainContainer.Name != mainContainerName { t.Fatalf("mainContainer.name = %q, want %q", got.ExtraPodSpec.MainContainer.Name, mainContainerName) } - gotHash, err := ComputeDGDWorkersSpecHash(out) - if err != nil { - t.Fatalf("ComputeDGDWorkersSpecHash(out) error = %v", err) - } - if gotHash != wantHash { - t.Fatalf("round-trip worker hash = %q, want %q", gotHash, wantHash) - } } func TestBugDGD_SpokeMultipleCompilationCacheVolumeMountsRoundTrip(t *testing.T) { @@ -292,11 +280,6 @@ func TestBugDGD_SpokeMultipleCompilationCacheVolumeMountsRoundTrip(t *testing.T) }, }, } - wantHash, err := ComputeDGDWorkersSpecHash(in) - if err != nil { - t.Fatalf("ComputeDGDWorkersSpecHash(in) error = %v", err) - } - hub := &v1beta1.DynamoGraphDeployment{} if err := in.ConvertTo(hub); err != nil { t.Fatalf("ConvertTo() error = %v", err) @@ -319,13 +302,6 @@ func TestBugDGD_SpokeMultipleCompilationCacheVolumeMountsRoundTrip(t *testing.T) if diff := cmp.Diff(in.Spec.Services["worker"].VolumeMounts, out.Spec.Services["worker"].VolumeMounts); diff != "" { t.Fatalf("volume mounts changed after round-trip (-want +got):\n%s", diff) } - gotHash, err := ComputeDGDWorkersSpecHash(out) - if err != nil { - t.Fatalf("ComputeDGDWorkersSpecHash(out) error = %v", err) - } - if gotHash != wantHash { - t.Fatalf("round-trip worker hash = %q, want %q", gotHash, wantHash) - } } func TestBugDGD_ChangedCompilationCacheDoesNotRestoreStaleVolumeMounts(t *testing.T) { diff --git a/deploy/operator/internal/consts/consts.go b/deploy/operator/internal/consts/consts.go index db134cb8ebda..7c924562208f 100644 --- a/deploy/operator/internal/consts/consts.go +++ b/deploy/operator/internal/consts/consts.go @@ -75,11 +75,9 @@ const ( KubeAnnotationDynamoBaseModel = "nvidia.com/dynamo-base-model" KubeLabelDynamoDiscoveryBackend = "nvidia.com/dynamo-discovery-backend" KubeLabelDynamoDiscoveryEnabled = "nvidia.com/dynamo-discovery-enabled" - // KubeLabelDynamoWorkerHash is the worker generation label on worker DCDs - // and worker pods. During v1/v2 hash compatibility the label key remains - // stable and the value may be either the active v1 hash or the active v2 hash - // recorded on the parent DGD. Older operators understand only the v1 value, - // so v1-compatible releases continue to generate new DCDs with the v1 value. + // KubeLabelDynamoWorkerHash is the opaque worker generation identity on + // worker DCDs and pods. Unchanged generations created by Dynamo 1.2 may keep + // their v1 identity; fresh generations and genuine worker updates use v2. KubeLabelDynamoWorkerHash = "nvidia.com/dynamo-worker-hash" // CheckpointAutoAnnotation marks operator-created checkpoints whose @@ -242,24 +240,19 @@ const ( // these annotations remain on the previously serving worker generation until // the new generation is fully ready and old workers have drained. // - // The compatibility contract is intentionally additive: existing annotation - // and label keys keep their old meaning. AnnotationCurrentWorkerHash stores - // the v1alpha1-compatible worker hash so a downgrade can still understand the - // active generation. AnnotationCurrentWorkerHashV2 stores the v2 worker hash - // for the same active generation. A worker DCD whose - // KubeLabelDynamoWorkerHash value matches either annotation is current. While - // v1 compatibility is required, generated worker DCDs use the v1 hash as the - // label value. If a worker change is visible only to v2, the controller - // removes the v1 annotation and rolls to a v2-labeled DCD because the v1 hash - // can no longer prove pod-template compatibility. A future v2-only release - // can start using the v2 value with the same label key and keep accepting the - // v1 annotation until the next v2 generation change drains old workers. - - // AnnotationCurrentWorkerHash stores the active v1alpha1-compatible worker - // generation hash. + // The 1.2-to-1.4 compatibility contract converges lazily so an operator + // upgrade never rolls unchanged workers. AnnotationCurrentWorkerHash is the + // opaque identity stamped on the active DCD generation. For a 1.2 generation + // this may still be a v1 hash. AnnotationCurrentWorkerHashV2 records the v2 + // fingerprint of that same generation, allowing the controller to detect real + // worker changes without recomputing the old hash. The next genuine worker + // update creates a v2-identified generation and removes the sidecar annotation. + + // AnnotationCurrentWorkerHash stores the active worker generation identity. AnnotationCurrentWorkerHash = "nvidia.com/current-worker-hash" - // AnnotationCurrentWorkerHashV2 stores the active v2 worker generation hash. + // AnnotationCurrentWorkerHashV2 stores the transitional v2 fingerprint for + // an unchanged 1.2 worker generation. AnnotationCurrentWorkerHashV2 = "nvidia.com/current-worker-hash-v2" // LegacyWorkerHash is a sentinel value used during migration from pre-rolling-update diff --git a/deploy/operator/internal/controller/dynamographdeployment_controller.go b/deploy/operator/internal/controller/dynamographdeployment_controller.go index 3a28963b8177..6aaec3bbe06a 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_controller.go +++ b/deploy/operator/internal/controller/dynamographdeployment_controller.go @@ -231,15 +231,15 @@ func (r *DynamoGraphDeploymentReconciler) Reconcile(ctx context.Context, req ctr } } } else { - if r.currentWorkerHashes(dynamoDeployment).empty() { - hashes, err := r.desiredWorkerHashes(dynamoDeployment) + if r.currentWorkerState(dynamoDeployment).empty() { + hash, err := r.desiredWorkerHash(dynamoDeployment) if err != nil { logger.Error(err, "Failed to compute worker hash for unsupported pathway") reason = reasonFailedToInitializeWorkerHash message = Message(err.Error()) return ctrl.Result{}, err } - r.setCurrentWorkerHashes(dynamoDeployment, hashes) + r.setCurrentWorkerState(dynamoDeployment, workerGenerationState{activeGeneration: hash}) if updateErr := r.Update(ctx, dynamoDeployment); updateErr != nil { logger.Error(updateErr, "Failed to initialize worker hash for unsupported pathway") reason = reasonFailedToInitializeWorkerHash @@ -264,11 +264,10 @@ func (r *DynamoGraphDeploymentReconciler) Reconcile(ctx context.Context, req ctr r.Recorder.Event(dynamoDeployment, corev1.EventTypeWarning, "RollingUpdateNotSupported", "Worker spec changed but custom rolling updates are not supported for Grove/multinode deployments") - // Update the hash to prevent repeated warnings. If the unsupported - // path is processing a v2-only worker change, preserve the migrated - // v2-only state instead of resurrecting the downgrade-compatible v1 - // annotation for pod contents it no longer represents. - hashes, err := r.desiredWorkerHashes(dynamoDeployment) + // Update the hash to prevent repeated warnings. This branch only runs + // for a real v2 worker-spec change; unchanged 1.2 bridge state compares + // equal through current-worker-hash-v2 and remains untouched. + hash, err := r.desiredWorkerHash(dynamoDeployment) if err != nil { logger.Error(err, "Failed to compute worker hash for unsupported pathway") state = nvidiacomv1beta1.DGDStateFailed @@ -276,7 +275,7 @@ func (r *DynamoGraphDeploymentReconciler) Reconcile(ctx context.Context, req ctr message = Message(err.Error()) return ctrl.Result{}, err } - r.setCurrentWorkerHashes(dynamoDeployment, r.workerHashesForUnsupportedPathway(dynamoDeployment, hashes)) + r.setCurrentWorkerState(dynamoDeployment, workerGenerationState{activeGeneration: hash}) if updateErr := r.Update(ctx, dynamoDeployment); updateErr != nil { logger.Error(updateErr, "Failed to update worker hash for unsupported pathway") } @@ -1452,19 +1451,19 @@ func (r *DynamoGraphDeploymentReconciler) computeRestartStatus(ctx context.Conte // checkComponentFullyUpdated checks if a DynamoComponentDeployment is fully updated. func (r *DynamoGraphDeploymentReconciler) checkComponentFullyUpdated(ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment, componentName string) (bool, string) { - if r.currentWorkerHashes(dgd).empty() { + if r.currentWorkerState(dgd).empty() { resourceName := dynamo.GetDCDResourceName(dgd, componentName, "") return checkDCDReady(ctx, r.Client, resourceName, dgd.Namespace) } - hashes, err := r.desiredWorkerHashes(dgd) + hash, err := r.desiredWorkerHash(dgd) if err != nil { return false, err.Error() } var lastReason string - for _, hash := range r.activeWorkerHashCandidates(dgd, hashes) { - resourceName := dynamo.GetDCDResourceName(dgd, componentName, hash) + for _, candidate := range r.activeWorkerHashCandidates(dgd, hash) { + resourceName := dynamo.GetDCDResourceName(dgd, componentName, candidate) ready, reason := checkDCDReady(ctx, r.Client, resourceName, dgd.Namespace) if ready || reason != "resource not found" { return ready, reason @@ -1770,11 +1769,11 @@ func (r *DynamoGraphDeploymentReconciler) preserveExistingDCDBackendFramework(ct func (r *DynamoGraphDeploymentReconciler) getExistingRestartAnnotationsDCD(ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment) (map[string]string, error) { logger := log.FromContext(ctx) - hashes, err := r.desiredWorkerHashes(dgd) + hash, err := r.desiredWorkerHash(dgd) if err != nil { return nil, err } - workerHashes := r.activeWorkerHashCandidates(dgd, hashes) + workerHashes := r.activeWorkerHashCandidates(dgd, hash) restartAnnotations := make(map[string]string) for i := range dgd.Spec.Components { @@ -2337,7 +2336,7 @@ func (r *DynamoGraphDeploymentReconciler) checkpointWorkerHashForComponent(dgd * if r == nil { return "", nil } - desired, err := r.desiredWorkerHashes(dgd) + desired, err := r.desiredWorkerHash(dgd) if err != nil { return "", err } diff --git a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go index 543d3a65a599..027b10e2b3d4 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go @@ -2106,11 +2106,11 @@ func TestDynamoGraphDeploymentReconciler_checkpointWorkerHashForComponentUsesAct }, }, }) - desired, err := reconciler.desiredWorkerHashes(dgd) + desired, err := reconciler.desiredWorkerHash(dgd) if err != nil { - t.Fatalf("desiredWorkerHashes() error = %v", err) + t.Fatalf("desiredWorkerHash() error = %v", err) } - reconciler.setCurrentWorkerHashes(dgd, workerGenerationHashes{v1: "oldhash"}) + reconciler.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: "oldhash"}) workerHash, err := reconciler.checkpointWorkerHashForComponent(dgd, "worker") if err != nil { @@ -4237,7 +4237,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }), betaDCD(t, &v1alpha1.DynamoComponentDeployment{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-dgd-decode-e1f2a6fe", + Name: "test-dgd-decode-1b69c0d3", Namespace: "default", }, Spec: v1alpha1.DynamoComponentDeploymentSpec{ @@ -4256,7 +4256,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, Service: &v1alpha1.ServiceReplicaStatus{ ComponentKind: v1alpha1.ComponentKindDeployment, - ComponentNames: []string{"test-dgd-decode-e1f2a6fe-deployment"}, + ComponentNames: []string{"test-dgd-decode-1b69c0d3-deployment"}, Replicas: 2, UpdatedReplicas: 2, ReadyReplicas: ptr.To(int32(2)), @@ -4266,7 +4266,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }), betaDCD(t, &v1alpha1.DynamoComponentDeployment{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-dgd-prefill-e1f2a6fe", + Name: "test-dgd-prefill-1b69c0d3", Namespace: "default", }, Spec: v1alpha1.DynamoComponentDeploymentSpec{ @@ -4285,7 +4285,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, Service: &v1alpha1.ServiceReplicaStatus{ ComponentKind: v1alpha1.ComponentKindDeployment, - ComponentNames: []string{"test-dgd-prefill-e1f2a6fe-deployment"}, + ComponentNames: []string{"test-dgd-prefill-1b69c0d3-deployment"}, Replicas: 3, UpdatedReplicas: 3, ReadyReplicas: ptr.To(int32(3)), @@ -4309,7 +4309,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, "decode": { ComponentKind: v1beta1.ComponentKindDeployment, - ComponentNames: []string{"test-dgd-decode-e1f2a6fe-deployment"}, + ComponentNames: []string{"test-dgd-decode-1b69c0d3-deployment"}, Replicas: 2, UpdatedReplicas: 2, ReadyReplicas: ptr.To(int32(2)), @@ -4317,7 +4317,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, "prefill": { ComponentKind: v1beta1.ComponentKindDeployment, - ComponentNames: []string{"test-dgd-prefill-e1f2a6fe-deployment"}, + ComponentNames: []string{"test-dgd-prefill-1b69c0d3-deployment"}, Replicas: 3, UpdatedReplicas: 3, ReadyReplicas: ptr.To(int32(3)), @@ -4383,7 +4383,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }), betaDCD(t, &v1alpha1.DynamoComponentDeployment{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-dgd-decode-e1f2a6fe", + Name: "test-dgd-decode-1b69c0d3", Namespace: "default", }, Spec: v1alpha1.DynamoComponentDeploymentSpec{ @@ -4402,7 +4402,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, Service: &v1alpha1.ServiceReplicaStatus{ ComponentKind: v1alpha1.ComponentKindDeployment, - ComponentNames: []string{"test-dgd-decode-e1f2a6fe-deployment"}, + ComponentNames: []string{"test-dgd-decode-1b69c0d3-deployment"}, Replicas: 2, UpdatedReplicas: 1, ReadyReplicas: ptr.To(int32(1)), @@ -4412,7 +4412,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }), betaDCD(t, &v1alpha1.DynamoComponentDeployment{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-dgd-prefill-e1f2a6fe", + Name: "test-dgd-prefill-1b69c0d3", Namespace: "default", }, Spec: v1alpha1.DynamoComponentDeploymentSpec{ @@ -4431,7 +4431,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, Service: &v1alpha1.ServiceReplicaStatus{ ComponentKind: v1alpha1.ComponentKindDeployment, - ComponentNames: []string{"test-dgd-prefill-e1f2a6fe-deployment"}, + ComponentNames: []string{"test-dgd-prefill-1b69c0d3-deployment"}, Replicas: 3, UpdatedReplicas: 3, ReadyReplicas: ptr.To(int32(3)), @@ -4443,7 +4443,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { wantReconcileResult: ReconcileResult{ State: v1beta1.DGDStatePending, Reason: "some_resources_are_not_ready", - Message: "Resources not ready: test-dgd-decode-e1f2a6fe: Component deployment not ready - Available condition not true", + Message: "Resources not ready: test-dgd-decode-1b69c0d3: Component deployment not ready - Available condition not true", ComponentStatus: map[string]v1beta1.ComponentReplicaStatus{ "frontend": { ComponentKind: v1beta1.ComponentKindDeployment, @@ -4455,7 +4455,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, "decode": { ComponentKind: v1beta1.ComponentKindDeployment, - ComponentNames: []string{"test-dgd-decode-e1f2a6fe-deployment"}, + ComponentNames: []string{"test-dgd-decode-1b69c0d3-deployment"}, Replicas: 2, UpdatedReplicas: 1, ReadyReplicas: ptr.To(int32(1)), @@ -4463,7 +4463,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, "prefill": { ComponentKind: v1beta1.ComponentKindDeployment, - ComponentNames: []string{"test-dgd-prefill-e1f2a6fe-deployment"}, + ComponentNames: []string{"test-dgd-prefill-1b69c0d3-deployment"}, Replicas: 3, UpdatedReplicas: 3, ReadyReplicas: ptr.To(int32(3)), @@ -4523,7 +4523,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }), betaDCD(t, &v1alpha1.DynamoComponentDeployment{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-dgd-decode-5f3d46ba", + Name: "test-dgd-decode-cabcd5c9", Namespace: "default", }, Spec: v1alpha1.DynamoComponentDeploymentSpec{ @@ -4542,7 +4542,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, Service: &v1alpha1.ServiceReplicaStatus{ ComponentKind: v1alpha1.ComponentKindDeployment, - ComponentNames: []string{"test-dgd-decode-5f3d46ba-deployment"}, + ComponentNames: []string{"test-dgd-decode-cabcd5c9-deployment"}, Replicas: 2, UpdatedReplicas: 1, ReadyReplicas: ptr.To(int32(1)), @@ -4554,7 +4554,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { wantReconcileResult: ReconcileResult{ State: v1beta1.DGDStatePending, Reason: "some_resources_are_not_ready", - Message: "Resources not ready: test-dgd-decode-5f3d46ba: Component deployment not ready - Available condition not true; test-dgd-frontend: Component deployment not ready - Available condition not true", + Message: "Resources not ready: test-dgd-decode-cabcd5c9: Component deployment not ready - Available condition not true; test-dgd-frontend: Component deployment not ready - Available condition not true", ComponentStatus: map[string]v1beta1.ComponentReplicaStatus{ "frontend": { ComponentKind: v1beta1.ComponentKindDeployment, @@ -4566,7 +4566,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, "decode": { ComponentKind: v1beta1.ComponentKindDeployment, - ComponentNames: []string{"test-dgd-decode-5f3d46ba-deployment"}, + ComponentNames: []string{"test-dgd-decode-cabcd5c9-deployment"}, Replicas: 2, UpdatedReplicas: 1, ReadyReplicas: ptr.To(int32(1)), diff --git a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go index 400b6c9c321a..7e633e2b7a3e 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go +++ b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go @@ -36,123 +36,97 @@ import ( "github.com/ai-dynamo/dynamo/deploy/operator/internal/dynamo" ) -type workerGenerationHashes struct { - v1 string - v2 string +// workerGenerationState separates the identity stamped on the active DCD +// generation from the v2 fingerprint of its rendered worker spec. Dynamo 1.2 +// stored a v1 generation identity in current-worker-hash and the corresponding +// v2 fingerprint in current-worker-hash-v2. Keeping both values until a real +// worker change lets 1.4 stop computing v1 hashes without rolling unchanged +// workers merely to rename their generation. +type workerGenerationState struct { + activeGeneration string + v2Fingerprint string } -func (h workerGenerationHashes) empty() bool { - return h.v1 == "" && h.v2 == "" +func (s workerGenerationState) empty() bool { + return s.activeGeneration == "" && s.v2Fingerprint == "" } -func (h workerGenerationHashes) contains(hash string) bool { - if hash == "" { - return false +// activeHash returns the identity used by the serving DCD generation. A 1.2 +// v2-only generation stored its identity only in current-worker-hash-v2, so use +// that value when the canonical annotation is empty. +func (s workerGenerationState) activeHash() string { + if s.activeGeneration != "" { + return s.activeGeneration } - return hash == h.v1 || hash == h.v2 + return s.v2Fingerprint } -func (r *DynamoGraphDeploymentReconciler) desiredWorkerHashes( - dgd *nvidiacomv1beta1.DynamoGraphDeployment, -) (workerGenerationHashes, error) { - v1Hash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - if err != nil { - return workerGenerationHashes{}, fmt.Errorf("failed to compute v1 worker hash: %w", err) +func (s workerGenerationState) matchesDesired(desired string) bool { + if s.empty() { + return true } - - v2Hash, err := dynamo.ComputeDGDWorkersSpecHash(dgd) - if err != nil { - return workerGenerationHashes{}, fmt.Errorf("failed to compute v2 worker hash: %w", err) + if s.v2Fingerprint != "" { + return s.v2Fingerprint == desired } - - return workerGenerationHashes{v1: v1Hash, v2: v2Hash}, nil + return s.activeGeneration == desired } -func (r *DynamoGraphDeploymentReconciler) currentWorkerHashes( +func (r *DynamoGraphDeploymentReconciler) desiredWorkerHash( dgd *nvidiacomv1beta1.DynamoGraphDeployment, -) workerGenerationHashes { - return workerGenerationHashes{ - v1: r.getCurrentWorkerHash(dgd), - v2: r.getCurrentWorkerHashV2(dgd), - } -} - -func currentWorkerHashesMatchDesired(current, desired workerGenerationHashes) bool { - if current.empty() { - return true - } - if current.v1 != "" { - if current.v1 != desired.v1 { - return false - } - return current.v2 == "" || current.v2 == desired.v2 +) (string, error) { + hash, err := dynamo.ComputeDGDWorkersSpecHash(dgd) + if err != nil { + return "", fmt.Errorf("failed to compute worker hash: %w", err) } - return current.v2 == desired.v2 + return hash, nil } -func workerHashForDCDGeneration(current, desired workerGenerationHashes) string { - if current.v1 != "" { - if current.v1 == desired.v1 { - if current.v2 == "" || current.v2 == desired.v2 { - return desired.v1 - } - return desired.v2 - } - return desired.v1 - } - if current.v2 != "" { - return desired.v2 +func (r *DynamoGraphDeploymentReconciler) currentWorkerState( + dgd *nvidiacomv1beta1.DynamoGraphDeployment, +) workerGenerationState { + return workerGenerationState{ + activeGeneration: r.getCurrentWorkerHash(dgd), + v2Fingerprint: r.getCurrentWorkerHashV2(dgd), } - return desired.v1 } -func workerHashesForCompletedGeneration(newWorkerHash string, desired workerGenerationHashes) workerGenerationHashes { - if newWorkerHash == desired.v2 && desired.v2 != desired.v1 { - return workerGenerationHashes{v2: desired.v2} +func workerHashForDCDGeneration(current workerGenerationState, desired string) string { + if current.matchesDesired(desired) && current.activeHash() != "" { + return current.activeHash() } return desired } -func (r *DynamoGraphDeploymentReconciler) workerHashesForUnsupportedPathway( - dgd *nvidiacomv1beta1.DynamoGraphDeployment, - desired workerGenerationHashes, -) workerGenerationHashes { - newWorkerHash := r.activeWorkerHashForDCDGeneration(dgd, desired) - return workerHashesForCompletedGeneration(newWorkerHash, desired) -} - // shouldTriggerRollingUpdate compares desired worker hashes with the active // generation recorded on the DGD. // -// During v1/v2 compatibility a worker DCD is current if its worker-hash label -// matches either current-worker-hash (v1) or current-worker-hash-v2. This keeps -// the existing annotation/label meaning downgrade-safe while allowing the -// controller to record the v2 hash that will become primary later. +// For a 1.2 bridge state, current-worker-hash-v2 is the v2 fingerprint of the +// active v1-named generation. Comparing the desired hash to that fingerprint +// avoids an upgrade-induced rollout while still detecting real worker changes. func (r *DynamoGraphDeploymentReconciler) shouldTriggerRollingUpdate( dgd *nvidiacomv1beta1.DynamoGraphDeployment, ) (bool, error) { - desired, err := r.desiredWorkerHashes(dgd) + desired, err := r.desiredWorkerHash(dgd) if err != nil { return false, err } - current := r.currentWorkerHashes(dgd) - return !currentWorkerHashesMatchDesired(current, desired), nil + return !r.currentWorkerState(dgd).matchesDesired(desired), nil } // initializeWorkerHashIfNeeded establishes the DGD's active worker generation. -// New DGDs store the current v1 and v2 worker hashes immediately. DGDs created before -// managed rolling updates may already have worker DCDs without a hash label; in -// that case we label those DCDs with the legacy sentinel and let the normal -// rolling update path migrate from that sentinel to the desired compatibility hash. +// New DGDs use the v2 hash directly. DGDs created before managed rolling updates +// may already have worker DCDs without a hash label; in that case we label those +// DCDs with the legacy sentinel and let the normal rolling update path migrate +// from that sentinel to the desired v2 hash. func (r *DynamoGraphDeploymentReconciler) initializeWorkerHashIfNeeded( ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment, ) error { logger := log.FromContext(ctx) - if !r.currentWorkerHashes(dgd).empty() { - return r.migrateCurrentWorkerHashIfNeeded(ctx, dgd) + if !r.currentWorkerState(dgd).empty() { + return nil } // Check for legacy (pre-rolling-update) worker DCDs @@ -191,99 +165,77 @@ func (r *DynamoGraphDeploymentReconciler) initializeWorkerHashIfNeeded( return nil } - // Normal first deploy — set the actual computed compatibility hashes - hashes, err := r.desiredWorkerHashes(dgd) + // Normal first deploy — use the v2 hash as the canonical generation identity. + hash, err := r.desiredWorkerHash(dgd) if err != nil { return err } - r.setCurrentWorkerHashes(dgd, hashes) + r.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: hash}) if err := r.Update(ctx, dgd); err != nil { return fmt.Errorf("failed to initialize worker hash: %w", err) } - logger.Info("Initialized current worker hashes", "v1Hash", hashes.v1, "v2Hash", hashes.v2) + logger.Info("Initialized current worker hash", "hash", hash) return nil } -// migrateCurrentWorkerHashIfNeeded fills in additive v2 worker-hash state while -// the v1 hash still represents the active worker generation. If v2 changes -// without a v1 change, v1 compatibility no longer proves current pod contents, -// so the v1 annotation is removed before rolling to a v2-labeled DCD. +// migrateCurrentWorkerHashIfNeeded canonicalizes a v2-only state written by +// Dynamo 1.2. It is safe only when the sidecar fingerprint still matches the +// desired v2 hash; otherwise the object is transitioning to a newer generation +// and the normal rollout path must retain the old state until completion. func (r *DynamoGraphDeploymentReconciler) migrateCurrentWorkerHashIfNeeded( ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment, ) error { logger := log.FromContext(ctx) - current := r.currentWorkerHashes(dgd) - if current.empty() { - return nil - } - if current.v1 == consts.LegacyWorkerHash { + current := r.currentWorkerState(dgd) + if current.activeGeneration != "" || current.v2Fingerprint == "" { return nil } - desired, err := r.desiredWorkerHashes(dgd) + desired, err := r.desiredWorkerHash(dgd) if err != nil { return err } - - var next workerGenerationHashes - var eventMessage string - switch { - case current.v1 == desired.v1 && current.v2 == "": - next = current - next.v2 = desired.v2 - eventMessage = "Recorded compatible v1 and v2 worker hash annotations without rolling workers" - case current.v1 == desired.v1 && current.v2 != desired.v2: - next = workerGenerationHashes{v2: current.v2} - eventMessage = "Removed v1 worker hash annotation before rolling a v2-only worker change" - default: + if current.v2Fingerprint != desired { return nil } - if next == current { - return nil - } - r.setCurrentWorkerHashes(dgd, next) + r.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: current.v2Fingerprint}) if err := r.Update(ctx, dgd); err != nil { return fmt.Errorf("failed to migrate worker hash annotations: %w", err) } - logger.Info("Migrated worker hash annotations", - "v1Hash", next.v1, - "v2Hash", next.v2) - r.Recorder.Event(dgd, corev1.EventTypeNormal, "WorkerHashMigrated", eventMessage) + logger.Info("Promoted v2 worker hash to canonical annotation", "hash", desired) + r.Recorder.Event(dgd, corev1.EventTypeNormal, "WorkerHashMigrated", + "Promoted the existing v2 worker hash to the canonical annotation without rolling workers") return nil } // activeWorkerHashForDCDGeneration returns the hash used for generated worker -// DCD names and worker-hash labels in this reconcile. While v1 compatibility is -// required, new DCDs continue to use the v1 hash. If the active generation is -// already v2-labeled, preserve that value so future v2-only transitions do not -// create a v1-labeled replacement. +// DCD names and worker-hash labels in this reconcile. An unchanged 1.2 bridge +// state keeps its v1 generation identity; a real worker change uses the desired +// v2 hash and converges to a single-hash state when the rollout completes. func (r *DynamoGraphDeploymentReconciler) activeWorkerHashForDCDGeneration( dgd *nvidiacomv1beta1.DynamoGraphDeployment, - desired workerGenerationHashes, + desired string, ) string { return r.activeWorkerHashCandidates(dgd, desired)[0] } func (r *DynamoGraphDeploymentReconciler) activeWorkerHashCandidates( dgd *nvidiacomv1beta1.DynamoGraphDeployment, - desired workerGenerationHashes, + desired string, ) []string { - current := r.currentWorkerHashes(dgd) + current := r.currentWorkerState(dgd) candidates := make([]string, 0, 2) generated := workerHashForDCDGeneration(current, desired) candidates = append(candidates, generated) - if current.v1 == desired.v1 && (current.v2 == "" || current.v2 == desired.v2) && desired.v1 != generated { - candidates = append(candidates, desired.v1) - } - if current.contains(desired.v2) && desired.v2 != generated && desired.v2 != desired.v1 { - candidates = append(candidates, desired.v2) + if current.v2Fingerprint == desired && desired != generated { + candidates = append(candidates, desired) } return candidates } @@ -352,23 +304,23 @@ func (r *DynamoGraphDeploymentReconciler) getCurrentWorkerHashV2( return dgd.Annotations[consts.AnnotationCurrentWorkerHashV2] } -// setCurrentWorkerHashes stores the active worker hashes for one generation. -// Empty fields are deleted, which is how v2-only generations intentionally drop -// the downgrade-compatible v1 annotation. -func (r *DynamoGraphDeploymentReconciler) setCurrentWorkerHashes( +// setCurrentWorkerState stores the active generation identity and its optional +// 1.2 bridge fingerprint. Fresh and converged generations use only the canonical +// annotation; the sidecar is retained solely for unchanged v1-named generations. +func (r *DynamoGraphDeploymentReconciler) setCurrentWorkerState( dgd *nvidiacomv1beta1.DynamoGraphDeployment, - hashes workerGenerationHashes, + state workerGenerationState, ) { if dgd.Annotations == nil { dgd.Annotations = make(map[string]string) } - if hashes.v1 != "" { - dgd.Annotations[consts.AnnotationCurrentWorkerHash] = hashes.v1 + if state.activeGeneration != "" { + dgd.Annotations[consts.AnnotationCurrentWorkerHash] = state.activeGeneration } else { delete(dgd.Annotations, consts.AnnotationCurrentWorkerHash) } - if hashes.v2 != "" { - dgd.Annotations[consts.AnnotationCurrentWorkerHashV2] = hashes.v2 + if state.v2Fingerprint != "" { + dgd.Annotations[consts.AnnotationCurrentWorkerHashV2] = state.v2Fingerprint } else { delete(dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) } @@ -420,38 +372,37 @@ func (r *DynamoGraphDeploymentReconciler) reconcileRollingUpdate( rollingUpdateStatus := r.getOrCreateRollingUpdateStatus(dgd) - desired, err := r.desiredWorkerHashes(dgd) + desired, err := r.desiredWorkerHash(dgd) if err != nil { return err } newWorkerHash := r.activeWorkerHashForDCDGeneration(dgd, desired) - current := r.currentWorkerHashes(dgd) + current := r.currentWorkerState(dgd) logger.Info("Reconciling rolling update", "phase", rollingUpdateStatus.Phase, - "currentV1WorkerHash", current.v1, - "currentV2WorkerHash", current.v2, + "activeWorkerGeneration", current.activeHash(), + "currentV2Fingerprint", current.v2Fingerprint, "newWorkerHash", newWorkerHash, - "desiredV1WorkerHash", desired.v1, - "desiredV2WorkerHash", desired.v2) + "desiredWorkerHash", desired) - if rollingUpdateStatus.Phase == nvidiacomv1beta1.RollingUpdatePhaseCompleted && !current.contains(newWorkerHash) { + if rollingUpdateStatus.Phase == nvidiacomv1beta1.RollingUpdatePhaseCompleted && current.activeHash() != newWorkerHash { // Check if DCDs with the new hash already exist and are serving. // If so, this is just a stale annotation — update it without starting a new rollout. newInfo, err := r.getWorkerInfoForWorkerHash(ctx, dgd, newWorkerHash) oldInfo, oldErr := r.getOldWorkerInfo(ctx, dgd, newWorkerHash) if err == nil && oldErr == nil && workerGenerationComplete(dgd, oldInfo, newInfo) { logger.Info("Updating stale worker hash annotation", - "currentV1WorkerHash", current.v1, - "currentV2WorkerHash", current.v2, + "activeWorkerGeneration", current.activeHash(), + "currentV2Fingerprint", current.v2Fingerprint, "newHash", newWorkerHash) - r.setCurrentWorkerHashes(dgd, workerHashesForCompletedGeneration(newWorkerHash, desired)) + r.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: newWorkerHash}) return r.Update(ctx, dgd) } // New spec change: reset to start a proper rolling update cycle with surge/drain. logger.Info("New worker spec change detected, starting new rolling update cycle", - "currentV1WorkerHash", current.v1, - "currentV2WorkerHash", current.v2, + "activeWorkerGeneration", current.activeHash(), + "currentV2Fingerprint", current.v2Fingerprint, "newHash", newWorkerHash, "previousPhase", rollingUpdateStatus.Phase) rollingUpdateStatus.Phase = nvidiacomv1beta1.RollingUpdatePhaseNone @@ -460,7 +411,7 @@ func (r *DynamoGraphDeploymentReconciler) reconcileRollingUpdate( rollingUpdateStatus.UpdatedComponents = nil } - if current.contains(newWorkerHash) && + if current.activeHash() == newWorkerHash && rollingUpdateStatus.Phase == nvidiacomv1beta1.RollingUpdatePhaseInProgress { logger.Info("Detected stuck rolling update: hashes match but phase is InProgress", "hash", newWorkerHash, @@ -495,11 +446,11 @@ func (r *DynamoGraphDeploymentReconciler) startRollingUpdate( ) error { logger := log.FromContext(ctx) - current := r.currentWorkerHashes(dgd) + current := r.currentWorkerState(dgd) logger.Info("Starting rolling update", - "currentV1Hash", current.v1, - "currentV2Hash", current.v2, + "activeWorkerGeneration", current.activeHash(), + "currentV2Fingerprint", current.v2Fingerprint, "newHash", newWorkerHash) now := metav1.Now() @@ -613,17 +564,12 @@ func (r *DynamoGraphDeploymentReconciler) completeRollingUpdate( ) error { logger := log.FromContext(ctx) - desired, err := r.desiredWorkerHashes(dgd) - if err != nil { - return err - } - // Delete all non-current worker DCDs (any number of old generations) if err := r.deleteOldWorkerDCDs(ctx, dgd, newWorkerHash); err != nil { return fmt.Errorf("failed to delete old worker DCDs: %w", err) } - r.setCurrentWorkerHashes(dgd, workerHashesForCompletedGeneration(newWorkerHash, desired)) + r.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: newWorkerHash}) if err := r.Update(ctx, dgd); err != nil { return fmt.Errorf("failed to update current worker hash: %w", err) } @@ -1126,14 +1072,14 @@ func (r *DynamoGraphDeploymentReconciler) buildRollingUpdateContext( ) (dynamo.RollingUpdateContext, error) { logger := log.FromContext(ctx) - desiredHashes, err := r.desiredWorkerHashes(dgd) + desiredHash, err := r.desiredWorkerHash(dgd) if err != nil { return dynamo.RollingUpdateContext{}, err } - newWorkerHash := r.activeWorkerHashForDCDGeneration(dgd, desiredHashes) - currentHashes := r.currentWorkerHashes(dgd) + newWorkerHash := r.activeWorkerHashForDCDGeneration(dgd, desiredHash) + current := r.currentWorkerState(dgd) - if currentHashes.contains(newWorkerHash) { + if current.activeHash() == newWorkerHash { return dynamo.RollingUpdateContext{ NewWorkerHash: newWorkerHash, OldWorkerReplicaTargetsByComponent: make(map[string]int32), diff --git a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go index 705629220f29..1b2794a85642 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go @@ -107,10 +107,11 @@ func createTestReconcilerWithStatus(dgd *nvidiacomv1beta1.DynamoGraphDeployment, func TestShouldTriggerRollingUpdate(t *testing.T) { tests := []struct { - name string - services map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec - existingHash string // empty means no annotation, "compute" means compute from services - expected bool + name string + services map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec + currentState string + changeDesired bool + expected bool }{ { name: "new deployment - no hash annotation", @@ -120,22 +121,21 @@ func TestShouldTriggerRollingUpdate(t *testing.T) { Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, }, }, - existingHash: "", - expected: false, + expected: false, }, { - name: "hash unchanged - matches current spec", + name: "canonical v2 hash unchanged", services: map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, }, }, - existingHash: "compute", + currentState: "canonical", expected: false, }, { - name: "unversioned legacy alpha hash - compatible migration does not trigger rollout", + name: "1.2 bridge fingerprint unchanged", services: map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, @@ -145,19 +145,32 @@ func TestShouldTriggerRollingUpdate(t *testing.T) { }, }, }, - existingHash: "legacy-compute", + currentState: "bridge", expected: false, }, { - name: "hash changed - differs from current spec", + name: "canonical hash changed", services: map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "FOO", Value: "new-value"}}, }, }, - existingHash: "old-hash-12345678", - expected: true, + currentState: "canonical", + changeDesired: true, + expected: true, + }, + { + name: "1.2 bridge fingerprint changed", + services: map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: consts.ComponentTypeWorker, + Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, + }, + }, + currentState: "bridge", + changeDesired: true, + expected: true, }, { name: "frontend-only change - hash unchanged", @@ -171,7 +184,7 @@ func TestShouldTriggerRollingUpdate(t *testing.T) { Envs: []corev1.EnvVar{{Name: "WORKER_VAR", Value: "unchanged"}}, }, }, - existingHash: "compute", + currentState: "bridge", expected: false, }, } @@ -180,20 +193,23 @@ func TestShouldTriggerRollingUpdate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { dgd := createTestDGD("test-dgd", tt.services) - if tt.existingHash == "compute" { - hash := legacyDGDWorkersSpecHash(t, dgd) + desired := betaDGDWorkersSpecHash(t, dgd) + switch tt.currentState { + case "canonical": dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: hash, - consts.AnnotationCurrentWorkerHashV2: betaDGDWorkersSpecHash(t, dgd), + consts.AnnotationCurrentWorkerHash: desired, } - } else if tt.existingHash == "legacy-compute" { - hash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - require.NoError(t, err) + case "bridge": dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: hash, + consts.AnnotationCurrentWorkerHash: "v1hash00", + consts.AnnotationCurrentWorkerHashV2: desired, } - } else if tt.existingHash != "" { - dgd.Annotations = map[string]string{consts.AnnotationCurrentWorkerHash: tt.existingHash} + } + if tt.changeDesired { + dgd.Spec.Components[0].PodTemplate.Spec.Containers[0].Env = append( + dgd.Spec.Components[0].PodTemplate.Spec.Containers[0].Env, + corev1.EnvVar{Name: "CHANGED", Value: "true"}, + ) } r := createTestReconcilerWithStatus(dgd) @@ -214,20 +230,18 @@ func TestShouldTriggerRollingUpdate_IgnoresReplicaChanges(t *testing.T) { Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, }, }) - legacyHash := legacyDGDWorkersSpecHash(t, dgd) v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: legacyHash, + consts.AnnotationCurrentWorkerHash: "v1hash00", consts.AnnotationCurrentWorkerHashV2: v2Hash, } dgd.Spec.Components[0].Replicas = ptr.To(int32(10)) r := createTestReconcilerWithStatus(dgd) - desired, err := r.desiredWorkerHashes(dgd) + desired, err := r.desiredWorkerHash(dgd) require.NoError(t, err) - assert.Equal(t, legacyHash, desired.v1) - assert.Equal(t, v2Hash, desired.v2) + assert.Equal(t, v2Hash, desired) trigger, err := r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) @@ -256,12 +270,10 @@ func TestInitializeWorkerHashIfNeeded_FirstDeploy(t *testing.T) { hash := r.getCurrentWorkerHash(dgd) assert.NotEmpty(t, hash, "Hash should be set after initialization") - // Verify both compatibility hashes are correct. - expectedV1Hash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - require.NoError(t, err) - expectedV2Hash := betaDGDWorkersSpecHash(t, dgd) - assert.Equal(t, expectedV1Hash, hash, "v1 hash should remain the downgrade-compatible current hash") - assert.Equal(t, expectedV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + // Fresh deployments use one canonical v2 hash. + expectedHash := betaDGDWorkersSpecHash(t, dgd) + assert.Equal(t, expectedHash, hash) + assert.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) } func TestInitializeWorkerHashIfNeeded_AlreadyInitialized(t *testing.T) { @@ -291,91 +303,24 @@ func TestInitializeWorkerHashIfNeeded_AlreadyInitialized(t *testing.T) { assert.Equal(t, existingHash, hash, "Hash should not change when already initialized") } -func TestInitializeWorkerHashIfNeeded_PreservesLegacyAlphaHash(t *testing.T) { - alpha := &nvidiacomv1alpha1.DynamoGraphDeployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-dgd", - Namespace: "default", - Annotations: map[string]string{ - consts.AnnotationCurrentWorkerHash: "old-alpha-hash", - }, - }, - Spec: nvidiacomv1alpha1.DynamoGraphDeploymentSpec{ - Services: map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: consts.ComponentTypeWorker, - Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, - Resources: &nvidiacomv1alpha1.Resources{ - Requests: &nvidiacomv1alpha1.ResourceItem{CPU: "1"}, - }, - }, - }, - }, - } - dgd := &nvidiacomv1beta1.DynamoGraphDeployment{} - require.NoError(t, alpha.ConvertTo(dgd)) - legacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - require.NoError(t, err) - v2Hash := betaDGDWorkersSpecHash(t, dgd) - require.NotEqual(t, legacyHash, v2Hash) - if dgd.Annotations == nil { - dgd.Annotations = map[string]string{} - } - dgd.Annotations[consts.AnnotationCurrentWorkerHash] = legacyHash - - r := createTestReconcilerWithStatus(dgd) - err = r.initializeWorkerHashIfNeeded(context.Background(), dgd) - require.NoError(t, err) - - assert.Equal(t, legacyHash, r.getCurrentWorkerHash(dgd)) - assert.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) - trigger, err := r.shouldTriggerRollingUpdate(dgd) - require.NoError(t, err) - assert.False(t, trigger) - - ctx, err := r.buildRollingUpdateContext(context.Background(), dgd) - require.NoError(t, err) - assert.Equal(t, legacyHash, ctx.NewWorkerHash) - assert.False(t, ctx.InProgress()) - assert.NotEqual(t, v2Hash, ctx.NewWorkerHash) -} - -func TestLegacyAlphaHashCompatibility_NoOpUpgradeUsesExistingWorkerGeneration(t *testing.T) { - alpha := &nvidiacomv1alpha1.DynamoGraphDeployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "qwen", - Namespace: "default", - Annotations: map[string]string{ - consts.AnnotationCurrentWorkerHash: "old-alpha-hash", - }, - }, - Spec: nvidiacomv1alpha1.DynamoGraphDeploymentSpec{ - BackendFramework: "vllm", - Services: map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ - "VllmDecodeWorker": { - ComponentType: consts.ComponentTypeWorker, - SubComponentType: consts.ComponentTypeDecode, - Envs: []corev1.EnvVar{{Name: "MODEL_PATH", Value: "Qwen/Qwen3-0.6B"}}, - Resources: &nvidiacomv1alpha1.Resources{ - Requests: &nvidiacomv1alpha1.ResourceItem{GPU: "1"}, - }, - }, - }, +func TestBridgeState_NoOpUpgradeUsesExistingWorkerGeneration(t *testing.T) { + dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: consts.ComponentTypeWorker, + Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, }, - } - dgd := &nvidiacomv1beta1.DynamoGraphDeployment{} - require.NoError(t, alpha.ConvertTo(dgd)) - legacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - require.NoError(t, err) + }) + activeHash := "v1hash00" v2Hash := betaDGDWorkersSpecHash(t, dgd) - require.NotEqual(t, legacyHash, v2Hash) - if dgd.Annotations == nil { - dgd.Annotations = map[string]string{} + dgd.Annotations = map[string]string{ + consts.AnnotationCurrentWorkerHash: activeHash, + consts.AnnotationCurrentWorkerHashV2: v2Hash, } - dgd.Annotations[consts.AnnotationCurrentWorkerHash] = legacyHash r := createTestReconcilerWithStatus(dgd) - require.NoError(t, r.initializeWorkerHashIfNeeded(context.Background(), dgd)) + require.NoError(t, r.migrateCurrentWorkerHashIfNeeded(context.Background(), dgd)) + require.Equal(t, activeHash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) trigger, err := r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) @@ -383,50 +328,35 @@ func TestLegacyAlphaHashCompatibility_NoOpUpgradeUsesExistingWorkerGeneration(t rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) require.NoError(t, err) - require.Equal(t, legacyHash, rollingCtx.NewWorkerHash) + require.Equal(t, activeHash, rollingCtx.NewWorkerHash) require.False(t, rollingCtx.InProgress()) dcds, err := dynamo.GenerateDynamoComponentsDeployments(dgd, nil, nil, rollingCtx) require.NoError(t, err) - require.Equal(t, "qwen-vllmdecodeworker-"+legacyHash, dcds["VllmDecodeWorker"].Name) - require.NotEqual(t, "qwen-vllmdecodeworker-"+v2Hash, dcds["VllmDecodeWorker"].Name) + require.Equal(t, "test-dgd-worker-"+activeHash, dcds["worker"].Name) } -func TestLegacyAlphaHashCompatibility_WorkerSpecChangeUsesNewV1Generation(t *testing.T) { +func TestBridgeState_WorkerSpecChangeUsesNewV2Generation(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, - Resources: &nvidiacomv1alpha1.Resources{ - Requests: &nvidiacomv1alpha1.ResourceItem{CPU: "1"}, - }, }, }) - legacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - require.NoError(t, err) - v2Hash := betaDGDWorkersSpecHash(t, dgd) - require.NotEqual(t, legacyHash, v2Hash) - if dgd.Annotations == nil { - dgd.Annotations = map[string]string{} + oldV2Hash := betaDGDWorkersSpecHash(t, dgd) + dgd.Annotations = map[string]string{ + consts.AnnotationCurrentWorkerHash: "v1hash00", + consts.AnnotationCurrentWorkerHashV2: oldV2Hash, } - dgd.Annotations[consts.AnnotationCurrentWorkerHash] = legacyHash - - r := createTestReconcilerWithStatus(dgd) - require.NoError(t, r.initializeWorkerHashIfNeeded(context.Background(), dgd)) - require.Equal(t, legacyHash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) dgd.Spec.Components[0].PodTemplate.Spec.Containers[0].Env = append( dgd.Spec.Components[0].PodTemplate.Spec.Containers[0].Env, corev1.EnvVar{Name: "NEW_WORKER_SETTING", Value: "true"}, ) newV2Hash := betaDGDWorkersSpecHash(t, dgd) - newLegacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - require.NoError(t, err) - require.NotEqual(t, v2Hash, newV2Hash) - require.NotEqual(t, legacyHash, newLegacyHash) + require.NotEqual(t, oldV2Hash, newV2Hash) - require.NoError(t, r.migrateCurrentWorkerHashIfNeeded(context.Background(), dgd)) + r := createTestReconcilerWithStatus(dgd) trigger, err := r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) @@ -434,54 +364,69 @@ func TestLegacyAlphaHashCompatibility_WorkerSpecChangeUsesNewV1Generation(t *tes rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) require.NoError(t, err) - require.Equal(t, newLegacyHash, rollingCtx.NewWorkerHash) - require.NotEqual(t, newV2Hash, rollingCtx.NewWorkerHash) + require.Equal(t, newV2Hash, rollingCtx.NewWorkerHash) + require.True(t, rollingCtx.InProgress()) + + require.NoError(t, r.completeRollingUpdate(context.Background(), dgd, newV2Hash)) + require.Equal(t, newV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) } -func TestLegacyAlphaHashCompatibility_V2OnlyChangeUsesNewV2Generation(t *testing.T) { +func TestV2OnlyStatePromotesCanonicalHashWithoutRollout(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, }, }) - dgd.Spec.BackendFramework = "vllm" - legacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - require.NoError(t, err) v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: legacyHash, consts.AnnotationCurrentWorkerHashV2: v2Hash, } r := createTestReconcilerWithStatus(dgd) - dgd.Spec.BackendFramework = "sglang" + require.NoError(t, r.migrateCurrentWorkerHashIfNeeded(context.Background(), dgd)) + require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) - newLegacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + trigger, err := r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) - newV2Hash := betaDGDWorkersSpecHash(t, dgd) - require.Equal(t, legacyHash, newLegacyHash) - require.NotEqual(t, v2Hash, newV2Hash) + require.False(t, trigger) + + rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) + require.NoError(t, err) + require.Equal(t, v2Hash, rollingCtx.NewWorkerHash) + require.False(t, rollingCtx.InProgress()) +} + +func TestV2OnlyTransitionDoesNotPromoteStaleFingerprint(t *testing.T) { + dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: consts.ComponentTypeWorker, + Envs: []corev1.EnvVar{{Name: "FOO", Value: "new"}}, + }, + }) + dgd.Annotations = map[string]string{ + consts.AnnotationCurrentWorkerHashV2: "oldv2hash", + } + r := createTestReconcilerWithStatus(dgd) require.NoError(t, r.migrateCurrentWorkerHashIfNeeded(context.Background(), dgd)) - require.Empty(t, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHash) + require.Equal(t, "oldv2hash", dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + desired := betaDGDWorkersSpecHash(t, dgd) trigger, err := r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) require.True(t, trigger) rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) require.NoError(t, err) - require.Equal(t, newV2Hash, rollingCtx.NewWorkerHash) - require.NotEqual(t, newLegacyHash, rollingCtx.NewWorkerHash) - - require.NoError(t, r.completeRollingUpdate(context.Background(), dgd, newV2Hash)) - require.Empty(t, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.Equal(t, newV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + require.Equal(t, desired, rollingCtx.NewWorkerHash) + require.True(t, rollingCtx.InProgress()) } -func TestUnsupportedPathwayMigratesV1OnlyAndKeepsV2OnlyGeneration(t *testing.T) { +func TestUnsupportedPathwayKeepsBridgeUntilRealWorkerChange(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, @@ -490,46 +435,35 @@ func TestUnsupportedPathwayMigratesV1OnlyAndKeepsV2OnlyGeneration(t *testing.T) }, }) dgd.Spec.BackendFramework = "vllm" - legacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - require.NoError(t, err) - v2Hash := betaDGDWorkersSpecHash(t, dgd) + oldV2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: legacyHash, + consts.AnnotationCurrentWorkerHash: "v1hash00", + consts.AnnotationCurrentWorkerHashV2: oldV2Hash, } r := createTestReconcilerWithStatus(dgd) require.False(t, r.supportsManagedRollingUpdate(dgd)) require.NoError(t, r.migrateCurrentWorkerHashIfNeeded(context.Background(), dgd)) - require.Equal(t, legacyHash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + require.Equal(t, "v1hash00", dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.Equal(t, oldV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) trigger, err := r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) require.False(t, trigger) dgd.Spec.BackendFramework = "sglang" - - newLegacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - require.NoError(t, err) newV2Hash := betaDGDWorkersSpecHash(t, dgd) - require.Equal(t, legacyHash, newLegacyHash) - require.NotEqual(t, v2Hash, newV2Hash) - - require.NoError(t, r.migrateCurrentWorkerHashIfNeeded(context.Background(), dgd)) - require.Empty(t, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) - - desired, err := r.desiredWorkerHashes(dgd) + require.NotEqual(t, oldV2Hash, newV2Hash) + trigger, err = r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) - completed := r.workerHashesForUnsupportedPathway(dgd, desired) - require.Empty(t, completed.v1) - require.Equal(t, newV2Hash, completed.v2) + require.True(t, trigger) - r.setCurrentWorkerHashes(dgd, completed) + r.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: newV2Hash}) rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) require.NoError(t, err) require.Equal(t, newV2Hash, rollingCtx.NewWorkerHash) + require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) } func TestSupportsManagedRollingUpdate(t *testing.T) { @@ -1501,7 +1435,7 @@ func TestGetExistingRestartAnnotationsDCD(t *testing.T) { }, }) // Annotation hash can differ from computed hash — function uses active compatibility hash. - computedHash := legacyDGDWorkersSpecHash(t, dgd) + computedHash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: "oldhash", } @@ -1550,7 +1484,7 @@ func TestGetExistingRestartAnnotationsDCD(t *testing.T) { ComponentType: consts.ComponentTypeWorker, }, }) - legacyHash := legacyDGDWorkersSpecHash(t, dgd) + legacyHash := betaDGDWorkersSpecHash(t, dgd) v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: legacyHash, @@ -1659,7 +1593,7 @@ func TestCheckComponentFullyUpdated(t *testing.T) { ComponentType: consts.ComponentTypeWorker, }, }) - workerHash := legacyDGDWorkersSpecHash(t, dgd) + workerHash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: workerHash, } @@ -1695,7 +1629,7 @@ func TestCheckComponentFullyUpdated(t *testing.T) { ComponentType: consts.ComponentTypeWorker, }, }) - legacyHash := legacyDGDWorkersSpecHash(t, dgd) + legacyHash := betaDGDWorkersSpecHash(t, dgd) v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: legacyHash, @@ -3105,7 +3039,7 @@ func TestReconcileRollingUpdate_NoChange(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": {ComponentType: consts.ComponentTypeWorker}, }) - hash := legacyDGDWorkersSpecHash(t, dgd) + hash := betaDGDWorkersSpecHash(t, dgd) v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: hash, @@ -3158,7 +3092,7 @@ func TestReconcileRollingUpdate_StuckDetection(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": {ComponentType: consts.ComponentTypeWorker}, }) - hash := legacyDGDWorkersSpecHash(t, dgd) + hash := betaDGDWorkersSpecHash(t, dgd) // Hash matches current but phase is InProgress — stuck dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: hash, @@ -3226,7 +3160,7 @@ func TestReconcileRollingUpdate_StaleAnnotationRequiresAllNewWorkersReady(t *tes dgd.Status.RollingUpdate = &nvidiacomv1beta1.RollingUpdateStatus{ Phase: nvidiacomv1beta1.RollingUpdatePhaseCompleted, } - newHash := legacyDGDWorkersSpecHash(t, dgd) + newHash := betaDGDWorkersSpecHash(t, dgd) require.NotEqual(t, testOldWorkerHash, newHash) newPrefillDCD := betaDCD(t, &nvidiacomv1alpha1.DynamoComponentDeployment{ @@ -3268,7 +3202,7 @@ func TestReconcileRollingUpdate_StaleAnnotationUpdatesAfterAllNewWorkersReady(t dgd.Status.RollingUpdate = &nvidiacomv1beta1.RollingUpdateStatus{ Phase: nvidiacomv1beta1.RollingUpdatePhaseCompleted, } - newHash := legacyDGDWorkersSpecHash(t, dgd) + newHash := betaDGDWorkersSpecHash(t, dgd) require.NotEqual(t, testOldWorkerHash, newHash) makeReadyDCD := func(componentName, componentType string) *nvidiacomv1beta1.DynamoComponentDeployment { @@ -3334,10 +3268,9 @@ func TestReconcileRollingUpdate_StuckDetection_CompletesViaCompleteRollingUpdate "prefill": {ComponentType: consts.ComponentTypePrefill}, "decode": {ComponentType: consts.ComponentTypeDecode}, }) - legacyHash := legacyDGDWorkersSpecHash(t, dgd) v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: legacyHash, + consts.AnnotationCurrentWorkerHash: v2Hash, consts.AnnotationCurrentWorkerHashV2: v2Hash, } dgd.Status.RollingUpdate = &nvidiacomv1beta1.RollingUpdateStatus{ @@ -3355,9 +3288,9 @@ func TestReconcileRollingUpdate_StuckDetection_CompletesViaCompleteRollingUpdate // UpdatedComponents should contain all worker services assert.Contains(t, dgd.Status.RollingUpdate.UpdatedComponents, "prefill") assert.Contains(t, dgd.Status.RollingUpdate.UpdatedComponents, "decode") - // Completion records both active compatibility hashes. - assert.Equal(t, legacyHash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - assert.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + // Completion collapses an already-v2 generation to the canonical annotation. + assert.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + assert.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) } func TestBuildRollingUpdateContext(t *testing.T) { @@ -3653,7 +3586,7 @@ func TestBuildRollingUpdateContext(t *testing.T) { } // Compute the actual new DCD label hash from the DGD spec. - newHash := legacyDGDWorkersSpecHash(t, dgd) + newHash := betaDGDWorkersSpecHash(t, dgd) require.NotEqual(t, testOldWorkerHash, newHash, "test setup: computed hash must differ from old hash") // Collect all mock objects @@ -3719,7 +3652,7 @@ func TestBuildRollingUpdateContext_NoNewDCDExists(t *testing.T) { }, }) - newHash := legacyDGDWorkersSpecHash(t, dgd) + newHash := betaDGDWorkersSpecHash(t, dgd) assert.NotEqual(t, testOldWorkerHash, newHash, "test setup: computed hash must differ from old hash") r := createTestReconcilerWithStatus(dgd, withObjects(oldDCD)) @@ -3745,7 +3678,7 @@ func TestBuildRollingUpdateContext_ListOldDCDsError(t *testing.T) { consts.AnnotationCurrentWorkerHash: testOldWorkerHash, } - assert.NotEqual(t, testOldWorkerHash, legacyDGDWorkersSpecHash(t, dgd), + assert.NotEqual(t, testOldWorkerHash, betaDGDWorkersSpecHash(t, dgd), "test setup: computed hash must differ so we proceed past the early-return") injectedErr := errors.New("simulated apiserver list failure") @@ -3776,7 +3709,7 @@ func TestBuildRollingUpdateContext_GetNewDCDError(t *testing.T) { consts.AnnotationCurrentWorkerHash: testOldWorkerHash, } - require.NotEqual(t, testOldWorkerHash, legacyDGDWorkersSpecHash(t, dgd), + require.NotEqual(t, testOldWorkerHash, betaDGDWorkersSpecHash(t, dgd), "test setup: computed hash must differ so we proceed past the early-return") injectedErr := errors.New("simulated apiserver get failure") diff --git a/deploy/operator/internal/controller/test_beta_helpers_test.go b/deploy/operator/internal/controller/test_beta_helpers_test.go index 9eda856d7268..9a0b2c1153af 100644 --- a/deploy/operator/internal/controller/test_beta_helpers_test.go +++ b/deploy/operator/internal/controller/test_beta_helpers_test.go @@ -86,15 +86,6 @@ func betaDGDWorkersSpecHash(t testing.TB, dgd *v1beta1.DynamoGraphDeployment) st return hash } -func legacyDGDWorkersSpecHash(t testing.TB, dgd *v1beta1.DynamoGraphDeployment) string { - t.Helper() - hash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - if err != nil { - t.Fatalf("compute v1alpha1-compatible DGD worker hash: %v", err) - } - return hash -} - func betaRestartStatus(src *v1alpha1.RestartStatus) *v1beta1.RestartStatus { if src == nil { return nil diff --git a/deploy/operator/internal/dynamo/hash.go b/deploy/operator/internal/dynamo/hash.go index 126f9b625563..3b71c7be6f79 100644 --- a/deploy/operator/internal/dynamo/hash.go +++ b/deploy/operator/internal/dynamo/hash.go @@ -23,23 +23,11 @@ import ( "encoding/json" "fmt" - "github.com/ai-dynamo/dynamo/deploy/operator/api/v1alpha1" "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" ) const dgdWorkerHashPlaceholderValue = "worker-hash-placeholder" -// ComputeLegacyAlphaDGDWorkersSpecHash returns the v1alpha1 worker hash that a -// pre-v1beta1 controller would compute for the DGD's current spec. Conversion -// must preserve every v1alpha1 hash input shape this depends on. -func ComputeLegacyAlphaDGDWorkersSpecHash(dgd *v1beta1.DynamoGraphDeployment) (string, error) { - alpha := &v1alpha1.DynamoGraphDeployment{} - if err := alpha.ConvertFrom(dgd); err != nil { - return "", err - } - return v1alpha1.ComputeDGDWorkersSpecHash(alpha) -} - // ComputeDGDWorkersSpecHash computes the v2 worker hash from the worker DCDs // generated by the same DGD-to-DCD path used by reconciliation. The active // worker hash is replaced by a fixed placeholder before generation so the hash diff --git a/deploy/operator/internal/dynamo/hash_test.go b/deploy/operator/internal/dynamo/hash_test.go index db835638885c..fc2d2812b048 100644 --- a/deploy/operator/internal/dynamo/hash_test.go +++ b/deploy/operator/internal/dynamo/hash_test.go @@ -66,72 +66,6 @@ func TestComputeBetaDGDWorkersSpecHash_Deterministic(t *testing.T) { assert.Len(t, h1, 8) } -func TestComputeLegacyAlphaDGDWorkersSpecHash_MatchesV1Alpha1Hash(t *testing.T) { - alpha := baseDGD(map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, - Resources: &v1alpha1.Resources{ - Requests: &v1alpha1.ResourceItem{CPU: "1", Memory: "1Gi"}, - }, - Labels: map[string]string{"resource-label": "ignored-by-legacy-hash"}, - Annotations: map[string]string{"resource-annotation": "ignored-by-legacy-hash"}, - }, - }) - alpha.Annotations = map[string]string{"nvidia.com/current-worker-hash": "old-alpha-hash"} - beta := &v1beta1.DynamoGraphDeployment{} - assert.NoError(t, alpha.ConvertTo(beta)) - - legacyHash, err := ComputeLegacyAlphaDGDWorkersSpecHash(beta) - assert.NoError(t, err) - expectedLegacyHash, err := v1alpha1.ComputeDGDWorkersSpecHash(alpha) - assert.NoError(t, err) - assert.Equal(t, expectedLegacyHash, legacyHash) - assert.NotEqual(t, mustComputeBetaDGDWorkersSpecHash(t, beta), legacyHash) -} - -func TestComputeLegacyAlphaDGDWorkersSpecHash_RecoversNameOnlyMainContainerHash(t *testing.T) { - alpha := baseDGD(map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - ExtraPodSpec: &v1alpha1.ExtraPodSpec{ - MainContainer: &corev1.Container{Name: commonconsts.MainContainerName}, - }, - }, - }) - directAlphaHash, err := v1alpha1.ComputeDGDWorkersSpecHash(alpha) - assert.NoError(t, err) - assert.Equal(t, "0c322ce0", directAlphaHash) - - beta := &v1beta1.DynamoGraphDeployment{} - assert.NoError(t, alpha.ConvertTo(beta)) - recomputedHash, err := ComputeLegacyAlphaDGDWorkersSpecHash(beta) - assert.NoError(t, err) - - assert.Equal(t, directAlphaHash, recomputedHash) -} - -func TestComputeLegacyAlphaDGDWorkersSpecHash_RecoversMultipleCompilationCacheVolumeMounts(t *testing.T) { - alpha := baseDGD(map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: commonconsts.ComponentTypeWorker, - VolumeMounts: []v1alpha1.VolumeMount{ - {Name: "model-cache", MountPoint: "/models", UseAsCompilationCache: true}, - {Name: "compile-cache", MountPoint: "/compile", UseAsCompilationCache: true}, - }, - }, - }) - directAlphaHash, err := v1alpha1.ComputeDGDWorkersSpecHash(alpha) - assert.NoError(t, err) - - beta := &v1beta1.DynamoGraphDeployment{} - assert.NoError(t, alpha.ConvertTo(beta)) - recomputedHash, err := ComputeLegacyAlphaDGDWorkersSpecHash(beta) - assert.NoError(t, err) - - assert.Equal(t, directAlphaHash, recomputedHash) -} - func TestComputeBetaDGDWorkersSpecHash_IgnoresNonWorkers(t *testing.T) { withFrontend := baseDGD(map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": {ComponentType: commonconsts.ComponentTypeWorker}, From ac994a38a85c3999a0ca6a8dd56b8cfc50252817 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 Jul 2026 13:50:43 +0200 Subject: [PATCH 02/10] fixup! operator/worker-hash: converge generations lazily to v2 Signed-off-by: Dr. Stefan Schimanski --- .../api/v1alpha1/legacy_worker_hash.go | 71 +++ .../api/v1alpha1/legacy_worker_hash_test.go | 522 ++++++++++++++++++ .../shared_spec_conversion_bugs_test.go | 22 + deploy/operator/internal/consts/consts.go | 2 + .../dynamographdeployment_controller.go | 26 +- .../dynamographdeployment_controller_test.go | 6 +- .../dynamographdeployment_rollingupdate.go | 97 +++- ...ynamographdeployment_rollingupdate_test.go | 105 ++++ deploy/operator/internal/dynamo/hash.go | 12 + deploy/operator/internal/dynamo/hash_test.go | 66 +++ 10 files changed, 890 insertions(+), 39 deletions(-) create mode 100644 deploy/operator/api/v1alpha1/legacy_worker_hash.go create mode 100644 deploy/operator/api/v1alpha1/legacy_worker_hash_test.go diff --git a/deploy/operator/api/v1alpha1/legacy_worker_hash.go b/deploy/operator/api/v1alpha1/legacy_worker_hash.go new file mode 100644 index 000000000000..c1116679fe9b --- /dev/null +++ b/deploy/operator/api/v1alpha1/legacy_worker_hash.go @@ -0,0 +1,71 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package v1alpha1 + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "sort" +) + +// ComputeDGDWorkersSpecHash computes the worker hash used by the +// v1alpha1 DGD controller. Controller reconciliation may use this to compare +// v1/v2 worker generations, but API conversion must not derive controller +// rollout state from it. +// +// Keep this in the api/v1alpha1 package so internal controller helpers can +// reproduce the legacy hash without duplicating the old algorithm. +func ComputeDGDWorkersSpecHash(dgd *DynamoGraphDeployment) (string, error) { + if dgd == nil { + return "", fmt.Errorf("nil DynamoGraphDeployment") + } + + var workerNames []string + for name, spec := range dgd.Spec.Services { + if spec != nil && isV1alpha1WorkerComponent(spec.ComponentType) { + workerNames = append(workerNames, name) + } + } + sort.Strings(workerNames) + + hashInputs := make(map[string]DynamoComponentDeploymentSharedSpec) + for _, name := range workerNames { + hashInputs[name] = stripV1alpha1NonPodTemplateFields(dgd.Spec.Services[name]) + } + + data, err := json.Marshal(hashInputs) + if err != nil { + return "", err + } + + hash := sha256.Sum256(data) + return hex.EncodeToString(hash[:])[:8], nil +} + +func isV1alpha1WorkerComponent(componentType string) bool { + return componentType == "worker" || componentType == "prefill" || componentType == "decode" +} + +func stripV1alpha1NonPodTemplateFields(spec *DynamoComponentDeploymentSharedSpec) DynamoComponentDeploymentSharedSpec { + stripped := *spec + + stripped.Annotations = nil + stripped.Labels = nil + stripped.ServiceName = "" + stripped.ComponentType = "" + stripped.SubComponentType = "" + stripped.DynamoNamespace = nil + stripped.Replicas = nil + stripped.Autoscaling = nil //nolint:staticcheck // SA1019: intentionally matching the old v1alpha1 worker hash + stripped.ScalingAdapter = nil + stripped.Ingress = nil + stripped.ModelRef = nil + stripped.EPPConfig = nil + + return stripped +} diff --git a/deploy/operator/api/v1alpha1/legacy_worker_hash_test.go b/deploy/operator/api/v1alpha1/legacy_worker_hash_test.go new file mode 100644 index 000000000000..d79c987311e6 --- /dev/null +++ b/deploy/operator/api/v1alpha1/legacy_worker_hash_test.go @@ -0,0 +1,522 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package v1alpha1 + +import ( + "testing" + "unicode/utf8" + + "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" + commonconsts "github.com/ai-dynamo/dynamo/deploy/operator/internal/consts" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" +) + +const currentWorkerHashAnnotation = "nvidia.com/current-worker-hash" + +func TestComputeDGDWorkersSpecHashGolden(t *testing.T) { + for _, tt := range legacyWorkerHashGoldenCases() { + t.Run(tt.name, func(t *testing.T) { + got1, err := ComputeDGDWorkersSpecHash(tt.dgd) + if err != nil { + t.Fatalf("ComputeDGDWorkersSpecHash: %v", err) + } + got2, err := ComputeDGDWorkersSpecHash(tt.dgd) + if err != nil { + t.Fatalf("second ComputeDGDWorkersSpecHash: %v", err) + } + if got1 != got2 { + t.Fatalf("hash is not stable: first %q, second %q", got1, got2) + } + if got1 != tt.want { + t.Fatalf("ComputeDGDWorkersSpecHash() = %q, want golden %q", got1, tt.want) + } + }) + } +} + +type legacyWorkerHashGoldenCase struct { + name string + dgd *DynamoGraphDeployment + want string +} + +// legacyWorkerHashGoldenCases are golden values from the v1.1.x worker-hash +// algorithm in deploy/operator/internal/dynamo/hash.go. Changing any value here +// is a rollout-compatibility change and needs an explicit migration plan. +func legacyWorkerHashGoldenCases() []legacyWorkerHashGoldenCase { + return []legacyWorkerHashGoldenCase{ + { + name: "worker", + dgd: legacyWorkerHashDGD(), + want: "9b66accc", + }, + { + name: "resource metadata and non-workers ignored", + dgd: legacyWorkerHashDGDWithResourceMetadataAndNonWorkerChanges(), + want: "9b66accc", + }, + { + name: "pod metadata included", + dgd: legacyWorkerHashDGDWithPodMetadataChanges(), + want: "af8a6c60", + }, + { + name: "no workers", + dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "frontend": {ComponentType: commonconsts.ComponentTypeFrontend}, + }), + want: "44136fa3", + }, + { + name: "nil services", + dgd: legacyWorkerHashDGDFromServices(nil), + want: "44136fa3", + }, + { + name: "worker ordering", + dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "z-worker": {ComponentType: commonconsts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "Z", Value: "1"}}}, + "a-worker": {ComponentType: commonconsts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "A", Value: "1"}}}, + }), + want: "59cae8b3", + }, + { + name: "all worker component types", + dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "decode": {ComponentType: commonconsts.ComponentTypeDecode, Envs: []corev1.EnvVar{{Name: "ROLE", Value: "decode"}}}, + "prefill": {ComponentType: commonconsts.ComponentTypePrefill, Envs: []corev1.EnvVar{{Name: "ROLE", Value: "prefill"}}}, + "worker": {ComponentType: commonconsts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "ROLE", Value: "worker"}}}, + }), + want: "b175ee30", + }, + { + name: "main container name only", + dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + ExtraPodSpec: &ExtraPodSpec{ + MainContainer: &corev1.Container{Name: commonconsts.MainContainerName}, + }, + }, + }), + want: "0c322ce0", + }, + { + name: "main container rich", + dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + ExtraPodSpec: &ExtraPodSpec{ + MainContainer: &corev1.Container{ + Name: commonconsts.MainContainerName, + Image: "worker:1", + Command: []string{"python", "-m", "server"}, + Args: []string{"--model", "qwen"}, + Env: []corev1.EnvVar{{Name: "EXTRA", Value: "true"}}, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("2")}, + }, + }, + }, + }, + }), + want: "9e64367a", + }, + { + name: "pod spec rich", + dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + ExtraPodSpec: &ExtraPodSpec{ + PodSpec: &corev1.PodSpec{ + NodeSelector: map[string]string{"gpu": "true"}, + Tolerations: []corev1.Toleration{{ + Key: "nvidia.com/gpu", + Operator: corev1.TolerationOpExists, + }}, + Volumes: []corev1.Volume{{ + Name: "cache", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }}, + }, + }, + }, + }), + want: "1fcd5d7c", + }, + { + name: "resources requests limits claims", + dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + Resources: &Resources{ + Requests: &ResourceItem{CPU: "1", Memory: "4Gi", GPU: "1", GPUType: "nvidia.com/gpu"}, + Limits: &ResourceItem{CPU: "2", Memory: "8Gi", GPU: "1"}, + Claims: []corev1.ResourceClaim{{Name: "gpu-claim"}}, + }, + }, + }), + want: "a4172e4e", + }, + { + name: "envs volume mounts secret", + dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + Envs: []corev1.EnvVar{{Name: "MODEL", Value: "llama"}}, + EnvFromSecret: ptr.To("worker-secret"), + VolumeMounts: []VolumeMount{{Name: "models", MountPoint: "/models", UseAsCompilationCache: true}}, + SharedMemory: &SharedMemorySpec{Size: resource.MustParse("2Gi")}, + LivenessProbe: &corev1.Probe{InitialDelaySeconds: 5}, + ReadinessProbe: &corev1.Probe{TimeoutSeconds: 3}, + }, + }), + want: "a0ceefd2", + }, + { + name: "multiple compilation cache volume mounts", + dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + VolumeMounts: []VolumeMount{ + {Name: "model-cache", MountPoint: "/models", UseAsCompilationCache: true}, + {Name: "compile-cache", MountPoint: "/compile", UseAsCompilationCache: true}, + }, + }, + }), + want: "5a3c0f65", + }, + { + name: "ignored scaling ingress model", + dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + Annotations: map[string]string{"ignored": "true"}, + Labels: map[string]string{"ignored": "true"}, + Replicas: ptr.To(int32(3)), + Autoscaling: &Autoscaling{Enabled: true, MinReplicas: 1, MaxReplicas: 10}, + Ingress: &IngressSpec{Enabled: true, Host: "example.com"}, + ModelRef: &ModelReference{Name: "model"}, + ScalingAdapter: &ScalingAdapter{Enabled: true}, + }, + }), + want: "769fa7c7", + }, + { + name: "multinode sidecar checkpoint", + dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + Multinode: &MultinodeSpec{NodeCount: 2}, + FrontendSidecar: &FrontendSidecarSpec{ + Image: "frontend:1", + Args: []string{"--router-mode", "direct"}, + }, + Checkpoint: &ServiceCheckpointConfig{Enabled: true}, + }, + }), + want: "bf66a8e3", + }, + { + name: "probe handler", + dgd: legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromString("http"), + }, + }, + }, + }, + }), + want: "8ddbbadc", + }, + } +} + +func TestComputeDGDWorkersSpecHashNil(t *testing.T) { + if _, err := ComputeDGDWorkersSpecHash(nil); err == nil { + t.Fatal("ComputeDGDWorkersSpecHash(nil) error = nil, want error") + } +} + +func TestDGDConvertToPreservesWorkerHashAnnotationOpaquely(t *testing.T) { + src := legacyWorkerHashDGD() + src.Annotations = map[string]string{ + currentWorkerHashAnnotation: "controller-owned-hash", + "user": "kept", + } + + hub := &v1beta1.DynamoGraphDeployment{} + if err := src.ConvertTo(hub); err != nil { + t.Fatalf("ConvertTo: %v", err) + } + + if got := hub.Annotations[currentWorkerHashAnnotation]; got != "controller-owned-hash" { + t.Fatalf("%s = %q, want controller-owned-hash", currentWorkerHashAnnotation, got) + } + if got := hub.Annotations["user"]; got != "kept" { + t.Fatalf("user annotation = %q, want kept", got) + } +} + +func TestComputeDGDWorkersSpecHashConversionRoundTripExact(t *testing.T) { + alpha := legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + ExtraPodSpec: &ExtraPodSpec{ + MainContainer: &corev1.Container{Name: commonconsts.MainContainerName}, + }, + }, + }) + + directHash, err := ComputeDGDWorkersSpecHash(alpha) + if err != nil { + t.Fatalf("ComputeDGDWorkersSpecHash(alpha): %v", err) + } + roundTripHash, err := legacyWorkerHashAfterBetaRoundTrip(alpha) + if err != nil { + t.Fatalf("legacyWorkerHashAfterBetaRoundTrip: %v", err) + } + + if directHash != "0c322ce0" { + t.Fatalf("direct alpha hash = %q, want 0c322ce0", directHash) + } + if roundTripHash != directHash { + t.Fatalf("round-tripped alpha hash = %q, want direct alpha hash %q", roundTripHash, directHash) + } +} + +func TestComputeDGDWorkersSpecHashConversionRoundTripStableSubset(t *testing.T) { + for _, tt := range legacyWorkerHashGoldenCases() { + t.Run(tt.name, func(t *testing.T) { + roundTripHash, err := legacyWorkerHashAfterBetaRoundTrip(tt.dgd) + if err != nil { + t.Fatalf("legacyWorkerHashAfterBetaRoundTrip: %v", err) + } + if roundTripHash != tt.want { + t.Fatalf("round-tripped alpha hash = %q, want direct golden %q", roundTripHash, tt.want) + } + }) + } +} + +func TestComputeDGDWorkersSpecHashTracksPodMetadata(t *testing.T) { + baseHash, err := ComputeDGDWorkersSpecHash(legacyWorkerHashDGD()) + if err != nil { + t.Fatalf("ComputeDGDWorkersSpecHash: %v", err) + } + + mutated := legacyWorkerHashDGDWithPodMetadataChanges() + + got, err := ComputeDGDWorkersSpecHash(mutated) + if err != nil { + t.Fatalf("ComputeDGDWorkersSpecHash: %v", err) + } + if got == baseHash { + t.Fatalf("pod metadata change did not change preserved legacy worker hash: %q", got) + } +} + +func TestComputeDGDWorkersSpecHashIgnoresResourceMetadataAndNonWorkers(t *testing.T) { + baseHash, err := ComputeDGDWorkersSpecHash(legacyWorkerHashDGD()) + if err != nil { + t.Fatalf("ComputeDGDWorkersSpecHash: %v", err) + } + + mutated := legacyWorkerHashDGDWithResourceMetadataAndNonWorkerChanges() + + got, err := ComputeDGDWorkersSpecHash(mutated) + if err != nil { + t.Fatalf("ComputeDGDWorkersSpecHash: %v", err) + } + if got != baseHash { + t.Fatalf("non-pod-template metadata/non-worker changes changed preserved legacy worker hash: got %q, want %q", got, baseHash) + } +} + +func FuzzComputeDGDWorkersSpecHashDeterministic(f *testing.F) { + f.Add("worker", "MODEL", "llama", "checksum/config", "base", false) + f.Add("prefill", "ROLE", "prefill", "rollout", "v1", true) + f.Add("decode", "", "", "", "", false) + + f.Fuzz(func(t *testing.T, componentType, envName, envValue, metadataKey, metadataValue string, includeMainContainer bool) { + switch componentType { + case commonconsts.ComponentTypeWorker, commonconsts.ComponentTypePrefill, commonconsts.ComponentTypeDecode: + default: + componentType = commonconsts.ComponentTypeWorker + } + + spec := &DynamoComponentDeploymentSharedSpec{ + ComponentType: componentType, + Envs: []corev1.EnvVar{{Name: envName, Value: envValue}}, + ExtraPodMetadata: &ExtraPodMetadata{ + Labels: map[string]string{metadataKey: metadataValue}, + }, + } + if includeMainContainer { + spec.ExtraPodSpec = &ExtraPodSpec{ + MainContainer: &corev1.Container{Name: commonconsts.MainContainerName, Image: envValue}, + } + } + + dgd := legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "worker": spec, + }) + got1, err := ComputeDGDWorkersSpecHash(dgd) + if err != nil { + t.Fatalf("ComputeDGDWorkersSpecHash: %v", err) + } + got2, err := ComputeDGDWorkersSpecHash(dgd) + if err != nil { + t.Fatalf("second ComputeDGDWorkersSpecHash: %v", err) + } + if got1 != got2 { + t.Fatalf("hash is not deterministic: first %q, second %q", got1, got2) + } + if len(got1) != 8 { + t.Fatalf("hash length = %d, want 8: %q", len(got1), got1) + } + }) +} + +func FuzzComputeDGDWorkersSpecHashConversionRoundTripStableSubset(f *testing.F) { + f.Add("worker", "MODEL", "llama", "rollout", "base", "1", "4Gi") + f.Add("prefill", "ROLE", "prefill", "checksum", "v1", "", "") + f.Add("decode", "", "", "", "", "250m", "1Gi") + + f.Fuzz(func(t *testing.T, componentType, envName, envValue, metadataKey, metadataValue, cpu, memory string) { + for _, value := range []string{componentType, envName, envValue, metadataKey, metadataValue, cpu, memory} { + if !utf8.ValidString(value) { + t.Skip("Kubernetes JSON strings are valid UTF-8") + } + } + if cpu != "" { + if _, err := resource.ParseQuantity(cpu); err != nil { + t.Skip("invalid CPU quantity") + } + } + if memory != "" { + if _, err := resource.ParseQuantity(memory); err != nil { + t.Skip("invalid memory quantity") + } + } + switch componentType { + case commonconsts.ComponentTypeWorker, commonconsts.ComponentTypePrefill, commonconsts.ComponentTypeDecode: + default: + componentType = commonconsts.ComponentTypeWorker + } + + alpha := legacyWorkerHashDGDFromServices(map[string]*DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: componentType, + Envs: []corev1.EnvVar{{Name: envName, Value: envValue}}, + ExtraPodMetadata: &ExtraPodMetadata{ + Labels: map[string]string{metadataKey: metadataValue}, + }, + Resources: &Resources{ + Requests: &ResourceItem{CPU: cpu, Memory: memory}, + }, + }, + }) + + directHash, err := ComputeDGDWorkersSpecHash(alpha) + if err != nil { + t.Fatalf("ComputeDGDWorkersSpecHash(alpha): %v", err) + } + roundTripHash, err := legacyWorkerHashAfterBetaRoundTrip(alpha) + if err != nil { + t.Fatalf("legacyWorkerHashAfterBetaRoundTrip: %v", err) + } + if directHash != roundTripHash { + t.Fatalf("round-trip changed stable-subset hash: direct %q, round-trip %q", directHash, roundTripHash) + } + }) +} + +func legacyWorkerHashAfterBetaRoundTrip(src *DynamoGraphDeployment) (string, error) { + hub := &v1beta1.DynamoGraphDeployment{} + if err := src.ConvertTo(hub); err != nil { + return "", err + } + + roundTripped := &DynamoGraphDeployment{} + if err := roundTripped.ConvertFrom(hub); err != nil { + return "", err + } + return ComputeDGDWorkersSpecHash(roundTripped) +} + +func legacyWorkerHashDGDFromServices(services map[string]*DynamoComponentDeploymentSharedSpec) *DynamoGraphDeployment { + return &DynamoGraphDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "legacy-worker-hash", + Namespace: "ns", + }, + Spec: DynamoGraphDeploymentSpec{ + Services: services, + }, + } +} + +func legacyWorkerHashDGD() *DynamoGraphDeployment { + return &DynamoGraphDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "legacy-worker-hash", + Namespace: "ns", + }, + Spec: DynamoGraphDeploymentSpec{ + Services: map[string]*DynamoComponentDeploymentSharedSpec{ + "frontend": { + ComponentType: commonconsts.ComponentTypeFrontend, + Envs: []corev1.EnvVar{{Name: "FRONTEND_ONLY", Value: "ignored"}}, + }, + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + Annotations: map[string]string{"resource": "base"}, + Labels: map[string]string{"resource": "base"}, + Replicas: ptr.To(int32(2)), + Envs: []corev1.EnvVar{{Name: "MODEL", Value: "llama"}}, + ExtraPodMetadata: &ExtraPodMetadata{ + Labels: map[string]string{"rollout": "base"}, + Annotations: map[string]string{"checksum/config": "base"}, + }, + }, + }, + }, + } +} + +func legacyWorkerHashDGDWithPodMetadataChanges() *DynamoGraphDeployment { + mutated := legacyWorkerHashDGD() + mutated.Spec.Services["worker"].ExtraPodMetadata = &ExtraPodMetadata{ + Labels: map[string]string{"rollout": "changed"}, + Annotations: map[string]string{"checksum/config": "changed"}, + } + return mutated +} + +func legacyWorkerHashDGDWithResourceMetadataAndNonWorkerChanges() *DynamoGraphDeployment { + mutated := legacyWorkerHashDGD() + mutated.Spec.Services["worker"].Annotations = map[string]string{"resource": "changed"} + mutated.Spec.Services["worker"].Labels = map[string]string{"resource": "changed"} + mutated.Spec.Services["worker"].Replicas = ptr.To(int32(99)) + mutated.Spec.Services["worker"].Ingress = &IngressSpec{ + Enabled: true, + Host: "changed.example.com", + } + mutated.Spec.Services["frontend"].Envs = []corev1.EnvVar{{Name: "FRONTEND_ONLY", Value: "changed"}} + return mutated +} diff --git a/deploy/operator/api/v1alpha1/shared_spec_conversion_bugs_test.go b/deploy/operator/api/v1alpha1/shared_spec_conversion_bugs_test.go index c62d3e6c5377..555351060a1f 100644 --- a/deploy/operator/api/v1alpha1/shared_spec_conversion_bugs_test.go +++ b/deploy/operator/api/v1alpha1/shared_spec_conversion_bugs_test.go @@ -247,6 +247,10 @@ func TestBugDGD_SpokeMainContainerNameOnlyRoundTrips(t *testing.T) { }, }, } + wantHash, err := ComputeDGDWorkersSpecHash(in) + if err != nil { + t.Fatalf("ComputeDGDWorkersSpecHash(in) error = %v", err) + } hub := &v1beta1.DynamoGraphDeployment{} if err := in.ConvertTo(hub); err != nil { t.Fatalf("ConvertTo() error = %v", err) @@ -263,6 +267,13 @@ func TestBugDGD_SpokeMainContainerNameOnlyRoundTrips(t *testing.T) { if got.ExtraPodSpec.MainContainer.Name != mainContainerName { t.Fatalf("mainContainer.name = %q, want %q", got.ExtraPodSpec.MainContainer.Name, mainContainerName) } + gotHash, err := ComputeDGDWorkersSpecHash(out) + if err != nil { + t.Fatalf("ComputeDGDWorkersSpecHash(out) error = %v", err) + } + if gotHash != wantHash { + t.Fatalf("round-trip worker hash = %q, want %q", gotHash, wantHash) + } } func TestBugDGD_SpokeMultipleCompilationCacheVolumeMountsRoundTrip(t *testing.T) { @@ -280,6 +291,10 @@ func TestBugDGD_SpokeMultipleCompilationCacheVolumeMountsRoundTrip(t *testing.T) }, }, } + wantHash, err := ComputeDGDWorkersSpecHash(in) + if err != nil { + t.Fatalf("ComputeDGDWorkersSpecHash(in) error = %v", err) + } hub := &v1beta1.DynamoGraphDeployment{} if err := in.ConvertTo(hub); err != nil { t.Fatalf("ConvertTo() error = %v", err) @@ -302,6 +317,13 @@ func TestBugDGD_SpokeMultipleCompilationCacheVolumeMountsRoundTrip(t *testing.T) if diff := cmp.Diff(in.Spec.Services["worker"].VolumeMounts, out.Spec.Services["worker"].VolumeMounts); diff != "" { t.Fatalf("volume mounts changed after round-trip (-want +got):\n%s", diff) } + gotHash, err := ComputeDGDWorkersSpecHash(out) + if err != nil { + t.Fatalf("ComputeDGDWorkersSpecHash(out) error = %v", err) + } + if gotHash != wantHash { + t.Fatalf("round-trip worker hash = %q, want %q", gotHash, wantHash) + } } func TestBugDGD_ChangedCompilationCacheDoesNotRestoreStaleVolumeMounts(t *testing.T) { diff --git a/deploy/operator/internal/consts/consts.go b/deploy/operator/internal/consts/consts.go index 7c924562208f..6ea6c5b082e6 100644 --- a/deploy/operator/internal/consts/consts.go +++ b/deploy/operator/internal/consts/consts.go @@ -247,6 +247,8 @@ const ( // fingerprint of that same generation, allowing the controller to detect real // worker changes without recomputing the old hash. The next genuine worker // update creates a v2-identified generation and removes the sidecar annotation. + // A literal "legacy" generation is the one exception: it keeps computing its + // frozen v1 target until the migration already defined by 1.2 completes. // AnnotationCurrentWorkerHash stores the active worker generation identity. AnnotationCurrentWorkerHash = "nvidia.com/current-worker-hash" diff --git a/deploy/operator/internal/controller/dynamographdeployment_controller.go b/deploy/operator/internal/controller/dynamographdeployment_controller.go index 6aaec3bbe06a..bb32496333e4 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_controller.go +++ b/deploy/operator/internal/controller/dynamographdeployment_controller.go @@ -232,14 +232,14 @@ func (r *DynamoGraphDeploymentReconciler) Reconcile(ctx context.Context, req ctr } } else { if r.currentWorkerState(dynamoDeployment).empty() { - hash, err := r.desiredWorkerHash(dynamoDeployment) + target, err := r.desiredWorkerGeneration(dynamoDeployment) if err != nil { logger.Error(err, "Failed to compute worker hash for unsupported pathway") reason = reasonFailedToInitializeWorkerHash message = Message(err.Error()) return ctrl.Result{}, err } - r.setCurrentWorkerState(dynamoDeployment, workerGenerationState{activeGeneration: hash}) + r.setCurrentWorkerState(dynamoDeployment, workerStateForCompletedGeneration(target.generation, target)) if updateErr := r.Update(ctx, dynamoDeployment); updateErr != nil { logger.Error(updateErr, "Failed to initialize worker hash for unsupported pathway") reason = reasonFailedToInitializeWorkerHash @@ -264,10 +264,10 @@ func (r *DynamoGraphDeploymentReconciler) Reconcile(ctx context.Context, req ctr r.Recorder.Event(dynamoDeployment, corev1.EventTypeWarning, "RollingUpdateNotSupported", "Worker spec changed but custom rolling updates are not supported for Grove/multinode deployments") - // Update the hash to prevent repeated warnings. This branch only runs - // for a real v2 worker-spec change; unchanged 1.2 bridge state compares - // equal through current-worker-hash-v2 and remains untouched. - hash, err := r.desiredWorkerHash(dynamoDeployment) + // Update the hash to prevent repeated warnings. Unchanged 1.2 bridge + // state compares equal through current-worker-hash-v2. A literal legacy + // migration retains the 1.2 v1 target and its v2 fingerprint. + target, err := r.desiredWorkerGeneration(dynamoDeployment) if err != nil { logger.Error(err, "Failed to compute worker hash for unsupported pathway") state = nvidiacomv1beta1.DGDStateFailed @@ -275,7 +275,7 @@ func (r *DynamoGraphDeploymentReconciler) Reconcile(ctx context.Context, req ctr message = Message(err.Error()) return ctrl.Result{}, err } - r.setCurrentWorkerState(dynamoDeployment, workerGenerationState{activeGeneration: hash}) + r.setCurrentWorkerState(dynamoDeployment, workerStateForCompletedGeneration(target.generation, target)) if updateErr := r.Update(ctx, dynamoDeployment); updateErr != nil { logger.Error(updateErr, "Failed to update worker hash for unsupported pathway") } @@ -1456,13 +1456,13 @@ func (r *DynamoGraphDeploymentReconciler) checkComponentFullyUpdated(ctx context return checkDCDReady(ctx, r.Client, resourceName, dgd.Namespace) } - hash, err := r.desiredWorkerHash(dgd) + target, err := r.desiredWorkerGeneration(dgd) if err != nil { return false, err.Error() } var lastReason string - for _, candidate := range r.activeWorkerHashCandidates(dgd, hash) { + for _, candidate := range r.activeWorkerHashCandidates(dgd, target) { resourceName := dynamo.GetDCDResourceName(dgd, componentName, candidate) ready, reason := checkDCDReady(ctx, r.Client, resourceName, dgd.Namespace) if ready || reason != "resource not found" { @@ -1769,11 +1769,11 @@ func (r *DynamoGraphDeploymentReconciler) preserveExistingDCDBackendFramework(ct func (r *DynamoGraphDeploymentReconciler) getExistingRestartAnnotationsDCD(ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment) (map[string]string, error) { logger := log.FromContext(ctx) - hash, err := r.desiredWorkerHash(dgd) + target, err := r.desiredWorkerGeneration(dgd) if err != nil { return nil, err } - workerHashes := r.activeWorkerHashCandidates(dgd, hash) + workerHashes := r.activeWorkerHashCandidates(dgd, target) restartAnnotations := make(map[string]string) for i := range dgd.Spec.Components { @@ -2336,11 +2336,11 @@ func (r *DynamoGraphDeploymentReconciler) checkpointWorkerHashForComponent(dgd * if r == nil { return "", nil } - desired, err := r.desiredWorkerHash(dgd) + target, err := r.desiredWorkerGeneration(dgd) if err != nil { return "", err } - return r.activeWorkerHashForDCDGeneration(dgd, desired), nil + return activeWorkerHashForDCDGeneration(target), nil } // buildCheckpointJobPodTemplate builds a checkpoint job template from the same diff --git a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go index 027b10e2b3d4..9c8154635b6e 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go @@ -2106,9 +2106,9 @@ func TestDynamoGraphDeploymentReconciler_checkpointWorkerHashForComponentUsesAct }, }, }) - desired, err := reconciler.desiredWorkerHash(dgd) + target, err := reconciler.desiredWorkerGeneration(dgd) if err != nil { - t.Fatalf("desiredWorkerHash() error = %v", err) + t.Fatalf("desiredWorkerGeneration() error = %v", err) } reconciler.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: "oldhash"}) @@ -2116,7 +2116,7 @@ func TestDynamoGraphDeploymentReconciler_checkpointWorkerHashForComponentUsesAct if err != nil { t.Fatalf("checkpointWorkerHashForComponent() error = %v", err) } - want := reconciler.activeWorkerHashForDCDGeneration(dgd, desired) + want := activeWorkerHashForDCDGeneration(target) if workerHash != want { t.Fatalf("checkpoint worker hash = %s, want active generated hash %s", workerHash, want) } diff --git a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go index 7e633e2b7a3e..9c6dfe4675bc 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go +++ b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go @@ -40,13 +40,23 @@ import ( // generation from the v2 fingerprint of its rendered worker spec. Dynamo 1.2 // stored a v1 generation identity in current-worker-hash and the corresponding // v2 fingerprint in current-worker-hash-v2. Keeping both values until a real -// worker change lets 1.4 stop computing v1 hashes without rolling unchanged -// workers merely to rename their generation. +// worker change lets normal 1.4 reconciliation stop computing v1 hashes without +// rolling unchanged workers merely to rename their generation. The literal +// legacy migration remains the sole exception until it reaches its 1.2 target. type workerGenerationState struct { activeGeneration string v2Fingerprint string } +// workerGenerationTarget is the generation this reconcile must create or +// continue, plus the v2 fingerprint that determines whether the worker spec +// changed. They differ while retaining a 1.2 v1-named generation, including +// while finishing a literal legacy migration on its original v1 target. +type workerGenerationTarget struct { + generation string + v2Fingerprint string +} + func (s workerGenerationState) empty() bool { return s.activeGeneration == "" && s.v2Fingerprint == "" } @@ -81,6 +91,29 @@ func (r *DynamoGraphDeploymentReconciler) desiredWorkerHash( return hash, nil } +func (r *DynamoGraphDeploymentReconciler) desiredWorkerGeneration( + dgd *nvidiacomv1beta1.DynamoGraphDeployment, +) (workerGenerationTarget, error) { + v2Fingerprint, err := r.desiredWorkerHash(dgd) + if err != nil { + return workerGenerationTarget{}, err + } + + current := r.currentWorkerState(dgd) + if current.activeGeneration == consts.LegacyWorkerHash { + generation, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + if err != nil { + return workerGenerationTarget{}, fmt.Errorf("failed to compute in-progress legacy migration target: %w", err) + } + return workerGenerationTarget{generation: generation, v2Fingerprint: v2Fingerprint}, nil + } + + return workerGenerationTarget{ + generation: workerHashForDCDGeneration(current, v2Fingerprint), + v2Fingerprint: v2Fingerprint, + }, nil +} + func (r *DynamoGraphDeploymentReconciler) currentWorkerState( dgd *nvidiacomv1beta1.DynamoGraphDeployment, ) workerGenerationState { @@ -97,6 +130,16 @@ func workerHashForDCDGeneration(current workerGenerationState, desired string) s return desired } +func workerStateForCompletedGeneration(newWorkerHash string, target workerGenerationTarget) workerGenerationState { + if newWorkerHash == target.v2Fingerprint { + return workerGenerationState{activeGeneration: newWorkerHash} + } + return workerGenerationState{ + activeGeneration: newWorkerHash, + v2Fingerprint: target.v2Fingerprint, + } +} + // shouldTriggerRollingUpdate compares desired worker hashes with the active // generation recorded on the DGD. // @@ -106,19 +149,24 @@ func workerHashForDCDGeneration(current workerGenerationState, desired string) s func (r *DynamoGraphDeploymentReconciler) shouldTriggerRollingUpdate( dgd *nvidiacomv1beta1.DynamoGraphDeployment, ) (bool, error) { - desired, err := r.desiredWorkerHash(dgd) + current := r.currentWorkerState(dgd) + if current.empty() { + return false, nil + } + + target, err := r.desiredWorkerGeneration(dgd) if err != nil { return false, err } - return !r.currentWorkerState(dgd).matchesDesired(desired), nil + return current.activeHash() != target.generation, nil } // initializeWorkerHashIfNeeded establishes the DGD's active worker generation. // New DGDs use the v2 hash directly. DGDs created before managed rolling updates // may already have worker DCDs without a hash label; in that case we label those -// DCDs with the legacy sentinel and let the normal rolling update path migrate -// from that sentinel to the desired v2 hash. +// DCDs with the legacy sentinel and let the normal rolling update path finish +// the v1 target selected by 1.2. That target then becomes a normal v1/v2 bridge. func (r *DynamoGraphDeploymentReconciler) initializeWorkerHashIfNeeded( ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment, @@ -217,25 +265,24 @@ func (r *DynamoGraphDeploymentReconciler) migrateCurrentWorkerHashIfNeeded( // activeWorkerHashForDCDGeneration returns the hash used for generated worker // DCD names and worker-hash labels in this reconcile. An unchanged 1.2 bridge -// state keeps its v1 generation identity; a real worker change uses the desired -// v2 hash and converges to a single-hash state when the rollout completes. -func (r *DynamoGraphDeploymentReconciler) activeWorkerHashForDCDGeneration( - dgd *nvidiacomv1beta1.DynamoGraphDeployment, - desired string, +// state keeps its v1 generation identity; a literal legacy migration keeps its +// 1.2 v1 target; every later real change uses v2 and converges to one hash. +func activeWorkerHashForDCDGeneration( + target workerGenerationTarget, ) string { - return r.activeWorkerHashCandidates(dgd, desired)[0] + return target.generation } func (r *DynamoGraphDeploymentReconciler) activeWorkerHashCandidates( dgd *nvidiacomv1beta1.DynamoGraphDeployment, - desired string, + target workerGenerationTarget, ) []string { current := r.currentWorkerState(dgd) candidates := make([]string, 0, 2) - generated := workerHashForDCDGeneration(current, desired) + generated := target.generation candidates = append(candidates, generated) - if current.v2Fingerprint == desired && desired != generated { - candidates = append(candidates, desired) + if current.v2Fingerprint == target.v2Fingerprint && target.v2Fingerprint != generated { + candidates = append(candidates, target.v2Fingerprint) } return candidates } @@ -372,11 +419,11 @@ func (r *DynamoGraphDeploymentReconciler) reconcileRollingUpdate( rollingUpdateStatus := r.getOrCreateRollingUpdateStatus(dgd) - desired, err := r.desiredWorkerHash(dgd) + target, err := r.desiredWorkerGeneration(dgd) if err != nil { return err } - newWorkerHash := r.activeWorkerHashForDCDGeneration(dgd, desired) + newWorkerHash := activeWorkerHashForDCDGeneration(target) current := r.currentWorkerState(dgd) logger.Info("Reconciling rolling update", @@ -384,7 +431,7 @@ func (r *DynamoGraphDeploymentReconciler) reconcileRollingUpdate( "activeWorkerGeneration", current.activeHash(), "currentV2Fingerprint", current.v2Fingerprint, "newWorkerHash", newWorkerHash, - "desiredWorkerHash", desired) + "desiredWorkerHash", target.v2Fingerprint) if rollingUpdateStatus.Phase == nvidiacomv1beta1.RollingUpdatePhaseCompleted && current.activeHash() != newWorkerHash { // Check if DCDs with the new hash already exist and are serving. @@ -396,7 +443,7 @@ func (r *DynamoGraphDeploymentReconciler) reconcileRollingUpdate( "activeWorkerGeneration", current.activeHash(), "currentV2Fingerprint", current.v2Fingerprint, "newHash", newWorkerHash) - r.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: newWorkerHash}) + r.setCurrentWorkerState(dgd, workerStateForCompletedGeneration(newWorkerHash, target)) return r.Update(ctx, dgd) } // New spec change: reset to start a proper rolling update cycle with surge/drain. @@ -563,13 +610,17 @@ func (r *DynamoGraphDeploymentReconciler) completeRollingUpdate( newWorkerHash string, ) error { logger := log.FromContext(ctx) + target, err := r.desiredWorkerGeneration(dgd) + if err != nil { + return err + } // Delete all non-current worker DCDs (any number of old generations) if err := r.deleteOldWorkerDCDs(ctx, dgd, newWorkerHash); err != nil { return fmt.Errorf("failed to delete old worker DCDs: %w", err) } - r.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: newWorkerHash}) + r.setCurrentWorkerState(dgd, workerStateForCompletedGeneration(newWorkerHash, target)) if err := r.Update(ctx, dgd); err != nil { return fmt.Errorf("failed to update current worker hash: %w", err) } @@ -1072,11 +1123,11 @@ func (r *DynamoGraphDeploymentReconciler) buildRollingUpdateContext( ) (dynamo.RollingUpdateContext, error) { logger := log.FromContext(ctx) - desiredHash, err := r.desiredWorkerHash(dgd) + target, err := r.desiredWorkerGeneration(dgd) if err != nil { return dynamo.RollingUpdateContext{}, err } - newWorkerHash := r.activeWorkerHashForDCDGeneration(dgd, desiredHash) + newWorkerHash := activeWorkerHashForDCDGeneration(target) current := r.currentWorkerState(dgd) if current.activeHash() == newWorkerHash { diff --git a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go index 1b2794a85642..8095a50af443 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go @@ -336,6 +336,111 @@ func TestBridgeState_NoOpUpgradeUsesExistingWorkerGeneration(t *testing.T) { require.Equal(t, "test-dgd-worker-"+activeHash, dcds["worker"].Name) } +func TestBridgeState_LeftoverLegacyDCDDoesNotTriggerAnotherRollout(t *testing.T) { + dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: consts.ComponentTypeWorker, + Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, + }, + }) + activeHash := "v1hash00" + v2Hash := betaDGDWorkersSpecHash(t, dgd) + dgd.Annotations = map[string]string{ + consts.AnnotationCurrentWorkerHash: activeHash, + consts.AnnotationCurrentWorkerHashV2: v2Hash, + } + legacyDCD := betaDCD(t, &nvidiacomv1alpha1.DynamoComponentDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-dgd-worker", + Namespace: "default", + Labels: map[string]string{ + consts.KubeLabelDynamoGraphDeploymentName: "test-dgd", + consts.KubeLabelDynamoWorkerHash: consts.LegacyWorkerHash, + }, + }, + Spec: nvidiacomv1alpha1.DynamoComponentDeploymentSpec{ + DynamoComponentDeploymentSharedSpec: nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ + ComponentType: consts.ComponentTypeWorker, + ServiceName: "worker", + }, + }, + }) + + r := createTestReconcilerWithStatus(dgd, withObjects(legacyDCD)) + trigger, err := r.shouldTriggerRollingUpdate(dgd) + require.NoError(t, err) + require.False(t, trigger) + + rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) + require.NoError(t, err) + require.Equal(t, activeHash, rollingCtx.NewWorkerHash) + require.False(t, rollingCtx.InProgress()) +} + +func TestLegacyMigrationFinishesOnV1TargetWithoutSecondRollout(t *testing.T) { + dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: consts.ComponentTypeWorker, + Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, + }, + }) + v1Hash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + require.NoError(t, err) + v2Hash := betaDGDWorkersSpecHash(t, dgd) + require.NotEqual(t, v1Hash, v2Hash) + dgd.Annotations = map[string]string{ + consts.AnnotationCurrentWorkerHash: consts.LegacyWorkerHash, + } + dgd.Status.RollingUpdate = &nvidiacomv1beta1.RollingUpdateStatus{ + Phase: nvidiacomv1beta1.RollingUpdatePhaseInProgress, + } + + r := createTestReconcilerWithStatus(dgd) + target, err := r.desiredWorkerGeneration(dgd) + require.NoError(t, err) + require.Equal(t, v1Hash, target.generation) + require.Equal(t, v2Hash, target.v2Fingerprint) + + rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) + require.NoError(t, err) + require.Equal(t, v1Hash, rollingCtx.NewWorkerHash) + + require.NoError(t, r.completeRollingUpdate(context.Background(), dgd, v1Hash)) + require.Equal(t, v1Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + + trigger, err := r.shouldTriggerRollingUpdate(dgd) + require.NoError(t, err) + require.False(t, trigger) +} + +func TestBridgeState_StaleInProgressCompletionPreservesFingerprint(t *testing.T) { + dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: consts.ComponentTypeWorker, + Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, + }, + }) + v1Hash := "v1hash00" + v2Hash := betaDGDWorkersSpecHash(t, dgd) + dgd.Annotations = map[string]string{ + consts.AnnotationCurrentWorkerHash: v1Hash, + consts.AnnotationCurrentWorkerHashV2: v2Hash, + } + dgd.Status.RollingUpdate = &nvidiacomv1beta1.RollingUpdateStatus{ + Phase: nvidiacomv1beta1.RollingUpdatePhaseInProgress, + } + + r := createTestReconcilerWithStatus(dgd) + require.NoError(t, r.reconcileRollingUpdate(context.Background(), dgd)) + require.Equal(t, v1Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + + trigger, err := r.shouldTriggerRollingUpdate(dgd) + require.NoError(t, err) + require.False(t, trigger) +} + func TestBridgeState_WorkerSpecChangeUsesNewV2Generation(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { diff --git a/deploy/operator/internal/dynamo/hash.go b/deploy/operator/internal/dynamo/hash.go index 3b71c7be6f79..8ad6ac6b2111 100644 --- a/deploy/operator/internal/dynamo/hash.go +++ b/deploy/operator/internal/dynamo/hash.go @@ -23,11 +23,23 @@ import ( "encoding/json" "fmt" + "github.com/ai-dynamo/dynamo/deploy/operator/api/v1alpha1" "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" ) const dgdWorkerHashPlaceholderValue = "worker-hash-placeholder" +// ComputeLegacyAlphaDGDWorkersSpecHash returns the frozen v1alpha1 worker hash. +// New and stable generations must not use it; it exists only so an in-progress +// literal "legacy" migration can finish on the target selected by Dynamo 1.2. +func ComputeLegacyAlphaDGDWorkersSpecHash(dgd *v1beta1.DynamoGraphDeployment) (string, error) { + alpha := &v1alpha1.DynamoGraphDeployment{} + if err := alpha.ConvertFrom(dgd); err != nil { + return "", err + } + return v1alpha1.ComputeDGDWorkersSpecHash(alpha) +} + // ComputeDGDWorkersSpecHash computes the v2 worker hash from the worker DCDs // generated by the same DGD-to-DCD path used by reconciliation. The active // worker hash is replaced by a fixed placeholder before generation so the hash diff --git a/deploy/operator/internal/dynamo/hash_test.go b/deploy/operator/internal/dynamo/hash_test.go index fc2d2812b048..db835638885c 100644 --- a/deploy/operator/internal/dynamo/hash_test.go +++ b/deploy/operator/internal/dynamo/hash_test.go @@ -66,6 +66,72 @@ func TestComputeBetaDGDWorkersSpecHash_Deterministic(t *testing.T) { assert.Len(t, h1, 8) } +func TestComputeLegacyAlphaDGDWorkersSpecHash_MatchesV1Alpha1Hash(t *testing.T) { + alpha := baseDGD(map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, + Resources: &v1alpha1.Resources{ + Requests: &v1alpha1.ResourceItem{CPU: "1", Memory: "1Gi"}, + }, + Labels: map[string]string{"resource-label": "ignored-by-legacy-hash"}, + Annotations: map[string]string{"resource-annotation": "ignored-by-legacy-hash"}, + }, + }) + alpha.Annotations = map[string]string{"nvidia.com/current-worker-hash": "old-alpha-hash"} + beta := &v1beta1.DynamoGraphDeployment{} + assert.NoError(t, alpha.ConvertTo(beta)) + + legacyHash, err := ComputeLegacyAlphaDGDWorkersSpecHash(beta) + assert.NoError(t, err) + expectedLegacyHash, err := v1alpha1.ComputeDGDWorkersSpecHash(alpha) + assert.NoError(t, err) + assert.Equal(t, expectedLegacyHash, legacyHash) + assert.NotEqual(t, mustComputeBetaDGDWorkersSpecHash(t, beta), legacyHash) +} + +func TestComputeLegacyAlphaDGDWorkersSpecHash_RecoversNameOnlyMainContainerHash(t *testing.T) { + alpha := baseDGD(map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + ExtraPodSpec: &v1alpha1.ExtraPodSpec{ + MainContainer: &corev1.Container{Name: commonconsts.MainContainerName}, + }, + }, + }) + directAlphaHash, err := v1alpha1.ComputeDGDWorkersSpecHash(alpha) + assert.NoError(t, err) + assert.Equal(t, "0c322ce0", directAlphaHash) + + beta := &v1beta1.DynamoGraphDeployment{} + assert.NoError(t, alpha.ConvertTo(beta)) + recomputedHash, err := ComputeLegacyAlphaDGDWorkersSpecHash(beta) + assert.NoError(t, err) + + assert.Equal(t, directAlphaHash, recomputedHash) +} + +func TestComputeLegacyAlphaDGDWorkersSpecHash_RecoversMultipleCompilationCacheVolumeMounts(t *testing.T) { + alpha := baseDGD(map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + VolumeMounts: []v1alpha1.VolumeMount{ + {Name: "model-cache", MountPoint: "/models", UseAsCompilationCache: true}, + {Name: "compile-cache", MountPoint: "/compile", UseAsCompilationCache: true}, + }, + }, + }) + directAlphaHash, err := v1alpha1.ComputeDGDWorkersSpecHash(alpha) + assert.NoError(t, err) + + beta := &v1beta1.DynamoGraphDeployment{} + assert.NoError(t, alpha.ConvertTo(beta)) + recomputedHash, err := ComputeLegacyAlphaDGDWorkersSpecHash(beta) + assert.NoError(t, err) + + assert.Equal(t, directAlphaHash, recomputedHash) +} + func TestComputeBetaDGDWorkersSpecHash_IgnoresNonWorkers(t *testing.T) { withFrontend := baseDGD(map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": {ComponentType: commonconsts.ComponentTypeWorker}, From 4600c1d33e563debefa006c23b8073050d754a79 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 Jul 2026 13:51:22 +0200 Subject: [PATCH 03/10] fixup! operator/worker-hash: converge generations lazily to v2 Signed-off-by: Dr. Stefan Schimanski --- .../operator/api/v1alpha1/shared_spec_conversion_bugs_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/deploy/operator/api/v1alpha1/shared_spec_conversion_bugs_test.go b/deploy/operator/api/v1alpha1/shared_spec_conversion_bugs_test.go index 555351060a1f..7d8a7b7ed2d2 100644 --- a/deploy/operator/api/v1alpha1/shared_spec_conversion_bugs_test.go +++ b/deploy/operator/api/v1alpha1/shared_spec_conversion_bugs_test.go @@ -251,6 +251,7 @@ func TestBugDGD_SpokeMainContainerNameOnlyRoundTrips(t *testing.T) { if err != nil { t.Fatalf("ComputeDGDWorkersSpecHash(in) error = %v", err) } + hub := &v1beta1.DynamoGraphDeployment{} if err := in.ConvertTo(hub); err != nil { t.Fatalf("ConvertTo() error = %v", err) @@ -295,6 +296,7 @@ func TestBugDGD_SpokeMultipleCompilationCacheVolumeMountsRoundTrip(t *testing.T) if err != nil { t.Fatalf("ComputeDGDWorkersSpecHash(in) error = %v", err) } + hub := &v1beta1.DynamoGraphDeployment{} if err := in.ConvertTo(hub); err != nil { t.Fatalf("ConvertTo() error = %v", err) From add4bf12d838c41fc8538c72e0b5bd0b65fea483 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 Jul 2026 14:43:20 +0200 Subject: [PATCH 04/10] fixup! operator/worker-hash: converge generations lazily to v2 Signed-off-by: Dr. Stefan Schimanski --- deploy/operator/internal/consts/consts.go | 37 +- .../dynamographdeployment_controller.go | 33 +- .../dynamographdeployment_controller_test.go | 8 +- .../dynamographdeployment_rollingupdate.go | 298 ++++++------- ...ynamographdeployment_rollingupdate_test.go | 414 +++++++++--------- .../controller/test_beta_helpers_test.go | 9 + deploy/operator/internal/dynamo/hash.go | 6 +- 7 files changed, 402 insertions(+), 403 deletions(-) diff --git a/deploy/operator/internal/consts/consts.go b/deploy/operator/internal/consts/consts.go index 6ea6c5b082e6..db134cb8ebda 100644 --- a/deploy/operator/internal/consts/consts.go +++ b/deploy/operator/internal/consts/consts.go @@ -75,9 +75,11 @@ const ( KubeAnnotationDynamoBaseModel = "nvidia.com/dynamo-base-model" KubeLabelDynamoDiscoveryBackend = "nvidia.com/dynamo-discovery-backend" KubeLabelDynamoDiscoveryEnabled = "nvidia.com/dynamo-discovery-enabled" - // KubeLabelDynamoWorkerHash is the opaque worker generation identity on - // worker DCDs and pods. Unchanged generations created by Dynamo 1.2 may keep - // their v1 identity; fresh generations and genuine worker updates use v2. + // KubeLabelDynamoWorkerHash is the worker generation label on worker DCDs + // and worker pods. During v1/v2 hash compatibility the label key remains + // stable and the value may be either the active v1 hash or the active v2 hash + // recorded on the parent DGD. Older operators understand only the v1 value, + // so v1-compatible releases continue to generate new DCDs with the v1 value. KubeLabelDynamoWorkerHash = "nvidia.com/dynamo-worker-hash" // CheckpointAutoAnnotation marks operator-created checkpoints whose @@ -240,21 +242,24 @@ const ( // these annotations remain on the previously serving worker generation until // the new generation is fully ready and old workers have drained. // - // The 1.2-to-1.4 compatibility contract converges lazily so an operator - // upgrade never rolls unchanged workers. AnnotationCurrentWorkerHash is the - // opaque identity stamped on the active DCD generation. For a 1.2 generation - // this may still be a v1 hash. AnnotationCurrentWorkerHashV2 records the v2 - // fingerprint of that same generation, allowing the controller to detect real - // worker changes without recomputing the old hash. The next genuine worker - // update creates a v2-identified generation and removes the sidecar annotation. - // A literal "legacy" generation is the one exception: it keeps computing its - // frozen v1 target until the migration already defined by 1.2 completes. - - // AnnotationCurrentWorkerHash stores the active worker generation identity. + // The compatibility contract is intentionally additive: existing annotation + // and label keys keep their old meaning. AnnotationCurrentWorkerHash stores + // the v1alpha1-compatible worker hash so a downgrade can still understand the + // active generation. AnnotationCurrentWorkerHashV2 stores the v2 worker hash + // for the same active generation. A worker DCD whose + // KubeLabelDynamoWorkerHash value matches either annotation is current. While + // v1 compatibility is required, generated worker DCDs use the v1 hash as the + // label value. If a worker change is visible only to v2, the controller + // removes the v1 annotation and rolls to a v2-labeled DCD because the v1 hash + // can no longer prove pod-template compatibility. A future v2-only release + // can start using the v2 value with the same label key and keep accepting the + // v1 annotation until the next v2 generation change drains old workers. + + // AnnotationCurrentWorkerHash stores the active v1alpha1-compatible worker + // generation hash. AnnotationCurrentWorkerHash = "nvidia.com/current-worker-hash" - // AnnotationCurrentWorkerHashV2 stores the transitional v2 fingerprint for - // an unchanged 1.2 worker generation. + // AnnotationCurrentWorkerHashV2 stores the active v2 worker generation hash. AnnotationCurrentWorkerHashV2 = "nvidia.com/current-worker-hash-v2" // LegacyWorkerHash is a sentinel value used during migration from pre-rolling-update diff --git a/deploy/operator/internal/controller/dynamographdeployment_controller.go b/deploy/operator/internal/controller/dynamographdeployment_controller.go index bb32496333e4..0f783616a96c 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_controller.go +++ b/deploy/operator/internal/controller/dynamographdeployment_controller.go @@ -231,15 +231,15 @@ func (r *DynamoGraphDeploymentReconciler) Reconcile(ctx context.Context, req ctr } } } else { - if r.currentWorkerState(dynamoDeployment).empty() { - target, err := r.desiredWorkerGeneration(dynamoDeployment) + if r.currentWorkerHashes(dynamoDeployment).empty() { + hashes, err := r.desiredWorkerHashes(dynamoDeployment) if err != nil { logger.Error(err, "Failed to compute worker hash for unsupported pathway") reason = reasonFailedToInitializeWorkerHash message = Message(err.Error()) return ctrl.Result{}, err } - r.setCurrentWorkerState(dynamoDeployment, workerStateForCompletedGeneration(target.generation, target)) + r.setCurrentWorkerHashes(dynamoDeployment, workerHashesForCompletedGeneration(hashes.v2, hashes)) if updateErr := r.Update(ctx, dynamoDeployment); updateErr != nil { logger.Error(updateErr, "Failed to initialize worker hash for unsupported pathway") reason = reasonFailedToInitializeWorkerHash @@ -264,10 +264,11 @@ func (r *DynamoGraphDeploymentReconciler) Reconcile(ctx context.Context, req ctr r.Recorder.Event(dynamoDeployment, corev1.EventTypeWarning, "RollingUpdateNotSupported", "Worker spec changed but custom rolling updates are not supported for Grove/multinode deployments") - // Update the hash to prevent repeated warnings. Unchanged 1.2 bridge - // state compares equal through current-worker-hash-v2. A literal legacy - // migration retains the 1.2 v1 target and its v2 fingerprint. - target, err := r.desiredWorkerGeneration(dynamoDeployment) + // Update the hash to prevent repeated warnings. If the unsupported + // path is processing a v2-only worker change, preserve the migrated + // v2-only state instead of resurrecting the downgrade-compatible v1 + // annotation for pod contents it no longer represents. + hashes, err := r.desiredWorkerHashes(dynamoDeployment) if err != nil { logger.Error(err, "Failed to compute worker hash for unsupported pathway") state = nvidiacomv1beta1.DGDStateFailed @@ -275,7 +276,7 @@ func (r *DynamoGraphDeploymentReconciler) Reconcile(ctx context.Context, req ctr message = Message(err.Error()) return ctrl.Result{}, err } - r.setCurrentWorkerState(dynamoDeployment, workerStateForCompletedGeneration(target.generation, target)) + r.setCurrentWorkerHashes(dynamoDeployment, r.workerHashesForUnsupportedPathway(dynamoDeployment, hashes)) if updateErr := r.Update(ctx, dynamoDeployment); updateErr != nil { logger.Error(updateErr, "Failed to update worker hash for unsupported pathway") } @@ -1451,19 +1452,19 @@ func (r *DynamoGraphDeploymentReconciler) computeRestartStatus(ctx context.Conte // checkComponentFullyUpdated checks if a DynamoComponentDeployment is fully updated. func (r *DynamoGraphDeploymentReconciler) checkComponentFullyUpdated(ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment, componentName string) (bool, string) { - if r.currentWorkerState(dgd).empty() { + if r.currentWorkerHashes(dgd).empty() { resourceName := dynamo.GetDCDResourceName(dgd, componentName, "") return checkDCDReady(ctx, r.Client, resourceName, dgd.Namespace) } - target, err := r.desiredWorkerGeneration(dgd) + hashes, err := r.desiredWorkerHashes(dgd) if err != nil { return false, err.Error() } var lastReason string - for _, candidate := range r.activeWorkerHashCandidates(dgd, target) { - resourceName := dynamo.GetDCDResourceName(dgd, componentName, candidate) + for _, hash := range r.activeWorkerHashCandidates(dgd, hashes) { + resourceName := dynamo.GetDCDResourceName(dgd, componentName, hash) ready, reason := checkDCDReady(ctx, r.Client, resourceName, dgd.Namespace) if ready || reason != "resource not found" { return ready, reason @@ -1769,11 +1770,11 @@ func (r *DynamoGraphDeploymentReconciler) preserveExistingDCDBackendFramework(ct func (r *DynamoGraphDeploymentReconciler) getExistingRestartAnnotationsDCD(ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment) (map[string]string, error) { logger := log.FromContext(ctx) - target, err := r.desiredWorkerGeneration(dgd) + hashes, err := r.desiredWorkerHashes(dgd) if err != nil { return nil, err } - workerHashes := r.activeWorkerHashCandidates(dgd, target) + workerHashes := r.activeWorkerHashCandidates(dgd, hashes) restartAnnotations := make(map[string]string) for i := range dgd.Spec.Components { @@ -2336,11 +2337,11 @@ func (r *DynamoGraphDeploymentReconciler) checkpointWorkerHashForComponent(dgd * if r == nil { return "", nil } - target, err := r.desiredWorkerGeneration(dgd) + desired, err := r.desiredWorkerHashes(dgd) if err != nil { return "", err } - return activeWorkerHashForDCDGeneration(target), nil + return r.activeWorkerHashForDCDGeneration(dgd, desired), nil } // buildCheckpointJobPodTemplate builds a checkpoint job template from the same diff --git a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go index 9c8154635b6e..dc45e060d664 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go @@ -2106,17 +2106,17 @@ func TestDynamoGraphDeploymentReconciler_checkpointWorkerHashForComponentUsesAct }, }, }) - target, err := reconciler.desiredWorkerGeneration(dgd) + reconciler.setCurrentWorkerHashes(dgd, workerGenerationHashes{v1: "oldhash"}) + desired, err := reconciler.desiredWorkerHashes(dgd) if err != nil { - t.Fatalf("desiredWorkerGeneration() error = %v", err) + t.Fatalf("desiredWorkerHashes() error = %v", err) } - reconciler.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: "oldhash"}) workerHash, err := reconciler.checkpointWorkerHashForComponent(dgd, "worker") if err != nil { t.Fatalf("checkpointWorkerHashForComponent() error = %v", err) } - want := activeWorkerHashForDCDGeneration(target) + want := reconciler.activeWorkerHashForDCDGeneration(dgd, desired) if workerHash != want { t.Fatalf("checkpoint worker hash = %s, want active generated hash %s", workerHash, want) } diff --git a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go index 9c6dfe4675bc..211c8667cebc 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go +++ b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go @@ -36,145 +36,130 @@ import ( "github.com/ai-dynamo/dynamo/deploy/operator/internal/dynamo" ) -// workerGenerationState separates the identity stamped on the active DCD -// generation from the v2 fingerprint of its rendered worker spec. Dynamo 1.2 -// stored a v1 generation identity in current-worker-hash and the corresponding -// v2 fingerprint in current-worker-hash-v2. Keeping both values until a real -// worker change lets normal 1.4 reconciliation stop computing v1 hashes without -// rolling unchanged workers merely to rename their generation. The literal -// legacy migration remains the sole exception until it reaches its 1.2 target. -type workerGenerationState struct { - activeGeneration string - v2Fingerprint string +type workerGenerationHashes struct { + v1 string + v2 string } -// workerGenerationTarget is the generation this reconcile must create or -// continue, plus the v2 fingerprint that determines whether the worker spec -// changed. They differ while retaining a 1.2 v1-named generation, including -// while finishing a literal legacy migration on its original v1 target. -type workerGenerationTarget struct { - generation string - v2Fingerprint string +func (h workerGenerationHashes) empty() bool { + return h.v1 == "" && h.v2 == "" } -func (s workerGenerationState) empty() bool { - return s.activeGeneration == "" && s.v2Fingerprint == "" +func (h workerGenerationHashes) contains(hash string) bool { + if hash == "" { + return false + } + return hash == h.v1 || hash == h.v2 } -// activeHash returns the identity used by the serving DCD generation. A 1.2 -// v2-only generation stored its identity only in current-worker-hash-v2, so use -// that value when the canonical annotation is empty. -func (s workerGenerationState) activeHash() string { - if s.activeGeneration != "" { - return s.activeGeneration +func (r *DynamoGraphDeploymentReconciler) desiredWorkerHashes( + dgd *nvidiacomv1beta1.DynamoGraphDeployment, +) (workerGenerationHashes, error) { + v2Hash, err := dynamo.ComputeDGDWorkersSpecHash(dgd) + if err != nil { + return workerGenerationHashes{}, fmt.Errorf("failed to compute v2 worker hash: %w", err) } - return s.v2Fingerprint -} -func (s workerGenerationState) matchesDesired(desired string) bool { - if s.empty() { - return true + current := r.currentWorkerHashes(dgd) + v1Hash := v2Hash + if current.v2 != "" { + v1Hash = current.v1 } - if s.v2Fingerprint != "" { - return s.v2Fingerprint == desired + if current.v2 == "" && current.v1 != "" && current.v1 != v2Hash { + v1Hash, err = dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + if err != nil { + return workerGenerationHashes{}, fmt.Errorf("failed to compute v1 worker hash: %w", err) + } } - return s.activeGeneration == desired + + return workerGenerationHashes{v1: v1Hash, v2: v2Hash}, nil } -func (r *DynamoGraphDeploymentReconciler) desiredWorkerHash( +func (r *DynamoGraphDeploymentReconciler) currentWorkerHashes( dgd *nvidiacomv1beta1.DynamoGraphDeployment, -) (string, error) { - hash, err := dynamo.ComputeDGDWorkersSpecHash(dgd) - if err != nil { - return "", fmt.Errorf("failed to compute worker hash: %w", err) +) workerGenerationHashes { + return workerGenerationHashes{ + v1: r.getCurrentWorkerHash(dgd), + v2: r.getCurrentWorkerHashV2(dgd), } - return hash, nil } -func (r *DynamoGraphDeploymentReconciler) desiredWorkerGeneration( - dgd *nvidiacomv1beta1.DynamoGraphDeployment, -) (workerGenerationTarget, error) { - v2Fingerprint, err := r.desiredWorkerHash(dgd) - if err != nil { - return workerGenerationTarget{}, err +func currentWorkerHashesMatchDesired(current, desired workerGenerationHashes) bool { + if current.empty() { + return true } - - current := r.currentWorkerState(dgd) - if current.activeGeneration == consts.LegacyWorkerHash { - generation, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - if err != nil { - return workerGenerationTarget{}, fmt.Errorf("failed to compute in-progress legacy migration target: %w", err) + if current.v1 != "" { + if current.v1 != desired.v1 { + return false } - return workerGenerationTarget{generation: generation, v2Fingerprint: v2Fingerprint}, nil + return current.v2 == "" || current.v2 == desired.v2 } - - return workerGenerationTarget{ - generation: workerHashForDCDGeneration(current, v2Fingerprint), - v2Fingerprint: v2Fingerprint, - }, nil + return current.v2 == desired.v2 } -func (r *DynamoGraphDeploymentReconciler) currentWorkerState( - dgd *nvidiacomv1beta1.DynamoGraphDeployment, -) workerGenerationState { - return workerGenerationState{ - activeGeneration: r.getCurrentWorkerHash(dgd), - v2Fingerprint: r.getCurrentWorkerHashV2(dgd), +func workerHashForDCDGeneration(current, desired workerGenerationHashes) string { + if current.v1 != "" { + if current.v1 == desired.v1 { + if current.v2 == "" || current.v2 == desired.v2 { + return desired.v1 + } + return desired.v2 + } + return desired.v1 } + if current.v2 != "" { + return desired.v2 + } + return desired.v1 } -func workerHashForDCDGeneration(current workerGenerationState, desired string) string { - if current.matchesDesired(desired) && current.activeHash() != "" { - return current.activeHash() +func workerHashesForCompletedGeneration(newWorkerHash string, desired workerGenerationHashes) workerGenerationHashes { + if newWorkerHash == desired.v2 { + return workerGenerationHashes{v1: desired.v2} } return desired } -func workerStateForCompletedGeneration(newWorkerHash string, target workerGenerationTarget) workerGenerationState { - if newWorkerHash == target.v2Fingerprint { - return workerGenerationState{activeGeneration: newWorkerHash} - } - return workerGenerationState{ - activeGeneration: newWorkerHash, - v2Fingerprint: target.v2Fingerprint, - } +func (r *DynamoGraphDeploymentReconciler) workerHashesForUnsupportedPathway( + dgd *nvidiacomv1beta1.DynamoGraphDeployment, + desired workerGenerationHashes, +) workerGenerationHashes { + newWorkerHash := r.activeWorkerHashForDCDGeneration(dgd, desired) + return workerHashesForCompletedGeneration(newWorkerHash, desired) } // shouldTriggerRollingUpdate compares desired worker hashes with the active // generation recorded on the DGD. // -// For a 1.2 bridge state, current-worker-hash-v2 is the v2 fingerprint of the -// active v1-named generation. Comparing the desired hash to that fingerprint -// avoids an upgrade-induced rollout while still detecting real worker changes. +// During v1/v2 compatibility a worker DCD is current if its worker-hash label +// matches either current-worker-hash (v1) or current-worker-hash-v2. This keeps +// the existing annotation/label meaning downgrade-safe while allowing the +// controller to record the v2 hash that will become primary later. func (r *DynamoGraphDeploymentReconciler) shouldTriggerRollingUpdate( dgd *nvidiacomv1beta1.DynamoGraphDeployment, ) (bool, error) { - current := r.currentWorkerState(dgd) - if current.empty() { - return false, nil - } - - target, err := r.desiredWorkerGeneration(dgd) + desired, err := r.desiredWorkerHashes(dgd) if err != nil { return false, err } - return current.activeHash() != target.generation, nil + current := r.currentWorkerHashes(dgd) + return !currentWorkerHashesMatchDesired(current, desired), nil } // initializeWorkerHashIfNeeded establishes the DGD's active worker generation. -// New DGDs use the v2 hash directly. DGDs created before managed rolling updates -// may already have worker DCDs without a hash label; in that case we label those -// DCDs with the legacy sentinel and let the normal rolling update path finish -// the v1 target selected by 1.2. That target then becomes a normal v1/v2 bridge. +// New DGDs store the current v1 and v2 worker hashes immediately. DGDs created before +// managed rolling updates may already have worker DCDs without a hash label; in +// that case we label those DCDs with the legacy sentinel and let the normal +// rolling update path migrate from that sentinel to the desired compatibility hash. func (r *DynamoGraphDeploymentReconciler) initializeWorkerHashIfNeeded( ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment, ) error { logger := log.FromContext(ctx) - if !r.currentWorkerState(dgd).empty() { - return nil + if !r.currentWorkerHashes(dgd).empty() { + return r.migrateCurrentWorkerHashIfNeeded(ctx, dgd) } // Check for legacy (pre-rolling-update) worker DCDs @@ -213,76 +198,93 @@ func (r *DynamoGraphDeploymentReconciler) initializeWorkerHashIfNeeded( return nil } - // Normal first deploy — use the v2 hash as the canonical generation identity. - hash, err := r.desiredWorkerHash(dgd) + // Normal first deploy — set the actual computed compatibility hashes + hashes, err := r.desiredWorkerHashes(dgd) if err != nil { return err } - r.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: hash}) + r.setCurrentWorkerHashes(dgd, workerHashesForCompletedGeneration(hashes.v2, hashes)) if err := r.Update(ctx, dgd); err != nil { return fmt.Errorf("failed to initialize worker hash: %w", err) } - logger.Info("Initialized current worker hash", "hash", hash) + logger.Info("Initialized current worker hashes", "v1Hash", hashes.v1, "v2Hash", hashes.v2) return nil } -// migrateCurrentWorkerHashIfNeeded canonicalizes a v2-only state written by -// Dynamo 1.2. It is safe only when the sidecar fingerprint still matches the -// desired v2 hash; otherwise the object is transitioning to a newer generation -// and the normal rollout path must retain the old state until completion. +// migrateCurrentWorkerHashIfNeeded completes partially persisted 1.2 hash state +// without rolling workers. func (r *DynamoGraphDeploymentReconciler) migrateCurrentWorkerHashIfNeeded( ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment, ) error { logger := log.FromContext(ctx) - current := r.currentWorkerState(dgd) - if current.activeGeneration != "" || current.v2Fingerprint == "" { + current := r.currentWorkerHashes(dgd) + if current.empty() || current.v1 == consts.LegacyWorkerHash { return nil } - desired, err := r.desiredWorkerHash(dgd) + desired, err := r.desiredWorkerHashes(dgd) if err != nil { return err } - if current.v2Fingerprint != desired { + + var next workerGenerationHashes + switch { + case current.v1 == "" && current.v2 == desired.v2: + next = workerGenerationHashes{v1: current.v2} + case current.v1 != "" && current.v2 == "" && current.v1 != desired.v2: + v1Hash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + if err != nil { + return fmt.Errorf("failed to compute v1 worker hash: %w", err) + } + if current.v1 != v1Hash { + return nil + } + next = workerGenerationHashes{v1: current.v1, v2: desired.v2} + default: return nil } - r.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: current.v2Fingerprint}) + r.setCurrentWorkerHashes(dgd, next) if err := r.Update(ctx, dgd); err != nil { return fmt.Errorf("failed to migrate worker hash annotations: %w", err) } - logger.Info("Promoted v2 worker hash to canonical annotation", "hash", desired) + logger.Info("Migrated worker hash annotations", "v1Hash", next.v1, "v2Hash", next.v2) r.Recorder.Event(dgd, corev1.EventTypeNormal, "WorkerHashMigrated", - "Promoted the existing v2 worker hash to the canonical annotation without rolling workers") + "Completed worker hash migration without rolling workers") return nil } // activeWorkerHashForDCDGeneration returns the hash used for generated worker -// DCD names and worker-hash labels in this reconcile. An unchanged 1.2 bridge -// state keeps its v1 generation identity; a literal legacy migration keeps its -// 1.2 v1 target; every later real change uses v2 and converges to one hash. -func activeWorkerHashForDCDGeneration( - target workerGenerationTarget, +// DCD names and worker-hash labels in this reconcile. While v1 compatibility is +// required, new DCDs continue to use the v1 hash. If the active generation is +// already v2-labeled, preserve that value so future v2-only transitions do not +// create a v1-labeled replacement. +func (r *DynamoGraphDeploymentReconciler) activeWorkerHashForDCDGeneration( + dgd *nvidiacomv1beta1.DynamoGraphDeployment, + desired workerGenerationHashes, ) string { - return target.generation + return r.activeWorkerHashCandidates(dgd, desired)[0] } func (r *DynamoGraphDeploymentReconciler) activeWorkerHashCandidates( dgd *nvidiacomv1beta1.DynamoGraphDeployment, - target workerGenerationTarget, + desired workerGenerationHashes, ) []string { - current := r.currentWorkerState(dgd) + current := r.currentWorkerHashes(dgd) candidates := make([]string, 0, 2) - generated := target.generation + generated := workerHashForDCDGeneration(current, desired) candidates = append(candidates, generated) - if current.v2Fingerprint == target.v2Fingerprint && target.v2Fingerprint != generated { - candidates = append(candidates, target.v2Fingerprint) + if current.v1 == desired.v1 && (current.v2 == "" || current.v2 == desired.v2) && desired.v1 != generated { + candidates = append(candidates, desired.v1) + } + if current.contains(desired.v2) && desired.v2 != generated && desired.v2 != desired.v1 { + candidates = append(candidates, desired.v2) } return candidates } @@ -351,23 +353,23 @@ func (r *DynamoGraphDeploymentReconciler) getCurrentWorkerHashV2( return dgd.Annotations[consts.AnnotationCurrentWorkerHashV2] } -// setCurrentWorkerState stores the active generation identity and its optional -// 1.2 bridge fingerprint. Fresh and converged generations use only the canonical -// annotation; the sidecar is retained solely for unchanged v1-named generations. -func (r *DynamoGraphDeploymentReconciler) setCurrentWorkerState( +// setCurrentWorkerHashes stores the active worker hashes for one generation. +// Empty fields are deleted, which is how v2-only generations intentionally drop +// the downgrade-compatible v1 annotation. +func (r *DynamoGraphDeploymentReconciler) setCurrentWorkerHashes( dgd *nvidiacomv1beta1.DynamoGraphDeployment, - state workerGenerationState, + hashes workerGenerationHashes, ) { if dgd.Annotations == nil { dgd.Annotations = make(map[string]string) } - if state.activeGeneration != "" { - dgd.Annotations[consts.AnnotationCurrentWorkerHash] = state.activeGeneration + if hashes.v1 != "" { + dgd.Annotations[consts.AnnotationCurrentWorkerHash] = hashes.v1 } else { delete(dgd.Annotations, consts.AnnotationCurrentWorkerHash) } - if state.v2Fingerprint != "" { - dgd.Annotations[consts.AnnotationCurrentWorkerHashV2] = state.v2Fingerprint + if hashes.v2 != "" { + dgd.Annotations[consts.AnnotationCurrentWorkerHashV2] = hashes.v2 } else { delete(dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) } @@ -419,37 +421,38 @@ func (r *DynamoGraphDeploymentReconciler) reconcileRollingUpdate( rollingUpdateStatus := r.getOrCreateRollingUpdateStatus(dgd) - target, err := r.desiredWorkerGeneration(dgd) + desired, err := r.desiredWorkerHashes(dgd) if err != nil { return err } - newWorkerHash := activeWorkerHashForDCDGeneration(target) - current := r.currentWorkerState(dgd) + newWorkerHash := r.activeWorkerHashForDCDGeneration(dgd, desired) + current := r.currentWorkerHashes(dgd) logger.Info("Reconciling rolling update", "phase", rollingUpdateStatus.Phase, - "activeWorkerGeneration", current.activeHash(), - "currentV2Fingerprint", current.v2Fingerprint, + "currentV1WorkerHash", current.v1, + "currentV2WorkerHash", current.v2, "newWorkerHash", newWorkerHash, - "desiredWorkerHash", target.v2Fingerprint) + "desiredV1WorkerHash", desired.v1, + "desiredV2WorkerHash", desired.v2) - if rollingUpdateStatus.Phase == nvidiacomv1beta1.RollingUpdatePhaseCompleted && current.activeHash() != newWorkerHash { + if rollingUpdateStatus.Phase == nvidiacomv1beta1.RollingUpdatePhaseCompleted && !current.contains(newWorkerHash) { // Check if DCDs with the new hash already exist and are serving. // If so, this is just a stale annotation — update it without starting a new rollout. newInfo, err := r.getWorkerInfoForWorkerHash(ctx, dgd, newWorkerHash) oldInfo, oldErr := r.getOldWorkerInfo(ctx, dgd, newWorkerHash) if err == nil && oldErr == nil && workerGenerationComplete(dgd, oldInfo, newInfo) { logger.Info("Updating stale worker hash annotation", - "activeWorkerGeneration", current.activeHash(), - "currentV2Fingerprint", current.v2Fingerprint, + "currentV1WorkerHash", current.v1, + "currentV2WorkerHash", current.v2, "newHash", newWorkerHash) - r.setCurrentWorkerState(dgd, workerStateForCompletedGeneration(newWorkerHash, target)) + r.setCurrentWorkerHashes(dgd, workerHashesForCompletedGeneration(newWorkerHash, desired)) return r.Update(ctx, dgd) } // New spec change: reset to start a proper rolling update cycle with surge/drain. logger.Info("New worker spec change detected, starting new rolling update cycle", - "activeWorkerGeneration", current.activeHash(), - "currentV2Fingerprint", current.v2Fingerprint, + "currentV1WorkerHash", current.v1, + "currentV2WorkerHash", current.v2, "newHash", newWorkerHash, "previousPhase", rollingUpdateStatus.Phase) rollingUpdateStatus.Phase = nvidiacomv1beta1.RollingUpdatePhaseNone @@ -458,7 +461,7 @@ func (r *DynamoGraphDeploymentReconciler) reconcileRollingUpdate( rollingUpdateStatus.UpdatedComponents = nil } - if current.activeHash() == newWorkerHash && + if current.contains(newWorkerHash) && rollingUpdateStatus.Phase == nvidiacomv1beta1.RollingUpdatePhaseInProgress { logger.Info("Detected stuck rolling update: hashes match but phase is InProgress", "hash", newWorkerHash, @@ -493,11 +496,11 @@ func (r *DynamoGraphDeploymentReconciler) startRollingUpdate( ) error { logger := log.FromContext(ctx) - current := r.currentWorkerState(dgd) + current := r.currentWorkerHashes(dgd) logger.Info("Starting rolling update", - "activeWorkerGeneration", current.activeHash(), - "currentV2Fingerprint", current.v2Fingerprint, + "currentV1Hash", current.v1, + "currentV2Hash", current.v2, "newHash", newWorkerHash) now := metav1.Now() @@ -610,7 +613,8 @@ func (r *DynamoGraphDeploymentReconciler) completeRollingUpdate( newWorkerHash string, ) error { logger := log.FromContext(ctx) - target, err := r.desiredWorkerGeneration(dgd) + + desired, err := r.desiredWorkerHashes(dgd) if err != nil { return err } @@ -620,7 +624,7 @@ func (r *DynamoGraphDeploymentReconciler) completeRollingUpdate( return fmt.Errorf("failed to delete old worker DCDs: %w", err) } - r.setCurrentWorkerState(dgd, workerStateForCompletedGeneration(newWorkerHash, target)) + r.setCurrentWorkerHashes(dgd, workerHashesForCompletedGeneration(newWorkerHash, desired)) if err := r.Update(ctx, dgd); err != nil { return fmt.Errorf("failed to update current worker hash: %w", err) } @@ -1123,14 +1127,14 @@ func (r *DynamoGraphDeploymentReconciler) buildRollingUpdateContext( ) (dynamo.RollingUpdateContext, error) { logger := log.FromContext(ctx) - target, err := r.desiredWorkerGeneration(dgd) + desiredHashes, err := r.desiredWorkerHashes(dgd) if err != nil { return dynamo.RollingUpdateContext{}, err } - newWorkerHash := activeWorkerHashForDCDGeneration(target) - current := r.currentWorkerState(dgd) + newWorkerHash := r.activeWorkerHashForDCDGeneration(dgd, desiredHashes) + currentHashes := r.currentWorkerHashes(dgd) - if current.activeHash() == newWorkerHash { + if currentHashes.contains(newWorkerHash) { return dynamo.RollingUpdateContext{ NewWorkerHash: newWorkerHash, OldWorkerReplicaTargetsByComponent: make(map[string]int32), diff --git a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go index 8095a50af443..bb63e90662e0 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go @@ -107,11 +107,10 @@ func createTestReconcilerWithStatus(dgd *nvidiacomv1beta1.DynamoGraphDeployment, func TestShouldTriggerRollingUpdate(t *testing.T) { tests := []struct { - name string - services map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec - currentState string - changeDesired bool - expected bool + name string + services map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec + existingHash string // empty means no annotation, "compute" means compute from services + expected bool }{ { name: "new deployment - no hash annotation", @@ -121,21 +120,22 @@ func TestShouldTriggerRollingUpdate(t *testing.T) { Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, }, }, - expected: false, + existingHash: "", + expected: false, }, { - name: "canonical v2 hash unchanged", + name: "hash unchanged - matches current spec", services: map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, }, }, - currentState: "canonical", + existingHash: "compute", expected: false, }, { - name: "1.2 bridge fingerprint unchanged", + name: "unversioned legacy alpha hash - compatible migration does not trigger rollout", services: map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, @@ -145,32 +145,19 @@ func TestShouldTriggerRollingUpdate(t *testing.T) { }, }, }, - currentState: "bridge", + existingHash: "legacy-compute", expected: false, }, { - name: "canonical hash changed", + name: "hash changed - differs from current spec", services: map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "FOO", Value: "new-value"}}, }, }, - currentState: "canonical", - changeDesired: true, - expected: true, - }, - { - name: "1.2 bridge fingerprint changed", - services: map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: consts.ComponentTypeWorker, - Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, - }, - }, - currentState: "bridge", - changeDesired: true, - expected: true, + existingHash: "old-hash-12345678", + expected: true, }, { name: "frontend-only change - hash unchanged", @@ -184,7 +171,7 @@ func TestShouldTriggerRollingUpdate(t *testing.T) { Envs: []corev1.EnvVar{{Name: "WORKER_VAR", Value: "unchanged"}}, }, }, - currentState: "bridge", + existingHash: "compute", expected: false, }, } @@ -193,23 +180,20 @@ func TestShouldTriggerRollingUpdate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { dgd := createTestDGD("test-dgd", tt.services) - desired := betaDGDWorkersSpecHash(t, dgd) - switch tt.currentState { - case "canonical": + if tt.existingHash == "compute" { + hash := legacyDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: desired, + consts.AnnotationCurrentWorkerHash: hash, + consts.AnnotationCurrentWorkerHashV2: betaDGDWorkersSpecHash(t, dgd), } - case "bridge": + } else if tt.existingHash == "legacy-compute" { + hash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + require.NoError(t, err) dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: "v1hash00", - consts.AnnotationCurrentWorkerHashV2: desired, + consts.AnnotationCurrentWorkerHash: hash, } - } - if tt.changeDesired { - dgd.Spec.Components[0].PodTemplate.Spec.Containers[0].Env = append( - dgd.Spec.Components[0].PodTemplate.Spec.Containers[0].Env, - corev1.EnvVar{Name: "CHANGED", Value: "true"}, - ) + } else if tt.existingHash != "" { + dgd.Annotations = map[string]string{consts.AnnotationCurrentWorkerHash: tt.existingHash} } r := createTestReconcilerWithStatus(dgd) @@ -230,18 +214,20 @@ func TestShouldTriggerRollingUpdate_IgnoresReplicaChanges(t *testing.T) { Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, }, }) + legacyHash := legacyDGDWorkersSpecHash(t, dgd) v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: "v1hash00", + consts.AnnotationCurrentWorkerHash: legacyHash, consts.AnnotationCurrentWorkerHashV2: v2Hash, } dgd.Spec.Components[0].Replicas = ptr.To(int32(10)) r := createTestReconcilerWithStatus(dgd) - desired, err := r.desiredWorkerHash(dgd) + desired, err := r.desiredWorkerHashes(dgd) require.NoError(t, err) - assert.Equal(t, v2Hash, desired) + assert.Equal(t, legacyHash, desired.v1) + assert.Equal(t, v2Hash, desired.v2) trigger, err := r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) @@ -270,9 +256,9 @@ func TestInitializeWorkerHashIfNeeded_FirstDeploy(t *testing.T) { hash := r.getCurrentWorkerHash(dgd) assert.NotEmpty(t, hash, "Hash should be set after initialization") - // Fresh deployments use one canonical v2 hash. - expectedHash := betaDGDWorkersSpecHash(t, dgd) - assert.Equal(t, expectedHash, hash) + // Fresh deployments store one canonical v2 hash. + expectedV2Hash := betaDGDWorkersSpecHash(t, dgd) + assert.Equal(t, expectedV2Hash, hash) assert.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) } @@ -303,165 +289,142 @@ func TestInitializeWorkerHashIfNeeded_AlreadyInitialized(t *testing.T) { assert.Equal(t, existingHash, hash, "Hash should not change when already initialized") } -func TestBridgeState_NoOpUpgradeUsesExistingWorkerGeneration(t *testing.T) { - dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: consts.ComponentTypeWorker, - Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, +func TestInitializeWorkerHashIfNeeded_PreservesLegacyAlphaHash(t *testing.T) { + alpha := &nvidiacomv1alpha1.DynamoGraphDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-dgd", + Namespace: "default", + Annotations: map[string]string{ + consts.AnnotationCurrentWorkerHash: "old-alpha-hash", + }, }, - }) - activeHash := "v1hash00" + Spec: nvidiacomv1alpha1.DynamoGraphDeploymentSpec{ + Services: map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: consts.ComponentTypeWorker, + Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, + Resources: &nvidiacomv1alpha1.Resources{ + Requests: &nvidiacomv1alpha1.ResourceItem{CPU: "1"}, + }, + }, + }, + }, + } + dgd := &nvidiacomv1beta1.DynamoGraphDeployment{} + require.NoError(t, alpha.ConvertTo(dgd)) + legacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + require.NoError(t, err) v2Hash := betaDGDWorkersSpecHash(t, dgd) - dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: activeHash, - consts.AnnotationCurrentWorkerHashV2: v2Hash, + require.NotEqual(t, legacyHash, v2Hash) + if dgd.Annotations == nil { + dgd.Annotations = map[string]string{} } + dgd.Annotations[consts.AnnotationCurrentWorkerHash] = legacyHash r := createTestReconcilerWithStatus(dgd) - require.NoError(t, r.migrateCurrentWorkerHashIfNeeded(context.Background(), dgd)) - require.Equal(t, activeHash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) - - trigger, err := r.shouldTriggerRollingUpdate(dgd) + err = r.initializeWorkerHashIfNeeded(context.Background(), dgd) require.NoError(t, err) - require.False(t, trigger) - rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) + assert.Equal(t, legacyHash, r.getCurrentWorkerHash(dgd)) + assert.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + trigger, err := r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) - require.Equal(t, activeHash, rollingCtx.NewWorkerHash) - require.False(t, rollingCtx.InProgress()) + assert.False(t, trigger) - dcds, err := dynamo.GenerateDynamoComponentsDeployments(dgd, nil, nil, rollingCtx) + ctx, err := r.buildRollingUpdateContext(context.Background(), dgd) require.NoError(t, err) - require.Equal(t, "test-dgd-worker-"+activeHash, dcds["worker"].Name) + assert.Equal(t, legacyHash, ctx.NewWorkerHash) + assert.False(t, ctx.InProgress()) + assert.NotEqual(t, v2Hash, ctx.NewWorkerHash) } -func TestBridgeState_LeftoverLegacyDCDDoesNotTriggerAnotherRollout(t *testing.T) { - dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: consts.ComponentTypeWorker, - Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, - }, - }) - activeHash := "v1hash00" - v2Hash := betaDGDWorkersSpecHash(t, dgd) - dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: activeHash, - consts.AnnotationCurrentWorkerHashV2: v2Hash, - } - legacyDCD := betaDCD(t, &nvidiacomv1alpha1.DynamoComponentDeployment{ +func TestLegacyAlphaHashCompatibility_NoOpUpgradeUsesExistingWorkerGeneration(t *testing.T) { + alpha := &nvidiacomv1alpha1.DynamoGraphDeployment{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-dgd-worker", + Name: "qwen", Namespace: "default", - Labels: map[string]string{ - consts.KubeLabelDynamoGraphDeploymentName: "test-dgd", - consts.KubeLabelDynamoWorkerHash: consts.LegacyWorkerHash, + Annotations: map[string]string{ + consts.AnnotationCurrentWorkerHash: "old-alpha-hash", }, }, - Spec: nvidiacomv1alpha1.DynamoComponentDeploymentSpec{ - DynamoComponentDeploymentSharedSpec: nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ - ComponentType: consts.ComponentTypeWorker, - ServiceName: "worker", + Spec: nvidiacomv1alpha1.DynamoGraphDeploymentSpec{ + BackendFramework: "vllm", + Services: map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ + "VllmDecodeWorker": { + ComponentType: consts.ComponentTypeWorker, + SubComponentType: consts.ComponentTypeDecode, + Envs: []corev1.EnvVar{{Name: "MODEL_PATH", Value: "Qwen/Qwen3-0.6B"}}, + Resources: &nvidiacomv1alpha1.Resources{ + Requests: &nvidiacomv1alpha1.ResourceItem{GPU: "1"}, + }, + }, }, }, - }) - - r := createTestReconcilerWithStatus(dgd, withObjects(legacyDCD)) - trigger, err := r.shouldTriggerRollingUpdate(dgd) - require.NoError(t, err) - require.False(t, trigger) - - rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) - require.NoError(t, err) - require.Equal(t, activeHash, rollingCtx.NewWorkerHash) - require.False(t, rollingCtx.InProgress()) -} - -func TestLegacyMigrationFinishesOnV1TargetWithoutSecondRollout(t *testing.T) { - dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: consts.ComponentTypeWorker, - Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, - }, - }) - v1Hash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + } + dgd := &nvidiacomv1beta1.DynamoGraphDeployment{} + require.NoError(t, alpha.ConvertTo(dgd)) + legacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) require.NoError(t, err) v2Hash := betaDGDWorkersSpecHash(t, dgd) - require.NotEqual(t, v1Hash, v2Hash) - dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: consts.LegacyWorkerHash, - } - dgd.Status.RollingUpdate = &nvidiacomv1beta1.RollingUpdateStatus{ - Phase: nvidiacomv1beta1.RollingUpdatePhaseInProgress, + require.NotEqual(t, legacyHash, v2Hash) + if dgd.Annotations == nil { + dgd.Annotations = map[string]string{} } + dgd.Annotations[consts.AnnotationCurrentWorkerHash] = legacyHash r := createTestReconcilerWithStatus(dgd) - target, err := r.desiredWorkerGeneration(dgd) + require.NoError(t, r.initializeWorkerHashIfNeeded(context.Background(), dgd)) + + trigger, err := r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) - require.Equal(t, v1Hash, target.generation) - require.Equal(t, v2Hash, target.v2Fingerprint) + require.False(t, trigger) rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) require.NoError(t, err) - require.Equal(t, v1Hash, rollingCtx.NewWorkerHash) - - require.NoError(t, r.completeRollingUpdate(context.Background(), dgd, v1Hash)) - require.Equal(t, v1Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + require.Equal(t, legacyHash, rollingCtx.NewWorkerHash) + require.False(t, rollingCtx.InProgress()) - trigger, err := r.shouldTriggerRollingUpdate(dgd) + dcds, err := dynamo.GenerateDynamoComponentsDeployments(dgd, nil, nil, rollingCtx) require.NoError(t, err) - require.False(t, trigger) + require.Equal(t, "qwen-vllmdecodeworker-"+legacyHash, dcds["VllmDecodeWorker"].Name) + require.NotEqual(t, "qwen-vllmdecodeworker-"+v2Hash, dcds["VllmDecodeWorker"].Name) } -func TestBridgeState_StaleInProgressCompletionPreservesFingerprint(t *testing.T) { +func TestLegacyAlphaHashCompatibility_WorkerSpecChangeUsesNewV2Generation(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, + Resources: &nvidiacomv1alpha1.Resources{ + Requests: &nvidiacomv1alpha1.ResourceItem{CPU: "1"}, + }, }, }) - v1Hash := "v1hash00" + legacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + require.NoError(t, err) v2Hash := betaDGDWorkersSpecHash(t, dgd) - dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: v1Hash, - consts.AnnotationCurrentWorkerHashV2: v2Hash, - } - dgd.Status.RollingUpdate = &nvidiacomv1beta1.RollingUpdateStatus{ - Phase: nvidiacomv1beta1.RollingUpdatePhaseInProgress, + require.NotEqual(t, legacyHash, v2Hash) + if dgd.Annotations == nil { + dgd.Annotations = map[string]string{} } + dgd.Annotations[consts.AnnotationCurrentWorkerHash] = legacyHash r := createTestReconcilerWithStatus(dgd) - require.NoError(t, r.reconcileRollingUpdate(context.Background(), dgd)) - require.Equal(t, v1Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.NoError(t, r.initializeWorkerHashIfNeeded(context.Background(), dgd)) + require.Equal(t, legacyHash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) - trigger, err := r.shouldTriggerRollingUpdate(dgd) - require.NoError(t, err) - require.False(t, trigger) -} - -func TestBridgeState_WorkerSpecChangeUsesNewV2Generation(t *testing.T) { - dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: consts.ComponentTypeWorker, - Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, - }, - }) - oldV2Hash := betaDGDWorkersSpecHash(t, dgd) - dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: "v1hash00", - consts.AnnotationCurrentWorkerHashV2: oldV2Hash, - } - dgd.Spec.Components[0].PodTemplate.Spec.Containers[0].Env = append( dgd.Spec.Components[0].PodTemplate.Spec.Containers[0].Env, corev1.EnvVar{Name: "NEW_WORKER_SETTING", Value: "true"}, ) newV2Hash := betaDGDWorkersSpecHash(t, dgd) - require.NotEqual(t, oldV2Hash, newV2Hash) + newLegacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + require.NoError(t, err) + require.NotEqual(t, v2Hash, newV2Hash) + require.NotEqual(t, legacyHash, newLegacyHash) - r := createTestReconcilerWithStatus(dgd) + require.NoError(t, r.migrateCurrentWorkerHashIfNeeded(context.Background(), dgd)) trigger, err := r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) @@ -470,68 +433,53 @@ func TestBridgeState_WorkerSpecChangeUsesNewV2Generation(t *testing.T) { rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) require.NoError(t, err) require.Equal(t, newV2Hash, rollingCtx.NewWorkerHash) - require.True(t, rollingCtx.InProgress()) - - require.NoError(t, r.completeRollingUpdate(context.Background(), dgd, newV2Hash)) - require.Equal(t, newV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) + require.NotEqual(t, newLegacyHash, rollingCtx.NewWorkerHash) } -func TestV2OnlyStatePromotesCanonicalHashWithoutRollout(t *testing.T) { +func TestLegacyAlphaHashCompatibility_V2OnlyChangeUsesNewV2Generation(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, Envs: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, }, }) + dgd.Spec.BackendFramework = "vllm" + legacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + require.NoError(t, err) v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ + consts.AnnotationCurrentWorkerHash: legacyHash, consts.AnnotationCurrentWorkerHashV2: v2Hash, } r := createTestReconcilerWithStatus(dgd) - require.NoError(t, r.migrateCurrentWorkerHashIfNeeded(context.Background(), dgd)) - require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) - - trigger, err := r.shouldTriggerRollingUpdate(dgd) - require.NoError(t, err) - require.False(t, trigger) + dgd.Spec.BackendFramework = "sglang" - rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) + newLegacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) require.NoError(t, err) - require.Equal(t, v2Hash, rollingCtx.NewWorkerHash) - require.False(t, rollingCtx.InProgress()) -} - -func TestV2OnlyTransitionDoesNotPromoteStaleFingerprint(t *testing.T) { - dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ - "worker": { - ComponentType: consts.ComponentTypeWorker, - Envs: []corev1.EnvVar{{Name: "FOO", Value: "new"}}, - }, - }) - dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHashV2: "oldv2hash", - } + newV2Hash := betaDGDWorkersSpecHash(t, dgd) + require.Equal(t, legacyHash, newLegacyHash) + require.NotEqual(t, v2Hash, newV2Hash) - r := createTestReconcilerWithStatus(dgd) require.NoError(t, r.migrateCurrentWorkerHashIfNeeded(context.Background(), dgd)) - require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHash) - require.Equal(t, "oldv2hash", dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + require.Equal(t, legacyHash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) - desired := betaDGDWorkersSpecHash(t, dgd) trigger, err := r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) require.True(t, trigger) rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) require.NoError(t, err) - require.Equal(t, desired, rollingCtx.NewWorkerHash) - require.True(t, rollingCtx.InProgress()) + require.Equal(t, newV2Hash, rollingCtx.NewWorkerHash) + require.NotEqual(t, newLegacyHash, rollingCtx.NewWorkerHash) + + require.NoError(t, r.completeRollingUpdate(context.Background(), dgd, newV2Hash)) + require.Equal(t, newV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) } -func TestUnsupportedPathwayKeepsBridgeUntilRealWorkerChange(t *testing.T) { +func TestUnsupportedPathwayMigratesV1OnlyAndConvergesToCanonicalV2(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, @@ -540,35 +488,46 @@ func TestUnsupportedPathwayKeepsBridgeUntilRealWorkerChange(t *testing.T) { }, }) dgd.Spec.BackendFramework = "vllm" - oldV2Hash := betaDGDWorkersSpecHash(t, dgd) + legacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + require.NoError(t, err) + v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: "v1hash00", - consts.AnnotationCurrentWorkerHashV2: oldV2Hash, + consts.AnnotationCurrentWorkerHash: legacyHash, } r := createTestReconcilerWithStatus(dgd) require.False(t, r.supportsManagedRollingUpdate(dgd)) require.NoError(t, r.migrateCurrentWorkerHashIfNeeded(context.Background(), dgd)) - require.Equal(t, "v1hash00", dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.Equal(t, oldV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + require.Equal(t, legacyHash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) trigger, err := r.shouldTriggerRollingUpdate(dgd) require.NoError(t, err) require.False(t, trigger) dgd.Spec.BackendFramework = "sglang" + + newLegacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + require.NoError(t, err) newV2Hash := betaDGDWorkersSpecHash(t, dgd) - require.NotEqual(t, oldV2Hash, newV2Hash) - trigger, err = r.shouldTriggerRollingUpdate(dgd) + require.Equal(t, legacyHash, newLegacyHash) + require.NotEqual(t, v2Hash, newV2Hash) + + require.NoError(t, r.migrateCurrentWorkerHashIfNeeded(context.Background(), dgd)) + require.Equal(t, legacyHash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + + desired, err := r.desiredWorkerHashes(dgd) require.NoError(t, err) - require.True(t, trigger) + completed := r.workerHashesForUnsupportedPathway(dgd, desired) + require.Equal(t, newV2Hash, completed.v1) + require.Empty(t, completed.v2) - r.setCurrentWorkerState(dgd, workerGenerationState{activeGeneration: newV2Hash}) + r.setCurrentWorkerHashes(dgd, completed) rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) require.NoError(t, err) require.Equal(t, newV2Hash, rollingCtx.NewWorkerHash) - require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) } func TestSupportsManagedRollingUpdate(t *testing.T) { @@ -1540,7 +1499,7 @@ func TestGetExistingRestartAnnotationsDCD(t *testing.T) { }, }) // Annotation hash can differ from computed hash — function uses active compatibility hash. - computedHash := betaDGDWorkersSpecHash(t, dgd) + computedHash := legacyDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: "oldhash", } @@ -1589,7 +1548,7 @@ func TestGetExistingRestartAnnotationsDCD(t *testing.T) { ComponentType: consts.ComponentTypeWorker, }, }) - legacyHash := betaDGDWorkersSpecHash(t, dgd) + legacyHash := legacyDGDWorkersSpecHash(t, dgd) v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: legacyHash, @@ -1698,7 +1657,7 @@ func TestCheckComponentFullyUpdated(t *testing.T) { ComponentType: consts.ComponentTypeWorker, }, }) - workerHash := betaDGDWorkersSpecHash(t, dgd) + workerHash := legacyDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: workerHash, } @@ -1734,7 +1693,7 @@ func TestCheckComponentFullyUpdated(t *testing.T) { ComponentType: consts.ComponentTypeWorker, }, }) - legacyHash := betaDGDWorkersSpecHash(t, dgd) + legacyHash := legacyDGDWorkersSpecHash(t, dgd) v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: legacyHash, @@ -3144,7 +3103,7 @@ func TestReconcileRollingUpdate_NoChange(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": {ComponentType: consts.ComponentTypeWorker}, }) - hash := betaDGDWorkersSpecHash(t, dgd) + hash := legacyDGDWorkersSpecHash(t, dgd) v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: hash, @@ -3197,7 +3156,7 @@ func TestReconcileRollingUpdate_StuckDetection(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": {ComponentType: consts.ComponentTypeWorker}, }) - hash := betaDGDWorkersSpecHash(t, dgd) + hash := legacyDGDWorkersSpecHash(t, dgd) // Hash matches current but phase is InProgress — stuck dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: hash, @@ -3214,6 +3173,26 @@ func TestReconcileRollingUpdate_StuckDetection(t *testing.T) { assert.Equal(t, nvidiacomv1beta1.RollingUpdatePhaseCompleted, dgd.Status.RollingUpdate.Phase) } +func TestLegacyMigrationCompletesWithoutV2Reroll(t *testing.T) { + dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ + "worker": {ComponentType: consts.ComponentTypeWorker}, + }) + dgd.Annotations = map[string]string{ + consts.AnnotationCurrentWorkerHash: consts.LegacyWorkerHash, + } + r := createTestReconcilerWithStatus(dgd) + + desired, err := r.desiredWorkerHashes(dgd) + require.NoError(t, err) + require.NoError(t, r.completeRollingUpdate(context.Background(), dgd, desired.v1)) + require.Equal(t, desired.v1, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.Equal(t, desired.v2, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) + + trigger, err := r.shouldTriggerRollingUpdate(dgd) + require.NoError(t, err) + require.False(t, trigger) +} + func TestReconcileRollingUpdate_NewRollingUpdate(t *testing.T) { newHash := "newhash1" dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ @@ -3265,7 +3244,7 @@ func TestReconcileRollingUpdate_StaleAnnotationRequiresAllNewWorkersReady(t *tes dgd.Status.RollingUpdate = &nvidiacomv1beta1.RollingUpdateStatus{ Phase: nvidiacomv1beta1.RollingUpdatePhaseCompleted, } - newHash := betaDGDWorkersSpecHash(t, dgd) + newHash := legacyDGDWorkersSpecHash(t, dgd) require.NotEqual(t, testOldWorkerHash, newHash) newPrefillDCD := betaDCD(t, &nvidiacomv1alpha1.DynamoComponentDeployment{ @@ -3307,7 +3286,7 @@ func TestReconcileRollingUpdate_StaleAnnotationUpdatesAfterAllNewWorkersReady(t dgd.Status.RollingUpdate = &nvidiacomv1beta1.RollingUpdateStatus{ Phase: nvidiacomv1beta1.RollingUpdatePhaseCompleted, } - newHash := betaDGDWorkersSpecHash(t, dgd) + newHash := legacyDGDWorkersSpecHash(t, dgd) require.NotEqual(t, testOldWorkerHash, newHash) makeReadyDCD := func(componentName, componentType string) *nvidiacomv1beta1.DynamoComponentDeployment { @@ -3373,9 +3352,10 @@ func TestReconcileRollingUpdate_StuckDetection_CompletesViaCompleteRollingUpdate "prefill": {ComponentType: consts.ComponentTypePrefill}, "decode": {ComponentType: consts.ComponentTypeDecode}, }) + legacyHash := legacyDGDWorkersSpecHash(t, dgd) v2Hash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: v2Hash, + consts.AnnotationCurrentWorkerHash: legacyHash, consts.AnnotationCurrentWorkerHashV2: v2Hash, } dgd.Status.RollingUpdate = &nvidiacomv1beta1.RollingUpdateStatus{ @@ -3393,9 +3373,9 @@ func TestReconcileRollingUpdate_StuckDetection_CompletesViaCompleteRollingUpdate // UpdatedComponents should contain all worker services assert.Contains(t, dgd.Status.RollingUpdate.UpdatedComponents, "prefill") assert.Contains(t, dgd.Status.RollingUpdate.UpdatedComponents, "decode") - // Completion collapses an already-v2 generation to the canonical annotation. - assert.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - assert.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) + // Completion records both active compatibility hashes. + assert.Equal(t, legacyHash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + assert.Equal(t, v2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) } func TestBuildRollingUpdateContext(t *testing.T) { @@ -3691,7 +3671,7 @@ func TestBuildRollingUpdateContext(t *testing.T) { } // Compute the actual new DCD label hash from the DGD spec. - newHash := betaDGDWorkersSpecHash(t, dgd) + newHash := legacyDGDWorkersSpecHash(t, dgd) require.NotEqual(t, testOldWorkerHash, newHash, "test setup: computed hash must differ from old hash") // Collect all mock objects @@ -3757,7 +3737,7 @@ func TestBuildRollingUpdateContext_NoNewDCDExists(t *testing.T) { }, }) - newHash := betaDGDWorkersSpecHash(t, dgd) + newHash := legacyDGDWorkersSpecHash(t, dgd) assert.NotEqual(t, testOldWorkerHash, newHash, "test setup: computed hash must differ from old hash") r := createTestReconcilerWithStatus(dgd, withObjects(oldDCD)) @@ -3783,7 +3763,7 @@ func TestBuildRollingUpdateContext_ListOldDCDsError(t *testing.T) { consts.AnnotationCurrentWorkerHash: testOldWorkerHash, } - assert.NotEqual(t, testOldWorkerHash, betaDGDWorkersSpecHash(t, dgd), + assert.NotEqual(t, testOldWorkerHash, legacyDGDWorkersSpecHash(t, dgd), "test setup: computed hash must differ so we proceed past the early-return") injectedErr := errors.New("simulated apiserver list failure") @@ -3814,7 +3794,7 @@ func TestBuildRollingUpdateContext_GetNewDCDError(t *testing.T) { consts.AnnotationCurrentWorkerHash: testOldWorkerHash, } - require.NotEqual(t, testOldWorkerHash, betaDGDWorkersSpecHash(t, dgd), + require.NotEqual(t, testOldWorkerHash, legacyDGDWorkersSpecHash(t, dgd), "test setup: computed hash must differ so we proceed past the early-return") injectedErr := errors.New("simulated apiserver get failure") diff --git a/deploy/operator/internal/controller/test_beta_helpers_test.go b/deploy/operator/internal/controller/test_beta_helpers_test.go index 9a0b2c1153af..9eda856d7268 100644 --- a/deploy/operator/internal/controller/test_beta_helpers_test.go +++ b/deploy/operator/internal/controller/test_beta_helpers_test.go @@ -86,6 +86,15 @@ func betaDGDWorkersSpecHash(t testing.TB, dgd *v1beta1.DynamoGraphDeployment) st return hash } +func legacyDGDWorkersSpecHash(t testing.TB, dgd *v1beta1.DynamoGraphDeployment) string { + t.Helper() + hash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + if err != nil { + t.Fatalf("compute v1alpha1-compatible DGD worker hash: %v", err) + } + return hash +} + func betaRestartStatus(src *v1alpha1.RestartStatus) *v1beta1.RestartStatus { if src == nil { return nil diff --git a/deploy/operator/internal/dynamo/hash.go b/deploy/operator/internal/dynamo/hash.go index 8ad6ac6b2111..126f9b625563 100644 --- a/deploy/operator/internal/dynamo/hash.go +++ b/deploy/operator/internal/dynamo/hash.go @@ -29,9 +29,9 @@ import ( const dgdWorkerHashPlaceholderValue = "worker-hash-placeholder" -// ComputeLegacyAlphaDGDWorkersSpecHash returns the frozen v1alpha1 worker hash. -// New and stable generations must not use it; it exists only so an in-progress -// literal "legacy" migration can finish on the target selected by Dynamo 1.2. +// ComputeLegacyAlphaDGDWorkersSpecHash returns the v1alpha1 worker hash that a +// pre-v1beta1 controller would compute for the DGD's current spec. Conversion +// must preserve every v1alpha1 hash input shape this depends on. func ComputeLegacyAlphaDGDWorkersSpecHash(dgd *v1beta1.DynamoGraphDeployment) (string, error) { alpha := &v1alpha1.DynamoGraphDeployment{} if err := alpha.ConvertFrom(dgd); err != nil { From 1fa4bb806899d5228a1f6029facc0f250d174ec9 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 Jul 2026 15:01:55 +0200 Subject: [PATCH 05/10] fixup! operator/worker-hash: converge generations lazily to v2 Signed-off-by: Dr. Stefan Schimanski --- .../dynamographdeployment_controller_test.go | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go index dc45e060d664..7d58c270a4bd 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go @@ -4181,7 +4181,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, }, { - name: "multiple services - all DCDs ready", + name: "multiple services - fresh v2 DCDs all ready", dgdSpec: v1alpha1.DynamoGraphDeploymentSpec{ BackendFramework: "vllm", Services: map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ @@ -4327,7 +4327,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, }, { - name: "multiple services - some DCDs ready, some not ready", + name: "multiple services - fresh v2 DCDs partially ready", dgdSpec: v1alpha1.DynamoGraphDeploymentSpec{ BackendFramework: "vllm", Services: map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ @@ -4593,6 +4593,24 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { Spec: tt.dgdSpec, }) + // Full reconciliation initializes a fresh DGD with its v2 hash before calling this helper. + workerHash := betaDGDWorkersSpecHash(t, dgd) + dgd.Annotations = map[string]string{ + commonconsts.AnnotationCurrentWorkerHash: workerHash, + } + for _, object := range tt.existingDCDs { + dcd, ok := object.(*v1beta1.DynamoComponentDeployment) + if !ok || !dynamo.IsWorkerComponent(string(dcd.Spec.ComponentType)) { + continue + } + dcd.Labels[commonconsts.KubeLabelDynamoWorkerHash] = workerHash + dcd.Spec.PodTemplate = &corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{commonconsts.KubeLabelDynamoWorkerHash: workerHash}, + }, + } + } + var objects []client.Object objects = append(objects, dgd) objects = append(objects, tt.existingDCDs...) From 564845b2d5b0b809148a42e4b3a098981d9766b8 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 Jul 2026 15:07:16 +0200 Subject: [PATCH 06/10] fixup! operator/worker-hash: converge generations lazily to v2 Signed-off-by: Dr. Stefan Schimanski --- .../dynamographdeployment_controller_test.go | 42 ++++++++++--------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go index 7d58c270a4bd..781c92babf3b 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go @@ -3995,6 +3995,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { tests := []struct { name string dgdSpec v1alpha1.DynamoGraphDeploymentSpec + dgdAnnotations map[string]string existingDCDs []client.Object wantReconcileResult ReconcileResult }{ @@ -4205,6 +4206,9 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, }, }, + dgdAnnotations: map[string]string{ + commonconsts.AnnotationCurrentWorkerHash: "1b69c0d3", + }, existingDCDs: []client.Object{ betaDCD(t, &v1alpha1.DynamoComponentDeployment{ ObjectMeta: metav1.ObjectMeta{ @@ -4245,6 +4249,9 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { DynamoComponentDeploymentSharedSpec: v1alpha1.DynamoComponentDeploymentSharedSpec{ ServiceName: "decode", Replicas: ptr.To(int32(2)), + Labels: map[string]string{ + commonconsts.KubeLabelDynamoWorkerHash: "1b69c0d3", + }, }, }, Status: v1alpha1.DynamoComponentDeploymentStatus{ @@ -4274,6 +4281,9 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { DynamoComponentDeploymentSharedSpec: v1alpha1.DynamoComponentDeploymentSharedSpec{ ServiceName: "prefill", Replicas: ptr.To(int32(3)), + Labels: map[string]string{ + commonconsts.KubeLabelDynamoWorkerHash: "1b69c0d3", + }, }, }, Status: v1alpha1.DynamoComponentDeploymentStatus{ @@ -4351,6 +4361,9 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, }, }, + dgdAnnotations: map[string]string{ + commonconsts.AnnotationCurrentWorkerHash: "1b69c0d3", + }, existingDCDs: []client.Object{ betaDCD(t, &v1alpha1.DynamoComponentDeployment{ ObjectMeta: metav1.ObjectMeta{ @@ -4391,6 +4404,9 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { DynamoComponentDeploymentSharedSpec: v1alpha1.DynamoComponentDeploymentSharedSpec{ ServiceName: "decode", Replicas: ptr.To(int32(2)), + Labels: map[string]string{ + commonconsts.KubeLabelDynamoWorkerHash: "1b69c0d3", + }, }, }, Status: v1alpha1.DynamoComponentDeploymentStatus{ @@ -4420,6 +4436,9 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { DynamoComponentDeploymentSharedSpec: v1alpha1.DynamoComponentDeploymentSharedSpec{ ServiceName: "prefill", Replicas: ptr.To(int32(3)), + Labels: map[string]string{ + commonconsts.KubeLabelDynamoWorkerHash: "1b69c0d3", + }, }, }, Status: v1alpha1.DynamoComponentDeploymentStatus{ @@ -4587,30 +4606,13 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { dgd := betaDGD(t, &v1alpha1.DynamoGraphDeployment{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-dgd", - Namespace: "default", + Name: "test-dgd", + Namespace: "default", + Annotations: tt.dgdAnnotations, }, Spec: tt.dgdSpec, }) - // Full reconciliation initializes a fresh DGD with its v2 hash before calling this helper. - workerHash := betaDGDWorkersSpecHash(t, dgd) - dgd.Annotations = map[string]string{ - commonconsts.AnnotationCurrentWorkerHash: workerHash, - } - for _, object := range tt.existingDCDs { - dcd, ok := object.(*v1beta1.DynamoComponentDeployment) - if !ok || !dynamo.IsWorkerComponent(string(dcd.Spec.ComponentType)) { - continue - } - dcd.Labels[commonconsts.KubeLabelDynamoWorkerHash] = workerHash - dcd.Spec.PodTemplate = &corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{commonconsts.KubeLabelDynamoWorkerHash: workerHash}, - }, - } - } - var objects []client.Object objects = append(objects, dgd) objects = append(objects, tt.existingDCDs...) From 7a4222fe111e7383b7a2bf2dd0c09ff9916a8157 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 Jul 2026 15:28:23 +0200 Subject: [PATCH 07/10] fixup! operator/worker-hash: converge generations lazily to v2 Signed-off-by: Dr. Stefan Schimanski --- deploy/operator/internal/consts/consts.go | 33 ++++++-------- .../dynamographdeployment_rollingupdate.go | 28 ++++++------ ...ynamographdeployment_rollingupdate_test.go | 43 +++++++++++++++---- 3 files changed, 64 insertions(+), 40 deletions(-) diff --git a/deploy/operator/internal/consts/consts.go b/deploy/operator/internal/consts/consts.go index db134cb8ebda..20fa93a0f8aa 100644 --- a/deploy/operator/internal/consts/consts.go +++ b/deploy/operator/internal/consts/consts.go @@ -77,9 +77,9 @@ const ( KubeLabelDynamoDiscoveryEnabled = "nvidia.com/dynamo-discovery-enabled" // KubeLabelDynamoWorkerHash is the worker generation label on worker DCDs // and worker pods. During v1/v2 hash compatibility the label key remains - // stable and the value may be either the active v1 hash or the active v2 hash - // recorded on the parent DGD. Older operators understand only the v1 value, - // so v1-compatible releases continue to generate new DCDs with the v1 value. + // stable. Its value is the active generation identity in + // AnnotationCurrentWorkerHash, which may be a bridged v1 hash or a canonical + // v2 hash. KubeLabelDynamoWorkerHash = "nvidia.com/dynamo-worker-hash" // CheckpointAutoAnnotation marks operator-created checkpoints whose @@ -242,24 +242,19 @@ const ( // these annotations remain on the previously serving worker generation until // the new generation is fully ready and old workers have drained. // - // The compatibility contract is intentionally additive: existing annotation - // and label keys keep their old meaning. AnnotationCurrentWorkerHash stores - // the v1alpha1-compatible worker hash so a downgrade can still understand the - // active generation. AnnotationCurrentWorkerHashV2 stores the v2 worker hash - // for the same active generation. A worker DCD whose - // KubeLabelDynamoWorkerHash value matches either annotation is current. While - // v1 compatibility is required, generated worker DCDs use the v1 hash as the - // label value. If a worker change is visible only to v2, the controller - // removes the v1 annotation and rolls to a v2-labeled DCD because the v1 hash - // can no longer prove pod-template compatibility. A future v2-only release - // can start using the v2 value with the same label key and keep accepting the - // v1 annotation until the next v2 generation change drains old workers. - - // AnnotationCurrentWorkerHash stores the active v1alpha1-compatible worker - // generation hash. + // AnnotationCurrentWorkerHash stores the active DCD generation identity. For + // a bridged 1.2 generation it contains the v1alpha1-compatible hash and + // AnnotationCurrentWorkerHashV2 stores the v2 fingerprint for the same spec. + // Fresh and completed canonical generations instead store the v2 identity in + // AnnotationCurrentWorkerHash and omit AnnotationCurrentWorkerHashV2. Existing + // bridge annotations remain unchanged until a real worker change selects and + // completes a canonical v2 generation, avoiding a rollout during upgrade. + + // AnnotationCurrentWorkerHash stores the active worker generation identity. AnnotationCurrentWorkerHash = "nvidia.com/current-worker-hash" - // AnnotationCurrentWorkerHashV2 stores the active v2 worker generation hash. + // AnnotationCurrentWorkerHashV2 stores the v2 fingerprint for a bridged v1 + // generation and is absent for a canonical v2 generation. AnnotationCurrentWorkerHashV2 = "nvidia.com/current-worker-hash-v2" // LegacyWorkerHash is a sentinel value used during migration from pre-rolling-update diff --git a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go index 211c8667cebc..172dcbc59d17 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go +++ b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go @@ -66,10 +66,14 @@ func (r *DynamoGraphDeploymentReconciler) desiredWorkerHashes( v1Hash = current.v1 } if current.v2 == "" && current.v1 != "" && current.v1 != v2Hash { - v1Hash, err = dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + // Keep v1 only when its value proves a pre-dual generation or is the explicit sentinel. + legacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) if err != nil { return workerGenerationHashes{}, fmt.Errorf("failed to compute v1 worker hash: %w", err) } + if current.v1 == consts.LegacyWorkerHash || current.v1 == legacyHash { + v1Hash = legacyHash + } } return workerGenerationHashes{v1: v1Hash, v2: v2Hash}, nil @@ -132,9 +136,8 @@ func (r *DynamoGraphDeploymentReconciler) workerHashesForUnsupportedPathway( // generation recorded on the DGD. // // During v1/v2 compatibility a worker DCD is current if its worker-hash label -// matches either current-worker-hash (v1) or current-worker-hash-v2. This keeps -// the existing annotation/label meaning downgrade-safe while allowing the -// controller to record the v2 hash that will become primary later. +// matches either the active generation in current-worker-hash or its v2 +// fingerprint in current-worker-hash-v2. func (r *DynamoGraphDeploymentReconciler) shouldTriggerRollingUpdate( dgd *nvidiacomv1beta1.DynamoGraphDeployment, ) (bool, error) { @@ -148,7 +151,7 @@ func (r *DynamoGraphDeploymentReconciler) shouldTriggerRollingUpdate( } // initializeWorkerHashIfNeeded establishes the DGD's active worker generation. -// New DGDs store the current v1 and v2 worker hashes immediately. DGDs created before +// New DGDs store the canonical v2 worker hash in current-worker-hash. DGDs created before // managed rolling updates may already have worker DCDs without a hash label; in // that case we label those DCDs with the legacy sentinel and let the normal // rolling update path migrate from that sentinel to the desired compatibility hash. @@ -198,7 +201,7 @@ func (r *DynamoGraphDeploymentReconciler) initializeWorkerHashIfNeeded( return nil } - // Normal first deploy — set the actual computed compatibility hashes + // Normal first deploy — set the canonical v2 hash. hashes, err := r.desiredWorkerHashes(dgd) if err != nil { return err @@ -261,10 +264,9 @@ func (r *DynamoGraphDeploymentReconciler) migrateCurrentWorkerHashIfNeeded( } // activeWorkerHashForDCDGeneration returns the hash used for generated worker -// DCD names and worker-hash labels in this reconcile. While v1 compatibility is -// required, new DCDs continue to use the v1 hash. If the active generation is -// already v2-labeled, preserve that value so future v2-only transitions do not -// create a v1-labeled replacement. +// DCD names and worker-hash labels in this reconcile. Existing bridge generations +// keep their v1 identity until a worker change selects v2; canonical generations +// continue directly from one v2 identity to the next. func (r *DynamoGraphDeploymentReconciler) activeWorkerHashForDCDGeneration( dgd *nvidiacomv1beta1.DynamoGraphDeployment, desired workerGenerationHashes, @@ -353,9 +355,9 @@ func (r *DynamoGraphDeploymentReconciler) getCurrentWorkerHashV2( return dgd.Annotations[consts.AnnotationCurrentWorkerHashV2] } -// setCurrentWorkerHashes stores the active worker hashes for one generation. -// Empty fields are deleted, which is how v2-only generations intentionally drop -// the downgrade-compatible v1 annotation. +// setCurrentWorkerHashes stores one active generation. The historical v1 field +// maps to current-worker-hash and may contain either a v1 bridge identity or the +// canonical v2 identity. The v2 field is the optional bridge fingerprint. func (r *DynamoGraphDeploymentReconciler) setCurrentWorkerHashes( dgd *nvidiacomv1beta1.DynamoGraphDeployment, hashes workerGenerationHashes, diff --git a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go index bb63e90662e0..463830ca0274 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go @@ -234,13 +234,16 @@ func TestShouldTriggerRollingUpdate_IgnoresReplicaChanges(t *testing.T) { assert.False(t, trigger) } -func TestInitializeWorkerHashIfNeeded_FirstDeploy(t *testing.T) { +func TestCanonicalWorkerHashLifecycle_FirstDeploySpecChangeAndCompletion(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, Envs: []corev1.EnvVar{ {Name: "FOO", Value: "bar"}, }, + Resources: &nvidiacomv1alpha1.Resources{ + Requests: &nvidiacomv1alpha1.ResourceItem{CPU: "1"}, + }, }, }) @@ -260,6 +263,30 @@ func TestInitializeWorkerHashIfNeeded_FirstDeploy(t *testing.T) { expectedV2Hash := betaDGDWorkersSpecHash(t, dgd) assert.Equal(t, expectedV2Hash, hash) assert.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) + + // A later worker change rolls directly from one canonical v2 generation to the next. + dgd.Spec.Components[0].PodTemplate.Spec.Containers[0].Env = append( + dgd.Spec.Components[0].PodTemplate.Spec.Containers[0].Env, + corev1.EnvVar{Name: "NEW_WORKER_SETTING", Value: "true"}, + ) + newV2Hash := betaDGDWorkersSpecHash(t, dgd) + newLegacyHash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) + require.NoError(t, err) + require.NotEqual(t, expectedV2Hash, newV2Hash) + require.NotEqual(t, newLegacyHash, newV2Hash) + + trigger, err := r.shouldTriggerRollingUpdate(dgd) + require.NoError(t, err) + require.True(t, trigger) + + rollingCtx, err := r.buildRollingUpdateContext(ctx, dgd) + require.NoError(t, err) + require.Equal(t, newV2Hash, rollingCtx.NewWorkerHash) + require.NotEqual(t, newLegacyHash, rollingCtx.NewWorkerHash) + + require.NoError(t, r.completeRollingUpdate(ctx, dgd, newV2Hash)) + require.Equal(t, newV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) } func TestInitializeWorkerHashIfNeeded_AlreadyInitialized(t *testing.T) { @@ -1498,8 +1525,8 @@ func TestGetExistingRestartAnnotationsDCD(t *testing.T) { ComponentType: consts.ComponentTypeWorker, }, }) - // Annotation hash can differ from computed hash — function uses active compatibility hash. - computedHash := legacyDGDWorkersSpecHash(t, dgd) + // A spec change selects the new canonical v2 hash instead of the stale current identity. + computedHash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: "oldhash", } @@ -3244,7 +3271,7 @@ func TestReconcileRollingUpdate_StaleAnnotationRequiresAllNewWorkersReady(t *tes dgd.Status.RollingUpdate = &nvidiacomv1beta1.RollingUpdateStatus{ Phase: nvidiacomv1beta1.RollingUpdatePhaseCompleted, } - newHash := legacyDGDWorkersSpecHash(t, dgd) + newHash := betaDGDWorkersSpecHash(t, dgd) require.NotEqual(t, testOldWorkerHash, newHash) newPrefillDCD := betaDCD(t, &nvidiacomv1alpha1.DynamoComponentDeployment{ @@ -3286,7 +3313,7 @@ func TestReconcileRollingUpdate_StaleAnnotationUpdatesAfterAllNewWorkersReady(t dgd.Status.RollingUpdate = &nvidiacomv1beta1.RollingUpdateStatus{ Phase: nvidiacomv1beta1.RollingUpdatePhaseCompleted, } - newHash := legacyDGDWorkersSpecHash(t, dgd) + newHash := betaDGDWorkersSpecHash(t, dgd) require.NotEqual(t, testOldWorkerHash, newHash) makeReadyDCD := func(componentName, componentType string) *nvidiacomv1beta1.DynamoComponentDeployment { @@ -3671,7 +3698,7 @@ func TestBuildRollingUpdateContext(t *testing.T) { } // Compute the actual new DCD label hash from the DGD spec. - newHash := legacyDGDWorkersSpecHash(t, dgd) + newHash := betaDGDWorkersSpecHash(t, dgd) require.NotEqual(t, testOldWorkerHash, newHash, "test setup: computed hash must differ from old hash") // Collect all mock objects @@ -3737,7 +3764,7 @@ func TestBuildRollingUpdateContext_NoNewDCDExists(t *testing.T) { }, }) - newHash := legacyDGDWorkersSpecHash(t, dgd) + newHash := betaDGDWorkersSpecHash(t, dgd) assert.NotEqual(t, testOldWorkerHash, newHash, "test setup: computed hash must differ from old hash") r := createTestReconcilerWithStatus(dgd, withObjects(oldDCD)) @@ -3763,7 +3790,7 @@ func TestBuildRollingUpdateContext_ListOldDCDsError(t *testing.T) { consts.AnnotationCurrentWorkerHash: testOldWorkerHash, } - assert.NotEqual(t, testOldWorkerHash, legacyDGDWorkersSpecHash(t, dgd), + assert.NotEqual(t, testOldWorkerHash, betaDGDWorkersSpecHash(t, dgd), "test setup: computed hash must differ so we proceed past the early-return") injectedErr := errors.New("simulated apiserver list failure") From 4b3916ad80fbf1e5cefd84bbdb2792cb51bbfc15 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 Jul 2026 15:34:33 +0200 Subject: [PATCH 08/10] fixup! operator/worker-hash: converge generations lazily to v2 Signed-off-by: Dr. Stefan Schimanski --- deploy/operator/internal/consts/consts.go | 39 +++++++++++------------ 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/deploy/operator/internal/consts/consts.go b/deploy/operator/internal/consts/consts.go index 20fa93a0f8aa..5a1b27dc5a0b 100644 --- a/deploy/operator/internal/consts/consts.go +++ b/deploy/operator/internal/consts/consts.go @@ -75,11 +75,9 @@ const ( KubeAnnotationDynamoBaseModel = "nvidia.com/dynamo-base-model" KubeLabelDynamoDiscoveryBackend = "nvidia.com/dynamo-discovery-backend" KubeLabelDynamoDiscoveryEnabled = "nvidia.com/dynamo-discovery-enabled" - // KubeLabelDynamoWorkerHash is the worker generation label on worker DCDs - // and worker pods. During v1/v2 hash compatibility the label key remains - // stable. Its value is the active generation identity in - // AnnotationCurrentWorkerHash, which may be a bridged v1 hash or a canonical - // v2 hash. + // KubeLabelDynamoWorkerHash identifies a worker generation on DCDs and pods. + // The parent DGD records the active label value in + // AnnotationCurrentWorkerHash. KubeLabelDynamoWorkerHash = "nvidia.com/dynamo-worker-hash" // CheckpointAutoAnnotation marks operator-created checkpoints whose @@ -236,25 +234,24 @@ const ( ResourceStateNotReady = "not_ready" ResourceStateUnknown = "unknown" - // Worker hash rolling-update annotations are controller-owned annotations on - // DynamoGraphDeployment. They record the active worker generation and must not - // be treated as user-configurable inputs. During a managed rolling update, - // these annotations remain on the previously serving worker generation until - // the new generation is fully ready and old workers have drained. + // These controller-owned annotations exist on the DGD, not on worker DCDs. + // Worker DCDs and pods carry KubeLabelDynamoWorkerHash instead. During a + // managed rolling update, the DGD annotations remain on the previously + // serving generation until the new generation is ready and old workers have + // drained. // - // AnnotationCurrentWorkerHash stores the active DCD generation identity. For - // a bridged 1.2 generation it contains the v1alpha1-compatible hash and - // AnnotationCurrentWorkerHashV2 stores the v2 fingerprint for the same spec. - // Fresh and completed canonical generations instead store the v2 identity in - // AnnotationCurrentWorkerHash and omit AnnotationCurrentWorkerHashV2. Existing - // bridge annotations remain unchanged until a real worker change selects and - // completes a canonical v2 generation, avoiding a rollout during upgrade. - - // AnnotationCurrentWorkerHash stores the active worker generation identity. + // For a DGD bridged from 1.2, AnnotationCurrentWorkerHash contains the v1 hash + // used as the active DCD label. AnnotationCurrentWorkerHashV2 contains only + // the v2 fingerprint for the same worker spec; it is not a DCD annotation. + // Fresh DGDs and generations completed on v2 store the v2 DCD label in + // AnnotationCurrentWorkerHash and omit AnnotationCurrentWorkerHashV2. + + // AnnotationCurrentWorkerHash stores, on the DGD, the worker-hash label value + // of its active DCD generation. AnnotationCurrentWorkerHash = "nvidia.com/current-worker-hash" - // AnnotationCurrentWorkerHashV2 stores the v2 fingerprint for a bridged v1 - // generation and is absent for a canonical v2 generation. + // AnnotationCurrentWorkerHashV2 stores, on a bridged DGD, the v2 fingerprint + // for its worker spec. It is not a DCD annotation. AnnotationCurrentWorkerHashV2 = "nvidia.com/current-worker-hash-v2" // LegacyWorkerHash is a sentinel value used during migration from pre-rolling-update From 34281ace11ec412623d44271c457bb463f16bd8a Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 Jul 2026 15:40:51 +0200 Subject: [PATCH 09/10] fixup! operator/worker-hash: converge generations lazily to v2 Signed-off-by: Dr. Stefan Schimanski --- deploy/operator/internal/consts/consts.go | 20 +++++++++--------- .../dynamographdeployment_controller_test.go | 4 ++-- .../dynamographdeployment_rollingupdate.go | 17 ++++++--------- ...ynamographdeployment_rollingupdate_test.go | 21 ++++++++++--------- 4 files changed, 29 insertions(+), 33 deletions(-) diff --git a/deploy/operator/internal/consts/consts.go b/deploy/operator/internal/consts/consts.go index 5a1b27dc5a0b..7cc09b455671 100644 --- a/deploy/operator/internal/consts/consts.go +++ b/deploy/operator/internal/consts/consts.go @@ -76,8 +76,8 @@ const ( KubeLabelDynamoDiscoveryBackend = "nvidia.com/dynamo-discovery-backend" KubeLabelDynamoDiscoveryEnabled = "nvidia.com/dynamo-discovery-enabled" // KubeLabelDynamoWorkerHash identifies a worker generation on DCDs and pods. - // The parent DGD records the active label value in - // AnnotationCurrentWorkerHash. + // The parent DGD records the active label value in one of the worker-hash + // annotations below. KubeLabelDynamoWorkerHash = "nvidia.com/dynamo-worker-hash" // CheckpointAutoAnnotation marks operator-created checkpoints whose @@ -241,17 +241,17 @@ const ( // drained. // // For a DGD bridged from 1.2, AnnotationCurrentWorkerHash contains the v1 hash - // used as the active DCD label. AnnotationCurrentWorkerHashV2 contains only - // the v2 fingerprint for the same worker spec; it is not a DCD annotation. - // Fresh DGDs and generations completed on v2 store the v2 DCD label in - // AnnotationCurrentWorkerHash and omit AnnotationCurrentWorkerHashV2. + // used as the active DCD label and AnnotationCurrentWorkerHashV2 contains the + // v2 fingerprint for the same worker spec. Fresh DGDs and generations + // completed on v2 omit AnnotationCurrentWorkerHash and store the active v2 DCD + // label in AnnotationCurrentWorkerHashV2. - // AnnotationCurrentWorkerHash stores, on the DGD, the worker-hash label value - // of its active DCD generation. + // AnnotationCurrentWorkerHash stores, on the DGD, the v1 worker-hash label + // value of a bridged generation. AnnotationCurrentWorkerHash = "nvidia.com/current-worker-hash" - // AnnotationCurrentWorkerHashV2 stores, on a bridged DGD, the v2 fingerprint - // for its worker spec. It is not a DCD annotation. + // AnnotationCurrentWorkerHashV2 stores, on the DGD, the v2 worker-spec hash. + // After v2 convergence it is also the active DCD worker-hash label value. AnnotationCurrentWorkerHashV2 = "nvidia.com/current-worker-hash-v2" // LegacyWorkerHash is a sentinel value used during migration from pre-rolling-update diff --git a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go index 781c92babf3b..b912166b5d8f 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go @@ -4207,7 +4207,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, }, dgdAnnotations: map[string]string{ - commonconsts.AnnotationCurrentWorkerHash: "1b69c0d3", + commonconsts.AnnotationCurrentWorkerHashV2: "1b69c0d3", }, existingDCDs: []client.Object{ betaDCD(t, &v1alpha1.DynamoComponentDeployment{ @@ -4362,7 +4362,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, }, dgdAnnotations: map[string]string{ - commonconsts.AnnotationCurrentWorkerHash: "1b69c0d3", + commonconsts.AnnotationCurrentWorkerHashV2: "1b69c0d3", }, existingDCDs: []client.Object{ betaDCD(t, &v1alpha1.DynamoComponentDeployment{ diff --git a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go index 172dcbc59d17..9a8d0037dc3d 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go +++ b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go @@ -119,7 +119,7 @@ func workerHashForDCDGeneration(current, desired workerGenerationHashes) string func workerHashesForCompletedGeneration(newWorkerHash string, desired workerGenerationHashes) workerGenerationHashes { if newWorkerHash == desired.v2 { - return workerGenerationHashes{v1: desired.v2} + return workerGenerationHashes{v2: desired.v2} } return desired } @@ -151,7 +151,7 @@ func (r *DynamoGraphDeploymentReconciler) shouldTriggerRollingUpdate( } // initializeWorkerHashIfNeeded establishes the DGD's active worker generation. -// New DGDs store the canonical v2 worker hash in current-worker-hash. DGDs created before +// New DGDs store only the canonical v2 worker hash. DGDs created before // managed rolling updates may already have worker DCDs without a hash label; in // that case we label those DCDs with the legacy sentinel and let the normal // rolling update path migrate from that sentinel to the desired compatibility hash. @@ -237,8 +237,6 @@ func (r *DynamoGraphDeploymentReconciler) migrateCurrentWorkerHashIfNeeded( var next workerGenerationHashes switch { - case current.v1 == "" && current.v2 == desired.v2: - next = workerGenerationHashes{v1: current.v2} case current.v1 != "" && current.v2 == "" && current.v1 != desired.v2: v1Hash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) if err != nil { @@ -333,10 +331,8 @@ func (r *DynamoGraphDeploymentReconciler) supportsManagedRollingUpdate( return !r.isGrovePathway(dgd) && !dgd.HasAnyMultinodeComponent() } -// getCurrentWorkerHash returns the active worker generation stored on the DGD. -// During a rolling update this is the previous serving hash; it is not advanced -// to the desired hash until the new generation is ready and the old generation -// has drained. Empty means this DGD has not initialized rolling-update state. +// getCurrentWorkerHash returns the v1 worker generation stored on the DGD. +// It is empty after the DGD has converged to a v2-only generation. func (r *DynamoGraphDeploymentReconciler) getCurrentWorkerHash( dgd *nvidiacomv1beta1.DynamoGraphDeployment, ) string { @@ -355,9 +351,8 @@ func (r *DynamoGraphDeploymentReconciler) getCurrentWorkerHashV2( return dgd.Annotations[consts.AnnotationCurrentWorkerHashV2] } -// setCurrentWorkerHashes stores one active generation. The historical v1 field -// maps to current-worker-hash and may contain either a v1 bridge identity or the -// canonical v2 identity. The v2 field is the optional bridge fingerprint. +// setCurrentWorkerHashes stores the v1 and v2 generation values on the DGD. +// A bridged generation has both; a canonical v2 generation has only v2. func (r *DynamoGraphDeploymentReconciler) setCurrentWorkerHashes( dgd *nvidiacomv1beta1.DynamoGraphDeployment, hashes workerGenerationHashes, diff --git a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go index 463830ca0274..456a24ab7ae5 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go @@ -255,14 +255,14 @@ func TestCanonicalWorkerHashLifecycle_FirstDeploySpecChangeAndCompletion(t *test err := r.initializeWorkerHashIfNeeded(ctx, dgd) require.NoError(t, err) - // Verify the hash was set - hash := r.getCurrentWorkerHash(dgd) + // Verify only the v2 hash was set. + hash := r.getCurrentWorkerHashV2(dgd) assert.NotEmpty(t, hash, "Hash should be set after initialization") + assert.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHash) // Fresh deployments store one canonical v2 hash. expectedV2Hash := betaDGDWorkersSpecHash(t, dgd) assert.Equal(t, expectedV2Hash, hash) - assert.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) // A later worker change rolls directly from one canonical v2 generation to the next. dgd.Spec.Components[0].PodTemplate.Spec.Containers[0].Env = append( @@ -285,8 +285,8 @@ func TestCanonicalWorkerHashLifecycle_FirstDeploySpecChangeAndCompletion(t *test require.NotEqual(t, newLegacyHash, rollingCtx.NewWorkerHash) require.NoError(t, r.completeRollingUpdate(ctx, dgd, newV2Hash)) - require.Equal(t, newV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) + require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHash) + require.Equal(t, newV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) } func TestInitializeWorkerHashIfNeeded_AlreadyInitialized(t *testing.T) { @@ -502,8 +502,8 @@ func TestLegacyAlphaHashCompatibility_V2OnlyChangeUsesNewV2Generation(t *testing require.NotEqual(t, newLegacyHash, rollingCtx.NewWorkerHash) require.NoError(t, r.completeRollingUpdate(context.Background(), dgd, newV2Hash)) - require.Equal(t, newV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHashV2) + require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHash) + require.Equal(t, newV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) } func TestUnsupportedPathwayMigratesV1OnlyAndConvergesToCanonicalV2(t *testing.T) { @@ -548,8 +548,8 @@ func TestUnsupportedPathwayMigratesV1OnlyAndConvergesToCanonicalV2(t *testing.T) desired, err := r.desiredWorkerHashes(dgd) require.NoError(t, err) completed := r.workerHashesForUnsupportedPathway(dgd, desired) - require.Equal(t, newV2Hash, completed.v1) - require.Empty(t, completed.v2) + require.Empty(t, completed.v1) + require.Equal(t, newV2Hash, completed.v2) r.setCurrentWorkerHashes(dgd, completed) rollingCtx, err := r.buildRollingUpdateContext(context.Background(), dgd) @@ -3350,7 +3350,8 @@ func TestReconcileRollingUpdate_StaleAnnotationUpdatesAfterAllNewWorkersReady(t err := r.reconcileRollingUpdate(context.Background(), dgd) require.NoError(t, err) - assert.Equal(t, newHash, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) + assert.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHash) + assert.Equal(t, newHash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) assert.Equal(t, nvidiacomv1beta1.RollingUpdatePhaseCompleted, dgd.Status.RollingUpdate.Phase) } From 678ff5c549c131ffcbf3dd7c1ddea96ed72362c5 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 Jul 2026 15:47:13 +0200 Subject: [PATCH 10/10] fixup! operator/worker-hash: converge generations lazily to v2 Signed-off-by: Dr. Stefan Schimanski --- deploy/operator/internal/consts/consts.go | 36 +++++++------- .../dynamographdeployment_controller_test.go | 30 ++++-------- .../dynamographdeployment_rollingupdate.go | 47 ++++++++++--------- ...ynamographdeployment_rollingupdate_test.go | 33 ++++--------- 4 files changed, 64 insertions(+), 82 deletions(-) diff --git a/deploy/operator/internal/consts/consts.go b/deploy/operator/internal/consts/consts.go index 7cc09b455671..df3b89871f50 100644 --- a/deploy/operator/internal/consts/consts.go +++ b/deploy/operator/internal/consts/consts.go @@ -75,9 +75,11 @@ const ( KubeAnnotationDynamoBaseModel = "nvidia.com/dynamo-base-model" KubeLabelDynamoDiscoveryBackend = "nvidia.com/dynamo-discovery-backend" KubeLabelDynamoDiscoveryEnabled = "nvidia.com/dynamo-discovery-enabled" - // KubeLabelDynamoWorkerHash identifies a worker generation on DCDs and pods. - // The parent DGD records the active label value in one of the worker-hash - // annotations below. + // KubeLabelDynamoWorkerHash is the worker generation label on worker DCDs + // and worker pods. During v1/v2 hash compatibility the label key remains + // stable and the value may be either the active v1 hash or the active v2 hash + // recorded on the parent DGD. Older operators understand only the v1 value, + // so v1-compatible releases continue to generate new DCDs with the v1 value. KubeLabelDynamoWorkerHash = "nvidia.com/dynamo-worker-hash" // CheckpointAutoAnnotation marks operator-created checkpoints whose @@ -234,24 +236,22 @@ const ( ResourceStateNotReady = "not_ready" ResourceStateUnknown = "unknown" - // These controller-owned annotations exist on the DGD, not on worker DCDs. - // Worker DCDs and pods carry KubeLabelDynamoWorkerHash instead. During a - // managed rolling update, the DGD annotations remain on the previously - // serving generation until the new generation is ready and old workers have - // drained. + // Worker hash rolling-update annotations are controller-owned annotations on + // DynamoGraphDeployment, not on worker DCDs. During a managed rolling update, + // they remain on the previously serving generation until the new generation + // is fully ready and old workers have drained. // - // For a DGD bridged from 1.2, AnnotationCurrentWorkerHash contains the v1 hash - // used as the active DCD label and AnnotationCurrentWorkerHashV2 contains the - // v2 fingerprint for the same worker spec. Fresh DGDs and generations - // completed on v2 omit AnnotationCurrentWorkerHash and store the active v2 DCD - // label in AnnotationCurrentWorkerHashV2. - - // AnnotationCurrentWorkerHash stores, on the DGD, the v1 worker-hash label - // value of a bridged generation. + // Existing 1.2 DGDs keep both annotations until a worker change completes. + // AnnotationCurrentWorkerHash stores their active v1 hash and + // AnnotationCurrentWorkerHashV2 the v2 hash for the same worker spec. Fresh + // DGDs and completed v2 generations omit AnnotationCurrentWorkerHash and use + // AnnotationCurrentWorkerHashV2 as the active DCD generation hash. + + // AnnotationCurrentWorkerHash stores the active v1alpha1-compatible worker + // generation hash. AnnotationCurrentWorkerHash = "nvidia.com/current-worker-hash" - // AnnotationCurrentWorkerHashV2 stores, on the DGD, the v2 worker-spec hash. - // After v2 convergence it is also the active DCD worker-hash label value. + // AnnotationCurrentWorkerHashV2 stores the active v2 worker generation hash. AnnotationCurrentWorkerHashV2 = "nvidia.com/current-worker-hash-v2" // LegacyWorkerHash is a sentinel value used during migration from pre-rolling-update diff --git a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go index b912166b5d8f..2b7eefeddb56 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go @@ -4182,7 +4182,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, }, { - name: "multiple services - fresh v2 DCDs all ready", + name: "multiple services - all DCDs ready", dgdSpec: v1alpha1.DynamoGraphDeploymentSpec{ BackendFramework: "vllm", Services: map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ @@ -4206,9 +4206,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, }, }, - dgdAnnotations: map[string]string{ - commonconsts.AnnotationCurrentWorkerHashV2: "1b69c0d3", - }, + dgdAnnotations: map[string]string{commonconsts.AnnotationCurrentWorkerHashV2: "1b69c0d3"}, existingDCDs: []client.Object{ betaDCD(t, &v1alpha1.DynamoComponentDeployment{ ObjectMeta: metav1.ObjectMeta{ @@ -4249,9 +4247,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { DynamoComponentDeploymentSharedSpec: v1alpha1.DynamoComponentDeploymentSharedSpec{ ServiceName: "decode", Replicas: ptr.To(int32(2)), - Labels: map[string]string{ - commonconsts.KubeLabelDynamoWorkerHash: "1b69c0d3", - }, + Labels: map[string]string{commonconsts.KubeLabelDynamoWorkerHash: "1b69c0d3"}, }, }, Status: v1alpha1.DynamoComponentDeploymentStatus{ @@ -4281,9 +4277,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { DynamoComponentDeploymentSharedSpec: v1alpha1.DynamoComponentDeploymentSharedSpec{ ServiceName: "prefill", Replicas: ptr.To(int32(3)), - Labels: map[string]string{ - commonconsts.KubeLabelDynamoWorkerHash: "1b69c0d3", - }, + Labels: map[string]string{commonconsts.KubeLabelDynamoWorkerHash: "1b69c0d3"}, }, }, Status: v1alpha1.DynamoComponentDeploymentStatus{ @@ -4337,7 +4331,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, }, { - name: "multiple services - fresh v2 DCDs partially ready", + name: "multiple services - some DCDs ready, some not ready", dgdSpec: v1alpha1.DynamoGraphDeploymentSpec{ BackendFramework: "vllm", Services: map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ @@ -4361,9 +4355,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, }, }, - dgdAnnotations: map[string]string{ - commonconsts.AnnotationCurrentWorkerHashV2: "1b69c0d3", - }, + dgdAnnotations: map[string]string{commonconsts.AnnotationCurrentWorkerHashV2: "1b69c0d3"}, existingDCDs: []client.Object{ betaDCD(t, &v1alpha1.DynamoComponentDeployment{ ObjectMeta: metav1.ObjectMeta{ @@ -4404,9 +4396,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { DynamoComponentDeploymentSharedSpec: v1alpha1.DynamoComponentDeploymentSharedSpec{ ServiceName: "decode", Replicas: ptr.To(int32(2)), - Labels: map[string]string{ - commonconsts.KubeLabelDynamoWorkerHash: "1b69c0d3", - }, + Labels: map[string]string{commonconsts.KubeLabelDynamoWorkerHash: "1b69c0d3"}, }, }, Status: v1alpha1.DynamoComponentDeploymentStatus{ @@ -4436,9 +4426,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { DynamoComponentDeploymentSharedSpec: v1alpha1.DynamoComponentDeploymentSharedSpec{ ServiceName: "prefill", Replicas: ptr.To(int32(3)), - Labels: map[string]string{ - commonconsts.KubeLabelDynamoWorkerHash: "1b69c0d3", - }, + Labels: map[string]string{commonconsts.KubeLabelDynamoWorkerHash: "1b69c0d3"}, }, }, Status: v1alpha1.DynamoComponentDeploymentStatus{ @@ -4510,6 +4498,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { }, }, }, + dgdAnnotations: map[string]string{commonconsts.AnnotationCurrentWorkerHashV2: "cabcd5c9"}, existingDCDs: []client.Object{ betaDCD(t, &v1alpha1.DynamoComponentDeployment{ ObjectMeta: metav1.ObjectMeta{ @@ -4550,6 +4539,7 @@ func Test_reconcileDynamoComponentsDeployments(t *testing.T) { DynamoComponentDeploymentSharedSpec: v1alpha1.DynamoComponentDeploymentSharedSpec{ ServiceName: "decode", Replicas: ptr.To(int32(2)), + Labels: map[string]string{commonconsts.KubeLabelDynamoWorkerHash: "cabcd5c9"}, }, }, Status: v1alpha1.DynamoComponentDeploymentStatus{ diff --git a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go index 9a8d0037dc3d..b455638b9f8a 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go +++ b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate.go @@ -136,8 +136,9 @@ func (r *DynamoGraphDeploymentReconciler) workerHashesForUnsupportedPathway( // generation recorded on the DGD. // // During v1/v2 compatibility a worker DCD is current if its worker-hash label -// matches either the active generation in current-worker-hash or its v2 -// fingerprint in current-worker-hash-v2. +// matches either current-worker-hash (v1) or current-worker-hash-v2. This keeps +// the existing annotation/label meaning downgrade-safe while allowing the +// controller to record the v2 hash that will become primary later. func (r *DynamoGraphDeploymentReconciler) shouldTriggerRollingUpdate( dgd *nvidiacomv1beta1.DynamoGraphDeployment, ) (bool, error) { @@ -217,8 +218,8 @@ func (r *DynamoGraphDeploymentReconciler) initializeWorkerHashIfNeeded( return nil } -// migrateCurrentWorkerHashIfNeeded completes partially persisted 1.2 hash state -// without rolling workers. +// migrateCurrentWorkerHashIfNeeded fills in additive v2 worker-hash state while +// the v1 hash still represents the active worker generation. func (r *DynamoGraphDeploymentReconciler) migrateCurrentWorkerHashIfNeeded( ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment, @@ -226,7 +227,10 @@ func (r *DynamoGraphDeploymentReconciler) migrateCurrentWorkerHashIfNeeded( logger := log.FromContext(ctx) current := r.currentWorkerHashes(dgd) - if current.empty() || current.v1 == consts.LegacyWorkerHash { + if current.empty() { + return nil + } + if current.v1 == consts.LegacyWorkerHash { return nil } @@ -236,35 +240,35 @@ func (r *DynamoGraphDeploymentReconciler) migrateCurrentWorkerHashIfNeeded( } var next workerGenerationHashes + var eventMessage string switch { - case current.v1 != "" && current.v2 == "" && current.v1 != desired.v2: - v1Hash, err := dynamo.ComputeLegacyAlphaDGDWorkersSpecHash(dgd) - if err != nil { - return fmt.Errorf("failed to compute v1 worker hash: %w", err) - } - if current.v1 != v1Hash { - return nil - } - next = workerGenerationHashes{v1: current.v1, v2: desired.v2} + case current.v1 == desired.v1 && current.v2 == "" && current.v1 != desired.v2: + next = current + next.v2 = desired.v2 + eventMessage = "Recorded compatible v1 and v2 worker hash annotations without rolling workers" default: return nil } + if next == current { + return nil + } r.setCurrentWorkerHashes(dgd, next) if err := r.Update(ctx, dgd); err != nil { return fmt.Errorf("failed to migrate worker hash annotations: %w", err) } - logger.Info("Migrated worker hash annotations", "v1Hash", next.v1, "v2Hash", next.v2) - r.Recorder.Event(dgd, corev1.EventTypeNormal, "WorkerHashMigrated", - "Completed worker hash migration without rolling workers") + logger.Info("Migrated worker hash annotations", + "v1Hash", next.v1, + "v2Hash", next.v2) + r.Recorder.Event(dgd, corev1.EventTypeNormal, "WorkerHashMigrated", eventMessage) return nil } // activeWorkerHashForDCDGeneration returns the hash used for generated worker // DCD names and worker-hash labels in this reconcile. Existing bridge generations -// keep their v1 identity until a worker change selects v2; canonical generations -// continue directly from one v2 identity to the next. +// keep their v1 identity until a worker change selects v2. Already v2-labeled +// generations preserve that value. func (r *DynamoGraphDeploymentReconciler) activeWorkerHashForDCDGeneration( dgd *nvidiacomv1beta1.DynamoGraphDeployment, desired workerGenerationHashes, @@ -351,8 +355,9 @@ func (r *DynamoGraphDeploymentReconciler) getCurrentWorkerHashV2( return dgd.Annotations[consts.AnnotationCurrentWorkerHashV2] } -// setCurrentWorkerHashes stores the v1 and v2 generation values on the DGD. -// A bridged generation has both; a canonical v2 generation has only v2. +// setCurrentWorkerHashes stores the active worker hashes for one generation. +// Empty fields are deleted, which is how v2-only generations intentionally drop +// the downgrade-compatible v1 annotation. func (r *DynamoGraphDeploymentReconciler) setCurrentWorkerHashes( dgd *nvidiacomv1beta1.DynamoGraphDeployment, hashes workerGenerationHashes, diff --git a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go index 456a24ab7ae5..9f1a5635c8b4 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_rollingupdate_test.go @@ -502,11 +502,11 @@ func TestLegacyAlphaHashCompatibility_V2OnlyChangeUsesNewV2Generation(t *testing require.NotEqual(t, newLegacyHash, rollingCtx.NewWorkerHash) require.NoError(t, r.completeRollingUpdate(context.Background(), dgd, newV2Hash)) - require.NotContains(t, dgd.Annotations, consts.AnnotationCurrentWorkerHash) + require.Empty(t, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) require.Equal(t, newV2Hash, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) } -func TestUnsupportedPathwayMigratesV1OnlyAndConvergesToCanonicalV2(t *testing.T) { +func TestUnsupportedPathwayMigratesV1OnlyAndKeepsV2OnlyGeneration(t *testing.T) { dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ "worker": { ComponentType: consts.ComponentTypeWorker, @@ -1525,7 +1525,6 @@ func TestGetExistingRestartAnnotationsDCD(t *testing.T) { ComponentType: consts.ComponentTypeWorker, }, }) - // A spec change selects the new canonical v2 hash instead of the stale current identity. computedHash := betaDGDWorkersSpecHash(t, dgd) dgd.Annotations = map[string]string{ consts.AnnotationCurrentWorkerHash: "oldhash", @@ -1861,6 +1860,14 @@ func TestInitializeWorkerHashIfNeeded_LegacyDCDsMigration(t *testing.T) { require.NoError(t, err) assert.Equal(t, consts.LegacyWorkerHash, updatedDCD.Labels[consts.KubeLabelDynamoWorkerHash], "Legacy DCD should have worker hash label backfilled") + + desired, err := r.desiredWorkerHashes(dgd) + require.NoError(t, err) + require.NoError(t, r.completeRollingUpdate(ctx, dgd, desired.v1)) + + trigger, err := r.shouldTriggerRollingUpdate(dgd) + require.NoError(t, err) + require.False(t, trigger) } func TestInitializeWorkerHashIfNeeded_LegacyMultipleWorkers(t *testing.T) { @@ -3200,26 +3207,6 @@ func TestReconcileRollingUpdate_StuckDetection(t *testing.T) { assert.Equal(t, nvidiacomv1beta1.RollingUpdatePhaseCompleted, dgd.Status.RollingUpdate.Phase) } -func TestLegacyMigrationCompletesWithoutV2Reroll(t *testing.T) { - dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{ - "worker": {ComponentType: consts.ComponentTypeWorker}, - }) - dgd.Annotations = map[string]string{ - consts.AnnotationCurrentWorkerHash: consts.LegacyWorkerHash, - } - r := createTestReconcilerWithStatus(dgd) - - desired, err := r.desiredWorkerHashes(dgd) - require.NoError(t, err) - require.NoError(t, r.completeRollingUpdate(context.Background(), dgd, desired.v1)) - require.Equal(t, desired.v1, dgd.Annotations[consts.AnnotationCurrentWorkerHash]) - require.Equal(t, desired.v2, dgd.Annotations[consts.AnnotationCurrentWorkerHashV2]) - - trigger, err := r.shouldTriggerRollingUpdate(dgd) - require.NoError(t, err) - require.False(t, trigger) -} - func TestReconcileRollingUpdate_NewRollingUpdate(t *testing.T) { newHash := "newhash1" dgd := createTestDGD("test-dgd", map[string]*nvidiacomv1alpha1.DynamoComponentDeploymentSharedSpec{