Skip to content

Commit 0344f29

Browse files
authored
Merge pull request kubernetes#125778 from haitch/haitao/controllermgr-emulatever
add emulated-version flag to kube-controller-manager to control the feature gate.
2 parents 5420b2f + 1d92758 commit 0344f29

File tree

3 files changed

+192
-32
lines changed

3 files changed

+192
-32
lines changed

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ package app
2222
import (
2323
"context"
2424
"fmt"
25-
"k8s.io/apimachinery/pkg/runtime/schema"
2625
"math/rand"
2726
"net/http"
2827
"os"
@@ -33,13 +32,15 @@ import (
3332

3433
"k8s.io/apimachinery/pkg/api/meta"
3534
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
35+
"k8s.io/apimachinery/pkg/runtime/schema"
3636
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
3737
"k8s.io/apimachinery/pkg/util/sets"
3838
"k8s.io/apimachinery/pkg/util/uuid"
3939
"k8s.io/apimachinery/pkg/util/wait"
4040
"k8s.io/apiserver/pkg/server/healthz"
4141
"k8s.io/apiserver/pkg/server/mux"
4242
utilfeature "k8s.io/apiserver/pkg/util/feature"
43+
utilversion "k8s.io/apiserver/pkg/util/version"
4344
cacheddiscovery "k8s.io/client-go/discovery/cached/memory"
4445
"k8s.io/client-go/informers"
4546
v1core "k8s.io/client-go/kubernetes/typed/core/v1"
@@ -104,6 +105,9 @@ const (
104105

105106
// NewControllerManagerCommand creates a *cobra.Command object with default parameters
106107
func NewControllerManagerCommand() *cobra.Command {
108+
_, _ = utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister(
109+
utilversion.DefaultKubeComponent, utilversion.DefaultBuildEffectiveVersion(), utilfeature.DefaultMutableFeatureGate)
110+
107111
s, err := options.NewKubeControllerManagerOptions()
108112
if err != nil {
109113
klog.Background().Error(err, "Unable to initialize command options")
@@ -125,7 +129,8 @@ controller, and serviceaccounts controller.`,
125129
// kube-controller-manager generically watches APIs (including deprecated ones),
126130
// and CI ensures it works properly against matching kube-apiserver versions.
127131
restclient.SetDefaultWarningHandler(restclient.NoWarnings{})
128-
return nil
132+
// makes sure feature gates are set before RunE.
133+
return s.ComponentGlobalsRegistry.Set()
129134
},
130135
RunE: func(cmd *cobra.Command, args []string) error {
131136
verflag.PrintAndExitIfRequested()
@@ -141,8 +146,10 @@ controller, and serviceaccounts controller.`,
141146
if err != nil {
142147
return err
143148
}
149+
144150
// add feature enablement metrics
145-
utilfeature.DefaultMutableFeatureGate.AddMetrics()
151+
fg := s.ComponentGlobalsRegistry.FeatureGateFor(utilversion.DefaultKubeComponent)
152+
fg.(featuregate.MutableFeatureGate).AddMetrics()
146153
return Run(context.Background(), c.Complete())
147154
},
148155
Args: func(cmd *cobra.Command, args []string) error {

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

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ import (
2323

2424
v1 "k8s.io/api/core/v1"
2525
utilerrors "k8s.io/apimachinery/pkg/util/errors"
26+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
2627
apiserveroptions "k8s.io/apiserver/pkg/server/options"
2728
utilfeature "k8s.io/apiserver/pkg/util/feature"
29+
utilversion "k8s.io/apiserver/pkg/util/version"
2830
clientset "k8s.io/client-go/kubernetes"
2931
clientgokubescheme "k8s.io/client-go/kubernetes/scheme"
3032
restclient "k8s.io/client-go/rest"
@@ -97,6 +99,9 @@ type KubeControllerManagerOptions struct {
9799

98100
Master string
99101
ShowHiddenMetricsForVersion string
102+
103+
// ComponentGlobalsRegistry is the registry where the effective versions and feature gates for all components are stored.
104+
ComponentGlobalsRegistry utilversion.ComponentGlobalsRegistry
100105
}
101106

102107
// NewKubeControllerManagerOptions creates a new KubeControllerManagerOptions with a default config.
@@ -106,6 +111,12 @@ func NewKubeControllerManagerOptions() (*KubeControllerManagerOptions, error) {
106111
return nil, err
107112
}
108113

114+
if utilversion.DefaultComponentGlobalsRegistry.EffectiveVersionFor(utilversion.DefaultKubeComponent) == nil {
115+
featureGate := utilfeature.DefaultMutableFeatureGate
116+
effectiveVersion := utilversion.DefaultKubeEffectiveVersion()
117+
utilruntime.Must(utilversion.DefaultComponentGlobalsRegistry.Register(utilversion.DefaultKubeComponent, effectiveVersion, featureGate))
118+
}
119+
109120
s := KubeControllerManagerOptions{
110121
Generic: cmoptions.NewGenericControllerManagerConfigurationOptions(&componentConfig.Generic),
111122
KubeCloudShared: cpoptions.NewKubeCloudSharedOptions(&componentConfig.KubeCloudShared),
@@ -190,11 +201,12 @@ func NewKubeControllerManagerOptions() (*KubeControllerManagerOptions, error) {
190201
ValidatingAdmissionPolicyStatusController: &ValidatingAdmissionPolicyStatusControllerOptions{
191202
&componentConfig.ValidatingAdmissionPolicyStatusController,
192203
},
193-
SecureServing: apiserveroptions.NewSecureServingOptions().WithLoopback(),
194-
Authentication: apiserveroptions.NewDelegatingAuthenticationOptions(),
195-
Authorization: apiserveroptions.NewDelegatingAuthorizationOptions(),
196-
Metrics: metrics.NewOptions(),
197-
Logs: logs.NewOptions(),
204+
SecureServing: apiserveroptions.NewSecureServingOptions().WithLoopback(),
205+
Authentication: apiserveroptions.NewDelegatingAuthenticationOptions(),
206+
Authorization: apiserveroptions.NewDelegatingAuthorizationOptions(),
207+
Metrics: metrics.NewOptions(),
208+
Logs: logs.NewOptions(),
209+
ComponentGlobalsRegistry: utilversion.DefaultComponentGlobalsRegistry,
198210
}
199211

200212
s.Authentication.RemoteKubeConfigFileOptional = true
@@ -273,13 +285,16 @@ func (s *KubeControllerManagerOptions) Flags(allControllers []string, disabledBy
273285
fs := fss.FlagSet("misc")
274286
fs.StringVar(&s.Master, "master", s.Master, "The address of the Kubernetes API server (overrides any value in kubeconfig).")
275287
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).")
276-
utilfeature.DefaultMutableFeatureGate.AddFlag(fss.FlagSet("generic"))
288+
s.ComponentGlobalsRegistry.AddFlags(fss.FlagSet("generic"))
277289

278290
return fss
279291
}
280292

281293
// ApplyTo fills up controller manager config with options.
282294
func (s *KubeControllerManagerOptions) ApplyTo(c *kubecontrollerconfig.Config, allControllers []string, disabledByDefaultControllers []string, controllerAliases map[string]string) error {
295+
if err := s.ComponentGlobalsRegistry.SetFallback(); err != nil {
296+
return err
297+
}
283298
if err := s.Generic.ApplyTo(&c.ComponentConfig.Generic, allControllers, disabledByDefaultControllers, controllerAliases); err != nil {
284299
return err
285300
}
@@ -385,6 +400,11 @@ func (s *KubeControllerManagerOptions) ApplyTo(c *kubecontrollerconfig.Config, a
385400
func (s *KubeControllerManagerOptions) Validate(allControllers []string, disabledByDefaultControllers []string, controllerAliases map[string]string) error {
386401
var errs []error
387402

403+
if err := s.ComponentGlobalsRegistry.SetFallback(); err != nil {
404+
errs = append(errs, err)
405+
}
406+
407+
errs = append(errs, s.ComponentGlobalsRegistry.Validate()...)
388408
errs = append(errs, s.Generic.Validate(allControllers, disabledByDefaultControllers, controllerAliases)...)
389409
errs = append(errs, s.KubeCloudShared.Validate()...)
390410
errs = append(errs, s.AttachDetachController.Validate()...)
@@ -417,6 +437,7 @@ func (s *KubeControllerManagerOptions) Validate(allControllers []string, disable
417437
errs = append(errs, s.Authentication.Validate()...)
418438
errs = append(errs, s.Authorization.Validate()...)
419439
errs = append(errs, s.Metrics.Validate()...)
440+
errs = append(errs, utilversion.ValidateKubeEffectiveVersion(s.ComponentGlobalsRegistry.EffectiveVersionFor(utilversion.DefaultKubeComponent)))
420441

421442
// TODO: validate component config, master and kubeconfig
422443

cmd/kube-controller-manager/app/options/options_test.go

Lines changed: 155 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,23 +27,29 @@ import (
2727
"github.com/google/go-cmp/cmp"
2828
"github.com/spf13/pflag"
2929

30-
eventv1 "k8s.io/api/events/v1"
3130
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3231
utilerrors "k8s.io/apimachinery/pkg/util/errors"
32+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
33+
"k8s.io/apimachinery/pkg/util/version"
34+
3335
"k8s.io/apiserver/pkg/apis/apiserver"
3436
apiserveroptions "k8s.io/apiserver/pkg/server/options"
35-
cpconfig "k8s.io/cloud-provider/config"
36-
serviceconfig "k8s.io/cloud-provider/controllers/service/config"
37-
cpoptions "k8s.io/cloud-provider/options"
37+
utilversion "k8s.io/apiserver/pkg/util/version"
38+
3839
componentbaseconfig "k8s.io/component-base/config"
40+
"k8s.io/component-base/featuregate"
3941
"k8s.io/component-base/logs"
4042
"k8s.io/component-base/metrics"
43+
44+
cpconfig "k8s.io/cloud-provider/config"
45+
serviceconfig "k8s.io/cloud-provider/controllers/service/config"
46+
cpoptions "k8s.io/cloud-provider/options"
47+
48+
eventv1 "k8s.io/api/events/v1"
49+
clientgofeaturegate "k8s.io/client-go/features"
4150
cmconfig "k8s.io/controller-manager/config"
4251
cmoptions "k8s.io/controller-manager/options"
4352
migration "k8s.io/controller-manager/pkg/leadermigration/options"
44-
netutils "k8s.io/utils/net"
45-
46-
clientgofeaturegate "k8s.io/client-go/features"
4753
kubecontrollerconfig "k8s.io/kubernetes/cmd/kube-controller-manager/app/config"
4854
kubectrlmgrconfig "k8s.io/kubernetes/pkg/controller/apis/config"
4955
csrsigningconfig "k8s.io/kubernetes/pkg/controller/certificates/signer/config"
@@ -70,6 +76,7 @@ import (
7076
attachdetachconfig "k8s.io/kubernetes/pkg/controller/volume/attachdetach/config"
7177
ephemeralvolumeconfig "k8s.io/kubernetes/pkg/controller/volume/ephemeral/config"
7278
persistentvolumeconfig "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/config"
79+
netutils "k8s.io/utils/net"
7380
)
7481

7582
var args = []string{
@@ -165,11 +172,7 @@ var args = []string{
165172
}
166173

167174
func TestAddFlags(t *testing.T) {
168-
fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError)
169-
s, _ := NewKubeControllerManagerOptions()
170-
for _, f := range s.Flags([]string{""}, []string{""}, nil).FlagSets {
171-
fs.AddFlagSet(f)
172-
}
175+
fs, s := setupControllerManagerFlagSet(t)
173176

174177
fs.Parse(args)
175178
// Sort GCIgnoredResources because it's built from a map, which means the
@@ -442,9 +445,10 @@ func TestAddFlags(t *testing.T) {
442445
AlwaysAllowPaths: []string{"/healthz", "/readyz", "/livez"}, // note: this does not match /healthz/ or /healthz/*
443446
AlwaysAllowGroups: []string{"system:masters"},
444447
},
445-
Master: "192.168.4.20",
446-
Metrics: &metrics.Options{},
447-
Logs: logs.NewOptions(),
448+
Master: "192.168.4.20",
449+
Metrics: &metrics.Options{},
450+
Logs: logs.NewOptions(),
451+
ComponentGlobalsRegistry: utilversion.DefaultComponentGlobalsRegistry,
448452
}
449453

450454
// Sort GCIgnoredResources because it's built from a map, which means the
@@ -457,12 +461,7 @@ func TestAddFlags(t *testing.T) {
457461
}
458462

459463
func TestApplyTo(t *testing.T) {
460-
fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError)
461-
s, _ := NewKubeControllerManagerOptions()
462-
// flag set to parse the args that are required to start the kube controller manager
463-
for _, f := range s.Flags([]string{""}, []string{""}, nil).FlagSets {
464-
fs.AddFlagSet(f)
465-
}
464+
fs, s := setupControllerManagerFlagSet(t)
466465

467466
fs.Parse(args)
468467
// Sort GCIgnoredResources because it's built from a map, which means the
@@ -657,6 +656,96 @@ func TestApplyTo(t *testing.T) {
657656
}
658657
}
659658

659+
func TestEmulatedVersion(t *testing.T) {
660+
var cleanupAndSetupFunc = func() featuregate.FeatureGate {
661+
componentGlobalsRegistry := utilversion.DefaultComponentGlobalsRegistry
662+
componentGlobalsRegistry.Reset() // make sure this test have a clean state
663+
t.Cleanup(func() {
664+
componentGlobalsRegistry.Reset() // make sure this test doesn't leak a dirty state
665+
})
666+
667+
verKube := utilversion.NewEffectiveVersion("1.32")
668+
fg := featuregate.NewVersionedFeatureGate(version.MustParse("1.32"))
669+
utilruntime.Must(fg.AddVersioned(map[featuregate.Feature]featuregate.VersionedSpecs{
670+
"kubeA": {
671+
{Version: version.MustParse("1.32"), Default: true, LockToDefault: true, PreRelease: featuregate.GA},
672+
{Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Beta},
673+
},
674+
"kubeB": {
675+
{Version: version.MustParse("1.31"), Default: false, PreRelease: featuregate.Alpha},
676+
},
677+
}))
678+
utilruntime.Must(componentGlobalsRegistry.Register(utilversion.DefaultKubeComponent, verKube, fg))
679+
return fg
680+
}
681+
682+
testcases := []struct {
683+
name string
684+
flags []string // not a good place to test flagParse error
685+
wantErr bool // this won't apply to flagParse, it only apply to KubeControllerManagerOptions.Validate
686+
errorSubString string
687+
wantFeaturesGates map[string]bool
688+
}{
689+
{
690+
name: "default feature gates at binary version",
691+
flags: []string{},
692+
wantErr: false,
693+
wantFeaturesGates: map[string]bool{"kubeA": true, "kubeB": false},
694+
},
695+
{
696+
name: "emulating version out of range",
697+
flags: []string{
698+
"--emulated-version=1.28",
699+
},
700+
wantErr: true,
701+
errorSubString: "emulation version 1.28 is not between",
702+
wantFeaturesGates: nil,
703+
},
704+
{
705+
name: "default feature gates at emulated version",
706+
flags: []string{
707+
"--emulated-version=1.31",
708+
},
709+
wantFeaturesGates: map[string]bool{"kubeA": false, "kubeB": false},
710+
},
711+
{
712+
name: "set feature gates at emulated version",
713+
flags: []string{
714+
"--emulated-version=1.31",
715+
"--feature-gates=kubeA=false,kubeB=true",
716+
},
717+
wantFeaturesGates: map[string]bool{"kubeA": false, "kubeB": true},
718+
},
719+
{
720+
name: "cannot set locked feature gate",
721+
flags: []string{
722+
"--emulated-version=1.32",
723+
"--feature-gates=kubeA=false,kubeB=true",
724+
},
725+
errorSubString: "cannot set feature gate kubeA to false, feature is locked to true",
726+
wantErr: true,
727+
},
728+
}
729+
730+
for _, tc := range testcases {
731+
t.Run(tc.name, func(t *testing.T) {
732+
fg := cleanupAndSetupFunc()
733+
734+
fs, s := setupControllerManagerFlagSet(t)
735+
err := fs.Parse(tc.flags)
736+
checkTestError(t, err, false, "")
737+
err = s.Validate([]string{""}, []string{""}, nil)
738+
checkTestError(t, err, tc.wantErr, tc.errorSubString)
739+
740+
for feature, expected := range tc.wantFeaturesGates {
741+
if fg.Enabled(featuregate.Feature(feature)) != expected {
742+
t.Errorf("expected %s to be %v", feature, expected)
743+
}
744+
}
745+
})
746+
}
747+
}
748+
660749
func TestValidateControllersOptions(t *testing.T) {
661750
testCases := []struct {
662751
name string
@@ -1334,7 +1423,11 @@ func TestWatchListClientFlagUsage(t *testing.T) {
13341423

13351424
func TestWatchListClientFlagChange(t *testing.T) {
13361425
fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError)
1337-
s, _ := NewKubeControllerManagerOptions()
1426+
s, err := NewKubeControllerManagerOptions()
1427+
if err != nil {
1428+
t.Fatal(fmt.Errorf("NewKubeControllerManagerOptions failed with %w", err))
1429+
}
1430+
13381431
for _, f := range s.Flags([]string{""}, []string{""}, nil).FlagSets {
13391432
fs.AddFlagSet(f)
13401433
}
@@ -1344,7 +1437,13 @@ func TestWatchListClientFlagChange(t *testing.T) {
13441437

13451438
args := []string{fmt.Sprintf("--feature-gates=%v=true", clientgofeaturegate.WatchListClient)}
13461439
if err := fs.Parse(args); err != nil {
1347-
t.Fatal(err)
1440+
t.Fatal(fmt.Errorf("FlatSet.Parse failed with %w", err))
1441+
}
1442+
1443+
// this is needed to Apply parsed flags to GlobalRegistry, so the DefaultFeatureGate values can be set from the flag
1444+
err = s.ComponentGlobalsRegistry.Set()
1445+
if err != nil {
1446+
t.Fatal(fmt.Errorf("ComponentGlobalsRegistry.Set failed with %w", err))
13481447
}
13491448

13501449
watchListClientValue := clientgofeaturegate.FeatureGates().Enabled(clientgofeaturegate.WatchListClient)
@@ -1373,6 +1472,39 @@ func assertWatchListCommandLineDefaultValue(t *testing.T, fs *pflag.FlagSet) {
13731472
}
13741473
}
13751474

1475+
func setupControllerManagerFlagSet(t *testing.T) (*pflag.FlagSet, *KubeControllerManagerOptions) {
1476+
fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError)
1477+
s, err := NewKubeControllerManagerOptions()
1478+
if err != nil {
1479+
t.Fatal(fmt.Errorf("NewKubeControllerManagerOptions failed with %w", err))
1480+
}
1481+
1482+
for _, f := range s.Flags([]string{""}, []string{""}, nil).FlagSets {
1483+
fs.AddFlagSet(f)
1484+
}
1485+
return fs, s
1486+
}
1487+
1488+
// caution: checkTestError use t.Fatal, to simplify caller handling.
1489+
// it also means it may break test code execution flow.
1490+
func checkTestError(t *testing.T, err error, expectingErr bool, expectedErrorSubString string) {
1491+
if !expectingErr {
1492+
if err != nil { // not expecting, but got error
1493+
t.Fatal(fmt.Errorf("expected no error, got %w", err))
1494+
}
1495+
return // not expecting, and no error
1496+
}
1497+
1498+
// from this point we do expecting error
1499+
if err == nil {
1500+
t.Fatal("expected error, got nil")
1501+
}
1502+
1503+
if expectedErrorSubString != "" && !strings.Contains(err.Error(), expectedErrorSubString) {
1504+
t.Fatalf("expected error to contain %q, but got %q", expectedErrorSubString, err.Error())
1505+
}
1506+
}
1507+
13761508
type sortedGCIgnoredResources []garbagecollectorconfig.GroupResource
13771509

13781510
func (r sortedGCIgnoredResources) Len() int {

0 commit comments

Comments
 (0)