[chore] Fix featuregate usage in controller tests

We haven't been unsetting feature gates in controller tests after ending
the test, leading to them being enabled for the duration of the test
suite. In one case, a test actually depended on this fact, and I needed
to set the gate in it explicitly.

Also switched to use the gates explicitly vs parsing flags.

Author: swiatekm

controllers/builder_test.go

@@ -15,7 +15,6 @@
 package controllers
 
 import (
-	"strings"
 	"testing"
 
 	cmv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
@@ -1245,7 +1244,7 @@ service:
 		name         string
 		args         args
 		want         []client.Object
-		featuregates []string
+		featuregates []*colfeaturegate.Gate
 		wantErr      bool
 		opts         []config.Option
 	}{
@@ -2188,8 +2187,7 @@ prometheus_cr:
 					},
 				},
 			},
-			wantErr:      false,
-			featuregates: []string{},
+			wantErr: false,
 		},
 		{
 			name: "target allocator mtls enabled",
@@ -2827,7 +2825,7 @@ prometheus_cr:
 			opts: []config.Option{
 				config.WithCertManagerAvailability(certmanager.Available),
 			},
-			featuregates: []string{"operator.targetallocator.mtls"},
+			featuregates: []*colfeaturegate.Gate{featuregate.EnableTargetAllocatorMTLS},
 		},
 	}
 	for _, tt := range tests {
@@ -2848,13 +2846,18 @@ prometheus_cr:
 			targetAllocator, err := collector.TargetAllocator(params)
 			require.NoError(t, err)
 			params.TargetAllocator = targetAllocator
-			if len(tt.featuregates) > 0 {
-				fg := strings.Join(tt.featuregates, ",")
-				flagset := featuregate.Flags(colfeaturegate.GlobalRegistry())
-				if err = flagset.Set(featuregate.FeatureGatesFlag, fg); err != nil {
-					t.Errorf("featuregate setting error = %v", err)
+			registry := colfeaturegate.GlobalRegistry()
+			for _, gate := range tt.featuregates {
+				current := gate.IsEnabled()
+				require.False(t, current, "only enable gates which are disabled by default")
+				if setErr := registry.Set(gate.ID(), true); setErr != nil {
+					require.NoError(t, setErr)
 					return
 				}
+				t.Cleanup(func() {
+					setErr := registry.Set(gate.ID(), current)
+					require.NoError(t, setErr)
+				})
 			}
 			got, err := BuildCollector(params)
 			if (err != nil) != tt.wantErr {
@@ -2909,7 +2912,7 @@ service:
 		name         string
 		args         args
 		want         []client.Object
-		featuregates []string
+		featuregates []*colfeaturegate.Gate
 		wantErr      bool
 		opts         []config.Option
 	}{
@@ -3396,7 +3399,7 @@ service:
 				},
 			},
 			wantErr:      false,
-			featuregates: []string{},
+			featuregates: []*colfeaturegate.Gate{},
 		},
 	}
 	for _, tt := range tests {
@@ -3417,13 +3420,20 @@ service:
 			targetAllocator, err := collector.TargetAllocator(params)
 			require.NoError(t, err)
 			params.TargetAllocator = targetAllocator
-			featuregates := []string{"operator.collector.targetallocatorcr"}
+			featuregates := []*colfeaturegate.Gate{featuregate.CollectorUsesTargetAllocatorCR}
 			featuregates = append(featuregates, tt.featuregates...)
-			fg := strings.Join(featuregates, ",")
-			flagset := featuregate.Flags(colfeaturegate.GlobalRegistry())
-			if err = flagset.Set(featuregate.FeatureGatesFlag, fg); err != nil {
-				t.Errorf("featuregate setting error = %v", err)
-				return
+			registry := colfeaturegate.GlobalRegistry()
+			for _, gate := range featuregates {
+				current := gate.IsEnabled()
+				require.False(t, current, "only enable gates which are disabled by default")
+				if setErr := registry.Set(gate.ID(), true); setErr != nil {
+					require.NoError(t, setErr)
+					return
+				}
+				t.Cleanup(func() {
+					setErr := registry.Set(gate.ID(), current)
+					require.NoError(t, setErr)
+				})
 			}
 			got, err := BuildCollector(params)
 			if (err != nil) != tt.wantErr {
@@ -3445,7 +3455,7 @@ func TestBuildTargetAllocator(t *testing.T) {
 		name         string
 		args         args
 		want         []client.Object
-		featuregates []string
+		featuregates []*colfeaturegate.Gate
 		wantErr      bool
 		opts         []config.Option
 	}{
@@ -4019,8 +4029,7 @@ prometheus_cr:
 					},
 				},
 			},
-			wantErr:      false,
-			featuregates: []string{},
+			wantErr: false,
 		},
 		{
 			name: "collector present",
@@ -4776,7 +4785,7 @@ prometheus_cr:
 			opts: []config.Option{
 				config.WithCertManagerAvailability(certmanager.Available),
 			},
-			featuregates: []string{"operator.targetallocator.mtls"},
+			featuregates: []*colfeaturegate.Gate{featuregate.EnableTargetAllocatorMTLS},
 		},
 	}
 	for _, tt := range tests {
@@ -4795,13 +4804,18 @@ prometheus_cr:
 				TargetAllocator: tt.args.instance,
 				Collector:       tt.args.collector,
 			}
-			if len(tt.featuregates) > 0 {
-				fg := strings.Join(tt.featuregates, ",")
-				flagset := featuregate.Flags(colfeaturegate.GlobalRegistry())
-				if err := flagset.Set(featuregate.FeatureGatesFlag, fg); err != nil {
-					t.Errorf("featuregate setting error = %v", err)
+			registry := colfeaturegate.GlobalRegistry()
+			for _, gate := range tt.featuregates {
+				current := gate.IsEnabled()
+				require.False(t, current, "only enable gates which are disabled by default")
+				if err := registry.Set(gate.ID(), true); err != nil {
+					require.NoError(t, err)
 					return
 				}
+				t.Cleanup(func() {
+					err := registry.Set(gate.ID(), current)
+					require.NoError(t, err)
+				})
 			}
 			got, err := BuildTargetAllocator(params)
 			if (err != nil) != tt.wantErr {

controllers/reconcile_test.go

@@ -22,6 +22,7 @@ import (
 	routev1 "github.com/openshift/api/route/v1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	colfeaturegate "go.opentelemetry.io/collector/featuregate"
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	v1 "k8s.io/api/core/v1"
@@ -48,6 +49,7 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/internal/config"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
 	"github.com/open-telemetry/opentelemetry-operator/internal/naming"
+	"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
 )
 
 const (
@@ -74,6 +76,18 @@ var (
 type check[T any] func(t *testing.T, params T)
 
 func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) {
+	// enable the collector CR feature flag, as these tests assume it
+	// TODO: drop this after the flag is enabled by default
+	registry := colfeaturegate.GlobalRegistry()
+	current := featuregate.CollectorUsesTargetAllocatorCR.IsEnabled()
+	require.False(t, current, "don't set gates which are enabled by default")
+	err := registry.Set(featuregate.CollectorUsesTargetAllocatorCR.ID(), true)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		err := registry.Set(featuregate.CollectorUsesTargetAllocatorCR.ID(), current)
+		require.NoError(t, err)
+	})
+
 	addedMetadataDeployment := testCollectorWithMode("test-deployment", v1alpha1.ModeDeployment)
 	addedMetadataDeployment.Labels = map[string]string{
 		labelName: labelVal,