Merge pull request #126318 from hoskeri/node-self-register-continue-on-forbidden

node self-register continue if CREATE is 403.
This commit is contained in:
Kubernetes Prow Robot 2024-09-10 00:30:35 +01:00 committed by GitHub
commit 36fafafdb0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 126 additions and 56 deletions

View File

@ -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)

View File

@ -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},
},
}

View File

@ -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

View File

@ -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)
}
})
}
}

View File

@ -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