diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index c30588fb4bb..fe474402f96 100644 --- a/pkg/features/kube_features.go +++ b/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) diff --git a/pkg/features/versioned_kube_features.go b/pkg/features/versioned_kube_features.go index e88426ed0e7..48b2aac3eb6 100644 --- a/pkg/features/versioned_kube_features.go +++ b/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}, + }, } diff --git a/pkg/kubelet/kubelet_node_status.go b/pkg/kubelet/kubelet_node_status.go index ce3b3afa83f..6298144d333 100644 --- a/pkg/kubelet/kubelet_node_status.go +++ b/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 diff --git a/pkg/kubelet/kubelet_node_status_test.go b/pkg/kubelet/kubelet_node_status_test.go index 2cee198ecf7..2df53690a8c 100644 --- a/pkg/kubelet/kubelet_node_status_test.go +++ b/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 - newNode *v1.Node - existingNode *v1.Node - createError error - getError error - patchError error - deleteError error - expectedResult bool - expectedActions int - testSavedNode bool - savedNodeIndex int - savedNodeCMAD bool + name string + newNode *v1.Node + existingNode *v1.Node + createError error + getError error + patchError error + deleteError error + expectedResult bool + expectedActions int + testSavedNode bool + getOnForbiddenDisabled 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 */) - defer testKubelet.Cleanup() - kubelet := testKubelet.kubelet - kubeClient := testKubelet.fakeKubeClient - - 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 + t.Run(tc.name, func(t *testing.T) { + if tc.getOnForbiddenDisabled { + featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, utilversion.MustParse("1.32")) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KubeletRegistrationGetOnExistsOnly, true) } - return notImplemented(action) - }) - kubeClient.AddReactor("delete", "nodes", func(action core.Action) (bool, runtime.Object, error) { - return true, nil, tc.deleteError - }) - addNotImplatedReaction(kubeClient) + testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled is a don't-care for this test */) + defer testKubelet.Cleanup() + kubelet := testKubelet.kubelet + kubeClient := testKubelet.fakeKubeClient - result := kubelet.tryRegisterWithAPIServer(tc.newNode) - require.Equal(t, tc.expectedResult, result, "test [%s]", tc.name) + 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) + }) + kubeClient.AddReactor("delete", "nodes", func(action core.Action) (bool, runtime.Object, error) { + return true, nil, tc.deleteError + }) + addNotImplatedReaction(kubeClient) - actions := kubeClient.Actions() - assert.Len(t, actions, tc.expectedActions, "test [%s]", tc.name) + result := kubelet.tryRegisterWithAPIServer(tc.newNode) + require.Equal(t, tc.expectedResult, result, "test [%s]", tc.name) - if tc.testSavedNode { - var savedNode *v1.Node + actions := kubeClient.Actions() + assert.Len(t, actions, tc.expectedActions, "test [%s]", tc.name) - t.Logf("actions: %v: %+v", len(actions), actions) - action := actions[tc.savedNodeIndex] - if action.GetVerb() == "create" { - createAction := action.(core.CreateAction) - obj := createAction.GetObject() - require.IsType(t, &v1.Node{}, obj) - savedNode = obj.(*v1.Node) - } else if action.GetVerb() == "patch" { - patchAction := action.(core.PatchActionImpl) - var err error - savedNode, err = applyNodeStatusPatch(tc.existingNode, patchAction.GetPatch()) - require.NoError(t, err) + if tc.testSavedNode { + var savedNode *v1.Node + + t.Logf("actions: %v: %+v", len(actions), actions) + action := actions[tc.savedNodeIndex] + if action.GetVerb() == "create" { + createAction := action.(core.CreateAction) + obj := createAction.GetObject() + require.IsType(t, &v1.Node{}, obj) + savedNode = obj.(*v1.Node) + } else if action.GetVerb() == "patch" { + 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) - } + }) } } diff --git a/test/featuregates_linter/test_data/versioned_feature_list.yaml b/test/featuregates_linter/test_data/versioned_feature_list.yaml index 2a5653ac3ba..3461c147927 100644 --- a/test/featuregates_linter/test_data/versioned_feature_list.yaml +++ b/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