From 3b4c7bddb3f946bfe92c1d8576585559ca1541c9 Mon Sep 17 00:00:00 2001 From: Indeed Date: Mon, 1 Mar 2021 10:29:44 -0800 Subject: [PATCH] implement options parsing and defaults for leader migration. --- api/api-rules/violation_exceptions.list | 2 + .../k8s.io/controller-manager/config/types.go | 4 + .../config/v1alpha1/types.go | 4 + .../v1alpha1/zz_generated.conversion.go | 8 ++ .../config/v1alpha1/zz_generated.deepcopy.go | 1 + .../config/zz_generated.deepcopy.go | 1 + .../controller-manager/options/generic.go | 9 +- .../pkg/features/kube_features.go | 13 +- .../pkg/leadermigration/feature.go | 28 ++++ .../pkg/leadermigration/options/options.go | 88 ++++++++++++ .../leadermigration/options/options_test.go | 130 ++++++++++++++++++ vendor/modules.txt | 3 + 12 files changed, 287 insertions(+), 4 deletions(-) create mode 100644 staging/src/k8s.io/controller-manager/pkg/leadermigration/feature.go create mode 100644 staging/src/k8s.io/controller-manager/pkg/leadermigration/options/options.go create mode 100644 staging/src/k8s.io/controller-manager/pkg/leadermigration/options/options_test.go diff --git a/api/api-rules/violation_exceptions.list b/api/api-rules/violation_exceptions.list index 8e44b017620..3b89f73ac3a 100644 --- a/api/api-rules/violation_exceptions.list +++ b/api/api-rules/violation_exceptions.list @@ -500,6 +500,8 @@ API rule violation: names_match,k8s.io/controller-manager/config/v1alpha1,Generi API rule violation: names_match,k8s.io/controller-manager/config/v1alpha1,GenericControllerManagerConfiguration,Controllers API rule violation: names_match,k8s.io/controller-manager/config/v1alpha1,GenericControllerManagerConfiguration,Debugging API rule violation: names_match,k8s.io/controller-manager/config/v1alpha1,GenericControllerManagerConfiguration,LeaderElection +API rule violation: names_match,k8s.io/controller-manager/config/v1alpha1,GenericControllerManagerConfiguration,LeaderMigration +API rule violation: names_match,k8s.io/controller-manager/config/v1alpha1,GenericControllerManagerConfiguration,LeaderMigrationEnabled API rule violation: names_match,k8s.io/controller-manager/config/v1alpha1,GenericControllerManagerConfiguration,MinResyncPeriod API rule violation: names_match,k8s.io/controller-manager/config/v1alpha1,GenericControllerManagerConfiguration,Port API rule violation: names_match,k8s.io/kube-controller-manager/config/v1alpha1,AttachDetachControllerConfiguration,DisableAttachDetachReconcilerSync diff --git a/staging/src/k8s.io/controller-manager/config/types.go b/staging/src/k8s.io/controller-manager/config/types.go index dbd5460494c..1d95422d911 100644 --- a/staging/src/k8s.io/controller-manager/config/types.go +++ b/staging/src/k8s.io/controller-manager/config/types.go @@ -46,6 +46,10 @@ type GenericControllerManagerConfiguration struct { Controllers []string // DebuggingConfiguration holds configuration for Debugging related features. Debugging componentbaseconfig.DebuggingConfiguration + // LeaderMigrationEnabled indicates whether Leader Migration should be enabled for the controller manager. + LeaderMigrationEnabled bool + // LeaderMigration holds the configuration for Leader Migration. + LeaderMigration LeaderMigrationConfiguration } // LeaderMigrationConfiguration provides versioned configuration for all migrating leader locks. diff --git a/staging/src/k8s.io/controller-manager/config/v1alpha1/types.go b/staging/src/k8s.io/controller-manager/config/v1alpha1/types.go index 1924ab8a859..84d3c1b82ff 100644 --- a/staging/src/k8s.io/controller-manager/config/v1alpha1/types.go +++ b/staging/src/k8s.io/controller-manager/config/v1alpha1/types.go @@ -45,6 +45,10 @@ type GenericControllerManagerConfiguration struct { Controllers []string // DebuggingConfiguration holds configuration for Debugging related features. Debugging componentbaseconfigv1alpha1.DebuggingConfiguration + // LeaderMigrationEnabled indicates whether Leader Migration should be enabled for the controller manager. + LeaderMigrationEnabled bool + // LeaderMigration holds the configuration for Leader Migration. + LeaderMigration LeaderMigrationConfiguration } // LeaderMigrationConfiguration provides versioned configuration for all migrating leader locks. diff --git a/staging/src/k8s.io/controller-manager/config/v1alpha1/zz_generated.conversion.go b/staging/src/k8s.io/controller-manager/config/v1alpha1/zz_generated.conversion.go index 1f3894ca370..47945ccfca6 100644 --- a/staging/src/k8s.io/controller-manager/config/v1alpha1/zz_generated.conversion.go +++ b/staging/src/k8s.io/controller-manager/config/v1alpha1/zz_generated.conversion.go @@ -106,6 +106,10 @@ func autoConvert_v1alpha1_GenericControllerManagerConfiguration_To_config_Generi if err := configv1alpha1.Convert_v1alpha1_DebuggingConfiguration_To_config_DebuggingConfiguration(&in.Debugging, &out.Debugging, s); err != nil { return err } + out.LeaderMigrationEnabled = in.LeaderMigrationEnabled + if err := Convert_v1alpha1_LeaderMigrationConfiguration_To_config_LeaderMigrationConfiguration(&in.LeaderMigration, &out.LeaderMigration, s); err != nil { + return err + } return nil } @@ -124,6 +128,10 @@ func autoConvert_config_GenericControllerManagerConfiguration_To_v1alpha1_Generi if err := configv1alpha1.Convert_config_DebuggingConfiguration_To_v1alpha1_DebuggingConfiguration(&in.Debugging, &out.Debugging, s); err != nil { return err } + out.LeaderMigrationEnabled = in.LeaderMigrationEnabled + if err := Convert_config_LeaderMigrationConfiguration_To_v1alpha1_LeaderMigrationConfiguration(&in.LeaderMigration, &out.LeaderMigration, s); err != nil { + return err + } return nil } diff --git a/staging/src/k8s.io/controller-manager/config/v1alpha1/zz_generated.deepcopy.go b/staging/src/k8s.io/controller-manager/config/v1alpha1/zz_generated.deepcopy.go index 4bbdcb467a9..60363b32f6d 100644 --- a/staging/src/k8s.io/controller-manager/config/v1alpha1/zz_generated.deepcopy.go +++ b/staging/src/k8s.io/controller-manager/config/v1alpha1/zz_generated.deepcopy.go @@ -53,6 +53,7 @@ func (in *GenericControllerManagerConfiguration) DeepCopyInto(out *GenericContro copy(*out, *in) } in.Debugging.DeepCopyInto(&out.Debugging) + in.LeaderMigration.DeepCopyInto(&out.LeaderMigration) return } diff --git a/staging/src/k8s.io/controller-manager/config/zz_generated.deepcopy.go b/staging/src/k8s.io/controller-manager/config/zz_generated.deepcopy.go index 023d4c80437..ea24e6b6520 100644 --- a/staging/src/k8s.io/controller-manager/config/zz_generated.deepcopy.go +++ b/staging/src/k8s.io/controller-manager/config/zz_generated.deepcopy.go @@ -53,6 +53,7 @@ func (in *GenericControllerManagerConfiguration) DeepCopyInto(out *GenericContro copy(*out, *in) } out.Debugging = in.Debugging + in.LeaderMigration.DeepCopyInto(&out.LeaderMigration) return } diff --git a/staging/src/k8s.io/controller-manager/options/generic.go b/staging/src/k8s.io/controller-manager/options/generic.go index b3999dd4940..a22369d63b5 100644 --- a/staging/src/k8s.io/controller-manager/options/generic.go +++ b/staging/src/k8s.io/controller-manager/options/generic.go @@ -24,12 +24,15 @@ import ( cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/config/options" cmconfig "k8s.io/controller-manager/config" + migration "k8s.io/controller-manager/pkg/leadermigration/options" ) // GenericControllerManagerConfigurationOptions holds the options which are generic. type GenericControllerManagerConfigurationOptions struct { *cmconfig.GenericControllerManagerConfiguration Debugging *DebuggingOptions + // LeaderMigration is the options for leader migration, a nil indicates default options should be applied. + LeaderMigration *migration.LeaderMigrationOptions } // NewGenericControllerManagerConfigurationOptions returns generic configuration default values for both @@ -39,6 +42,7 @@ func NewGenericControllerManagerConfigurationOptions(cfg *cmconfig.GenericContro o := &GenericControllerManagerConfigurationOptions{ GenericControllerManagerConfiguration: cfg, Debugging: RecommendedDebuggingOptions(), + LeaderMigration: nil, } return o @@ -51,6 +55,7 @@ func (o *GenericControllerManagerConfigurationOptions) AddFlags(fss *cliflag.Nam } o.Debugging.AddFlags(fss.FlagSet("debugging")) + o.LeaderMigration.AddFlags(fss.FlagSet("leader-migration")) genericfs := fss.FlagSet("generic") genericfs.DurationVar(&o.MinResyncPeriod.Duration, "min-resync-period", o.MinResyncPeriod.Duration, "The resync period in reflectors will be random between MinResyncPeriod and 2*MinResyncPeriod.") genericfs.StringVar(&o.ClientConnection.ContentType, "kube-api-content-type", o.ClientConnection.ContentType, "Content type of requests sent to apiserver.") @@ -74,7 +79,9 @@ func (o *GenericControllerManagerConfigurationOptions) ApplyTo(cfg *cmconfig.Gen if err := o.Debugging.ApplyTo(&cfg.Debugging); err != nil { return err } - + if err := o.LeaderMigration.ApplyTo(cfg); err != nil { + return err + } cfg.Port = o.Port cfg.Address = o.Address cfg.MinResyncPeriod = o.MinResyncPeriod diff --git a/staging/src/k8s.io/controller-manager/pkg/features/kube_features.go b/staging/src/k8s.io/controller-manager/pkg/features/kube_features.go index ae6e82aebef..04deab8c947 100644 --- a/staging/src/k8s.io/controller-manager/pkg/features/kube_features.go +++ b/staging/src/k8s.io/controller-manager/pkg/features/kube_features.go @@ -50,6 +50,12 @@ const ( // Enables ipv6 dual stack // Original copy from k8s.io/kubernetes/pkg/features/kube_features.go IPv6DualStack featuregate.Feature = "IPv6DualStack" + + // owner: @jiahuif + // alpha: v1.21 + // + // Enables Leader Migration for kube-controller-manager and cloud-controller-manager + ControllerManagerLeaderMigration featuregate.Feature = "ControllerManagerLeaderMigration" ) func SetupCurrentKubernetesSpecificFeatureGates(featuregates featuregate.MutableFeatureGate) error { @@ -59,7 +65,8 @@ func SetupCurrentKubernetesSpecificFeatureGates(featuregates featuregate.Mutable // cloudPublicFeatureGates consists of cloud-specific feature keys. // To add a new feature, define a key for it at k8s.io/api/pkg/features and add it here. var cloudPublicFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ - LegacyNodeRoleBehavior: {Default: false, PreRelease: featuregate.GA, LockToDefault: true}, - ServiceNodeExclusion: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, - IPv6DualStack: {Default: true, PreRelease: featuregate.Beta}, + LegacyNodeRoleBehavior: {Default: false, PreRelease: featuregate.GA, LockToDefault: true}, + ServiceNodeExclusion: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, + IPv6DualStack: {Default: true, PreRelease: featuregate.Beta}, + ControllerManagerLeaderMigration: {Default: false, PreRelease: featuregate.Alpha}, } diff --git a/staging/src/k8s.io/controller-manager/pkg/leadermigration/feature.go b/staging/src/k8s.io/controller-manager/pkg/leadermigration/feature.go new file mode 100644 index 00000000000..fec07edb155 --- /dev/null +++ b/staging/src/k8s.io/controller-manager/pkg/leadermigration/feature.go @@ -0,0 +1,28 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package leadermigration + +import ( + "k8s.io/apiserver/pkg/util/feature" + "k8s.io/controller-manager/pkg/features" + _ "k8s.io/controller-manager/pkg/features/register" +) + +// FeatureEnabled tells if leader migration is enabled through the feature gate. +func FeatureEnabled() bool { + return feature.DefaultMutableFeatureGate.Enabled(features.ControllerManagerLeaderMigration) +} diff --git a/staging/src/k8s.io/controller-manager/pkg/leadermigration/options/options.go b/staging/src/k8s.io/controller-manager/pkg/leadermigration/options/options.go new file mode 100644 index 00000000000..cc8eb83bd4d --- /dev/null +++ b/staging/src/k8s.io/controller-manager/pkg/leadermigration/options/options.go @@ -0,0 +1,88 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package options + +import ( + "fmt" + + "github.com/spf13/pflag" + "k8s.io/controller-manager/config" + "k8s.io/controller-manager/pkg/leadermigration" + migrationconfig "k8s.io/controller-manager/pkg/leadermigration/config" +) + +// LeaderMigrationOptions is the set of options for Leader Migration, +// which is given to the controller manager through flags +type LeaderMigrationOptions struct { + // Enabled indicates whether leader migration is enabled through the --enabled-leader-migration flag. + Enabled bool + + // ControllerMigrationConfig is the path to the file of LeaderMigrationConfiguration type. + // It can be set with --leader-migration-config flag + // If the path is "" (default vaule), the default vaule will be used. + ControllerMigrationConfig string +} + +// DefaultLeaderMigrationOptions returns a LeaderMigrationOptions with default values. +func DefaultLeaderMigrationOptions() *LeaderMigrationOptions { + return &LeaderMigrationOptions{ + Enabled: false, + ControllerMigrationConfig: "", + } +} + +// AddFlags adds all flags related to leader migration to given flag set. +func (o *LeaderMigrationOptions) AddFlags(fs *pflag.FlagSet) { + if o == nil { + return + } + fs.BoolVar(&o.Enabled, "enable-leader-migration", false, "Whether to enable controller leader migration.") + fs.StringVar(&o.ControllerMigrationConfig, "leader-migration-config", "", + "Path to the config file for controller leader migration, "+ + "or empty to use the value that reflects default configuration of the controller manager. "+ + "The config file should be of type LeaderMigrationConfiguration, group controllermanager.config.k8s.io, version v1alpha1.") +} + +// ApplyTo applies the options of leader migration to generic configuration. +func (o *LeaderMigrationOptions) ApplyTo(cfg *config.GenericControllerManagerConfiguration) error { + if o == nil { + // an nil LeaderMigrationOptions indicates that default options should be used + // in which case leader migration will be disabled + cfg.LeaderMigrationEnabled = false + return nil + } + if o.Enabled && !leadermigration.FeatureEnabled() { + return fmt.Errorf("Leader Migration is not enabled through feature gate") + } + cfg.LeaderMigrationEnabled = o.Enabled + if !cfg.LeaderMigrationEnabled { + return nil + } + if o.ControllerMigrationConfig == "" { + return fmt.Errorf("--leader-migration-config is required") + } + leaderMigrationConfig, err := migrationconfig.ReadLeaderMigrationConfiguration(o.ControllerMigrationConfig) + if err != nil { + return err + } + errs := migrationconfig.ValidateLeaderMigrationConfiguration(leaderMigrationConfig) + if len(errs) != 0 { + return fmt.Errorf("failed to parse leader migration configuration: %v", errs) + } + cfg.LeaderMigration = *leaderMigrationConfig + return nil +} diff --git a/staging/src/k8s.io/controller-manager/pkg/leadermigration/options/options_test.go b/staging/src/k8s.io/controller-manager/pkg/leadermigration/options/options_test.go new file mode 100644 index 00000000000..8d668896749 --- /dev/null +++ b/staging/src/k8s.io/controller-manager/pkg/leadermigration/options/options_test.go @@ -0,0 +1,130 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package options + +import ( + "io/ioutil" + "os" + "reflect" + "testing" + + "github.com/spf13/pflag" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/controller-manager/config" + "k8s.io/controller-manager/pkg/features" +) + +func TestLeaderMigrationOptions(t *testing.T) { + testCases := []struct { + name string + flags []string + configContent string + expectEnabled bool + expectErr bool + enableFeatureGate bool + expectConfig *config.LeaderMigrationConfiguration + }{ + { + name: "default (disabled), with feature gate disabled", + flags: []string{}, + enableFeatureGate: false, + expectEnabled: false, + expectErr: false, + }, + { + name: "enabled, with feature gate disabled", + flags: []string{"--enable-leader-migration"}, + enableFeatureGate: false, + expectErr: true, + }, + { + name: "enabled, but missing configuration file", + flags: []string{"--enable-leader-migration"}, + enableFeatureGate: true, + expectEnabled: true, + expectErr: true, + }, + { + name: "enabled, with custom configuration file", + flags: []string{"--enable-leader-migration"}, + enableFeatureGate: true, + expectEnabled: true, + configContent: ` +apiVersion: controllermanager.config.k8s.io/v1alpha1 +kind: LeaderMigrationConfiguration +leaderName: test-leader-migration +resourceLock: leases +controllerLeaders: [] +`, + expectErr: false, + expectConfig: &config.LeaderMigrationConfiguration{ + LeaderName: "test-leader-migration", + ResourceLock: "leases", + ControllerLeaders: []config.ControllerLeaderConfiguration{}, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ControllerManagerLeaderMigration, tc.enableFeatureGate)() + flags := tc.flags + if tc.configContent != "" { + configFile, err := ioutil.TempFile("", tc.name) + if err != nil { + t.Fatal(err) + } + defer os.Remove(configFile.Name()) + err = ioutil.WriteFile(configFile.Name(), []byte(tc.configContent), os.FileMode(0755)) + if err != nil { + t.Fatal(err) + } + flags = append(flags, "--leader-migration-config="+configFile.Name()) + } + genericConfig := new(config.GenericControllerManagerConfiguration) + options := new(LeaderMigrationOptions) + fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError) + options.AddFlags(fs) + err := fs.Parse(flags) + if err != nil { + t.Errorf("cannot parse leader-migration-config: %v", err) + return + } + err = options.ApplyTo(genericConfig) + if err != nil { + if !tc.expectErr { + t.Errorf("unexpected error: %v", err) + return + } + // expect err and got err, finish the test case. + return + } + if err == nil && tc.expectErr { + t.Errorf("expected error but got nil") + return + } + if genericConfig.LeaderMigrationEnabled != tc.expectEnabled { + t.Errorf("expected Enabled=%v, got %v", tc.expectEnabled, options.Enabled) + return + } + if tc.expectEnabled && !reflect.DeepEqual(tc.expectConfig, &genericConfig.LeaderMigration) { + t.Errorf("expected config %#v but got %#v", tc.expectConfig, genericConfig.LeaderMigration) + } + }) + } + +} diff --git a/vendor/modules.txt b/vendor/modules.txt index a4fa1590d82..1b999fbe954 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2255,6 +2255,9 @@ k8s.io/controller-manager/pkg/clientbuilder k8s.io/controller-manager/pkg/features k8s.io/controller-manager/pkg/features/register k8s.io/controller-manager/pkg/informerfactory +k8s.io/controller-manager/pkg/leadermigration +k8s.io/controller-manager/pkg/leadermigration/config +k8s.io/controller-manager/pkg/leadermigration/options # k8s.io/cri-api v0.0.0 => ./staging/src/k8s.io/cri-api ## explicit # k8s.io/cri-api => ./staging/src/k8s.io/cri-api