Skip to content

Commit

Permalink
allow forwarder to be used by all controllers (#1514)
Browse files Browse the repository at this point in the history
* allow forwarder to be used by all controllers

* fix test and add test case

* fix names

---------

Co-authored-by: levan-m <[email protected]>
  • Loading branch information
celenechang and levan-m authored Nov 21, 2024
1 parent b70d772 commit 715c6f6
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 23 deletions.
4 changes: 2 additions & 2 deletions internal/controller/datadogagent/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ type Reconciler struct {

// NewReconciler returns a reconciler for DatadogAgent
func NewReconciler(options ReconcilerOptions, client client.Client, platformInfo kubernetes.PlatformInfo,
scheme *runtime.Scheme, log logr.Logger, recorder record.EventRecorder, metricForwarder datadog.MetricForwardersManager,
scheme *runtime.Scheme, log logr.Logger, recorder record.EventRecorder, metricForwardersMgr datadog.MetricForwardersManager,
) (*Reconciler, error) {
return &Reconciler{
options: options,
Expand All @@ -88,7 +88,7 @@ func NewReconciler(options ReconcilerOptions, client client.Client, platformInfo
scheme: scheme,
log: log,
recorder: recorder,
forwarders: metricForwarder,
forwarders: metricForwardersMgr,
}, nil
}

Expand Down
8 changes: 3 additions & 5 deletions internal/controller/datadogagent_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func (r *DatadogAgentReconciler) Reconcile(ctx context.Context, req ctrl.Request
}

// SetupWithManager creates a new DatadogAgent controller.
func (r *DatadogAgentReconciler) SetupWithManager(mgr ctrl.Manager) error {
func (r *DatadogAgentReconciler) SetupWithManager(mgr ctrl.Manager, metricForwardersMgr datadog.MetricForwardersManager) error {
builder := ctrl.NewControllerManagedBy(mgr).
Owns(&corev1.Secret{}).
Owns(&corev1.ConfigMap{}).
Expand Down Expand Up @@ -241,14 +241,12 @@ func (r *DatadogAgentReconciler) SetupWithManager(mgr ctrl.Manager) error {
builder = builder.Owns(policy)
}

var metricForwarder datadog.MetricForwardersManager
var builderOptions []ctrlbuilder.ForOption
if r.Options.OperatorMetricsEnabled {
metricForwarder = datadog.NewForwardersManager(r.Client, &r.PlatformInfo)
builderOptions = append(builderOptions, ctrlbuilder.WithPredicates(predicate.Funcs{
// On `DatadogAgent` object creation, we register a metrics forwarder for it.
CreateFunc: func(e event.CreateEvent) bool {
metricForwarder.Register(e.Object)
metricForwardersMgr.Register(e.Object)
return true
},
}))
Expand All @@ -258,7 +256,7 @@ func (r *DatadogAgentReconciler) SetupWithManager(mgr ctrl.Manager) error {
return err
}

internal, err := datadogagent.NewReconciler(r.Options, r.Client, r.PlatformInfo, r.Scheme, r.Log, r.Recorder, metricForwarder)
internal, err := datadogagent.NewReconciler(r.Options, r.Client, r.PlatformInfo, r.Scheme, r.Log, r.Recorder, metricForwardersMgr)
if err != nil {
return err
}
Expand Down
22 changes: 14 additions & 8 deletions internal/controller/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
componentagent "github.com/DataDog/datadog-operator/internal/controller/datadogagent/component/agent"

"github.com/DataDog/datadog-operator/pkg/config"
"github.com/DataDog/datadog-operator/pkg/controller/utils/datadog"
"github.com/DataDog/datadog-operator/pkg/datadogclient"
"github.com/DataDog/datadog-operator/pkg/kubernetes"
"github.com/DataDog/datadog-operator/pkg/utils"
Expand Down Expand Up @@ -65,7 +66,7 @@ type ExtendedDaemonsetOptions struct {
CanaryAutoPauseMaxSlowStartDuration time.Duration
}

type starterFunc func(logr.Logger, manager.Manager, kubernetes.PlatformInfo, SetupOptions) error
type starterFunc func(logr.Logger, manager.Manager, kubernetes.PlatformInfo, SetupOptions, datadog.MetricForwardersManager) error

var controllerStarters = map[string]starterFunc{
agentControllerName: startDatadogAgent,
Expand Down Expand Up @@ -104,8 +105,13 @@ func SetupControllers(logger logr.Logger, mgr manager.Manager, options SetupOpti
}
platformInfo := kubernetes.NewPlatformInfo(versionInfo, groups, resources)

var metricForwardersMgr datadog.MetricForwardersManager
if options.OperatorMetricsEnabled {
metricForwardersMgr = datadog.NewForwardersManager(mgr.GetClient(), &platformInfo)
}

for controller, starter := range controllerStarters {
if err := starter(logger, mgr, platformInfo, options); err != nil {
if err := starter(logger, mgr, platformInfo, options, metricForwardersMgr); err != nil {
logger.Error(err, "Couldn't start controller", "controller", controller)
}
}
Expand All @@ -124,7 +130,7 @@ func getServerGroupsAndResources(log logr.Logger, discoveryClient *discovery.Dis
return groups, resources, nil
}

func startDatadogAgent(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.PlatformInfo, options SetupOptions) error {
func startDatadogAgent(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.PlatformInfo, options SetupOptions, metricForwardersMgr datadog.MetricForwardersManager) error {
if !options.DatadogAgentEnabled {
logger.Info("Feature disabled, not starting the controller", "controller", agentControllerName)

Expand Down Expand Up @@ -156,10 +162,10 @@ func startDatadogAgent(logger logr.Logger, mgr manager.Manager, pInfo kubernetes
DatadogAgentProfileEnabled: options.DatadogAgentProfileEnabled,
OtelAgentEnabled: options.OtelAgentEnabled,
},
}).SetupWithManager(mgr)
}).SetupWithManager(mgr, metricForwardersMgr)
}

func startDatadogMonitor(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.PlatformInfo, options SetupOptions) error {
func startDatadogMonitor(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.PlatformInfo, options SetupOptions, metricForwardersMgr datadog.MetricForwardersManager) error {
if !options.DatadogMonitorEnabled {
logger.Info("Feature disabled, not starting the controller", "controller", monitorControllerName)

Expand All @@ -180,7 +186,7 @@ func startDatadogMonitor(logger logr.Logger, mgr manager.Manager, pInfo kubernet
}).SetupWithManager(mgr)
}

func startDatadogDashboard(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.PlatformInfo, options SetupOptions) error {
func startDatadogDashboard(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.PlatformInfo, options SetupOptions, metricForwardersMgr datadog.MetricForwardersManager) error {
if !options.DatadogDashboardEnabled {
logger.Info("Feature disabled, not starting the controller", "controller", dashboardControllerName)
return nil
Expand All @@ -200,7 +206,7 @@ func startDatadogDashboard(logger logr.Logger, mgr manager.Manager, pInfo kubern
}).SetupWithManager(mgr)
}

func startDatadogSLO(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.PlatformInfo, options SetupOptions) error {
func startDatadogSLO(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.PlatformInfo, options SetupOptions, metricForwardersMgr datadog.MetricForwardersManager) error {
if !options.DatadogSLOEnabled {
logger.Info("Feature disabled, not starting the controller", "controller", sloControllerName)
return nil
Expand All @@ -222,7 +228,7 @@ func startDatadogSLO(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.P
return controller.SetupWithManager(mgr)
}

func startDatadogAgentProfiles(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.PlatformInfo, options SetupOptions) error {
func startDatadogAgentProfiles(logger logr.Logger, mgr manager.Manager, pInfo kubernetes.PlatformInfo, options SetupOptions, metricForwardersMgr datadog.MetricForwardersManager) error {
if !options.DatadogAgentProfileEnabled {
logger.Info("Feature disabled, not starting the controller", "controller", profileControllerName)
return nil
Expand Down
5 changes: 3 additions & 2 deletions pkg/controller/utils/datadog/forwarders_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ type ForwardersManager struct {
k8sClient client.Client
platformInfo *kubernetes.PlatformInfo
forwarders map[string]*metricsForwarder
decryptor secrets.Decryptor
wg sync.WaitGroup
// TODO expand this to include a metadataForwarder
decryptor secrets.Decryptor
wg sync.WaitGroup
sync.Mutex
}

Expand Down
15 changes: 12 additions & 3 deletions pkg/controller/utils/datadog/metrics_forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -563,17 +563,26 @@ func (mf *metricsForwarder) getDatadogAgent() (*v2alpha1.DatadogAgent, error) {
return dda, err
}

// getCredentials retrieves the api key configured in the DatadogAgent
// getCredentials retrieves the API key configured in the Operator or the DatadogAgent
func (mf *metricsForwarder) getCredentials(dda *v2alpha1.DatadogAgent) (string, error) {

// Check Operator configured credentials first
if mf.credsManager != nil {
if creds, err := mf.credsManager.GetCredentials(); err == nil {
return creds.APIKey, nil
}
}

// Check DatadogAgent credentials
if dda.Spec.Global == nil || dda.Spec.Global.Credentials == nil {
return "", fmt.Errorf("credentials not configured in the DatadogAgent")
}

defaultSecretName := v2alpha1.GetDefaultCredentialsSecretName(dda)

var err error
apiKey := ""

defaultSecretName := v2alpha1.GetDefaultCredentialsSecretName(dda)

if dda.Spec.Global != nil && dda.Spec.Global.Credentials != nil && dda.Spec.Global.Credentials.APIKey != nil && *dda.Spec.Global.Credentials.APIKey != "" {
apiKey = *dda.Spec.Global.Credentials.APIKey
} else {
Expand Down
43 changes: 40 additions & 3 deletions pkg/controller/utils/datadog/metrics_forwarder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,18 @@ package datadog
import (
"context"
"errors"
"os"
"reflect"
"sort"
"strings"
"sync"
"testing"

apicommon "github.com/DataDog/datadog-operator/api/datadoghq/common"
datadoghqv2alpha1 "github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1"
test "github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1/test"
apiutils "github.com/DataDog/datadog-operator/api/utils"
"github.com/DataDog/datadog-operator/pkg/config"
"github.com/DataDog/datadog-operator/pkg/kubernetes"
"github.com/DataDog/datadog-operator/pkg/secrets"

Expand Down Expand Up @@ -170,6 +173,25 @@ func TestReconcileDatadogAgent_getCredentials(t *testing.T) {
wantErr bool
wantFunc func(*metricsForwarder, *secrets.DummyDecryptor) error
}{
{
name: "creds found in credential manager",
fields: fields{
client: fake.NewFakeClient(),
},
args: args{
dda: test.NewDatadogAgent("foo", "bar", &datadoghqv2alpha1.GlobalConfig{
Credentials: &datadoghqv2alpha1.DatadogCredentials{
APIKey: apiutils.NewStringPointer(apiKey),
},
}),
loadFunc: func(m *metricsForwarder, d *secrets.DummyDecryptor) {
os.Setenv(apicommon.DDAPIKey, "test123")
os.Setenv(apicommon.DDAppKey, "testabc")
},
},
wantAPIKey: "test123",
wantErr: false,
},
{
name: "creds found in CR",
fields: fields{
Expand All @@ -181,6 +203,10 @@ func TestReconcileDatadogAgent_getCredentials(t *testing.T) {
APIKey: apiutils.NewStringPointer(apiKey),
},
}),
loadFunc: func(m *metricsForwarder, d *secrets.DummyDecryptor) {
os.Unsetenv(apicommon.DDAPIKey)
os.Unsetenv(apicommon.DDAppKey)
},
},
wantAPIKey: "foundAPIKey",
wantErr: false,
Expand All @@ -200,6 +226,8 @@ func TestReconcileDatadogAgent_getCredentials(t *testing.T) {
},
}),
loadFunc: func(m *metricsForwarder, d *secrets.DummyDecryptor) {
os.Unsetenv(apicommon.DDAPIKey)
os.Unsetenv(apicommon.DDAppKey)
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "datadog-creds-api",
Expand Down Expand Up @@ -227,6 +255,8 @@ func TestReconcileDatadogAgent_getCredentials(t *testing.T) {
},
}),
loadFunc: func(m *metricsForwarder, d *secrets.DummyDecryptor) {
os.Unsetenv(apicommon.DDAPIKey)
os.Unsetenv(apicommon.DDAppKey)
m.cleanSecretsCache()
m.creds.Store(encAPIKey, "cachedAPIKey")
},
Expand All @@ -253,6 +283,8 @@ func TestReconcileDatadogAgent_getCredentials(t *testing.T) {
},
}),
loadFunc: func(m *metricsForwarder, d *secrets.DummyDecryptor) {
os.Unsetenv(apicommon.DDAPIKey)
os.Unsetenv(apicommon.DDAppKey)
m.cleanSecretsCache()
d.On("Decrypt", []string{encAPIKey}).Once()
},
Expand All @@ -275,6 +307,10 @@ func TestReconcileDatadogAgent_getCredentials(t *testing.T) {
},
args: args{
dda: test.NewDatadogAgent("foo", "bar", &datadoghqv2alpha1.GlobalConfig{}),
loadFunc: func(m *metricsForwarder, d *secrets.DummyDecryptor) {
os.Unsetenv(apicommon.DDAPIKey)
os.Unsetenv(apicommon.DDAppKey)
},
},
wantErr: true,
},
Expand All @@ -283,9 +319,10 @@ func TestReconcileDatadogAgent_getCredentials(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
d := &secrets.DummyDecryptor{}
mf := &metricsForwarder{
k8sClient: tt.fields.client,
decryptor: d,
creds: sync.Map{},
k8sClient: tt.fields.client,
decryptor: d,
creds: sync.Map{},
credsManager: config.NewCredentialManager(),
}
if tt.args.loadFunc != nil {
tt.args.loadFunc(mf, d)
Expand Down

0 comments on commit 715c6f6

Please sign in to comment.