increase unit test coverage

Author: adel121

internal/controller/setup.go

@@ -10,7 +10,10 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 
 	"github.com/DataDog/datadog-operator/internal/controller/datadogagent"
@@ -273,17 +276,28 @@ func startDatadogAgentProfiles(logger logr.Logger, mgr manager.Manager, pInfo ku
 	}).SetupWithManager(mgr)
 }
 
-func startDatadogCSIDriver(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.PlatformInfo, options SetupOptions, metricForwardersMgr datadog.MetricsForwardersManager) error {
-	if !options.DatadogCSIDriverEnabled {
-		logger.Info("Feature disabled, not starting the controller", "controller", csiDriverControllerName)
-		return nil
-	}
+// csiDriverManagerDeps is the subset of manager.Manager required to construct DatadogCSIDriverReconciler.
+type csiDriverManagerDeps interface {
+	GetClient() client.Client
+	GetScheme() *runtime.Scheme
+	GetEventRecorderFor(name string) record.EventRecorder
+}
 
-	return (&DatadogCSIDriverReconciler{
+func newDatadogCSIDriverReconciler(mgr csiDriverManagerDeps, options SetupOptions) *DatadogCSIDriverReconciler {
+	return &DatadogCSIDriverReconciler{
 		Client:   mgr.GetClient(),
 		Scheme:   mgr.GetScheme(),
 		Recorder: mgr.GetEventRecorderFor(csiDriverControllerName),
 		// Inject startup toleration on CSI node DaemonSet only when untaint coordinates with CSI.
 		UntaintInjectCSIStartupToleration: options.UntaintControllerEnabled && options.UntaintControllerWaitForCSIDriver,
-	}).SetupWithManager(mgr)
+	}
+}
+
+func startDatadogCSIDriver(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.PlatformInfo, options SetupOptions, metricForwardersMgr datadog.MetricsForwardersManager) error {
+	if !options.DatadogCSIDriverEnabled {
+		logger.Info("Feature disabled, not starting the controller", "controller", csiDriverControllerName)
+		return nil
+	}
+
+	return newDatadogCSIDriverReconciler(mgr, options).SetupWithManager(mgr)
 }

internal/controller/setup_test.go

@@ -7,10 +7,19 @@ package controller
 
 import (
 	"errors"
+	"os"
 	"testing"
 
 	"github.com/go-logr/logr"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/runtime"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/record"
+	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/log"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 
@@ -42,3 +51,57 @@ func TestSetupControllers_StarterErrorsAreBestEffort(t *testing.T) {
 		SetupOptions{UntaintControllerEnabled: true},
 	))
 }
+
+type csiMgrStub struct {
+	cli client.Client
+	sch *runtime.Scheme
+	rec record.EventRecorder
+}
+
+func (s *csiMgrStub) GetClient() client.Client { return s.cli }
+
+func (s *csiMgrStub) GetScheme() *runtime.Scheme { return s.sch }
+
+func (s *csiMgrStub) GetEventRecorderFor(string) record.EventRecorder { return s.rec }
+
+func TestNewDatadogCSIDriverReconciler_UntaintInjectCSIStartupToleration(t *testing.T) {
+	s := runtime.NewScheme()
+	cli := fake.NewClientBuilder().WithScheme(s).Build()
+	rec := record.NewFakeRecorder(1)
+	stub := &csiMgrStub{cli: cli, sch: s, rec: rec}
+
+	for _, tc := range []struct {
+		name    string
+		untaint bool
+		waitCSI bool
+		want    bool
+	}{
+		{"both true", true, true, true},
+		{"untaint off", false, true, false},
+		{"wait CSI off", true, false, false},
+		{"both off", false, false, false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			r := newDatadogCSIDriverReconciler(stub, SetupOptions{
+				UntaintControllerEnabled:          tc.untaint,
+				UntaintControllerWaitForCSIDriver: tc.waitCSI,
+			})
+			assert.Equal(t, tc.want, r.UntaintInjectCSIStartupToleration)
+		})
+	}
+}
+
+func TestStartUntaint_NewUntaintReconcilerErrorIsWrapped(t *testing.T) {
+	ctrl.SetLogger(logr.Discard())
+	t.Setenv(EnvTimeoutPolicy, "unknown")
+	t.Cleanup(func() { _ = os.Unsetenv(EnvTimeoutPolicy) })
+
+	s := runtime.NewScheme()
+	require.NoError(t, clientgoscheme.AddToScheme(s))
+	mgr, err := ctrl.NewManager(&rest.Config{}, manager.Options{Scheme: s, LeaderElection: false})
+	require.NoError(t, err)
+
+	err = startUntaint(logr.Discard(), mgr, kubernetes.PlatformInfo{}, SetupOptions{UntaintControllerEnabled: true}, nil)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "untaint controller setup")
+}

