node self-register continue if CREATE is 403.

tryRegisterWithAPIServer issues node create requests
regardless of whether the Node already exists.

If there's an admission policy forbidding the creation
of new nodes, the policy would return 403 even if the
Node actually exists.

This change makes Kubelet proceed to attempt to GET
the node in case the Create returns 403.

The feature `KubeletRegistrationGetOnExistsOnly`
restores the previous behavior.

pkg/features/kube_features.go

@@ -219,6 +219,13 @@ const (
 	// Remove in v1.33
 	AllowInsecureKubeletCertificateSigningRequests featuregate.Feature = "AllowInsecureKubeletCertificateSigningRequests"
 
+	// owner: @hoskeri
+	// Deprecated: v1.32
+	//
+	// Restores previous behavior where Kubelet fails self registration if node create returns 403 Forbidden.
+	// Remove in v1.34
+	KubeletRegistrationGetOnExistsOnly featuregate.Feature = "KubeletRegistrationGetOnExistsOnly"
+
 	// owner: @HirazawaUi
 	// kep: http://kep.k8s.io/4004
 	// Deprecated: v1.29 (default off)

pkg/features/versioned_kube_features.go

@@ -443,4 +443,7 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
 	ImageVolume: {
 		{Version: version.MustParse("1.31"), Default: false, PreRelease: featuregate.Alpha},
 	},
+	KubeletRegistrationGetOnExistsOnly: {
+		{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Deprecated},
+	},
 }

pkg/kubelet/kubelet_node_status.go

@@ -32,12 +32,14 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	cloudprovider "k8s.io/cloud-provider"
 	cloudproviderapi "k8s.io/cloud-provider/api"
 	nodeutil "k8s.io/component-helpers/node/util"
 	"k8s.io/klog/v2"
 	kubeletapis "k8s.io/kubelet/pkg/apis"
 	v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
+	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/kubelet/events"
 	"k8s.io/kubernetes/pkg/kubelet/nodestatus"
 	taintutil "k8s.io/kubernetes/pkg/util/taints"
@@ -91,7 +93,16 @@ func (kl *Kubelet) tryRegisterWithAPIServer(node *v1.Node) bool {
 		return true
 	}
 
