Controller refactoring

internal/controller/controller_utils.go

@@ -0,0 +1,120 @@
+/*
+Copyright 2025 Intel Corporation. All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controller
+
+import (
+	v1alpha "github.com/intel/gpu-base-operator/api/v1alpha1"
+	core "k8s.io/api/core/v1"
+)
+
+func generateNodeSelector(cp *v1alpha.ClusterPolicy) map[string]string {
+	ns := map[string]string{
+		"kubernetes.io/arch": "amd64",
+	}
+
+	if len(cp.Spec.NodeSelector) > 0 {
+		for k, v := range cp.Spec.NodeSelector {
+			ns[k] = v
+		}
+	}
+
+	if cp.Spec.UseNFDLabeling {
+		ns["intel.feature.node.kubernetes.io/gpu"] = trueValue
+	}
+
+	return ns
+}
+
+func generateTolerations(cp *v1alpha.ClusterPolicy) []core.Toleration {
+	tolerations := []core.Toleration{}
+
+	if len(cp.Spec.Tolerations) > 0 {
+		tolerations = cp.Spec.Tolerations
+	}
+
+	return tolerations
+}
+
+func isClusterPolicyBeingDeleted(cp *v1alpha.ClusterPolicy) bool {
+	// CP is nil, which means the CR was deleted, so we should remove everything.
+	if cp == nil {
+		return true
+	}
+
+	// CP is marked for deletion, so we should remove everything.
+	if !cp.DeletionTimestamp.IsZero() {
+		return true
+	}
+
+	return false
+}
+
+func shouldRemoveDRA(cp *v1alpha.ClusterPolicy) bool {
+	if isClusterPolicyBeingDeleted(cp) {
+		return true
+	}
+
+	// DRA not selected
+	if cp.Spec.ResourceRegistration != resourceModeDRA {
+		return true
+	}
+
+	return false
+}
+
+func shouldRemoveDevicePlugin(cp *v1alpha.ClusterPolicy) bool {
+	if isClusterPolicyBeingDeleted(cp) {
+		return true
+	}
+
+	// DP not selected
+	if cp.Spec.ResourceRegistration != resourceModeDP {
+		return true
+	}
+
+	return false
+}
+
+func shouldRemoveXpumd(cp *v1alpha.ClusterPolicy) bool {
+	if isClusterPolicyBeingDeleted(cp) {
+		return true
+	}
+
+	// No resource monitoring, no xpumd
+	if !cp.Spec.ResourceMonitoring {
+		return true
+	}
+
+	return false
+}
+
+// Convert the integer based log level to a string based log level for the OTel config.
+func logLevelForXpum(cp *v1alpha.ClusterPolicy) string {
+	v := cp.Spec.XpuManagerSpec.LogLevel
+	v = max(cp.Spec.LogLevel, v)
+
+	switch v {
+	case 0:
+		return "error"
+	case 1:
+		return "warn"
+	case 2:
+		return "info"
+	default:
+		return "debug"
+	}
+}

internal/controller/deviceplugin_controller.go

@@ -26,13 +26,13 @@ import (
 	core "k8s.io/api/core/v1"
 	"k8s.io/klog/v2"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
-	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
 	v1alpha "github.com/intel/gpu-base-operator/api/v1alpha1"
 	"github.com/intel/gpu-base-operator/config/deployments"
 )
@@ -144,8 +144,18 @@ func dpArgs(spec *v1alpha.ClusterPolicy) []string {
 	return args
 }
 
+func (r *DevicePluginReconciler) buildDaemonSet(spec *v1alpha.ClusterPolicy) *apps.DaemonSet {
+	ds := deployments.DevicePluginDaemonset()
+	r.updateDaemonSetObject(ds, spec)
+	return ds
+}
+
+func (r *DevicePluginReconciler) buildDaemonSetName(crName string) string {
+	return fmt.Sprintf("%s-device-plugin", crName)
+}
+
 func (r *DevicePluginReconciler) updateDaemonSetObject(ds *apps.DaemonSet, spec *v1alpha.ClusterPolicy) {
-	name := fmt.Sprintf("%s-device-plugin", spec.Name)
+	name := r.buildDaemonSetName(spec.Name)
 
 	ds.Name = name
 	ds.Namespace = r.Opts.Namespace
@@ -157,25 +167,8 @@ func (r *DevicePluginReconciler) updateDaemonSetObject(ds *apps.DaemonSet, spec
 
 	ds.Spec.Template.Spec.Containers[0].Args = dpArgs(spec)
 
-	ds.Spec.Template.Spec.NodeSelector = map[string]string{
-		"kubernetes.io/arch": "amd64",
-	}
-
-	if len(spec.Spec.NodeSelector) > 0 {
-		for k, v := range spec.Spec.NodeSelector {
-			ds.Spec.Template.Spec.NodeSelector[k] = v
-		}
-	}
-
-	if spec.Spec.UseNFDLabeling {
-		ds.Spec.Template.Spec.NodeSelector["intel.feature.node.kubernetes.io/gpu"] = trueValue
-	}
-
-	if len(spec.Spec.Tolerations) > 0 {
-		ds.Spec.Template.Spec.Tolerations = spec.Spec.Tolerations
-	} else {
-		ds.Spec.Template.Spec.Tolerations = nil
-	}
+	ds.Spec.Template.Spec.NodeSelector = generateNodeSelector(spec)
+	ds.Spec.Template.Spec.Tolerations = generateTolerations(spec)
 
 	cspec := &ds.Spec.Template.Spec
 
@@ -233,29 +226,7 @@ func (r *DevicePluginReconciler) cleanupOpenShiftResources(ctx context.Context,
 	deleteOpenShiftSCCResources(ctx, r.Client, sccName, roleName, bindingName, saName, r.Opts.Namespace)
 }
 
-func (r *DevicePluginReconciler) createDaemonSet(ctx context.Context, obj client.Object) (ctrl.Result, error) {
-	spec := obj.(*v1alpha.ClusterPolicy)
-
-	ds := deployments.DevicePluginDaemonset()
-
-	r.updateDaemonSetObject(ds, spec)
-
-	if err := ctrl.SetControllerReference(obj, ds, r.Scheme); err != nil {
-		klog.Error(err, "unable to set controller reference")
-
-		return ctrl.Result{}, err
-	}
-
-	if err := r.Create(ctx, ds); err != nil {
-		klog.Error(err, "unable to create DaemonSet")
-
-		return ctrl.Result{}, err
-	}
-
-	return ctrl.Result{}, nil
-}
-
-func (r *DevicePluginReconciler) removeDeploymentIfExists(ctx context.Context) (ctrl.Result, error) {
+func (r *DevicePluginReconciler) removeDeploymentIfExists(ctx context.Context, cp *v1alpha.ClusterPolicy) (ctrl.Result, error) {
 	klog.V(4).Info("Removing Device Plugin deployment")
 
 	crName := r.Opts.ReqName
@@ -264,51 +235,48 @@ func (r *DevicePluginReconciler) removeDeploymentIfExists(ctx context.Context) (
 		r.cleanupOpenShiftResources(ctx, crName)
 	}
 
-	dss := &apps.DaemonSetList{}
-	labels := client.MatchingLabels{
-		appLabel: dpValue,
-		ownerKey: crName,
-	}
-
-	if err := r.List(ctx, dss, client.InNamespace(r.Opts.Namespace), labels); err != nil {
-		klog.Error(err, "unable to list child DaemonSets")
+	name := r.buildDaemonSetName(crName)
 
-		return ctrl.Result{}, err
+	ds := &apps.DaemonSet{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: r.Opts.Namespace,
+		},
 	}
 
-	if len(dss.Items) == 0 {
-		klog.V(4).Info("No DevicePlugin deployment found, nothing to do")
-
-		return ctrl.Result{}, nil
+	if err := r.Delete(ctx, ds); client.IgnoreNotFound(err) != nil {
+		return ctrl.Result{}, err
 	}
 
-	if err := r.Delete(ctx, &dss.Items[0]); err != nil {
-		return ctrl.Result{}, err
+	if cp != nil {
+		cp.Status.DevicePluginStatus = notAvailableStatus
 	}
 
 	klog.V(4).Info("DevicePlugin deployment removed")
 
 	return ctrl.Result{}, nil
 }
 
-func (r *DevicePluginReconciler) Reconcile(ctx context.Context, cp *v1alpha.ClusterPolicy) (ctrl.Result, error) {
-	_ = logf.FromContext(ctx)
+func (r *DevicePluginReconciler) updateStatus(ctx context.Context, cp *v1alpha.ClusterPolicy) error {
+	ds := &apps.DaemonSet{}
 
-	if cp == nil || !cp.DeletionTimestamp.IsZero() {
-		return r.removeDeploymentIfExists(ctx)
+	if err := r.Get(ctx, client.ObjectKey{Name: r.buildDaemonSetName(cp.Name), Namespace: r.Opts.Namespace}, ds); err != nil {
+		klog.Error(err, "unable to get DP DaemonSet to update status")
+
+		return err
 	}
 
-	if cp.Spec.ResourceRegistration != "dp" {
-		cp.Status.DevicePluginStatus = notAvailableStatus
+	cp.Status.DevicePluginStatus = fmt.Sprintf("%d/%d",
+		ds.Status.NumberReady, ds.Status.DesiredNumberScheduled)
 
-		return r.removeDeploymentIfExists(ctx)
-	}
+	return nil
+}
 
-	var olderDs apps.DaemonSetList
-	if err := r.List(ctx, &olderDs, client.InNamespace(r.Opts.Namespace), client.MatchingLabels{appLabel: dpValue}); err != nil {
-		klog.Error(err, "unable to list child DaemonSets")
+func (r *DevicePluginReconciler) Reconcile(ctx context.Context, cp *v1alpha.ClusterPolicy) (ctrl.Result, error) {
+	_ = logf.FromContext(ctx)
 
-		return ctrl.Result{}, err
+	if shouldRemoveDevicePlugin(cp) {
+		return r.removeDeploymentIfExists(ctx, cp)
 	}
 
 	if r.Opts.OpenShift {
@@ -319,36 +287,17 @@ func (r *DevicePluginReconciler) Reconcile(ctx context.Context, cp *v1alpha.Clus
 		}
 	}
 
-	if len(olderDs.Items) == 0 {
-		return r.createDaemonSet(ctx, cp)
-	}
-
-	// Update DaemonSet
-
-	ds := &olderDs.Items[0]
-	originalDs := ds.DeepCopy()
+	ds := r.buildDaemonSet(cp)
 
-	r.updateDaemonSetObject(ds, cp)
+	if _, err := controllerutil.CreateOrPatch(ctx, r.Client, ds, func() error {
+		r.updateDaemonSetObject(ds, cp)
 
-	dsDiff := cmp.Diff(originalDs.Spec.Template.Spec, ds.Spec.Template.Spec, cmpopts.EquateEmpty())
-	if len(dsDiff) > 0 {
-		klog.Info("DS difference", "diff", dsDiff)
-
-		if err := r.Update(ctx, ds); err != nil {
-			klog.Error(err, "unable to update daemonset", "DaemonSet", ds)
-
-			return ctrl.Result{}, err
-		}
-	}
-
-	if err := r.List(ctx, &olderDs, client.InNamespace(r.Opts.Namespace), client.MatchingLabels{appLabel: dpValue}); err != nil {
-		klog.Error(err, "unable to list child DaemonSets")
+		return nil
+	}); err != nil {
+		klog.Error(err, "unable to create or patch DP DaemonSet")
 
 		return ctrl.Result{}, err
 	}
 
-	cp.Status.DevicePluginStatus = fmt.Sprintf("%d/%d",
-		olderDs.Items[0].Status.NumberReady, olderDs.Items[0].Status.DesiredNumberScheduled)
-
-	return ctrl.Result{}, nil
+	return ctrl.Result{}, r.updateStatus(ctx, cp)
 }