diff --git a/deploy/operator/internal/AGENTS.md b/deploy/operator/internal/AGENTS.md index 60cf8947b5bd..f1d9cb00a46e 100644 --- a/deploy/operator/internal/AGENTS.md +++ b/deploy/operator/internal/AGENTS.md @@ -39,6 +39,16 @@ reconciliation, rollout, restart, readiness, status, watches, and provider integ - The program request contains only the explicitly mutable DGD. Clients, recorders, configuration, and other dependencies live on the concrete program or collaborator that uses them. +- A `DynamoGraphDeployment` value always represents the complete API object. Never + construct or pass a copy with `spec.components` filtered. +- When a composite program divides workload ownership between managed and delegated + paths, set the ownership predicate once at the program composition root and pass it + with the complete DGD to nested reconcilers that select or observe provider-owned + workloads. Those reconcilers must not reclassify component ownership or accept a + redundant component slice they can derive from the request. Graph-wide reconcilers + consume the complete DGD directly when workload ownership is irrelevant. + A renderer may copy and transform one component locally while emitting provider + resources; it must not expose a parallel component collection as another graph view. - The program result contains `ctrl.Result`, the complete desired DGD status, and queued status-transition events. - Do not add speculative `Facts`, `Inputs`, `ComputedState`, `WorkloadModel`, render diff --git a/deploy/operator/internal/controller/dgd_grove_lpx_restart.go b/deploy/operator/internal/controller/dgd_grove_lpx_restart.go index 6fd00a896129..9ac838eaf241 100644 --- a/deploy/operator/internal/controller/dgd_grove_lpx_restart.go +++ b/deploy/operator/internal/controller/dgd_grove_lpx_restart.go @@ -37,36 +37,39 @@ func (r *lpxRestartProgressResolver) Resolve( // resolveCompositeGroveRestartProgress composes child-owned LPX and ordinary Grove observations. func resolveCompositeGroveRestartProgress( ctx context.Context, - source *v1beta1.DynamoGraphDeployment, - ordinaryDGD *v1beta1.DynamoGraphDeployment, + req groveReconcileRequest, inProgress []string, ordinaryResolver *groveRestartProgressResolver, lpxResolver *lpxRestartProgressResolver, ) []string { ordinary := make([]string, 0, len(inProgress)) - lpxComponents := make([]string, 0, len(inProgress)) + delegated := make([]string, 0, len(inProgress)) pending := make(map[string]bool, len(inProgress)) + delegatedNames := make(map[string]struct{}) + for _, component := range req.DelegatedComponents() { + delegatedNames[component.ComponentName] = struct{}{} + } for _, name := range inProgress { - component := source.GetComponentByName(name) + component := req.DGD.GetComponentByName(name) if component == nil { continue } - if component.IsLPX() { - lpxComponents = append(lpxComponents, name) + if _, found := delegatedNames[name]; found { + delegated = append(delegated, name) } else { ordinary = append(ordinary, name) } } // Observe the shared LPX child before ordinary Grove restart progress. - if len(lpxComponents) > 0 { - for _, name := range lpxResolver.Resolve(ctx, source, lpxComponents) { + if len(delegated) > 0 { + for _, name := range lpxResolver.Resolve(ctx, req.DGD, delegated) { pending[name] = true } } if len(ordinary) > 0 { - for _, name := range ordinaryResolver.Resolve(ctx, ordinaryDGD, ordinary) { + for _, name := range ordinaryResolver.Resolve(ctx, req, ordinary) { pending[name] = true } } diff --git a/deploy/operator/internal/controller/dgd_grove_program.go b/deploy/operator/internal/controller/dgd_grove_program.go index 5f3d9620d3b8..8d5e2cbaccd0 100644 --- a/deploy/operator/internal/controller/dgd_grove_program.go +++ b/deploy/operator/internal/controller/dgd_grove_program.go @@ -20,7 +20,6 @@ package controller import ( "context" "fmt" - "slices" "time" nvidiacomv1beta1 "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" @@ -43,6 +42,41 @@ type groveProgram struct { lpx *dgdLPXHandoff } +// groveReconcileRequest keeps the complete DGD together with the component +// ownership boundary selected by the outer Grove program. +type groveReconcileRequest struct { + DGD *nvidiacomv1beta1.DynamoGraphDeployment + + // IsDelegated reports whether another controller reconciles this component's + // workloads. A nil predicate delegates no components. + IsDelegated func(*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec) bool +} + +func (r groveReconcileRequest) ManagedComponents() []nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec { + components := make([]nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec, 0, len(r.DGD.Spec.Components)) + for i := range r.DGD.Spec.Components { + component := &r.DGD.Spec.Components[i] + if r.IsDelegated == nil || !r.IsDelegated(component) { + components = append(components, *component) + } + } + return components +} + +func (r groveReconcileRequest) DelegatedComponents() []nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec { + if r.IsDelegated == nil { + return nil + } + components := make([]nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec, 0, len(r.DGD.Spec.Components)) + for i := range r.DGD.Spec.Components { + component := &r.DGD.Spec.Components[i] + if r.IsDelegated(component) { + components = append(components, *component) + } + } + return components +} + // newGroveProgram wires the Grove pathway at the DGD composition root. func (r *DynamoGraphDeploymentReconciler) newGroveProgram() *groveProgram { rollout := newDGDWorkerRolloutReconciler(r.Client, r.Recorder) @@ -81,8 +115,12 @@ func (r *DynamoGraphDeploymentReconciler) newGroveProgram() *groveProgram { // are persisted through req.DGD; status accumulates in the returned result. func (p *groveProgram) Reconcile( ctx context.Context, - req workloadProgramRequest, + programReq workloadProgramRequest, ) (programResult workloadProgramResult, retErr error) { + req := groveReconcileRequest{ + DGD: programReq.DGD, + IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController, + } programResult = newWorkloadProgramResult(req.DGD) clearComponentGPUShapes(programResult.Status.Components) @@ -95,8 +133,6 @@ func (p *groveProgram) Reconcile( programResult.Fail(req.DGD.Generation, reasonSelectedWorkloadProviderUnavailable, err) return programResult, reconcile.TerminalError(err) } - var ordinaryDGD *nvidiacomv1beta1.DynamoGraphDeployment - defer func() { if retErr != nil { reason := reasonFailedToReconcileResources @@ -105,10 +141,7 @@ func (p *groveProgram) Reconcile( } programResult.Fail(req.DGD.Generation, reason, retErr) } - if ordinaryDGD == nil { - ordinaryDGD = projectWithoutExternallyManagedComponents(req.DGD) - } - p.topology.Reconcile(ctx, ordinaryDGD, &programResult) + p.topology.Reconcile(ctx, req, &programResult) }() log.FromContext(ctx).Info( "Reconciling Grove resources", @@ -126,18 +159,15 @@ func (p *groveProgram) Reconcile( if err != nil { return programResult, err } - ordinaryDGD = projectWithoutExternallyManagedComponents(req.DGD) - previousRestart := programResult.Status.Restart restart := p.restart.Resolve( ctx, req.DGD, &programResult.Status, - func(ctx context.Context, source *nvidiacomv1beta1.DynamoGraphDeployment, inProgress []string) []string { + func(ctx context.Context, _ *nvidiacomv1beta1.DynamoGraphDeployment, inProgress []string) []string { return resolveCompositeGroveRestartProgress( ctx, - source, - ordinaryDGD, + req, inProgress, p.restartProgress, p.lpxRestartProgress, @@ -149,8 +179,7 @@ func (p *groveProgram) Reconcile( result, err := p.workloads.Reconcile( ctx, - req.DGD, - ordinaryDGD, + req, restart.State, checkpoints.Infos, ) @@ -181,7 +210,7 @@ func (p *groveProgram) Reconcile( result = applyCheckpointStartupReadiness(result, checkpoints.Infos) if result.State != nvidiacomv1beta1.DGDStatePending || result.Reason != reasonWaitingForCheckpoint { - if err := p.scalingAdapters.Reconcile(ctx, ordinaryDGD); err != nil { + if err := p.scalingAdapters.Reconcile(ctx, req.DGD); err != nil { log.FromContext(ctx).Error(err, "Failed to reconcile scaling adapters") return programResult, fmt.Errorf("failed to reconcile scaling adapters: %w", err) } @@ -190,16 +219,3 @@ func (p *groveProgram) Reconcile( programResult.applyReconcileResult(req.DGD.Generation, result) return programResult, nil } - -// projectWithoutExternallyManagedComponents copies source without externally managed components. -// The source must be non-nil; shared nested data must not be mutated by callers. -func projectWithoutExternallyManagedComponents(source *nvidiacomv1beta1.DynamoGraphDeployment) *nvidiacomv1beta1.DynamoGraphDeployment { - projected := *source - projected.Spec.Components = slices.DeleteFunc( - slices.Clone(projected.Spec.Components), - func(component nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec) bool { - return component.ManagedByExternalController() - }, - ) - return &projected -} diff --git a/deploy/operator/internal/controller/dgd_grove_restart_progress_resolver.go b/deploy/operator/internal/controller/dgd_grove_restart_progress_resolver.go index 367f3e0c494a..86eccc013599 100644 --- a/deploy/operator/internal/controller/dgd_grove_restart_progress_resolver.go +++ b/deploy/operator/internal/controller/dgd_grove_restart_progress_resolver.go @@ -20,7 +20,6 @@ package controller import ( "context" - nvidiacomv1beta1 "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" "github.com/ai-dynamo/dynamo/deploy/operator/internal/dynamo" grovev1alpha1 "github.com/ai-dynamo/grove/operator/api/core/v1alpha1" "k8s.io/apimachinery/pkg/types" @@ -40,26 +39,26 @@ func newGroveRestartProgressResolver(reader client.Reader) *groveRestartProgress func (r *groveRestartProgressResolver) Resolve( ctx context.Context, - dgd *nvidiacomv1beta1.DynamoGraphDeployment, + req groveReconcileRequest, inProgress []string, ) []string { logger := log.FromContext(ctx) pcs := &grovev1alpha1.PodCliqueSet{} - pcsName := dynamo.PCSNameForDGD(dgd.Name, dgd.Spec.Components) - if err := r.reader.Get(ctx, types.NamespacedName{Name: pcsName, Namespace: dgd.Namespace}, pcs); err != nil { + pcsName := dynamo.PCSNameForDGD(req.DGD, req.IsDelegated) + if err := r.reader.Get(ctx, types.NamespacedName{Name: pcsName, Namespace: req.DGD.Namespace}, pcs); err != nil { logger.Error(err, "failed to get PodCliqueSet") return inProgress } if pcs.Status.ObservedGeneration == nil { - logger.Info("PodCliqueSet observedGeneration is nil", "name", dgd.Name) + logger.Info("PodCliqueSet observedGeneration is nil", "name", req.DGD.Name) return inProgress } if *pcs.Status.ObservedGeneration < pcs.Generation { logger.Info( "PodCliqueSet not yet reconciled", - "name", dgd.Name, + "name", req.DGD.Name, "generation", pcs.Generation, "observedGeneration", *pcs.Status.ObservedGeneration, ) @@ -68,12 +67,12 @@ func (r *groveRestartProgressResolver) Resolve( updatedInProgress := make([]string, 0, len(inProgress)) for _, componentName := range inProgress { - component := dgd.GetComponentByName(componentName) + component := req.DGD.GetComponentByName(componentName) if component == nil { logger.V(1).Info("component not found in DGD", "componentName", componentName) continue } - resourceName := dynamo.GroveComponentResourceName(dgd, componentName) + resourceName := dynamo.GroveComponentResourceName(pcsName, componentName) var ( isReady bool @@ -87,7 +86,7 @@ func (r *groveRestartProgressResolver) Resolve( ctx, r.reader, resourceName, - dgd.Namespace, + req.DGD.Namespace, logger, ) } else { @@ -95,7 +94,7 @@ func (r *groveRestartProgressResolver) Resolve( ctx, r.reader, resourceName, - dgd.Namespace, + req.DGD.Namespace, logger, ) } diff --git a/deploy/operator/internal/controller/dgd_grove_scaler.go b/deploy/operator/internal/controller/dgd_grove_scaler.go index d24e242851ef..22f24c03b3e1 100644 --- a/deploy/operator/internal/controller/dgd_grove_scaler.go +++ b/deploy/operator/internal/controller/dgd_grove_scaler.go @@ -22,7 +22,6 @@ import ( "fmt" nvidiacomv1alpha1 "github.com/ai-dynamo/dynamo/deploy/operator/api/v1alpha1" - nvidiacomv1beta1 "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" "github.com/ai-dynamo/dynamo/deploy/operator/internal/checkpoint" "github.com/ai-dynamo/dynamo/deploy/operator/internal/consts" commoncontroller "github.com/ai-dynamo/dynamo/deploy/operator/internal/controller_common" @@ -45,14 +44,16 @@ func newGroveScaler(kubeClient client.Client) *groveScaler { // asynchronously from the PodCliqueSet. func (s *groveScaler) Reconcile( ctx context.Context, - dgd *nvidiacomv1beta1.DynamoGraphDeployment, + req groveReconcileRequest, checkpointInfos map[string]*checkpoint.CheckpointInfo, ) error { logger := log.FromContext(ctx) logger.V(1).Info("Reconciling Grove scaling operations") + managedComponents := req.ManagedComponents() + pcsName := dynamo.PCSNameForDGD(req.DGD, req.IsDelegated) - for i := range dgd.Spec.Components { - component := &dgd.Spec.Components[i] + for i := range managedComponents { + component := &managedComponents[i] componentName := component.ComponentName info := checkpointInfos[componentName] gated := info != nil && @@ -71,7 +72,7 @@ func (s *groveScaler) Reconcile( } usesPCSG := component.UsesPCSG() - resourceName := dynamo.GroveComponentResourceName(dgd, componentName) + resourceName := dynamo.GroveComponentResourceName(pcsName, componentName) resourceKind := "PodClique" gvr := consts.PodCliqueGVR if usesPCSG { @@ -82,7 +83,7 @@ func (s *groveScaler) Reconcile( ctx, gvr, resourceName, - dgd.Namespace, + req.DGD.Namespace, replicas, ); err != nil { logger.Error( diff --git a/deploy/operator/internal/controller/dgd_grove_scaler_clustertest_test.go b/deploy/operator/internal/controller/dgd_grove_scaler_clustertest_test.go index 618d1bd4a014..42b292babf15 100644 --- a/deploy/operator/internal/controller/dgd_grove_scaler_clustertest_test.go +++ b/deploy/operator/internal/controller/dgd_grove_scaler_clustertest_test.go @@ -50,7 +50,7 @@ func TestClusterGroveScalerUsesScaleSubresource(t *testing.T) { require.NoError(t, env.Client().Create(ctx, podClique)) t.Log("Scale the PodClique through the production controller-runtime subresource path") - require.NoError(t, newGroveScaler(env.Client()).Reconcile(ctx, dgd, nil)) + require.NoError(t, newGroveScaler(env.Client()).Reconcile(ctx, groveReconcileRequest{DGD: dgd}, nil)) t.Log("Read the PodClique back and verify the API server applied the scale update") require.NoError(t, env.Client().Get(ctx, client.ObjectKeyFromObject(podClique), podClique)) diff --git a/deploy/operator/internal/controller/dgd_grove_scaler_test.go b/deploy/operator/internal/controller/dgd_grove_scaler_test.go index 576820f82755..b324472ca016 100644 --- a/deploy/operator/internal/controller/dgd_grove_scaler_test.go +++ b/deploy/operator/internal/controller/dgd_grove_scaler_test.go @@ -73,7 +73,7 @@ func TestGroveScaler_ReconcileTargetsExpectedGroveChildren(t *testing.T) { err := newGroveScaler(kubeClient).Reconcile( t.Context(), - dgd, + groveReconcileRequest{DGD: dgd}, map[string]*checkpoint.CheckpointInfo{ "gated": { Enabled: true, @@ -172,7 +172,7 @@ func TestGroveScaler_ReconcileHandlesScaleReadErrors(t *testing.T) { WithScheme(newDynamoGraphDeploymentControllerTestScheme(t)). WithRESTMapper(groveScaleRESTMapper()). Build() - require.NoError(t, newGroveScaler(kubeClient).Reconcile(t.Context(), dgd, nil)) + require.NoError(t, newGroveScaler(kubeClient).Reconcile(t.Context(), groveReconcileRequest{DGD: dgd}, nil)) }) t.Run("other errors are propagated", func(t *testing.T) { @@ -185,7 +185,7 @@ func TestGroveScaler_ReconcileHandlesScaleReadErrors(t *testing.T) { }, }). Build() - err := newGroveScaler(kubeClient).Reconcile(t.Context(), dgd, nil) + err := newGroveScaler(kubeClient).Reconcile(t.Context(), groveReconcileRequest{DGD: dgd}, nil) require.ErrorContains(t, err, "scale read failed") }) } diff --git a/deploy/operator/internal/controller/dgd_grove_stable_resources_reconciler.go b/deploy/operator/internal/controller/dgd_grove_stable_resources_reconciler.go index 146a610d4bec..f5d7c3429e8b 100644 --- a/deploy/operator/internal/controller/dgd_grove_stable_resources_reconciler.go +++ b/deploy/operator/internal/controller/dgd_grove_stable_resources_reconciler.go @@ -26,6 +26,7 @@ import ( commonconsts "github.com/ai-dynamo/dynamo/deploy/operator/internal/consts" commoncontroller "github.com/ai-dynamo/dynamo/deploy/operator/internal/controller_common" "github.com/ai-dynamo/dynamo/deploy/operator/internal/dynamo" + grovev1alpha1 "github.com/ai-dynamo/grove/operator/api/core/v1alpha1" networkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" @@ -57,16 +58,18 @@ func newGroveStableResourcesReconciler( func (r *groveStableResourcesReconciler) Reconcile( ctx context.Context, - dgd *nvidiacomv1beta1.DynamoGraphDeployment, - renderDeployment *nvidiacomv1beta1.DynamoGraphDeployment, + req groveReconcileRequest, + podCliqueSet *grovev1alpha1.PodCliqueSet, ) ([]Resource, error) { logger := log.FromContext(ctx) + + // Model discovery is owned by the DGD and spans managed and external components. if err := dynamo.ReconcileModelServicesForComponents( ctx, r, - dgd, - dynamo.ComponentsByName(dgd), - dgd.Namespace, + req.DGD, + dynamo.ComponentsByName(req.DGD), + req.DGD.Namespace, ); err != nil { logger.Error(err, "failed to reconcile model services") return nil, fmt.Errorf("failed to reconcile model services: %w", err) @@ -75,16 +78,17 @@ func (r *groveStableResourcesReconciler) Reconcile( resources := []Resource{} isK8sDiscoveryEnabled := commoncontroller.IsK8sDiscoveryEnabled( r.config.Discovery.Backend, - dgd.Annotations, + req.DGD.Annotations, ) - for i := range renderDeployment.Spec.Components { - component := &renderDeployment.Spec.Components[i] + managedComponents := req.ManagedComponents() + for i := range managedComponents { + component := &managedComponents[i] if isK8sDiscoveryEnabled || string(component.ComponentType) == commonconsts.ComponentTypeFrontend { serviceResource, err := r.reconcileComponentService( ctx, - dgd, - renderDeployment, + req.DGD, component, + podCliqueSet, isK8sDiscoveryEnabled, ) if err != nil { @@ -99,9 +103,9 @@ func (r *groveStableResourcesReconciler) Reconcile( // Sync every component so one that stops qualifying has its Service deleted. epService, err := r.reconcileElasticEPLeaderService( ctx, - dgd, - renderDeployment, + req.DGD, component, + podCliqueSet, !isSinglePodElasticEPLeader(component), ) if err != nil { @@ -116,7 +120,7 @@ func (r *groveStableResourcesReconciler) Reconcile( } ingressResources, err := r.reconcileFrontendIngress( ctx, - dgd, + req.DGD, component, ) if err != nil { @@ -128,25 +132,54 @@ func (r *groveStableResourcesReconciler) Reconcile( return resources, nil } +// groveComponentServiceParams derives service identity from the rendered workload, +// preserving legacy component labels. dgd and component must be non-nil; a nil +// podCliqueSet uses the component's labels when no rendered workload is available. +func groveComponentServiceParams( + dgd *nvidiacomv1beta1.DynamoGraphDeployment, + component *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec, + podCliqueSet *grovev1alpha1.PodCliqueSet, +) dynamo.ComponentServiceParams { + componentName := component.ComponentName + componentType := string(component.ComponentType) + labels := dynamo.GetDGDComponentResourceLabels(dgd, componentName, component) + if clique := podCliqueSetCliqueForComponent(podCliqueSet, componentName); clique != nil { + // Preserve only the subcomponent label added by legacy worker conversion. + if renderedType := clique.Labels[commonconsts.KubeLabelDynamoComponentType]; renderedType != "" && renderedType != componentType { + if _, supplied := dynamo.GetPodTemplateLabels(component)[commonconsts.KubeLabelDynamoSubComponentType]; !supplied && componentType != "" { + labels[commonconsts.KubeLabelDynamoSubComponentType] = componentType + } + componentType = renderedType + } + + // Worker hashes follow the rendered workload in either mode. + if value := clique.Labels[commonconsts.KubeLabelDynamoWorkerHash]; value != "" { + labels[commonconsts.KubeLabelDynamoWorkerHash] = value + } + } + return dynamo.ComponentServiceParams{ + ServiceName: dynamo.GetDCDResourceName(dgd, componentName, ""), + Namespace: dgd.Namespace, + ComponentType: componentType, + DynamoNamespace: dgd.GetDynamoNamespaceForComponent(component), + ComponentName: componentName, + Labels: labels, + Annotations: dynamo.GetDGDComponentResourceAnnotations(dgd, componentName, component), + } +} + func (r *groveStableResourcesReconciler) reconcileComponentService( ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment, - renderDeployment *nvidiacomv1beta1.DynamoGraphDeployment, component *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec, + podCliqueSet *grovev1alpha1.PodCliqueSet, isK8sDiscoveryEnabled bool, ) (Resource, error) { logger := log.FromContext(ctx) componentName := component.ComponentName - service, err := dynamo.GenerateComponentService(dynamo.ComponentServiceParams{ - ServiceName: dynamo.GetDCDResourceName(dgd, componentName, ""), - Namespace: dgd.Namespace, - ComponentType: string(component.ComponentType), - DynamoNamespace: renderDeployment.GetDynamoNamespaceForComponent(component), - ComponentName: componentName, - Labels: dynamo.GetDGDComponentResourceLabels(renderDeployment, componentName, component), - Annotations: dynamo.GetDGDComponentResourceAnnotations(renderDeployment, componentName, component), - IsK8sDiscovery: isK8sDiscoveryEnabled, - }) + params := groveComponentServiceParams(dgd, component, podCliqueSet) + params.IsK8sDiscovery = isK8sDiscoveryEnabled + service, err := dynamo.GenerateComponentService(params) if err != nil { logger.Error(err, "failed to generate the main component service") return nil, fmt.Errorf("failed to generate the main component service: %w", err) @@ -168,12 +201,7 @@ func (r *groveStableResourcesReconciler) reconcileComponentService( return nil, nil } - desiredAnnotations := dynamo.GetDGDComponentResourceAnnotations( - renderDeployment, - componentName, - component, - ) - if err := r.syncServiceAnnotations(ctx, dgd, syncedService, desiredAnnotations, componentName); err != nil { + if err := r.syncServiceAnnotations(ctx, dgd, syncedService, params.Annotations, componentName); err != nil { logger.Error(err, "Failed to update main component service", "component", componentName) return nil, fmt.Errorf("failed to update main component service %s: %w", componentName, err) } @@ -259,21 +287,14 @@ func isSinglePodElasticEPLeader(component *nvidiacomv1beta1.DynamoComponentDeplo func (r *groveStableResourcesReconciler) reconcileElasticEPLeaderService( ctx context.Context, dgd *nvidiacomv1beta1.DynamoGraphDeployment, - renderDeployment *nvidiacomv1beta1.DynamoGraphDeployment, component *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec, + podCliqueSet *grovev1alpha1.PodCliqueSet, toDelete bool, ) (Resource, error) { + // Use the same rendered identity as discovery Services so followers find the leader. componentName := component.ComponentName - desiredAnnotations := dynamo.GetDGDComponentResourceAnnotations(renderDeployment, componentName, component) - service := dynamo.GenerateElasticEPHeadlessService(dynamo.ComponentServiceParams{ - ServiceName: dynamo.GetDCDResourceName(dgd, componentName, ""), - Namespace: dgd.Namespace, - ComponentType: string(component.ComponentType), - DynamoNamespace: renderDeployment.GetDynamoNamespaceForComponent(component), - ComponentName: componentName, - Labels: dynamo.GetDGDComponentResourceLabels(renderDeployment, componentName, component), - Annotations: desiredAnnotations, - }) + params := groveComponentServiceParams(dgd, component, podCliqueSet) + service := dynamo.GenerateElasticEPHeadlessService(params) // Handle removal here rather than through SyncResource, which resolves the live object // by name alone and would delete a Service this DGD never created. @@ -297,7 +318,7 @@ func (r *groveStableResourcesReconciler) reconcileElasticEPLeaderService( return nil, nil } - if err := r.syncServiceAnnotations(ctx, dgd, syncedService, desiredAnnotations, componentName); err != nil { + if err := r.syncServiceAnnotations(ctx, dgd, syncedService, params.Annotations, componentName); err != nil { return nil, fmt.Errorf("failed to update the elastic-EP leader service %s: %w", componentName, err) } diff --git a/deploy/operator/internal/controller/dgd_grove_stable_resources_reconciler_test.go b/deploy/operator/internal/controller/dgd_grove_stable_resources_reconciler_test.go index 6f65cb928895..f69f5b352edf 100644 --- a/deploy/operator/internal/controller/dgd_grove_stable_resources_reconciler_test.go +++ b/deploy/operator/internal/controller/dgd_grove_stable_resources_reconciler_test.go @@ -24,10 +24,14 @@ import ( configv1alpha1 "github.com/ai-dynamo/dynamo/deploy/operator/api/config/v1alpha1" "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" commonconsts "github.com/ai-dynamo/dynamo/deploy/operator/internal/consts" + commoncontroller "github.com/ai-dynamo/dynamo/deploy/operator/internal/controller_common" "github.com/ai-dynamo/dynamo/deploy/operator/internal/dynamo" + grovev1alpha1 "github.com/ai-dynamo/grove/operator/api/core/v1alpha1" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/events" "k8s.io/utils/ptr" @@ -190,7 +194,7 @@ func TestGroveStableResourcesReconcilerElasticEPLeaderServiceLifecycle(t *testin t.Log("Reconcile the Grove stable resources") ctx := context.Background() reconciler, kubeClient := newElasticEPTestStableResourcesReconciler(t, dgd) - if _, err := reconciler.Reconcile(ctx, dgd, dgd); err != nil { + if _, err := reconciler.Reconcile(ctx, groveReconcileRequest{DGD: dgd}, nil); err != nil { t.Fatalf("Reconcile returned an error: %v", err) } @@ -223,12 +227,94 @@ func TestGroveStableResourcesReconcilerElasticEPLeaderServiceLifecycle(t *testin } } +func TestGroveStableResourcesReconcilerElasticEPServicesSelectRenderedPods(t *testing.T) { + for _, componentType := range []v1beta1.ComponentType{v1beta1.ComponentTypeDecode, v1beta1.ComponentTypePrefill} { + for _, tc := range []struct { + name string + legacy bool + noType bool + labels map[string]string + }{ + {name: "native"}, + {name: "legacy", legacy: true}, + {name: "type-from-pod-label", noType: true}, + {name: "native-custom", labels: map[string]string{commonconsts.KubeLabelDynamoSubComponentType: "custom"}}, + {name: "legacy-custom", legacy: true, labels: map[string]string{commonconsts.KubeLabelDynamoSubComponentType: "custom"}}, + {name: "native-empty", labels: map[string]string{commonconsts.KubeLabelDynamoSubComponentType: ""}}, + {name: "legacy-empty", legacy: true, labels: map[string]string{commonconsts.KubeLabelDynamoSubComponentType: ""}}, + } { + t.Run(string(componentType)+"/"+tc.name, func(t *testing.T) { + t.Log("Build an elastic-EP leader and its existing workload labels") + component := newElasticEPComponent(elasticEPArgs) + component.ComponentType = componentType + component.PodTemplate.Spec.Containers[0].Image = "nvcr.io/nvidia/ai-dynamo/vllm-runtime:1.5.0" + component.PodTemplate.Spec.Containers[0].Command = []string{"python3", "-m", "dynamo.vllm"} + component.PodTemplate.Spec.Containers[0].Args = []string{"--model", "test", "--enable-elastic-ep", "--data-parallel-backend", "ray"} + component.PodTemplate.Labels = tc.labels + if tc.noType { + component.ComponentType = "" + component.PodTemplate.Labels = map[string]string{commonconsts.KubeLabelDynamoComponentType: commonconsts.ComponentTypeWorker} + } + dgd := newElasticEPTestDGD(component) + reconciler, kubeClient := newElasticEPTestStableResourcesReconciler(t, dgd) + original := dgd.DeepCopy() + var previous *grovev1alpha1.PodCliqueSet + if tc.legacy { + previous = &grovev1alpha1.PodCliqueSet{Spec: grovev1alpha1.PodCliqueSetSpec{ + Template: grovev1alpha1.PodCliqueSetTemplateSpec{ + Cliques: []*grovev1alpha1.PodCliqueTemplateSpec{{Labels: map[string]string{ + commonconsts.KubeLabelDynamoComponent: component.ComponentName, + commonconsts.KubeLabelDynamoComponentType: commonconsts.ComponentTypeWorker, + commonconsts.KubeLabelDynamoSubComponentType: string(componentType), + }}}, + }, + }} + } + + t.Log("Render the actual Grove workload and reconcile its discovery and Ray Services") + config := &configv1alpha1.OperatorConfiguration{ + Discovery: configv1alpha1.DiscoveryConfiguration{Backend: configv1alpha1.DiscoveryBackendKubernetes}, + } + pcs, err := dynamo.GenerateGrovePodCliqueSet(t.Context(), dgd, nil, config, &commoncontroller.RuntimeConfig{}, nil, nil, nil, previous, false, nil) + require.NoError(t, err) + reconciler.config = config + _, err = reconciler.Reconcile(t.Context(), groveReconcileRequest{DGD: dgd}, pcs) + require.NoError(t, err) + + t.Log("Verify both Services select the rendered leader without changing the source DGD") + podLabels := pcs.Spec.Template.Cliques[0].Labels + wantType := string(componentType) + if tc.legacy || tc.noType { + wantType = commonconsts.ComponentTypeWorker + } + require.Equal(t, wantType, podLabels[commonconsts.KubeLabelDynamoComponentType]) + componentServiceName := dynamo.GetDCDResourceName(dgd, component.ComponentName, "") + for _, name := range []string{componentServiceName, dynamo.ElasticEPLeaderServiceName(componentServiceName)} { + service := &corev1.Service{} + require.NoError(t, kubeClient.Get(t.Context(), client.ObjectKey{Namespace: dgd.Namespace, Name: name}, service)) + require.True(t, labels.SelectorFromSet(service.Spec.Selector).Matches(labels.Set(podLabels)), + "Service %s selector %v does not match rendered pod labels %v", name, service.Spec.Selector, podLabels) + require.Equal(t, podLabels[commonconsts.KubeLabelDynamoWorkerHash], service.Labels[commonconsts.KubeLabelDynamoWorkerHash]) + if tc.labels != nil { + require.Subset(t, service.Labels, tc.labels) + } else if tc.legacy { + require.Equal(t, string(componentType), service.Labels[commonconsts.KubeLabelDynamoSubComponentType]) + } else { + require.NotContains(t, service.Labels, commonconsts.KubeLabelDynamoSubComponentType) + } + } + require.Equal(t, original, dgd) + }) + } + } +} + func TestGroveStableResourcesReconcilerDeletesElasticEPLeaderServiceWhenEligibilityIsRemoved(t *testing.T) { t.Log("Reconcile a single-pod elastic-EP component so the leader Service is created") ctx := context.Background() dgd := newElasticEPTestDGD(newElasticEPComponent(elasticEPArgs)) reconciler, kubeClient := newElasticEPTestStableResourcesReconciler(t, dgd) - if _, err := reconciler.Reconcile(ctx, dgd, dgd); err != nil { + if _, err := reconciler.Reconcile(ctx, groveReconcileRequest{DGD: dgd}, nil); err != nil { t.Fatalf("first Reconcile returned an error: %v", err) } @@ -242,7 +328,7 @@ func TestGroveStableResourcesReconcilerDeletesElasticEPLeaderServiceWhenEligibil t.Log("Drop elastic EP from the component and reconcile again") dgd.Spec.Components[0].PodTemplate.Spec.Containers[0].Args = []string{"python3 -m dynamo.vllm"} - if _, err := reconciler.Reconcile(ctx, dgd, dgd); err != nil { + if _, err := reconciler.Reconcile(ctx, groveReconcileRequest{DGD: dgd}, nil); err != nil { t.Fatalf("second Reconcile returned an error: %v", err) } @@ -269,7 +355,7 @@ func TestGroveStableResourcesReconcilerLeavesAnUnownedNameCollisionAlone(t *test reconciler, kubeClient := newElasticEPTestStableResourcesReconciler(t, dgd, unowned) t.Log("Reconcile a component that does not qualify, so it takes the delete path") - if _, err := reconciler.Reconcile(ctx, dgd, dgd); err != nil { + if _, err := reconciler.Reconcile(ctx, groveReconcileRequest{DGD: dgd}, nil); err != nil { t.Fatalf("Reconcile returned an error: %v", err) } @@ -319,7 +405,7 @@ func TestGroveStableResourcesReconcilerDeletesTheExactOwnershipCheckedService(t events.NewFakeRecorder(100), &configv1alpha1.OperatorConfiguration{}, ) - if _, err := reconciler.Reconcile(ctx, dgd, dgd); err != nil { + if _, err := reconciler.Reconcile(ctx, groveReconcileRequest{DGD: dgd}, nil); err != nil { t.Fatalf("Reconcile returned an error: %v", err) } @@ -342,13 +428,13 @@ func TestGroveStableResourcesReconcilerConvergesLeaderServiceAnnotations(t *test dgd := newElasticEPTestDGD(newElasticEPComponent(elasticEPArgs)) dgd.Spec.Annotations = map[string]string{"example.com/note": "before"} reconciler, kubeClient := newElasticEPTestStableResourcesReconciler(t, dgd) - if _, err := reconciler.Reconcile(ctx, dgd, dgd); err != nil { + if _, err := reconciler.Reconcile(ctx, groveReconcileRequest{DGD: dgd}, nil); err != nil { t.Fatalf("first Reconcile returned an error: %v", err) } t.Log("Edit the DGD annotation and reconcile again") dgd.Spec.Annotations["example.com/note"] = "after" - if _, err := reconciler.Reconcile(ctx, dgd, dgd); err != nil { + if _, err := reconciler.Reconcile(ctx, groveReconcileRequest{DGD: dgd}, nil); err != nil { t.Fatalf("second Reconcile returned an error: %v", err) } diff --git a/deploy/operator/internal/controller/dgd_grove_topology_condition_reconciler.go b/deploy/operator/internal/controller/dgd_grove_topology_condition_reconciler.go index 17eec3c45fe0..3e976063a9a1 100644 --- a/deploy/operator/internal/controller/dgd_grove_topology_condition_reconciler.go +++ b/deploy/operator/internal/controller/dgd_grove_topology_condition_reconciler.go @@ -49,11 +49,11 @@ func newDGDGroveTopologyConditionReconciler( func (r *dgdGroveTopologyConditionReconciler) Reconcile( ctx context.Context, - dgd *nvidiacomv1beta1.DynamoGraphDeployment, + req groveReconcileRequest, result *workloadProgramResult, ) { // Project status only when typed or provider-native topology is configured. - if result == nil || (!dgd.HasAnyTopologyConstraint() && !provideroverride.HasGroveTopologyOverrides(dgd)) { + if result == nil || (!req.DGD.HasAnyTopologyConstraint() && !provideroverride.HasGroveTopologyOverrides(req.DGD)) { return } status := &result.Status @@ -61,8 +61,8 @@ func (r *dgdGroveTopologyConditionReconciler) Reconcile( pcs := &grovev1alpha1.PodCliqueSet{} if err := r.reader.Get(ctx, types.NamespacedName{ - Name: dynamo.PCSNameForDGD(dgd.Name, dgd.Spec.Components), - Namespace: dgd.Namespace, + Name: dynamo.PCSNameForDGD(req.DGD, req.IsDelegated), + Namespace: req.DGD.Namespace, }, pcs); err != nil { if !apierrors.IsNotFound(err) { logger.V(1).Info("failed to read PCS for topology condition projection", "error", err) diff --git a/deploy/operator/internal/controller/dgd_grove_worker_hash_suffix_envtest_test.go b/deploy/operator/internal/controller/dgd_grove_worker_hash_suffix_envtest_test.go index b17040a56a64..5f177c3b532a 100644 --- a/deploy/operator/internal/controller/dgd_grove_worker_hash_suffix_envtest_test.go +++ b/deploy/operator/internal/controller/dgd_grove_worker_hash_suffix_envtest_test.go @@ -154,7 +154,7 @@ func createLegacyGroveWorkerHashSuffixTestDGD( legacyPCS := &grovev1alpha1.PodCliqueSet{ ObjectMeta: metav1.ObjectMeta{ - Name: dynamo.PCSNameForDGD(dgd.Name, dgd.Spec.Components), + Name: dynamo.PCSNameForDGD(dgd, nil), Namespace: dgd.Namespace, OwnerReferences: []metav1.OwnerReference{ *metav1.NewControllerRef(dgd, nvidiacomv1beta1.GroupVersion.WithKind("DynamoGraphDeployment")), @@ -265,7 +265,7 @@ func checkGroveWorkerHashSuffixes( ) (bool, string) { t.Helper() key := types.NamespacedName{ - Name: dynamo.PCSNameForDGD(dgd.Name, dgd.Spec.Components), + Name: dynamo.PCSNameForDGD(dgd, nil), Namespace: dgd.Namespace, } pcs := &grovev1alpha1.PodCliqueSet{} @@ -317,7 +317,7 @@ func waitForGroveFrontendEnv( ) { t.Helper() key := types.NamespacedName{ - Name: dynamo.PCSNameForDGD(dgd.Name, dgd.Spec.Components), + Name: dynamo.PCSNameForDGD(dgd, nil), Namespace: dgd.Namespace, } dynamotesting.Eventually(t, func() (bool, string) { diff --git a/deploy/operator/internal/controller/dgd_grove_workload_renderer.go b/deploy/operator/internal/controller/dgd_grove_workload_renderer.go index c5c32f86ba00..48b63c7cea92 100644 --- a/deploy/operator/internal/controller/dgd_grove_workload_renderer.go +++ b/deploy/operator/internal/controller/dgd_grove_workload_renderer.go @@ -30,7 +30,6 @@ import ( commoncontroller "github.com/ai-dynamo/dynamo/deploy/operator/internal/controller_common" "github.com/ai-dynamo/dynamo/deploy/operator/internal/dynamo" grovev1alpha1 "github.com/ai-dynamo/grove/operator/api/core/v1alpha1" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -46,13 +45,12 @@ type groveWorkloadRenderer struct { dockerSecretRetriever DockerSecretRetriever } -// grovePodCliqueSetRender couples the desired PCS and rendered DGD to the -// exact observation used to decide compatibility and the worker hash suffix. +// grovePodCliqueSetRender couples the desired PCS to the exact observation used +// to decide compatibility and the worker hash suffix. type grovePodCliqueSetRender struct { - existing *grovev1alpha1.PodCliqueSet - desired *grovev1alpha1.PodCliqueSet - renderDeployment *nvidiacomv1beta1.DynamoGraphDeployment - gpuShapes map[string]dynamo.GPUShape + existing *grovev1alpha1.PodCliqueSet + desired *grovev1alpha1.PodCliqueSet + gpuShapes map[string]dynamo.GPUShape } func newGroveWorkloadRenderer( @@ -71,12 +69,12 @@ func newGroveWorkloadRenderer( func (r *groveWorkloadRenderer) Render( ctx context.Context, - ordinary *nvidiacomv1beta1.DynamoGraphDeployment, + req groveReconcileRequest, restartState *dynamo.RestartState, checkpointInfos map[string]*checkpoint.CheckpointInfo, workerGenerationChanged bool, ) (*grovePodCliqueSetRender, error) { - if ordinary == nil { + if req.DGD == nil { return nil, fmt.Errorf("cannot render Grove PodCliqueSet without a DynamoGraphDeployment") } if r.reader == nil { @@ -85,8 +83,8 @@ func (r *groveWorkloadRenderer) Render( existingPodCliqueSet := &grovev1alpha1.PodCliqueSet{} key := types.NamespacedName{ - Name: dynamo.PCSNameForDGD(ordinary.Name, ordinary.Spec.Components), - Namespace: ordinary.Namespace, + Name: dynamo.PCSNameForDGD(req.DGD, req.IsDelegated), + Namespace: req.DGD.Namespace, } if err := r.reader.Get(ctx, key, existingPodCliqueSet); err != nil { if !apierrors.IsNotFound(err) { @@ -95,16 +93,11 @@ func (r *groveWorkloadRenderer) Render( existingPodCliqueSet = nil } - workerHashSuffixNeeded := shouldRenderGroveWorkerHashSuffix(ordinary, existingPodCliqueSet, workerGenerationChanged) - renderDeployment, err := groveRenderDeployment(ordinary, existingPodCliqueSet, workerHashSuffixNeeded) - if err != nil { - return nil, err - } // Render ordinary workloads and retain the observed server-owned fields. - existingRestartAnnotations := restartAnnotationsFromPodCliqueSet(existingPodCliqueSet) + workerHashSuffix := shouldRenderGroveWorkerHashSuffix(req, existingPodCliqueSet, workerGenerationChanged) desired, err := dynamo.GenerateGrovePodCliqueSet( - ctx, renderDeployment, r.config, r.runtimeConfig, r.reader, - r.dockerSecretRetriever, restartState, existingRestartAnnotations, checkpointInfos, + ctx, req.DGD, req.IsDelegated, r.config, r.runtimeConfig, r.reader, + r.dockerSecretRetriever, restartState, existingPodCliqueSet, workerHashSuffix, checkpointInfos, ) if err != nil { return nil, err @@ -117,79 +110,36 @@ func (r *groveWorkloadRenderer) Render( checkpointInfos, ) - // Resolve capacity from the same rendered ordinary workload. - gpuShapes, err := dynamo.ResolveGroveGPUShapes(ctx, r.reader, renderDeployment, desired) + gpuShapes, err := dynamo.ResolveGroveGPUShapes(ctx, r.reader, req.DGD, req.IsDelegated, desired) if err != nil { return nil, err } return &grovePodCliqueSetRender{ - existing: existingPodCliqueSet, - desired: desired, - renderDeployment: renderDeployment, - gpuShapes: gpuShapes, + existing: existingPodCliqueSet, + desired: desired, + gpuShapes: gpuShapes, }, nil } -func groveRenderDeployment( - ordinary *nvidiacomv1beta1.DynamoGraphDeployment, - pcs *grovev1alpha1.PodCliqueSet, - workerHashSuffix bool, -) (*nvidiacomv1beta1.DynamoGraphDeployment, error) { - renderDeployment := ordinary.DeepCopy() - - // Compatibility and worker labels mutate only the independently owned copy. - applyGroveCompatibility(renderDeployment, pcs) - if !workerHashSuffix { - return renderDeployment, nil - } - if err := applyGroveWorkerHashSuffix(renderDeployment, ordinary); err != nil { - return nil, err - } - return renderDeployment, nil -} - -func applyGroveWorkerHashSuffix( - renderDeployment *nvidiacomv1beta1.DynamoGraphDeployment, - hashSource *nvidiacomv1beta1.DynamoGraphDeployment, -) error { - workerHash, err := dynamo.ComputeDGDWorkersSpecHash(hashSource) - if err != nil { - return fmt.Errorf("compute Grove worker hash suffix: %w", err) - } - for i := range renderDeployment.Spec.Components { - component := &renderDeployment.Spec.Components[i] - if !dynamo.IsWorkerComponent(string(component.ComponentType)) { - continue - } - if component.PodTemplate == nil { - component.PodTemplate = &corev1.PodTemplateSpec{} - } - if component.PodTemplate.Labels == nil { - component.PodTemplate.Labels = make(map[string]string) - } - component.PodTemplate.Labels[commonconsts.KubeLabelDynamoWorkerHash] = workerHash - } - return nil -} - func shouldRenderGroveWorkerHashSuffix( - dgd *nvidiacomv1beta1.DynamoGraphDeployment, + req groveReconcileRequest, existing *grovev1alpha1.PodCliqueSet, hashChanged bool, ) bool { - if !dgdHasWorkerComponents(dgd) { + managedComponents := req.ManagedComponents() + if !hasWorkerComponents(managedComponents) { return false } - if existing == nil || podCliqueSetUsesGroveWorkerHashSuffix(dgd, existing) { + if existing == nil || podCliqueSetUsesGroveWorkerHashSuffix(req, existing) { return true } return hashChanged } -func dgdHasWorkerComponents(dgd *nvidiacomv1beta1.DynamoGraphDeployment) bool { - for i := range dgd.Spec.Components { - if dynamo.IsWorkerComponent(string(dgd.Spec.Components[i].ComponentType)) { +func hasWorkerComponents(components []nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec) bool { + for i := range components { + if dynamo.IsWorkerComponent(string(components[i].ComponentType)) { return true } } @@ -197,11 +147,12 @@ func dgdHasWorkerComponents(dgd *nvidiacomv1beta1.DynamoGraphDeployment) bool { } func podCliqueSetUsesGroveWorkerHashSuffix( - dgd *nvidiacomv1beta1.DynamoGraphDeployment, + req groveReconcileRequest, pcs *grovev1alpha1.PodCliqueSet, ) bool { - for i := range dgd.Spec.Components { - component := &dgd.Spec.Components[i] + managedComponents := req.ManagedComponents() + for i := range managedComponents { + component := &managedComponents[i] if !dynamo.IsWorkerComponent(string(component.ComponentType)) { continue } @@ -216,21 +167,22 @@ func podCliqueSetUsesGroveWorkerHashSuffix( // podCliqueSetObservesWorkerHash reports whether every worker clique in pcs carries the // computed DGD worker hash, or, for a pre-existing legacy PCS, that no clique is stamped. func podCliqueSetObservesWorkerHash( - dgd *nvidiacomv1beta1.DynamoGraphDeployment, + req groveReconcileRequest, pcs *grovev1alpha1.PodCliqueSet, acceptAllUnstamped bool, ) (bool, error) { - if !dgdHasWorkerComponents(dgd) { + managedComponents := req.ManagedComponents() + if !hasWorkerComponents(managedComponents) { return true, nil } - want, err := dynamo.ComputeDGDWorkersSpecHash(dgd) + want, err := dynamo.ComputeDGDWorkersSpecHash(req.DGD) if err != nil { return false, fmt.Errorf("compute desired Grove worker hash: %w", err) } allCanonical := true allUnstamped := acceptAllUnstamped - for i := range dgd.Spec.Components { - component := &dgd.Spec.Components[i] + for i := range managedComponents { + component := &managedComponents[i] if !dynamo.IsWorkerComponent(string(component.ComponentType)) { continue } @@ -260,39 +212,6 @@ func podCliqueSetCliqueForComponent( return nil } -func applyGroveCompatibility( - dgd *nvidiacomv1beta1.DynamoGraphDeployment, - pcs *grovev1alpha1.PodCliqueSet, -) { - for i := range dgd.Spec.Components { - component := &dgd.Spec.Components[i] - componentType := string(component.ComponentType) - if !groveComponentTypeCanUseLegacyWorkerSelector(componentType) { - continue - } - if podCliqueSetHasLegacyWorkerSelector(pcs, component.ComponentName, componentType) { - applyLegacyGroveWorkerComponentType(component, componentType) - } - } -} - -func restartAnnotationsFromPodCliqueSet(pcs *grovev1alpha1.PodCliqueSet) map[string]string { - restartAnnotations := make(map[string]string) - if pcs == nil { - return restartAnnotations - } - for _, clique := range pcs.Spec.Template.Cliques { - if clique.Annotations != nil { - if timestamp, ok := clique.Annotations[commonconsts.RestartAnnotation]; ok { - if componentName, ok := clique.Labels[commonconsts.KubeLabelDynamoComponent]; ok { - restartAnnotations[componentName] = timestamp - } - } - } - } - return restartAnnotations -} - func preserveGrovePodCliqueSetOrder( desired *grovev1alpha1.PodCliqueSet, existing *grovev1alpha1.PodCliqueSet, @@ -514,43 +433,3 @@ func podCliqueScalingGroupConfigName(config grovev1alpha1.PodCliqueScalingGroupC func resourceClaimTemplateConfigName(config grovev1alpha1.ResourceClaimTemplateConfig) string { return config.Name } - -func groveComponentTypeCanUseLegacyWorkerSelector(componentType string) bool { - return componentType == commonconsts.ComponentTypePrefill || - componentType == commonconsts.ComponentTypeDecode -} - -func podCliqueSetHasLegacyWorkerSelector( - pcs *grovev1alpha1.PodCliqueSet, - componentName string, - componentType string, -) bool { - if pcs == nil { - return false - } - for _, clique := range pcs.Spec.Template.Cliques { - if clique == nil || clique.Labels[commonconsts.KubeLabelDynamoComponent] != componentName { - continue - } - if hasLegacyWorkerSelector(clique.Labels, componentType) { - return true - } - } - return false -} - -func applyLegacyGroveWorkerComponentType( - component *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec, - subComponentType string, -) { - component.ComponentType = nvidiacomv1beta1.ComponentTypeWorker - if component.PodTemplate == nil { - component.PodTemplate = &corev1.PodTemplateSpec{} - } - if component.PodTemplate.Labels == nil { - component.PodTemplate.Labels = map[string]string{} - } - if _, ok := component.PodTemplate.Labels[commonconsts.KubeLabelDynamoSubComponentType]; !ok { - component.PodTemplate.Labels[commonconsts.KubeLabelDynamoSubComponentType] = subComponentType - } -} diff --git a/deploy/operator/internal/controller/dgd_grove_workloads_reconciler.go b/deploy/operator/internal/controller/dgd_grove_workloads_reconciler.go index 5a073de6fb68..6d58154ccbc6 100644 --- a/deploy/operator/internal/controller/dgd_grove_workloads_reconciler.go +++ b/deploy/operator/internal/controller/dgd_grove_workloads_reconciler.go @@ -71,14 +71,15 @@ func newGroveWorkloadsReconciler( func (r *groveWorkloadsReconciler) Reconcile( ctx context.Context, - source *nvidiacomv1beta1.DynamoGraphDeployment, - ordinary *nvidiacomv1beta1.DynamoGraphDeployment, + req groveReconcileRequest, restartState *dynamo.RestartState, checkpointInfos map[string]*checkpoint.CheckpointInfo, ) (ReconcileResult, error) { - // An LPX-only graph still reconciles its stable resources. - if len(ordinary.Spec.Components) == 0 { - stableResources, err := r.stableResources.Reconcile(ctx, source, ordinary) + managedComponents := req.ManagedComponents() + + // A graph with only external components still reconciles graph-wide stable resources. + if len(managedComponents) == 0 { + stableResources, err := r.stableResources.Reconcile(ctx, req, nil) if err != nil { return ReconcileResult{}, err } @@ -87,13 +88,13 @@ func (r *groveWorkloadsReconciler) Reconcile( logger := log.FromContext(ctx) - workerHashTransition, err := r.rollout.planUnsupportedWorkerHashTransition(source) + workerHashTransition, err := r.rollout.planUnsupportedWorkerHashTransition(req.DGD) if err != nil { return ReconcileResult{}, failWorkloadProgram(reasonRollingUpdateFailed, err) } renderedPodCliqueSet, err := r.renderer.Render( ctx, - ordinary, + req, restartState, checkpointInfos, workerHashTransition.hashChanged, @@ -104,7 +105,7 @@ func (r *groveWorkloadsReconciler) Reconcile( } // Converge the ordinary PCS before rollout or readiness observation. - syncedPodCliqueSet, pcsWasWritten, err := r.reconcilePodCliqueSet(ctx, source, renderedPodCliqueSet) + syncedPodCliqueSet, pcsWasWritten, err := r.reconcilePodCliqueSet(ctx, req.DGD, renderedPodCliqueSet) if err != nil { return ReconcileResult{}, fmt.Errorf("failed to reconcile the Grove PodCliqueSet: %w", err) } @@ -114,8 +115,8 @@ func (r *groveWorkloadsReconciler) Reconcile( // Pre-existing legacy PCS without a suffix is the only case where unstamped is valid. isLegacyUnsuffixed := workerHashTransition.noCurrentAnnotation && renderedPodCliqueSet.existing != nil && - !podCliqueSetUsesGroveWorkerHashSuffix(source, renderedPodCliqueSet.existing) - observed, err := podCliqueSetObservesWorkerHash(source, renderedPodCliqueSet.existing, isLegacyUnsuffixed) + !podCliqueSetUsesGroveWorkerHashSuffix(req, renderedPodCliqueSet.existing) + observed, err := podCliqueSetObservesWorkerHash(req, renderedPodCliqueSet.existing, isLegacyUnsuffixed) if err != nil { return ReconcileResult{}, failWorkloadProgram( reasonRollingUpdateFailed, @@ -123,7 +124,7 @@ func (r *groveWorkloadsReconciler) Reconcile( ) } if observed { - if err := r.rollout.commitUnsupportedWorkerHashTransition(ctx, source, workerHashTransition, true); err != nil { + if err := r.rollout.commitUnsupportedWorkerHashTransition(ctx, req.DGD, workerHashTransition, true); err != nil { return ReconcileResult{}, failWorkloadProgram( reasonRollingUpdateFailed, fmt.Errorf("project observed Grove worker hash: %w", err), @@ -132,19 +133,19 @@ func (r *groveWorkloadsReconciler) Reconcile( } } - if err := r.scaler.Reconcile(ctx, renderedPodCliqueSet.renderDeployment, checkpointInfos); err != nil { + if err := r.scaler.Reconcile(ctx, req, checkpointInfos); err != nil { logger.Error(err, "failed to reconcile Grove scaling") return ReconcileResult{}, fmt.Errorf("failed to reconcile Grove scaling: %w", err) } - stableResources, err := r.stableResources.Reconcile(ctx, source, renderedPodCliqueSet.renderDeployment) + stableResources, err := r.stableResources.Reconcile(ctx, req, syncedPodCliqueSet) if err != nil { return ReconcileResult{}, err } podCliqueSetResource, readiness, err := r.observePodCliqueSetReadiness( ctx, - renderedPodCliqueSet.renderDeployment, + req, syncedPodCliqueSet, ) if err != nil { @@ -206,10 +207,10 @@ func (r *groveWorkloadsReconciler) reconcilePodCliqueSet( // readiness interface without further Kubernetes reads. func (r *groveWorkloadsReconciler) observePodCliqueSetReadiness( ctx context.Context, - dgd *nvidiacomv1beta1.DynamoGraphDeployment, + req groveReconcileRequest, podCliqueSet *grovev1alpha1.PodCliqueSet, ) (*commoncontroller.Resource, dynamo.GroveReadiness, error) { - readiness, err := dynamo.EvaluateGroveReadiness(ctx, r.reader, dgd, podCliqueSet) + readiness, err := dynamo.EvaluateGroveReadiness(ctx, r.reader, req.DGD, req.IsDelegated, podCliqueSet) if err != nil { return nil, dynamo.GroveReadiness{}, err } diff --git a/deploy/operator/internal/controller/dgd_grove_workloads_reconciler_test.go b/deploy/operator/internal/controller/dgd_grove_workloads_reconciler_test.go index 595b56b0098e..933ab2986ada 100644 --- a/deploy/operator/internal/controller/dgd_grove_workloads_reconciler_test.go +++ b/deploy/operator/internal/controller/dgd_grove_workloads_reconciler_test.go @@ -48,6 +48,32 @@ import ( const updatedWorkerVersion = "new" +func TestGroveReconcileRequestUsesDelegationPredicate(t *testing.T) { + dgd := newLPXHandoffSource(t, "node-local-v2-hybrid") + dgd.Spec.Components = append(dgd.Spec.Components, nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec{ + ComponentName: "frontend", + ComponentType: nvidiacomv1beta1.ComponentTypeFrontend, + }) + req := groveReconcileRequest{ + DGD: dgd, + IsDelegated: func(component *nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec) bool { + return component.ComponentName == "lpx" + }, + } + + t.Log("Select exactly the component managed by Grove") + components := req.ManagedComponents() + require.Len(t, components, 1) + require.Equal(t, "frontend", components[0].ComponentName) + require.False(t, components[0].IsLPX()) + require.Equal(t, []nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec{dgd.Spec.Components[0]}, req.DelegatedComponents()) + + t.Log("Treat every component as managed when no delegation predicate is supplied") + defaultReq := groveReconcileRequest{DGD: dgd} + require.Equal(t, dgd.Spec.Components, defaultReq.ManagedComponents()) + require.Empty(t, defaultReq.DelegatedComponents()) +} + func TestGroveWorkloadsReconciler_EvaluatesReadinessOnce(t *testing.T) { t.Log("Build a ready frontend alongside an independently owned LPX component") dgd := betaDGD(t, &nvidiacomv1alpha1.DynamoGraphDeployment{ @@ -119,8 +145,7 @@ func TestGroveWorkloadsReconciler_EvaluatesReadinessOnce(t *testing.T) { t.Log("Reconcile workloads and reuse the single child observation for readiness") result, err := reconciler.newGroveProgram().workloads.Reconcile( context.Background(), - dgd, - projectWithoutExternallyManagedComponents(dgd), + groveReconcileRequest{DGD: dgd, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil, ) @@ -137,11 +162,10 @@ func TestGroveWorkloadsReconciler_EvaluatesReadinessOnce(t *testing.T) { } func TestGroveWorkloadsReconcilerUsesStableReadinessWithoutOrdinaryPodCliqueSet(t *testing.T) { - t.Log("Project an LPX-only graph without an ordinary PodCliqueSet") + t.Log("Use an LPX-only graph without an ordinary PodCliqueSet") const dgdName = "graph" source := newLPXHandoffSource(t, "node-local-v2-lpu-only") source.Name, source.Namespace, source.UID = dgdName, corev1.NamespaceDefault, "dgd-uid" - ordinary := projectWithoutExternallyManagedComponents(source) pcsReads := 0 kubeClient := fake.NewClientBuilder(). WithScheme(newDynamoGraphDeploymentControllerTestScheme(t)). @@ -167,7 +191,7 @@ func TestGroveWorkloadsReconcilerUsesStableReadinessWithoutOrdinaryPodCliqueSet( } t.Log("Report stable-resource readiness without looking up an ordinary PodCliqueSet") - result, err := reconciler.newGroveProgram().workloads.Reconcile(t.Context(), source, ordinary, nil, nil) + result, err := reconciler.newGroveProgram().workloads.Reconcile(t.Context(), groveReconcileRequest{DGD: source, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil) require.NoError(t, err) require.Equal(t, nvidiacomv1beta1.DGDStateSuccessful, result.State) require.Zero(t, pcsReads) @@ -206,7 +230,7 @@ func TestGroveWorkloadsReconciler_DoesNotCommitWorkerHashWhenPodCliqueSetSyncFai if tt.existingPCS { existingPCS = &grovev1alpha1.PodCliqueSet{ ObjectMeta: metav1.ObjectMeta{ - Name: dynamo.PCSNameForDGD(dgd.Name, dgd.Spec.Components), + Name: dynamo.PCSNameForDGD(dgd, nil), Namespace: dgd.Namespace, OwnerReferences: []metav1.OwnerReference{ *metav1.NewControllerRef(dgd, nvidiacomv1beta1.GroupVersion.WithKind("DynamoGraphDeployment")), @@ -277,7 +301,7 @@ func TestGroveWorkloadsReconciler_DoesNotCommitWorkerHashWhenPodCliqueSetSyncFai t.Log("Reconcile the full workload transition") observedDGD := &nvidiacomv1beta1.DynamoGraphDeployment{} require.NoError(t, kubeClient.Get(context.Background(), client.ObjectKeyFromObject(dgd), observedDGD)) - _, err = workloads.Reconcile(context.Background(), observedDGD, projectWithoutExternallyManagedComponents(observedDGD), nil, nil) + _, err = workloads.Reconcile(context.Background(), groveReconcileRequest{DGD: observedDGD, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil) t.Log("Verify the failed PCS sync leaves the persisted DGD hash unchanged") require.Error(t, err) @@ -308,7 +332,7 @@ func TestGroveWorkloadsReconciler_RecoversWorkerHashCommitAfterPodCliqueSetSync( require.NoError(t, err) legacyPCS := &grovev1alpha1.PodCliqueSet{ ObjectMeta: metav1.ObjectMeta{ - Name: dynamo.PCSNameForDGD(dgd.Name, dgd.Spec.Components), + Name: dynamo.PCSNameForDGD(dgd, nil), Namespace: dgd.Namespace, OwnerReferences: []metav1.OwnerReference{ *metav1.NewControllerRef(dgd, nvidiacomv1beta1.GroupVersion.WithKind("DynamoGraphDeployment")), @@ -364,7 +388,7 @@ func TestGroveWorkloadsReconciler_RecoversWorkerHashCommitAfterPodCliqueSetSync( t.Log("Persist the PCS suffix without projecting the DGD hash from the write receipt") observedDGD := &nvidiacomv1beta1.DynamoGraphDeployment{} require.NoError(t, kubeClient.Get(context.Background(), client.ObjectKeyFromObject(dgd), observedDGD)) - _, err = workloads.Reconcile(context.Background(), observedDGD, projectWithoutExternallyManagedComponents(observedDGD), nil, nil) + _, err = workloads.Reconcile(context.Background(), groveReconcileRequest{DGD: observedDGD, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil) require.NoError(t, err) t.Log("Verify the write receipt leaves the parent hash unchanged") @@ -383,7 +407,7 @@ func TestGroveWorkloadsReconciler_RecoversWorkerHashCommitAfterPodCliqueSetSync( t.Log("Observe the suffix on a later reconcile, then reject the parent projection") freshDGD := &nvidiacomv1beta1.DynamoGraphDeployment{} require.NoError(t, kubeClient.Get(context.Background(), client.ObjectKeyFromObject(dgd), freshDGD)) - _, err = workloads.Reconcile(context.Background(), freshDGD, projectWithoutExternallyManagedComponents(freshDGD), nil, nil) + _, err = workloads.Reconcile(context.Background(), groveReconcileRequest{DGD: freshDGD, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil) require.Error(t, err) assert.Equal(t, 1, pcsUpdateCalls) assert.Equal(t, 1, dgdUpdateCalls) @@ -392,7 +416,7 @@ func TestGroveWorkloadsReconciler_RecoversWorkerHashCommitAfterPodCliqueSetSync( failDGDUpdate = false freshDGD = &nvidiacomv1beta1.DynamoGraphDeployment{} require.NoError(t, kubeClient.Get(context.Background(), client.ObjectKeyFromObject(dgd), freshDGD)) - _, err = workloads.Reconcile(context.Background(), freshDGD, projectWithoutExternallyManagedComponents(freshDGD), nil, nil) + _, err = workloads.Reconcile(context.Background(), groveReconcileRequest{DGD: freshDGD, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil) require.NoError(t, err) t.Log("Verify the retry commits the target hash without rewriting the PCS") @@ -405,7 +429,7 @@ func TestGroveWorkloadsReconciler_RecoversWorkerHashCommitAfterPodCliqueSetSync( t.Log("Verify the completed transition is idempotent") idempotentDGD := &nvidiacomv1beta1.DynamoGraphDeployment{} require.NoError(t, kubeClient.Get(context.Background(), client.ObjectKeyFromObject(dgd), idempotentDGD)) - _, err = workloads.Reconcile(context.Background(), idempotentDGD, projectWithoutExternallyManagedComponents(idempotentDGD), nil, nil) + _, err = workloads.Reconcile(context.Background(), groveReconcileRequest{DGD: idempotentDGD, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil) require.NoError(t, err) assert.Equal(t, 1, pcsUpdateCalls) assert.Equal(t, 2, dgdUpdateCalls) @@ -625,7 +649,7 @@ func TestPodCliqueSetObservesWorkerHash(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := podCliqueSetObservesWorkerHash(dgd, tt.pcs, tt.acceptAllUnstamped) + got, err := podCliqueSetObservesWorkerHash(groveReconcileRequest{DGD: dgd, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, tt.pcs, tt.acceptAllUnstamped) require.NoError(t, err) assert.Equal(t, tt.want, got) }) @@ -647,7 +671,7 @@ func TestGroveWorkloadsReconciler_SkipsHashObservationWhenHashIsCurrent(t *testi t.Log("Seed a PCS carrying the current hash so the sync is a no-op") existingPCS := &grovev1alpha1.PodCliqueSet{ ObjectMeta: metav1.ObjectMeta{ - Name: dynamo.PCSNameForDGD(dgd.Name, dgd.Spec.Components), + Name: dynamo.PCSNameForDGD(dgd, nil), Namespace: dgd.Namespace, OwnerReferences: []metav1.OwnerReference{ *metav1.NewControllerRef(dgd, nvidiacomv1beta1.GroupVersion.WithKind("DynamoGraphDeployment")), @@ -689,7 +713,7 @@ func TestGroveWorkloadsReconciler_SkipsHashObservationWhenHashIsCurrent(t *testi t.Log("Reconcile: hash observation block must be skipped entirely when needsCommit is false") observedDGD := &nvidiacomv1beta1.DynamoGraphDeployment{} require.NoError(t, kubeClient.Get(context.Background(), client.ObjectKeyFromObject(dgd), observedDGD)) - _, err = workloads.Reconcile(context.Background(), observedDGD, projectWithoutExternallyManagedComponents(observedDGD), nil, nil) + _, err = workloads.Reconcile(context.Background(), groveReconcileRequest{DGD: observedDGD, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil) require.NoError(t, err) assert.Zero(t, dgdUpdateCalls, "DGD must not be updated when the hash annotation is already current") @@ -717,7 +741,7 @@ func TestGroveWorkloadsReconciler_DefersHashCommitUntilPCSWriteObserved(t *testi name: "legacy unsuffixed PCS — bookkeeping write defers commit, second reconcile commits", existingPCS: &grovev1alpha1.PodCliqueSet{ ObjectMeta: metav1.ObjectMeta{ - Name: dynamo.PCSNameForDGD(dgd.Name, dgd.Spec.Components), + Name: dynamo.PCSNameForDGD(dgd, nil), Namespace: dgd.Namespace, OwnerReferences: []metav1.OwnerReference{ *metav1.NewControllerRef(dgd, nvidiacomv1beta1.GroupVersion.WithKind("DynamoGraphDeployment")), @@ -768,14 +792,14 @@ func TestGroveWorkloadsReconciler_DefersHashCommitUntilPCSWriteObserved(t *testi t.Log("First reconcile writes the PCS; commit must be deferred") observedDGD := &nvidiacomv1beta1.DynamoGraphDeployment{} require.NoError(t, kubeClient.Get(context.Background(), client.ObjectKeyFromObject(dgd), observedDGD)) - _, err = workloads.Reconcile(context.Background(), observedDGD, projectWithoutExternallyManagedComponents(observedDGD), nil, nil) + _, err = workloads.Reconcile(context.Background(), groveReconcileRequest{DGD: observedDGD, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil) require.NoError(t, err) assert.Zero(t, dgdUpdateCalls, "hash annotation must not be committed on the reconcile that writes the PCS") t.Log("Second reconcile is a no-op PCS sync; commit must proceed") freshDGD := &nvidiacomv1beta1.DynamoGraphDeployment{} require.NoError(t, kubeClient.Get(context.Background(), client.ObjectKeyFromObject(dgd), freshDGD)) - _, err = workloads.Reconcile(context.Background(), freshDGD, projectWithoutExternallyManagedComponents(freshDGD), nil, nil) + _, err = workloads.Reconcile(context.Background(), groveReconcileRequest{DGD: freshDGD, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil) require.NoError(t, err) assert.Equal(t, 1, dgdUpdateCalls, "hash annotation must be committed once the PCS write is observed") diff --git a/deploy/operator/internal/controller/dgd_lpx_handoff_test.go b/deploy/operator/internal/controller/dgd_lpx_handoff_test.go index 6c3fe9e6a81f..7d4942344d7f 100644 --- a/deploy/operator/internal/controller/dgd_lpx_handoff_test.go +++ b/deploy/operator/internal/controller/dgd_lpx_handoff_test.go @@ -63,7 +63,7 @@ func newLPXHandoffFixture(t *testing.T, fixture string) (*v1alpha1.LPXGraphDeplo return child, source, kube } -func TestProjectWithoutExternallyManagedComponentsDoesNotMutateSource(t *testing.T) { +func TestGroveComponentSelectionDoesNotMutateDGD(t *testing.T) { for _, test := range []struct { name string longNames bool @@ -74,7 +74,7 @@ func TestProjectWithoutExternallyManagedComponentsDoesNotMutateSource(t *testing {name: "LPX only", lpxOnly: true}, } { t.Run(test.name, func(t *testing.T) { - t.Log("Project the exact DGD-owned subset without changing the source") + t.Log("Select the exact managed subset without changing the DGD") source := newLPXHandoffSource(t, "node-local-v2-hybrid") lpxComponent := &source.Spec.Components[0] if test.longNames { @@ -93,15 +93,21 @@ func TestProjectWithoutExternallyManagedComponentsDoesNotMutateSource(t *testing }) } before := source.DeepCopy() - ordinary := projectWithoutExternallyManagedComponents(source) - require.Nil(t, ordinary.GetComponentByName(lpxName)) - require.Len(t, ordinary.Spec.Components, len(source.Spec.Components)-1) + req := groveReconcileRequest{DGD: source, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController} + managed := req.ManagedComponents() + delegated := req.DelegatedComponents() + require.Len(t, managed, len(source.Spec.Components)-1) + require.Len(t, delegated, 1) + require.Equal(t, lpxName, delegated[0].ComponentName) + for i := range managed { + require.NotEqual(t, lpxName, managed[i].ComponentName) + } - t.Log("Seed the projected PCS only for mixed graphs, including the truncated-name case") - pcsName := dynamo.PCSNameForDGD(ordinary.Name, ordinary.Spec.Components) + t.Log("Seed the ordinary PCS only for mixed graphs, including the truncated-name case") + pcsName := dynamo.PCSNameForDGD(req.DGD, req.IsDelegated) if test.longNames { require.NotEqual(t, source.Name, pcsName) - require.NotEqual(t, dynamo.PCSNameForDGD(source.Name, source.Spec.Components), pcsName) + require.NotEqual(t, dynamo.PCSNameForDGD(source, nil), pcsName) } pcs := &grovev1alpha1.PodCliqueSet{ ObjectMeta: metav1.ObjectMeta{Name: pcsName, Namespace: source.Namespace, Generation: 1}, @@ -111,7 +117,7 @@ func TestProjectWithoutExternallyManagedComponentsDoesNotMutateSource(t *testing }}}, } clique := &grovev1alpha1.PodClique{ - ObjectMeta: metav1.ObjectMeta{Name: dynamo.GroveComponentResourceName(ordinary, "ordinary-frontend"), Namespace: source.Namespace, Generation: 1}, + ObjectMeta: metav1.ObjectMeta{Name: dynamo.GroveComponentResourceName(pcsName, "ordinary-frontend"), Namespace: source.Namespace, Generation: 1}, Spec: grovev1alpha1.PodCliqueSpec{Replicas: 1}, Status: grovev1alpha1.PodCliqueStatus{ObservedGeneration: ptr.To(int64(1)), Replicas: 1, UpdatedReplicas: 1, ReadyReplicas: 1}, } @@ -124,19 +130,19 @@ func TestProjectWithoutExternallyManagedComponentsDoesNotMutateSource(t *testing t.Log("Render and observe restarts against that same ordinary PCS") if !test.lpxOnly { renderer := newGroveWorkloadRenderer(kube, &configv1alpha1.OperatorConfiguration{}, &commoncontroller.RuntimeConfig{}, nil) - rendered, err := renderer.Render(t.Context(), ordinary, nil, nil, false) + rendered, err := renderer.Render(t.Context(), req, nil, nil, false) require.NoError(t, err) require.NotNil(t, rendered.existing) require.Equal(t, pcsName, rendered.existing.Name) require.Equal(t, pcsName, rendered.desired.Name) } - remaining := resolveCompositeGroveRestartProgress(t.Context(), source, ordinary, + remaining := resolveCompositeGroveRestartProgress(t.Context(), req, []string{lpxName, "ordinary-frontend"}, newGroveRestartProgressResolver(kube), newLPXRestartProgressResolver(kube)) require.Equal(t, []string{lpxName}, remaining) t.Log("Observe the graph-level topology through the same ordinary PCS") result := newWorkloadProgramResult(source) - newDGDGroveTopologyConditionReconciler(kube).Reconcile(t.Context(), ordinary, &result) + newDGDGroveTopologyConditionReconciler(kube).Reconcile(t.Context(), req, &result) condition := meta.FindStatusCondition(result.Status.Conditions, v1beta1.ConditionTypeTopologyLevelsAvailable) if test.lpxOnly { require.Nil(t, condition) @@ -148,10 +154,10 @@ func TestProjectWithoutExternallyManagedComponentsDoesNotMutateSource(t *testing require.Equal(t, "missing ordinary topology", condition.Message) } - t.Log("Keep the source unchanged after observations and projected component-slice edits") + t.Log("Keep the DGD unchanged after observations and managed component-slice edits") require.Equal(t, before, source) - if len(ordinary.Spec.Components) > 0 { - ordinary.Spec.Components[0].ComponentName = "ordinary-copy" + if len(managed) > 0 { + managed[0].ComponentName = "ordinary-copy" } require.Equal(t, before, source) }) @@ -333,10 +339,9 @@ func TestLPXRestartUsesPersistedSelectionAndCurrentChildStatus(t *testing.T) { program := (&DynamoGraphDeploymentReconciler{ Client: kube, RuntimeConfig: &commoncontroller.RuntimeConfig{}, }).newGroveProgram() - ordinaryDGD := projectWithoutExternallyManagedComponents(source) progress := func(ctx context.Context, source *v1beta1.DynamoGraphDeployment, inProgress []string) []string { return resolveCompositeGroveRestartProgress( - ctx, source, ordinaryDGD, inProgress, program.restartProgress, program.lpxRestartProgress, + ctx, groveReconcileRequest{DGD: source, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, inProgress, program.restartProgress, program.lpxRestartProgress, ) } restart := newDGDRestartReconciler().Resolve(t.Context(), source, &source.Status, progress) @@ -407,16 +412,17 @@ func TestParallelRestartStartsAllLPXAndOrdinaryWorkloadsBeforeReadiness(t *testi _, err := program.Reconcile(t.Context(), workloadProgramRequest{DGD: source}) require.NoError(t, err) - ordinary := projectWithoutExternallyManagedComponents(source) + groveReq := groveReconcileRequest{DGD: source, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController} + pcsName := dynamo.PCSNameForDGD(groveReq.DGD, groveReq.IsDelegated) pcs := &grovev1alpha1.PodCliqueSet{} require.NoError(t, kube.Get(t.Context(), client.ObjectKey{ - Namespace: source.Namespace, Name: dynamo.PCSNameForDGD(ordinary.Name, ordinary.Spec.Components), + Namespace: source.Namespace, Name: pcsName, }, pcs)) pcs.Generation = 1 pcs.Status.ObservedGeneration = ptr.To(pcs.Generation) require.NoError(t, kube.Update(t.Context(), pcs)) frontend := &grovev1alpha1.PodClique{ - ObjectMeta: metav1.ObjectMeta{Name: dynamo.GroveComponentResourceName(ordinary, "frontend"), Namespace: source.Namespace, Generation: 1}, + ObjectMeta: metav1.ObjectMeta{Name: dynamo.GroveComponentResourceName(pcsName, "frontend"), Namespace: source.Namespace, Generation: 1}, Spec: grovev1alpha1.PodCliqueSpec{Replicas: 1}, Status: grovev1alpha1.PodCliqueStatus{ObservedGeneration: ptr.To(int64(1)), Replicas: 1, UpdatedReplicas: 1, ReadyReplicas: 1}, } @@ -591,10 +597,9 @@ func TestSpecDecodeRestartRollsTheSharedChildOnce(t *testing.T) { program := (&DynamoGraphDeploymentReconciler{ Client: observed, RuntimeConfig: &commoncontroller.RuntimeConfig{}, }).newGroveProgram() - ordinaryDGD := projectWithoutExternallyManagedComponents(source) progress := func(ctx context.Context, source *v1beta1.DynamoGraphDeployment, inProgress []string) []string { return resolveCompositeGroveRestartProgress( - ctx, source, ordinaryDGD, inProgress, program.restartProgress, program.lpxRestartProgress, + ctx, groveReconcileRequest{DGD: source, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, inProgress, program.restartProgress, program.lpxRestartProgress, ) } restarter := newDGDRestartReconciler() @@ -646,7 +651,6 @@ func TestSpecDecodeRestartRollsTheSharedChildOnce(t *testing.T) { v1beta1.DynamoComponentDeploymentSharedSpec{ComponentName: "frontend", ComponentType: v1beta1.ComponentTypeFrontend, Replicas: ptr.To(int32(1))}, v1beta1.DynamoComponentDeploymentSharedSpec{ComponentName: "prefill", ComponentType: v1beta1.ComponentTypePrefill, Replicas: ptr.To(int32(1))}, ) - ordinaryDGD = projectWithoutExternallyManagedComponents(source) requested := []string{"draft", "frontend", "lpx", "prefill", "removed"} beforeSource := source.DeepCopy() childReads, pcsReads = 0, 0 @@ -655,8 +659,9 @@ func TestSpecDecodeRestartRollsTheSharedChildOnce(t *testing.T) { require.Equal(t, 1, pcsReads) t.Log("An unobserved ordinary PCS cannot hold back either ready LPX member") + groveReq := groveReconcileRequest{DGD: source, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController} pcs := &grovev1alpha1.PodCliqueSet{ - ObjectMeta: metav1.ObjectMeta{Name: dynamo.PCSNameForDGD(ordinaryDGD.Name, ordinaryDGD.Spec.Components), Namespace: source.Namespace, Generation: 2}, + ObjectMeta: metav1.ObjectMeta{Name: dynamo.PCSNameForDGD(groveReq.DGD, groveReq.IsDelegated), Namespace: source.Namespace, Generation: 2}, Status: grovev1alpha1.PodCliqueSetStatus{ObservedGeneration: ptr.To(int64(1))}, } require.NoError(t, kube.Create(t.Context(), pcs)) @@ -669,7 +674,7 @@ func TestSpecDecodeRestartRollsTheSharedChildOnce(t *testing.T) { pcs.Status.ObservedGeneration = ptr.To(pcs.Generation) require.NoError(t, kube.Update(t.Context(), pcs)) prefill := &grovev1alpha1.PodClique{ - ObjectMeta: metav1.ObjectMeta{Name: dynamo.GroveComponentResourceName(ordinaryDGD, "prefill"), Namespace: source.Namespace, Generation: 1}, + ObjectMeta: metav1.ObjectMeta{Name: dynamo.GroveComponentResourceName(dynamo.PCSNameForDGD(groveReq.DGD, groveReq.IsDelegated), "prefill"), Namespace: source.Namespace, Generation: 1}, Spec: grovev1alpha1.PodCliqueSpec{Replicas: 1}, Status: grovev1alpha1.PodCliqueStatus{ObservedGeneration: ptr.To(int64(1)), Replicas: 1, UpdatedReplicas: 1, ReadyReplicas: 1}, } @@ -797,22 +802,27 @@ func TestLPXPendingDownloadDoesNotBlockOrdinaryWorkloads(t *testing.T) { RuntimeConfig: &commoncontroller.RuntimeConfig{Gate: features.Gates{Grove: true, LPX: true, Checkpoint: true}}, } program := parent.newGroveProgram() - var adapterDeletes []string - program.scalingAdapters.Client = interceptor.NewClient(kube.(client.WithWatch), interceptor.Funcs{ - Delete: func(ctx context.Context, delegated client.WithWatch, obj client.Object, opts ...client.DeleteOption) error { - adapterDeletes = append(adapterDeletes, obj.GetName()) - return delegated.Delete(ctx, obj, opts...) + + t.Log("Leave a graph-owned adapter behind after ordinary component scaling was disabled") + disabledAdapter := &v1alpha1.DynamoGraphDeploymentScalingAdapter{ + ObjectMeta: metav1.ObjectMeta{ + Name: generateAdapterName(source.Name, "prefill"), Namespace: source.Namespace, + OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(source, v1beta1.DynamoGraphDeploymentGVK)}, }, - }) + Spec: v1alpha1.DynamoGraphDeploymentScalingAdapterSpec{ + DGDRef: v1alpha1.DynamoGraphDeploymentServiceRef{Name: source.Name, ServiceName: "prefill"}, + }, + } + require.NoError(t, kube.Create(t.Context(), disabledAdapter)) t.Log("Publish ordinary resources independently of the child's pending download") result, err := program.Reconcile(t.Context(), workloadProgramRequest{DGD: source}) require.NoError(t, err) require.Equal(t, v1beta1.DGDStatePending, result.Status.State) - require.Equal(t, []string{generateAdapterName(source.Name, "prefill")}, adapterDeletes) + require.True(t, apierrors.IsNotFound(kube.Get(t.Context(), client.ObjectKeyFromObject(disabledAdapter), disabledAdapter))) pcs := &grovev1alpha1.PodCliqueSet{} - ordinaryDGD := projectWithoutExternallyManagedComponents(source) - key := client.ObjectKey{Namespace: source.Namespace, Name: dynamo.PCSNameForDGD(ordinaryDGD.Name, ordinaryDGD.Spec.Components)} + groveReq := groveReconcileRequest{DGD: source, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController} + key := client.ObjectKey{Namespace: source.Namespace, Name: dynamo.PCSNameForDGD(groveReq.DGD, groveReq.IsDelegated)} require.NoError(t, kube.Get(t.Context(), key, pcs)) require.True(t, metav1.IsControlledBy(pcs, source)) require.Len(t, pcs.Spec.Template.Cliques, 1) @@ -883,3 +893,46 @@ func TestLPXPendingDownloadDoesNotBlockOrdinaryWorkloads(t *testing.T) { require.Contains(t, failed.Message, "storage volume mount is missing") require.Equal(t, v1beta1.ComponentCheckpointStatus{CheckpointName: friendlyCheckpointName}, result.Status.Checkpoints["prefill"]) } + +func TestGroveProgramPreservesAnotherGraphsScalingAdapter(t *testing.T) { + t.Log("Store two graphs whose component names produce the same adapter name") + source := newLPXHandoffSource(t, "node-local-v2-lpu-only") + source.Name = "graph" + source.Spec.Components[0].ComponentName = "foo-bar" + source.Annotations[consts.KubeAnnotationDynamoDiscoveryBackend] = string(configv1alpha1.DiscoveryBackendKubernetes) + ordinary := &v1beta1.DynamoGraphDeployment{ + ObjectMeta: metav1.ObjectMeta{Name: "graph-foo", Namespace: source.Namespace, UID: "ordinary-uid"}, + Spec: v1beta1.DynamoGraphDeploymentSpec{Components: []v1beta1.DynamoComponentDeploymentSharedSpec{{ + ComponentName: "bar", ComponentType: v1beta1.ComponentTypeWorker, + Replicas: ptr.To(int32(3)), ScalingAdapter: &v1beta1.ScalingAdapter{}, + }}}, + } + kube := fake.NewClientBuilder().WithScheme(newDynamoGraphDeploymentControllerTestScheme(t)). + WithObjects(source, ordinary).Build() + require.NoError(t, rbacv1.AddToScheme(kube.Scheme())) + + t.Log("Create the ordinary graph's adapter through the production reconciler") + require.NoError(t, newDGDScalingAdaptersReconciler(kube, nil).Reconcile(t.Context(), ordinary)) + key := client.ObjectKey{Namespace: source.Namespace, Name: generateAdapterName(ordinary.Name, "bar")} + require.Equal(t, key.Name, generateAdapterName(source.Name, source.Spec.Components[0].ComponentName)) + adapter := &v1alpha1.DynamoGraphDeploymentScalingAdapter{} + require.NoError(t, kube.Get(t.Context(), key, adapter)) + before := adapter.DeepCopy() + + t.Log("Reconcile the complete Grove program with the LPX graph") + config := &configv1alpha1.OperatorConfiguration{} + config.Namespace.Restricted = source.Namespace + program := (&DynamoGraphDeploymentReconciler{ + Client: kube, Config: config, Recorder: events.NewFakeRecorder(100), + RuntimeConfig: &commoncontroller.RuntimeConfig{Gate: features.Gates{Grove: true, LPX: true}}, + }).newGroveProgram() + _, err := program.Reconcile(t.Context(), workloadProgramRequest{DGD: source}) + require.NoError(t, err) + child := &v1alpha1.LPXGraphDeployment{} + require.NoError(t, kube.Get(t.Context(), client.ObjectKeyFromObject(source), child)) + require.True(t, metav1.IsControlledBy(child, source)) + + t.Log("Preserve the colliding adapter and its original ownership and replicas") + require.NoError(t, kube.Get(t.Context(), key, adapter)) + require.Equal(t, before, adapter) +} diff --git a/deploy/operator/internal/controller/dgd_scaling_adapters_reconciler.go b/deploy/operator/internal/controller/dgd_scaling_adapters_reconciler.go index 45445bd9a729..9a1c8513fe26 100644 --- a/deploy/operator/internal/controller/dgd_scaling_adapters_reconciler.go +++ b/deploy/operator/internal/controller/dgd_scaling_adapters_reconciler.go @@ -72,7 +72,18 @@ func (r *dgdScalingAdaptersReconciler) Reconcile( // Remove the adapter when component-level scaling is no longer enabled. if component.ScalingAdapter == nil { - if err := r.Delete(ctx, adapter); err != nil { + if err := r.Get(ctx, client.ObjectKeyFromObject(adapter), adapter); err != nil { + if apierrors.IsNotFound(err) { + continue + } + return fmt.Errorf("get scaling adapter %s: %w", adapterName, err) + } + if !metav1.IsControlledBy(adapter, dgd) { + continue + } + + // Bind deletion to the observed ownership, including concurrent owner changes. + if err := r.Delete(ctx, adapter, client.Preconditions{UID: &adapter.UID, ResourceVersion: &adapter.ResourceVersion}); err != nil { if apierrors.IsNotFound(err) { continue } @@ -169,8 +180,8 @@ func (r *dgdScalingAdaptersReconciler) Reconcile( adapter := &adapterList.Items[i] componentName := adapter.Spec.DGDRef.ServiceName - // Retain adapters whose component still exists. - if dgd.GetComponentByName(componentName) != nil { + // Labels alone do not establish ownership, even for a removed component. + if !metav1.IsControlledBy(adapter, dgd) || dgd.GetComponentByName(componentName) != nil { continue } @@ -179,7 +190,7 @@ func (r *dgdScalingAdaptersReconciler) Reconcile( "adapter", adapter.Name, "component", componentName, ) - if err := r.Delete(ctx, adapter); err != nil { + if err := r.Delete(ctx, adapter, client.Preconditions{UID: &adapter.UID, ResourceVersion: &adapter.ResourceVersion}); err != nil { if apierrors.IsNotFound(err) { continue } diff --git a/deploy/operator/internal/controller/dgd_worker_rollout_reconciler_test.go b/deploy/operator/internal/controller/dgd_worker_rollout_reconciler_test.go index 9e0eb4977001..6087417b0b9d 100644 --- a/deploy/operator/internal/controller/dgd_worker_rollout_reconciler_test.go +++ b/deploy/operator/internal/controller/dgd_worker_rollout_reconciler_test.go @@ -188,7 +188,7 @@ func TestGroveWorkerHashSuffixMigration(t *testing.T) { } t.Log("Verify suffix rendering from the worker generation") - if got := shouldRenderGroveWorkerHashSuffix(dgd, existing, tt.hashChanged); got != tt.wantSuffix { + if got := shouldRenderGroveWorkerHashSuffix(groveReconcileRequest{DGD: dgd, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, existing, tt.hashChanged); got != tt.wantSuffix { t.Fatalf("shouldRenderGroveWorkerHashSuffix() = %t, want %t", got, tt.wantSuffix) } }) @@ -242,7 +242,7 @@ func TestPlanUnsupportedWorkerHashTransitionDoesNotCommit(t *testing.T) { assert.Equal(t, currentHash, currentWorkerHashV2(dgd), "planning must not commit the DGD hash") } -func TestGroveRenderDeploymentWorkerHashSuffix(t *testing.T) { +func TestGenerateGrovePodCliqueSetWorkerHashSuffix(t *testing.T) { tests := []struct { name string workerHashSuffix bool @@ -269,25 +269,29 @@ func TestGroveRenderDeploymentWorkerHashSuffix(t *testing.T) { }) before := dgd.DeepCopy() - t.Log("Render the Grove deployment") - rendered, err := groveRenderDeployment(projectWithoutExternallyManagedComponents(dgd), nil, tt.workerHashSuffix) + t.Log("Render the managed Grove components") + req := groveReconcileRequest{DGD: dgd, IsDelegated: (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController} + pcs, err := dynamo.GenerateGrovePodCliqueSet( + t.Context(), dgd, req.IsDelegated, + &configv1alpha1.OperatorConfiguration{}, &commonController.RuntimeConfig{}, + nil, nil, nil, nil, tt.workerHashSuffix, nil, + ) require.NoError(t, err) - worker := rendered.GetComponentByName("worker") + worker := podCliqueSetCliqueForComponent(pcs, "worker") require.NotNil(t, worker) - require.Len(t, rendered.Spec.Components, 2) - require.Nil(t, rendered.GetComponentByName("lpx")) - require.Equal(t, "worker", rendered.Spec.Components[0].ComponentName) - require.Equal(t, "frontend", rendered.Spec.Components[1].ComponentName) - rendered.Spec.Components[1].PodTemplate.Labels["test"] = "rendered" + require.Len(t, pcs.Spec.Template.Cliques, 2) + require.Nil(t, podCliqueSetCliqueForComponent(pcs, "lpx")) + frontend := podCliqueSetCliqueForComponent(pcs, "frontend") + require.NotNil(t, frontend) + frontend.Labels["test"] = "rendered" t.Log("Verify the rendered suffix and source DGD immutability") if tt.workerHashSuffix { wantHash, err := dynamo.ComputeDGDWorkersSpecHash(dgd) require.NoError(t, err) - require.NotNil(t, worker.PodTemplate) - assert.Equal(t, wantHash, worker.PodTemplate.Labels[consts.KubeLabelDynamoWorkerHash]) + assert.Equal(t, wantHash, worker.Labels[consts.KubeLabelDynamoWorkerHash]) } else { - assert.Nil(t, worker.PodTemplate) + assert.Empty(t, worker.Labels[consts.KubeLabelDynamoWorkerHash]) } assert.Nil(t, dgd.GetComponentByName("worker").PodTemplate) assert.Equal(t, before, dgd) diff --git a/deploy/operator/internal/controller/dgd_workload_program_test.go b/deploy/operator/internal/controller/dgd_workload_program_test.go index 033830e35751..0c71f8beaede 100644 --- a/deploy/operator/internal/controller/dgd_workload_program_test.go +++ b/deploy/operator/internal/controller/dgd_workload_program_test.go @@ -485,14 +485,14 @@ func TestGroveRendererFailsWhenResolvedDRADependencyDisappears(t *testing.T) { ) t.Log("Verify the renderer initially publishes the full multinode shape") - rendered, err := renderer.Render(t.Context(), projectWithoutExternallyManagedComponents(dgd), nil, nil, false) + rendered, err := renderer.Render(t.Context(), groveReconcileRequest{DGD: dgd}, nil, nil, false) require.NoError(t, err) assert.Equal(t, int64(4), rendered.gpuShapes["decode"].GPUsPerEngine) assert.Equal(t, int64(4), rendered.gpuShapes["decode"].GPUsPerReplica) t.Log("Delete the dependency without changing DGD generation and render again") require.NoError(t, kubeClient.Delete(t.Context(), claimTemplate)) - _, err = renderer.Render(t.Context(), projectWithoutExternallyManagedComponents(dgd), nil, nil, false) + _, err = renderer.Render(t.Context(), groveReconcileRequest{DGD: dgd}, nil, nil, false) require.ErrorContains(t, err, "ResourceClaimTemplate default/gpu-template") } diff --git a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go index 1172ada3db4f..18c4f99fef98 100644 --- a/deploy/operator/internal/controller/dynamographdeployment_controller_test.go +++ b/deploy/operator/internal/controller/dynamographdeployment_controller_test.go @@ -504,6 +504,7 @@ func TestDGDScalingAdaptersReconciler_Reconcile(t *testing.T) { Kind: "DynamoGraphDeployment", Name: "test-dgd", UID: "test-uid", + Controller: ptr.To(true), }, }, }, @@ -528,6 +529,7 @@ func TestDGDScalingAdaptersReconciler_Reconcile(t *testing.T) { Kind: "DynamoGraphDeployment", Name: "test-dgd", UID: "test-uid", + Controller: ptr.To(true), }, }, }, @@ -577,6 +579,7 @@ func TestDGDScalingAdaptersReconciler_Reconcile(t *testing.T) { Kind: "DynamoGraphDeployment", Name: "test-dgd", UID: "test-uid", + Controller: ptr.To(true), }, }, }, @@ -712,80 +715,103 @@ func TestGenerateAdapterName(t *testing.T) { }) } -func TestDGDScalingAdaptersReconciler_EmitsDeleteEventOnlyAfterSuccessfulDelete(t *testing.T) { - notFound := apierrors.NewNotFound( - schema.GroupResource{ - Group: v1alpha1.GroupVersion.Group, - Resource: "dynamographdeploymentscalingadapters", - }, - "test-dgd-removed", - ) - +func TestDGDScalingAdaptersReconciler_DeletesOnlyObservedOwnedAdapters(t *testing.T) { tests := []struct { - name string - deleteErr error - wantEvent bool + name string + ownerUID types.UID + deleteErr error + changeOwner bool + wantDelete bool + wantEvent bool }{ + {name: "owned adapter", ownerUID: "test-uid", wantDelete: true, wantEvent: true}, + {name: "unowned adapter"}, + {name: "another graph owns the adapter", ownerUID: "other-uid"}, { - name: "successful delete emits event", - wantEvent: true, - }, - { - name: "already absent adapter emits no event", - deleteErr: notFound, + name: "adapter disappeared before delete", ownerUID: "test-uid", wantDelete: true, + deleteErr: apierrors.NewNotFound(schema.GroupResource{ + Group: v1alpha1.GroupVersion.Group, Resource: "dynamographdeploymentscalingadapters", + }, "test-dgd-worker"), }, + {name: "ownership changed before delete", ownerUID: "test-uid", changeOwner: true, wantDelete: true}, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - dgd := &v1beta1.DynamoGraphDeployment{ - ObjectMeta: metav1.ObjectMeta{Name: "test-dgd", Namespace: "default"}, - } - adapter := &v1alpha1.DynamoGraphDeploymentScalingAdapter{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-dgd-removed", - Namespace: "default", - Labels: map[string]string{ - commonconsts.KubeLabelDynamoGraphDeploymentName: dgd.Name, + for _, disabled := range []bool{false, true} { + for _, tt := range tests { + t.Run(fmt.Sprintf("disabled=%t/%s", disabled, tt.name), func(t *testing.T) { + t.Log("Store an adapter selected either by component name or by graph label") + dgd := &v1beta1.DynamoGraphDeployment{ + ObjectMeta: metav1.ObjectMeta{Name: "test-dgd", Namespace: "default", UID: "test-uid"}, + } + if disabled { + dgd.Spec.Components = []v1beta1.DynamoComponentDeploymentSharedSpec{{ComponentName: "worker"}} + } + adapter := &v1alpha1.DynamoGraphDeploymentScalingAdapter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-dgd-worker", Namespace: dgd.Namespace, UID: "adapter-uid", + Labels: map[string]string{commonconsts.KubeLabelDynamoGraphDeploymentName: dgd.Name}, }, - }, - Spec: v1alpha1.DynamoGraphDeploymentScalingAdapterSpec{ - DGDRef: v1alpha1.DynamoGraphDeploymentServiceRef{ - Name: dgd.Name, - ServiceName: "removed", + Spec: v1alpha1.DynamoGraphDeploymentScalingAdapterSpec{ + DGDRef: v1alpha1.DynamoGraphDeploymentServiceRef{Name: dgd.Name, ServiceName: "worker"}, }, - }, - } - kubeClient := fake.NewClientBuilder(). - WithScheme(newDynamoGraphDeploymentControllerTestScheme(t)). - WithObjects(dgd, adapter). - WithInterceptorFuncs(interceptor.Funcs{ - Delete: func( - ctx context.Context, - writer client.WithWatch, - obj client.Object, - opts ...client.DeleteOption, - ) error { - if tt.deleteErr != nil { - return tt.deleteErr - } - return writer.Delete(ctx, obj, opts...) - }, - }). - Build() - recorder := events.NewFakeRecorder(10) - reconciler := &DynamoGraphDeploymentReconciler{ - Client: kubeClient, - Recorder: recorder, - } + } + if tt.ownerUID != "" { + adapter.OwnerReferences = []metav1.OwnerReference{{ + APIVersion: v1beta1.GroupVersion.String(), Kind: "DynamoGraphDeployment", + Name: dgd.Name, UID: tt.ownerUID, Controller: ptr.To(true), + }} + } - require.NoError(t, newDGDScalingAdaptersReconciler(reconciler.Client, reconciler.Recorder).Reconcile(context.Background(), dgd)) - if tt.wantEvent { - assert.Len(t, recorder.Events, 1) - return - } - assert.Empty(t, recorder.Events) - }) + t.Log("Observe delete preconditions and simulate changes after the ownership read") + deleteCalled := false + kubeClient := fake.NewClientBuilder(). + WithScheme(newDynamoGraphDeploymentControllerTestScheme(t)). + WithObjects(dgd, adapter). + WithInterceptorFuncs(interceptor.Funcs{ + Delete: func(ctx context.Context, writer client.WithWatch, obj client.Object, opts ...client.DeleteOption) error { + deleteCalled = true + options := (&client.DeleteOptions{}).ApplyOptions(opts) + require.NotNil(t, options.Preconditions) + require.Equal(t, ptr.To(obj.GetUID()), options.Preconditions.UID) + require.NotEmpty(t, obj.GetResourceVersion()) + require.Equal(t, ptr.To(obj.GetResourceVersion()), options.Preconditions.ResourceVersion) + if tt.deleteErr != nil { + return tt.deleteErr + } + if tt.changeOwner { + replacement := obj.DeepCopyObject().(client.Object) + replacement.SetOwnerReferences(nil) + require.NoError(t, writer.Update(ctx, replacement)) + } + return writer.Delete(ctx, obj, opts...) + }, + }).Build() + recorder := events.NewFakeRecorder(10) + + t.Log("Reconcile and publish an event only for a successful owned deletion") + err := newDGDScalingAdaptersReconciler(kubeClient, recorder).Reconcile(t.Context(), dgd) + if tt.changeOwner { + require.True(t, apierrors.IsConflict(err), "expected conflict, got %v", err) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantDelete, deleteCalled) + if tt.wantEvent { + require.Len(t, recorder.Events, 1) + require.Contains(t, <-recorder.Events, "AdapterDeleted") + } else { + require.Empty(t, recorder.Events) + } + + t.Log("Preserve unrelated adapters and objects whose ownership changed concurrently") + err = kubeClient.Get(t.Context(), client.ObjectKeyFromObject(adapter), adapter) + if tt.wantEvent { + require.True(t, apierrors.IsNotFound(err), "expected deletion, got %v", err) + } else { + require.NoError(t, err) + } + }) + } } } @@ -1203,8 +1229,7 @@ func TestGroveWorkloadsReconciler_Reconcile(t *testing.T) { result, err := reconciler.newGroveProgram().workloads.Reconcile( ctx, - dgd, - projectWithoutExternallyManagedComponents(dgd), + groveReconcileRequest{DGD: dgd, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil, ) @@ -1245,8 +1270,7 @@ func TestGroveWorkloadsReconciler_Reconcile(t *testing.T) { result, err = reconciler.newGroveProgram().workloads.Reconcile( ctx, - dgd, - projectWithoutExternallyManagedComponents(dgd), + groveReconcileRequest{DGD: dgd, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil, ) @@ -1342,8 +1366,7 @@ func TestGroveWorkloadsReconciler_UsesPreservedAlphaServiceIngress(t *testing.T) _, err := reconciler.newGroveProgram().workloads.Reconcile( ctx, - dgd, - projectWithoutExternallyManagedComponents(dgd), + groveReconcileRequest{DGD: dgd, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil, ) @@ -1430,26 +1453,11 @@ func TestGroveWorkloadRendererRenderPreservesLegacyWorkerSelectors(t *testing.T) nil, ) - renderedPCS, err := renderer.Render(ctx, projectWithoutExternallyManagedComponents(dgd), nil, nil, false) + renderedPCS, err := renderer.Render(ctx, groveReconcileRequest{DGD: dgd, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil, false) g.Expect(err).NotTo(gomega.HaveOccurred()) generatedPCS := renderedPCS.desired - renderDGD := renderedPCS.renderDeployment g.Expect(dgd.GetComponentByName("VllmDecodeWorker").ComponentType).To(gomega.Equal(v1beta1.ComponentTypeDecode)) - prefill := renderDGD.GetComponentByName("VllmPrefillWorker") - if prefill == nil { - t.Fatal("expected rendered prefill component") - } - g.Expect(prefill.ComponentType).To(gomega.Equal(v1beta1.ComponentTypeWorker)) - g.Expect(prefill.PodTemplate.Labels[commonconsts.KubeLabelDynamoSubComponentType]).To(gomega.Equal(commonconsts.ComponentTypePrefill)) - - decode := renderDGD.GetComponentByName("VllmDecodeWorker") - if decode == nil { - t.Fatal("expected rendered decode component") - } - g.Expect(decode.ComponentType).To(gomega.Equal(v1beta1.ComponentTypeWorker)) - g.Expect(decode.PodTemplate.Labels[commonconsts.KubeLabelDynamoSubComponentType]).To(gomega.Equal(commonconsts.ComponentTypeDecode)) - g.Expect(generatedPCS.Spec.Template.Cliques[0].Name).To(gomega.Equal("vllmprefillworker")) var prefillClique *grovev1alpha1.PodCliqueTemplateSpec @@ -1466,16 +1474,23 @@ func TestGroveWorkloadRendererRenderPreservesLegacyWorkerSelectors(t *testing.T) g.Expect(prefillClique.Labels[commonconsts.KubeLabelDynamoSubComponentType]).To(gomega.Equal(commonconsts.ComponentTypePrefill)) g.Expect(prefillClique.Annotations[commonconsts.KubeAnnotationDynamoOperatorOriginVersion]).To(gomega.Equal("1.1.0")) - decodeService, err := dynamo.GenerateComponentService(dynamo.ComponentServiceParams{ - ServiceName: dynamo.GetDCDResourceName(renderDGD, "VllmDecodeWorker", ""), - Namespace: renderDGD.Namespace, - ComponentType: string(decode.ComponentType), - DynamoNamespace: renderDGD.GetDynamoNamespaceForComponent(decode), - ComponentName: "VllmDecodeWorker", - Labels: dynamo.GetDGDComponentResourceLabels(renderDGD, "VllmDecodeWorker", decode), - Annotations: dynamo.GetDGDComponentResourceAnnotations(renderDGD, "VllmDecodeWorker", decode), - IsK8sDiscovery: true, - }) + decodeClique := podCliqueSetCliqueForComponent(generatedPCS, "VllmDecodeWorker") + g.Expect(decodeClique).NotTo(gomega.BeNil()) + g.Expect(decodeClique.Labels[commonconsts.KubeLabelDynamoComponentType]).To(gomega.Equal(commonconsts.ComponentTypeWorker)) + g.Expect(decodeClique.Labels[commonconsts.KubeLabelDynamoSubComponentType]).To(gomega.Equal(commonconsts.ComponentTypeDecode)) + + stableResources := newGroveStableResourcesReconciler( + fakeKubeClient, + events.NewFakeRecorder(10), + &configv1alpha1.OperatorConfiguration{Discovery: configv1alpha1.DiscoveryConfiguration{Backend: configv1alpha1.DiscoveryBackendKubernetes}}, + ) + _, err = stableResources.Reconcile(ctx, groveReconcileRequest{DGD: dgd}, generatedPCS) + g.Expect(err).NotTo(gomega.HaveOccurred()) + decodeService := &corev1.Service{} + err = fakeKubeClient.Get(ctx, types.NamespacedName{ + Name: dynamo.GetDCDResourceName(dgd, "VllmDecodeWorker", ""), + Namespace: dgd.Namespace, + }, decodeService) g.Expect(err).NotTo(gomega.HaveOccurred()) g.Expect(decodeService.Spec.Selector[commonconsts.KubeLabelDynamoComponentType]).To(gomega.Equal(commonconsts.ComponentTypeWorker)) } @@ -1728,14 +1743,11 @@ func TestGroveWorkloadRendererRenderKeepsNativeWorkerSelectors(t *testing.T) { &controller_common.RuntimeConfig{}, nil, ) - renderedPCS, err := renderer.Render(ctx, projectWithoutExternallyManagedComponents(dgd), nil, nil, false) + renderedPCS, err := renderer.Render(ctx, groveReconcileRequest{DGD: dgd, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, nil, nil, false) g.Expect(err).NotTo(gomega.HaveOccurred()) - renderDGD := renderedPCS.renderDeployment - prefill := renderDGD.GetComponentByName("prefill") - if prefill == nil { - t.Fatal("expected rendered prefill component") - } - g.Expect(prefill.ComponentType).To(gomega.Equal(v1beta1.ComponentTypePrefill)) + prefill := podCliqueSetCliqueForComponent(renderedPCS.desired, "prefill") + g.Expect(prefill).NotTo(gomega.BeNil()) + g.Expect(prefill.Labels[commonconsts.KubeLabelDynamoComponentType]).To(gomega.Equal(commonconsts.ComponentTypePrefill)) } func TestDGDRestartReconciler_ComputeStatus(t *testing.T) { @@ -2706,7 +2718,10 @@ func TestDGDRestartReconciler_ComputeStatus(t *testing.T) { restartReconciler := newDGDRestartReconciler() var resolveProgress restartProgressResolver = newComponentRestartProgressResolver(reconciler.Client).Resolve if tt.groveEnabled { - resolveProgress = newGroveRestartProgressResolver(reconciler.Client).Resolve + resolver := newGroveRestartProgressResolver(reconciler.Client) + resolveProgress = func(ctx context.Context, dgd *v1beta1.DynamoGraphDeployment, inProgress []string) []string { + return resolver.Resolve(ctx, groveReconcileRequest{DGD: dgd, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, inProgress) + } } result := restartReconciler.computeRestartStatusWithProgressResolver(ctx, dgd, resolveProgress) @@ -3593,7 +3608,7 @@ func TestDGDGroveTopologyConditionReconciler_Reconcile(t *testing.T) { programResult := newWorkloadProgramResult(tt.dgd) if tt.groveEnabled { newDGDGroveTopologyConditionReconciler(reconciler.Client). - Reconcile(ctx, tt.dgd, &programResult) + Reconcile(ctx, groveReconcileRequest{DGD: tt.dgd, IsDelegated: (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController}, &programResult) } g.Expect(tt.dgd.Status).To(gomega.Equal(originalStatus), "status projection must not mutate request.DGD.Status") diff --git a/deploy/operator/internal/controller/lpx/render_test.go b/deploy/operator/internal/controller/lpx/render_test.go index 6f5dac4bba04..7553db102432 100644 --- a/deploy/operator/internal/controller/lpx/render_test.go +++ b/deploy/operator/internal/controller/lpx/render_test.go @@ -101,14 +101,10 @@ func TestGenerateGrovePodCliqueSet_FromDGDYaml(t *testing.T) { require.True(t, component.IsLPX()) } - t.Log("Render conventional components independently from their ordinary-only input") - ordinary := dynamoDeployment.DeepCopy() - ordinary.Spec.Components = slices.DeleteFunc(ordinary.Spec.Components, func(component v1beta1.DynamoComponentDeploymentSharedSpec) bool { - return component.IsLPX() - }) + t.Log("Render ordinary components independently from their explicit selection") normal, err := dynamo.GenerateGrovePodCliqueSet( - t.Context(), ordinary, controllerConfig, runtimeConfig, - kubeClient, nil, nil, nil, nil, + t.Context(), &dynamoDeployment, (*v1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController, controllerConfig, runtimeConfig, + kubeClient, nil, nil, nil, false, nil, ) require.NoError(t, err) require.NotEqual(t, normal.Name, got.Name) diff --git a/deploy/operator/internal/controller/upgrade_test.go b/deploy/operator/internal/controller/upgrade_test.go index a8c428d53863..bcdc6147d0a1 100644 --- a/deploy/operator/internal/controller/upgrade_test.go +++ b/deploy/operator/internal/controller/upgrade_test.go @@ -36,7 +36,9 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8sruntime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" kruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/client-go/tools/events" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -806,25 +808,25 @@ spec: &controller_common.RuntimeConfig{}, nil, ) - renderedPCS, err := renderer.Render(ctx, projectWithoutExternallyManagedComponents(dgd), nil, nil, false) + renderedPCS, err := renderer.Render(ctx, groveReconcileRequest{DGD: dgd}, nil, nil, false) require.NoError(t, err) pcs := renderedPCS.desired - renderDGD := renderedPCS.renderDeployment - t.Log("generate the decode service selector from the same prepared Grove component") - decodeComponent := renderDGD.GetComponentByName("VllmDecodeWorker") - require.NotNil(t, decodeComponent) - service, err := dynamo.GenerateComponentService(dynamo.ComponentServiceParams{ - ServiceName: dynamo.GetDCDResourceName(renderDGD, "VllmDecodeWorker", ""), - Namespace: renderDGD.Namespace, - ComponentType: string(decodeComponent.ComponentType), - DynamoNamespace: renderDGD.GetDynamoNamespaceForComponent(decodeComponent), - ComponentName: "VllmDecodeWorker", - Labels: dynamo.GetDGDComponentResourceLabels(renderDGD, "VllmDecodeWorker", decodeComponent), - Annotations: dynamo.GetDGDComponentResourceAnnotations(renderDGD, "VllmDecodeWorker", decodeComponent), - IsK8sDiscovery: true, - }) + t.Log("generate the decode service selector from the rendered Grove workload") + stableResources := newGroveStableResourcesReconciler( + reconciler.Client, + events.NewFakeRecorder(10), + &configv1alpha1.OperatorConfiguration{ + Discovery: configv1alpha1.DiscoveryConfiguration{Backend: configv1alpha1.DiscoveryBackendKubernetes}, + }, + ) + _, err = stableResources.Reconcile(ctx, groveReconcileRequest{DGD: dgd}, pcs) require.NoError(t, err) + service := &corev1.Service{} + require.NoError(t, reconciler.Client.Get(ctx, types.NamespacedName{ + Name: dynamo.GetDCDResourceName(dgd, "VllmDecodeWorker", ""), + Namespace: dgd.Namespace, + }, service)) return pcs, service.Spec.Selector }, childPodLabels: func(t *testing.T, obj client.Object) map[string]map[string]string { @@ -932,15 +934,11 @@ func TestGroveNativeWorkerIdentityLabelsStayNative(t *testing.T) { &controller_common.RuntimeConfig{}, nil, ) - renderedPCS, err := renderer.Render(ctx, projectWithoutExternallyManagedComponents(dgd), nil, nil, false) + renderedPCS, err := renderer.Render(ctx, groveReconcileRequest{DGD: dgd}, nil, nil, false) require.NoError(t, err) desired := renderedPCS.desired - renderDGD := renderedPCS.renderDeployment t.Log("assert the native prefill component stays prefill instead of legacy worker") - prefillComponent := renderDGD.GetComponentByName("prefill") - require.NotNil(t, prefillComponent) - require.Equal(t, v1beta1.ComponentTypePrefill, prefillComponent.ComponentType) prefillClique := requireGroveClique(t, desired, "prefill") require.Equal(t, commonconsts.ComponentTypePrefill, prefillClique.Labels[commonconsts.KubeLabelDynamoComponentType]) } diff --git a/deploy/operator/internal/dynamo/gpu_shape.go b/deploy/operator/internal/dynamo/gpu_shape.go index 85f6d826e420..88bebd213c4f 100644 --- a/deploy/operator/internal/dynamo/gpu_shape.go +++ b/deploy/operator/internal/dynamo/gpu_shape.go @@ -25,18 +25,22 @@ type GPUShape struct { GPUsPerReplica int64 } -// ResolveGroveGPUShapes computes one GPU shape per rendered DGD component. +// ResolveGroveGPUShapes computes one GPU shape per Grove-managed component. // Structural role multiplicities are used so checkpoint gating to zero does // not erase the future cost of one component replica. func ResolveGroveGPUShapes( ctx context.Context, reader client.Reader, dgd *v1beta1.DynamoGraphDeployment, + isDelegated func(*v1beta1.DynamoComponentDeploymentSharedSpec) bool, pcs *grovev1alpha1.PodCliqueSet, ) (map[string]GPUShape, error) { shapes := make(map[string]GPUShape) for i := range dgd.Spec.Components { component := &dgd.Spec.Components[i] + if isDelegatedComponent(component, isDelegated) { + continue + } roleCounts := make(map[string]int32) for _, role := range expandRolesForComponent( component.ComponentName, diff --git a/deploy/operator/internal/dynamo/gpu_shape_test.go b/deploy/operator/internal/dynamo/gpu_shape_test.go index 06a81860416f..1892bc1b21cc 100644 --- a/deploy/operator/internal/dynamo/gpu_shape_test.go +++ b/deploy/operator/internal/dynamo/gpu_shape_test.go @@ -154,7 +154,7 @@ func TestResolveGroveGPUShapesIncludesUntypedAndMultinodeComponents(t *testing.T }}, }} - shapes, err := ResolveGroveGPUShapes(t.Context(), nil, dgd, pcs) + shapes, err := ResolveGroveGPUShapes(t.Context(), nil, dgd, nil, pcs) require.NoError(t, err) assert.Equal(t, GPUShape{GPUsPerEngine: 2, GPUsPerReplica: 2}, shapes["custom"]) wantReplica := int64(8) @@ -218,7 +218,7 @@ func TestResolveGroveGPUShapesPublishesExplicitZero(t *testing.T) { require.NoError(t, resourcev1.AddToScheme(scheme)) reader := fake.NewClientBuilder().WithScheme(scheme).WithObjects(template, deviceClass).Build() - shapes, err := ResolveGroveGPUShapes(t.Context(), reader, dgd, pcs) + shapes, err := ResolveGroveGPUShapes(t.Context(), reader, dgd, nil, pcs) require.NoError(t, err) assert.Equal(t, GPUShape{}, shapes["frontend"]) } @@ -302,7 +302,7 @@ func TestResolveGroveGPUShapesCountsInterPodSharedGPUsOnce(t *testing.T) { } pcs, err := GenerateGrovePodCliqueSet( t.Context(), - dgd, + dgd, nil, &configv1alpha1.OperatorConfiguration{ Discovery: configv1alpha1.DiscoveryConfiguration{Backend: "kubernetes"}, Infrastructure: configv1alpha1.InfrastructureConfiguration{ @@ -315,11 +315,12 @@ func TestResolveGroveGPUShapesCountsInterPodSharedGPUsOnce(t *testing.T) { nil, nil, nil, + false, nil, ) require.NoError(t, err) - shapes, err := ResolveGroveGPUShapes(t.Context(), nil, dgd, pcs) + shapes, err := ResolveGroveGPUShapes(t.Context(), nil, dgd, nil, pcs) require.NoError(t, err) assert.Equal(t, GPUShape{GPUsPerEngine: 8, GPUsPerReplica: tt.wantReplica}, shapes["decode"]) }) diff --git a/deploy/operator/internal/dynamo/graph.go b/deploy/operator/internal/dynamo/graph.go index baa983143735..b667613f4a0e 100644 --- a/deploy/operator/internal/dynamo/graph.go +++ b/deploy/operator/internal/dynamo/graph.go @@ -1367,11 +1367,17 @@ func expandMultinodeGMSRoles(componentName string, numberOfNodes int32, totalEng // For short DGD names the PCS name equals the DGD name (backwards compatible). // For long names, the PCS name is truncated with a deterministic 4-char hash // suffix to guarantee uniqueness and reconcile-loop stability. -// Components must be the exact subset materialized by this PodCliqueSet. -func PCSNameForDGD(dgdName string, components []v1beta1.DynamoComponentDeploymentSharedSpec) string { +func PCSNameForDGD( + dgd *v1beta1.DynamoGraphDeployment, + isDelegated func(*v1beta1.DynamoComponentDeploymentSharedSpec) bool, +) string { maxComponentBudget := 0 - for i := range components { - budget := ComponentNameBudget(&components[i]) + for i := range dgd.Spec.Components { + component := &dgd.Spec.Components[i] + if isDelegatedComponent(component, isDelegated) { + continue + } + budget := ComponentNameBudget(component) if budget > maxComponentBudget { maxComponentBudget = budget } @@ -1384,15 +1390,22 @@ func PCSNameForDGD(dgdName string, components []v1beta1.DynamoComponentDeploymen pcsBudget = minPCSNameLength } - if len(dgdName) <= pcsBudget { - return dgdName + if len(dgd.Name) <= pcsBudget { + return dgd.Name } // Truncate with a deterministic hash suffix for uniqueness hash := fnv.New32a() - hash.Write([]byte(dgdName)) + hash.Write([]byte(dgd.Name)) suffix := fmt.Sprintf("%04x", hash.Sum32()&0xFFFF) - return dgdName[:pcsBudget-5] + "-" + suffix + return dgd.Name[:pcsBudget-5] + "-" + suffix +} + +func isDelegatedComponent( + component *v1beta1.DynamoComponentDeploymentSharedSpec, + isDelegated func(*v1beta1.DynamoComponentDeploymentSharedSpec) bool, +) bool { + return isDelegated != nil && isDelegated(component) } // Define BackendFramework enum for sglang, vllm, trtllm @@ -2556,18 +2569,19 @@ func resolveGroveSchedulerQueue( } // GenerateGrovePodCliqueSet reads the provider inputs needed to construct the -// desired PodCliqueSet. Resolved domain values stay local and are passed to -// the leaf rendering helpers that consume them. -// The deployment must contain only components owned by this PCS. +// desired PodCliqueSet. The predicate excludes components delegated to another +// controller while the complete DGD remains available for graph-wide settings. func GenerateGrovePodCliqueSet( ctx context.Context, dynamoDeployment *v1beta1.DynamoGraphDeployment, + isDelegated func(*v1beta1.DynamoComponentDeploymentSharedSpec) bool, operatorConfig *configv1alpha1.OperatorConfiguration, runtimeConfig *controller_common.RuntimeConfig, reader ctrlclient.Reader, secretsRetriever SecretsRetriever, restartState *RestartState, - existingRestartAnnotations map[string]string, + existingPodCliqueSet *grovev1alpha1.PodCliqueSet, + workerHashSuffix bool, checkpointInfoByComponent map[string]*checkpoint.CheckpointInfo, ) (*grovev1alpha1.PodCliqueSet, error) { // Construct the common PCS envelope before rendering ordinary components. @@ -2575,7 +2589,7 @@ func GenerateGrovePodCliqueSet( if err != nil { return nil, err } - gangSet.Name = PCSNameForDGD(dynamoDeployment.Name, dynamoDeployment.Spec.Components) + gangSet.Name = PCSNameForDGD(dynamoDeployment, isDelegated) validatedQueueName, err := resolveGroveSchedulerQueue(ctx, dynamoDeployment.Annotations, runtimeConfig) if err != nil { @@ -2599,9 +2613,22 @@ func GenerateGrovePodCliqueSet( scalingGroups []grovev1alpha1.PodCliqueScalingGroupConfig resourceClaimTemplates []grovev1alpha1.ResourceClaimTemplateConfig ) + existingRestartAnnotations := groveRestartAnnotations(existingPodCliqueSet) + workerHash := "" + if workerHashSuffix { + workerHash, err = ComputeDGDWorkersSpecHash(dynamoDeployment) + if err != nil { + return nil, fmt.Errorf("compute Grove worker hash suffix: %w", err) + } + } for i := range dynamoDeployment.Spec.Components { - component := dynamoDeployment.Spec.Components[i].DeepCopy() + sourceComponent := &dynamoDeployment.Spec.Components[i] + if isDelegatedComponent(sourceComponent, isDelegated) { + continue + } + component := sourceComponent.DeepCopy() + prepareGroveComponentForRendering(component, existingPodCliqueSet, workerHash) componentName := component.ComponentName dynamoNamespace := GetDynamoNamespace(dynamoDeployment, component) propagateDGDAnnotations(dynamoDeployment.GetAnnotations(), component) @@ -2690,6 +2717,65 @@ func GenerateGrovePodCliqueSet( return gangSet, nil } +func prepareGroveComponentForRendering( + component *v1beta1.DynamoComponentDeploymentSharedSpec, + existingPodCliqueSet *grovev1alpha1.PodCliqueSet, + workerHash string, +) { + componentType := string(component.ComponentType) + if grovePodCliqueSetUsesLegacyWorkerSelector(existingPodCliqueSet, component.ComponentName, componentType) { + component.ComponentType = v1beta1.ComponentTypeWorker + podTemplate := ensurePodTemplate(component) + if _, ok := podTemplate.Labels[commonconsts.KubeLabelDynamoSubComponentType]; !ok { + podTemplate.Labels[commonconsts.KubeLabelDynamoSubComponentType] = componentType + } + } + if workerHash != "" && IsWorkerComponent(string(component.ComponentType)) { + ensurePodTemplate(component).Labels[commonconsts.KubeLabelDynamoWorkerHash] = workerHash + } +} + +func grovePodCliqueSetUsesLegacyWorkerSelector( + pcs *grovev1alpha1.PodCliqueSet, + componentName string, + componentType string, +) bool { + if pcs == nil || (componentType != commonconsts.ComponentTypePrefill && componentType != commonconsts.ComponentTypeDecode) { + return false + } + for _, clique := range pcs.Spec.Template.Cliques { + if clique == nil || clique.Labels[commonconsts.KubeLabelDynamoComponent] != componentName { + continue + } + if clique.Labels[commonconsts.KubeLabelDynamoComponentType] != commonconsts.ComponentTypeWorker { + continue + } + subComponentType := clique.Labels[commonconsts.KubeLabelDynamoSubComponentType] + if subComponentType == "" || subComponentType == componentType { + return true + } + } + return false +} + +func groveRestartAnnotations(pcs *grovev1alpha1.PodCliqueSet) map[string]string { + restartAnnotations := make(map[string]string) + if pcs == nil { + return restartAnnotations + } + for _, clique := range pcs.Spec.Template.Cliques { + if clique == nil { + continue + } + timestamp, hasTimestamp := clique.Annotations[commonconsts.RestartAnnotation] + componentName, hasComponent := clique.Labels[commonconsts.KubeLabelDynamoComponent] + if hasTimestamp && hasComponent { + restartAnnotations[componentName] = timestamp + } + } + return restartAnnotations +} + // newGrovePodCliqueSet constructs the shared Grove envelope. The caller assigns // the workload identity and fills its cliques. Inputs must be non-nil and are not mutated. func newGrovePodCliqueSet( diff --git a/deploy/operator/internal/dynamo/graph_lpx_test.go b/deploy/operator/internal/dynamo/graph_lpx_test.go index 2c39fb288ab5..eaf5aca6bdc1 100644 --- a/deploy/operator/internal/dynamo/graph_lpx_test.go +++ b/deploy/operator/internal/dynamo/graph_lpx_test.go @@ -144,8 +144,8 @@ func TestLPXPCSNameUsesStableMaterializationIdentity(t *testing.T) { t.Log("Keep the deployment name visible with four hexadecimal identity characters") require.Equal(t, "chat-3efb", name) t.Log("An ordinary DGD named chat-lpx must not collide with the LPX PCS of chat") - require.NotEqual(t, PCSNameForDGD("chat-lpx", nil), name) - require.NotEqual(t, PCSNameForDGD(deployment.Name, nil), name) + require.NotEqual(t, PCSNameForDGD(&v1beta1.DynamoGraphDeployment{ObjectMeta: metav1.ObjectMeta{Name: "chat-lpx"}}, nil), name) + require.NotEqual(t, PCSNameForDGD(&v1beta1.DynamoGraphDeployment{ObjectMeta: metav1.ObjectMeta{Name: deployment.Name}}, nil), name) for _, test := range []struct { name string diff --git a/deploy/operator/internal/dynamo/graph_test.go b/deploy/operator/internal/dynamo/graph_test.go index e2683323b334..25925a9a14fc 100644 --- a/deploy/operator/internal/dynamo/graph_test.go +++ b/deploy/operator/internal/dynamo/graph_test.go @@ -729,7 +729,8 @@ func TestGenerateDynamoComponentsDeployments(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GenerateDynamoComponentsDeployments(betaDGD(t, tt.args.parentDynamoGraphDeployment), nil, nil, RollingUpdateContext{}) + converted := betaDGD(t, tt.args.parentDynamoGraphDeployment) + got, err := GenerateDynamoComponentsDeployments(converted, nil, nil, RollingUpdateContext{}) if (err != nil) != tt.wantErr { t.Errorf("GenerateDynamoComponentsDeployments() error = %v, wantErr %v", err, tt.wantErr) return @@ -765,7 +766,8 @@ func Test_GetDynamoComponentDeploymentsGlobalNamespace(t *testing.T) { }, } - got, err := GenerateDynamoComponentsDeployments(betaDGD(t, dgd), nil, nil, RollingUpdateContext{}) + converted := betaDGD(t, dgd) + got, err := GenerateDynamoComponentsDeployments(converted, nil, nil, RollingUpdateContext{}) if !assert.NoError(t, err) { return } @@ -1065,13 +1067,14 @@ func TestTopologyLabelMetadataFromConvertedAlphaDGD(t *testing.T) { pcs, err := GenerateGrovePodCliqueSet( context.Background(), - beta, + beta, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, + false, nil, ) require.NoError(t, err) @@ -1140,13 +1143,14 @@ func TestGenerateGrovePodCliqueSet_AddsTopologyLabelAnnotationToWorkerCliques(t got, err := GenerateGrovePodCliqueSet( context.Background(), - dgd, + dgd, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, + false, nil, ) require.NoError(t, err) @@ -1201,13 +1205,14 @@ func TestGenerateGrovePodCliqueSet_ProjectsClusterTopologyDomainsToWorkerCliques runtimeConfig := &controller_common.RuntimeConfig{} got, err := GenerateGrovePodCliqueSet( context.Background(), - dgd, + dgd, nil, operatorConfig, runtimeConfig, kubeClient, nil, nil, nil, + false, nil, ) require.NoError(t, err) @@ -4406,7 +4411,8 @@ func TestGenerateGrovePodCliqueSet(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GenerateGrovePodCliqueSet(tt.args.ctx, betaDGD(t, tt.args.dynamoDeployment), tt.args.controllerConfig, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, nil) + converted := betaDGD(t, tt.args.dynamoDeployment) + got, err := GenerateGrovePodCliqueSet(tt.args.ctx, converted, nil, tt.args.controllerConfig, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, false, nil) if (err != nil) != tt.wantErr { t.Errorf("GenerateGrovePodCliqueSet() error = %v, wantErr %v", err, tt.wantErr) return @@ -4508,13 +4514,14 @@ func TestGenerateGrovePodCliqueSet_DoesNotResolveUnusedContainerGPUCount(t *test got, err := GenerateGrovePodCliqueSet( t.Context(), - dgd, + dgd, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, + false, nil, ) require.NoError(t, err) @@ -4567,13 +4574,14 @@ func TestGenerateGrovePodCliqueSet_VLLMMultinodeDRA(t *testing.T) { } got, err := GenerateGrovePodCliqueSet( context.Background(), - dgd, + dgd, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, kubeClient, nil, nil, nil, + false, nil, ) require.NoError(t, err) @@ -4640,13 +4648,14 @@ func TestGenerateGrovePodCliqueSet_TRTLLMMultinodeDRA(t *testing.T) { got, err := GenerateGrovePodCliqueSet( t.Context(), - dgd, + dgd, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, kubeClient, nil, nil, nil, + false, nil, ) require.NoError(t, err) @@ -4686,7 +4695,8 @@ func Test_GeneratePodCliqueSetGlobalDynamoNamespace(t *testing.T) { }, } - got, err := GenerateGrovePodCliqueSet(context.Background(), betaDGD(t, dynamoDeployment), &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, nil) + converted := betaDGD(t, dynamoDeployment) + got, err := GenerateGrovePodCliqueSet(context.Background(), converted, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, false, nil) if !assert.NoError(t, err) { return } @@ -6005,7 +6015,8 @@ func TestGenerateGrovePodCliqueSet_StartsAfterDependencies(t *testing.T) { }, } - got, err := GenerateGrovePodCliqueSet(context.Background(), betaDGD(t, dynamoDeployment), controllerConfig, &controller_common.RuntimeConfig{}, nil, secretsRetriever, nil, nil, nil) + converted := betaDGD(t, dynamoDeployment) + got, err := GenerateGrovePodCliqueSet(context.Background(), converted, nil, controllerConfig, &controller_common.RuntimeConfig{}, nil, secretsRetriever, nil, nil, false, nil) if err != nil { t.Errorf("GenerateGrovePodCliqueSet() error = %v", err) return @@ -7766,15 +7777,17 @@ func TestGenerateGrovePodCliqueSet_ConvertedCompilationCacheMountIsNotDuplicated } t.Log("Render the reported v1alpha1 compilation-cache fixture as a Grove PodCliqueSet") + converted := betaDGD(t, dgd) got, err := GenerateGrovePodCliqueSet( context.Background(), - betaDGD(t, dgd), + converted, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, + false, nil, ) require.NoError(t, err) @@ -8704,7 +8717,8 @@ func TestGenerateGrovePodCliqueSet_RestartAnnotations(t *testing.T) { }, } - got, err := GenerateGrovePodCliqueSet(context.Background(), betaDGD(t, dgd), controllerConfig, &controller_common.RuntimeConfig{}, nil, nil, tt.restartState, nil, nil) + converted := betaDGD(t, dgd) + got, err := GenerateGrovePodCliqueSet(context.Background(), converted, nil, controllerConfig, &controller_common.RuntimeConfig{}, nil, nil, tt.restartState, nil, false, nil) if err != nil { t.Fatalf("GenerateGrovePodCliqueSet() error = %v", err) } @@ -8859,7 +8873,8 @@ func TestGenerateGrovePodCliqueSet_GMSPodsDoNotCarryDiscoveryLabels(t *testing.T }, } - got, err := GenerateGrovePodCliqueSet(context.Background(), betaDGD(t, dgd), controllerConfig, &controller_common.RuntimeConfig{Gate: features.Gates{DRA: true}}, nil, nil, nil, nil, nil) + converted := betaDGD(t, dgd) + got, err := GenerateGrovePodCliqueSet(context.Background(), converted, nil, controllerConfig, &controller_common.RuntimeConfig{Gate: features.Gates{DRA: true}}, nil, nil, nil, nil, false, nil) require.NoError(t, err) require.NotNil(t, got) @@ -8943,7 +8958,8 @@ func TestGenerateGrovePodCliqueSet_GMSPodsAreNotCheckpointTargets(t *testing.T) }, } - got, err := GenerateGrovePodCliqueSet(context.Background(), betaDGD(t, dgd), controllerConfig, &controller_common.RuntimeConfig{Gate: features.Gates{Checkpoint: true, DRA: true}}, nil, nil, nil, nil, infoByService) + converted := betaDGD(t, dgd) + got, err := GenerateGrovePodCliqueSet(context.Background(), converted, nil, controllerConfig, &controller_common.RuntimeConfig{Gate: features.Gates{Checkpoint: true, DRA: true}}, nil, nil, nil, nil, false, infoByService) require.NoError(t, err) require.NotNil(t, got) @@ -9042,7 +9058,8 @@ func TestGenerateGrovePodCliqueSet_IntraPodFailoverCheckpointTargets(t *testing. }, } - got, err := GenerateGrovePodCliqueSet(context.Background(), betaDGD(t, dgd), controllerConfig, &controller_common.RuntimeConfig{Gate: features.Gates{Checkpoint: true, DRA: true}}, nil, nil, nil, nil, infoByService) + converted := betaDGD(t, dgd) + got, err := GenerateGrovePodCliqueSet(context.Background(), converted, nil, controllerConfig, &controller_common.RuntimeConfig{Gate: features.Gates{Checkpoint: true, DRA: true}}, nil, nil, nil, nil, false, infoByService) require.NoError(t, err) require.NotNil(t, got) @@ -9095,15 +9112,17 @@ func TestGenerateGrovePodCliqueSet_WaitForCheckpointGatesPodCliqueScalingGroup(t }, } + converted := betaDGD(t, dgd) got, err := GenerateGrovePodCliqueSet( context.Background(), - betaDGD(t, dgd), + converted, nil, &configv1alpha1.OperatorConfiguration{Checkpoint: configv1alpha1.CheckpointConfiguration{Enabled: true}}, &controller_common.RuntimeConfig{Gate: features.Gates{Checkpoint: true, DRA: true}}, nil, nil, nil, nil, + false, map[string]*checkpoint.CheckpointInfo{ "decode": { Enabled: true, @@ -9204,12 +9223,13 @@ func TestGenerateGrovePodCliqueSet_ComponentMinAvailable(t *testing.T) { }, } + converted := betaDGD(t, dgd) got, err := GenerateGrovePodCliqueSet( context.Background(), - betaDGD(t, dgd), + converted, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, - nil, nil, nil, nil, nil, + nil, nil, nil, nil, false, nil, ) require.NoError(t, err) require.NotNil(t, got) @@ -9259,10 +9279,10 @@ func TestGenerateGrovePodCliqueSet_SingleNodeForceScalingGroup(t *testing.T) { got, err := GenerateGrovePodCliqueSet( t.Context(), - beta, + beta, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, - nil, nil, nil, nil, nil, + nil, nil, nil, nil, false, nil, ) require.NoError(t, err) require.NotNil(t, got) @@ -9314,15 +9334,16 @@ func TestGenerateGrovePodCliqueSet_MinAvailable_FailoverShadowsAreRedundant(t *t }, } + converted := betaDGD(t, dgd) got, err := GenerateGrovePodCliqueSet( context.Background(), - betaDGD(t, dgd), + converted, nil, &configv1alpha1.OperatorConfiguration{ Discovery: configv1alpha1.DiscoveryConfiguration{Backend: "kubernetes"}, Infrastructure: configv1alpha1.InfrastructureConfiguration{ETCDAddress: "etcd-address", NATSAddress: "nats-address"}, }, &controller_common.RuntimeConfig{Gate: features.Gates{DRA: true}}, - nil, nil, nil, nil, nil, + nil, nil, nil, nil, false, nil, ) require.NoError(t, err) require.NotNil(t, got) @@ -9466,7 +9487,8 @@ func TestGenerateSingleDCD_RollingUpdateContext(t *testing.T) { NewWorkerReplicaTargetsByComponent: map[string]int32{"prefill": 2}, } - dcds, err := GenerateDynamoComponentsDeployments(betaDGD(t, dgd), &RestartState{}, nil, ruCtx) + converted := betaDGD(t, dgd) + dcds, err := GenerateDynamoComponentsDeployments(converted, &RestartState{}, nil, ruCtx) assert.NoError(t, err) // Worker DCD: hash suffix in name, hash label, replica override @@ -9584,7 +9606,8 @@ func TestGenerateDynamoComponentsDeployments_InferBackendFrameworkForGeneratedDC }, } - dcds, err := GenerateDynamoComponentsDeployments(betaDGD(t, dgd), &RestartState{}, nil, RollingUpdateContext{NewWorkerHash: "2dad72b9"}) + converted := betaDGD(t, dgd) + dcds, err := GenerateDynamoComponentsDeployments(converted, &RestartState{}, nil, RollingUpdateContext{NewWorkerHash: "2dad72b9"}) require.NoError(t, err) assert.Equal(t, string(BackendFrameworkVLLM), dcds["decode"].Spec.BackendFramework) @@ -9601,7 +9624,8 @@ func TestGenerateSingleDCD_NoRollingUpdate(t *testing.T) { }, } - dcds, err := GenerateDynamoComponentsDeployments(betaDGD(t, dgd), &RestartState{}, nil, RollingUpdateContext{}) + converted := betaDGD(t, dgd) + dcds, err := GenerateDynamoComponentsDeployments(converted, &RestartState{}, nil, RollingUpdateContext{}) assert.NoError(t, err) dcd := dcds["worker"] @@ -9629,7 +9653,8 @@ func TestGenerateSingleDCD_RollingUpdateZeroReplicas(t *testing.T) { NewWorkerReplicaTargetsByComponent: map[string]int32{"decode": 0}, } - dcds, err := GenerateDynamoComponentsDeployments(betaDGD(t, dgd), &RestartState{}, nil, ruCtx) + converted := betaDGD(t, dgd) + dcds, err := GenerateDynamoComponentsDeployments(converted, &RestartState{}, nil, ruCtx) assert.NoError(t, err) decodeDCD := dcds["decode"] @@ -10377,7 +10402,8 @@ func TestGenerateGrovePodCliqueSet_SpecMetadataPropagation(t *testing.T) { }, } - pcs, err := GenerateGrovePodCliqueSet(context.Background(), betaDGD(t, dgd), &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, nil) + converted := betaDGD(t, dgd) + pcs, err := GenerateGrovePodCliqueSet(context.Background(), converted, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, false, nil) require.NoError(t, err) // PCS object-level metadata @@ -10491,7 +10517,8 @@ func TestGenerateGrovePodCliqueSet_MetadataVolcanoQueuePropagation(t *testing.T) }, } - pcs, err := GenerateGrovePodCliqueSet(context.Background(), betaDGD(t, dgd), &configv1alpha1.OperatorConfiguration{}, tt.runtimeConfig, nil, nil, nil, nil, nil) + converted := betaDGD(t, dgd) + pcs, err := GenerateGrovePodCliqueSet(context.Background(), converted, nil, &configv1alpha1.OperatorConfiguration{}, tt.runtimeConfig, nil, nil, nil, nil, false, nil) require.NoError(t, err) require.NotNil(t, pcs) if tt.expectQueue { @@ -10523,15 +10550,17 @@ func TestGenerateGrovePodCliqueSet_VolcanoSchedulerInjection(t *testing.T) { }, } + converted := betaDGD(t, dgd) pcs, err := GenerateGrovePodCliqueSet( context.Background(), - betaDGD(t, dgd), + converted, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{Gate: features.Gates{Grove: true, VolcanoScheduler: true}}, nil, nil, nil, nil, + false, nil, ) require.NoError(t, err) @@ -10556,15 +10585,17 @@ func TestGenerateGrovePodCliqueSet_SchedulerIntegrationMutualExclusion(t *testin }, } + converted := betaDGD(t, dgd) _, err := GenerateGrovePodCliqueSet( context.Background(), - betaDGD(t, dgd), + converted, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{Gate: features.Gates{Grove: true, KaiScheduler: true, VolcanoScheduler: true}}, nil, nil, nil, nil, + false, nil, ) require.Error(t, err) @@ -10588,7 +10619,8 @@ func TestGenerateGrovePodCliqueSet_PriorityClassName(t *testing.T) { }, } - pcs, err := GenerateGrovePodCliqueSet(context.Background(), betaDGD(t, dgd), &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, nil) + converted := betaDGD(t, dgd) + pcs, err := GenerateGrovePodCliqueSet(context.Background(), converted, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, false, nil) require.NoError(t, err) assert.Equal(t, "high-priority", pcs.Spec.Template.PriorityClassName) @@ -10659,7 +10691,8 @@ func TestGenerateGrovePodCliqueSet_UpdateStrategy(t *testing.T) { } } - pcs, err := GenerateGrovePodCliqueSet(context.Background(), betaDGD(t, dgd), &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, nil) + converted := betaDGD(t, dgd) + pcs, err := GenerateGrovePodCliqueSet(context.Background(), converted, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, false, nil) if tt.wantErr != "" { require.Error(t, err) assert.Contains(t, err.Error(), tt.wantErr) @@ -10696,7 +10729,8 @@ func TestGenerateDynamoComponentsDeployments_SpecMetadataPropagation(t *testing. }, } - dcds, err := GenerateDynamoComponentsDeployments(betaDGD(t, dgd), nil, nil, RollingUpdateContext{}) + converted := betaDGD(t, dgd) + dcds, err := GenerateDynamoComponentsDeployments(converted, nil, nil, RollingUpdateContext{}) require.NoError(t, err) dcd := dcds["frontend"] @@ -10914,15 +10948,17 @@ func TestGenerateGrovePodCliqueSet_TopologyConstraints(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + converted := betaDGD(t, tt.deployment) pcs, err := GenerateGrovePodCliqueSet( context.Background(), - betaDGD(t, tt.deployment), + converted, nil, operatorConfig, &controller_common.RuntimeConfig{}, nil, secretsRetriever, &RestartState{}, nil, + false, nil, ) assert.NoError(t, err) @@ -11074,7 +11110,11 @@ func TestPCSNameForDGD(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := PCSNameForDGD(tt.dgdName, tt.components) + dgd := &v1beta1.DynamoGraphDeployment{ + ObjectMeta: metav1.ObjectMeta{Name: tt.dgdName}, + Spec: v1beta1.DynamoGraphDeploymentSpec{Components: tt.components}, + } + got := PCSNameForDGD(dgd, nil) if tt.want != "" { if got != tt.want { @@ -11088,7 +11128,7 @@ func TestPCSNameForDGD(t *testing.T) { } // Verify determinism - got2 := PCSNameForDGD(tt.dgdName, tt.components) + got2 := PCSNameForDGD(dgd, nil) if got != got2 { t.Errorf("PCSNameForDGD() not deterministic: %q != %q", got, got2) } diff --git a/deploy/operator/internal/dynamo/grove.go b/deploy/operator/internal/dynamo/grove.go index 4706da0a9f96..563a727a5a6a 100644 --- a/deploy/operator/internal/dynamo/grove.go +++ b/deploy/operator/internal/dynamo/grove.go @@ -43,13 +43,13 @@ type GroveMultinodeDeployer struct { Rank int32 // explicit node rank (used when IsInterPodGMS is true) } -// GroveComponentResourceName returns the Grove child resource name for a DGD +// GroveComponentResourceName returns the Grove child resource name for a PCS // component. Grove currently creates one PodClique or PodCliqueScalingGroup // instance per component at PodCliqueSet replica index zero. -func GroveComponentResourceName(dgd *v1beta1.DynamoGraphDeployment, componentName string) string { +func GroveComponentResourceName(pcsName, componentName string) string { return fmt.Sprintf( "%s-0-%s", - PCSNameForDGD(dgd.Name, dgd.Spec.Components), + pcsName, strings.ToLower(componentName), ) } @@ -106,16 +106,16 @@ type GroveReadiness struct { ComponentStatuses map[string]v1beta1.ComponentReplicaStatus } -// EvaluateGroveReadiness resolves one Grove readiness snapshot from the -// supplied PCS observation. A nil PCS represents an observed missing PCS. -// The deployment must contain only components owned by this PCS. +// EvaluateGroveReadiness observes readiness for the Grove-managed components +// represented by the supplied PCS. A nil PCS represents an observed missing PCS. func EvaluateGroveReadiness( ctx context.Context, reader client.Reader, dgd *v1beta1.DynamoGraphDeployment, + isDelegated func(*v1beta1.DynamoComponentDeploymentSharedSpec) bool, pcs *grovev1alpha1.PodCliqueSet, ) (GroveReadiness, error) { - allReady, classification, message, componentStatuses, err := evaluateGroveComponents(ctx, reader, dgd, pcs) + allReady, classification, message, componentStatuses, err := evaluateGroveComponents(ctx, reader, dgd, isDelegated, pcs) if err != nil { return GroveReadiness{}, err } @@ -151,20 +151,25 @@ func evaluateGroveComponents( ctx context.Context, reader client.Reader, dgd *v1beta1.DynamoGraphDeployment, + isDelegated func(*v1beta1.DynamoComponentDeploymentSharedSpec) bool, pcs *grovev1alpha1.PodCliqueSet, ) (allReady bool, classificationReason string, message string, componentStatuses map[string]v1beta1.ComponentReplicaStatus, err error) { logger := log.FromContext(ctx) var notReadyComponents []string aggregatedReason := "" componentReadinesses := make(map[string]groveComponentReadiness, len(dgd.Spec.Components)) + pcsName := PCSNameForDGD(dgd, isDelegated) for i := range dgd.Spec.Components { component := &dgd.Spec.Components[i] + if isDelegatedComponent(component, isDelegated) { + continue + } componentName := component.ComponentName var componentReadiness groveComponentReadiness var checkErr error - resourceName := GroveComponentResourceName(dgd, componentName) + resourceName := GroveComponentResourceName(pcsName, componentName) if component.UsesPCSG() { componentReadiness, checkErr = observePCSGReadiness(ctx, reader, resourceName, dgd.Namespace, logger) } else { @@ -192,14 +197,17 @@ func evaluateGroveComponents( } } - namespacePlan, err := newGroveRuntimeNamespacePlan(dgd, pcs, componentReadinesses) + namespacePlan, err := newGroveRuntimeNamespacePlan(dgd, isDelegated, pcs, componentReadinesses) if err != nil { return false, "", "", nil, err } - componentStatuses = make(map[string]v1beta1.ComponentReplicaStatus, len(dgd.Spec.Components)) + componentStatuses = make(map[string]v1beta1.ComponentReplicaStatus, len(componentReadinesses)) for i := range dgd.Spec.Components { component := &dgd.Spec.Components[i] + if isDelegatedComponent(component, isDelegated) { + continue + } componentReadiness := componentReadinesses[component.ComponentName] componentStatus := componentReadiness.status componentStatus.RuntimeNamespace = namespacePlan.runtimeNamespace(dgd, component) @@ -284,11 +292,12 @@ type groveRuntimeNamespacePlan struct { func newGroveRuntimeNamespacePlan( dgd *v1beta1.DynamoGraphDeployment, + isDelegated func(*v1beta1.DynamoComponentDeploymentSharedSpec) bool, pcs *grovev1alpha1.PodCliqueSet, componentReadinesses map[string]groveComponentReadiness, ) (groveRuntimeNamespacePlan, error) { acceptedPCSRevisionHash := getAcceptedPCSRevisionHash(pcs) - workerHash, workersUseHashSuffix, err := acceptedGroveWorkerHash(dgd, pcs, acceptedPCSRevisionHash) + workerHash, workersUseHashSuffix, err := acceptedGroveWorkerHash(dgd, isDelegated, pcs, acceptedPCSRevisionHash) if err != nil { return groveRuntimeNamespacePlan{}, err } @@ -297,7 +306,7 @@ func newGroveRuntimeNamespacePlan( acceptedPCSRevisionHash: acceptedPCSRevisionHash, workerHash: workerHash, workersUseHashSuffix: workersUseHashSuffix, - workersCompleted: groveWorkersCompletedAcceptedPCSRevision(dgd, componentReadinesses, acceptedPCSRevisionHash), + workersCompleted: groveWorkersCompletedAcceptedPCSRevision(dgd, isDelegated, componentReadinesses, acceptedPCSRevisionHash), }, nil } @@ -320,16 +329,18 @@ func (p groveRuntimeNamespacePlan) runtimeNamespace( return ComponentRuntimeNamespace(baseNamespace, string(component.ComponentType), p.workerHash) } -// groveWorkersCompletedAcceptedPCSRevision reports whether every worker child -// completed the accepted PCS revision. dgd must contain at least one worker. func groveWorkersCompletedAcceptedPCSRevision( dgd *v1beta1.DynamoGraphDeployment, + isDelegated func(*v1beta1.DynamoComponentDeploymentSharedSpec) bool, componentReadinesses map[string]groveComponentReadiness, acceptedPCSRevisionHash *string, ) bool { workerCount := 0 for i := range dgd.Spec.Components { component := &dgd.Spec.Components[i] + if isDelegatedComponent(component, isDelegated) { + continue + } if !IsWorkerComponent(string(component.ComponentType)) { continue } @@ -345,6 +356,7 @@ func groveWorkersCompletedAcceptedPCSRevision( // PCS revision. The second result reports whether that accepted revision is suffixed. func acceptedGroveWorkerHash( dgd *v1beta1.DynamoGraphDeployment, + isDelegated func(*v1beta1.DynamoComponentDeploymentSharedSpec) bool, pcs *grovev1alpha1.PodCliqueSet, acceptedPCSRevisionHash *string, ) (string, bool, error) { @@ -355,6 +367,9 @@ func acceptedGroveWorkerHash( workerHash := "" for i := range dgd.Spec.Components { component := &dgd.Spec.Components[i] + if isDelegatedComponent(component, isDelegated) { + continue + } if !IsWorkerComponent(string(component.ComponentType)) { continue } @@ -381,6 +396,9 @@ func acceptedGroveWorkerHash( for i := range dgd.Spec.Components { component := &dgd.Spec.Components[i] + if isDelegatedComponent(component, isDelegated) { + continue + } if !IsWorkerComponent(string(component.ComponentType)) { continue } diff --git a/deploy/operator/internal/dynamo/grove_schema_test.go b/deploy/operator/internal/dynamo/grove_schema_test.go index 354454d3bac6..18c097ff8fa9 100644 --- a/deploy/operator/internal/dynamo/grove_schema_test.go +++ b/deploy/operator/internal/dynamo/grove_schema_test.go @@ -111,15 +111,17 @@ func generateTopologyTestPodCliqueSet(t *testing.T, deploymentPack v1alpha1.Topo }, } + converted := betaDGD(t, deployment) pcs, err := GenerateGrovePodCliqueSet( t.Context(), - betaDGD(t, deployment), + converted, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, &mockSecretsRetriever{}, &RestartState{}, nil, + false, nil, ) require.NoError(t, err) diff --git a/deploy/operator/internal/dynamo/grove_test.go b/deploy/operator/internal/dynamo/grove_test.go index 89e49d7b29c0..9609d2710b5f 100644 --- a/deploy/operator/internal/dynamo/grove_test.go +++ b/deploy/operator/internal/dynamo/grove_test.go @@ -1001,8 +1001,8 @@ func TestEvaluateGroveReadinessUsesDeclaredLayout(t *testing.T) { } } reader := newFakeGroveClient(g) - pcs, err := GenerateGrovePodCliqueSet(t.Context(), dgd, &configv1alpha1.OperatorConfiguration{}, - &controller_common.RuntimeConfig{Gate: features.Gates{DRA: true}}, reader, &mockSecretsRetriever{}, nil, nil, nil) + pcs, err := GenerateGrovePodCliqueSet(t.Context(), dgd, nil, &configv1alpha1.OperatorConfiguration{}, + &controller_common.RuntimeConfig{Gate: features.Gates{DRA: true}}, reader, &mockSecretsRetriever{}, nil, nil, false, nil) g.Expect(err).NotTo(gomega.HaveOccurred()) t.Log("Prove emitted scaling groups use the component identity") @@ -1023,7 +1023,7 @@ func TestEvaluateGroveReadinessUsesDeclaredLayout(t *testing.T) { } pcs.Spec.Template.PodCliqueScalingGroupConfigs = []grovev1alpha1.PodCliqueScalingGroupConfig{{Name: "undeclared", CliqueNames: names}} } - readiness, err := EvaluateGroveReadiness(t.Context(), reader, dgd, pcs) + readiness, err := EvaluateGroveReadiness(t.Context(), reader, dgd, nil, pcs) g.Expect(err).NotTo(gomega.HaveOccurred()) g.Expect(readiness.Ready).To(gomega.BeFalse()) g.Expect(readiness.ComponentStatuses).To(gomega.HaveLen(1)) @@ -1039,7 +1039,7 @@ func TestEvaluateGroveReadinessUsesDeclaredLayout(t *testing.T) { Replicas: 2, AvailableReplicas: 2, UpdatedReplicas: 2, ObservedGeneration: ptr.To(int64(1)), }, })).To(gomega.Succeed()) - readiness, err = EvaluateGroveReadiness(t.Context(), reader, dgd, pcs) + readiness, err = EvaluateGroveReadiness(t.Context(), reader, dgd, nil, pcs) g.Expect(err).NotTo(gomega.HaveOccurred()) g.Expect(readiness.Ready).To(gomega.BeTrue()) } @@ -1388,7 +1388,7 @@ func TestEvaluateGroveReadiness(t *testing.T) { WithStatusSubresource(objects...). Build() - readiness, err := EvaluateGroveReadiness(ctx, fakeKubeClient, betaDGD, nil) + readiness, err := EvaluateGroveReadiness(ctx, fakeKubeClient, betaDGD, nil, nil) g.Expect(err).NotTo(gomega.HaveOccurred()) g.Expect(readiness.Ready).To(gomega.Equal(tt.wantReady)) @@ -1492,7 +1492,7 @@ func TestEvaluateGroveReadinessPublishesWorkerRuntimeNamespaceAfterCutover(t *te } podCliqueSet := &grovev1alpha1.PodCliqueSet{ ObjectMeta: metav1.ObjectMeta{ - Name: PCSNameForDGD(dgd.Name, dgd.Spec.Components), + Name: PCSNameForDGD(dgd, nil), Namespace: dgd.Namespace, Generation: 1, }, @@ -1517,7 +1517,7 @@ func TestEvaluateGroveReadinessPublishesWorkerRuntimeNamespaceAfterCutover(t *te } replicas := ptr.Deref(tt.childReplicas, int32(1)) podClique := &grovev1alpha1.PodClique{ - ObjectMeta: metav1.ObjectMeta{Name: GroveComponentResourceName(dgd, componentName), Namespace: dgd.Namespace, Generation: childGeneration}, + ObjectMeta: metav1.ObjectMeta{Name: GroveComponentResourceName(PCSNameForDGD(dgd, nil), componentName), Namespace: dgd.Namespace, Generation: childGeneration}, Spec: grovev1alpha1.PodCliqueSpec{Replicas: replicas}, Status: grovev1alpha1.PodCliqueStatus{ Replicas: replicas, @@ -1539,7 +1539,7 @@ func TestEvaluateGroveReadinessPublishesWorkerRuntimeNamespaceAfterCutover(t *te } t.Log("Evaluate runtime namespace selection from the accepted PCS snapshot") - readiness, err := EvaluateGroveReadiness(ctx, newFakeGroveClient(gomega.NewWithT(t), podClique), dgd, podCliqueSet) + readiness, err := EvaluateGroveReadiness(ctx, newFakeGroveClient(gomega.NewWithT(t), podClique), dgd, nil, podCliqueSet) if err != nil { t.Fatalf("EvaluateGroveReadiness() error = %v", err) } @@ -1597,7 +1597,7 @@ func TestEvaluateGroveReadinessSwitchesWorkersAtomically(t *testing.T) { } podCliqueSet := &grovev1alpha1.PodCliqueSet{ - ObjectMeta: metav1.ObjectMeta{Name: PCSNameForDGD(dgd.Name, dgd.Spec.Components), Namespace: dgd.Namespace, Generation: 1}, + ObjectMeta: metav1.ObjectMeta{Name: PCSNameForDGD(dgd, nil), Namespace: dgd.Namespace, Generation: 1}, Spec: grovev1alpha1.PodCliqueSetSpec{Template: grovev1alpha1.PodCliqueSetTemplateSpec{ Cliques: []*grovev1alpha1.PodCliqueTemplateSpec{ {Labels: map[string]string{commonconsts.KubeLabelDynamoComponent: "prefill", commonconsts.KubeLabelDynamoWorkerHash: acceptedHash}}, @@ -1608,7 +1608,7 @@ func TestEvaluateGroveReadinessSwitchesWorkersAtomically(t *testing.T) { } completedAt := metav1.Now() prefill := &grovev1alpha1.PodClique{ - ObjectMeta: metav1.ObjectMeta{Name: GroveComponentResourceName(dgd, "prefill"), Namespace: dgd.Namespace, Generation: 1}, + ObjectMeta: metav1.ObjectMeta{Name: GroveComponentResourceName(PCSNameForDGD(dgd, nil), "prefill"), Namespace: dgd.Namespace, Generation: 1}, Spec: grovev1alpha1.PodCliqueSpec{Replicas: 1}, Status: grovev1alpha1.PodCliqueStatus{Replicas: 1, ReadyReplicas: 1, UpdatedReplicas: 1, ObservedGeneration: ptr.To(int64(1)), CurrentPodCliqueSetGenerationHash: ptr.To(acceptedRevision), UpdateProgress: &grovev1alpha1.PodCliqueUpdateProgress{UpdateEndedAt: &completedAt}}, } @@ -1617,13 +1617,13 @@ func TestEvaluateGroveReadinessSwitchesWorkersAtomically(t *testing.T) { decodeRevision = acceptedRevision } decode := &grovev1alpha1.PodClique{ - ObjectMeta: metav1.ObjectMeta{Name: GroveComponentResourceName(dgd, "decode"), Namespace: dgd.Namespace, Generation: 1}, + ObjectMeta: metav1.ObjectMeta{Name: GroveComponentResourceName(PCSNameForDGD(dgd, nil), "decode"), Namespace: dgd.Namespace, Generation: 1}, Spec: grovev1alpha1.PodCliqueSpec{Replicas: 1}, Status: grovev1alpha1.PodCliqueStatus{Replicas: 1, ReadyReplicas: 1, UpdatedReplicas: 1, ObservedGeneration: ptr.To(int64(1)), CurrentPodCliqueSetGenerationHash: ptr.To(decodeRevision), UpdateProgress: &grovev1alpha1.PodCliqueUpdateProgress{UpdateEndedAt: &completedAt}}, } t.Log("Evaluate the worker group against the single accepted PCS snapshot") - readiness, err := EvaluateGroveReadiness(ctx, newFakeGroveClient(gomega.NewWithT(t), prefill, decode), dgd, podCliqueSet) + readiness, err := EvaluateGroveReadiness(ctx, newFakeGroveClient(gomega.NewWithT(t), prefill, decode), dgd, nil, podCliqueSet) if err != nil { t.Fatalf("EvaluateGroveReadiness() error = %v", err) } @@ -1666,7 +1666,7 @@ func TestEvaluateGroveReadinessPreservesPreviousWorkerNamespaceWithoutPodCliqueS } podClique := &grovev1alpha1.PodClique{ ObjectMeta: metav1.ObjectMeta{ - Name: GroveComponentResourceName(dgd, componentName), + Name: GroveComponentResourceName(PCSNameForDGD(dgd, nil), componentName), Namespace: dgd.Namespace, Generation: 1, }, @@ -1681,7 +1681,7 @@ func TestEvaluateGroveReadinessPreservesPreviousWorkerNamespaceWithoutPodCliqueS } t.Log("Evaluate readiness from an observed missing PodCliqueSet") - readiness, err := EvaluateGroveReadiness(ctx, newFakeGroveClient(gomega.NewWithT(t), podClique), dgd, nil) + readiness, err := EvaluateGroveReadiness(ctx, newFakeGroveClient(gomega.NewWithT(t), podClique), dgd, nil, nil) t.Log("Verify the missing PCS cannot replace the previously published namespace") if err != nil { @@ -1755,7 +1755,7 @@ func TestEvaluateGroveReadinessRejectsInvalidAcceptedWorkerPodCliqueSet(t *testi } podCliqueSet := &grovev1alpha1.PodCliqueSet{ ObjectMeta: metav1.ObjectMeta{ - Name: PCSNameForDGD(dgd.Name, dgd.Spec.Components), + Name: PCSNameForDGD(dgd, nil), Namespace: dgd.Namespace, Generation: 1, }, @@ -1773,7 +1773,7 @@ func TestEvaluateGroveReadinessRejectsInvalidAcceptedWorkerPodCliqueSet(t *testi component := &dgd.Spec.Components[i] children = append(children, &grovev1alpha1.PodClique{ ObjectMeta: metav1.ObjectMeta{ - Name: GroveComponentResourceName(dgd, component.ComponentName), + Name: GroveComponentResourceName(PCSNameForDGD(dgd, nil), component.ComponentName), Namespace: dgd.Namespace, Generation: 1, }, @@ -1791,7 +1791,7 @@ func TestEvaluateGroveReadinessRejectsInvalidAcceptedWorkerPodCliqueSet(t *testi } t.Log("Evaluate runtime namespace selection from the invalid accepted snapshot") - _, err := EvaluateGroveReadiness(ctx, newFakeGroveClient(gomega.NewWithT(t), children...), dgd, podCliqueSet) + _, err := EvaluateGroveReadiness(ctx, newFakeGroveClient(gomega.NewWithT(t), children...), dgd, nil, podCliqueSet) t.Log("Verify the invalid accepted layout is rejected rather than publishing a namespace") if err == nil || !strings.Contains(err.Error(), tt.wantMessage) { @@ -1833,7 +1833,7 @@ func TestEvaluateGroveReadinessPublishesAcceptedNamespaceForZeroReplicaWorkers(t } podCliqueSet := &grovev1alpha1.PodCliqueSet{ ObjectMeta: metav1.ObjectMeta{ - Name: PCSNameForDGD(dgd.Name, dgd.Spec.Components), + Name: PCSNameForDGD(dgd, nil), Namespace: dgd.Namespace, Generation: 1, }, @@ -1854,7 +1854,7 @@ func TestEvaluateGroveReadinessPublishesAcceptedNamespaceForZeroReplicaWorkers(t if tt.multinode { child = &grovev1alpha1.PodCliqueScalingGroup{ ObjectMeta: metav1.ObjectMeta{ - Name: GroveComponentResourceName(dgd, componentName), + Name: GroveComponentResourceName(PCSNameForDGD(dgd, nil), componentName), Namespace: dgd.Namespace, Generation: 1, }, @@ -1869,7 +1869,7 @@ func TestEvaluateGroveReadinessPublishesAcceptedNamespaceForZeroReplicaWorkers(t } else { child = &grovev1alpha1.PodClique{ ObjectMeta: metav1.ObjectMeta{ - Name: GroveComponentResourceName(dgd, componentName), + Name: GroveComponentResourceName(PCSNameForDGD(dgd, nil), componentName), Namespace: dgd.Namespace, Generation: 1, }, @@ -1882,7 +1882,7 @@ func TestEvaluateGroveReadinessPublishesAcceptedNamespaceForZeroReplicaWorkers(t } t.Log("Evaluate the accepted zero-replica revision") - readiness, err := EvaluateGroveReadiness(ctx, newFakeGroveClient(gomega.NewWithT(t), child), dgd, podCliqueSet) + readiness, err := EvaluateGroveReadiness(ctx, newFakeGroveClient(gomega.NewWithT(t), child), dgd, nil, podCliqueSet) t.Log("Verify both child kinds publish the accepted worker namespace") if err != nil { @@ -1926,7 +1926,7 @@ func TestEvaluateGroveReadinessReadsEachGroveChildOnce(t *testing.T) { component := dgd.GetComponentByName("prefill") podCliqueSet := &grovev1alpha1.PodCliqueSet{ ObjectMeta: metav1.ObjectMeta{ - Name: PCSNameForDGD(dgd.Name, dgd.Spec.Components), + Name: PCSNameForDGD(dgd, nil), Namespace: dgd.Namespace, Generation: 1, }, @@ -1943,7 +1943,7 @@ func TestEvaluateGroveReadinessReadsEachGroveChildOnce(t *testing.T) { } podClique := &grovev1alpha1.PodClique{ ObjectMeta: metav1.ObjectMeta{ - Name: GroveComponentResourceName(dgd, component.ComponentName), + Name: GroveComponentResourceName(PCSNameForDGD(dgd, nil), component.ComponentName), Namespace: dgd.Namespace, Generation: 1, }, @@ -1975,7 +1975,7 @@ func TestEvaluateGroveReadinessReadsEachGroveChildOnce(t *testing.T) { Build() t.Log("Evaluate readiness and assert that namespace cutover reuses the child read.") - readiness, err := EvaluateGroveReadiness(ctx, reader, dgd, podCliqueSet) + readiness, err := EvaluateGroveReadiness(ctx, reader, dgd, nil, podCliqueSet) g.Expect(err).NotTo(gomega.HaveOccurred()) g.Expect(readiness.Ready).To(gomega.BeTrue()) g.Expect(childReads).To(gomega.Equal(1)) @@ -2391,7 +2391,7 @@ func TestGroveReadinessTransientErrorsPropagate(t *testing.T) { }, }) c := newClient(g) - readiness, err := EvaluateGroveReadiness(ctx, c, dgd, nil) + readiness, err := EvaluateGroveReadiness(ctx, c, dgd, nil, nil) g.Expect(err).To(gomega.HaveOccurred()) g.Expect(err.Error()).To(gomega.ContainSubstring("transient API error")) // A transient error is not a normal not-ready result: Ready is false and diff --git a/deploy/operator/internal/dynamo/hash_test.go b/deploy/operator/internal/dynamo/hash_test.go index fc29ee2abd26..bcdbe8fd0422 100644 --- a/deploy/operator/internal/dynamo/hash_test.go +++ b/deploy/operator/internal/dynamo/hash_test.go @@ -99,7 +99,7 @@ func TestComputeBetaDGDWorkersSpecHash_CanonicalizesForceScalingGroupFalse(t *te base := betaDGDWithRuntimeVersion(t, "nvcr.io/nvidia/ai-dynamo/vllm-runtime:1.5.0", "") base.Spec.Components[0].Experimental = nil omittedHash := mustComputeBetaDGDWorkersSpecHash(t, base) - omittedRendered, err := GenerateGrovePodCliqueSet(context.Background(), base, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, nil) + omittedRendered, err := GenerateGrovePodCliqueSet(context.Background(), base, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, false, nil) require.NoError(t, err) t.Log("Compare optional outer objects and explicit false with complete omission") @@ -117,7 +117,7 @@ func TestComputeBetaDGDWorkersSpecHash_CanonicalizesForceScalingGroupFalse(t *te candidate := base.DeepCopy() candidate.Spec.Components[0].Experimental = tc.experimental original := candidate.DeepCopy() - rendered, err := GenerateGrovePodCliqueSet(context.Background(), candidate, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, nil) + rendered, err := GenerateGrovePodCliqueSet(context.Background(), candidate, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, false, nil) require.NoError(t, err) require.Equal(t, omittedRendered.Spec, rendered.Spec, "precondition: effective workload must be identical") @@ -561,9 +561,9 @@ func TestComputeBetaDGDWorkersSpecHash_CanonicalizesDisabledCheckpoint(t *testin original := disabled.DeepCopy() t.Log("Verify equal rendered workloads and worker hashes") - omittedRendered, err := GenerateGrovePodCliqueSet(context.Background(), omitted, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, nil) + omittedRendered, err := GenerateGrovePodCliqueSet(context.Background(), omitted, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, false, nil) require.NoError(t, err) - disabledRendered, err := GenerateGrovePodCliqueSet(context.Background(), disabled, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, nil) + disabledRendered, err := GenerateGrovePodCliqueSet(context.Background(), disabled, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, false, nil) require.NoError(t, err) require.Equal(t, omittedRendered.Spec, disabledRendered.Spec) assert.Equal(t, mustComputeBetaDGDWorkersSpecHash(t, omitted), mustComputeBetaDGDWorkersSpecHash(t, disabled)) @@ -589,9 +589,9 @@ func TestComputeBetaDGDWorkersSpecHash_CanonicalizesCompilationCacheMountPath(t originalOmitted, originalExplicit := omitted.DeepCopy(), explicit.DeepCopy() t.Log("Verify equal rendered workloads and worker hashes without mutating either input") - omittedRendered, err := GenerateGrovePodCliqueSet(context.Background(), omitted, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, nil) + omittedRendered, err := GenerateGrovePodCliqueSet(context.Background(), omitted, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, false, nil) require.NoError(t, err) - explicitRendered, err := GenerateGrovePodCliqueSet(context.Background(), explicit, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, nil) + explicitRendered, err := GenerateGrovePodCliqueSet(context.Background(), explicit, nil, &configv1alpha1.OperatorConfiguration{}, &controller_common.RuntimeConfig{}, nil, nil, nil, nil, false, nil) require.NoError(t, err) require.Equal(t, omittedRendered.Spec, explicitRendered.Spec) assert.Equal(t, mustComputeBetaDGDWorkersSpecHash(t, omitted), mustComputeBetaDGDWorkersSpecHash(t, explicit)) diff --git a/deploy/operator/internal/webhook/validation/dynamographdeployment.go b/deploy/operator/internal/webhook/validation/dynamographdeployment.go index 5bb5f0614c7e..72d9e2456397 100644 --- a/deploy/operator/internal/webhook/validation/dynamographdeployment.go +++ b/deploy/operator/internal/webhook/validation/dynamographdeployment.go @@ -217,7 +217,10 @@ func (v *dynamoGraphDeploymentValidation) validateDynamoGraphDeployment( oldComponents: oldComponents, } if grovePathway { - specOpts.pcsName = dynamo.PCSNameForDGD(dgd.Name, ordinaryGroveComponents(dgd.Spec.Components)) + specOpts.pcsName = dynamo.PCSNameForDGD( + dgd, + (*nvidiacomv1beta1.DynamoComponentDeploymentSharedSpec).ManagedByExternalController, + ) } allErrs = append(allErrs, v.validateDynamoGraphDeploymentSpec(&dgd.Spec, field.NewPath("spec"), specOpts)...) diff --git a/deploy/operator/internal/webhook/validation/dynamographdeployment_lpx.go b/deploy/operator/internal/webhook/validation/dynamographdeployment_lpx.go deleted file mode 100644 index 30e1769d98d2..000000000000 --- a/deploy/operator/internal/webhook/validation/dynamographdeployment_lpx.go +++ /dev/null @@ -1,19 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -package validation - -import ( - "slices" - - v1beta1 "github.com/ai-dynamo/dynamo/deploy/operator/api/v1beta1" -) - -func ordinaryGroveComponents(components []v1beta1.DynamoComponentDeploymentSharedSpec) []v1beta1.DynamoComponentDeploymentSharedSpec { - return slices.DeleteFunc( - slices.Clone(components), - func(component v1beta1.DynamoComponentDeploymentSharedSpec) bool { - return component.ManagedByExternalController() - }, - ) -}