Merge pull request #121611 from atiratree/test-gated-controllers

controller descriptors should not be feature gated

cmd/kube-controller-manager/app/controllermanager.go

@@ -75,7 +75,6 @@ import (
 	"k8s.io/kubernetes/cmd/kube-controller-manager/names"
 	kubectrlmgrconfig "k8s.io/kubernetes/pkg/controller/apis/config"
 	serviceaccountcontroller "k8s.io/kubernetes/pkg/controller/serviceaccount"
-	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/serviceaccount"
 )
 
@@ -557,10 +556,7 @@ func NewControllerDescriptors() map[string]*ControllerDescriptor {
 	register(newResourceClaimControllerDescriptor())
 	register(newLegacyServiceAccountTokenCleanerControllerDescriptor())
 	register(newValidatingAdmissionPolicyStatusControllerDescriptor())
-	if utilfeature.DefaultFeatureGate.Enabled(features.SeparateTaintEvictionController) {
+	register(newTaintEvictionControllerDescriptor())
-		// register the flag only if the SeparateTaintEvictionController flag is enabled
-		register(newTaintEvictionControllerDescriptor())
-	}
 
 	for _, alias := range aliases.UnsortedList() {
 		if _, ok := controllers[alias]; ok {

cmd/kube-controller-manager/app/controllermanager_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package app
 
 import (
+	"context"
 	"regexp"
 	"strings"
 	"testing"
@@ -28,9 +29,10 @@ import (
 	cpnames "k8s.io/cloud-provider/names"
 	"k8s.io/component-base/featuregate"
 	featuregatetesting "k8s.io/component-base/featuregate/testing"
+	controllermanagercontroller "k8s.io/controller-manager/controller"
+	"k8s.io/klog/v2/ktesting"
 	"k8s.io/kubernetes/cmd/kube-controller-manager/names"
 	"k8s.io/kubernetes/pkg/features"
-	"k8s.io/utils/strings/slices"
 )
 
 func TestControllerNamesConsistency(t *testing.T) {
@@ -105,6 +107,9 @@ func TestNewControllerDescriptorsShouldNotPanic(t *testing.T) {
 }
 
 func TestNewControllerDescriptorsAlwaysReturnsDescriptorsForAllControllers(t *testing.T) {
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, "AllAlpha", false)()
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, "AllBeta", false)()
+
 	controllersWithoutFeatureGates := KnownControllers()
 
 	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, "AllAlpha", true)()
@@ -159,16 +164,56 @@ func TestFeatureGatedControllersShouldNotDefineAliases(t *testing.T) {
 	}
 }
 
-// TestTaintEvictionControllerDeclaration ensures that it is possible to run taint-manager as a separated controller
+// TestTaintEvictionControllerGating ensures that it is possible to run taint-manager as a separated controller
 // only when the SeparateTaintEvictionController feature is enabled
-func TestTaintEvictionControllerDeclaration(t *testing.T) {
+func TestTaintEvictionControllerGating(t *testing.T) {
-	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SeparateTaintEvictionController, true)()
+	tests := []struct {
-	if !slices.Contains(KnownControllers(), names.TaintEvictionController) {
+		name               string
-		t.Errorf("TaintEvictionController should be a registered controller when the SeparateTaintEvictionController feature is enabled")
+		enableFeatureGate  bool
+		expectInitFuncCall bool
+	}{
+		{
+			name:               "standalone taint-eviction-controller should run when SeparateTaintEvictionController feature gate is enabled",
+			enableFeatureGate:  true,
+			expectInitFuncCall: true,
+		},
+		{
+			name:               "standalone taint-eviction-controller should not run when SeparateTaintEvictionController feature gate is not enabled",
+			enableFeatureGate:  false,
+			expectInitFuncCall: false,
+		},
 	}
 
-	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SeparateTaintEvictionController, false)()
+	for _, test := range tests {
-	if slices.Contains(KnownControllers(), names.TaintEvictionController) {
+		t.Run(test.name, func(t *testing.T) {
-		t.Errorf("TaintEvictionController should not be a registered controller when the SeparateTaintEvictionController feature is disabled")
+			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SeparateTaintEvictionController, test.enableFeatureGate)()
+			_, ctx := ktesting.NewTestContext(t)
+			ctx, cancel := context.WithCancel(ctx)
+			defer cancel()
+
+			controllerCtx := ControllerContext{}
+			controllerCtx.ComponentConfig.Generic.Controllers = []string{names.TaintEvictionController}
+
+			initFuncCalled := false
+
+			taintEvictionControllerDescriptor := NewControllerDescriptors()[names.TaintEvictionController]
+			taintEvictionControllerDescriptor.initFunc = func(ctx context.Context, controllerContext ControllerContext, controllerName string) (controller controllermanagercontroller.Interface, enabled bool, err error) {
+				initFuncCalled = true
+				return nil, true, nil
+			}
+
+			healthCheck, err := StartController(ctx, controllerCtx, taintEvictionControllerDescriptor, nil)
+			if err != nil {
+				t.Errorf("starting a TaintEvictionController controller should not return an error")
+			}
+			if test.expectInitFuncCall != initFuncCalled {
+				t.Errorf("TaintEvictionController init call check failed: expected=%v, got=%v", test.expectInitFuncCall, initFuncCalled)
+			}
+			hasHealthCheck := healthCheck != nil
+			expectHealthCheck := test.expectInitFuncCall
+			if expectHealthCheck != hasHealthCheck {
+				t.Errorf("TaintEvictionController healthCheck check failed: expected=%v, got=%v", expectHealthCheck, hasHealthCheck)
+			}
+		})
 	}
 }