Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions api/datadoghq/v1alpha1/datadogagentprofile_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ import (
)

// ValidateDatadogAgentProfileSpec is used to check if a DatadogAgentProfileSpec is valid
func ValidateDatadogAgentProfileSpec(spec *DatadogAgentProfileSpec, datadogAgentInternalEnabled bool) error {
func ValidateDatadogAgentProfileSpec(spec *DatadogAgentProfileSpec) error {
if err := validateProfileAffinity(spec.ProfileAffinity); err != nil {
return err
}
if err := validateConfig(spec.Config, datadogAgentInternalEnabled); err != nil {
if err := validateConfig(spec.Config); err != nil {
return err
}

Expand All @@ -39,20 +39,17 @@ func validateProfileAffinity(profileAffinity *ProfileAffinity) error {
return nil
}

func validateConfig(spec *v2alpha1.DatadogAgentSpec, datadogAgentInternalEnabled bool) error {
func validateConfig(spec *v2alpha1.DatadogAgentSpec) error {
if spec == nil {
return undefinedError("config")
}
if err := validateFeatures(spec.Features, datadogAgentInternalEnabled); err != nil {
if err := validateFeatures(spec.Features); err != nil {
return err
}
// global is not supported
if spec.Global != nil {
return unsupportedError("global")
}
if !datadogAgentInternalEnabled && spec.Override == nil {
return undefinedError("config override")
}
for component, override := range spec.Override {
if err := validateOverride(component, override); err != nil {
return err
Expand All @@ -62,13 +59,10 @@ func validateConfig(spec *v2alpha1.DatadogAgentSpec, datadogAgentInternalEnabled
return nil
}

func validateFeatures(features *v2alpha1.DatadogFeatures, datadogAgentInternalEnabled bool) error {
func validateFeatures(features *v2alpha1.DatadogFeatures) error {
if features == nil {
return nil
}
if !datadogAgentInternalEnabled {
return fmt.Errorf("the 'features' field is only supported when DatadogAgentInternal is enabled")
}

// Only GPU feature is currently supported in DatadogAgentProfile context.
// Remove supported features from the `unsupportedFeatures` array.
Expand Down
101 changes: 35 additions & 66 deletions api/datadoghq/v1alpha1/datadogagentprofile_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,101 +155,70 @@ func TestIsValidDatadogAgentProfile(t *testing.T) {
},
},
}
invalidFeaturesNoDDAI := &DatadogAgentProfileSpec{
ProfileAffinity: basicProfileAffinity,
Config: &v2alpha1.DatadogAgentSpec{
Features: &v2alpha1.DatadogFeatures{
GPU: &v2alpha1.GPUFeatureConfig{
Enabled: ptr.To(true),
},
},
},
}

testCases := []struct {
name string
spec *DatadogAgentProfileSpec
datadogAgentInternalEnabled bool
wantErr string
name string
spec *DatadogAgentProfileSpec
wantErr string
}{
{
name: "valid dap",
spec: valid,
datadogAgentInternalEnabled: true,
},
{
name: "valid dap, resources specified in one container only",
spec: validResourceOverrideInOneContainerOnly,
datadogAgentInternalEnabled: true,
name: "valid dap",
spec: valid,
},
{
name: "invalid component override",
spec: invalidComponentOverride,
datadogAgentInternalEnabled: true,
wantErr: "component node selector override is not supported",
name: "valid dap, resources specified in one container only",
spec: validResourceOverrideInOneContainerOnly,
},
{
name: "invalid container override",
spec: invalidContainerOverride,
datadogAgentInternalEnabled: true,
wantErr: "container command override is not supported",
name: "invalid component override",
spec: invalidComponentOverride,
wantErr: "component node selector override is not supported",
},
{
name: "missing override when ddai disabled",
spec: missingOverride,
datadogAgentInternalEnabled: false,
wantErr: "config override must be defined",
name: "invalid container override",
spec: invalidContainerOverride,
wantErr: "container command override is not supported",
},
{
name: "missing config",
spec: missingConfig,
datadogAgentInternalEnabled: true,
wantErr: "config must be defined",
name: "missing override is valid",
spec: missingOverride,
},
{
name: "missing node selector requirement",
spec: missingNSR,
datadogAgentInternalEnabled: true,
wantErr: "profileNodeAffinity must have at least 1 requirement",
name: "missing config",
spec: missingConfig,
wantErr: "config must be defined",
},
{
name: "missing profile node affinity",
spec: missingNodeAffinity,
datadogAgentInternalEnabled: true,
wantErr: "profileNodeAffinity must be defined",
name: "missing node selector requirement",
spec: missingNSR,
wantErr: "profileNodeAffinity must have at least 1 requirement",
},
{
name: "missing profile affinity",
spec: missingProfileAffinity,
datadogAgentInternalEnabled: true,
wantErr: "profileAffinity must be defined",
name: "missing profile node affinity",
spec: missingNodeAffinity,
wantErr: "profileNodeAffinity must be defined",
},
{
name: "gpu feature override",
spec: validGPUFeature,
datadogAgentInternalEnabled: true,
name: "missing profile affinity",
spec: missingProfileAffinity,
wantErr: "profileAffinity must be defined",
},
{
name: "valid dap with features only when ddai enabled",
spec: validFeaturesNoOverride,
datadogAgentInternalEnabled: true,
name: "gpu feature override",
spec: validGPUFeature,
},
{
name: "dap with unsupported feature when ddai enabled",
spec: invalidFeatures,
datadogAgentInternalEnabled: true,
wantErr: "npm override is not supported",
name: "valid dap with features only, no override",
spec: validFeaturesNoOverride,
},
{
name: "features not supported when ddai disabled",
spec: invalidFeaturesNoDDAI,
datadogAgentInternalEnabled: false,
wantErr: "the 'features' field is only supported when DatadogAgentInternal is enabled",
name: "dap with unsupported feature",
spec: invalidFeatures,
wantErr: "npm override is not supported",
},
}
for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
result := ValidateDatadogAgentProfileSpec(test.spec, test.datadogAgentInternalEnabled)
result := ValidateDatadogAgentProfileSpec(test.spec)
if test.wantErr != "" {
assert.EqualError(t, result, test.wantErr)
} else {
Expand Down
55 changes: 18 additions & 37 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ type options struct {
edsSlowStartAdditiveIncrease string
supportCilium bool
datadogAgentEnabled bool
datadogAgentInternalEnabled bool
createControllerRevisions bool
datadogMonitorEnabled bool
datadogSLOEnabled bool
Expand Down Expand Up @@ -189,7 +188,6 @@ func (opts *options) Parse() {
flag.BoolVar(&opts.datadogCSIDriverEnabled, "datadogCSIDriverEnabled", false, "Enable the DatadogCSIDriver controller")

// DatadogAgentInternal
flag.BoolVar(&opts.datadogAgentInternalEnabled, "datadogAgentInternalEnabled", true, "Enable the DatadogAgentInternal controller")
flag.BoolVar(&opts.createControllerRevisions, "createControllerRevisions", false, "Enable creation of ControllerRevision snapshots on each DDA spec change")
Comment on lines 190 to 191
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve deprecated flag until bundled manifests drop it

Removing the datadogAgentInternalEnabled flag here makes the operator exit with flag provided but not defined in deployments that still pass it, and this repository still ships such a deployment (marketplaces/addon_manifest.yaml includes -datadogAgentInternalEnabled=true at line 18373). In that environment the manager never starts and the pod crashloops, so either a compatibility alias should be kept for one release or all shipped manifests using the old flag must be updated in the same change.

Useful? React with 👍 / 👎.


// ExtendedDaemonset configuration
Expand Down Expand Up @@ -235,10 +233,6 @@ func run(opts *options) error {
}
version.PrintVersionLogs(setupLog)

if !opts.datadogAgentInternalEnabled {
setupLog.Error(nil, "[WARNING] DatadogAgentInternal controller is disabled. This flag will be removed in Operator v1.27 and enabling DatadogAgentInternal will be required.")
}

// submits the maximum go routine setting as a metric
metrics.MaxGoroutines.Set(float64(opts.maximumGoroutines))

Expand Down Expand Up @@ -292,7 +286,6 @@ func run(opts *options) error {
RetryPeriod: &retryPeriod,
Cache: config.CacheOptions(setupLog, config.WatchOptions{
DatadogAgentEnabled: opts.datadogAgentEnabled,
DatadogAgentInternalEnabled: opts.datadogAgentInternalEnabled,
DatadogMonitorEnabled: opts.datadogMonitorEnabled,
DatadogSLOEnabled: opts.datadogSLOEnabled,
DatadogAgentProfileEnabled: opts.datadogAgentProfileEnabled,
Expand Down Expand Up @@ -322,6 +315,9 @@ func run(opts *options) error {
// Reader interface as returned from mgr.GetAPIReader() reads directly from API server bypassing cache and informer initialization.
credsManager := config.NewCredentialManagerWithDecryptor(mgr.GetAPIReader(), secrets.NewSecretBackend())
creds, err := credsManager.GetCredentials()
if err != nil {
setupLog.Error(err, "Unable to get credentials")
}

if opts.secretRefreshInterval > 0 && opts.secretBackendCommand == "" {
setupLog.Error(nil, "secretRefreshInterval is set but secretBackendCommand is not configured")
Expand All @@ -346,29 +342,13 @@ func run(opts *options) error {
}

if opts.remoteUpdatesEnabled {
if rcErr := setupFleetDaemon(setupLog, mgr, rcUpdater.Client(), opts.createControllerRevisions && opts.datadogAgentInternalEnabled); rcErr != nil {
if rcErr := setupFleetDaemon(setupLog, mgr, rcUpdater.Client(), opts.createControllerRevisions && opts.datadogAgentEnabled); rcErr != nil {
setupErrorf(setupLog, rcErr, "Unable to setup Fleet daemon")
}
}
}()
}

// Cleanup leftover DatadogAgentInternal resources if DDAI controller is disabled
if opts.datadogAgentEnabled && !opts.datadogAgentInternalEnabled {
go func() {
// Block until this controller manager is elected leader and controllers are set up
<-mgr.Elected()

// Wait a bit more to ensure reconciliation has had a chance to patch ownerRefs
time.Sleep(60 * time.Second)

setupLog.Info("Starting cleanup of DatadogAgentInternal resources")
if err = controller.CleanupDatadogAgentInternalResources(setupLog, restConfig); err != nil {
setupLog.Error(err, "Failed to cleanup DatadogAgentInternal resources")
}
}()
}

options := controller.SetupOptions{
SupportExtendedDaemonset: controller.ExtendedDaemonsetOptions{
Enabled: opts.supportExtendedDaemonset,
Expand All @@ -386,8 +366,7 @@ func run(opts *options) error {
SupportCilium: opts.supportCilium,
CredsManager: credsManager,
DatadogAgentEnabled: opts.datadogAgentEnabled,
DatadogAgentInternalEnabled: opts.datadogAgentInternalEnabled,
CreateControllerRevisions: opts.createControllerRevisions && opts.datadogAgentInternalEnabled, // Only enable if DDAI is also enabled.
CreateControllerRevisions: opts.createControllerRevisions && opts.datadogAgentEnabled,
DatadogMonitorEnabled: opts.datadogMonitorEnabled,
DatadogSLOEnabled: opts.datadogSLOEnabled,
OperatorMetricsEnabled: opts.operatorMetricsEnabled,
Expand Down Expand Up @@ -633,15 +612,16 @@ func setupAndStartOperatorMetadataForwarder(logger logr.Logger, client client.Re
DatadogSLOEnabled: options.datadogSLOEnabled,
DatadogGenericResourceEnabled: options.datadogGenericResourceEnabled,
DatadogAgentProfileEnabled: options.datadogAgentProfileEnabled,
DatadogAgentInternalEnabled: options.datadogAgentInternalEnabled,
LeaderElectionEnabled: options.enableLeaderElection,
ExtendedDaemonSetEnabled: options.supportExtendedDaemonset,
RemoteConfigEnabled: options.remoteConfigEnabled,
RemoteUpdatesEnabled: options.remoteUpdatesEnabled,
IntrospectionEnabled: options.introspectionEnabled,
ConfigDDURL: os.Getenv(constants.DDURL),
ConfigDDSite: os.Getenv(constants.DDSite),
ResourceCounts: make(map[string]int),
// Since v1.27, DDAI is always tied to DDA — no separate flag. Kept in telemetry for metric continuity.
DatadogAgentInternalEnabled: options.datadogAgentEnabled,
LeaderElectionEnabled: options.enableLeaderElection,
ExtendedDaemonSetEnabled: options.supportExtendedDaemonset,
RemoteConfigEnabled: options.remoteConfigEnabled,
RemoteUpdatesEnabled: options.remoteUpdatesEnabled,
IntrospectionEnabled: options.introspectionEnabled,
ConfigDDURL: os.Getenv(constants.DDURL),
ConfigDDSite: os.Getenv(constants.DDSite),
ResourceCounts: make(map[string]int),
}

omf.Start()
Expand All @@ -655,8 +635,9 @@ func setupAndStartCRDMetadataForwarder(logger logr.Logger, client client.Reader,
version.GetVersion(),
credsManager,
metadata.EnabledCRDKindsConfig{
DatadogAgentEnabled: options.datadogAgentEnabled,
DatadogAgentInternalEnabled: options.datadogAgentInternalEnabled,
DatadogAgentEnabled: options.datadogAgentEnabled,
// Since v1.27, DDAI is always tied to DDA — no separate flag. Kept in telemetry for metric continuity.
DatadogAgentInternalEnabled: options.datadogAgentEnabled,
DatadogAgentProfileEnabled: options.datadogAgentProfileEnabled,
},
)
Expand Down
22 changes: 7 additions & 15 deletions internal/controller/datadogagent/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (

"github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1"
componentagent "github.com/DataDog/datadog-operator/internal/controller/datadogagent/component/agent"
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/feature"
"github.com/DataDog/datadog-operator/pkg/controller/utils/datadog"
"github.com/DataDog/datadog-operator/pkg/kubernetes"

Expand Down Expand Up @@ -70,14 +69,13 @@ const (

// ReconcilerOptions provides options read from command line
type ReconcilerOptions struct {
ExtendedDaemonsetOptions componentagent.ExtendedDaemonsetOptions
SupportCilium bool
OperatorMetricsEnabled bool
IntrospectionEnabled bool
DatadogAgentProfileEnabled bool
DatadogAgentInternalEnabled bool
DatadogCSIDriverEnabled bool
CreateControllerRevisions bool
ExtendedDaemonsetOptions componentagent.ExtendedDaemonsetOptions
SupportCilium bool
OperatorMetricsEnabled bool
IntrospectionEnabled bool
DatadogAgentProfileEnabled bool
DatadogCSIDriverEnabled bool
CreateControllerRevisions bool
// ExperimentTimeout overrides ExperimentDefaultTimeout. Zero means use the default.
ExperimentTimeout time.Duration
}
Expand Down Expand Up @@ -134,12 +132,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, dda *v2alpha1.DatadogAgent)
return resp, err
}

func reconcilerOptionsToFeatureOptions(opts *ReconcilerOptions, logger logr.Logger) *feature.Options {
return &feature.Options{
Logger: logger,
}
}

// metricsForwarderProcessError convert the reconciler errors into metrics if metrics forwarder is enabled
func (r *Reconciler) metricsForwarderProcessError(dda *v2alpha1.DatadogAgent, err error) {
if r.options.OperatorMetricsEnabled {
Expand Down
Loading
Loading