introduce CCM controller aliases and unify controller names

This commit is contained in:
Filip Křepinský 2023-02-15 15:52:05 +01:00
parent 94792d85de
commit 9fd8f568fe
9 changed files with 212 additions and 34 deletions

View File

@ -31,12 +31,14 @@ import (
cloudprovider "k8s.io/cloud-provider"
"k8s.io/cloud-provider/app"
cloudcontrollerconfig "k8s.io/cloud-provider/app/config"
"k8s.io/cloud-provider/names"
"k8s.io/cloud-provider/options"
"k8s.io/component-base/cli"
cliflag "k8s.io/component-base/cli/flag"
_ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins
_ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration
"k8s.io/klog/v2"
kcmnames "k8s.io/kubernetes/cmd/kube-controller-manager/names"
// For existing cloud providers, the option to import legacy providers is still available.
// e.g. _"k8s.io/legacy-cloud-providers/<provider>"
)
@ -48,6 +50,7 @@ func main() {
}
controllerInitializers := app.DefaultInitFuncConstructors
controllerAliases := names.CCMControllerAliases()
// Here is an example to remove the controller which is not needed.
// e.g. remove the cloud-node-lifecycle controller which current cloud provider does not need.
//delete(controllerInitializers, "cloud-node-lifecycle")
@ -61,7 +64,7 @@ func main() {
fss := cliflag.NamedFlagSets{}
nodeIpamController.nodeIPAMControllerOptions.AddFlags(fss.FlagSet("nodeipam controller"))
controllerInitializers["nodeipam"] = app.ControllerInitFuncConstructor{
controllerInitializers[kcmnames.NodeIpamController] = app.ControllerInitFuncConstructor{
// "node-controller" is the shared identity of all node controllers, including node, node lifecycle, and node ipam.
// See https://github.com/kubernetes/kubernetes/pull/72764#issuecomment-453300990 for more context.
InitContext: app.ControllerInitContext{
@ -69,8 +72,9 @@ func main() {
},
Constructor: nodeIpamController.StartNodeIpamControllerWrapper,
}
controllerAliases["nodeipam"] = kcmnames.NodeIpamController
command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers, fss, wait.NeverStop)
command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers, controllerAliases, fss, wait.NeverStop)
code := cli.Run(command)
os.Exit(code)
}

View File

