Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 24 additions & 52 deletions cmd/kube-controller-manager/app/controllermanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,12 @@ import (
"github.com/google/go-cmp/cmp"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/version"
"k8s.io/apiserver/pkg/server/healthz"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/component-base/featuregate"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/klog/v2/ktesting"
"k8s.io/kubernetes/cmd/kube-controller-manager/names"
"k8s.io/kubernetes/pkg/features"
)

func TestControllerNamesConsistency(t *testing.T) {
Expand Down Expand Up @@ -176,60 +174,34 @@ func TestFeatureGatedControllersShouldNotDefineAliases(t *testing.T) {
}
}

// TestTaintEvictionControllerGating ensures that it is possible to run taint-manager as a separated controller
// only when the SeparateTaintEvictionController feature is enabled
func TestTaintEvictionControllerGating(t *testing.T) {
tests := []struct {
name string
enableFeatureGate bool
expectInitFuncCall bool
}{
{
name: "standalone taint-eviction-controller should run when SeparateTaintEvictionController feature gate is enabled",
enableFeatureGate: true,
expectInitFuncCall: true,
},
{
name: "standalone taint-eviction-controller should not run when SeparateTaintEvictionController feature gate is not enabled",
enableFeatureGate: false,
expectInitFuncCall: false,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, version.MustParse("1.33"))
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SeparateTaintEvictionController, test.enableFeatureGate)
_, ctx := ktesting.NewTestContext(t)
ctx, cancel := context.WithCancel(ctx)
defer cancel()
// TestTaintEvictionController ensures that it is possible to run taint-eviction-controller as a separated controller
func TestTaintEvictionController(t *testing.T) {
_, ctx := ktesting.NewTestContext(t)
ctx, cancel := context.WithCancel(ctx)
defer cancel()

controllerCtx := ControllerContext{}
controllerCtx.ComponentConfig.Generic.Controllers = []string{names.TaintEvictionController}
controllerCtx := ControllerContext{}
controllerCtx.ComponentConfig.Generic.Controllers = []string{names.TaintEvictionController}

initFuncCalled := false
initFuncCalled := false

taintEvictionControllerDescriptor := NewControllerDescriptors()[names.TaintEvictionController]
taintEvictionControllerDescriptor.constructor = func(ctx context.Context, controllerContext ControllerContext, controllerName string) (Controller, error) {
initFuncCalled = true
return newControllerLoop(func(ctx context.Context) {}, controllerName), nil
}
taintEvictionControllerDescriptor := NewControllerDescriptors()[names.TaintEvictionController]
taintEvictionControllerDescriptor.constructor = func(ctx context.Context, controllerContext ControllerContext, controllerName string) (Controller, error) {
initFuncCalled = true
return newControllerLoop(func(ctx context.Context) {}, controllerName), nil
}

var healthChecks mockHealthCheckAdder
if err := runControllers(ctx, controllerCtx, map[string]*ControllerDescriptor{
names.TaintEvictionController: taintEvictionControllerDescriptor,
}, &healthChecks); err != nil {
t.Errorf("starting a TaintEvictionController controller should not return an error")
}
if test.expectInitFuncCall != initFuncCalled {
t.Errorf("TaintEvictionController init call check failed: expected=%v, got=%v", test.expectInitFuncCall, initFuncCalled)
}
hasHealthCheck := len(healthChecks.Checks) > 0
expectHealthCheck := test.expectInitFuncCall
if expectHealthCheck != hasHealthCheck {
t.Errorf("TaintEvictionController healthCheck check failed: expected=%v, got=%v", expectHealthCheck, hasHealthCheck)
}
})
var healthChecks mockHealthCheckAdder
if err := runControllers(ctx, controllerCtx, map[string]*ControllerDescriptor{
names.TaintEvictionController: taintEvictionControllerDescriptor,
}, &healthChecks); err != nil {
t.Errorf("starting a TaintEvictionController controller should not return an error")
}
if !initFuncCalled {
t.Errorf("TaintEvictionController init func not called")
}
if len(healthChecks.Checks) == 0 {
t.Errorf("TaintEvictionController healthCheck check failed: expected health check to be added")
}
}

