From de9a0fa33fce0f31d0100054db74f957bdeeecd5 Mon Sep 17 00:00:00 2001 From: Karthik Vetrivel Date: Thu, 11 Jun 2026 14:46:40 -0400 Subject: [PATCH 1/2] Source driver host root via info catalog instead of ClusterPolicy CR Signed-off-by: Karthik Vetrivel --- controllers/nvidiadriver_controller.go | 4 ++-- internal/state/driver.go | 11 +++++++---- internal/state/driver_test.go | 14 ++++++++++++++ internal/state/info_source.go | 2 +- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/controllers/nvidiadriver_controller.go b/controllers/nvidiadriver_controller.go index 4fda8cecd9..0c1d5f16f4 100644 --- a/controllers/nvidiadriver_controller.go +++ b/controllers/nvidiadriver_controller.go @@ -153,8 +153,8 @@ func (r *NVIDIADriverReconciler) Reconcile(ctx context.Context, req ctrl.Request // Add an entry for ClusterInfo, which was collected before the NVIDIADriver controller was started infoCatalog.Add(state.InfoTypeClusterInfo, r.ClusterInfo) - // Add an entry for Clusterpolicy, which is needed to deploy the driver daemonset - infoCatalog.Add(state.InfoTypeClusterPolicyCR, clusterPolicyInstance) + // Add the host root, which is needed to deploy the driver daemonset + infoCatalog.Add(state.InfoTypeHostRoot, clusterPolicyInstance.Spec.HostPaths.RootFS) // Verify the nodeSelector configured for this NVIDIADriver instance does // not conflict with any other instances. This ensures only one driver diff --git a/internal/state/driver.go b/internal/state/driver.go index 004dab2247..daebc4d0c2 100644 --- a/internal/state/driver.go +++ b/internal/state/driver.go @@ -253,11 +253,14 @@ func (s *stateDriver) cleanupStaleDriverDaemonsets(ctx context.Context, cr *nvid func (s *stateDriver) getManifestObjects(ctx context.Context, cr *nvidiav1alpha1.NVIDIADriver, infoCatalog InfoCatalog) ([]*unstructured.Unstructured, error) { logger := log.FromContext(ctx) - info := infoCatalog.Get(InfoTypeClusterPolicyCR) + info := infoCatalog.Get(InfoTypeHostRoot) if info == nil { - return nil, fmt.Errorf("failed to get ClusterPolicy CR from info catalog") + return nil, fmt.Errorf("failed to get host root from info catalog") + } + hostRoot, ok := info.(string) + if !ok { + return nil, fmt.Errorf("host root in info catalog has unexpected type %T", info) } - clusterPolicy := info.(gpuv1.ClusterPolicy) info = infoCatalog.Get(InfoTypeClusterInfo) if info == nil { @@ -287,7 +290,7 @@ func (s *stateDriver) getManifestObjects(ctx context.Context, cr *nvidiav1alpha1 GPUDirectRDMA: gpuDirectRDMASpec, Runtime: runtimeSpec, ModeSelectorValue: modeSelectorValue, - HostRoot: clusterPolicy.Spec.HostPaths.RootFS, + HostRoot: hostRoot, } if len(nodePools) == 0 { diff --git a/internal/state/driver_test.go b/internal/state/driver_test.go index 2f46230ec1..5c5e9be008 100644 --- a/internal/state/driver_test.go +++ b/internal/state/driver_test.go @@ -201,6 +201,20 @@ func TestDriverHostSysDevicesSystemVolumeUsesStableParentDirectory(t *testing.T) assert.Empty(t, hostSysDevicesSystemMount.SubPath) } +func TestDriverRenderMissingHostRoot(t *testing.T) { + state, err := NewStateDriver(nil, "", nil, manifestDir) + require.Nil(t, err) + stateDriver, ok := state.(*stateDriver) + require.True(t, ok) + + catalog := NewInfoCatalog() + catalog.Add(InfoTypeClusterInfo, testClusterInfo{}) + + _, err = stateDriver.getManifestObjects(context.Background(), &nvidiav1alpha1.NVIDIADriver{}, catalog) + require.Error(t, err, "rendering must fail when no host root is in the catalog") + require.Contains(t, err.Error(), "host root") +} + func TestDriverHostNetwork(t *testing.T) { const ( testName = "driver-hostnetwork" diff --git a/internal/state/info_source.go b/internal/state/info_source.go index 4f6409a904..a30b0b6bbf 100644 --- a/internal/state/info_source.go +++ b/internal/state/info_source.go @@ -20,7 +20,7 @@ type InfoType uint const ( InfoTypeClusterInfo = iota - InfoTypeClusterPolicyCR + InfoTypeHostRoot ) func NewInfoCatalog() InfoCatalog { From 7f38f41653cadabe20249672ad434342795ee10e Mon Sep 17 00:00:00 2001 From: Karthik Vetrivel Date: Thu, 11 Jun 2026 14:51:21 -0400 Subject: [PATCH 2/2] Support NVIDIADriver reconciliation without a ClusterPolicy Signed-off-by: Karthik Vetrivel --- controllers/gpucluster_controller_test.go | 11 ++- controllers/nvidiadriver_controller.go | 69 +++++++++------- controllers/nvidiadriver_controller_test.go | 91 +++++++++++++++++++++ 3 files changed, 138 insertions(+), 33 deletions(-) diff --git a/controllers/gpucluster_controller_test.go b/controllers/gpucluster_controller_test.go index 562ecf87b4..ff1f29026b 100644 --- a/controllers/gpucluster_controller_test.go +++ b/controllers/gpucluster_controller_test.go @@ -61,14 +61,17 @@ func newGPUClusterReconciler(t *testing.T, objs ...client.Object) (*GPUClusterRe } // fakeStateManager returns canned SyncState results so the controller tests don't load -// real manifests. GetWatchSources is promoted from the embedded (nil) interface and is -// never called here — only SetupWithManager calls it, which these tests skip. +// real manifests. It records the last info catalog passed to SyncState so tests can +// assert on its entries. GetWatchSources is promoted from the embedded (nil) interface +// and is never called here — only SetupWithManager calls it, which these tests skip. type fakeStateManager struct { state.Manager - results state.Results + results state.Results + lastCatalog state.InfoCatalog } -func (f *fakeStateManager) SyncState(_ context.Context, _ interface{}, _ state.InfoCatalog) state.Results { +func (f *fakeStateManager) SyncState(_ context.Context, _ interface{}, catalog state.InfoCatalog) state.Results { + f.lastCatalog = catalog return f.results } diff --git a/controllers/nvidiadriver_controller.go b/controllers/nvidiadriver_controller.go index 0c1d5f16f4..2fe79ede56 100644 --- a/controllers/nvidiadriver_controller.go +++ b/controllers/nvidiadriver_controller.go @@ -64,6 +64,7 @@ type NVIDIADriverReconciler struct { //+kubebuilder:rbac:groups=nvidia.com,resources=nvidiadrivers,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=nvidia.com,resources=nvidiadrivers/status,verbs=get;update;patch //+kubebuilder:rbac:groups=nvidia.com,resources=nvidiadrivers/finalizers,verbs=update +//+kubebuilder:rbac:groups=nvidia.com,resources=gpuclusters,verbs=get;list;watch // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -98,45 +99,26 @@ func (r *NVIDIADriverReconciler) Reconcile(ctx context.Context, req ctrl.Request return reconcile.Result{}, nil } - // Get the singleton NVIDIA ClusterPolicy object in the cluster. - clusterPolicyList := &gpuv1.ClusterPolicyList{} - if err := r.List(ctx, clusterPolicyList); err != nil { - wrappedErr := fmt.Errorf("error getting ClusterPolicy list: %w", err) - logger.Error(err, "error getting ClusterPolicy list") - instance.Status.State = nvidiav1alpha1.NotReady - if condErr := r.conditionUpdater.SetConditionsError(ctx, instance, conditions.ReconcileFailed, err.Error()); condErr != nil { - logger.Error(condErr, "failed to set condition") - } - return reconcile.Result{}, wrappedErr - } - - if len(clusterPolicyList.Items) == 0 { - err := fmt.Errorf("no ClusterPolicy object found in the cluster") - logger.Error(err, "failed to get ClusterPolicy object") + // Source the cluster-wide host root from the active configuration: a ClusterPolicy takes + // precedence, otherwise the controller runs standalone against the GPUCluster. + clusterPolicy, gpuCluster, err := resolveActiveConfig(ctx, r.Client) + if err != nil { + logger.Error(err, "error resolving active cluster configuration") instance.Status.State = nvidiav1alpha1.NotReady if condErr := r.conditionUpdater.SetConditionsError(ctx, instance, conditions.ReconcileFailed, err.Error()); condErr != nil { logger.Error(condErr, "failed to set condition") } return reconcile.Result{}, err } - clusterPolicyInstance := clusterPolicyList.Items[0] // Ensure the NVIDIADriver CR has a consumer: either the ClusterPolicy delegates its // driver to the NVIDIADriver CRD, or a GPUCluster exists. GPUCluster does // not manage the driver itself — it is either preinstalled on the host (no NVIDIADriver // CR) or installed via NVIDIADriver CRs, so any CR that exists alongside one is in use. - if !clusterPolicyInstance.Spec.Driver.UseNvidiaDriverCRDType() { - gpuClusters := &nvidiav1alpha1.GPUClusterList{} - if err := r.List(ctx, gpuClusters); err != nil { - wrappedErr := fmt.Errorf("error getting GPUCluster list: %w", err) - logger.Error(err, "error getting GPUCluster list") - instance.Status.State = nvidiav1alpha1.NotReady - if condErr := r.conditionUpdater.SetConditionsError(ctx, instance, conditions.ReconcileFailed, err.Error()); condErr != nil { - logger.Error(condErr, "failed to set condition") - } - return reconcile.Result{}, wrappedErr - } - if len(gpuClusters.Items) == 0 { + var hostRoot string + switch { + case clusterPolicy != nil: + if !clusterPolicy.Spec.Driver.UseNvidiaDriverCRDType() && gpuCluster == nil { msg := "useNvidiaDriverCRD is not enabled in ClusterPolicy and no GPUCluster exists" logger.V(consts.LogLevelWarning).Info("NVIDIADriver reconciliation skipped", "reason", msg) instance.Status.State = nvidiav1alpha1.Disabled @@ -145,6 +127,17 @@ func (r *NVIDIADriverReconciler) Reconcile(ctx context.Context, req ctrl.Request } return reconcile.Result{}, nil } + hostRoot = clusterPolicy.Spec.HostPaths.RootFS + case gpuCluster != nil: + hostRoot = gpuCluster.Spec.HostPaths.RootFS + default: + err := fmt.Errorf("no ClusterPolicy or GPUCluster object found in the cluster") + logger.Error(err, "failed to get a cluster-wide configuration object") + instance.Status.State = nvidiav1alpha1.NotReady + if condErr := r.conditionUpdater.SetConditionsError(ctx, instance, conditions.ReconcileFailed, err.Error()); condErr != nil { + logger.Error(condErr, "failed to set condition") + } + return reconcile.Result{}, err } // Create a new InfoCatalog which is a generic interface for passing information to state managers @@ -154,7 +147,7 @@ func (r *NVIDIADriverReconciler) Reconcile(ctx context.Context, req ctrl.Request infoCatalog.Add(state.InfoTypeClusterInfo, r.ClusterInfo) // Add the host root, which is needed to deploy the driver daemonset - infoCatalog.Add(state.InfoTypeHostRoot, clusterPolicyInstance.Spec.HostPaths.RootFS) + infoCatalog.Add(state.InfoTypeHostRoot, hostRoot) // Verify the nodeSelector configured for this NVIDIADriver instance does // not conflict with any other instances. This ensures only one driver @@ -393,6 +386,24 @@ func (r *NVIDIADriverReconciler) SetupWithManager(ctx context.Context, mgr ctrl. return err } + // Watch for changes to GPUCluster. Whenever an event is generated for + // GPUCluster, enqueue a reconcile request for all NVIDIADriver instances. + gpuClusterMapFn := func(ctx context.Context, _ *nvidiav1alpha1.GPUCluster) []reconcile.Request { + return r.enqueueAllNVIDIADrivers(ctx) + } + + err = c.Watch( + source.Kind( + mgr.GetCache(), + &nvidiav1alpha1.GPUCluster{}, + handler.TypedEnqueueRequestsFromMapFunc(gpuClusterMapFn), + predicate.TypedGenerationChangedPredicate[*nvidiav1alpha1.GPUCluster]{}, + ), + ) + if err != nil { + return err + } + nodePredicate := predicate.TypedFuncs[*corev1.Node]{ CreateFunc: func(e event.TypedCreateEvent[*corev1.Node]) bool { labels := e.Object.GetLabels() diff --git a/controllers/nvidiadriver_controller_test.go b/controllers/nvidiadriver_controller_test.go index 76cc4db1f0..a3d6542d44 100644 --- a/controllers/nvidiadriver_controller_test.go +++ b/controllers/nvidiadriver_controller_test.go @@ -39,6 +39,7 @@ import ( gpuv1 "github.com/NVIDIA/gpu-operator/api/nvidia/v1" nvidiav1alpha1 "github.com/NVIDIA/gpu-operator/api/nvidia/v1alpha1" + "github.com/NVIDIA/gpu-operator/internal/state" "github.com/NVIDIA/gpu-operator/internal/validator" ) @@ -238,6 +239,96 @@ func TestReconcile(t *testing.T) { } } +// TestReconcileStandalone covers the no-ClusterPolicy path: the controller falls back +// to the GPUCluster for the cluster-wide configuration, and fails early when +// neither object exists. +func TestReconcileStandalone(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, nvidiav1alpha1.AddToScheme(scheme)) + require.NoError(t, gpuv1.AddToScheme(scheme)) + + cp := &gpuv1.ClusterPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster-policy"}, + Spec: gpuv1.ClusterPolicySpec{ + Driver: gpuv1.DriverSpec{ + UseNvidiaDriverCRD: ptr.To(true), + }, + HostPaths: gpuv1.HostPathsSpec{RootFS: "/cp-root"}, + }, + } + gpuCluster := &nvidiav1alpha1.GPUCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "gpu-cluster-config"}, + Spec: nvidiav1alpha1.GPUClusterSpec{ + HostPaths: gpuv1.HostPathsSpec{RootFS: "/gpuCluster-root"}, + }, + } + + tests := []struct { + name string + objects []client.Object + expectedErr string + expectedHostRoot string + }{ + { + name: "no ClusterPolicy, GPUCluster provides the host root", + objects: []client.Object{gpuCluster}, + expectedHostRoot: "/gpuCluster-root", + }, + { + name: "ClusterPolicy preferred over GPUCluster", + objects: []client.Object{cp, gpuCluster}, + expectedHostRoot: "/cp-root", + }, + { + name: "neither ClusterPolicy nor GPUCluster", + expectedErr: "no ClusterPolicy or GPUCluster object found in the cluster", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + driver := &nvidiav1alpha1.NVIDIADriver{ + ObjectMeta: metav1.ObjectMeta{Name: "test-driver"}, + } + + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(append([]client.Object{driver}, tc.objects...)...). + WithStatusSubresource(&nvidiav1alpha1.NVIDIADriver{}). + Build() + + updater := &FakeConditionUpdater{} + stateManager := &fakeStateManager{results: state.Results{Status: state.SyncStateReady}} + + reconciler := &NVIDIADriverReconciler{ + Client: c, + Scheme: scheme, + conditionUpdater: updater, + nodeSelectorValidator: &FakeNodeSelectorValidator{}, + stateManager: stateManager, + } + + req := ctrl.Request{NamespacedName: types.NamespacedName{Name: driver.Name}} + _, err := reconciler.Reconcile(context.Background(), req) + + if tc.expectedErr != "" { + require.ErrorContains(t, err, tc.expectedErr) + require.Equal(t, nvidiav1alpha1.NotReady, updater.LastErrorState) + return + } + require.NoError(t, err) + + hostRoot, ok := stateManager.lastCatalog.Get(state.InfoTypeHostRoot).(string) + require.True(t, ok, "info catalog must hold a host root string") + require.Equal(t, tc.expectedHostRoot, hostRoot) + + instance := &nvidiav1alpha1.NVIDIADriver{} + require.NoError(t, c.Get(context.Background(), types.NamespacedName{Name: driver.Name}, instance)) + require.Equal(t, nvidiav1alpha1.Ready, instance.Status.State) + }) + } +} + func TestReconcileConflictSetsNotReadyState(t *testing.T) { scheme := runtime.NewScheme() require.NoError(t, nvidiav1alpha1.AddToScheme(scheme))