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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -117,6 +118,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down
9 changes: 9 additions & 0 deletions deploy/operator/api/v1alpha1/shared_spec_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,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
}
Expand Down Expand Up @@ -1618,6 +1622,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
}
Expand Down
20 changes: 20 additions & 0 deletions deploy/operator/api/v1beta1/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,22 @@ 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.
// 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
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
Expand All @@ -183,6 +199,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
Expand Down
19 changes: 17 additions & 2 deletions deploy/operator/api/v1beta1/dynamocomponentdeployment_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -327,6 +329,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()
Comment thread
julienmancuso marked this conversation as resolved.
}

// IsInterPodFailoverEnabled reports whether inter-pod GMS failover is configured.
func (s *DynamoComponentDeploymentSharedSpec) IsInterPodFailoverEnabled() bool {
return s.Experimental != nil &&
Expand Down
20 changes: 20 additions & 0 deletions deploy/operator/api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -12186,6 +12186,23 @@ 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.
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
type: object
type: object
frontendSidecar:
description: |-
Expand All @@ -12208,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12644,6 +12644,23 @@ 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.
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
type: object
type: object
frontendSidecar:
description: |-
Expand All @@ -12666,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (r *groveRestartProgressResolver) Resolve(
// Any component represented by a PodCliqueScalingGroup must use the
// PCSG readiness path. Read failures conservatively keep the component
// in progress; authoritative readiness returns the error separately.
if component.GetNumberOfNodes() > 1 || component.IsInterPodGMSEnabled() {
if component.UsesPCSG() {
isReady, reason, _, _, _ = dynamo.CheckPCSGReady(
ctx,
r.reader,
Expand Down
2 changes: 1 addition & 1 deletion deploy/operator/internal/controller/dgd_grove_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (s *groveScaler) Reconcile(
replicas = 0
}

usesPCSG := component.GetNumberOfNodes() > 1 || component.IsInterPodGMSEnabled()
usesPCSG := component.UsesPCSG()
resourceName := dynamo.GroveComponentResourceName(dgd, componentName)
resourceKind := "PodClique"
gvr := consts.PodCliqueGVR
Expand Down
16 changes: 13 additions & 3 deletions deploy/operator/internal/dynamo/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -1161,6 +1161,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
Expand Down Expand Up @@ -1218,6 +1220,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)
}
Expand All @@ -1233,6 +1237,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},
Expand Down Expand Up @@ -1274,7 +1284,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
}

Expand All @@ -1301,7 +1311,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 {
Expand Down Expand Up @@ -2537,7 +2547,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

Expand Down
Loading
Loading