Skip to content

Commit

Permalink
Release leader for life lock in case pod is preempted. (#157)
Browse files Browse the repository at this point in the history 
* Release leader for life lock in case pod is preempted.

Signed-off-by: Jagpreet Singh Tamber <jagpreetstamber@gmail.com>

* Review comments

Signed-off-by: Jagpreet Singh Tamber <jagpreetstamber@gmail.com>

* Fix linting errors.

Signed-off-by: Jagpreet Singh Tamber <jagpreetstamber@gmail.com>

---------

Signed-off-by: Jagpreet Singh Tamber <jagpreetstamber@gmail.com>
Co-authored-by: Jagpreet Singh Tamber <jtamber@microsoft.com>
  • Loading branch information
@jagpreetstamber
jagpreetstamber and Jagpreet Singh Tamber committed Apr 1, 2024
1 parent 8e41bd5 commit 380bbcc
Show file tree
Hide file tree
Showing 2 changed files with 190 additions and 0 deletions.
14 changes: 14 additions & 0 deletions leader/leader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down Expand Up @@ -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.
Expand Down
176 changes: 176 additions & 0 deletions leader/leader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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 (
Expand All @@ -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
Expand Down

0 comments on commit 380bbcc

Please sign in to comment.