From 380bbcceaceee9c4d9763e4b6cdc1a4367235636 Mon Sep 17 00:00:00 2001 From: Jagpreet Singh Tamber Date: Mon, 1 Apr 2024 13:15:18 -0400 Subject: [PATCH] Release leader for life lock in case pod is preempted. (#157) * Release leader for life lock in case pod is preempted. Signed-off-by: Jagpreet Singh Tamber * Review comments Signed-off-by: Jagpreet Singh Tamber * Fix linting errors. Signed-off-by: Jagpreet Singh Tamber --------- Signed-off-by: Jagpreet Singh Tamber Co-authored-by: Jagpreet Singh Tamber --- leader/leader.go | 14 ++++ leader/leader_test.go | 176 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 190 insertions(+) diff --git a/leader/leader.go b/leader/leader.go index 16d497c..7623fe2 100644 --- a/leader/leader.go +++ b/leader/leader.go @@ -187,6 +187,14 @@ func Become(ctx context.Context, lockName string, opts ...Option) error { if err != nil { log.Error(err, "Leader pod could not be deleted.") } + case isPodPreempted(*leaderPod) && leaderPod.GetDeletionTimestamp() == nil: + log.Info("Operator pod with leader lock has been preempted.", "leader", leaderPod.Name) + log.Info("Deleting preempted leader.") + // Pod may not delete immediately, continue with backoff + err := config.Client.Delete(ctx, leaderPod) + if err != nil { + log.Error(err, "Leader pod could not be deleted.") + } case isNotReadyNode(ctx, config.Client, leaderPod.Spec.NodeName): log.Info("the status of the node where operator pod with leader lock was running has been 'notReady'") log.Info("Deleting the leader.") @@ -241,6 +249,12 @@ func isPodEvicted(pod corev1.Pod) bool { return podFailed && podEvicted } +func isPodPreempted(pod corev1.Pod) bool { + podFailed := pod.Status.Phase == corev1.PodFailed + podPreempted := pod.Status.Reason == "Preempting" + return podFailed && podPreempted +} + // getPod returns a Pod object that corresponds to the pod in which the code // is currently running. // It expects the environment variable POD_NAME to be set by the downwards API. diff --git a/leader/leader_test.go b/leader/leader_test.go index 4798795..5817211 100644 --- a/leader/leader_test.go +++ b/leader/leader_test.go @@ -24,6 +24,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" crclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" ) var _ = Describe("Leader election", func() { @@ -82,6 +83,156 @@ var _ = Describe("Leader election", func() { Expect(Become(context.TODO(), "leader-test", WithClient(client))).To(Succeed()) }) + It("should become leader when pod is evicted and rescheduled", func() { + + evictedPodStatusClient := fake.NewClientBuilder().WithObjects( + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "leader-test-new", + Namespace: "testns", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Pod", + Name: "leader-test-new", + }, + }, + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "leader-test", + Namespace: "testns", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Pod", + Name: "leader-test", + }, + }, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodFailed, + Reason: "Evicted", + }, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "leader-test", + Namespace: "testns", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Pod", + Name: "leader-test", + }, + }, + }, + }, + ).WithInterceptorFuncs( + interceptor.Funcs{ + // Mock garbage collection of the ConfigMap when the Pod is deleted. + Delete: func(ctx context.Context, client crclient.WithWatch, obj crclient.Object, opts ...crclient.DeleteOption) error { + if obj.GetObjectKind() != nil && obj.GetObjectKind().GroupVersionKind().Kind == "Pod" && obj.GetName() == "leader-test" { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "leader-test", + Namespace: "testns", + }, + } + + err := client.Delete(ctx, cm) + if err != nil { + return err + } + } + return nil + }, + }, + ).Build() + + os.Setenv("POD_NAME", "leader-test-new") + readNamespace = func() (string, error) { + return "testns", nil + } + + Expect(Become(context.TODO(), "leader-test", WithClient(evictedPodStatusClient))).To(Succeed()) + }) + It("should become leader when pod is preempted and rescheduled", func() { + + preemptedPodStatusClient := fake.NewClientBuilder().WithObjects( + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "leader-test-new", + Namespace: "testns", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Pod", + Name: "leader-test-new", + }, + }, + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "leader-test", + Namespace: "testns", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Pod", + Name: "leader-test", + }, + }, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodFailed, + Reason: "Preempting", + }, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "leader-test", + Namespace: "testns", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Pod", + Name: "leader-test", + }, + }, + }, + }, + ).WithInterceptorFuncs( + interceptor.Funcs{ + // Mock garbage collection of the ConfigMap when the Pod is deleted. + Delete: func(ctx context.Context, client crclient.WithWatch, obj crclient.Object, opts ...crclient.DeleteOption) error { + if obj.GetObjectKind() != nil && obj.GetObjectKind().GroupVersionKind().Kind == "Pod" && obj.GetName() == "leader-test" { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "leader-test", + Namespace: "testns", + }, + } + + err := client.Delete(ctx, cm) + if err != nil { + return err + } + } + return nil + }, + }, + ).Build() + + os.Setenv("POD_NAME", "leader-test-new") + readNamespace = func() (string, error) { + return "testns", nil + } + + Expect(Become(context.TODO(), "leader-test", WithClient(preemptedPodStatusClient))).To(Succeed()) + }) }) Describe("isPodEvicted", func() { var ( @@ -108,6 +259,31 @@ var _ = Describe("Leader election", func() { Expect(isPodEvicted(*leaderPod)).To(BeTrue()) }) }) + Describe("isPodPreempted", func() { + var ( + leaderPod *corev1.Pod + ) + BeforeEach(func() { + leaderPod = &corev1.Pod{} + }) + It("should return false with an empty status", func() { + Expect(isPodPreempted(*leaderPod)).To(BeFalse()) + }) + It("should return false if reason is incorrect", func() { + leaderPod.Status.Phase = corev1.PodFailed + leaderPod.Status.Reason = "invalid" + Expect(isPodPreempted(*leaderPod)).To(BeFalse()) + }) + It("should return false if pod is in the wrong phase", func() { + leaderPod.Status.Phase = corev1.PodRunning + Expect(isPodPreempted(*leaderPod)).To(BeFalse()) + }) + It("should return true when Phase and Reason are set", func() { + leaderPod.Status.Phase = corev1.PodFailed + leaderPod.Status.Reason = "Preempting" + Expect(isPodPreempted(*leaderPod)).To(BeTrue()) + }) + }) Describe("myOwnerRef", func() { var ( client crclient.Client