From 40464ac4b66ea051af241be9716956182ceef311 Mon Sep 17 00:00:00 2001 From: joeltg Date: Thu, 16 Jul 2026 15:23:29 +0000 Subject: [PATCH 1/3] feat(operator): add experimental grove.forceScalingGroup for single-node components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add components[].experimental.grove.forceScalingGroup (bool) to the v1beta1 shared component spec. False or omitted means automatic selection: multi-node and inter-pod GMS components render as a Grove PodCliqueScalingGroup, other single-node components pool their replicas as pods of one standalone PodClique — exactly today's behavior. Setting forceScalingGroup: true opts a single-node component into the PCSG layout: the scaling-group replica count carries the horizontal scale, with a single one-pod PodClique per replica, so each replica is an independently gang-scheduled unit (its own PodGang beyond minAvailable) and scaling stamps out or removes whole PodGangs. - Centralize the previously duplicated "multinode || inter-pod GMS" PCSG decision as DynamoComponentDeploymentSharedSpec.UsesPCSG() (= isMultinode || isInterPodGMS || forceScalingGroup) and use it in PCS rendering, Grove readiness aggregation, restart progress tracking, reconcileGroveScaling, and resource-name-length validation. - Validation: Grove pathway only (same rule as minAvailable); the effective opt-in is immutable after creation (same pattern as the inter-pod GMS layout toggle); explicit false is equivalent to omitted and freely add/removable. - Conversion: the grove block has no v1alpha1 representation; preserve it across spoke round-trips, sparsely when it sits alongside alpha-representable experimental fields. API shape per review discussion in #11774. Co-Authored-By: Claude Fable 5 Signed-off-by: joeltg --- .../conversion_field_coverage_test.go | 2 + ...namocomponentdeployment_conversion_test.go | 42 +++++++++++ .../api/v1alpha1/shared_spec_conversion.go | 9 +++ deploy/operator/api/v1beta1/common.go | 17 +++++ .../dynamocomponentdeployment_types.go | 13 ++++ .../api/v1beta1/zz_generated.deepcopy.go | 20 +++++ ...nvidia.com_dynamocomponentdeployments.yaml | 14 ++++ .../nvidia.com_dynamographdeployments.yaml | 14 ++++ .../dynamographdeployment_controller.go | 11 ++- deploy/operator/internal/dynamo/graph.go | 16 +++- deploy/operator/internal/dynamo/graph_test.go | 73 ++++++++++++++++++ deploy/operator/internal/dynamo/grove.go | 2 +- .../dynamographdeployment_helpers.go | 2 +- .../validation/dynamographdeployment_test.go | 75 +++++++++++++++++++ .../webhook/validation/shared_helpers.go | 6 ++ .../webhook/validation/shared_v1beta1.go | 23 ++++++ docs/kubernetes/api-reference.md | 17 +++++ 17 files changed, 345 insertions(+), 11 deletions(-) diff --git a/deploy/operator/api/v1alpha1/conversion_field_coverage_test.go b/deploy/operator/api/v1alpha1/conversion_field_coverage_test.go index 4f338d8c5eba..f6f045667370 100644 --- a/deploy/operator/api/v1alpha1/conversion_field_coverage_test.go +++ b/deploy/operator/api/v1alpha1/conversion_field_coverage_test.go @@ -58,6 +58,7 @@ DynamoComponentDeploymentSpec.experimental.gpuMemoryService.extraClientContainer DynamoComponentDeploymentSpec.experimental.gpuMemoryService.extraClientPods.name DynamoComponentDeploymentSpec.experimental.gpuMemoryService.extraClientPods.podTemplate DynamoComponentDeploymentSpec.experimental.gpuMemoryService.mode +DynamoComponentDeploymentSpec.experimental.grove.forceScalingGroup DynamoComponentDeploymentSpec.frontendSidecar DynamoComponentDeploymentSpec.globalDynamoNamespace DynamoComponentDeploymentSpec.minAvailable @@ -116,6 +117,7 @@ DynamoGraphDeploymentSpec.components.experimental.gpuMemoryService.extraClientCo DynamoGraphDeploymentSpec.components.experimental.gpuMemoryService.extraClientPods.name DynamoGraphDeploymentSpec.components.experimental.gpuMemoryService.extraClientPods.podTemplate DynamoGraphDeploymentSpec.components.experimental.gpuMemoryService.mode +DynamoGraphDeploymentSpec.components.experimental.grove.forceScalingGroup DynamoGraphDeploymentSpec.components.frontendSidecar DynamoGraphDeploymentSpec.components.globalDynamoNamespace DynamoGraphDeploymentSpec.components.minAvailable diff --git a/deploy/operator/api/v1alpha1/dynamocomponentdeployment_conversion_test.go b/deploy/operator/api/v1alpha1/dynamocomponentdeployment_conversion_test.go index cb2424be08a3..cde9a0f05e00 100644 --- a/deploy/operator/api/v1alpha1/dynamocomponentdeployment_conversion_test.go +++ b/deploy/operator/api/v1alpha1/dynamocomponentdeployment_conversion_test.go @@ -623,6 +623,48 @@ func TestDCD_RoundTrip_Experimental(t *testing.T) { } } +// The grove block has no v1alpha1 representation and must survive the spoke +// round-trip both alone (whole hub-only block preserved) and alongside +// alpha-representable fields (sparse preservation merged back). +func TestDCD_RoundTrip_ExperimentalGrove(t *testing.T) { + tests := []struct { + name string + experimental *v1beta1.ExperimentalSpec + }{ + { + name: "grove.forceScalingGroup only", + experimental: &v1beta1.ExperimentalSpec{ + Grove: &v1beta1.GroveSpec{ForceScalingGroup: true}, + }, + }, + { + name: "grove.forceScalingGroup alongside alpha-representable GMS", + experimental: &v1beta1.ExperimentalSpec{ + GPUMemoryService: &v1beta1.GPUMemoryServiceSpec{Mode: v1beta1.GMSModeIntraPod}, + Grove: &v1beta1.GroveSpec{ForceScalingGroup: true}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + src := &v1beta1.DynamoComponentDeployment{ + ObjectMeta: metav1.ObjectMeta{Name: "exp-grouping", Namespace: "ns"}, + Spec: v1beta1.DynamoComponentDeploymentSpec{ + DynamoComponentDeploymentSharedSpec: v1beta1.DynamoComponentDeploymentSharedSpec{ + ComponentName: "exp-grouping", + ComponentType: v1beta1.ComponentTypeWorker, + Experimental: tt.experimental.DeepCopy(), + }, + }, + } + got := dcdRoundTripFromV1beta1(t, src) + if diff := cmp.Diff(src, got, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("round-trip mismatch (-want +got):\n%s", diff) + } + }) + } +} + func TestDCD_ExperimentalModeValuesAreValidForIntermediateVersion(t *testing.T) { alpha := &DynamoComponentDeployment{ ObjectMeta: metav1.ObjectMeta{Name: "alpha-enums", Namespace: "ns"}, diff --git a/deploy/operator/api/v1alpha1/shared_spec_conversion.go b/deploy/operator/api/v1alpha1/shared_spec_conversion.go index 0f7dfc4781e4..9e3afb4b081d 100644 --- a/deploy/operator/api/v1alpha1/shared_spec_conversion.go +++ b/deploy/operator/api/v1alpha1/shared_spec_conversion.go @@ -573,6 +573,10 @@ func saveSharedHubOnlySpec(src *v1beta1.DynamoComponentDeploymentSharedSpec, con } if experimentalIsHubOnlyShape(src.Experimental) { save.Experimental = src.Experimental.DeepCopy() + } else if src.Experimental != nil && src.Experimental.Grove != nil { + // The grove block has no v1alpha1 representation; preserve it sparsely + // when the rest of the experimental block converts to alpha fields. + save.Experimental = &v1beta1.ExperimentalSpec{Grove: src.Experimental.Grove.DeepCopy()} } return nil } @@ -1595,6 +1599,11 @@ func restoreSharedHubOnlyFields(dst, preserved *v1beta1.DynamoComponentDeploymen restoreSharedHubOnlyFrontendSidecar(dst, preserved) if dst.Experimental == nil && experimentalIsHubOnlyShape(preserved.Experimental) { dst.Experimental = preserved.Experimental.DeepCopy() + } else if dst.Experimental != nil && preserved.Experimental != nil && + dst.Experimental.Grove == nil && preserved.Experimental.Grove != nil { + // The experimental block was rebuilt from alpha fields (GMS, failover, + // checkpoint); merge back the sparsely preserved hub-only grove block. + dst.Experimental.Grove = preserved.Experimental.Grove.DeepCopy() } return nil } diff --git a/deploy/operator/api/v1beta1/common.go b/deploy/operator/api/v1beta1/common.go index b3ba29bd53ac..aa926b39a012 100644 --- a/deploy/operator/api/v1beta1/common.go +++ b/deploy/operator/api/v1beta1/common.go @@ -164,6 +164,19 @@ const ( GMSModeInterPod GPUMemoryServiceMode = "InterPod" ) +// GroveSpec groups experimental Grove-specific rendering options. +type GroveSpec struct { + // forceScalingGroup opts a single-node component into rendering as a + // PodCliqueScalingGroup with one single-pod PodClique per replica, so + // each replica is gang-scheduled independently and scaling changes the + // scaling-group replica count. `false` or omitted means automatic + // selection (multi-node and inter-pod GMS components use a scaling + // group, other single-node components a standalone PodClique), not + // "force PodClique". Immutable after creation. + // +optional + ForceScalingGroup bool `json:"forceScalingGroup,omitempty"` +} + // ExperimentalSpec groups opt-in preview features whose API shape and behavior // may change in breaking ways between v1beta1 releases (including disappearing // without a name-preserving graduation path). Fields placed under @@ -183,6 +196,10 @@ type ExperimentalSpec struct { // +optional Failover *FailoverSpec `json:"failover,omitempty"` + // grove groups Grove-specific rendering options. + // +optional + Grove *GroveSpec `json:"grove,omitempty"` + // checkpoint configures container-image snapshotting and restore for // this component. Set `checkpoint.enabled: true` to opt in. Without // checkpointRef, the DGD controller creates a DGD-scoped DynamoCheckpoint diff --git a/deploy/operator/api/v1beta1/dynamocomponentdeployment_types.go b/deploy/operator/api/v1beta1/dynamocomponentdeployment_types.go index ce9644d8336f..fe437fc7bbc2 100644 --- a/deploy/operator/api/v1beta1/dynamocomponentdeployment_types.go +++ b/deploy/operator/api/v1beta1/dynamocomponentdeployment_types.go @@ -319,6 +319,19 @@ func (s *DynamoComponentDeploymentSharedSpec) IsInterPodGMSEnabled() bool { s.Experimental.GPUMemoryService.Mode == GMSModeInterPod } +// IsGroveScalingGroupForced reports whether the ScalingGroup layout is explicitly requested. +func (s *DynamoComponentDeploymentSharedSpec) IsGroveScalingGroupForced() bool { + return s.Experimental != nil && + s.Experimental.Grove != nil && + s.Experimental.Grove.ForceScalingGroup +} + +// UsesPCSG reports whether Grove renders this component as a +// PodCliqueScalingGroup rather than a standalone PodClique. +func (s *DynamoComponentDeploymentSharedSpec) UsesPCSG() bool { + return s.GetNumberOfNodes() > 1 || s.IsInterPodGMSEnabled() || s.IsGroveScalingGroupForced() +} + // IsInterPodFailoverEnabled reports whether inter-pod GMS failover is configured. func (s *DynamoComponentDeploymentSharedSpec) IsInterPodFailoverEnabled() bool { return s.Experimental != nil && diff --git a/deploy/operator/api/v1beta1/zz_generated.deepcopy.go b/deploy/operator/api/v1beta1/zz_generated.deepcopy.go index b9beb2e0c91c..0aede7914205 100644 --- a/deploy/operator/api/v1beta1/zz_generated.deepcopy.go +++ b/deploy/operator/api/v1beta1/zz_generated.deepcopy.go @@ -860,6 +860,11 @@ func (in *ExperimentalSpec) DeepCopyInto(out *ExperimentalSpec) { *out = new(FailoverSpec) **out = **in } + if in.Grove != nil { + in, out := &in.Grove, &out.Grove + *out = new(GroveSpec) + **out = **in + } if in.Checkpoint != nil { in, out := &in.Checkpoint, &out.Checkpoint *out = new(ComponentCheckpointConfig) @@ -960,6 +965,21 @@ func (in *GPUMemoryServiceSpec) DeepCopy() *GPUMemoryServiceSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GroveSpec) DeepCopyInto(out *GroveSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GroveSpec. +func (in *GroveSpec) DeepCopy() *GroveSpec { + if in == nil { + return nil + } + out := new(GroveSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HardwareSpec) DeepCopyInto(out *HardwareSpec) { *out = *in diff --git a/deploy/operator/config/crd/bases/nvidia.com_dynamocomponentdeployments.yaml b/deploy/operator/config/crd/bases/nvidia.com_dynamocomponentdeployments.yaml index ca49a34cba96..a76abcfb8d12 100644 --- a/deploy/operator/config/crd/bases/nvidia.com_dynamocomponentdeployments.yaml +++ b/deploy/operator/config/crd/bases/nvidia.com_dynamocomponentdeployments.yaml @@ -12176,6 +12176,20 @@ spec: rule: '!has(self.extraClientPods) || size(self.extraClientPods) == 0 || self.mode == ''InterPod''' - message: extraClientPods is reserved for inter-pod GMS and is not implemented yet rule: '!has(self.extraClientPods) || size(self.extraClientPods) == 0' + grove: + description: grove groups Grove-specific rendering options. + properties: + forceScalingGroup: + description: |- + forceScalingGroup opts a single-node component into rendering as a + PodCliqueScalingGroup with one single-pod PodClique per replica, so + each replica is gang-scheduled independently and scaling changes the + scaling-group replica count. `false` or omitted means automatic + selection (multi-node and inter-pod GMS components use a scaling + group, other single-node components a standalone PodClique), not + "force PodClique". Immutable after creation. + type: boolean + type: object type: object frontendSidecar: description: |- diff --git a/deploy/operator/config/crd/bases/nvidia.com_dynamographdeployments.yaml b/deploy/operator/config/crd/bases/nvidia.com_dynamographdeployments.yaml index fd90a86359c0..36716e4e9814 100644 --- a/deploy/operator/config/crd/bases/nvidia.com_dynamographdeployments.yaml +++ b/deploy/operator/config/crd/bases/nvidia.com_dynamographdeployments.yaml @@ -12613,6 +12613,20 @@ spec: rule: '!has(self.extraClientPods) || size(self.extraClientPods) == 0 || self.mode == ''InterPod''' - message: extraClientPods is reserved for inter-pod GMS and is not implemented yet rule: '!has(self.extraClientPods) || size(self.extraClientPods) == 0' + grove: + description: grove groups Grove-specific rendering options. + properties: + forceScalingGroup: + description: |- + forceScalingGroup opts a single-node component into rendering as a + PodCliqueScalingGroup with one single-pod PodClique per replica, so + each replica is gang-scheduled independently and scaling changes the + scaling-group replica count. `false` or omitted means automatic + selection (multi-node and inter-pod GMS components use a scaling + group, other single-node components a standalone PodClique), not + "force PodClique". Immutable after creation. + type: boolean + type: object type: object frontendSidecar: description: |- diff --git a/deploy/operator/internal/controller/dynamographdeployment_controller.go b/deploy/operator/internal/controller/dynamographdeployment_controller.go index 55164a744740..e7505c17ece7 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_controller.go +++ b/deploy/operator/internal/controller/dynamographdeployment_controller.go @@ -511,11 +511,10 @@ func (r *DynamoGraphDeploymentReconciler) getUpdatedInProgressForGrove(ctx conte var isReady bool var reason string // Keep in sync with reconcileGroveScaling and Grove status aggregation: - // any component that requires a PodCliqueScalingGroup (multinode or - // inter-pod GMS) must be queried via CheckPCSGReady, otherwise - // single-node GMS components stall in the in-progress list because the - // corresponding PodClique never exists. - usesPCSG := component.GetNumberOfNodes() > 1 || component.IsInterPodGMSEnabled() + // any component that requires a PodCliqueScalingGroup must be queried + // via CheckPCSGReady, otherwise PCSG-backed components stall in the + // in-progress list because the corresponding PodClique never exists. + usesPCSG := component.UsesPCSG() // A transient read error surfaces here as isReady=false, so the component // conservatively remains in-progress (we never mark a restart complete on // a component we could not read). The reconcile that owns readiness @@ -999,7 +998,7 @@ func (r *DynamoGraphDeploymentReconciler) reconcileGroveScaling( replicas = 0 } - usesPCSG := component.GetNumberOfNodes() > 1 || component.IsInterPodGMSEnabled() + usesPCSG := component.UsesPCSG() resourceName := fmt.Sprintf("%s-%d-%s", pcsName, replicaIndex, strings.ToLower(componentName)) if usesPCSG { diff --git a/deploy/operator/internal/dynamo/graph.go b/deploy/operator/internal/dynamo/graph.go index 6115c729dbd5..d8ec5d9c486f 100644 --- a/deploy/operator/internal/dynamo/graph.go +++ b/deploy/operator/internal/dynamo/graph.go @@ -1105,6 +1105,8 @@ const ( // multiple ServiceRoles depending on the deployment topology: // // - single-node, no GMS: 1 role (RoleMain) +// - single-node, forceScalingGroup: 1 role (RoleMain with a single pod; +// the PCSG replica count carries the horizontal scale) // - multinode, no GMS: 2 roles (RoleLeader + RoleWorker) // - single-node, inter-pod GMS: 1 engine PCLQ (replicated) + 1 RoleGMS // weight-server PCLQ @@ -1162,6 +1164,8 @@ func expandRolesForComponent(componentName string, componentReplicas *int32, num return expandMultinodeRoles(componentName, numberOfNodes) case isInterPodGMS: return expandSingleNodeGMSRoles(componentName, component.GetTotalEnginePods()) + case component.IsGroveScalingGroupForced(): + return expandSingleNodeScalingGroupRoles(componentName) default: return expandSingleNodeRoles(componentName, componentReplicas) } @@ -1177,6 +1181,12 @@ func expandSingleNodeRoles(componentName string, componentReplicas *int32) []Ser } } +func expandSingleNodeScalingGroupRoles(componentName string) []ServiceRole { + return []ServiceRole{ + {Name: componentName, Role: RoleMain, Replicas: 1}, + } +} + func expandMultinodeRoles(componentName string, numberOfNodes int32) []ServiceRole { return []ServiceRole{ {Name: componentName + "-" + commonconsts.GroveRoleSuffixLeader, Role: RoleLeader, Replicas: 1}, @@ -1218,7 +1228,7 @@ func LongestPodCliqueNameForDGDComponent( component *v1beta1.DynamoComponentDeploymentSharedSpec, ) string { lowerComponentName := strings.ToLower(componentName) - if component == nil || (component.GetNumberOfNodes() <= 1 && !component.IsInterPodGMSEnabled()) { + if component == nil || !component.UsesPCSG() { return lowerComponentName } @@ -1245,7 +1255,7 @@ func PCSNameForDGD(dgdName string, components []v1beta1.DynamoComponentDeploymen componentName := component.ComponentName lowerName := strings.ToLower(componentName) var budget int - if component.GetNumberOfNodes() > 1 || component.IsInterPodGMSEnabled() { + if component.UsesPCSG() { // PCSG = lowerName, PCLQ = longest rendered role name. budget = len(lowerName) + len(LongestPodCliqueNameForDGDComponent(componentName, component)) } else { @@ -2438,7 +2448,7 @@ func GenerateGrovePodCliqueSet( isMultinode := numberOfNodes > 1 isInterPodGMS := component.IsInterPodGMSEnabled() isInterPodFailover := component.IsInterPodFailoverEnabled() - usesPCSG := isMultinode || isInterPodGMS + usesPCSG := component.UsesPCSG() roles := expandRolesForComponent(componentName, component.Replicas, numberOfNodes, component) var cliqueNames []string diff --git a/deploy/operator/internal/dynamo/graph_test.go b/deploy/operator/internal/dynamo/graph_test.go index 0beab4518643..5afd723e3595 100644 --- a/deploy/operator/internal/dynamo/graph_test.go +++ b/deploy/operator/internal/dynamo/graph_test.go @@ -4961,6 +4961,23 @@ func TestExpandRolesForService(t *testing.T) { } } +// forceScalingGroup is v1beta1-only, so it gets its own case instead of a +// row in the alpha-shaped table above: the engine PCLQ holds one pod +// regardless of the component replica count (the PCSG carries the scale). +func TestExpandRolesForComponent_SingleNodeForceScalingGroup(t *testing.T) { + component := &v1beta1.DynamoComponentDeploymentSharedSpec{ + Replicas: ptr.To(int32(4)), + Experimental: &v1beta1.ExperimentalSpec{ + Grove: &v1beta1.GroveSpec{ForceScalingGroup: true}, + }, + } + got := expandRolesForComponent("svc", component.Replicas, 1, component) + want := []ServiceRole{{Name: "svc", Role: RoleMain, Replicas: 1}} + if !reflect.DeepEqual(got, want) { + t.Errorf("expandRolesForComponent() = %v, want %v", got, want) + } +} + func TestRoleEnum(t *testing.T) { // Test that role constants are defined correctly if RoleLeader != "leader" { @@ -8652,6 +8669,62 @@ func TestGenerateGrovePodCliqueSet_ComponentMinAvailable(t *testing.T) { } } +// TestGenerateGrovePodCliqueSet_SingleNodeForceScalingGroup pins the +// experimental grove.forceScalingGroup opt-in: a single-node component +// renders as a PCSG whose replica count carries the horizontal scale, with a +// single one-pod engine PCLQ per PCSG replica. +func TestGenerateGrovePodCliqueSet_SingleNodeForceScalingGroup(t *testing.T) { + dgd := &v1alpha1.DynamoGraphDeployment{ + ObjectMeta: metav1.ObjectMeta{Name: "test-dgd", Namespace: "test-ns"}, + Spec: v1alpha1.DynamoGraphDeploymentSpec{ + BackendFramework: "vllm", + Services: map[string]*v1alpha1.DynamoComponentDeploymentSharedSpec{ + "worker": { + ComponentType: commonconsts.ComponentTypeWorker, + Replicas: ptr.To(int32(4)), + MinAvailable: ptr.To(int32(2)), + Resources: &v1alpha1.Resources{ + Limits: &v1alpha1.ResourceItem{GPU: "1"}, + }, + }, + }, + }, + } + + // grove.forceScalingGroup is v1beta1-only, so set it after conversion. + beta := betaDGD(t, dgd) + require.Len(t, beta.Spec.Components, 1) + beta.Spec.Components[0].Experimental = &v1beta1.ExperimentalSpec{ + Grove: &v1beta1.GroveSpec{ForceScalingGroup: true}, + } + + got, err := GenerateGrovePodCliqueSet( + context.Background(), + beta, + &configv1alpha1.OperatorConfiguration{}, + &controller_common.RuntimeConfig{}, + nil, nil, nil, nil, nil, + ) + require.NoError(t, err) + require.NotNil(t, got) + + require.Len(t, got.Spec.Template.Cliques, 1) + clique := got.Spec.Template.Cliques[0] + assert.Equal(t, "worker", clique.Name) + assert.EqualValues(t, 1, clique.Spec.Replicas, "engine PCLQ must hold exactly one pod per PCSG replica") + require.NotNil(t, clique.Spec.MinAvailable) + assert.EqualValues(t, 1, *clique.Spec.MinAvailable) + + require.Len(t, got.Spec.Template.PodCliqueScalingGroupConfigs, 1) + pcsg := got.Spec.Template.PodCliqueScalingGroupConfigs[0] + assert.Equal(t, "worker", pcsg.Name) + assert.Equal(t, []string{"worker"}, pcsg.CliqueNames) + require.NotNil(t, pcsg.Replicas) + assert.EqualValues(t, 4, *pcsg.Replicas, "PCSG replicas must carry the component replica count") + require.NotNil(t, pcsg.MinAvailable) + assert.EqualValues(t, 2, *pcsg.MinAvailable) +} + // TestGenerateGrovePodCliqueSet_MinAvailable_FailoverShadowsAreRedundant pins // the contract that per-rank engine cliques in an inter-pod failover cohort // use MinAvailable=1 even when multinode (numberOfNodes > 1). Replicas here diff --git a/deploy/operator/internal/dynamo/grove.go b/deploy/operator/internal/dynamo/grove.go index 0c1b6566a1cd..fbb25626ed97 100644 --- a/deploy/operator/internal/dynamo/grove.go +++ b/deploy/operator/internal/dynamo/grove.go @@ -118,7 +118,7 @@ func evaluateGroveComponents(ctx context.Context, client client.Client, dgd *v1b for i := range dgd.Spec.Components { component := &dgd.Spec.Components[i] componentName := component.ComponentName - usesPCSG := component.GetNumberOfNodes() > 1 || component.IsInterPodGMSEnabled() + usesPCSG := component.UsesPCSG() resourceName := fmt.Sprintf("%s-0-%s", PCSNameForDGD(dgd.Name, dgd.Spec.Components), strings.ToLower(componentName)) var ok bool diff --git a/deploy/operator/internal/webhook/validation/dynamographdeployment_helpers.go b/deploy/operator/internal/webhook/validation/dynamographdeployment_helpers.go index bcdf39d195fd..901e19ccbd6a 100644 --- a/deploy/operator/internal/webhook/validation/dynamographdeployment_helpers.go +++ b/deploy/operator/internal/webhook/validation/dynamographdeployment_helpers.go @@ -159,7 +159,7 @@ func dgdComponentResourceNameLength( combinedLength := len(pcsName) + len(strings.ToLower(componentName)) detail := "PCS name + component name" - if component.GetNumberOfNodes() > 1 || component.IsInterPodGMSEnabled() { + if component.UsesPCSG() { longestPodCliqueName := dynamo.LongestPodCliqueNameForDGDComponent(componentName, component) combinedLength += len(longestPodCliqueName) detail = fmt.Sprintf("PCS name + PCSG name + longest PodClique name %q", longestPodCliqueName) diff --git a/deploy/operator/internal/webhook/validation/dynamographdeployment_test.go b/deploy/operator/internal/webhook/validation/dynamographdeployment_test.go index d77626e5ed83..f4e4f63304a6 100644 --- a/deploy/operator/internal/webhook/validation/dynamographdeployment_test.go +++ b/deploy/operator/internal/webhook/validation/dynamographdeployment_test.go @@ -426,6 +426,36 @@ func TestDynamoGraphDeploymentValidator_Validate(t *testing.T) { wantCELErr: "spec.components[1]: Invalid value: minAvailable is immutable after creation", }, + // Grove forceScalingGroup rules. + { + name: "component grove.forceScalingGroup requires Grove", + groveDisabled: true, + deployment: betaDGDForAdmission(func(dgd *nvidiacomv1beta1.DynamoGraphDeployment) { + betaWorkerComponent(dgd).Experimental = &nvidiacomv1beta1.ExperimentalSpec{ + Grove: &nvidiacomv1beta1.GroveSpec{ForceScalingGroup: true}, + } + }), + wantWebhookErrs: []string{"spec.components[1].experimental.grove.forceScalingGroup: Forbidden: is currently supported only for Grove-backed DynamoGraphDeployment components"}, + }, + { + name: "v1beta1 grove.forceScalingGroup on a single-node component reaches the webhook", + deployment: betaDGDForAdmission(func(dgd *nvidiacomv1beta1.DynamoGraphDeployment) { + betaWorkerComponent(dgd).Experimental = &nvidiacomv1beta1.ExperimentalSpec{ + Grove: &nvidiacomv1beta1.GroveSpec{ForceScalingGroup: true}, + } + }), + }, + { + name: "v1beta1 redundant grove.forceScalingGroup on a multinode component reaches the webhook", + deployment: betaDGDForAdmission(func(dgd *nvidiacomv1beta1.DynamoGraphDeployment) { + worker := betaWorkerComponent(dgd) + worker.Multinode = &nvidiacomv1beta1.MultinodeSpec{NodeCount: 2} + worker.Experimental = &nvidiacomv1beta1.ExperimentalSpec{ + Grove: &nvidiacomv1beta1.GroveSpec{ForceScalingGroup: true}, + } + }), + }, + // Checkpoint rules. { name: "v1beta1 valid checkpoint configuration reaches the webhook", @@ -1452,6 +1482,51 @@ func TestDynamoGraphDeploymentValidator_Validate(t *testing.T) { "spec.components[1].experimental.failover: Invalid value: null: inter-pod GMS failover cannot be toggled after creation; delete and recreate the DynamoGraphDeployment", }, }, + // Grove forceScalingGroup updates. + { + name: "grove.forceScalingGroup addition is immutable", + oldDeployment: newBetaDGDForValidation(), + deployment: betaDGDWithWorker(func(worker *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec) { + worker.Experimental = &nvidiacomv1beta1.ExperimentalSpec{ + Grove: &nvidiacomv1beta1.GroveSpec{ForceScalingGroup: true}, + } + }), + wantWebhookErrs: []string{"spec.components[1].experimental.grove.forceScalingGroup: Invalid value: true: cannot be toggled after creation; delete and recreate the DynamoGraphDeployment to change it"}, + }, + { + name: "grove.forceScalingGroup removal is immutable", + oldDeployment: betaDGDWithWorker(func(worker *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec) { + worker.Experimental = &nvidiacomv1beta1.ExperimentalSpec{ + Grove: &nvidiacomv1beta1.GroveSpec{ForceScalingGroup: true}, + } + }), + deployment: newBetaDGDForValidation(), + wantWebhookErrs: []string{"spec.components[1].experimental.grove.forceScalingGroup: Invalid value: null: cannot be toggled after creation; delete and recreate the DynamoGraphDeployment to change it"}, + }, + { + name: "unchanged grove.forceScalingGroup update reaches the webhook", + oldDeployment: betaDGDWithWorker(func(worker *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec) { + worker.Experimental = &nvidiacomv1beta1.ExperimentalSpec{ + Grove: &nvidiacomv1beta1.GroveSpec{ForceScalingGroup: true}, + } + }), + deployment: betaDGDWithWorker(func(worker *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec) { + worker.Experimental = &nvidiacomv1beta1.ExperimentalSpec{ + Grove: &nvidiacomv1beta1.GroveSpec{ForceScalingGroup: true}, + } + }), + }, + { + name: "explicit false grove.forceScalingGroup addition reaches the webhook", + oldDeployment: betaDGDWithWorker(func(worker *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec) { + worker.Experimental = &nvidiacomv1beta1.ExperimentalSpec{} + }), + deployment: betaDGDWithWorker(func(worker *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec) { + worker.Experimental = &nvidiacomv1beta1.ExperimentalSpec{ + Grove: &nvidiacomv1beta1.GroveSpec{ForceScalingGroup: false}, + } + }), + }, { name: "inter-pod failover shadow count is immutable", oldDeployment: alphaDGDForAdmission(func(dgd *nvidiacomv1alpha1.DynamoGraphDeployment) { diff --git a/deploy/operator/internal/webhook/validation/shared_helpers.go b/deploy/operator/internal/webhook/validation/shared_helpers.go index 0313eca7454d..56e5a4afeb3d 100644 --- a/deploy/operator/internal/webhook/validation/shared_helpers.go +++ b/deploy/operator/internal/webhook/validation/shared_helpers.go @@ -98,6 +98,12 @@ func failoverForExperimental(experimental *nvidiacomv1beta1.ExperimentalSpec) *n return experimental.Failover } +func forceScalingGroupFor(experimental *nvidiacomv1beta1.ExperimentalSpec) bool { + return experimental != nil && + experimental.Grove != nil && + experimental.Grove.ForceScalingGroup +} + func effectiveGMSMode(mode nvidiacomv1beta1.GPUMemoryServiceMode) nvidiacomv1beta1.GPUMemoryServiceMode { if mode == "" { return nvidiacomv1beta1.GMSModeIntraPod diff --git a/deploy/operator/internal/webhook/validation/shared_v1beta1.go b/deploy/operator/internal/webhook/validation/shared_v1beta1.go index 958b26f53db2..efb6bd871b17 100644 --- a/deploy/operator/internal/webhook/validation/shared_v1beta1.go +++ b/deploy/operator/internal/webhook/validation/shared_v1beta1.go @@ -65,6 +65,12 @@ func (v *sharedValidation) validateDynamoComponentDeploymentSharedSpec( "is currently supported only for Grove-backed DynamoGraphDeployment components", )) } + if forceScalingGroupFor(spec.Experimental) && !grovePathway { + allErrs = append(allErrs, field.Forbidden( + fldPath.Child("experimental", "grove", "forceScalingGroup"), + "is currently supported only for Grove-backed DynamoGraphDeployment components", + )) + } if spec.SharedMemorySize != nil && spec.SharedMemorySize.Sign() < 0 { allErrs = append(allErrs, field.Invalid( fldPath.Child("sharedMemorySize"), @@ -384,6 +390,13 @@ func (v *sharedValidation) validateDynamoComponentDeploymentSharedSpecUpdate( fmt.Sprintf("inter-pod GMS failover cannot be toggled after creation; delete and recreate the %s", ownerKind.Kind), )) } + if forceScalingGroupFor(oldComponent.Experimental) { + allErrs = append(allErrs, field.Invalid( + fldPath.Child("experimental", "grove", "forceScalingGroup"), + nil, + fmt.Sprintf("cannot be toggled after creation; delete and recreate the %s to change it", ownerKind.Kind), + )) + } } return allErrs } @@ -442,5 +455,15 @@ func (v *sharedValidation) validateExperimentalSpecUpdate( fmt.Sprintf("is immutable for inter-pod GMS failover; delete and recreate the %s to change it", ownerKind.Kind), )) } + + // false and omitted both mean automatic selection, so only the + // effective opt-in is immutable. + if forceScalingGroupFor(newExperimental) != forceScalingGroupFor(oldExperimental) { + allErrs = append(allErrs, field.Invalid( + fldPath.Child("grove", "forceScalingGroup"), + forceScalingGroupFor(newExperimental), + fmt.Sprintf("cannot be toggled after creation; delete and recreate the %s to change it", ownerKind.Kind), + )) + } return allErrs } diff --git a/docs/kubernetes/api-reference.md b/docs/kubernetes/api-reference.md index c5fd2036e848..3998c81e2109 100644 --- a/docs/kubernetes/api-reference.md +++ b/docs/kubernetes/api-reference.md @@ -2418,6 +2418,7 @@ _Appears in:_ | --- | --- | --- | --- | | `gpuMemoryService` _[GPUMemoryServiceSpec](#gpumemoryservicespec)_ | gpuMemoryService configures the GPU Memory Service (GMS). When set, GPU
access for GMS clients is managed via DRA. | | Optional: \{\}
| | `failover` _[FailoverSpec](#failoverspec)_ | failover configures active-passive GPU failover for this component.
Requires `gpuMemoryService` to also be set, and `failover.mode` must
match `gpuMemoryService.mode` (enforced by the validation webhook). | | Optional: \{\}
| +| `grove` _[GroveSpec](#grovespec)_ | grove groups Grove-specific rendering options. | | Optional: \{\}
| | `checkpoint` _[ComponentCheckpointConfig](#componentcheckpointconfig)_ | checkpoint configures container-image snapshotting and restore for
this component. Set `checkpoint.enabled: true` to opt in. Without
checkpointRef, the DGD controller creates a DGD-scoped DynamoCheckpoint
CR and later restores pods in the same DGD generation from that
checkpoint. With checkpointRef, the DGD restores from that existing
checkpoint instead. The user-facing shape of this field is still settling,
which is why it lives under `experimental` in v1beta1 instead of at the
top level. | | Optional: \{\}
| @@ -2551,6 +2552,22 @@ _Appears in:_ | `mi300` | | +#### GroveSpec + + + +GroveSpec groups experimental Grove-specific rendering options. + + + +_Appears in:_ +- [ExperimentalSpec](#experimentalspec) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `forceScalingGroup` _boolean_ | forceScalingGroup opts a single-node component into rendering as a
PodCliqueScalingGroup with one single-pod PodClique per replica, so
each replica is gang-scheduled independently and scaling changes the
scaling-group replica count. `false` or omitted means automatic
selection (multi-node and inter-pod GMS components use a scaling
group, other single-node components a standalone PodClique), not
"force PodClique". Immutable after creation. | | Optional: \{\}
| + + #### HardwareSpec From 58d678ebb25005156fdc0c968afada27cd88470e Mon Sep 17 00:00:00 2001 From: joeltg Date: Wed, 22 Jul 2026 02:13:15 +0000 Subject: [PATCH 2/3] refactor(operator): traverse GroveSpec structurally, qualify PodGang independence in docs Address review feedback on the grove.forceScalingGroup PR: - Add validateGroveSpec / validateGroveSpecUpdate and call them from the experimental validators in declaration order, passing grovePathway as ancestor context, instead of reaching through ExperimentalSpec from the shared-spec validator (per STRUCTURAL_VALIDATION.md). - Qualify the forceScalingGroup CRD description: the first minAvailable replicas share the base PodGang; only replicas beyond minAvailable get independent PodGangs, so full independence requires minAvailable: 1. Co-Authored-By: Claude Fable 5 Signed-off-by: joeltg --- deploy/operator/api/v1beta1/common.go | 15 +++-- ...nvidia.com_dynamocomponentdeployments.yaml | 15 +++-- .../nvidia.com_dynamographdeployments.yaml | 15 +++-- ...graphdeployment_validation_envtest_test.go | 12 ++++ .../webhook/validation/shared_helpers.go | 12 +++- .../webhook/validation/shared_test.go | 1 + .../webhook/validation/shared_v1beta1.go | 65 ++++++++++++++++--- docs/kubernetes/api-reference.md | 2 +- 8 files changed, 105 insertions(+), 32 deletions(-) diff --git a/deploy/operator/api/v1beta1/common.go b/deploy/operator/api/v1beta1/common.go index aa926b39a012..63098fab25d5 100644 --- a/deploy/operator/api/v1beta1/common.go +++ b/deploy/operator/api/v1beta1/common.go @@ -167,12 +167,15 @@ const ( // GroveSpec groups experimental Grove-specific rendering options. type GroveSpec struct { // forceScalingGroup opts a single-node component into rendering as a - // PodCliqueScalingGroup with one single-pod PodClique per replica, so - // each replica is gang-scheduled independently and scaling changes the - // scaling-group replica count. `false` or omitted means automatic - // selection (multi-node and inter-pod GMS components use a scaling - // group, other single-node components a standalone PodClique), not - // "force PodClique". Immutable after creation. + // PodCliqueScalingGroup with one single-pod PodClique per replica. + // Scaling changes the scaling-group replica count; the first + // `minAvailable` replicas share the base PodGang, and replicas beyond + // `minAvailable` are gang-scheduled independently in their own + // PodGangs (set `minAvailable: 1` for fully independent replicas). + // `false` or omitted means automatic selection (multi-node and + // inter-pod GMS components use a scaling group, other single-node + // components a standalone PodClique), not "force PodClique". + // Immutable after creation. // +optional ForceScalingGroup bool `json:"forceScalingGroup,omitempty"` } diff --git a/deploy/operator/config/crd/bases/nvidia.com_dynamocomponentdeployments.yaml b/deploy/operator/config/crd/bases/nvidia.com_dynamocomponentdeployments.yaml index a76abcfb8d12..430f4eeeecf5 100644 --- a/deploy/operator/config/crd/bases/nvidia.com_dynamocomponentdeployments.yaml +++ b/deploy/operator/config/crd/bases/nvidia.com_dynamocomponentdeployments.yaml @@ -12182,12 +12182,15 @@ spec: forceScalingGroup: description: |- forceScalingGroup opts a single-node component into rendering as a - PodCliqueScalingGroup with one single-pod PodClique per replica, so - each replica is gang-scheduled independently and scaling changes the - scaling-group replica count. `false` or omitted means automatic - selection (multi-node and inter-pod GMS components use a scaling - group, other single-node components a standalone PodClique), not - "force PodClique". Immutable after creation. + PodCliqueScalingGroup with one single-pod PodClique per replica. + Scaling changes the scaling-group replica count; the first + `minAvailable` replicas share the base PodGang, and replicas beyond + `minAvailable` are gang-scheduled independently in their own + PodGangs (set `minAvailable: 1` for fully independent replicas). + `false` or omitted means automatic selection (multi-node and + inter-pod GMS components use a scaling group, other single-node + components a standalone PodClique), not "force PodClique". + Immutable after creation. type: boolean type: object type: object diff --git a/deploy/operator/config/crd/bases/nvidia.com_dynamographdeployments.yaml b/deploy/operator/config/crd/bases/nvidia.com_dynamographdeployments.yaml index 36716e4e9814..59073e32e186 100644 --- a/deploy/operator/config/crd/bases/nvidia.com_dynamographdeployments.yaml +++ b/deploy/operator/config/crd/bases/nvidia.com_dynamographdeployments.yaml @@ -12619,12 +12619,15 @@ spec: forceScalingGroup: description: |- forceScalingGroup opts a single-node component into rendering as a - PodCliqueScalingGroup with one single-pod PodClique per replica, so - each replica is gang-scheduled independently and scaling changes the - scaling-group replica count. `false` or omitted means automatic - selection (multi-node and inter-pod GMS components use a scaling - group, other single-node components a standalone PodClique), not - "force PodClique". Immutable after creation. + PodCliqueScalingGroup with one single-pod PodClique per replica. + Scaling changes the scaling-group replica count; the first + `minAvailable` replicas share the base PodGang, and replicas beyond + `minAvailable` are gang-scheduled independently in their own + PodGangs (set `minAvailable: 1` for fully independent replicas). + `false` or omitted means automatic selection (multi-node and + inter-pod GMS components use a scaling group, other single-node + components a standalone PodClique), not "force PodClique". + Immutable after creation. type: boolean type: object type: object diff --git a/deploy/operator/internal/webhook/validation/dynamographdeployment_validation_envtest_test.go b/deploy/operator/internal/webhook/validation/dynamographdeployment_validation_envtest_test.go index 6d3d133164a2..3279b624e60b 100644 --- a/deploy/operator/internal/webhook/validation/dynamographdeployment_validation_envtest_test.go +++ b/deploy/operator/internal/webhook/validation/dynamographdeployment_validation_envtest_test.go @@ -1522,6 +1522,18 @@ func TestDynamoGraphDeploymentValidator_Validate(t *testing.T) { } }), }, + { + name: "grove block removal with retained experimental is immutable", + oldDeployment: betaDGDWithWorker(func(worker *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec) { + worker.Experimental = &nvidiacomv1beta1.ExperimentalSpec{ + Grove: &nvidiacomv1beta1.GroveSpec{ForceScalingGroup: true}, + } + }), + deployment: betaDGDWithWorker(func(worker *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec) { + worker.Experimental = &nvidiacomv1beta1.ExperimentalSpec{} + }), + wantWebhookErrs: []string{"spec.components[1].experimental.grove.forceScalingGroup: Invalid value: null: cannot be toggled after creation; delete and recreate the DynamoGraphDeployment to change it"}, + }, { name: "inter-pod failover shadow count is immutable", oldDeployment: alphaDGDForAdmission(func(dgd *nvidiacomv1alpha1.DynamoGraphDeployment) { diff --git a/deploy/operator/internal/webhook/validation/shared_helpers.go b/deploy/operator/internal/webhook/validation/shared_helpers.go index 56e5a4afeb3d..6500c982b071 100644 --- a/deploy/operator/internal/webhook/validation/shared_helpers.go +++ b/deploy/operator/internal/webhook/validation/shared_helpers.go @@ -98,10 +98,16 @@ func failoverForExperimental(experimental *nvidiacomv1beta1.ExperimentalSpec) *n return experimental.Failover } +func groveForExperimental(experimental *nvidiacomv1beta1.ExperimentalSpec) *nvidiacomv1beta1.GroveSpec { + if experimental == nil { + return nil + } + return experimental.Grove +} + func forceScalingGroupFor(experimental *nvidiacomv1beta1.ExperimentalSpec) bool { - return experimental != nil && - experimental.Grove != nil && - experimental.Grove.ForceScalingGroup + grove := groveForExperimental(experimental) + return grove != nil && grove.ForceScalingGroup } func effectiveGMSMode(mode nvidiacomv1beta1.GPUMemoryServiceMode) nvidiacomv1beta1.GPUMemoryServiceMode { diff --git a/deploy/operator/internal/webhook/validation/shared_test.go b/deploy/operator/internal/webhook/validation/shared_test.go index ca04ee926874..025bd6437f48 100644 --- a/deploy/operator/internal/webhook/validation/shared_test.go +++ b/deploy/operator/internal/webhook/validation/shared_test.go @@ -329,6 +329,7 @@ func TestValidateExperimentalSpecDoesNotExposePodTemplate(t *testing.T) { fldPath, nvidiacomv1beta1.ComponentTypeWorker, corev1.ResourceRequirements{}, + true, ) assertFieldPaths(t, errs, []string{"spec.components[0].experimental.gpuMemoryService"}) if errs[0].BadValue != "" { diff --git a/deploy/operator/internal/webhook/validation/shared_v1beta1.go b/deploy/operator/internal/webhook/validation/shared_v1beta1.go index b6b5046f1e89..51e9470eaf41 100644 --- a/deploy/operator/internal/webhook/validation/shared_v1beta1.go +++ b/deploy/operator/internal/webhook/validation/shared_v1beta1.go @@ -65,12 +65,6 @@ func (v *sharedValidation) validateDynamoComponentDeploymentSharedSpec( "is currently supported only for Grove-backed DynamoGraphDeployment components", )) } - if forceScalingGroupFor(spec.Experimental) && !grovePathway { - allErrs = append(allErrs, field.Forbidden( - fldPath.Child("experimental", "grove", "forceScalingGroup"), - "is currently supported only for Grove-backed DynamoGraphDeployment components", - )) - } if spec.SharedMemorySize != nil && spec.SharedMemorySize.Sign() < 0 { allErrs = append(allErrs, field.Invalid( fldPath.Child("sharedMemorySize"), @@ -127,6 +121,7 @@ func (v *sharedValidation) validateDynamoComponentDeploymentSharedSpec( fldPath.Child("experimental"), spec.ComponentType, dynamo.GetMainContainerResources(spec), + grovePathway, )...) } @@ -185,6 +180,7 @@ func (v *sharedValidation) validateExperimentalSpec( fldPath *field.Path, componentType nvidiacomv1beta1.ComponentType, resources corev1.ResourceRequirements, + grovePathway bool, ) field.ErrorList { allErrs := field.ErrorList{} if experimental.GPUMemoryService != nil { @@ -217,6 +213,13 @@ func (v *sharedValidation) validateExperimentalSpec( resources, )...) } + if experimental.Grove != nil { + allErrs = append(allErrs, v.validateGroveSpec( + experimental.Grove, + fldPath.Child("grove"), + grovePathway, + )...) + } if experimental.Checkpoint != nil { allErrs = append(allErrs, v.validateComponentCheckpointConfig( experimental.Checkpoint, @@ -286,6 +289,22 @@ func (v *sharedValidation) validateFailoverSpec( return allErrs } +// validateGroveSpec validates grove. grove and fldPath must not be nil. +// grovePathway is supplied by the owning resource. +func (v *sharedValidation) validateGroveSpec( + grove *nvidiacomv1beta1.GroveSpec, + fldPath *field.Path, + grovePathway bool, +) field.ErrorList { + if grove.ForceScalingGroup && !grovePathway { + return field.ErrorList{field.Forbidden( + fldPath.Child("forceScalingGroup"), + "is currently supported only for Grove-backed DynamoGraphDeployment components", + )} + } + return nil +} + // validateComponentCheckpointConfig validates checkpoint. checkpoint and fldPath must not be nil. // gms may be nil because checkpoint validates that sibling relationship. func (v *sharedValidation) validateComponentCheckpointConfig( @@ -460,14 +479,40 @@ func (v *sharedValidation) validateExperimentalSpecUpdate( )) } - // false and omitted both mean automatic selection, so only the - // effective opt-in is immutable. - if forceScalingGroupFor(newExperimental) != forceScalingGroupFor(oldExperimental) { + oldGrove := groveForExperimental(oldExperimental) + if newExperimental.Grove != nil { + allErrs = append(allErrs, v.validateGroveSpecUpdate( + newExperimental.Grove, + oldGrove, + fldPath.Child("grove"), + ownerKind, + )...) + } else if oldGrove != nil && oldGrove.ForceScalingGroup { allErrs = append(allErrs, field.Invalid( fldPath.Child("grove", "forceScalingGroup"), - forceScalingGroupFor(newExperimental), + nil, fmt.Sprintf("cannot be toggled after creation; delete and recreate the %s to change it", ownerKind.Kind), )) } return allErrs } + +// validateGroveSpecUpdate validates a grove update. newGrove and fldPath must +// not be nil; oldGrove may be nil for an addition. false and omitted both +// mean automatic selection, so only the effective opt-in is immutable. +func (v *sharedValidation) validateGroveSpecUpdate( + newGrove *nvidiacomv1beta1.GroveSpec, + oldGrove *nvidiacomv1beta1.GroveSpec, + fldPath *field.Path, + ownerKind schema.GroupKind, +) field.ErrorList { + oldForced := oldGrove != nil && oldGrove.ForceScalingGroup + if newGrove.ForceScalingGroup == oldForced { + return nil + } + return field.ErrorList{field.Invalid( + fldPath.Child("forceScalingGroup"), + newGrove.ForceScalingGroup, + fmt.Sprintf("cannot be toggled after creation; delete and recreate the %s to change it", ownerKind.Kind), + )} +} diff --git a/docs/kubernetes/api-reference.md b/docs/kubernetes/api-reference.md index 2a03058d7392..5810daa75430 100644 --- a/docs/kubernetes/api-reference.md +++ b/docs/kubernetes/api-reference.md @@ -2567,7 +2567,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `forceScalingGroup` _boolean_ | forceScalingGroup opts a single-node component into rendering as a
PodCliqueScalingGroup with one single-pod PodClique per replica, so
each replica is gang-scheduled independently and scaling changes the
scaling-group replica count. `false` or omitted means automatic
selection (multi-node and inter-pod GMS components use a scaling
group, other single-node components a standalone PodClique), not
"force PodClique". Immutable after creation. | | Optional: \{\}
| +| `forceScalingGroup` _boolean_ | forceScalingGroup opts a single-node component into rendering as a
PodCliqueScalingGroup with one single-pod PodClique per replica.
Scaling changes the scaling-group replica count; the first
`minAvailable` replicas share the base PodGang, and replicas beyond
`minAvailable` are gang-scheduled independently in their own
PodGangs (set `minAvailable: 1` for fully independent replicas).
`false` or omitted means automatic selection (multi-node and
inter-pod GMS components use a scaling group, other single-node
components a standalone PodClique), not "force PodClique".
Immutable after creation. | | Optional: \{\}
| #### HardwareSpec From a46cbdad84f0dea8332d9b228801ce7d4d77fd65 Mon Sep 17 00:00:00 2001 From: joeltg Date: Tue, 4 Aug 2026 18:20:34 +0000 Subject: [PATCH 3/3] docs(operator): describe minAvailable and forceScalingGroup PodGang mapping accurately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on the published API contract: - minAvailable: describe the Grove mapping in terms of scaling-group membership (multi-node, inter-pod GMS, or grove.forceScalingGroup; see UsesPCSG) instead of the stale single-node-vs-multi-node split. - forceScalingGroup: state the factual PodGang mapping — the first minAvailable replicas join the deployment's base PodGang together with its other base workloads — instead of promising fully independent replicas, which Grove's base-gang semantics cannot provide. Regenerate CRDs and the API reference from the updated comments. Co-Authored-By: Claude Fable 5 Signed-off-by: joeltg --- deploy/operator/api/v1beta1/common.go | 12 ++++++------ .../v1beta1/dynamocomponentdeployment_types.go | 6 ++++-- .../nvidia.com_dynamocomponentdeployments.yaml | 18 ++++++++++-------- .../nvidia.com_dynamographdeployments.yaml | 18 ++++++++++-------- .../additional-resources/api-reference-k8s.md | 6 +++--- 5 files changed, 33 insertions(+), 27 deletions(-) diff --git a/deploy/operator/api/v1beta1/common.go b/deploy/operator/api/v1beta1/common.go index d2278c0dbf50..f1fa4a834871 100644 --- a/deploy/operator/api/v1beta1/common.go +++ b/deploy/operator/api/v1beta1/common.go @@ -168,12 +168,12 @@ const ( type GroveSpec struct { // forceScalingGroup opts a single-node component into rendering as a // PodCliqueScalingGroup with one single-pod PodClique per replica. - // Scaling changes the scaling-group replica count; the first - // `minAvailable` replicas share the base PodGang, and replicas beyond - // `minAvailable` are gang-scheduled independently in their own - // PodGangs (set `minAvailable: 1` for fully independent replicas). - // `false` or omitted means automatic selection (multi-node and - // inter-pod GMS components use a scaling group, other single-node + // Scaling changes the scaling-group replica count. The first + // `minAvailable` replicas join the deployment's base PodGang together + // with its other base workloads; each replica beyond `minAvailable` + // gets its own PodGang, gang-scheduled separately from the rest of the + // deployment. `false` or omitted means automatic selection (multi-node + // and inter-pod GMS components use a scaling group, other single-node // components a standalone PodClique), not "force PodClique". // Immutable after creation. // +optional diff --git a/deploy/operator/api/v1beta1/dynamocomponentdeployment_types.go b/deploy/operator/api/v1beta1/dynamocomponentdeployment_types.go index 38579feb65f6..1797b0151b92 100644 --- a/deploy/operator/api/v1beta1/dynamocomponentdeployment_types.go +++ b/deploy/operator/api/v1beta1/dynamocomponentdeployment_types.go @@ -133,8 +133,10 @@ type DynamoComponentDeploymentSharedSpec struct { // +optional Replicas *int32 `json:"replicas,omitempty"` - // minAvailable maps to Grove PodClique minAvailable for single-node and - // Grove PodCliqueScalingGroup minAvailable for multi-node components. + // minAvailable maps to Grove PodCliqueScalingGroup minAvailable for + // components rendered as a scaling group (multi-node, inter-pod GMS, or + // `experimental.grove.forceScalingGroup`; see `UsesPCSG`) and to Grove + // PodClique minAvailable for all other single-node components. // This field determines 1) the minimum number of replicas guaranteed to be // gang-scheduled, and 2) when violating minAvailable replicas triggers gang // termination. diff --git a/deploy/operator/config/crd/bases/nvidia.com_dynamocomponentdeployments.yaml b/deploy/operator/config/crd/bases/nvidia.com_dynamocomponentdeployments.yaml index 0c6dfd9b6f71..947a2def3788 100644 --- a/deploy/operator/config/crd/bases/nvidia.com_dynamocomponentdeployments.yaml +++ b/deploy/operator/config/crd/bases/nvidia.com_dynamocomponentdeployments.yaml @@ -12193,12 +12193,12 @@ spec: description: |- forceScalingGroup opts a single-node component into rendering as a PodCliqueScalingGroup with one single-pod PodClique per replica. - Scaling changes the scaling-group replica count; the first - `minAvailable` replicas share the base PodGang, and replicas beyond - `minAvailable` are gang-scheduled independently in their own - PodGangs (set `minAvailable: 1` for fully independent replicas). - `false` or omitted means automatic selection (multi-node and - inter-pod GMS components use a scaling group, other single-node + Scaling changes the scaling-group replica count. The first + `minAvailable` replicas join the deployment's base PodGang together + with its other base workloads; each replica beyond `minAvailable` + gets its own PodGang, gang-scheduled separately from the rest of the + deployment. `false` or omitted means automatic selection (multi-node + and inter-pod GMS components use a scaling group, other single-node components a standalone PodClique), not "force PodClique". Immutable after creation. type: boolean @@ -12225,8 +12225,10 @@ spec: type: boolean minAvailable: description: |- - minAvailable maps to Grove PodClique minAvailable for single-node and - Grove PodCliqueScalingGroup minAvailable for multi-node components. + minAvailable maps to Grove PodCliqueScalingGroup minAvailable for + components rendered as a scaling group (multi-node, inter-pod GMS, or + `experimental.grove.forceScalingGroup`; see `UsesPCSG`) and to Grove + PodClique minAvailable for all other single-node components. This field determines 1) the minimum number of replicas guaranteed to be gang-scheduled, and 2) when violating minAvailable replicas triggers gang termination. diff --git a/deploy/operator/config/crd/bases/nvidia.com_dynamographdeployments.yaml b/deploy/operator/config/crd/bases/nvidia.com_dynamographdeployments.yaml index 354a1a6b2613..13c480c90e99 100644 --- a/deploy/operator/config/crd/bases/nvidia.com_dynamographdeployments.yaml +++ b/deploy/operator/config/crd/bases/nvidia.com_dynamographdeployments.yaml @@ -12651,12 +12651,12 @@ spec: description: |- forceScalingGroup opts a single-node component into rendering as a PodCliqueScalingGroup with one single-pod PodClique per replica. - Scaling changes the scaling-group replica count; the first - `minAvailable` replicas share the base PodGang, and replicas beyond - `minAvailable` are gang-scheduled independently in their own - PodGangs (set `minAvailable: 1` for fully independent replicas). - `false` or omitted means automatic selection (multi-node and - inter-pod GMS components use a scaling group, other single-node + Scaling changes the scaling-group replica count. The first + `minAvailable` replicas join the deployment's base PodGang together + with its other base workloads; each replica beyond `minAvailable` + gets its own PodGang, gang-scheduled separately from the rest of the + deployment. `false` or omitted means automatic selection (multi-node + and inter-pod GMS components use a scaling group, other single-node components a standalone PodClique), not "force PodClique". Immutable after creation. type: boolean @@ -12683,8 +12683,10 @@ spec: type: boolean minAvailable: description: |- - minAvailable maps to Grove PodClique minAvailable for single-node and - Grove PodCliqueScalingGroup minAvailable for multi-node components. + minAvailable maps to Grove PodCliqueScalingGroup minAvailable for + components rendered as a scaling group (multi-node, inter-pod GMS, or + `experimental.grove.forceScalingGroup`; see `UsesPCSG`) and to Grove + PodClique minAvailable for all other single-node components. This field determines 1) the minimum number of replicas guaranteed to be gang-scheduled, and 2) when violating minAvailable replicas triggers gang termination. diff --git a/docs/fern/pages/reference/kubernetes-api/additional-resources/api-reference-k8s.md b/docs/fern/pages/reference/kubernetes-api/additional-resources/api-reference-k8s.md index 008cda63f419..9f43a0a29bab 100644 --- a/docs/fern/pages/reference/kubernetes-api/additional-resources/api-reference-k8s.md +++ b/docs/fern/pages/reference/kubernetes-api/additional-resources/api-reference-k8s.md @@ -2127,7 +2127,7 @@ _Appears in:_ | `globalDynamoNamespace` _boolean_ | globalDynamoNamespace places the component in the global Dynamo
namespace rather than the per-deployment namespace derived from the
DGD name. | | Optional: \{\}
| | `podTemplate` _[PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#podtemplatespec-v1-core)_ | podTemplate defines the component's Pod configuration. New components must
include a container named "main" with a non-empty image. Existing components
created without a podTemplate may remain unchanged. The operator merges
defaults into the main container.
For DGD components whose main image tag is not a Dynamo semantic version,
set runtimeVersionOverride explicitly.
All other containers are user-managed sidecars and must specify their
required fields, including image. | | Optional: \{\}
| | `replicas` _integer_ | replicas is the desired number of Pods for this component. When
`scalingAdapter` is set on this component, this field is managed by
the DynamoGraphDeploymentScalingAdapter and should not be modified
directly. | | Minimum: 0
Optional: \{\}
| -| `minAvailable` _integer_ | minAvailable maps to Grove PodClique minAvailable for single-node and
Grove PodCliqueScalingGroup minAvailable for multi-node components.
This field determines 1) the minimum number of replicas guaranteed to be
gang-scheduled, and 2) when violating minAvailable replicas triggers gang
termination.
For Grove-backed DynamoGraphDeployment components, minAvailable defaults to
1 when omitted and is immutable after creation. Positive replica counts must
be greater than or equal to minAvailable. Replicas may be scaled to 0 as a
special scale-to-zero state; minAvailable remains configured but is not
enforced again until replicas is scaled back to a positive value.
For non-Grove deployments, setting this field will result in a validation error. | | Minimum: 1
Optional: \{\}
| +| `minAvailable` _integer_ | minAvailable maps to Grove PodCliqueScalingGroup minAvailable for
components rendered as a scaling group (multi-node, inter-pod GMS, or
`experimental.grove.forceScalingGroup`; see `UsesPCSG`) and to Grove
PodClique minAvailable for all other single-node components.
This field determines 1) the minimum number of replicas guaranteed to be
gang-scheduled, and 2) when violating minAvailable replicas triggers gang
termination.
For Grove-backed DynamoGraphDeployment components, minAvailable defaults to
1 when omitted and is immutable after creation. Positive replica counts must
be greater than or equal to minAvailable. Replicas may be scaled to 0 as a
special scale-to-zero state; minAvailable remains configured but is not
enforced again until replicas is scaled back to a positive value.
For non-Grove deployments, setting this field will result in a validation error. | | Minimum: 1
Optional: \{\}
| | `multinode` _[MultinodeSpec](#multinodespec)_ | multinode configures multinode components. | | Optional: \{\}
| | `sharedMemorySize` _[Quantity](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#quantity-resource-api)_ | sharedMemorySize controls the size of the tmpfs mounted at `/dev/shm`.
`nil` selects the operator default (8Gi), a positive quantity sets a
custom size, and `"0"` disables the shared-memory volume entirely.
Simpler replacement for v1alpha1's `SharedMemorySpec` struct with its
`disabled bool` + `size Quantity` pattern. | | Optional: \{\}
| | `modelRef` _[ModelReference](#modelreference)_ | modelRef references a model served by this component. When specified,
a headless service is created for endpoint discovery. | | Optional: \{\}
| @@ -2159,7 +2159,7 @@ _Appears in:_ | `globalDynamoNamespace` _boolean_ | globalDynamoNamespace places the component in the global Dynamo
namespace rather than the per-deployment namespace derived from the
DGD name. | | Optional: \{\}
| | `podTemplate` _[PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#podtemplatespec-v1-core)_ | podTemplate defines the component's Pod configuration. New components must
include a container named "main" with a non-empty image. Existing components
created without a podTemplate may remain unchanged. The operator merges
defaults into the main container.
For DGD components whose main image tag is not a Dynamo semantic version,
set runtimeVersionOverride explicitly.
All other containers are user-managed sidecars and must specify their
required fields, including image. | | Optional: \{\}
| | `replicas` _integer_ | replicas is the desired number of Pods for this component. When
`scalingAdapter` is set on this component, this field is managed by
the DynamoGraphDeploymentScalingAdapter and should not be modified
directly. | | Minimum: 0
Optional: \{\}
| -| `minAvailable` _integer_ | minAvailable maps to Grove PodClique minAvailable for single-node and
Grove PodCliqueScalingGroup minAvailable for multi-node components.
This field determines 1) the minimum number of replicas guaranteed to be
gang-scheduled, and 2) when violating minAvailable replicas triggers gang
termination.
For Grove-backed DynamoGraphDeployment components, minAvailable defaults to
1 when omitted and is immutable after creation. Positive replica counts must
be greater than or equal to minAvailable. Replicas may be scaled to 0 as a
special scale-to-zero state; minAvailable remains configured but is not
enforced again until replicas is scaled back to a positive value.
For non-Grove deployments, setting this field will result in a validation error. | | Minimum: 1
Optional: \{\}
| +| `minAvailable` _integer_ | minAvailable maps to Grove PodCliqueScalingGroup minAvailable for
components rendered as a scaling group (multi-node, inter-pod GMS, or
`experimental.grove.forceScalingGroup`; see `UsesPCSG`) and to Grove
PodClique minAvailable for all other single-node components.
This field determines 1) the minimum number of replicas guaranteed to be
gang-scheduled, and 2) when violating minAvailable replicas triggers gang
termination.
For Grove-backed DynamoGraphDeployment components, minAvailable defaults to
1 when omitted and is immutable after creation. Positive replica counts must
be greater than or equal to minAvailable. Replicas may be scaled to 0 as a
special scale-to-zero state; minAvailable remains configured but is not
enforced again until replicas is scaled back to a positive value.
For non-Grove deployments, setting this field will result in a validation error. | | Minimum: 1
Optional: \{\}
| | `multinode` _[MultinodeSpec](#multinodespec)_ | multinode configures multinode components. | | Optional: \{\}
| | `sharedMemorySize` _[Quantity](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#quantity-resource-api)_ | sharedMemorySize controls the size of the tmpfs mounted at `/dev/shm`.
`nil` selects the operator default (8Gi), a positive quantity sets a
custom size, and `"0"` disables the shared-memory volume entirely.
Simpler replacement for v1alpha1's `SharedMemorySpec` struct with its
`disabled bool` + `size Quantity` pattern. | | Optional: \{\}
| | `modelRef` _[ModelReference](#modelreference)_ | modelRef references a model served by this component. When specified,
a headless service is created for endpoint discovery. | | Optional: \{\}
| @@ -2616,7 +2616,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `forceScalingGroup` _boolean_ | forceScalingGroup opts a single-node component into rendering as a
PodCliqueScalingGroup with one single-pod PodClique per replica.
Scaling changes the scaling-group replica count; the first
`minAvailable` replicas share the base PodGang, and replicas beyond
`minAvailable` are gang-scheduled independently in their own
PodGangs (set `minAvailable: 1` for fully independent replicas).
`false` or omitted means automatic selection (multi-node and
inter-pod GMS components use a scaling group, other single-node
components a standalone PodClique), not "force PodClique".
Immutable after creation. | | Optional: \{\}
| +| `forceScalingGroup` _boolean_ | forceScalingGroup opts a single-node component into rendering as a
PodCliqueScalingGroup with one single-pod PodClique per replica.
Scaling changes the scaling-group replica count. The first
`minAvailable` replicas join the deployment's base PodGang together
with its other base workloads; each replica beyond `minAvailable`
gets its own PodGang, gang-scheduled separately from the rest of the
deployment. `false` or omitted means automatic selection (multi-node
and inter-pod GMS components use a scaling group, other single-node
components a standalone PodClique), not "force PodClique".
Immutable after creation. | | Optional: \{\}
| #### HardwareSpec