diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index 179231488b3..8885f9f9f1c 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/cmd/kube-controller-manager/app/controllermanager.go @@ -22,7 +22,6 @@ package app import ( "context" "fmt" - "k8s.io/apimachinery/pkg/runtime/schema" "math/rand" "net/http" "os" @@ -33,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/uuid" @@ -40,6 +40,7 @@ import ( "k8s.io/apiserver/pkg/server/healthz" "k8s.io/apiserver/pkg/server/mux" utilfeature "k8s.io/apiserver/pkg/util/feature" + utilversion "k8s.io/apiserver/pkg/util/version" cacheddiscovery "k8s.io/client-go/discovery/cached/memory" "k8s.io/client-go/informers" v1core "k8s.io/client-go/kubernetes/typed/core/v1" @@ -104,6 +105,9 @@ const ( // NewControllerManagerCommand creates a *cobra.Command object with default parameters func NewControllerManagerCommand() *cobra.Command { + _, _ = utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister( + utilversion.DefaultKubeComponent, utilversion.DefaultBuildEffectiveVersion(), utilfeature.DefaultMutableFeatureGate) + s, err := options.NewKubeControllerManagerOptions() if err != nil { klog.Background().Error(err, "Unable to initialize command options") @@ -125,7 +129,8 @@ controller, and serviceaccounts controller.`, // kube-controller-manager generically watches APIs (including deprecated ones), // and CI ensures it works properly against matching kube-apiserver versions. restclient.SetDefaultWarningHandler(restclient.NoWarnings{}) - return nil + // makes sure feature gates are set before RunE. + return s.ComponentGlobalsRegistry.Set() }, RunE: func(cmd *cobra.Command, args []string) error { verflag.PrintAndExitIfRequested() @@ -141,8 +146,10 @@ controller, and serviceaccounts controller.`, if err != nil { return err } + // add feature enablement metrics - utilfeature.DefaultMutableFeatureGate.AddMetrics() + fg := s.ComponentGlobalsRegistry.FeatureGateFor(utilversion.DefaultKubeComponent) + fg.(featuregate.MutableFeatureGate).AddMetrics() return Run(context.Background(), c.Complete()) }, Args: func(cmd *cobra.Command, args []string) error { diff --git a/cmd/kube-controller-manager/app/options/options.go b/cmd/kube-controller-manager/app/options/options.go index 1c3e4edb178..ffc24bdb23e 100644 --- a/cmd/kube-controller-manager/app/options/options.go +++ b/cmd/kube-controller-manager/app/options/options.go @@ -23,8 +23,10 @@ import ( v1 "k8s.io/api/core/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" apiserveroptions "k8s.io/apiserver/pkg/server/options" utilfeature "k8s.io/apiserver/pkg/util/feature" + utilversion "k8s.io/apiserver/pkg/util/version" clientset "k8s.io/client-go/kubernetes" clientgokubescheme "k8s.io/client-go/kubernetes/scheme" restclient "k8s.io/client-go/rest" @@ -97,6 +99,9 @@ type KubeControllerManagerOptions struct { Master string ShowHiddenMetricsForVersion string + + // ComponentGlobalsRegistry is the registry where the effective versions and feature gates for all components are stored. + ComponentGlobalsRegistry utilversion.ComponentGlobalsRegistry } // NewKubeControllerManagerOptions creates a new KubeControllerManagerOptions with a default config. @@ -106,6 +111,12 @@ func NewKubeControllerManagerOptions() (*KubeControllerManagerOptions, error) { return nil, err } + if utilversion.DefaultComponentGlobalsRegistry.EffectiveVersionFor(utilversion.DefaultKubeComponent) == nil { + featureGate := utilfeature.DefaultMutableFeatureGate + effectiveVersion := utilversion.DefaultKubeEffectiveVersion() + utilruntime.Must(utilversion.DefaultComponentGlobalsRegistry.Register(utilversion.DefaultKubeComponent, effectiveVersion, featureGate)) + } + s := KubeControllerManagerOptions{ Generic: cmoptions.NewGenericControllerManagerConfigurationOptions(&componentConfig.Generic), KubeCloudShared: cpoptions.NewKubeCloudSharedOptions(&componentConfig.KubeCloudShared), @@ -190,11 +201,12 @@ func NewKubeControllerManagerOptions() (*KubeControllerManagerOptions, error) { ValidatingAdmissionPolicyStatusController: &ValidatingAdmissionPolicyStatusControllerOptions{ &componentConfig.ValidatingAdmissionPolicyStatusController, }, - SecureServing: apiserveroptions.NewSecureServingOptions().WithLoopback(), - Authentication: apiserveroptions.NewDelegatingAuthenticationOptions(), - Authorization: apiserveroptions.NewDelegatingAuthorizationOptions(), - Metrics: metrics.NewOptions(), - Logs: logs.NewOptions(), + SecureServing: apiserveroptions.NewSecureServingOptions().WithLoopback(), + Authentication: apiserveroptions.NewDelegatingAuthenticationOptions(), + Authorization: apiserveroptions.NewDelegatingAuthorizationOptions(), + Metrics: metrics.NewOptions(), + Logs: logs.NewOptions(), + ComponentGlobalsRegistry: utilversion.DefaultComponentGlobalsRegistry, } s.Authentication.RemoteKubeConfigFileOptional = true @@ -273,13 +285,16 @@ func (s *KubeControllerManagerOptions) Flags(allControllers []string, disabledBy fs := fss.FlagSet("misc") fs.StringVar(&s.Master, "master", s.Master, "The address of the Kubernetes API server (overrides any value in kubeconfig).") fs.StringVar(&s.Generic.ClientConnection.Kubeconfig, "kubeconfig", s.Generic.ClientConnection.Kubeconfig, "Path to kubeconfig file with authorization and master location information (the master location can be overridden by the master flag).") - utilfeature.DefaultMutableFeatureGate.AddFlag(fss.FlagSet("generic")) + s.ComponentGlobalsRegistry.AddFlags(fss.FlagSet("generic")) return fss } // ApplyTo fills up controller manager config with options. func (s *KubeControllerManagerOptions) ApplyTo(c *kubecontrollerconfig.Config, allControllers []string, disabledByDefaultControllers []string, controllerAliases map[string]string) error { + if err := s.ComponentGlobalsRegistry.SetFallback(); err != nil { + return err + } if err := s.Generic.ApplyTo(&c.ComponentConfig.Generic, allControllers, disabledByDefaultControllers, controllerAliases); err != nil { return err } @@ -385,6 +400,11 @@ func (s *KubeControllerManagerOptions) ApplyTo(c *kubecontrollerconfig.Config, a func (s *KubeControllerManagerOptions) Validate(allControllers []string, disabledByDefaultControllers []string, controllerAliases map[string]string) error { var errs []error + if err := s.ComponentGlobalsRegistry.SetFallback(); err != nil { + errs = append(errs, err) + } + + errs = append(errs, s.ComponentGlobalsRegistry.Validate()...) errs = append(errs, s.Generic.Validate(allControllers, disabledByDefaultControllers, controllerAliases)...) errs = append(errs, s.KubeCloudShared.Validate()...) errs = append(errs, s.AttachDetachController.Validate()...) @@ -417,6 +437,7 @@ func (s *KubeControllerManagerOptions) Validate(allControllers []string, disable errs = append(errs, s.Authentication.Validate()...) errs = append(errs, s.Authorization.Validate()...) errs = append(errs, s.Metrics.Validate()...) + errs = append(errs, utilversion.ValidateKubeEffectiveVersion(s.ComponentGlobalsRegistry.EffectiveVersionFor(utilversion.DefaultKubeComponent))) // TODO: validate component config, master and kubeconfig diff --git a/cmd/kube-controller-manager/app/options/options_test.go b/cmd/kube-controller-manager/app/options/options_test.go index c1b81c6026e..be9f0c12082 100644 --- a/cmd/kube-controller-manager/app/options/options_test.go +++ b/cmd/kube-controller-manager/app/options/options_test.go @@ -27,23 +27,29 @@ import ( "github.com/google/go-cmp/cmp" "github.com/spf13/pflag" - eventv1 "k8s.io/api/events/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/version" + "k8s.io/apiserver/pkg/apis/apiserver" apiserveroptions "k8s.io/apiserver/pkg/server/options" + utilversion "k8s.io/apiserver/pkg/util/version" + + componentbaseconfig "k8s.io/component-base/config" + "k8s.io/component-base/featuregate" + "k8s.io/component-base/logs" + "k8s.io/component-base/metrics" + cpconfig "k8s.io/cloud-provider/config" serviceconfig "k8s.io/cloud-provider/controllers/service/config" cpoptions "k8s.io/cloud-provider/options" - componentbaseconfig "k8s.io/component-base/config" - "k8s.io/component-base/logs" - "k8s.io/component-base/metrics" + + eventv1 "k8s.io/api/events/v1" + clientgofeaturegate "k8s.io/client-go/features" cmconfig "k8s.io/controller-manager/config" cmoptions "k8s.io/controller-manager/options" migration "k8s.io/controller-manager/pkg/leadermigration/options" - netutils "k8s.io/utils/net" - - clientgofeaturegate "k8s.io/client-go/features" kubecontrollerconfig "k8s.io/kubernetes/cmd/kube-controller-manager/app/config" kubectrlmgrconfig "k8s.io/kubernetes/pkg/controller/apis/config" csrsigningconfig "k8s.io/kubernetes/pkg/controller/certificates/signer/config" @@ -70,6 +76,7 @@ import ( attachdetachconfig "k8s.io/kubernetes/pkg/controller/volume/attachdetach/config" ephemeralvolumeconfig "k8s.io/kubernetes/pkg/controller/volume/ephemeral/config" persistentvolumeconfig "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/config" + netutils "k8s.io/utils/net" ) var args = []string{ @@ -165,11 +172,7 @@ var args = []string{ } func TestAddFlags(t *testing.T) { - fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError) - s, _ := NewKubeControllerManagerOptions() - for _, f := range s.Flags([]string{""}, []string{""}, nil).FlagSets { - fs.AddFlagSet(f) - } + fs, s := setupControllerManagerFlagSet(t) fs.Parse(args) // Sort GCIgnoredResources because it's built from a map, which means the @@ -442,9 +445,10 @@ func TestAddFlags(t *testing.T) { AlwaysAllowPaths: []string{"/healthz", "/readyz", "/livez"}, // note: this does not match /healthz/ or /healthz/* AlwaysAllowGroups: []string{"system:masters"}, }, - Master: "192.168.4.20", - Metrics: &metrics.Options{}, - Logs: logs.NewOptions(), + Master: "192.168.4.20", + Metrics: &metrics.Options{}, + Logs: logs.NewOptions(), + ComponentGlobalsRegistry: utilversion.DefaultComponentGlobalsRegistry, } // Sort GCIgnoredResources because it's built from a map, which means the @@ -457,12 +461,7 @@ func TestAddFlags(t *testing.T) { } func TestApplyTo(t *testing.T) { - fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError) - s, _ := NewKubeControllerManagerOptions() - // flag set to parse the args that are required to start the kube controller manager - for _, f := range s.Flags([]string{""}, []string{""}, nil).FlagSets { - fs.AddFlagSet(f) - } + fs, s := setupControllerManagerFlagSet(t) fs.Parse(args) // Sort GCIgnoredResources because it's built from a map, which means the @@ -657,6 +656,96 @@ func TestApplyTo(t *testing.T) { } } +func TestEmulatedVersion(t *testing.T) { + var cleanupAndSetupFunc = func() featuregate.FeatureGate { + componentGlobalsRegistry := utilversion.DefaultComponentGlobalsRegistry + componentGlobalsRegistry.Reset() // make sure this test have a clean state + t.Cleanup(func() { + componentGlobalsRegistry.Reset() // make sure this test doesn't leak a dirty state + }) + + verKube := utilversion.NewEffectiveVersion("1.32") + fg := featuregate.NewVersionedFeatureGate(version.MustParse("1.32")) + utilruntime.Must(fg.AddVersioned(map[featuregate.Feature]featuregate.VersionedSpecs{ + "kubeA": { + {Version: version.MustParse("1.32"), Default: true, LockToDefault: true, PreRelease: featuregate.GA}, + {Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Beta}, + }, + "kubeB": { + {Version: version.MustParse("1.31"), Default: false, PreRelease: featuregate.Alpha}, + }, + })) + utilruntime.Must(componentGlobalsRegistry.Register(utilversion.DefaultKubeComponent, verKube, fg)) + return fg + } + + testcases := []struct { + name string + flags []string // not a good place to test flagParse error + wantErr bool // this won't apply to flagParse, it only apply to KubeControllerManagerOptions.Validate + errorSubString string + wantFeaturesGates map[string]bool + }{ + { + name: "default feature gates at binary version", + flags: []string{}, + wantErr: false, + wantFeaturesGates: map[string]bool{"kubeA": true, "kubeB": false}, + }, + { + name: "emulating version out of range", + flags: []string{ + "--emulated-version=1.28", + }, + wantErr: true, + errorSubString: "emulation version 1.28 is not between", + wantFeaturesGates: nil, + }, + { + name: "default feature gates at emulated version", + flags: []string{ + "--emulated-version=1.31", + }, + wantFeaturesGates: map[string]bool{"kubeA": false, "kubeB": false}, + }, + { + name: "set feature gates at emulated version", + flags: []string{ + "--emulated-version=1.31", + "--feature-gates=kubeA=false,kubeB=true", + }, + wantFeaturesGates: map[string]bool{"kubeA": false, "kubeB": true}, + }, + { + name: "cannot set locked feature gate", + flags: []string{ + "--emulated-version=1.32", + "--feature-gates=kubeA=false,kubeB=true", + }, + errorSubString: "cannot set feature gate kubeA to false, feature is locked to true", + wantErr: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + fg := cleanupAndSetupFunc() + + fs, s := setupControllerManagerFlagSet(t) + err := fs.Parse(tc.flags) + checkTestError(t, err, false, "") + err = s.Validate([]string{""}, []string{""}, nil) + checkTestError(t, err, tc.wantErr, tc.errorSubString) + + for feature, expected := range tc.wantFeaturesGates { + if fg.Enabled(featuregate.Feature(feature)) != expected { + t.Errorf("expected %s to be %v", feature, expected) + } + } + }) + } +} + func TestValidateControllersOptions(t *testing.T) { testCases := []struct { name string @@ -1334,7 +1423,11 @@ func TestWatchListClientFlagUsage(t *testing.T) { func TestWatchListClientFlagChange(t *testing.T) { fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError) - s, _ := NewKubeControllerManagerOptions() + s, err := NewKubeControllerManagerOptions() + if err != nil { + t.Fatal(fmt.Errorf("NewKubeControllerManagerOptions failed with %w", err)) + } + for _, f := range s.Flags([]string{""}, []string{""}, nil).FlagSets { fs.AddFlagSet(f) } @@ -1344,7 +1437,13 @@ func TestWatchListClientFlagChange(t *testing.T) { args := []string{fmt.Sprintf("--feature-gates=%v=true", clientgofeaturegate.WatchListClient)} if err := fs.Parse(args); err != nil { - t.Fatal(err) + t.Fatal(fmt.Errorf("FlatSet.Parse failed with %w", err)) + } + + // this is needed to Apply parsed flags to GlobalRegistry, so the DefaultFeatureGate values can be set from the flag + err = s.ComponentGlobalsRegistry.Set() + if err != nil { + t.Fatal(fmt.Errorf("ComponentGlobalsRegistry.Set failed with %w", err)) } watchListClientValue := clientgofeaturegate.FeatureGates().Enabled(clientgofeaturegate.WatchListClient) @@ -1373,6 +1472,39 @@ func assertWatchListCommandLineDefaultValue(t *testing.T, fs *pflag.FlagSet) { } } +func setupControllerManagerFlagSet(t *testing.T) (*pflag.FlagSet, *KubeControllerManagerOptions) { + fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError) + s, err := NewKubeControllerManagerOptions() + if err != nil { + t.Fatal(fmt.Errorf("NewKubeControllerManagerOptions failed with %w", err)) + } + + for _, f := range s.Flags([]string{""}, []string{""}, nil).FlagSets { + fs.AddFlagSet(f) + } + return fs, s +} + +// caution: checkTestError use t.Fatal, to simplify caller handling. +// it also means it may break test code execution flow. +func checkTestError(t *testing.T, err error, expectingErr bool, expectedErrorSubString string) { + if !expectingErr { + if err != nil { // not expecting, but got error + t.Fatal(fmt.Errorf("expected no error, got %w", err)) + } + return // not expecting, and no error + } + + // from this point we do expecting error + if err == nil { + t.Fatal("expected error, got nil") + } + + if expectedErrorSubString != "" && !strings.Contains(err.Error(), expectedErrorSubString) { + t.Fatalf("expected error to contain %q, but got %q", expectedErrorSubString, err.Error()) + } +} + type sortedGCIgnoredResources []garbagecollectorconfig.GroupResource func (r sortedGCIgnoredResources) Len() int {