internal/controller/untaint_controller_test.go

@@ -10,6 +10,7 @@ import (
 	"encoding/json"
 	"errors"
 	"os"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -721,6 +722,139 @@ func TestReconcile_CSI_agentReadyNoCsiPodKeepsTaint(t *testing.T) {
 	assert.True(t, hasTaint(fresh))
 }
 
+func TestReconcile_CSI_agentReadyNoCsiPod_zeroCreationTimestampRequeuesSchedulingTimeout(t *testing.T) {
+	now := testNow()
+	node := &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:              testNodeName,
+			CreationTimestamp: metav1.Time{}, // zero — defensive branch in reconcileAgentReadyWaitForCSI
+		},
+		Spec: corev1.NodeSpec{Taints: []corev1.Taint{untaint.AgentNotReadyTaint()}},
+	}
+	agent := agentPod(testPodName, testPodNS, testNodeName, true, 1*time.Minute, now)
+	c := newFakeClient(t, node, agent)
+	const scheduling = 7 * time.Minute
+	r, _ := newReconciler(t, c, now, PolicyRemove, time.Minute, scheduling, true)
+
+	result, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: testNodeName}})
+	require.NoError(t, err)
+	assert.Equal(t, scheduling, result.RequeueAfter)
+
+	fresh := &corev1.Node{}
+	require.NoError(t, c.Get(context.Background(), types.NamespacedName{Name: testNodeName}, fresh))
+	require.True(t, hasTaint(fresh))
+}
+
+func TestReconcile_CSI_agentReadyNoCsiPod_schedulingTimeoutRemovesTaint(t *testing.T) {
+	now := testNow()
+	const scheduling = 5 * time.Minute
+	node := taintedNode(testNodeName, scheduling+time.Minute, now)
+	agent := agentPod(testPodName, testPodNS, testNodeName, true, 1*time.Minute, now)
+	c := newFakeClient(t, node, agent)
+	r, _ := newReconciler(t, c, now, PolicyRemove, 10*time.Minute, scheduling, true)
+
+	result, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: testNodeName}})
+	require.NoError(t, err)
+	assert.Equal(t, ctrl.Result{}, result)
+
+	fresh := &corev1.Node{}
+	require.NoError(t, c.Get(context.Background(), types.NamespacedName{Name: testNodeName}, fresh))
+	assert.False(t, hasTaint(fresh))
+}
+
+func TestReconcile_CSI_agentReadyCsiNotReady_noStartTimeRequeuesReadinessTimeout(t *testing.T) {
+	now := testNow()
+	node := taintedNode(testNodeName, 0, now)
+	agent := agentPod(testPodName, testPodNS, testNodeName, true, 1*time.Minute, now)
+	// not Ready, startedAgo 0 → no Status.StartTime → latestPodStartTime !ok branch
+	csi := csiNodeServerPod("csi-1", testPodNS, testNodeName, false, 0, now)
+	c := newFakeClient(t, node, agent, csi)
+	const readiness = 9 * time.Minute
+	r, _ := newReconciler(t, c, now, PolicyRemove, readiness, time.Minute, true)
+
+	result, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: testNodeName}})
+	require.NoError(t, err)
+	assert.Equal(t, readiness, result.RequeueAfter)
+
+	fresh := &corev1.Node{}
+	require.NoError(t, c.Get(context.Background(), types.NamespacedName{Name: testNodeName}, fresh))
+	assert.True(t, hasTaint(fresh))
+}
+
+func TestReconcile_CSI_agentReadyCsiNotReady_readinessTimeoutRemovesTaint(t *testing.T) {
+	now := testNow()
+	node := taintedNode(testNodeName, 0, now)
+	agent := agentPod(testPodName, testPodNS, testNodeName, true, 1*time.Minute, now)
+	const readiness = time.Minute
+	csi := csiNodeServerPod("csi-1", testPodNS, testNodeName, false, 2*readiness, now)
+	c := newFakeClient(t, node, agent, csi)
+	r, _ := newReconciler(t, c, now, PolicyRemove, readiness, 5*time.Minute, true)
+
+	result, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: testNodeName}})
+	require.NoError(t, err)
+	assert.Equal(t, ctrl.Result{}, result)
+
+	fresh := &corev1.Node{}
+	require.NoError(t, c.Get(context.Background(), types.NamespacedName{Name: testNodeName}, fresh))
+	assert.False(t, hasTaint(fresh))
+}
+
+func TestReconcile_CSI_listDriverPodsError(t *testing.T) {
+	now := testNow()
+	node := taintedNode(testNodeName, 0, now)
+	agent := agentPod(testPodName, testPodNS, testNodeName, true, 1*time.Minute, now)
+	base := newFakeClient(t, node, agent)
+	var listCalls atomic.Int32
+	c := interceptor.NewClient(base, interceptor.Funcs{
+		List: func(ctx context.Context, cl client.WithWatch, list client.ObjectList, opts ...client.ListOption) error {
+			if listCalls.Add(1) == 2 {
+				return errors.New("simulated CSI pod list failure")
+			}
+			return base.List(ctx, list, opts...)
+		},
+	})
+	r, _ := newReconciler(t, c, now, PolicyRemove, time.Minute, time.Minute, true)
+
+	_, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: testNodeName}})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "failed to list CSI driver pods")
+}
+
+func TestReconcile_CSI_bothReadyConflictReturnsRequeue(t *testing.T) {
+	now := testNow()
+	node := taintedNode(testNodeName, 0, now)
+	agent := agentPod(testPodName, testPodNS, testNodeName, true, 1*time.Minute, now)
+	csi := csiNodeServerPod("csi-1", testPodNS, testNodeName, true, 1*time.Minute, now)
+	base := newFakeClient(t, node, agent, csi)
+	c := interceptor.NewClient(base, interceptor.Funcs{
+		Patch: func(context.Context, client.WithWatch, client.Object, client.Patch, ...client.PatchOption) error {
+			return apierrors.NewConflict(schema.GroupResource{Resource: "nodes"}, testNodeName, errors.New("race"))
+		},
+	})
+	r, _ := newReconciler(t, c, now, PolicyRemove, time.Minute, time.Minute, true)
+
+	result, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: testNodeName}})
+	require.NoError(t, err)
+	assert.Equal(t, conflictRequeueDelay, result.RequeueAfter)
+}
+
+func TestReconcile_CSI_bothReadyPatchErrorBubbles(t *testing.T) {
+	now := testNow()
+	node := taintedNode(testNodeName, 0, now)
+	agent := agentPod(testPodName, testPodNS, testNodeName, true, 1*time.Minute, now)
+	csi := csiNodeServerPod("csi-1", testPodNS, testNodeName, true, 1*time.Minute, now)
+	base := newFakeClient(t, node, agent, csi)
+	c := interceptor.NewClient(base, interceptor.Funcs{
+		Patch: func(context.Context, client.WithWatch, client.Object, client.Patch, ...client.PatchOption) error {
+			return errors.New("boom")
+		},
+	})
+	r, _ := newReconciler(t, c, now, PolicyRemove, time.Minute, time.Minute, true)
+
+	_, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: testNodeName}})
+	require.Error(t, err)
+}
+
 func TestPodWatchPredicate_withCSI(t *testing.T) {
 	now := testNow()
 	r, _ := newReconciler(t, newFakeClient(t), now, PolicyRemove, time.Minute, time.Minute, true)