@ -22,6 +22,7 @@ import (
"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/cloud-provider/names"
"k8s.io/cloud-provider/options"
cliflag "k8s.io/component-base/cli/flag"
"k8s.io/component-base/cli/globalflag"
@ -32,6 +33,7 @@ import (
type CommandBuilder struct {
webhookConfigs map[string]WebhookConfig
controllerInitFuncConstructors map[string]ControllerInitFuncConstructor
controllerAliases map[string]string
additionalFlags cliflag.NamedFlagSets
options *options.CloudControllerManagerOptions
cloudInitializer InitCloudFunc
@ -45,6 +47,7 @@ func NewBuilder() *CommandBuilder {
cb := CommandBuilder{}
cb.webhookConfigs = make(map[string]WebhookConfig)
cb.controllerInitFuncConstructors = make(map[string]ControllerInitFuncConstructor)
cb.controllerAliases = make(map[string]string)
return &cb
}
@ -56,14 +59,22 @@ func (cb *CommandBuilder) AddFlags(additionalFlags cliflag.NamedFlagSets) {
cb.additionalFlags = additionalFlags
}
func (cb *CommandBuilder) RegisterController(name string, constructor ControllerInitFuncConstructor) {
func (cb *CommandBuilder) RegisterController(name string, constructor ControllerInitFuncConstructor, aliases map[string]string) {
cb.controllerInitFuncConstructors[name] = constructor
for key, val := range aliases {
if name == val {
cb.controllerAliases[key] = val
}
}
}
func (cb *CommandBuilder) RegisterDefaultControllers() {
for key, val := range DefaultInitFuncConstructors {
cb.controllerInitFuncConstructors[key] = val
}
for key, val := range names.CCMControllerAliases() {
cb.controllerAliases[key] = val
}
}
func (cb *CommandBuilder) RegisterWebhook(name string, config WebhookConfig) {
@ -129,7 +140,7 @@ func (cb *CommandBuilder) BuildCommand() *cobra.Command {
cliflag.PrintFlags(cmd.Flags())
config, err := cb.options.Config(ControllerNames(cb.controllerInitFuncConstructors), ControllersDisabledByDefault.List(),
WebhookNames(cb.webhookConfigs), WebhooksDisabledByDefault.List())
cb.controllerAliases, WebhookNames(cb.webhookConfigs), WebhooksDisabledByDefault.List())
if err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
return err
@ -156,7 +167,8 @@ func (cb *CommandBuilder) BuildCommand() *cobra.Command {
}
fs := cmd.Flags()
namedFlagSets := cb.options.Flags(ControllerNames(cb.controllerInitFuncConstructors), ControllersDisabledByDefault.List(), WebhookNames(cb.webhookConfigs), WebhooksDisabledByDefault.List())
namedFlagSets := cb.options.Flags(ControllerNames(cb.controllerInitFuncConstructors), ControllersDisabledByDefault.List(), cb.controllerAliases,
WebhookNames(cb.webhookConfigs), WebhooksDisabledByDefault.List())
verflag.AddFlags(namedFlagSets.FlagSet("global"))
globalflag.AddGlobalFlags(namedFlagSets.FlagSet("global"), cmd.Name())

View File

@ -43,6 +43,7 @@ import (
"k8s.io/client-go/tools/leaderelection/resourcelock"
cloudprovider "k8s.io/cloud-provider"
cloudcontrollerconfig "k8s.io/cloud-provider/app/config"
"k8s.io/cloud-provider/names"
"k8s.io/cloud-provider/options"
cliflag "k8s.io/component-base/cli/flag"
"k8s.io/component-base/cli/globalflag"
@ -80,7 +81,7 @@ const (
// NewCloudControllerManagerCommand creates a *cobra.Command object with default parameters
// controllerInitFuncConstructors is a map of controller name(as defined by controllers flag in https://kubernetes.io/docs/reference/command-line-tools-reference/kube-controller-manager/#options) to their InitFuncConstructor.
// additionalFlags provides controller specific flags to be included in the complete set of controller manager flags
func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, controllerInitFuncConstructors map[string]ControllerInitFuncConstructor, additionalFlags cliflag.NamedFlagSets, stopCh <-chan struct{}) *cobra.Command {
func NewCloudControllerManagerCommand(s *options.CloudControllerManagerOptions, cloudInitializer InitCloudFunc, controllerInitFuncConstructors map[string]ControllerInitFuncConstructor, controllerAliases map[string]string, additionalFlags cliflag.NamedFlagSets, stopCh <-chan struct{}) *cobra.Command {
logOptions := logs.NewOptions()
cmd := &cobra.Command{
@ -96,7 +97,7 @@ the cloud specific control loops shipped with Kubernetes.`,
return err
}
c, err := s.Config(ControllerNames(controllerInitFuncConstructors), ControllersDisabledByDefault.List(), AllWebhooks, DisabledByDefaultWebhooks)
c, err := s.Config(ControllerNames(controllerInitFuncConstructors), ControllersDisabledByDefault.List(), controllerAliases, AllWebhooks, DisabledByDefaultWebhooks)
if err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
return err
@ -123,7 +124,7 @@ the cloud specific control loops shipped with Kubernetes.`,
}
fs := cmd.Flags()
namedFlagSets := s.Flags(ControllerNames(controllerInitFuncConstructors), ControllersDisabledByDefault.List(), AllWebhooks, DisabledByDefaultWebhooks)
namedFlagSets := s.Flags(ControllerNames(controllerInitFuncConstructors), ControllersDisabledByDefault.List(), controllerAliases, AllWebhooks, DisabledByDefaultWebhooks)
globalFlagSet := namedFlagSets.FlagSet("global")
verflag.AddFlags(globalFlagSet)
@ -441,25 +442,25 @@ var DefaultInitFuncConstructors = map[string]ControllerInitFuncConstructor{
// The cloud-node controller shares the "node-controller" identity with the cloud-node-lifecycle
// controller for historical reasons. See
// https://github.com/kubernetes/kubernetes/pull/72764#issuecomment-453300990 for more context.
"cloud-node": {
names.CloudNodeController: {
InitContext: ControllerInitContext{
ClientName: "node-controller",
},
Constructor: StartCloudNodeControllerWrapper,
},
"cloud-node-lifecycle": {
names.CloudNodeLifecycleController: {
InitContext: ControllerInitContext{
ClientName: "node-controller",
},
Constructor: StartCloudNodeLifecycleControllerWrapper,
},
"service": {
names.ServiceLBController: {
InitContext: ControllerInitContext{
ClientName: "service-controller",
},
Constructor: StartServiceControllerWrapper,
},
"route": {
names.NodeRouteController: {
InitContext: ControllerInitContext{
ClientName: "route-controller",
},

View File

@ -0,0 +1,54 @@
/*
Copyright 2023 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 app
import (
"regexp"
"strings"
"testing"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/cloud-provider/names"
)
func TestCloudControllerNamesConsistency(t *testing.T) {
controllerNameRegexp := regexp.MustCompile("^[a-z]([-a-z]*[a-z])?$")
for name := range DefaultInitFuncConstructors {
if !controllerNameRegexp.MatchString(name) {
t.Errorf("name consistency check failed: controller %q must consist of lower case alphabetic characters or '-', and must start and end with an alphabetic character", name)
}
if !strings.HasSuffix(name, "-controller") {
t.Errorf("name consistency check failed: controller %q must have \"-controller\" suffix", name)
}
}
}
func TestCloudControllerNamesDeclaration(t *testing.T) {
declaredControllers := sets.New(
names.CloudNodeController,
names.ServiceLBController,
names.NodeRouteController,
names.CloudNodeLifecycleController,
)
for name := range DefaultInitFuncConstructors {
if !declaredControllers.Has(name) {
t.Errorf("name declaration check failed: controller name %q should be declared in \"controller_names.go\" and added to this test", name)
}
}
}

View File

@ -31,6 +31,7 @@ import (
cloudprovider "k8s.io/cloud-provider"
"k8s.io/cloud-provider/app"
"k8s.io/cloud-provider/app/config"
"k8s.io/cloud-provider/names"
"k8s.io/cloud-provider/options"
cliflag "k8s.io/component-base/cli/flag"
"k8s.io/klog/v2"
@ -107,7 +108,7 @@ func StartTestServer(ctx context.Context, customFlags []string) (result TestServ
return cloud
}
fss := cliflag.NamedFlagSets{}
command := app.NewCloudControllerManagerCommand(s, cloudInitializer, app.DefaultInitFuncConstructors, fss, stopCh)
command := app.NewCloudControllerManagerCommand(s, cloudInitializer, app.DefaultInitFuncConstructors, names.CCMControllerAliases(), fss, stopCh)
commandArgs := []string{}
listeners := []net.Listener{}

View File

@ -0,0 +1,69 @@
/*
Copyright 2023 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 names
// Canonical controller names
//
// NAMING CONVENTIONS
// 1. naming should be consistent across the controllers
// 2. use of shortcuts should be avoided, unless they are well-known non-Kubernetes shortcuts
// 3. Kubernetes' resources should be written together without a hyphen ("-")
//
// CHANGE POLICY
// The controller names should be treated as IDs.
// They can only be changed if absolutely necessary. For example if an inappropriate name was chosen in the past, or if the scope of the controller changes.
// When a name is changed, the old name should be aliased in CCMControllerAliases, while preserving all old aliases.
// This is done to achieve backwards compatibility
//
// USE CASES
// The following places should use the controller name constants, when:
// 1. registering a controller in app.DefaultInitFuncConstructors or sample main.controllerInitializers:
// 1.1. disabling a controller by default in app.ControllersDisabledByDefault
// 1.2. checking if IsControllerEnabled
// 1.3. defining an alias in CCMControllerAliases (for backwards compatibility only)
// 2. used anywhere inside the controller itself:
// 2.1. [TODO] logger component should be configured with the controller name by calling LoggerWithName
// 2.2. [TODO] logging should use a canonical controller name when referencing a controller (Eg. Starting X, Shutting down X)
// 2.3. [TODO] emitted events should have an EventSource.Component set to the controller name (usually when initializing an EventRecorder)
// 2.4. [TODO] registering ControllerManagerMetrics with ControllerStarted and ControllerStopped
// 2.5. [TODO] calling WaitForNamedCacheSync
// 3. defining controller options for "--help" command or generated documentation
// 3.1. controller name should be used to create a pflag.FlagSet when registering controller options (the name is rendered in a controller flag group header)
// 3.2. when defined flag's help mentions a controller name
// 4. defining a new service account for a new controller (old controllers may have inconsistent service accounts to stay backwards compatible)
// 5. anywhere these controllers are used outside of this module (kube-controller-manager, cloud-provider sample)
const (
CloudNodeController = "cloud-node-controller"
ServiceLBController = "service-lb-controller"
NodeRouteController = "node-route-controller"
CloudNodeLifecycleController = "cloud-node-lifecycle-controller"
)
// CCMControllerAliases returns a mapping of aliases to canonical controller names
//
// These aliases ensure backwards compatibility and should never be removed!
// Only addition of new aliases is allowed, and only when a canonical name is changed (please see CHANGE POLICY of controller names)
func CCMControllerAliases() map[string]string {
// return a new reference to achieve immutability of the mapping
return map[string]string{
"cloud-node": CloudNodeController,
"service": ServiceLBController,
"route": NodeRouteController,
"cloud-node-lifecycle": CloudNodeLifecycleController,
}
}

View File

@ -141,9 +141,9 @@ func NewDefaultComponentConfig() (*ccmconfig.CloudControllerManagerConfiguration
}
// Flags returns flags for a specific CloudController by section name
func (o *CloudControllerManagerOptions) Flags(allControllers, disabledByDefaultControllers, allWebhooks, disabledByDefaultWebhooks []string) cliflag.NamedFlagSets {
func (o *CloudControllerManagerOptions) Flags(allControllers []string, disabledByDefaultControllers []string, controllerAliases map[string]string, allWebhooks, disabledByDefaultWebhooks []string) cliflag.NamedFlagSets {
fss := cliflag.NamedFlagSets{}
o.Generic.AddFlags(&fss, allControllers, disabledByDefaultControllers)
o.Generic.AddFlags(&fss, allControllers, disabledByDefaultControllers, controllerAliases)
o.KubeCloudShared.AddFlags(fss.FlagSet("generic"))
o.NodeController.AddFlags(fss.FlagSet("node controller"))
o.ServiceController.AddFlags(fss.FlagSet("service controller"))
@ -168,7 +168,7 @@ func (o *CloudControllerManagerOptions) Flags(allControllers, disabledByDefaultC
}
// ApplyTo fills up cloud controller manager config with options.
func (o *CloudControllerManagerOptions) ApplyTo(c *config.Config, userAgent string) error {
func (o *CloudControllerManagerOptions) ApplyTo(c *config.Config, allControllers []string, disabledByDefaultControllers []string, controllerAliases map[string]string, userAgent string) error {
var err error
// Build kubeconfig first to so that if it fails, it doesn't cause leaking
@ -184,7 +184,7 @@ func (o *CloudControllerManagerOptions) ApplyTo(c *config.Config, userAgent stri
c.Kubeconfig.QPS = o.Generic.ClientConnection.QPS
c.Kubeconfig.Burst = int(o.Generic.ClientConnection.Burst)
if err = o.Generic.ApplyTo(&c.ComponentConfig.Generic); err != nil {
if err = o.Generic.ApplyTo(&c.ComponentConfig.Generic, allControllers, disabledByDefaultControllers, controllerAliases); err != nil {
return err
}
if err = o.KubeCloudShared.ApplyTo(&c.ComponentConfig.KubeCloudShared); err != nil {
@ -246,10 +246,10 @@ func (o *CloudControllerManagerOptions) ApplyTo(c *config.Config, userAgent stri
}
// Validate is used to validate config before launching the cloud controller manager
func (o *CloudControllerManagerOptions) Validate(allControllers, disabledByDefaultControllers, allWebhooks, disabledByDefaultWebhooks []string) error {
func (o *CloudControllerManagerOptions) Validate(allControllers []string, disabledByDefaultControllers []string, controllerAliases map[string]string, allWebhooks, disabledByDefaultWebhooks []string) error {
errors := []error{}
errors = append(errors, o.Generic.Validate(allControllers, disabledByDefaultControllers)...)
errors = append(errors, o.Generic.Validate(allControllers, disabledByDefaultControllers, controllerAliases)...)
errors = append(errors, o.KubeCloudShared.Validate()...)
errors = append(errors, o.ServiceController.Validate()...)
errors = append(errors, o.SecureServing.Validate()...)
@ -282,8 +282,8 @@ func resyncPeriod(c *config.Config) func() time.Duration {
}
// Config return a cloud controller manager config objective
func (o *CloudControllerManagerOptions) Config(allControllers, disabledByDefaultControllers, allWebhooks, disabledByDefaultWebhooks []string) (*config.Config, error) {
if err := o.Validate(allControllers, disabledByDefaultControllers, allWebhooks, disabledByDefaultWebhooks); err != nil {
func (o *CloudControllerManagerOptions) Config(allControllers []string, disabledByDefaultControllers []string, controllerAliases map[string]string, allWebhooks, disabledByDefaultWebhooks []string) (*config.Config, error) {
if err := o.Validate(allControllers, disabledByDefaultControllers, controllerAliases, allWebhooks, disabledByDefaultWebhooks); err != nil {
return nil, err
}
@ -298,7 +298,7 @@ func (o *CloudControllerManagerOptions) Config(allControllers, disabledByDefault
}
c := &config.Config{}
if err := o.ApplyTo(c, CloudControllerManagerUserAgent); err != nil {
if err := o.ApplyTo(c, allControllers, disabledByDefaultControllers, controllerAliases, CloudControllerManagerUserAgent); err != nil {
return nil, err
}

View File

@ -162,7 +162,7 @@ func TestAddFlags(t *testing.T) {
t.Errorf("unexpected err: %v", err)
}
for _, f := range s.Flags([]string{""}, []string{""}, []string{""}, []string{""}).FlagSets {
for _, f := range s.Flags([]string{""}, []string{""}, nil, []string{""}, []string{""}).FlagSets {
fs.AddFlagSet(f)
}
@ -321,7 +321,7 @@ func TestCreateConfig(t *testing.T) {
t.Errorf("unexpected err: %v", err)
}
for _, f := range s.Flags([]string{""}, []string{""}, []string{""}, []string{""}).FlagSets {
for _, f := range s.Flags([]string{""}, []string{""}, nil, []string{""}, []string{""}).FlagSets {
fs.AddFlagSet(f)
}
@ -371,7 +371,7 @@ func TestCreateConfig(t *testing.T) {
fmt.Printf("%s: %s\n", f.Name, f.Value)
})
c, err := s.Config([]string{"foo", "bar"}, []string{}, []string{"foo", "bar", "baz"}, []string{})
c, err := s.Config([]string{"foo", "bar"}, []string{}, nil, []string{"foo", "bar", "baz"}, []string{})
assert.Nil(t, err, "unexpected error: %s", err)
expected := &appconfig.Config{
@ -447,3 +447,39 @@ func TestCreateConfig(t *testing.T) {
t.Errorf("Got different config than expected.\nDifference detected on:\n%s", cmp.Diff(expected, c))
}
}
func TestCloudControllerManagerAliases(t *testing.T) {
opts, err := NewCloudControllerManagerOptions()
if err != nil {
t.Errorf("expected no error, error found %+v", err)
}
opts.KubeCloudShared.CloudProvider.Name = "gce"
opts.Generic.Controllers = []string{"service-controller", "-service", "route", "-cloud-node-lifecycle-controller"}
expectedControllers := []string{"service-controller", "-service-controller", "route-controller", "-cloud-node-lifecycle-controller"}
allControllers := []string{
"cloud-node-controller",
"service-controller",
"route-controller",
"cloud-node-lifecycle-controller",
}
disabledByDefaultControllers := []string{}
controllerAliases := map[string]string{
"cloud-node": "cloud-node-controller",
"service": "service-controller",
"route": "route-controller",
"cloud-node-lifecycle": "cloud-node-lifecycle-controller",
}
if err := opts.Validate(allControllers, disabledByDefaultControllers, controllerAliases, nil, nil); err != nil {
t.Errorf("expected no error, error found %v", err)
}
cfg := &cmconfig.GenericControllerManagerConfiguration{}
if err := opts.Generic.ApplyTo(cfg, allControllers, disabledByDefaultControllers, controllerAliases); err != nil {
t.Errorf("expected no error, error found %v", err)
}
if !reflect.DeepEqual(cfg.Controllers, expectedControllers) {
t.Errorf("controller aliases not resolved correctly, expected %+v, got %+v", expectedControllers, cfg.Controllers)
}
}

View File

@ -27,6 +27,7 @@ import (
cloudprovider "k8s.io/cloud-provider"
"k8s.io/cloud-provider/app"
"k8s.io/cloud-provider/app/config"
"k8s.io/cloud-provider/names"
"k8s.io/cloud-provider/options"
"k8s.io/component-base/cli"
cliflag "k8s.io/component-base/cli/flag"
@ -45,7 +46,7 @@ func main() {
}
fss := cliflag.NamedFlagSets{}
command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers(), fss, wait.NeverStop)
command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers(), names.CCMControllerAliases(), fss, wait.NeverStop)
code := cli.Run(command)
os.Exit(code)
}
@ -55,21 +56,21 @@ func main() {
// separately.
func controllerInitializers() map[string]app.ControllerInitFuncConstructor {
controllerInitializers := app.DefaultInitFuncConstructors
if constructor, ok := controllerInitializers["cloud-node"]; ok {
if constructor, ok := controllerInitializers[names.CloudNodeController]; ok {
constructor.InitContext.ClientName = "mycloud-external-cloud-node-controller"
controllerInitializers["cloud-node"] = constructor
controllerInitializers[names.CloudNodeController] = constructor
}
if constructor, ok := controllerInitializers["cloud-node-lifecycle"]; ok {
if constructor, ok := controllerInitializers[names.CloudNodeLifecycleController]; ok {
constructor.InitContext.ClientName = "mycloud-external-cloud-node-lifecycle-controller"
controllerInitializers["cloud-node-lifecycle"] = constructor
controllerInitializers[names.CloudNodeLifecycleController] = constructor
}
if constructor, ok := controllerInitializers["service"]; ok {
if constructor, ok := controllerInitializers[names.ServiceLBController]; ok {
constructor.InitContext.ClientName = "mycloud-external-service-controller"
controllerInitializers["service"] = constructor
controllerInitializers[names.ServiceLBController] = constructor
}
if constructor, ok := controllerInitializers["route"]; ok {
if constructor, ok := controllerInitializers[names.NodeRouteController]; ok {
constructor.InitContext.ClientName = "mycloud-external-route-controller"
controllerInitializers["route"] = constructor
controllerInitializers[names.NodeRouteController] = constructor
}
return controllerInitializers
}