[CONTP-1504] Manage DatadogCSIDriver from DatadogAgent#2857
Conversation
Define the DatadogCSIDriver, DatadogCSIDriverSpec, DatadogCSIDriverOverride, and DatadogCSIDriverStatus types in api/datadoghq/v1alpha1. The CRD enables declarative management of the Datadog CSI Driver via the operator, replacing the standalone Helm chart deployment. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Controller-runtime wiring for the DatadogCSIDriver reconciler. Watches the primary CR with GenerationChangedPredicate, owned DaemonSets for all changes (including status), and CSIDriver objects via label-based enqueue for drift detection on the cluster-scoped resource. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Implements the reconciliation logic for the DatadogCSIDriver controller: - Deferred SSA status patch with ObservedGeneration tracking - CSIDriver object management with Get+DeepEqual+Update for full drift reversion - DaemonSet management with the same pattern, including label enforcement - Override system with merge-by-name semantics (env vars, volumes, mounts) - Image resolution via pkg/images (supports tag-only overrides) - Finalizer-based cleanup of the cluster-scoped CSIDriver on deletion - Comprehensive unit tests covering creation, updates, drift, deletion, overrides Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Register the controller in setup.go with a feature flag and add the -datadogCSIDriverEnabled flag (default: false) to cmd/main.go. Also registers the storagev1 scheme required for CSIDriver object management. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Treat the DatadogCSIDriver as a desired-state object like DDAI: if someone modifies it externally, reconcile it back to the operator's desired state on the next loop. Uses apiequality.Semantic.DeepEqual to compare specs and kubernetes.UpdateFromObject to update. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (79.24%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2857 +/- ##
==========================================
+ Coverage 40.69% 40.83% +0.14%
==========================================
Files 321 322 +1
Lines 28413 28519 +106
==========================================
+ Hits 11563 11647 +84
- Misses 16015 16026 +11
- Partials 835 846 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
…te-ddcsi-from-dda
…o to consolide with const.go
…te-ddcsi-from-dda
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b338d627f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… DatadogCSIDriver controller
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99fc2f521a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 231409b572
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89b95fc985
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
adel121
left a comment
There was a problem hiding this comment.
LGTM! Thank you 🙇
Approving with a minor nit.
* Add DatadogCSIDriver CRD types and generated manifests Define the DatadogCSIDriver, DatadogCSIDriverSpec, DatadogCSIDriverOverride, and DatadogCSIDriverStatus types in api/datadoghq/v1alpha1. The CRD enables declarative management of the Datadog CSI Driver via the operator, replacing the standalone Helm chart deployment. * Add DatadogCSIDriver outer controller with RBAC markers Controller-runtime wiring for the DatadogCSIDriver reconciler. Watches the primary CR with GenerationChangedPredicate, owned DaemonSets for all changes (including status), and CSIDriver objects via label-based enqueue for drift detection on the cluster-scoped resource. * Add DatadogCSIDriver reconciler, DaemonSet and CSIDriver builders Implements the reconciliation logic for the DatadogCSIDriver controller: - Deferred SSA status patch with ObservedGeneration tracking - CSIDriver object management with Get+DeepEqual+Update for full drift reversion - DaemonSet management with the same pattern, including label enforcement - Override system with merge-by-name semantics (env vars, volumes, mounts) - Image resolution via pkg/images (supports tag-only overrides) - Finalizer-based cleanup of the cluster-scoped CSIDriver on deletion - Comprehensive unit tests covering creation, updates, drift, deletion, overrides * Wire DatadogCSIDriver controller into operator startup Register the controller in setup.go with a feature flag and add the -datadogCSIDriverEnabled flag (default: false) to cmd/main.go. Also registers the storagev1 scheme required for CSIDriver object management. * Add CreateDatadogCSIDriver field to CSIConfig * Add reconcileDatadogCSIDriver for creating DatadogCSIDriver from DDA * Wire reconcileDatadogCSIDriver into manageDDADependenciesWithDDAI * Add tests for reconcileDatadogCSIDriver * Add update logic to createOrUpdateDatadogCSIDriver Treat the DatadogCSIDriver as a desired-state object like DDAI: if someone modifies it externally, reconcile it back to the operator's desired state on the next loop. Uses apiequality.Semantic.DeepEqual to compare specs and kubernetes.UpdateFromObject to update. * remove unnecessary variable declaration * fix test after merging main * Scope csidriver RBAC to Datadog driver * Add comment why these were re-implemented but could be extracted for future work * Move DD_APM_ENABLED to shared controller constants * Move image tag and registries to proper package and delete defaults.go to consolide with const.go * Remove useless variable after constant driver name * Switch to context based logging * Fix pointer use * Always create DDCSIDriver unless existing CSIDriver with name and managed by Helm, remove CRD option * Add labels on created DatadogCSIDriver * Add tolerations + customize socket paths based on config on DatadogAgent * Use JSON patch instead of UpdateFromObject to keep finalizer added by DatadogCSIDriver controller * Do not create DatadogCSIDriver if controller not enabled and share info to DDA * Do not crash existing CSIDrivers users, simply skip * add debug logs * add DDCSI to informer to reconcile instantly * Add manageDatadogCSIDriver flag to allow smooth transitions to user-managed DDCSIs * guard update on controllerref to avoid managing same namespace/name DDCSI * Add tolerations, affinity, nodeselector to CRD field + remove automatic tolerations from DDA * rename to autoManage --------- (cherry picked from commit e3f69bf) Co-authored-by: Timothée Bavelier <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
What does this PR do?
When
spec.global.csi.enabled=true, the DatadogAgent controller can now create and manage aDatadogCSIDrivercustom resource on behalf of the DDA. The DatadogCSIDriver controller then reconciles it into the CSI DaemonSet and the cluster-scopedk8s.csi.datadoghq.comCSIDriver.Controller-level opt-in. Management is gated on the existing operator flag
--datadogCSIDriverEnabled(defaultfalse). With the flag off,csi.enabled=truekeeps its long-standing semantics — the Agent uses a CSI driver that's installed by some other means (Helm chart, manual apply, GitOps) — and the operator doesn't touch anything. This preserves backward compatibility for existing clusters.Defer to external installs. On the first-time create path (no DDA-owned CR yet), if a cluster-scoped
k8s.csi.datadoghq.comCSIDriver is already present, the operator defers and skips creation. Avoids collisions with Helm/manual installs. Once the operator owns a CR, subsequent reconciles stay on the update path regardless of external CSIDriver presence.Per-DDA opt-out for migrations. New field
spec.global.csi.autoManage(*bool, defaulttrue). Flipping tofalsetriggers cleanup of the operator-owned CR so users can hand ownership over to aDatadogCSIDriverthey maintain themselves (useful when they need customizations not exposed via DDA spec) without having to togglecsi.enabledoff/on.Spec propagation from DDA. The created DatadogCSIDriver is built from:
Spec.APMSocketPath←dda.Spec.Features.APM.UnixDomainSocketConfig.PathSpec.DSDSocketPath←dda.Spec.Features.Dogstatsd.UnixDomainSocketConfig.PathSpec.Override.Tolerations←dda.Spec.Global.CSI.TolerationsSpec.Override.NodeSelector←dda.Spec.Global.CSI.NodeSelectorSpec.Override.Affinity.NodeAffinity←dda.Spec.Global.CSI.NodeAffinityapp.kubernetes.io/managed-by=datadog-operator, etc.)controllerutil.SetControllerReference→ ownerRef for cascade deleteScheduling fields (
tolerations,nodeSelector,nodeAffinity) are set explicitly onspec.global.csirather than being implicitly inherited from the node-agent component override. TheSpec.Overridestruct is only populated when at least one scheduling field is set.Update semantics. Spec-drift updates use a
client.MergeFromJSON merge patch (instead ofUpdateFromObject), so only the fields we own (.spec, labels, annotations) appear in the wire payload. Finalizers added by the sibling DatadogCSIDriver controller (finalizer.datadoghq.com/csi-driver) and.statusare not touched — prevents a class of bug where a spec-drift update would wipe the finalizer and a subsequent delete would leak the cluster-scoped CSIDriver.Event-driven reconciles. The DDA controller now declares
Owns(&v1alpha1.DatadogCSIDriver{})(gated onDatadogCSIDriverEnabledto avoid startup crashes when the CRD is absent, matching theDatadogAgentInternal/ExtendedDaemonSetpattern). Drift on the operator-owned CR now self-heals within milliseconds via watch events rather than waiting for the 15s periodic requeue.Motivation
Simplify CSI driver setup. Users can configure CSI on their DatadogAgent (and for cluster admins that want the operator to install the driver, flip
--datadogCSIDriverEnabledat deployment time) without maintaining a separateDatadogCSIDriverby hand. For advanced users who need finer-grained control,autoManage=falseoffers a seamless migration path off "automatic" operator management based on DDA spec.Additional Notes
reconcileDatadogCSIDriver.csidriversrules (list/watch/create cluster-wide, get/update/patch/delete scoped tok8s.csi.datadoghq.com) cover the new external-check Get.Minimum Agent Versions
N/A — this is an operator-level feature.
Describe your test plan
Unit tests in
ddcsi_test.gocover:Disabled— csi.enabled=false, no CR createdEnabledAndCreated— csi.enabled=true, flag on, CR created with owner ref and operator-managed labelsSpecFromDDA— APMSocketPath, DSDSocketPath, and tolerations (viaspec.global.csi.tolerations) propagated from DDANodeSelectorPropagated—spec.global.csi.nodeSelectorforwarded toSpec.Override.NodeSelectorNodeAffinityPropagated—spec.global.csi.nodeAffinityforwarded toSpec.Override.Affinity.NodeAffinityNoOverrideWhenNoTolerations— override not set when no scheduling fields are configuredAutoManageDisabledCleansUpAndDoesNotRecreate— opt-out deletes the CR and stays out of the wayAutoManageDisabledSkipsForeignCR— opt-out honorsIsControlledByguardControllerDisabled— flag off is a no-op (no error, no CR)DefersToExternalCSIDriver— pre-existing cluster-scoped CSIDriver blocks first-time createExternalAppearsAfterCRCreated— once owning a CR, external appearance doesn't abandon itIdempotent— repeat reconciles are no-opsCleanupOnDisable— owned resource deleted on toggleCleanupSkipsNotOwned— manually-created CRs left aloneUpdateOnSpecDrift— drift reconciled back via merge patchUpdatePreservesFinalizers— spec-drift update preserves externally-added finalizers (finalizer-wipe bug regression test)CleanupCRDNotAvailable— no error when CRD isn't presentChecklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel