Skip to content

Commit

Permalink
fix: don't mark pods with 'karpenter.sh/do-not-disrupt=true' as resch…
Browse files Browse the repository at this point in the history
…edulable
  • Loading branch information
saurav-agarwalla committed Feb 26, 2025
1 parent 82a7d80 commit 9117ad4
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 5 deletions.
8 changes: 4 additions & 4 deletions pkg/controllers/disruption/drift_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,8 +553,7 @@ var _ = Describe("Drift", func() {
Expect(ExpectNodes(ctx, env.Client)).To(HaveLen(1))
ExpectExists(ctx, env.Client, nodeClaim)
})
It("should drift nodes that have pods with the karpenter.sh/do-not-disrupt annotation when the NodePool's TerminationGracePeriod is not nil", func() {
nodeClaim.Spec.TerminationGracePeriod = &metav1.Duration{Duration: time.Second * 300}
It("should ignore nodes that have pods with the karpenter.sh/do-not-disrupt annotation when the NodePool's TerminationGracePeriod is not nil", func() {
pod := test.Pod(test.PodOptions{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
Expand All @@ -570,8 +569,9 @@ var _ = Describe("Drift", func() {

ExpectSingletonReconciled(ctx, disruptionController)

// Expect to create a replacement but not delete the old nodeclaim
Expect(ExpectNodeClaims(ctx, env.Client)).To(HaveLen(2)) // new nodeclaim is created for drift
// Pods with `karpenter.sh/do-not-disrupt` can't be evicted and hence can't be rescheduled on a new node.
// Expect no new nodeclaims to be created.
Expect(ExpectNodeClaims(ctx, env.Client)).To(HaveLen(1))
Expect(ExpectNodes(ctx, env.Client)).To(HaveLen(1))
ExpectExists(ctx, env.Client, nodeClaim)
})
Expand Down
4 changes: 3 additions & 1 deletion pkg/utils/pod/scheduling.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@ func IsActive(pod *corev1.Pod) bool {
// - Is an active pod (isn't terminal or actively terminating) OR Is owned by a StatefulSet and Is Terminating
// - Isn't owned by a DaemonSet
// - Isn't a mirror pod (https://kubernetes.io/docs/tasks/configure-pod-container/static-pod/)
// - Does not have the "karpenter.sh/do-not-disrupt=true" annotation (https://karpenter.sh/docs/concepts/disruption/#pod-level-controls)
func IsReschedulable(pod *corev1.Pod) bool {
// StatefulSet pods can be handled differently here because we know that StatefulSet pods MUST
// get deleted before new pods are re-created. This means that we can model terminating pods for StatefulSets
// differently for higher availability by considering terminating pods for scheduling
return (IsActive(pod) || (IsOwnedByStatefulSet(pod) && IsTerminating(pod))) &&
!IsOwnedByDaemonSet(pod) &&
!IsOwnedByNode(pod)
!IsOwnedByNode(pod) &&
!HasDoNotDisrupt(pod)
}

// IsEvictable checks if a pod is evictable by Karpenter by ensuring that the pod:
Expand Down

0 comments on commit 9117ad4

Please sign in to comment.