fixup

pkg/controllers/disruption/suite_test.go

@@ -1334,7 +1334,7 @@ var _ = Describe("Candidate Filtering", func() {
 		Expect(c.NodeClaim).ToNot(BeNil())
 		Expect(c.Node).ToNot(BeNil())
 	})
-	It("should consider candidates that have fully blocking PDBs with a terminationGracePeriod set for eventual disruption", func() {
+	It("should not consider candidates that have do-not-disrupt pods without a terminationGracePeriod set for graceful disruption", func() {
 		nodeClaim, node := test.NodeClaimAndNode(v1.NodeClaim{
 			ObjectMeta: metav1.ObjectMeta{
 				Labels: map[string]string{
@@ -1345,53 +1345,14 @@ var _ = Describe("Candidate Filtering", func() {
 				},
 			},
 		})
-		nodeClaim.Spec.TerminationGracePeriod = &metav1.Duration{Duration: time.Second * 300}
-		podLabels := map[string]string{"test": "value"}
 		pod := test.Pod(test.PodOptions{
 			ObjectMeta: metav1.ObjectMeta{
-				Labels: podLabels,
-			},
-		})
-		budget := test.PodDisruptionBudget(test.PDBOptions{
-			Labels:         podLabels,
-			MaxUnavailable: fromInt(0),
-		})
-		ExpectApplied(ctx, env.Client, nodePool, nodeClaim, node, pod, budget)
-		ExpectManualBinding(ctx, env.Client, pod, node)
-		ExpectMakeNodesAndNodeClaimsInitializedAndStateUpdated(ctx, env.Client, nodeStateController, nodeClaimStateController, []*corev1.Node{node}, []*v1.NodeClaim{nodeClaim})
-		var err error
-		pdbLimits, err = pdb.NewLimits(ctx, fakeClock, env.Client)
-		Expect(err).ToNot(HaveOccurred())
-
-		Expect(cluster.Nodes()).To(HaveLen(1))
-		c, err := disruption.NewCandidate(ctx, env.Client, recorder, fakeClock, cluster.Nodes()[0], pdbLimits, nodePoolMap, nodePoolInstanceTypeMap, queue, disruption.EventualDisruptionClass)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(c.NodeClaim).ToNot(BeNil())
-		Expect(c.Node).ToNot(BeNil())
-	})
-	It("should not consider candidates that have fully blocking PDBs with a terminationGracePeriod set for graceful disruption", func() {
-		nodeClaim, node := test.NodeClaimAndNode(v1.NodeClaim{
-			ObjectMeta: metav1.ObjectMeta{
-				Labels: map[string]string{
-					v1.NodePoolLabelKey:            nodePool.Name,
-					corev1.LabelInstanceTypeStable: mostExpensiveInstance.Name,
-					v1.CapacityTypeLabelKey:        mostExpensiveOffering.Requirements.Get(v1.CapacityTypeLabelKey).Any(),
-					corev1.LabelTopologyZone:       mostExpensiveOffering.Requirements.Get(corev1.LabelTopologyZone).Any(),
+				Annotations: map[string]string{
+					v1.DoNotDisruptAnnotationKey: "true",
 				},
 			},
 		})
-		nodeClaim.Spec.TerminationGracePeriod = &metav1.Duration{Duration: time.Second * 300}
-		podLabels := map[string]string{"test": "value"}
-		pod := test.Pod(test.PodOptions{
-			ObjectMeta: metav1.ObjectMeta{
-				Labels: podLabels,
-			},
-		})
-		budget := test.PodDisruptionBudget(test.PDBOptions{
-			Labels:         podLabels,
-			MaxUnavailable: fromInt(0),
-		})
-		ExpectApplied(ctx, env.Client, nodePool, nodeClaim, node, pod, budget)
+		ExpectApplied(ctx, env.Client, nodePool, nodeClaim, node, pod)
 		ExpectManualBinding(ctx, env.Client, pod, node)
 		ExpectMakeNodesAndNodeClaimsInitializedAndStateUpdated(ctx, env.Client, nodeStateController, nodeClaimStateController, []*corev1.Node{node}, []*v1.NodeClaim{nodeClaim})
 		var err error
@@ -1401,10 +1362,10 @@ var _ = Describe("Candidate Filtering", func() {
 		Expect(cluster.Nodes()).To(HaveLen(1))
 		_, err = disruption.NewCandidate(ctx, env.Client, recorder, fakeClock, cluster.Nodes()[0], pdbLimits, nodePoolMap, nodePoolInstanceTypeMap, queue, disruption.GracefulDisruptionClass)
 		Expect(err).To(HaveOccurred())
-		Expect(err.Error()).To(Equal(fmt.Sprintf(`pdb %q prevents pod evictions`, client.ObjectKeyFromObject(budget))))
-		Expect(recorder.DetectedEvent(fmt.Sprintf(`Cannot disrupt Node: pdb %q prevents pod evictions`, client.ObjectKeyFromObject(budget)))).To(BeTrue())
+		Expect(err.Error()).To(Equal(fmt.Sprintf(`pod %q has "karpenter.sh/do-not-disrupt" annotation`, client.ObjectKeyFromObject(pod))))
+		Expect(recorder.DetectedEvent(fmt.Sprintf(`Cannot disrupt Node: pod %q has "karpenter.sh/do-not-disrupt" annotation`, client.ObjectKeyFromObject(pod)))).To(BeTrue())
 	})
-	It("should not consider candidates that have fully blocking PDBs without a terminationGracePeriod set for eventual disruption", func() {
+	It("should not consider candidates that have fully blocking PDBs without a terminationGracePeriod set for graceful disruption", func() {
 		nodeClaim, node := test.NodeClaimAndNode(v1.NodeClaim{
 			ObjectMeta: metav1.ObjectMeta{
 				Labels: map[string]string{

pkg/controllers/node/termination/suite_test.go

@@ -732,7 +732,7 @@ var _ = Describe("Termination", func() {
 			ExpectNodeExists(ctx, env.Client, node.Name)
 			ExpectDeleted(ctx, env.Client, pod)
 		})
-		It("should not delete pods if their terminationGracePeriodSeconds is less than the the node's remaining terminationGracePeriod", func() {
+		It("should only delete pods when their terminationGracePeriodSeconds is less than the the node's remaining terminationGracePeriod", func() {
 			nodeClaim.Spec.TerminationGracePeriod = &metav1.Duration{Duration: time.Second * 300}
 			node.Annotations = map[string]string{
 				v1.NodeTerminationTimestampAnnotationKey: time.Now().Add(nodeClaim.Spec.TerminationGracePeriod.Duration).Format(time.RFC3339),

pkg/controllers/node/termination/terminator/terminator.go

@@ -55,11 +55,11 @@ func NewTerminator(clk clock.Clock, kubeClient client.Client, eq *Queue, recorde
 // Taint idempotently adds a given taint to a node with a NodeClaim
 func (t *Terminator) Taint(ctx context.Context, node *corev1.Node, taint corev1.Taint) error {
 	stored := node.DeepCopy()
-	// If the node already has the correct taint (key, value, and effect), do nothing.
+	// If the node already has the correct taint (key and effect), do nothing.
 	if _, ok := lo.Find(node.Spec.Taints, func(t corev1.Taint) bool {
 		return t.MatchTaint(&taint)
 	}); !ok {
-		// Otherwise, if the taint key exists (but with a different value or effect), remove it.
+		// Otherwise, if the taint key exists (but with a different effect), remove it.
 		node.Spec.Taints = lo.Reject(node.Spec.Taints, func(t corev1.Taint, _ int) bool {
 			return t.Key == taint.Key
 		})
@@ -101,7 +101,7 @@ func (t *Terminator) Drain(ctx context.Context, node *corev1.Node, nodeGracePeri
 		return podutil.IsWaitingEviction(p, t.clock) && !podutil.IsTerminating(p)
 	})
 	if err := t.DeleteExpiringPods(ctx, podsToDelete, nodeGracePeriodExpirationTime); err != nil {
-		return err
+		return fmt.Errorf("deleting expiring pods, %w", err)
 	}
 
 	// evictablePods are pods that aren't yet terminating are eligible to have the eviction API called against them
@@ -169,7 +169,7 @@ func (t *Terminator) DeleteExpiringPods(ctx context.Context, pods []*corev1.Pod,
 				GracePeriodSeconds: gracePeriodSeconds,
 			}
 			if err := t.kubeClient.Delete(ctx, pod, opts); err != nil && !apierrors.IsNotFound(err) { // ignore 404, not a problem
-				return err // otherwise, bubble up the error
+				return fmt.Errorf("deleting pod, %w", err) // otherwise, bubble up the error
 			}
 			log.FromContext(ctx).WithValues(
 				"namespace", pod.Namespace,

pkg/controllers/nodeclaim/termination/controller.go

@@ -93,7 +93,7 @@ func (c *Controller) finalize(ctx context.Context, nodeClaim *v1.NodeClaim) (rec
 	for _, node := range nodes {
 		err = c.ensureTerminationGracePeriodTerminationTimeAnnotation(ctx, node, nodeClaim)
 		if err != nil {
-			return reconcile.Result{}, err
+			return reconcile.Result{}, fmt.Errorf("adding nodeclaim terminationGracePeriod annotation, %w", err)
 		}
 
 		// If we still get the Node, but it's already marked as terminating, we don't need to call Delete again