support controller name aliases in GenericControllerManagerConfigurationOptions

This commit is contained in:
Filip Křepinský 2023-02-13 16:43:57 +01:00
parent 988094878e
commit ba1755132e
5 changed files with 338 additions and 61 deletions

View File

@ -49,7 +49,7 @@ func NewGenericControllerManagerConfigurationOptions(cfg *cmconfig.GenericContro
}
// AddFlags adds flags related to generic for controller manager to the specified FlagSet.
func (o *GenericControllerManagerConfigurationOptions) AddFlags(fss *cliflag.NamedFlagSets, allControllers, disabledByDefaultControllers []string) {
func (o *GenericControllerManagerConfigurationOptions) AddFlags(fss *cliflag.NamedFlagSets, allControllers, disabledByDefaultControllers []string, controllerAliases map[string]string) {
if o == nil {
return
}
@ -71,7 +71,7 @@ func (o *GenericControllerManagerConfigurationOptions) AddFlags(fss *cliflag.Nam
}
// ApplyTo fills up generic config with options.
func (o *GenericControllerManagerConfigurationOptions) ApplyTo(cfg *cmconfig.GenericControllerManagerConfiguration) error {
func (o *GenericControllerManagerConfigurationOptions) ApplyTo(cfg *cmconfig.GenericControllerManagerConfiguration, allControllers []string, disabledByDefaultControllers []string, controllerAliases map[string]string) error {
if o == nil {
return nil
}
@ -88,13 +88,26 @@ func (o *GenericControllerManagerConfigurationOptions) ApplyTo(cfg *cmconfig.Gen
cfg.ClientConnection = o.ClientConnection
cfg.ControllerStartInterval = o.ControllerStartInterval
cfg.LeaderElection = o.LeaderElection
cfg.Controllers = o.Controllers
// copy controller names and replace aliases with canonical names
cfg.Controllers = make([]string, len(o.Controllers))
for i, initialName := range o.Controllers {
initialNameWithoutPrefix := strings.TrimPrefix(initialName, "-")
controllerName := initialNameWithoutPrefix
if canonicalName, ok := controllerAliases[controllerName]; ok {
controllerName = canonicalName
}
if strings.HasPrefix(initialName, "-") {
controllerName = fmt.Sprintf("-%s", controllerName)
}
cfg.Controllers[i] = controllerName
}
return nil
}
// Validate checks validation of GenericOptions.
func (o *GenericControllerManagerConfigurationOptions) Validate(allControllers []string, disabledByDefaultControllers []string) []error {
func (o *GenericControllerManagerConfigurationOptions) Validate(allControllers []string, disabledByDefaultControllers []string, controllerAliases map[string]string) []error {
if o == nil {
return nil
}
@ -109,13 +122,17 @@ func (o *GenericControllerManagerConfigurationOptions) Validate(allControllers [
}
allControllersSet := sets.NewString(allControllers...)
for _, controller := range o.Controllers {
if controller == "*" {
for _, initialName := range o.Controllers {
if initialName == "*" {
continue
}
controller = strings.TrimPrefix(controller, "-")
if !allControllersSet.Has(controller) {
errs = append(errs, fmt.Errorf("%q is not in the list of known controllers", controller))
initialNameWithoutPrefix := strings.TrimPrefix(initialName, "-")
controllerName := initialNameWithoutPrefix
if canonicalName, ok := controllerAliases[controllerName]; ok {
controllerName = canonicalName
}
if !allControllersSet.Has(controllerName) {
errs = append(errs, fmt.Errorf("%q is not in the list of known controllers", initialNameWithoutPrefix))
}
}

View File

@ -0,0 +1,260 @@
/*
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 options
import (
"reflect"
"strings"
"testing"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/component-base/config"
cmconfig "k8s.io/controller-manager/config"
)
func TestValidateGenericControllerManagerConfigurationOptions(t *testing.T) {
testCases := []struct {
name string
allControllers []string
controllerAliases map[string]string
options *GenericControllerManagerConfigurationOptions
expectErrors bool
expectedErrorSubString string
}{
{
name: "no controllers defined",
allControllers: nil,
controllerAliases: nil,
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{
Controllers: []string{
"*",
},
}),
},
{
name: "recognizes empty controllers",
allControllers: getAllControllers(),
controllerAliases: getControllerAliases(),
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{}),
},
{
name: "recognizes controllers without any aliases",
allControllers: getAllControllers(),
controllerAliases: nil,
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{
Controllers: []string{
"blue-controller",
},
}),
},
{
name: "recognizes valid controllers",
allControllers: getAllControllers(),
controllerAliases: getControllerAliases(),
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{
Controllers: []string{
"*",
"-red-controller",
"blue-controller",
},
}),
},
{
name: "recognizes disabled controller",
allControllers: getAllControllers(),
controllerAliases: getControllerAliases(),
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{
Controllers: []string{
"green-controller",
},
}),
},
{
name: "recognized aliased controller",
allControllers: getAllControllers(),
controllerAliases: getControllerAliases(),
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{
Controllers: []string{
"ultramarine-controller",
"-pink-controller",
},
}),
},
{
name: "does not recognize controller",
allControllers: nil,
controllerAliases: nil,
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{
Controllers: []string{
"red-controller",
},
}),
expectErrors: true,
expectedErrorSubString: "\"red-controller\" is not in the list of known controllers",
},
{
name: "does not recognize controller with aliases",
allControllers: getAllControllers(),
controllerAliases: getControllerAliases(),
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{
Controllers: []string{
"crimson-controller",
"grey-controller",
},
}),
expectErrors: true,
expectedErrorSubString: "\"grey-controller\" is not in the list of known controllers",
},
{
name: "leader election accepts only leases",
allControllers: getAllControllers(),
controllerAliases: getControllerAliases(),
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{
LeaderElection: config.LeaderElectionConfiguration{
LeaderElect: true,
ResourceLock: "configmapsleases",
},
}),
expectErrors: true,
expectedErrorSubString: "resourceLock value must be \"leases\"",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
errs := tc.options.Validate(tc.allControllers, []string{"green-controller"}, tc.controllerAliases)
if len(errs) > 0 && !tc.expectErrors {
t.Errorf("expected no errors, errors found %+v", errs)
}
if len(errs) == 0 && tc.expectErrors {
t.Errorf("expected errors, no errors found")
}
if len(errs) > 0 && tc.expectErrors {
gotErr := utilerrors.NewAggregate(errs).Error()
if !strings.Contains(gotErr, tc.expectedErrorSubString) {
t.Errorf("expected error: %s, got err: %v", tc.expectedErrorSubString, gotErr)
}
}
})
}
}
func TestApplyToGenericControllerManagerConfigurationOptions(t *testing.T) {
testCases := []struct {
name string
allControllers []string
controllerAliases map[string]string
options *GenericControllerManagerConfigurationOptions
expectedControllers []string
}{
{
name: "no controllers defined",
allControllers: nil,
controllerAliases: nil,
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{
Controllers: []string{
"*",
},
}),
expectedControllers: []string{
"*",
},
},
{
name: "empty aliases",
allControllers: getAllControllers(),
controllerAliases: nil,
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{
Controllers: []string{
"-blue-controller",
},
}),
expectedControllers: []string{
"-blue-controller",
},
},
{
name: "applies valid controllers",
allControllers: getAllControllers(),
controllerAliases: getControllerAliases(),
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{
Controllers: []string{
"*",
"green-controller",
"-red-controller",
"blue-controller",
},
}),
expectedControllers: []string{
"*",
"green-controller",
"-red-controller",
"blue-controller",
},
},
{
name: "resolves aliases",
allControllers: getAllControllers(),
controllerAliases: getControllerAliases(),
options: NewGenericControllerManagerConfigurationOptions(&cmconfig.GenericControllerManagerConfiguration{
Controllers: []string{
"green-controller",
"-crimson-controller",
"ultramarine-controller",
"-pink-controller",
},
}),
expectedControllers: []string{
"green-controller",
"-red-controller",
"blue-controller",
"-red-controller",
},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
cfg := &cmconfig.GenericControllerManagerConfiguration{}
err := tc.options.ApplyTo(cfg, tc.allControllers, []string{"green-controller"}, tc.controllerAliases)
if err != nil {
t.Errorf("expected no errors, error found: %v", err)
}
if !reflect.DeepEqual(cfg.Controllers, tc.expectedControllers) {
t.Errorf("applyTo failed, expected controllers %q, got controllers %q", strings.Join(cfg.Controllers, ","), strings.Join(tc.expectedControllers, ","))
}
})
}
}
func getAllControllers() []string {
return []string{
"red-controller",
"green-controller",
"blue-controller",
}
}
func getControllerAliases() map[string]string {
return map[string]string{
"crimson-controller": "red-controller",
"pink-controller": "red-controller",
"ultramarine-controller": "blue-controller",
}
}

View File

@ -26,13 +26,13 @@ func DefaultLeaderMigrationConfiguration() *internal.LeaderMigrationConfiguratio
ResourceLock: ResourceLockLeases,
ControllerLeaders: []internal.ControllerLeaderConfiguration{
{
Name: "route",
Name: "route-controller",
Component: "*",
}, {
Name: "service",
Name: "service-controller",
Component: "*",
}, {
Name: "cloud-node-lifecycle",
Name: "cloud-node-lifecycle-controller",
Component: "*",
},
},

View File

@ -28,13 +28,13 @@ func TestLeaderMigratorFilterFunc(t *testing.T) {
LeaderName: "cloud-provider-extraction-migration",
ControllerLeaders: []internal.ControllerLeaderConfiguration{
{
Name: "route",
Name: "route-controller",
Component: "kube-controller-manager",
}, {
Name: "service",
Name: "service-controller",
Component: "kube-controller-manager",
}, {
Name: "cloud-node-lifecycle",
Name: "cloud-node-lifecycle-controller",
Component: "kube-controller-manager",
},
},
@ -44,13 +44,13 @@ func TestLeaderMigratorFilterFunc(t *testing.T) {
LeaderName: "cloud-provider-extraction-migration",
ControllerLeaders: []internal.ControllerLeaderConfiguration{
{
Name: "route",
Name: "route-controller",
Component: "cloud-controller-manager",
}, {
Name: "service",
Name: "service-controller",
Component: "cloud-controller-manager",
}, {
Name: "cloud-node-lifecycle",
Name: "cloud-node-lifecycle-controller",
Component: "cloud-controller-manager",
},
},
@ -60,13 +60,13 @@ func TestLeaderMigratorFilterFunc(t *testing.T) {
LeaderName: "cloud-provider-extraction-migration",
ControllerLeaders: []internal.ControllerLeaderConfiguration{
{
Name: "route",
Name: "route-controller",
Component: "*",
}, {
Name: "service",
Name: "service-controller",
Component: "*",
}, {
Name: "cloud-node-lifecycle",
Name: "cloud-node-lifecycle-controller",
Component: "*",
},
},
@ -83,10 +83,10 @@ func TestLeaderMigratorFilterFunc(t *testing.T) {
config: fromConfig,
component: "kube-controller-manager",
expectResult: map[string]FilterResult{
"deployment": ControllerNonMigrated,
"route": ControllerMigrated,
"service": ControllerMigrated,
"cloud-node-lifecycle": ControllerMigrated,
"deployment-controller": ControllerNonMigrated,
"route-controller": ControllerMigrated,
"service-controller": ControllerMigrated,
"cloud-node-lifecycle-controller": ControllerMigrated,
},
},
{
@ -95,9 +95,9 @@ func TestLeaderMigratorFilterFunc(t *testing.T) {
component: "cloud-controller-manager",
expectResult: map[string]FilterResult{
"cloud-node": ControllerNonMigrated,
"route": ControllerUnowned,
"service": ControllerUnowned,
"cloud-node-lifecycle": ControllerUnowned,
"route-controller": ControllerUnowned,
"service-controller": ControllerUnowned,
"cloud-node-lifecycle-controller": ControllerUnowned,
},
},
{
@ -105,10 +105,10 @@ func TestLeaderMigratorFilterFunc(t *testing.T) {
config: toConfig,
component: "kube-controller-manager",
expectResult: map[string]FilterResult{
"deployment": ControllerNonMigrated,
"route": ControllerUnowned,
"service": ControllerUnowned,
"cloud-node-lifecycle": ControllerUnowned,
"deployment-controller": ControllerNonMigrated,
"route-controller": ControllerUnowned,
"service-controller": ControllerUnowned,
"cloud-node-lifecycle-controller": ControllerUnowned,
},
},
{
@ -116,10 +116,10 @@ func TestLeaderMigratorFilterFunc(t *testing.T) {
config: toConfig,
component: "cloud-controller-manager",
expectResult: map[string]FilterResult{
"cloud-node": ControllerNonMigrated,
"route": ControllerMigrated,
"service": ControllerMigrated,
"cloud-node-lifecycle": ControllerMigrated,
"cloud-node-controller": ControllerNonMigrated,
"route-controller": ControllerMigrated,
"service-controller": ControllerMigrated,
"cloud-node-lifecycle-controller": ControllerMigrated,
},
},
{
@ -127,10 +127,10 @@ func TestLeaderMigratorFilterFunc(t *testing.T) {
config: wildcardConfig,
component: "kube-controller-manager",
expectResult: map[string]FilterResult{
"deployment": ControllerNonMigrated, // KCM only
"route": ControllerMigrated,
"service": ControllerMigrated,
"cloud-node-lifecycle": ControllerMigrated,
"deployment-controller": ControllerNonMigrated, // KCM only
"route-controller": ControllerMigrated,
"service-controller": ControllerMigrated,
"cloud-node-lifecycle-controller": ControllerMigrated,
},
},
{
@ -138,10 +138,10 @@ func TestLeaderMigratorFilterFunc(t *testing.T) {
config: wildcardConfig,
component: "cloud-controller-manager",
expectResult: map[string]FilterResult{
"cloud-node": ControllerNonMigrated, // CCM only
"route": ControllerMigrated,
"service": ControllerMigrated,
"cloud-node-lifecycle": ControllerMigrated,
"cloud-node-controller": ControllerNonMigrated, // CCM only
"route-controller": ControllerMigrated,
"service-controller": ControllerMigrated,
"cloud-node-lifecycle-controller": ControllerMigrated,
},
},
} {

View File

@ -107,13 +107,13 @@ apiVersion: controllermanager.config.k8s.io/v1
kind: LeaderMigrationConfiguration
leaderName: test-leader-migration
controllerLeaders:
- name: route
- name: route-controller
component: "*"
- name: service
- name: service-controller
component: "*"
- name: cloud-node-lifecycle
- name: cloud-node-lifecycle-controller
component: "*"
- name: nodeipam
- name: node-ipam-controller
component: "*"
`,
expectErr: false,
@ -122,19 +122,19 @@ controllerLeaders:
ResourceLock: "leases",
ControllerLeaders: []config.ControllerLeaderConfiguration{
{
Name: "route",
Name: "route-controller",
Component: "*",
},
{
Name: "service",
Name: "service-controller",
Component: "*",
},
{
Name: "cloud-node-lifecycle",
Name: "cloud-node-lifecycle-controller",
Component: "*",
},
{
Name: "nodeipam",
Name: "node-ipam-controller",
Component: "*",
},
},
@ -148,13 +148,13 @@ apiVersion: controllermanager.config.k8s.io/v1
kind: LeaderMigrationConfiguration
leaderName: test-leader-migration
controllerLeaders:
- name: route
- name: route-controller
component: "cloud-controller-manager"
- name: service
- name: service-controller
component: "cloud-controller-manager"
- name: cloud-node-lifecycle
- name: cloud-node-lifecycle-controller
component: "cloud-controller-manager"
- name: nodeipam
- name: node-ipam-controller
component: "kube-controller-manager"
`,
expectErr: false,
@ -163,19 +163,19 @@ controllerLeaders:
ResourceLock: "leases",
ControllerLeaders: []config.ControllerLeaderConfiguration{
{
Name: "route",
Name: "route-controller",
Component: "cloud-controller-manager",
},
{
Name: "service",
Name: "service-controller",
Component: "cloud-controller-manager",
},
{
Name: "cloud-node-lifecycle",
Name: "cloud-node-lifecycle-controller",
Component: "cloud-controller-manager",
},
{
Name: "nodeipam",
Name: "node-ipam-controller",
Component: "kube-controller-manager",
},
},