-	if !apierrors.IsAlreadyExists(err) {
+	switch {
+	case apierrors.IsAlreadyExists(err):
+		// Node already exists, proceed to reconcile node.
+	case apierrors.IsForbidden(err):
+		// Creating nodes is forbidden, but node may still exist, attempt to get the node.
+		if utilfeature.DefaultFeatureGate.Enabled(features.KubeletRegistrationGetOnExistsOnly) {
+			klog.ErrorS(err, "Unable to register node with API server, reason is forbidden", "node", klog.KObj(node))
+			return false
+		}
+	default:
 		klog.ErrorS(err, "Unable to register node with API server", "node", klog.KObj(node))
 		return false
 	}
@@ -101,6 +112,7 @@ func (kl *Kubelet) tryRegisterWithAPIServer(node *v1.Node) bool {
 		klog.ErrorS(err, "Unable to register node with API server, error getting existing node", "node", klog.KObj(node))
 		return false
 	}
+
 	if existingNode == nil {
 		klog.InfoS("Unable to register node with API server, no node instance returned", "node", klog.KObj(node))
 		return false

pkg/kubelet/kubelet_node_status_test.go

@@ -45,13 +45,17 @@ import (
 	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/apimachinery/pkg/util/strategicpatch"
 	"k8s.io/apimachinery/pkg/util/uuid"
+	utilversion "k8s.io/apimachinery/pkg/util/version"
 	"k8s.io/apimachinery/pkg/util/wait"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/rest"
 	core "k8s.io/client-go/testing"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	"k8s.io/component-base/version"
 	kubeletapis "k8s.io/kubelet/pkg/apis"
+	"k8s.io/kubernetes/pkg/features"
 	cadvisortest "k8s.io/kubernetes/pkg/kubelet/cadvisor/testing"
 	"k8s.io/kubernetes/pkg/kubelet/cm"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
@@ -1386,6 +1390,10 @@ func TestTryRegisterWithApiServer(t *testing.T) {
 		ErrStatus: metav1.Status{Reason: metav1.StatusReasonConflict},
 	}
 
+	forbidden := &apierrors.StatusError{
+		ErrStatus: metav1.Status{Reason: metav1.StatusReasonForbidden},
+	}
+
 	newNode := func(cmad bool) *v1.Node {
 		node := &v1.Node{
 			ObjectMeta: metav1.ObjectMeta{
@@ -1408,18 +1416,19 @@ func TestTryRegisterWithApiServer(t *testing.T) {
 	}
 
 	cases := []struct {
-		name            string
+		name                   string
-		newNode         *v1.Node
+		newNode                *v1.Node
-		existingNode    *v1.Node
+		existingNode           *v1.Node
-		createError     error
+		createError            error
-		getError        error
+		getError               error
-		patchError      error
+		patchError             error
-		deleteError     error
+		deleteError            error
-		expectedResult  bool
+		expectedResult         bool
-		expectedActions int
+		expectedActions        int
-		testSavedNode   bool
+		testSavedNode          bool
-		savedNodeIndex  int
+		getOnForbiddenDisabled bool
-		savedNodeCMAD   bool
+		savedNodeIndex         int
+		savedNodeCMAD          bool
 	}{
 		{
 			name:            "success case - new node",
@@ -1435,6 +1444,25 @@ func TestTryRegisterWithApiServer(t *testing.T) {
 			expectedResult:  true,
 			expectedActions: 2,
 		},
+		{
+			name:            "success case - existing node - create forbidden - no change in CMAD",
+			newNode:         newNode(true),
+			createError:     forbidden,
+			existingNode:    newNode(true),
+			expectedResult:  true,
+			expectedActions: 2,
+		},
+		{
+			name:            "success case - existing node - create forbidden - CMAD disabled",
+			newNode:         newNode(false),
+			createError:     forbidden,
+			existingNode:    newNode(true),
+			expectedResult:  true,
+			expectedActions: 3,
+			testSavedNode:   true,
+			savedNodeIndex:  2,
+			savedNodeCMAD:   false,
+		},
 		{
 			name:            "success case - existing node - CMAD disabled",
 			newNode:         newNode(false),
@@ -1464,6 +1492,14 @@ func TestTryRegisterWithApiServer(t *testing.T) {
 			expectedResult:  false,
 			expectedActions: 1,
 		},
+		{
+			name:                   "create failed with forbidden - get-on-forbidden feature is disabled",
+			newNode:                newNode(false),
+			getOnForbiddenDisabled: true,
+			createError:            forbidden,
+			expectedResult:         false,
+			expectedActions:        1,
+		},
 		{
 			name:            "get existing node failed",
 			newNode:         newNode(false),
@@ -1484,55 +1520,61 @@ func TestTryRegisterWithApiServer(t *testing.T) {
 	}
 
 	for _, tc := range cases {
-		testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled is a don't-care for this test */)
+		t.Run(tc.name, func(t *testing.T) {
-		defer testKubelet.Cleanup()
+			if tc.getOnForbiddenDisabled {
-		kubelet := testKubelet.kubelet
+				featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, utilversion.MustParse("1.32"))
-		kubeClient := testKubelet.fakeKubeClient
+				featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KubeletRegistrationGetOnExistsOnly, true)
-
-		kubeClient.AddReactor("create", "nodes", func(action core.Action) (bool, runtime.Object, error) {
-			return true, nil, tc.createError
-		})
-		kubeClient.AddReactor("get", "nodes", func(action core.Action) (bool, runtime.Object, error) {
-			// Return an existing (matching) node on get.
-			return true, tc.existingNode, tc.getError
-		})
-		kubeClient.AddReactor("patch", "nodes", func(action core.Action) (bool, runtime.Object, error) {
-			if action.GetSubresource() == "status" {
-				return true, nil, tc.patchError
 			}
-			return notImplemented(action)
+			testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled is a don't-care for this test */)
-		})
+			defer testKubelet.Cleanup()
-		kubeClient.AddReactor("delete", "nodes", func(action core.Action) (bool, runtime.Object, error) {
+			kubelet := testKubelet.kubelet
-			return true, nil, tc.deleteError
+			kubeClient := testKubelet.fakeKubeClient
-		})
-		addNotImplatedReaction(kubeClient)
 
-		result := kubelet.tryRegisterWithAPIServer(tc.newNode)
+			kubeClient.AddReactor("create", "nodes", func(action core.Action) (bool, runtime.Object, error) {
-		require.Equal(t, tc.expectedResult, result, "test [%s]", tc.name)
+				return true, nil, tc.createError
+			})
+			kubeClient.AddReactor("get", "nodes", func(action core.Action) (bool, runtime.Object, error) {
+				// Return an existing (matching) node on get.
+				return true, tc.existingNode, tc.getError
+			})
+			kubeClient.AddReactor("patch", "nodes", func(action core.Action) (bool, runtime.Object, error) {
+				if action.GetSubresource() == "status" {
+					return true, nil, tc.patchError
+				}
+				return notImplemented(action)
+			})
+			kubeClient.AddReactor("delete", "nodes", func(action core.Action) (bool, runtime.Object, error) {
+				return true, nil, tc.deleteError
+			})
+			addNotImplatedReaction(kubeClient)
 
-		actions := kubeClient.Actions()
+			result := kubelet.tryRegisterWithAPIServer(tc.newNode)
-		assert.Len(t, actions, tc.expectedActions, "test [%s]", tc.name)
+			require.Equal(t, tc.expectedResult, result, "test [%s]", tc.name)
 
-		if tc.testSavedNode {
+			actions := kubeClient.Actions()
-			var savedNode *v1.Node
+			assert.Len(t, actions, tc.expectedActions, "test [%s]", tc.name)
 
-			t.Logf("actions: %v: %+v", len(actions), actions)
+			if tc.testSavedNode {
-			action := actions[tc.savedNodeIndex]
+				var savedNode *v1.Node
-			if action.GetVerb() == "create" {
+
-				createAction := action.(core.CreateAction)
+				t.Logf("actions: %v: %+v", len(actions), actions)
-				obj := createAction.GetObject()
+				action := actions[tc.savedNodeIndex]
-				require.IsType(t, &v1.Node{}, obj)
+				if action.GetVerb() == "create" {
-				savedNode = obj.(*v1.Node)
+					createAction := action.(core.CreateAction)
-			} else if action.GetVerb() == "patch" {
+					obj := createAction.GetObject()
-				patchAction := action.(core.PatchActionImpl)
+					require.IsType(t, &v1.Node{}, obj)
-				var err error
+					savedNode = obj.(*v1.Node)
-				savedNode, err = applyNodeStatusPatch(tc.existingNode, patchAction.GetPatch())
+				} else if action.GetVerb() == "patch" {
-				require.NoError(t, err)
+					patchAction := action.(core.PatchActionImpl)
+					var err error
+					savedNode, err = applyNodeStatusPatch(tc.existingNode, patchAction.GetPatch())
+					require.NoError(t, err)
+				}
+
+				actualCMAD, _ := strconv.ParseBool(savedNode.Annotations[util.ControllerManagedAttachAnnotation])
+				assert.Equal(t, tc.savedNodeCMAD, actualCMAD, "test [%s]", tc.name)
 			}
-
+		})
-			actualCMAD, _ := strconv.ParseBool(savedNode.Annotations[util.ControllerManagedAttachAnnotation])
-			assert.Equal(t, tc.savedNodeCMAD, actualCMAD, "test [%s]", tc.name)
-		}
 	}
 }
 

test/featuregates_linter/test_data/versioned_feature_list.yaml

@@ -362,6 +362,12 @@
     lockToDefault: false
     preRelease: Alpha
     version: "1.27"
+- name: KubeletRegistrationGetOnExistsOnly
+  versionedSpecs:
+  - default: false
+    lockToDefault: false
+    preRelease: Deprecated
+    version: "1.32"
 - name: KubeletSeparateDiskGC
   versionedSpecs:
   - default: false