leader: check the node status for the leader-election (#24) (#25)

HyungJune · kasonglee · web-flow · commit d8d1759d7f07 · 2020-10-09T21:49:49.000-04:00
* leader: check the node status for the leader-election (#24) * leader: enhance the coverage Co-authored-by: kasong_lee <kasong_lee@tmax.co.kr>
diff --git a/leader/leader.go b/leader/leader.go
@@ -182,6 +182,14 @@ func Become(ctx context.Context, lockName string, opts ...Option) error {
 					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.")
+
+					//Mark the termainating status to the leaderPod and Delete the configmap lock
+					if err := deleteLeader(ctx, config.Client, leaderPod, existing); err != nil {
+						return err
+					}
 
 				default:
 					log.Info("Not the leader. Waiting.")
@@ -270,3 +278,44 @@ func getPod(ctx context.Context, client crclient.Client, ns string) (*corev1.Pod
 
 	return pod, nil
 }
+
+func getNode(ctx context.Context, client crclient.Client, nodeName string, node *corev1.Node) error {
+	key := crclient.ObjectKey{Namespace: "", Name: nodeName}
+	err := client.Get(ctx, key, node)
+	if err != nil {
+		log.Error(err, "Failed to get Node", "Node.Name", nodeName)
+		return err
+	}
+	return nil
+}
+
+func isNotReadyNode(ctx context.Context, client crclient.Client, nodeName string) bool {
+	leaderNode := &corev1.Node{}
+	if err := getNode(ctx, client, nodeName, leaderNode); err != nil {
+		return false
+	}
+	for _, condition := range leaderNode.Status.Conditions {
+		if condition.Type == corev1.NodeReady && condition.Status != corev1.ConditionTrue {
+			return true
+		}
+	}
+	return false
+
+}
+
+func deleteLeader(ctx context.Context, client crclient.Client, leaderPod *corev1.Pod, existing *corev1.ConfigMap) error {
+	err := client.Delete(ctx, leaderPod)
+	if err != nil {
+		log.Error(err, "Leader pod could not be deleted.")
+		return err
+	}
+	err = client.Delete(ctx, existing)
+	switch {
+	case apierrors.IsNotFound(err):
+		log.Info("ConfigMap has been deleted by prior operator.")
+		return err
+	case err != nil:
+		return err
+	}
+	return nil
+}
diff --git a/leader/leader_test.go b/leader/leader_test.go
@@ -209,4 +209,143 @@ var _ = Describe("Leader election", func() {
 			Expect(pod.TypeMeta.Kind).To(Equal("Pod"))
 		})
 	})
+
+	Describe("getNode", func() {
+		var (
+			client crclient.Client
+		)
+		BeforeEach(func() {
+			client = fake.NewFakeClient(
+				&corev1.Node{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "mynode",
+					},
+				},
+			)
+		})
+		It("should return an error if no node is found", func() {
+			node := corev1.Node{}
+			err := getNode(context.TODO(), client, "", &node)
+			Expect(err).ShouldNot(BeNil())
+		})
+		It("should return the node with the given name", func() {
+			node := corev1.Node{}
+			err := getNode(context.TODO(), client, "mynode", &node)
+			Expect(err).Should(BeNil())
+			Expect(node.TypeMeta.APIVersion).To(Equal("v1"))
+			Expect(node.TypeMeta.Kind).To(Equal("Node"))
+		})
+	})
+
+	Describe("isNotReadyNode", func() {
+		var (
+			nodeName string
+			node     *corev1.Node
+			client   crclient.Client
+		)
+		BeforeEach(func() {
+			nodeName = "mynode"
+			node = &corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: nodeName,
+				},
+				Status: corev1.NodeStatus{
+					Conditions: make([]corev1.NodeCondition, 1),
+				},
+			}
+		})
+
+		It("should return false if node is invalid", func() {
+			client = fake.NewFakeClient()
+			ret := isNotReadyNode(context.TODO(), client, "")
+			Expect(ret).To(Equal(false))
+		})
+		It("should return false if no NodeCondition is found", func() {
+			client = fake.NewFakeClient(node)
+			ret := isNotReadyNode(context.TODO(), client, nodeName)
+			Expect(ret).To(Equal(false))
+		})
+		It("should return false if type is incorrect", func() {
+			node.Status.Conditions[0].Type = corev1.NodeMemoryPressure
+			node.Status.Conditions[0].Status = corev1.ConditionFalse
+			client = fake.NewFakeClient(node)
+			ret := isNotReadyNode(context.TODO(), client, nodeName)
+			Expect(ret).To(Equal(false))
+		})
+		It("should return false if NodeReady's type is true", func() {
+			node.Status.Conditions[0].Type = corev1.NodeReady
+			node.Status.Conditions[0].Status = corev1.ConditionTrue
+			client = fake.NewFakeClient(node)
+			ret := isNotReadyNode(context.TODO(), client, nodeName)
+			Expect(ret).To(Equal(false))
+		})
+		It("should return true when Type is set and Status is set to false", func() {
+			node.Status.Conditions[0].Type = corev1.NodeReady
+			node.Status.Conditions[0].Status = corev1.ConditionFalse
+			client = fake.NewFakeClient(node)
+			ret := isNotReadyNode(context.TODO(), client, nodeName)
+			Expect(ret).To(Equal(true))
+		})
+	})
+	Describe("deleteLeader", func() {
+		var (
+			configmap *corev1.ConfigMap
+			pod       *corev1.Pod
+			client    crclient.Client
+		)
+		BeforeEach(func() {
+			pod = &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "leader-test",
+					Namespace: "testns",
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion: "v1",
+							Kind:       "Pod",
+							Name:       "leader-test",
+						},
+					},
+				},
+			}
+			configmap = &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "leader-test",
+					Namespace: "testns",
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion: "v1",
+							Kind:       "Pod",
+							Name:       "leader-test",
+						},
+					},
+				},
+			}
+		})
+		It("should return an error if existing is not found", func() {
+			client = fake.NewFakeClient(pod)
+			err := deleteLeader(context.TODO(), client, pod, configmap)
+			Expect(err).ShouldNot(BeNil())
+		})
+		It("should return an error if pod is not found", func() {
+			client = fake.NewFakeClient(configmap)
+			err := deleteLeader(context.TODO(), client, pod, configmap)
+			Expect(err).ShouldNot(BeNil())
+		})
+		It("should return an error if pod is nil", func() {
+			client = fake.NewFakeClient(pod, configmap)
+			err := deleteLeader(context.TODO(), client, nil, configmap)
+			Expect(err).ShouldNot(BeNil())
+		})
+		It("should return an error if configmap is nil", func() {
+			client = fake.NewFakeClient(pod, configmap)
+			err := deleteLeader(context.TODO(), client, pod, nil)
+			Expect(err).ShouldNot(BeNil())
+		})
+		It("should return nil if pod and configmap exists and configmap's owner is the pod", func() {
+			client = fake.NewFakeClient(pod, configmap)
+			err := deleteLeader(context.TODO(), client, pod, configmap)
+			Expect(err).Should(BeNil())
+		})
+
+	})
 })
