Make unregisterForwarder idempotent and send to debug when forwarder already removed (#2479)

Author: tbavelier

pkg/controller/utils/datadog/forwarders_manager.go

@@ -166,7 +166,10 @@ func (f *ForwardersManager) unregisterForwarder(id string) error {
 	f.Lock()
 	defer f.Unlock()
 	if _, found := f.metricsForwarders[id]; !found {
-		return fmt.Errorf("%s not found", id)
+		// Idempotency: deletion/finalization paths can call Unregister multiple times due to requeues.
+		// If the forwarder is already removed, treat it as a success to avoid noisy error logs.
+		log.V(1).Info("Metrics forwarder already unregistered", "ID", id)
+		return nil
 	}
 	f.metricsForwarders[id].stop()
 	delete(f.metricsForwarders, id)

pkg/controller/utils/datadog/forwarders_manager_test.go

@@ -0,0 +1,31 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2016-present Datadog, Inc.
+
+package datadog
+
+import "testing"
+
+func TestForwardersManager_unregisterForwarder_Idempotent(t *testing.T) {
+	t.Parallel()
+
+	fm := &ForwardersManager{
+		metricsForwarders: make(map[string]*metricsForwarder),
+	}
+
+	id := "DatadogAgentInternal/foo/test"
+	fm.metricsForwarders[id] = &metricsForwarder{
+		stopChan: make(chan struct{}),
+	}
+
+	// First unregister removes it.
+	if err := fm.unregisterForwarder(id); err != nil {
+		t.Fatalf("expected no error when unregistering existing forwarder, got: %v", err)
+	}
+
+	// Second unregister should still be a no-op.
+	if err := fm.unregisterForwarder(id); err != nil {
+		t.Fatalf("expected no error when unregistering already-unregistered forwarder, got: %v", err)
+	}
+}