[CONTP-1610][CONTP-1611] Wait for CSI driver node server pod readiness in untaint controller if csi feature is enabled

Author: adel121

docs/untaint_controller.md

@@ -5,27 +5,42 @@ This feature was introduced in Datadog Operator v1.28 and is currently in previe
 ## Overview
 
 The Untaint controller watches Kubernetes Nodes carrying the taint
-`agent.datadoghq.com/not-ready=presence:NoSchedule` and removes it once the
-Datadog Agent pod on that node is `Ready`. It is intended to run alongside a
-separate mechanism (cluster-autoscaler hook, CCM, admission webhook, etc.)
-that adds the taint to new nodes. The use case is keeping workloads off a
-node until the Datadog Agent is Ready, and recovering gracefully if the Agent never
-becomes Ready.
-
-Agent pods are matched by the label `agent.datadoghq.com/component=agent` in
-the operator's watched namespaces (`WATCH_NAMESPACE` /
+`agent.datadoghq.com/not-ready=presence:NoSchedule` and removes it when
+readiness criteria are met (see below), or after a configurable timeout. It is
+intended to run alongside a separate mechanism (cluster-autoscaler hook, CCM,
+admission webhook, etc.) that adds the taint to new nodes.
+
+**With `--untaintControllerEnabled` only** (or with `--datadogCSIDriverEnabled=false`):
+the controller removes the taint once the **node Agent** pod
+(`agent.datadoghq.com/component=agent`) on that node is `Ready`. Agent pods are
+listed in the operator's agent watch namespaces (`WATCH_NAMESPACE` /
 `DD_AGENT_WATCH_NAMESPACE`).
 
-If the Agent pod never reaches Ready on a tainted node, a configurable timeout
-policy ensures the node is never permanently unschedulable. Two clocks cover
-the two failure modes:
+**With both `--untaintControllerEnabled=true` and `--datadogCSIDriverEnabled=true`:**
+the controller waits until **both** the node Agent and **CSI
+node-server** pod (`app=datadog-csi-driver-node-server`) on the node are
+`Ready` before removing the taint. The taint stays until both are
+satisfied or a timeout fires. The operator's Pod informer then watches the
+**union** of `DD_AGENT_WATCH_NAMESPACE` and `DD_CSIDRIVER_WATCH_NAMESPACE` (all
+pods in those namespaces—keep namespaces tight). Ensure CSI namespaces are
+covered so the controller can list CSI pod status.
 
-- **Readiness timeout** — the Agent pod is on the node but not Ready. Clock:
-  `pod.Status.StartTime`. Pod recreation restarts the window; container
-  restarts inside the same pod do not.
-- **Scheduling timeout** — no Agent pod is on the node. Clock:
-  `node.metadata.creationTimestamp`. The expected path when a DaemonSet never
-  schedules a pod onto the node (taint not tolerated, missing labels, etc.).
+If a required pod never reaches Ready on a tainted node, a configurable timeout
+policy ensures the node is never permanently unschedulable. Two clocks cover
+the main failure modes:
+
+- **Readiness timeout** — at least one Agent pod is on the node but the Agent
+  is not Ready yet, **or** (with CSI enabled) the Agent is Ready but a CSI
+  node-server pod exists on the node and is not Ready. Clock: latest
+  `pod.Status.StartTime` among **Agent** pods in the first case, and among **CSI
+  node-server** pods only in the second (the Agent’s age does not shorten the
+  wait for CSI). Pod recreation restarts the window; container restarts inside the
+  same pod do not.
+- **Scheduling timeout** — no Agent pod is on the node, **or** (with CSI
+  enabled) the Agent is Ready but **no** CSI node-server pod is on the node
+  yet. Clock: `node.metadata.creationTimestamp`. Covers DaemonSets that never
+  schedule onto the node (taint not tolerated, missing labels, CSI still pulling,
+  etc.).
 
 A pod-recreation crash-loop faster than the readiness window can hold a node
 tainted indefinitely; run with `policy=keep` and alert on
