From dd09b2d65869c23efc254a6411139c5f09f32819 Mon Sep 17 00:00:00 2001 From: Jonathan Innis Date: Tue, 6 Aug 2024 16:34:49 -0700 Subject: [PATCH] chore: Mark the status condition as false when unregistered taint isn't present and block initialized (#1517) --- .../nodeclaim/lifecycle/registration.go | 1 + .../nodeclaim/lifecycle/registration_test.go | 2 ++ pkg/scheduling/taints.go | 23 +++++++++++-------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pkg/controllers/nodeclaim/lifecycle/registration.go b/pkg/controllers/nodeclaim/lifecycle/registration.go index da26e33374..0fb7ba6b28 100644 --- a/pkg/controllers/nodeclaim/lifecycle/registration.go +++ b/pkg/controllers/nodeclaim/lifecycle/registration.go @@ -64,6 +64,7 @@ func (r *Registration) Reconcile(ctx context.Context, nodeClaim *v1.NodeClaim) ( // check if sync succeeded but setting the registered status condition failed // if sync succeeded, then the label will be present and the taint will be gone if _, ok := node.Labels[v1.NodeRegisteredLabelKey]; !ok && !hasStartupTaint { + nodeClaim.StatusConditions().SetFalse(v1.ConditionTypeRegistered, "UnregisteredTaintNotFound", fmt.Sprintf("Invariant violated, %s taint must be present on Karpenter-managed nodes", v1.UnregisteredTaintKey)) return reconcile.Result{}, fmt.Errorf("missing required startup taint, %s", v1.UnregisteredTaintKey) } ctx = log.IntoContext(ctx, log.FromContext(ctx).WithValues("Node", klog.KRef("", node.Name))) diff --git a/pkg/controllers/nodeclaim/lifecycle/registration_test.go b/pkg/controllers/nodeclaim/lifecycle/registration_test.go index d02ebd2af0..f5812e381d 100644 --- a/pkg/controllers/nodeclaim/lifecycle/registration_test.go +++ b/pkg/controllers/nodeclaim/lifecycle/registration_test.go @@ -107,6 +107,8 @@ var _ = Describe("Registration", func() { node := test.Node(test.NodeOptions{ProviderID: nodeClaim.Status.ProviderID}) ExpectApplied(ctx, env.Client, node) _ = ExpectObjectReconcileFailed(ctx, env.Client, nodeClaimController, nodeClaim) + nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) + Expect(ExpectStatusConditionExists(nodeClaim, v1.ConditionTypeRegistered).Status).To(Equal(metav1.ConditionFalse)) }) It("should sync the labels to the Node when the Node comes online", func() { nodeClaim := test.NodeClaim(v1.NodeClaim{ diff --git a/pkg/scheduling/taints.go b/pkg/scheduling/taints.go index 3a9b94cc6c..102d9ce627 100644 --- a/pkg/scheduling/taints.go +++ b/pkg/scheduling/taints.go @@ -21,24 +21,27 @@ import ( "github.com/samber/lo" "go.uber.org/multierr" - v1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" cloudproviderapi "k8s.io/cloud-provider/api" + + v1 "sigs.k8s.io/karpenter/pkg/apis/v1" ) // KnownEphemeralTaints are taints that are expected to be added to a node while it's initializing // If the node is a Karpenter-managed node, we don't consider these taints while the node is uninitialized // since we expect these taints to eventually be removed -var KnownEphemeralTaints = []v1.Taint{ - {Key: v1.TaintNodeNotReady, Effect: v1.TaintEffectNoSchedule}, - {Key: v1.TaintNodeUnreachable, Effect: v1.TaintEffectNoSchedule}, - {Key: cloudproviderapi.TaintExternalCloudProvider, Effect: v1.TaintEffectNoSchedule, Value: "true"}, +var KnownEphemeralTaints = []corev1.Taint{ + {Key: corev1.TaintNodeNotReady, Effect: corev1.TaintEffectNoSchedule}, + {Key: corev1.TaintNodeUnreachable, Effect: corev1.TaintEffectNoSchedule}, + {Key: cloudproviderapi.TaintExternalCloudProvider, Effect: corev1.TaintEffectNoSchedule, Value: "true"}, + v1.UnregisteredNoExecuteTaint, } -// Taints is a decorated alias type for []v1.Taint -type Taints []v1.Taint +// Taints is a decorated alias type for []corev1.Taint +type Taints []corev1.Taint // Tolerates returns true if the pod tolerates all taints. -func (ts Taints) Tolerates(pod *v1.Pod) (errs error) { +func (ts Taints) Tolerates(pod *corev1.Pod) (errs error) { for i := range ts { taint := ts[i] tolerates := false @@ -54,11 +57,11 @@ func (ts Taints) Tolerates(pod *v1.Pod) (errs error) { // Merge merges in taints with the passed in taints. func (ts Taints) Merge(with Taints) Taints { - res := lo.Map(ts, func(t v1.Taint, _ int) v1.Taint { + res := lo.Map(ts, func(t corev1.Taint, _ int) corev1.Taint { return t }) for _, taint := range with { - if _, ok := lo.Find(res, func(t v1.Taint) bool { + if _, ok := lo.Find(res, func(t corev1.Taint) bool { return taint.MatchTaint(&t) }); !ok { res = append(res, taint)