Expand Down
3 changes: 0 additions & 3 deletions cmd/kube-controller-manager/app/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,6 @@ func newTaintEvictionControllerDescriptor() *ControllerDescriptor {
return &ControllerDescriptor{
name: names.TaintEvictionController,
constructor: newTaintEvictionController,
requiredFeatureGates: []featuregate.Feature{
features.SeparateTaintEvictionController,
},
}
}

Expand Down
24 changes: 0 additions & 24 deletions pkg/controller/nodelifecycle/node_lifecycle_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ import (
kubeletapis "k8s.io/kubelet/pkg/apis"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/kubernetes/pkg/controller/nodelifecycle/scheduler"
"k8s.io/kubernetes/pkg/controller/tainteviction"
consistencyutil "k8s.io/kubernetes/pkg/controller/util/consistency"
controllerutil "k8s.io/kubernetes/pkg/controller/util/node"
"k8s.io/kubernetes/pkg/features"
Expand Down Expand Up @@ -137,11 +136,6 @@ const (
podUpdateWorkerSize = 4
// nodeUpdateWorkerSize defines the size of workers for node update or/and pod update.
nodeUpdateWorkerSize = 8

// taintEvictionController is defined here in order to prevent imports of
// k8s.io/kubernetes/cmd/kube-controller-manager/names which would result in validation errors.
// This constant will be removed upon graduation of the SeparateTaintEvictionController feature.
taintEvictionController = "taint-eviction-controller"
)

// labelReconcileInfo lists Node labels to reconcile, and how to reconcile them.
Expand Down Expand Up @@ -222,8 +216,6 @@ type podUpdateItem struct {

// Controller is the controller that manages node's life cycle.
type Controller struct {
taintManager *tainteviction.Controller

podLister corelisters.PodLister
podInformerSynced cache.InformerSynced
kubeClient clientset.Interface
Expand Down Expand Up @@ -398,15 +390,6 @@ func NewNodeLifecycleController(
nc.podLister = podInformer.Lister()
nc.nodeLister = nodeInformer.Lister()

if !utilfeature.DefaultFeatureGate.Enabled(features.SeparateTaintEvictionController) {
logger.Info("Running TaintEvictionController as part of NodeLifecyleController")
tm, err := tainteviction.New(ctx, kubeClient, podInformer, nodeInformer, taintEvictionController)
if err != nil {
return nil, err
}
nc.taintManager = tm
}

logger.Info("Controller will reconcile labels")
nodeInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: controllerutil.CreateAddNodeHandler(func(node *v1.Node) error {
Expand Down Expand Up @@ -471,13 +454,6 @@ func (nc *Controller) Run(ctx context.Context) {
return
}

if !utilfeature.DefaultFeatureGate.Enabled(features.SeparateTaintEvictionController) {
logger.Info("Starting", "controller", taintEvictionController)
wg.Go(func() {
nc.taintManager.Run(ctx)
})
}

// Start workers to reconcile labels and/or update NoSchedule taint for nodes.
for i := 0; i < nodeUpdateWorkerSize; i++ {
// Thanks to "workqueue", each worker just need to get item from queue, because
Expand Down
13 changes: 0 additions & 13 deletions pkg/features/kube_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -1068,12 +1068,6 @@ const (
// Enables PreQueueingHint extension point to narrow pod evaluation on events.
SchedulerPreQueueingHints featuregate.Feature = "SchedulerPreQueueingHints"

// owner: @atosatto @yuanchen8911
// kep: http://kep.k8s.io/3902
//
// Decouples Taint Eviction Controller, performing taint-based Pod eviction, from Node Lifecycle Controller.
SeparateTaintEvictionController featuregate.Feature = "SeparateTaintEvictionController"

// owner: @aramase
// kep: https://kep.k8s.io/4412
//
Expand Down Expand Up @@ -2081,11 +2075,6 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
{Version: version.MustParse("1.37"), Default: false, PreRelease: featuregate.Alpha},
},

SeparateTaintEvictionController: {
{Version: version.MustParse("1.29"), Default: true, PreRelease: featuregate.Beta},
{Version: version.MustParse("1.34"), Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.37 (locked to default in 1.34)
},

ServiceAccountNodeAudienceRestriction: {
{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Beta},
{Version: version.MustParse("1.33"), Default: true, PreRelease: featuregate.Beta},
Expand Down Expand Up @@ -2793,8 +2782,6 @@ var defaultKubernetesFeatureGateDependencies = map[featuregate.Feature][]feature
SchedulerPopFromBackoffQ: {},
SchedulerPreQueueingHints: {},

SeparateTaintEvictionController: {},

ServiceAccountNodeAudienceRestriction: {},

ServiceAccountTokenJTI: {},
Expand Down
1 change: 0 additions & 1 deletion test/compatibility_lifecycle/reference/feature_list.md
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@
| SchedulerPopFromBackoffQ | :ballot_box_with_check:&nbsp;1.33+ | | | 1.33– | | | | [code](https://cs.k8s.io/?q=%5CbSchedulerPopFromBackoffQ%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/kubernetes) [KEPs](https://cs.k8s.io/?q=%5CbSchedulerPopFromBackoffQ%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/enhancements) |
| SchedulerPreQueueingHints | | | 1.37– | | | | | [code](https://cs.k8s.io/?q=%5CbSchedulerPreQueueingHints%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/kubernetes) [KEPs](https://cs.k8s.io/?q=%5CbSchedulerPreQueueingHints%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/enhancements) |
| SeparateCacheWatchRPC | :ballot_box_with_check:&nbsp;1.28+ | :closed_lock_with_key:&nbsp;1.36+ | | 1.28–1.32 | | 1.33– | | [code](https://cs.k8s.io/?q=%5CbSeparateCacheWatchRPC%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/kubernetes) [KEPs](https://cs.k8s.io/?q=%5CbSeparateCacheWatchRPC%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/enhancements) |
| SeparateTaintEvictionController | :ballot_box_with_check:&nbsp;1.29+ | :closed_lock_with_key:&nbsp;1.34+ | | 1.29–1.33 | 1.34– | | | [code](https://cs.k8s.io/?q=%5CbSeparateTaintEvictionController%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/kubernetes) [KEPs](https://cs.k8s.io/?q=%5CbSeparateTaintEvictionController%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/enhancements) |
| ServiceAccountNodeAudienceRestriction | :ballot_box_with_check:&nbsp;1.33+ | | | 1.32– | | | | [code](https://cs.k8s.io/?q=%5CbServiceAccountNodeAudienceRestriction%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/kubernetes) [KEPs](https://cs.k8s.io/?q=%5CbServiceAccountNodeAudienceRestriction%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/enhancements) |
| ServiceAccountTokenJTI | :ballot_box_with_check:&nbsp;1.30+ | :closed_lock_with_key:&nbsp;1.32+ | 1.29 | 1.30–1.31 | 1.32– | | | [code](https://cs.k8s.io/?q=%5CbServiceAccountTokenJTI%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/kubernetes) [KEPs](https://cs.k8s.io/?q=%5CbServiceAccountTokenJTI%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/enhancements) |
| ServiceAccountTokenNodeBinding | :ballot_box_with_check:&nbsp;1.31+ | :closed_lock_with_key:&nbsp;1.33+ | 1.29–1.30 | 1.31–1.32 | 1.33– | | ServiceAccountTokenNodeBindingValidation | [code](https://cs.k8s.io/?q=%5CbServiceAccountTokenNodeBinding%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/kubernetes) [KEPs](https://cs.k8s.io/?q=%5CbServiceAccountTokenNodeBinding%5Cb&i=nope&files=&excludeFiles=CHANGELOG&repos=kubernetes/enhancements) |
Expand Down
10 changes: 0 additions & 10 deletions test/compatibility_lifecycle/reference/versioned_feature_list.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1872,16 +1872,6 @@
lockToDefault: true
preRelease: Deprecated
version: "1.36"
- name: SeparateTaintEvictionController
versionedSpecs:
- default: true
lockToDefault: false
preRelease: Beta
version: "1.29"
- default: true
lockToDefault: true
preRelease: GA
version: "1.34"
- name: ServiceAccountNodeAudienceRestriction
versionedSpecs:
- default: false
Expand Down
Loading