From 45945b54bebc6cc5dc274389f5ab8b9f72f8b7c2 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 26 Oct 2026 13:20:28 -0700 Subject: [PATCH 1/3] fix: keep a ClusterPolicy precompiled DaemonSet per raw kernel The DaemonSet name used getSanitizedKernelVersion, which strips architecture and page size. The node selector and the image tag keep the raw kernel, so x86_64 and aarch64 updated one object. Keep those tokens in the name. After the new objects exist, delete a precompiled DaemonSet whose name is no longer desired. Signed-off-by: Sebastien Tardif --- controllers/object_controls.go | 50 +++++++++++++++++- controllers/object_controls_test.go | 81 +++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 2 deletions(-) diff --git a/controllers/object_controls.go b/controllers/object_controls.go index 28d39b130..4e19f13c5 100644 --- a/controllers/object_controls.go +++ b/controllers/object_controls.go @@ -3271,10 +3271,22 @@ func getSanitizedKernelVersion(kernelVersion string) string { return strings.ToLower(sanitizedVersion) } +// kernelSuffixForPrecompiledDaemonSet is the kernel portion of a precompiled +// driver DaemonSet name. It rewrites '_' so the result is a legal name, and +// it keeps architecture and page-size tokens. getSanitizedKernelVersion drops +// those tokens, but the node selector and the image tag keep the raw kernel, +// so kernels that sanitize together still need different objects. +func kernelSuffixForPrecompiledDaemonSet(kernelVersion string) string { + return strings.Trim(strings.ToLower(strings.ReplaceAll(kernelVersion, "_", ".")), ".") +} + +func precompiledDriverDaemonSetName(kernelVersion, nodeOS string) string { + return commonDriverDaemonsetName + "-" + kernelSuffixForPrecompiledDaemonSet(kernelVersion) + "-" + nodeOS +} + func transformPrecompiledDriverDaemonset(obj *appsv1.DaemonSet, n ClusterPolicyController) (err error) { - sanitizedVersion := getSanitizedKernelVersion(n.currentKernelVersion) // prepare the DaemonSet to be kernel-version specific - obj.Name += "-" + sanitizedVersion + "-" + n.kernelVersionMap[n.currentKernelVersion] + obj.Name = precompiledDriverDaemonSetName(n.currentKernelVersion, n.kernelVersionMap[n.currentKernelVersion]) // add unique labels for each kernel-version specific Daemonset obj.Labels[precompiledIdentificationLabelKey] = precompiledIdentificationLabelValue @@ -4376,7 +4388,9 @@ func precompiledDriverDaemonsets(ctx context.Context, n ClusterPolicyController) } n.logger.V(1).Info("preparing pre-compiled driver daemonsets") + desired := make(map[string]struct{}, len(n.kernelVersionMap)) for kernelVersion, os := range n.kernelVersionMap { + desired[precompiledDriverDaemonSetName(kernelVersion, os)] = struct{}{} // set current kernel version n.currentKernelVersion = kernelVersion @@ -4394,11 +4408,43 @@ func precompiledDriverDaemonsets(ctx context.Context, n ClusterPolicyController) } } + // A name that only used the arch-stripped kernel is not in desired. + // Leave it in place when this pass failed to create the replacements. + if len(errs) == 0 && len(desired) > 0 { + if err := n.deleteUndesiredPrecompiledDaemonsets(ctx, desired); err != nil { + errs = append(errs, err) + } + } + // reset current kernel version n.currentKernelVersion = "" return overallState, errs } +func (n ClusterPolicyController) deleteUndesiredPrecompiledDaemonsets(ctx context.Context, desired map[string]struct{}) error { + list := &appsv1.DaemonSetList{} + err := n.client.List(ctx, list, client.MatchingLabels{ + precompiledIdentificationLabelKey: precompiledIdentificationLabelValue, + }) + if err != nil { + return fmt.Errorf("list precompiled driver daemonsets: %w", err) + } + for i := range list.Items { + ds := &list.Items[i] + if _, ok := desired[ds.Name]; ok { + continue + } + if !strings.HasPrefix(ds.Name, commonDriverDaemonsetName) { + continue + } + n.logger.Info("Deleting precompiled driver DaemonSet that does not match a current kernel", "Name", ds.Name) + if err := n.client.Delete(ctx, ds); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("delete precompiled driver daemonset %s: %w", ds.Name, err) + } + } + return nil +} + // ocpDriverToolkitDaemonSets goes through all the RHCOS versions // found in the cluster, sets `currentRhcosVersion` and calls the // original DaemonSet() function to create/update the RHCOS-specific diff --git a/controllers/object_controls_test.go b/controllers/object_controls_test.go index fab4485ca..829599d26 100644 --- a/controllers/object_controls_test.go +++ b/controllers/object_controls_test.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "log" + "maps" "os" "path/filepath" goruntime "runtime" @@ -1939,6 +1940,86 @@ func TestGetSanitizedKernelVersion(t *testing.T) { } } +func TestPrecompiledDriverDaemonSetsStayDistinctWhenKernelsSanitizeTogether(t *testing.T) { + ctx := context.Background() + // TestGetSanitizedKernelVersion treats these as one string. RHEL uname -r + // and the NFD kernel-version.full label include the architecture. + const ( + x86Kernel = "5.14.0-427.37.1.el9_4.x86_64" + armKernel = "5.14.0-427.37.1.el9_4.aarch64" + ) + require.Equal(t, getSanitizedKernelVersion(x86Kernel), getSanitizedKernelVersion(armKernel)) + + nodes := &corev1.NodeList{} + require.NoError(t, clusterPolicyController.client.List(ctx, nodes)) + require.GreaterOrEqual(t, len(nodes.Items), 2) + + savedLabels := map[string]map[string]string{} + savedMap := clusterPolicyController.kernelVersionMap + savedPolicy := clusterPolicyController.singleton.DeepCopy() + defer func() { + for name, labels := range savedLabels { + node := &corev1.Node{} + if err := clusterPolicyController.client.Get(ctx, types.NamespacedName{Name: name}, node); err != nil { + t.Errorf("restore node %s: %v", name, err) + continue + } + node.Labels = labels + if err := clusterPolicyController.client.Update(ctx, node); err != nil { + t.Errorf("restore node %s labels: %v", name, err) + } + } + clusterPolicyController.kernelVersionMap = savedMap + if err := updateClusterPolicy(&clusterPolicyController, savedPolicy); err != nil { + t.Errorf("restore ClusterPolicy: %v", err) + } + }() + + kernels := []string{x86Kernel, armKernel} + for i := range kernels { + node := nodes.Items[i].DeepCopy() + savedLabels[node.Name] = maps.Clone(node.Labels) + node.Labels[nfdKernelLabelKey] = kernels[i] + require.NoError(t, clusterPolicyController.client.Update(ctx, node)) + } + + kernelVersionMap, err := clusterPolicyController.getKernelVersionsMap() + require.NoError(t, err) + require.Len(t, kernelVersionMap, 2) + clusterPolicyController.kernelVersionMap = kernelVersionMap + + cp := getDriverTestInput("precompiled") + require.NoError(t, updateClusterPolicy(&clusterPolicyController, cp)) + addState(&clusterPolicyController, filepath.Join(cfg.root, driverAssetsPath)) + defer func() { + if err := removeState(&clusterPolicyController, clusterPolicyController.idx-1); err != nil { + t.Errorf("remove driver state: %v", err) + return + } + clusterPolicyController.idx-- + }() + + _, err = clusterPolicyController.step() + require.NoError(t, err) + + list := &appsv1.DaemonSetList{} + require.NoError(t, clusterPolicyController.client.List(ctx, list, client.MatchingLabels{"app": "nvidia-driver-daemonset"})) + require.Len(t, list.Items, 2) + + got := map[string]string{} + for i := range list.Items { + ds := &list.Items[i] + kernel := ds.Spec.Template.Spec.NodeSelector[nfdKernelLabelKey] + got[kernel] = ds.Name + driver := findContainerByName(ds.Spec.Template.Spec.Containers, "nvidia-driver-ctr") + require.NotNil(t, driver) + require.Contains(t, driver.Image, kernel) + } + require.NotEqual(t, got[x86Kernel], got[armKernel]) + require.Contains(t, got[x86Kernel], "x86.64") + require.Contains(t, got[armKernel], "aarch64") +} + func TestServiceMonitor(t *testing.T) { const ( testNamespace = "test-namespace" From fd2dca91af901ca4c98a43590b7a0d2e58890a14 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Tue, 20 Oct 2026 10:35:05 -0700 Subject: [PATCH 2/3] Keep separate precompiled pools when kernel names match getNodePools keyed each precompiled pool on the sanitized kernel. That string drops x86_64, aarch64, and _64k, so an x86 node and an arm node, or a 4k node and a 64k node, became one pool. The selector and image tag then used only the first node's raw kernel. Key the pool map on the raw kernel as well, and sort equal names by that kernel. Nodes that share a raw kernel still share one pool. Signed-off-by: Sebastien Tardif --- internal/state/nodepool.go | 16 +++++++++-- internal/state/nodepool_test.go | 51 +++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/internal/state/nodepool.go b/internal/state/nodepool.go index 21bde3f9d..ae48b34b6 100644 --- a/internal/state/nodepool.go +++ b/internal/state/nodepool.go @@ -139,9 +139,16 @@ func getNodePools(ctx context.Context, k8sClient client.Client, cr *nvidiav1alph nodePool.name = rhcosVersion } - if _, exists := nodePoolMap[nodePool.name]; !exists { + // Pool names drop arch and _64k. The node selector and the + // precompiled image tag keep the raw kernel, so x86_64 and + // aarch64 (or 4k and 64k) stay separate pools when those names match. + poolKey := nodePool.name + if nodePool.kernel != "" { + poolKey = nodePool.name + "/" + nodePool.kernel + } + if _, exists := nodePoolMap[poolKey]; !exists { logger.Info("Detected new node pool", "NodePool", nodePool) - nodePoolMap[nodePool.name] = nodePool + nodePoolMap[poolKey] = nodePool } } @@ -154,7 +161,10 @@ func sortedNodePools(nodePoolMap map[string]nodePool) []nodePool { nodePools = append(nodePools, nodePool) } sort.Slice(nodePools, func(i, j int) bool { - return nodePools[i].name < nodePools[j].name + if nodePools[i].name != nodePools[j].name { + return nodePools[i].name < nodePools[j].name + } + return nodePools[i].kernel < nodePools[j].kernel }) return nodePools } diff --git a/internal/state/nodepool_test.go b/internal/state/nodepool_test.go index e7e36955b..aea13853d 100644 --- a/internal/state/nodepool_test.go +++ b/internal/state/nodepool_test.go @@ -307,6 +307,57 @@ func TestGetNodePoolsPartitionsPrecompiledNodesByKernel(t *testing.T) { require.Equal(t, "5.15.0-70-generic_x86_64", nodePools[0].nodeSelector[nfdKernelLabelKey]) } +func TestGetNodePoolsKeepsPrecompiledKernelsThatSanitizeTogether(t *testing.T) { + require.NoError(t, corev1.AddToScheme(scheme.Scheme)) + + // x86_64 and aarch64 sanitize to the same string. See TestGetSanitizedKernelVersion. + const ( + x86Kernel = "5.14.0-427.37.1.el9_4.x86_64" + armKernel = "5.14.0-427.37.1.el9_4.aarch64" + ) + node := func(name, kernel string) *corev1.Node { + return &corev1.Node{ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{ + consts.GPUPresentLabel: "true", + consts.NVIDIADriverOwnerLabel: "driver-a", + nfdOSReleaseIDLabelKey: "rhel", + nfdOSVersionIDLabelKey: "9.4", + nfdOSVersionIDMajorLabelKey: "9", + nfdKernelLabelKey: kernel, + }, + }} + } + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme.Scheme). + WithObjects( + node("x86-node", x86Kernel), + node("arm-node", armKernel), + node("second-x86-node", x86Kernel), + ). + Build() + driver := &nvidiav1alpha1.NVIDIADriver{ + ObjectMeta: metav1.ObjectMeta{Name: "driver-a"}, + Spec: nvidiav1alpha1.NVIDIADriverSpec{ + UsePrecompiled: new(true), + }, + } + + nodePools, err := getNodePools(context.Background(), k8sClient, driver, false) + + require.NoError(t, err) + require.Len(t, nodePools, 2) + got := map[string]string{} + for _, pool := range nodePools { + require.Equal(t, "rhel9-5.14.0-427.37.1.el9.4", pool.name) + got[pool.kernel] = pool.nodeSelector[nfdKernelLabelKey] + } + require.Equal(t, map[string]string{ + x86Kernel: x86Kernel, + armKernel: armKernel, + }, got) +} + func TestGetNodePoolsPartitionsOpenShiftNodesByRHCOSVersion(t *testing.T) { require.NoError(t, corev1.AddToScheme(scheme.Scheme)) From 8a4bce59a6a21d756f573ad310e611e62ebf8f86 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Tue, 3 Nov 2026 13:24:47 -0800 Subject: [PATCH 3/3] fix: keep precompiled DaemonSet names and cleanup in one namespace A kernel that uses '_' and one that already uses '.' in that spot were rewritten to the same DaemonSet name. A kernel that contains '_' now gets a short hash of the raw string. Kernels with no '_' keep the previous name. Cleanup lists only the operator namespace. One failed delete does not skip the remaining stale DaemonSets. Signed-off-by: Sebastien Tardif --- controllers/object_controls.go | 27 ++++++++--- controllers/object_controls_test.go | 69 +++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 6 deletions(-) diff --git a/controllers/object_controls.go b/controllers/object_controls.go index 4e19f13c5..61afcaa78 100644 --- a/controllers/object_controls.go +++ b/controllers/object_controls.go @@ -18,6 +18,8 @@ package controllers import ( "context" + "crypto/sha256" + "encoding/hex" "errors" "fmt" "maps" @@ -3276,8 +3278,18 @@ func getSanitizedKernelVersion(kernelVersion string) string { // it keeps architecture and page-size tokens. getSanitizedKernelVersion drops // those tokens, but the node selector and the image tag keep the raw kernel, // so kernels that sanitize together still need different objects. +// +// Replacing '_' with '.' would give 5.14.0-el9.x86_64 and 5.14.0-el9.x86.64 +// the same name. A kernel that contains '_' also gets a short hash of the +// lowercased raw string. A kernel with no '_' keeps the name it had before. func kernelSuffixForPrecompiledDaemonSet(kernelVersion string) string { - return strings.Trim(strings.ToLower(strings.ReplaceAll(kernelVersion, "_", ".")), ".") + lower := strings.ToLower(kernelVersion) + suffix := strings.Trim(strings.ReplaceAll(lower, "_", "."), ".") + if !strings.Contains(lower, "_") { + return suffix + } + sum := sha256.Sum256([]byte(lower)) + return suffix + "-" + hex.EncodeToString(sum[:4]) } func precompiledDriverDaemonSetName(kernelVersion, nodeOS string) string { @@ -4423,12 +4435,15 @@ func precompiledDriverDaemonsets(ctx context.Context, n ClusterPolicyController) func (n ClusterPolicyController) deleteUndesiredPrecompiledDaemonsets(ctx context.Context, desired map[string]struct{}) error { list := &appsv1.DaemonSetList{} - err := n.client.List(ctx, list, client.MatchingLabels{ - precompiledIdentificationLabelKey: precompiledIdentificationLabelValue, - }) + err := n.client.List(ctx, list, + client.InNamespace(n.operatorNamespace), + client.MatchingLabels{ + precompiledIdentificationLabelKey: precompiledIdentificationLabelValue, + }) if err != nil { return fmt.Errorf("list precompiled driver daemonsets: %w", err) } + var deleteErrs []error for i := range list.Items { ds := &list.Items[i] if _, ok := desired[ds.Name]; ok { @@ -4439,10 +4454,10 @@ func (n ClusterPolicyController) deleteUndesiredPrecompiledDaemonsets(ctx contex } n.logger.Info("Deleting precompiled driver DaemonSet that does not match a current kernel", "Name", ds.Name) if err := n.client.Delete(ctx, ds); err != nil && !apierrors.IsNotFound(err) { - return fmt.Errorf("delete precompiled driver daemonset %s: %w", ds.Name, err) + deleteErrs = append(deleteErrs, fmt.Errorf("delete precompiled driver daemonset %s: %w", ds.Name, err)) } } - return nil + return errors.Join(deleteErrs...) } // ocpDriverToolkitDaemonSets goes through all the RHCOS versions diff --git a/controllers/object_controls_test.go b/controllers/object_controls_test.go index 829599d26..f14986493 100644 --- a/controllers/object_controls_test.go +++ b/controllers/object_controls_test.go @@ -48,6 +48,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/log/zap" gpuv1 "github.com/NVIDIA/gpu-operator/api/nvidia/v1" @@ -2020,6 +2021,74 @@ func TestPrecompiledDriverDaemonSetsStayDistinctWhenKernelsSanitizeTogether(t *t require.Contains(t, got[armKernel], "aarch64") } +func TestKernelSuffixKeepsDotAndUnderscoreKernelsDistinct(t *testing.T) { + dotted := "5.14.0-427.el9.x86.64" + underscored := "5.14.0-427.el9.x86_64" + require.NotEqual(t, + kernelSuffixForPrecompiledDaemonSet(dotted), + kernelSuffixForPrecompiledDaemonSet(underscored)) + require.Equal(t, "5.4.0-generic", kernelSuffixForPrecompiledDaemonSet("5.4.0-generic")) +} + +func precompiledDriverDS(namespace, name string) *appsv1.DaemonSet { + return &appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + Labels: map[string]string{ + precompiledIdentificationLabelKey: precompiledIdentificationLabelValue, + }, + }, + } +} + +func TestDeleteUndesiredPrecompiledDaemonsets(t *testing.T) { + ctx := context.Background() + scheme := runtime.NewScheme() + require.NoError(t, appsv1.AddToScheme(scheme)) + + const ( + operatorNS = "gpu-operator" + legacyName = "nvidia-driver-daemonset-5.14.0-427.37.1.el9.4-rhcos" + keepName = "nvidia-driver-daemonset-5.14.0-427.37.1.el9.4.x86.64-rhcos" + otherName = "nvidia-driver-daemonset-5.14.0-427.37.1.el9.4-rhcos" + failName = "nvidia-driver-daemonset-fail" + laterName = "nvidia-driver-daemonset-later" + ) + legacy := precompiledDriverDS(operatorNS, legacyName) + keep := precompiledDriverDS(operatorNS, keepName) + otherNS := precompiledDriverDS("other", otherName) + k8s := fake.NewClientBuilder().WithScheme(scheme).WithObjects(legacy, keep, otherNS).Build() + n := ClusterPolicyController{ + client: k8s, + operatorNamespace: operatorNS, + logger: ctrl.Log.WithName("test"), + } + require.NoError(t, n.deleteUndesiredPrecompiledDaemonsets(ctx, map[string]struct{}{keepName: {}})) + + err := k8s.Get(ctx, types.NamespacedName{Namespace: operatorNS, Name: legacyName}, &appsv1.DaemonSet{}) + require.True(t, apierrors.IsNotFound(err)) + require.NoError(t, k8s.Get(ctx, types.NamespacedName{Namespace: operatorNS, Name: keepName}, &appsv1.DaemonSet{})) + require.NoError(t, k8s.Get(ctx, types.NamespacedName{Namespace: "other", Name: otherName}, &appsv1.DaemonSet{})) + + fail := precompiledDriverDS(operatorNS, failName) + later := precompiledDriverDS(operatorNS, laterName) + failing := fake.NewClientBuilder().WithScheme(scheme).WithObjects(fail, later). + WithInterceptorFuncs(interceptor.Funcs{ + Delete: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.DeleteOption) error { + if obj.GetName() == failName { + return fmt.Errorf("injected delete error") + } + return c.Delete(ctx, obj, opts...) + }, + }).Build() + n.client = failing + err = n.deleteUndesiredPrecompiledDaemonsets(ctx, map[string]struct{}{}) + require.ErrorContains(t, err, "injected delete error") + err = failing.Get(ctx, types.NamespacedName{Namespace: operatorNS, Name: laterName}, &appsv1.DaemonSet{}) + require.True(t, apierrors.IsNotFound(err)) +} + func TestServiceMonitor(t *testing.T) { const ( testNamespace = "test-namespace"