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
1 change: 1 addition & 0 deletions cmd/kube-controller-manager/app/controller_descriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ func NewControllerDescriptors() map[string]*ControllerDescriptor {
register(newEndpointSliceMirroringControllerDescriptor())
register(newReplicationControllerDescriptor())
register(newPodGarbageCollectorControllerDescriptor())
register(newNodeMaintenanceControllerDescriptor())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

NodeMaintenance controller is added to the always-registered set without feature-gate or disabled-by-default

The new controller is registered alongside long-established controllers (outside the // feature gated block) and its descriptor does not set requiredFeatureGates or isDisabledByDefault. In kube-controller-manager, this typically means it will run by default when --controllers=* (or default controller set) is used.

Given this controller cordons/drains nodes, default-enabling is a high-impact behavior change if clusters have (or later add) the triggering annotations/configmap. Consider marking it feature-gated or disabled-by-default until the feature is GA.

Concrete mitigation (minimum): disable by default in the descriptor:

🛠️ Code Suggestions
 func newNodeMaintenanceControllerDescriptor() *ControllerDescriptor {
 	return &ControllerDescriptor{
 		name:        names.NodeMaintenanceController,
-		aliases:     []string{"node-maintenance"},
+		// Consider gating/alpha rollout; at minimum don't auto-enable.
+		isDisabledByDefault: true,
+		aliases:             []string{"node-maintenance"},
 		constructor: newNodeMaintenanceController,
 	}
 }
(If there is an existing feature gate for this, prefer `requiredFeatureGates: []featuregate.Feature{...}` instead.)
🤖 AI Fix Prompt - Copy this into your AI coding agent
1. VALIDATION
Open cmd/kube-controller-manager/app/controller_descriptor.go and confirm NodeMaintenance is registered in the always-on controller descriptor list (not inside a feature-gated section) and that newNodeMaintenanceControllerDescriptor() does not set requiredFeatureGates or isDisabledByDefault. If it’s already gated/disabled elsewhere (or intentionally always-on with explicit docs/release note), don’t change behavior.

2. PROBLEM & LOCATION
File: cmd/kube-controller-manager/app/controller_descriptor.go
Locate the NodeMaintenance controller descriptor (function newNodeMaintenanceControllerDescriptor) and the place where controller descriptors are added to the “always registered” set (outside the “// feature gated” block). The descriptor currently only sets name/aliases/constructor (pattern like name: names.NodeMaintenanceController, aliases: ..., constructor: ...) which means it will be enabled by default when the default controller set is used (e.g., --controllers=*), causing a high-impact behavioral change since this controller cordons/drains nodes.

3. FIX
Prefer feature-gating if a feature gate already exists for this controller:
- Search the codebase for an existing feature gate constant related to NodeMaintenance (look under pkg/features, staging/src/k8s.io/*/features, or similar).
- If found, add requiredFeatureGates: []featuregate.Feature{<that gate>} to the ControllerDescriptor returned by newNodeMaintenanceControllerDescriptor(), and ensure the controller is listed in the feature-gated controllers section if that’s the existing pattern in this file.

If no feature gate exists (minimum mitigation requested):
- Update newNodeMaintenanceControllerDescriptor() to set isDisabledByDefault: true in the returned ControllerDescriptor, keeping existing fields intact.
- Do not move the controller unless this file already has a standard pattern for “disabled-by-default” controllers; follow the existing pattern in this same file for other disabled-by-default descriptors (search for isDisabledByDefault usage and mirror it).

4. VERIFY
Run unit tests that cover controller descriptor registration / default controller sets (any tests under cmd/kube-controller-manager/app/ or related packages). Also sanity-check that kube-controller-manager still starts and that NodeMaintenance is not included in the default enabled controller list unless explicitly enabled (e.g., via --controllers=..., or by turning on the feature gate if you chose that path).

🔧 Fix in IDE — opens your editor with the fix prompt ready:

Fix in Cursor Fix in VS Code Fix in Codex Fix in Claude Code


↑ Back to Summary

register(newResourceQuotaControllerDescriptor())
register(newNamespaceControllerDescriptor())
register(newServiceAccountControllerDescriptor())
Expand Down
27 changes: 27 additions & 0 deletions cmd/kube-controller-manager/app/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (
nodeipamconfig "k8s.io/kubernetes/pkg/controller/nodeipam/config"
"k8s.io/kubernetes/pkg/controller/nodeipam/ipam"
lifecyclecontroller "k8s.io/kubernetes/pkg/controller/nodelifecycle"
"k8s.io/kubernetes/pkg/controller/nodemaintenance"
"k8s.io/kubernetes/pkg/controller/podgc"
replicationcontroller "k8s.io/kubernetes/pkg/controller/replication"
"k8s.io/kubernetes/pkg/controller/resourceclaim"
Expand Down Expand Up @@ -552,6 +553,32 @@ func newReplicationController(ctx context.Context, controllerContext ControllerC
}, controllerName), nil
}

func newNodeMaintenanceControllerDescriptor() *ControllerDescriptor {
return &ControllerDescriptor{
name: names.NodeMaintenanceController,
aliases: []string{"node-maintenance"},
constructor: newNodeMaintenanceController,
}
}

func newNodeMaintenanceController(ctx context.Context, controllerContext ControllerContext, controllerName string) (Controller, error) {
client, err := controllerContext.NewClient("node-maintenance-controller")
if err != nil {
return nil, err
}

nmc := nodemaintenance.NewController(
ctx,
client,
controllerContext.InformerFactory.Core().V1().Nodes(),
controllerContext.InformerFactory.Core().V1().ConfigMaps(),
controllerContext.InformerFactory.Core().V1().Pods(),
)
return newControllerLoop(func(ctx context.Context) {
nmc.Run(ctx, 1)
}, controllerName), nil
}

func newPodGarbageCollectorControllerDescriptor() *ControllerDescriptor {
return &ControllerDescriptor{
name: names.PodGarbageCollectorController,
Expand Down
1 change: 1 addition & 0 deletions cmd/kube-controller-manager/names/controller_names.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,5 @@ const (
ServiceCIDRController = "service-cidr-controller"
StorageVersionMigratorController = "storage-version-migrator-controller"
SELinuxWarningController = "selinux-warning-controller"
NodeMaintenanceController = "node-maintenance-controller"
)
74 changes: 74 additions & 0 deletions pkg/controller/nodemaintenance/concurrency.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
Copyright 2025 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 nodemaintenance

import "sync"

// concurrencyLimiter bounds how many nodes may be under active maintenance
// (cordoned + draining) at the same time, cluster-wide. It is safe for
// concurrent use.
type concurrencyLimiter struct {
mu sync.Mutex
tokens chan struct{}
}

// newConcurrencyLimiter returns a limiter that allows up to max concurrent
// holders.
func newConcurrencyLimiter(max int) *concurrencyLimiter {
if max < 0 {
max = 0
}
return &concurrencyLimiter{tokens: make(chan struct{}, max)}
}

// TryAcquire attempts to claim a slot without blocking. It returns false if
// the limiter is already at capacity.
func (l *concurrencyLimiter) TryAcquire() bool {
l.mu.Lock()
tokens := l.tokens
l.mu.Unlock()

select {
case tokens <- struct{}{}:
return true
default:
return false
}
}

// Release frees a previously-acquired slot.
func (l *concurrencyLimiter) Release() {
l.mu.Lock()
tokens := l.tokens
l.mu.Unlock()

select {
case <-tokens:
default:
}
}

// Resize changes the limiter's capacity to max, for example after the
// maintenance policy ConfigMap is edited.
func (l *concurrencyLimiter) Resize(max int) {
if max < 0 {
max = 0
}
l.mu.Lock()
defer l.mu.Unlock()
l.tokens = make(chan struct{}, max)
}
105 changes: 105 additions & 0 deletions pkg/controller/nodemaintenance/concurrency_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
Copyright 2025 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 nodemaintenance

import "testing"

func TestConcurrencyLimiter_AcquireUpToCapacity(t *testing.T) {
l := newConcurrencyLimiter(2)

if !l.TryAcquire() {
t.Fatal("first TryAcquire() = false, want true")
}
if !l.TryAcquire() {
t.Fatal("second TryAcquire() = false, want true")
}
if l.TryAcquire() {
t.Fatal("third TryAcquire() = true, want false (at capacity)")
}
}

func TestConcurrencyLimiter_ReleaseFreesASlot(t *testing.T) {
l := newConcurrencyLimiter(1)

if !l.TryAcquire() {
t.Fatal("TryAcquire() = false, want true")
}
if l.TryAcquire() {
t.Fatal("TryAcquire() while full = true, want false")
}

l.Release()

if !l.TryAcquire() {
t.Fatal("TryAcquire() after Release() = false, want true")
}
}

func TestConcurrencyLimiter_ZeroCapacityNeverAcquires(t *testing.T) {
l := newConcurrencyLimiter(0)
if l.TryAcquire() {
t.Fatal("TryAcquire() on a zero-capacity limiter = true, want false")
}
}

func TestConcurrencyLimiter_NegativeCapacityClampsToZero(t *testing.T) {
l := newConcurrencyLimiter(-3)
if l.TryAcquire() {
t.Fatal("TryAcquire() on a negative-capacity limiter = true, want false")
}
}

func TestConcurrencyLimiter_ResizeUp(t *testing.T) {
l := newConcurrencyLimiter(1)
if !l.TryAcquire() {
t.Fatal("TryAcquire() = false, want true")
}

l.Resize(3)

acquired := 0
for i := 0; i < 3; i++ {
if l.TryAcquire() {
acquired++
}
}
if acquired != 3 {
t.Errorf("acquired %d slots after resizing to 3, want 3", acquired)
}
}

func TestConcurrencyLimiter_ResizeDown(t *testing.T) {
l := newConcurrencyLimiter(5)
l.Resize(1)

if !l.TryAcquire() {
t.Fatal("first TryAcquire() after shrinking = false, want true")
}
if l.TryAcquire() {
t.Fatal("second TryAcquire() after shrinking to 1 = true, want false")
}
}

func TestConcurrencyLimiter_ReleaseWithoutAcquireIsSafe(t *testing.T) {
l := newConcurrencyLimiter(1)
// Should not panic or block.
l.Release()

if !l.TryAcquire() {
t.Fatal("TryAcquire() after a spurious Release() = false, want true")
}
}
Loading