add unit tests for NewControllerDescriptors

- controller descriptors should not be feature gated
- aliases should not be defined for new controllers and have only a
  canonical name
This commit is contained in:
Filip Křepinský
2023-09-19 13:31:02 +02:00
parent 44cac26667
commit 1591a0e132
2 changed files with 62 additions and 2 deletions

View File

@@ -21,8 +21,13 @@ import (
"strings"
"testing"
"github.com/google/go-cmp/cmp"
"k8s.io/apimachinery/pkg/util/sets"
utilfeature "k8s.io/apiserver/pkg/util/feature"
cpnames "k8s.io/cloud-provider/names"
"k8s.io/component-base/featuregate"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/cmd/kube-controller-manager/names"
)
@@ -96,3 +101,58 @@ func TestControllerNamesDeclaration(t *testing.T) {
func TestNewControllerDescriptorsShouldNotPanic(t *testing.T) {
NewControllerDescriptors()
}
func TestNewControllerDescriptorsAlwaysReturnsDescriptorsForAllControllers(t *testing.T) {
controllersWithoutFeatureGates := KnownControllers()
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, "AllAlpha", true)()
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, "AllBeta", true)()
controllersWithFeatureGates := KnownControllers()
if diff := cmp.Diff(controllersWithoutFeatureGates, controllersWithFeatureGates); diff != "" {
t.Errorf("unexpected controllers after enabling feature gates, NewControllerDescriptors should always return all controller descriptors. Controllers should define required feature gates in ControllerDescriptor.requiredFeatureGates. Diff of returned controllers:\n%s", diff)
}
}
func TestFeatureGatedControllersShouldNotDefineAliases(t *testing.T) {
featureGateRegex := regexp.MustCompile("^([a-zA-Z0-9]+)")
alphaFeatures := sets.NewString()
for _, featureText := range utilfeature.DefaultFeatureGate.KnownFeatures() {
// we have to parse this from KnownFeatures, because usage of mutable FeatureGate is not allowed in unit tests
feature := featureGateRegex.FindString(featureText)
if strings.Contains(featureText, string(featuregate.Alpha)) && feature != "AllAlpha" {
alphaFeatures.Insert(feature)
}
}
for name, controller := range NewControllerDescriptors() {
if len(controller.GetAliases()) == 0 {
continue
}
requiredFeatureGates := controller.GetRequiredFeatureGates()
if len(requiredFeatureGates) == 0 {
continue
}
// DO NOT ADD any new controllers here. These two controllers are an exception, because they were added before this test was introduced
if name == names.LegacyServiceAccountTokenCleanerController || name == names.ResourceClaimController {
continue
}
areAllRequiredFeaturesAlpha := true
for _, feature := range requiredFeatureGates {
if !alphaFeatures.Has(string(feature)) {
areAllRequiredFeaturesAlpha = false
break
}
}
if areAllRequiredFeaturesAlpha {
t.Errorf("alias check failed: controller name %q should not be aliased as it is still guarded by alpha feature gates (%v) and thus should have only a canonical name", name, requiredFeatureGates)
}
}
}

View File

@@ -427,7 +427,7 @@ func newResourceClaimControllerDescriptor() *ControllerDescriptor {
aliases: []string{"resource-claim-controller"},
initFunc: startResourceClaimController,
requiredFeatureGates: []featuregate.Feature{
features.DynamicResourceAllocation,
features.DynamicResourceAllocation, // TODO update app.TestFeatureGatedControllersShouldNotDefineAliases when removing this feature
},
}
}
@@ -738,7 +738,7 @@ func newLegacyServiceAccountTokenCleanerControllerDescriptor() *ControllerDescri
aliases: []string{"legacy-service-account-token-cleaner"},
initFunc: startLegacyServiceAccountTokenCleanerController,
requiredFeatureGates: []featuregate.Feature{
features.LegacyServiceAccountTokenCleanUp,
features.LegacyServiceAccountTokenCleanUp, // TODO update app.TestFeatureGatedControllersShouldNotDefineAliases when removing this feature
},
}
}