@@ -49,12 +64,19 @@ args:
   - --untaintControllerEnabled=true
 ```
 
-When this flag is enabled, the operator also injects a toleration for
+| `--untaintControllerEnabled` | `--datadogCSIDriverEnabled` | Behavior |
+| ----------------------------- | --------------------------- | -------- |
+| `false` | any | Untaint controller off; no startup toleration injection for this feature on Agent or CSI. |
+| `true` | `false` | Agent-only readiness and Agent DaemonSet toleration (default historical behavior). |
+| `true` | `true` | Wait for Agent **and** CSI node-server Ready; widened Pod cache (agent + `DD_CSIDRIVER_WATCH_NAMESPACE` namespaces); toleration on Agent and CSI DaemonSets. |
+
+When this flag is enabled, the operator injects a toleration for
 `agent.datadoghq.com/not-ready=presence:NoSchedule` into the node Agent
 DaemonSet (or ExtendedDaemonSet) pod template, unless an equivalent toleration
-is already present. This avoids a deadlock where the node stays tainted because
-the Agent pod cannot schedule without the toleration, especially when admission
-webhook auto-injection is not in use.
+is already present. When **`--datadogCSIDriverEnabled`** is also true, the same
+toleration is injected into the **Datadog CSI node-server** DaemonSet pod
+template so the CSI workload can schedule on tainted nodes before the taint is
+removed.
 
 ## Configuration
 
@@ -81,6 +103,8 @@ Metrics, under the `untaint` Prometheus subsystem:
 
 Kubernetes Events (gated by `DD_UNTAINT_CONTROLLER_EVENTS_ENABLED=true`):
 
-- `TaintRemoved` (Normal) — taint removed because the Agent pod became Ready.
+- `TaintRemoved` (Normal) — taint removed after the Agent became Ready, or (when
+  the Datadog CSI driver controller is also enabled) after both the Agent and
+  CSI node-server pods became Ready.
 - `UntaintTimeout` — a timeout fired. Normal under `remove`, Warning under `keep`. Message carries the reason, elapsed time, and policy.
 

internal/controller/datadogcsidriver/const.go

@@ -6,8 +6,14 @@
 package datadogcsidriver
 
 const (
+	// AppLabelKey is the Kubernetes label key on CSI node-server pods.
+	AppLabelKey = "app"
+	// NodeServerDaemonSetAppValue is the label value identifying CSI node-server pods
+	// (and the default DaemonSet name).
+	NodeServerDaemonSetAppValue = "datadog-csi-driver-node-server"
+
 	// csiDsName is the default name of the CSI driver DaemonSet
-	csiDsName = "datadog-csi-driver-node-server"
+	csiDsName = NodeServerDaemonSetAppValue
 	// csiDriverName is the default name of the CSIDriver Kubernetes object
 	csiDriverName = "k8s.csi.datadoghq.com"
 	// defaultCSIDriverImageName is the default CSI driver container image name
@@ -51,7 +57,6 @@ const (
 	csiDriverPort = int32(5000)
 
 	// Pod labels
-	appLabelKey                     = "app"
 	admissionControllerEnabledLabel = "admission.datadoghq.com/enabled"
 
 	// finalizerName is the finalizer for CSIDriver object cleanup

internal/controller/datadogcsidriver/controller.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/DataDog/datadog-operator/api/datadoghq/v1alpha1"
 	"github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1"
+	componentagent "github.com/DataDog/datadog-operator/internal/controller/datadogagent/component/agent"
 )
 
 const (
@@ -35,17 +36,19 @@ const (
 
 // Reconciler reconciles a DatadogCSIDriver object
 type Reconciler struct {
-	client   client.Client
-	scheme   *runtime.Scheme
-	recorder record.EventRecorder
+	client                   client.Client
+	scheme                   *runtime.Scheme
+	recorder                 record.EventRecorder
+	untaintControllerEnabled bool
 }
 
 // NewReconciler creates a new DatadogCSIDriver reconciler
-func NewReconciler(client client.Client, scheme *runtime.Scheme, recorder record.EventRecorder) *Reconciler {
+func NewReconciler(client client.Client, scheme *runtime.Scheme, recorder record.EventRecorder, untaintControllerEnabled bool) *Reconciler {
 	return &Reconciler{
-		client:   client,
-		scheme:   scheme,
-		recorder: recorder,
+		client:                   client,
+		scheme:                   scheme,
+		recorder:                 recorder,
+		untaintControllerEnabled: untaintControllerEnabled,
 	}
 }
 
@@ -198,13 +201,16 @@ func (r *Reconciler) reconcileCSIDriver(ctx context.Context, instance *v1alpha1.
 }
 
 func (r *Reconciler) reconcileDaemonSet(ctx context.Context, instance *v1alpha1.DatadogCSIDriver) error {
+	logger := ctrl.LoggerFrom(ctx)
 	desired := buildDaemonSet(instance)
+	if r.untaintControllerEnabled {
+		componentagent.EnsureAgentNotReadyStartupToleration(logger, &desired.Spec.Template.Spec)
+	}
 
 	if err := controllerutil.SetControllerReference(instance, desired, r.scheme); err != nil {
 		return fmt.Errorf("setting owner reference: %w", err)
 	}
 
-	logger := ctrl.LoggerFrom(ctx)
 	nsName := types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}
 	current := &appsv1.DaemonSet{}
 	err := r.client.Get(ctx, nsName, current)

internal/controller/datadogcsidriver/controller_test.go

@@ -8,6 +8,7 @@ package datadogcsidriver
 import (
 	"context"
 	"fmt"
+	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -28,14 +29,15 @@ import (
 	"github.com/DataDog/datadog-operator/api/datadoghq/v1alpha1"
 	"github.com/DataDog/datadog-operator/pkg/images"
 	"github.com/DataDog/datadog-operator/pkg/kubernetes"
+	"github.com/DataDog/datadog-operator/pkg/untaint"
 )
 
 const (
 	testNamespace = "datadog"
 	testName      = "datadog-csi"
 )
 
-func newTestReconciler(t *testing.T, objects ...client.Object) (*Reconciler, client.Client) {
+func newTestReconciler(t *testing.T, untaintControllerEnabled bool, objects ...client.Object) (*Reconciler, client.Client) {
 	t.Helper()
 	s := scheme.Scheme
 	s.AddKnownTypes(v1alpha1.GroupVersion,
@@ -52,7 +54,7 @@ func newTestReconciler(t *testing.T, objects ...client.Object) (*Reconciler, cli
 	// Set the default controller-runtime logger so ctrl.LoggerFrom(ctx) works in tests
 	ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
 	recorder := record.NewFakeRecorder(10)
-	r := NewReconciler(c, s, recorder)
+	r := NewReconciler(c, s, recorder, untaintControllerEnabled)
 
 	return r, c
 }
@@ -73,7 +75,7 @@ func defaultCSIDriverCR() *v1alpha1.DatadogCSIDriver {
 
 func TestReconcile_CreatesResources(t *testing.T) {
 	instance := defaultCSIDriverCR()
-	r, c := newTestReconciler(t, instance)
+	r, c := newTestReconciler(t, false, instance)
 	ctx := context.Background()
 
 	// First reconcile: adds finalizer
@@ -150,7 +152,7 @@ func TestReconcile_CustomSocketPaths(t *testing.T) {
 	instance.Spec.APMSocketPath = &customAPM
 	instance.Spec.DSDSocketPath = &customDSD
 
-	r, c := newTestReconciler(t, instance)
+	r, c := newTestReconciler(t, false, instance)
 	ctx := context.Background()
 
 	// Reconcile twice (finalizer + create)
@@ -181,7 +183,7 @@ func TestReconcile_CustomSocketPaths(t *testing.T) {
 
 func TestReconcile_Deletion(t *testing.T) {
 	instance := defaultCSIDriverCR()
-	r, c := newTestReconciler(t, instance)
+	r, c := newTestReconciler(t, false, instance)
 	ctx := context.Background()
 
 	// Reconcile to add finalizer + create resources
@@ -224,7 +226,7 @@ func TestReconcile_Deletion(t *testing.T) {
 
 func TestReconcile_UpdateDaemonSetOnSpecChange(t *testing.T) {
 	instance := defaultCSIDriverCR()
-	r, c := newTestReconciler(t, instance)
+	r, c := newTestReconciler(t, false, instance)
 	ctx := context.Background()
 
 	// Reconcile to create resources
@@ -266,7 +268,7 @@ func TestReconcile_UpdateDaemonSetOnSpecChange(t *testing.T) {
 
 func TestReconcile_IdempotentNoUpdate(t *testing.T) {
 	instance := defaultCSIDriverCR()
-	r, c := newTestReconciler(t, instance)
+	r, c := newTestReconciler(t, false, instance)
 	ctx := context.Background()
 
 	// Reconcile to create resources
@@ -316,7 +318,7 @@ func TestReconcile_CSIDriverLabelsAdoption(t *testing.T) {
 		},
 	}
 
-	r, c := newTestReconciler(t, instance, existingCSIDriver)
+	r, c := newTestReconciler(t, false, instance, existingCSIDriver)
 	ctx := context.Background()
 
 	// Reconcile: add finalizer
@@ -369,7 +371,7 @@ func TestReconcile_Overrides(t *testing.T) {
 		},
 	}
 
-	r, c := newTestReconciler(t, instance)
+	r, c := newTestReconciler(t, false, instance)
 	ctx := context.Background()
 
 	// Reconcile twice (finalizer + create)
@@ -388,7 +390,7 @@ func TestReconcile_Overrides(t *testing.T) {
 	// Labels merged into pod template
 	assert.Equal(t, "containers", ds.Spec.Template.Labels["team"])
 	// Default labels still present
-	assert.Equal(t, csiDsName, ds.Spec.Template.Labels[appLabelKey])
+	assert.Equal(t, csiDsName, ds.Spec.Template.Labels[AppLabelKey])
 
 	// Tolerations applied
 	require.Len(t, ds.Spec.Template.Spec.Tolerations, 1)
@@ -423,7 +425,7 @@ func TestReconcile_StatusConditionOnCSIDriverError(t *testing.T) {
 	// Instead, we test that when the reconcile succeeds, the condition is Ready=True,
 	// and verify the status structure.
 	instance := defaultCSIDriverCR()
-	r, c := newTestReconciler(t, instance)
+	r, c := newTestReconciler(t, false, instance)
 	ctx := context.Background()
 
 	// Reconcile to add finalizer
@@ -450,7 +452,7 @@ func TestReconcile_StatusConditionOnCSIDriverError(t *testing.T) {
 
 func TestReconcile_CSIDriverSpecDriftIsReconciled(t *testing.T) {
 	instance := defaultCSIDriverCR()
-	r, c := newTestReconciler(t, instance)
+	r, c := newTestReconciler(t, false, instance)
 	ctx := context.Background()
 
 	// Reconcile to create resources.
@@ -487,6 +489,49 @@ func TestReconcile_CSIDriverSpecDriftIsReconciled(t *testing.T) {
 	assert.Contains(t, csiDriver.Spec.VolumeLifecycleModes, storagev1.VolumeLifecycleEphemeral)
 }
 
+func TestReconcile_DaemonSetIncludesStartupTolerationWhenUntaintEnabled(t *testing.T) {
+	instance := defaultCSIDriverCR()
+	r, c := newTestReconciler(t, true, instance)
+	ctx := context.Background()
+
+	_, err := r.Reconcile(ctx, instance)
+	require.NoError(t, err)
+	require.NoError(t, c.Get(ctx, types.NamespacedName{Name: testName, Namespace: testNamespace}, instance))
+
+	_, err = r.Reconcile(ctx, instance)
+	require.NoError(t, err)
+
+	ds := &appsv1.DaemonSet{}
+	require.NoError(t, c.Get(ctx, types.NamespacedName{Name: csiDsName, Namespace: testNamespace}, ds))
+	want := untaint.AgentNotReadyEqualToleration()
+	assert.True(t, tolerationListContains(ds.Spec.Template.Spec.Tolerations, want),
+		"expected %+v in %+v", want, ds.Spec.Template.Spec.Tolerations)
+}
+
+func TestReconcile_DaemonSetOmitsStartupTolerationWhenUntaintDisabled(t *testing.T) {
+	instance := defaultCSIDriverCR()
+	r, c := newTestReconciler(t, false, instance)
+	ctx := context.Background()
+	_, err := r.Reconcile(ctx, instance)
+	require.NoError(t, err)
+	require.NoError(t, c.Get(ctx, types.NamespacedName{Name: testName, Namespace: testNamespace}, instance))
+	_, err = r.Reconcile(ctx, instance)
+	require.NoError(t, err)
+	ds := &appsv1.DaemonSet{}
+	require.NoError(t, c.Get(ctx, types.NamespacedName{Name: csiDsName, Namespace: testNamespace}, ds))
+	want := untaint.AgentNotReadyEqualToleration()
+	assert.False(t, tolerationListContains(ds.Spec.Template.Spec.Tolerations, want))
+}
+
+func tolerationListContains(tols []corev1.Toleration, want corev1.Toleration) bool {
+	for i := range tols {
+		if reflect.DeepEqual(tols[i], want) {
+			return true
+		}
+	}
+	return false
+}
+
 // findCondition returns the condition with the given type, or nil.
 func findCondition(conditions []metav1.Condition, condType string) *metav1.Condition {
 	for i := range conditions {

internal/controller/datadogcsidriver/daemonset.go

@@ -29,10 +29,10 @@ func buildDaemonSet(instance *datadoghqv1alpha1.DatadogCSIDriver) *appsv1.Daemon
 	dsdSocketDir := filepath.Dir(dsdSocketPath)
 
 	labels := map[string]string{
-		appLabelKey: csiDsName,
+		AppLabelKey: csiDsName,
 	}
 	podLabels := map[string]string{
-		appLabelKey:                     csiDsName,
+		AppLabelKey:                     csiDsName,
 		admissionControllerEnabledLabel: "false",
 	}
 
@@ -50,7 +50,7 @@ func buildDaemonSet(instance *datadoghqv1alpha1.DatadogCSIDriver) *appsv1.Daemon
 		Spec: appsv1.DaemonSetSpec{
 			Selector: &metav1.LabelSelector{
 				MatchLabels: map[string]string{
-					appLabelKey: csiDsName,
+					AppLabelKey: csiDsName,
 				},
 			},
 			RevisionHistoryLimit: &revisionHistoryLimit,

internal/controller/datadogcsidriver_controller.go

@@ -27,10 +27,11 @@ import (
 
 // DatadogCSIDriverReconciler reconciles a DatadogCSIDriver object.
 type DatadogCSIDriverReconciler struct {
-	Client   client.Client
-	Scheme   *runtime.Scheme
-	Recorder record.EventRecorder
-	internal *datadogcsidriver.Reconciler
+	Client                   client.Client
+	Scheme                   *runtime.Scheme
+	Recorder                 record.EventRecorder
+	UntaintControllerEnabled bool
+	internal                 *datadogcsidriver.Reconciler
 }
 
 // RBACs for DatadogCSIDriver objects
@@ -60,7 +61,7 @@ func (r *DatadogCSIDriverReconciler) Reconcile(ctx context.Context, instance *da
 
 // SetupWithManager creates a new DatadogCSIDriver controller.
 func (r *DatadogCSIDriverReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	r.internal = datadogcsidriver.NewReconciler(r.Client, r.Scheme, r.Recorder)
+	r.internal = datadogcsidriver.NewReconciler(r.Client, r.Scheme, r.Recorder, r.UntaintControllerEnabled)
 
 	or := reconcile.AsReconciler[*datadoghqv1alpha1.DatadogCSIDriver](r.Client, r)
 	return ctrl.NewControllerManagedBy(mgr).