fix bug and add UTs

pingcap · Apr 27, 2020 · e813f50 · e813f50
1 parent c40197b
commit e813f50
Show file tree

Hide file tree

Showing 2 changed files with 198 additions and 1 deletion.
diff --git a/pkg/manager/member/pd_member_manager.go b/pkg/manager/member/pd_member_manager.go
@@ -274,10 +274,11 @@ func (pmm *pdMemberManager) shouldRecover(tc *v1alpha1.TidbCluster) bool {
 	// healthy, we can perform our failover recovery operation.
 	// Note that failover pods may fail (e.g. lack of resources) and we don't care
 	// about them because we're going to delete them.
-	for _, ordinal := range tc.PDStsDesiredOrdinals(true) {
+	for ordinal := range tc.PDStsDesiredOrdinals(true) {
 		name := fmt.Sprintf("%s-%d", controller.PDMemberName(tc.GetName()), ordinal)
 		pod, err := pmm.podLister.Pods(tc.Namespace).Get(name)
 		if err != nil {
+			klog.Errorf("pod %s/%s does not exist: %v", tc.Namespace, name, err)
 			return false
 		}
 		if !podutil.IsPodReady(pod) {

diff --git a/pkg/manager/member/pd_member_manager_test.go b/pkg/manager/member/pd_member_manager_test.go
@@ -14,6 +14,7 @@
 package member
 
 import (
+	"context"
 	"fmt"
 	"strings"
 	"testing"
@@ -1687,3 +1688,198 @@ func TestPDMemberManagerSyncPDStsWhenPdNotJoinCluster(t *testing.T) {
 		t.Logf("end: %s", tests[i].name)
 	}
 }
+
+func TestPDShouldRecover(t *testing.T) {
+	pods := []*v1.Pod{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "failover-pd-0",
+				Namespace: v1.NamespaceDefault,
+			},
+			Status: v1.PodStatus{
+				Conditions: []v1.PodCondition{
+					{
+						Type:   corev1.PodReady,
+						Status: corev1.ConditionTrue,
+					},
+				},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "failover-pd-1",
+				Namespace: v1.NamespaceDefault,
+			},
+			Status: v1.PodStatus{
+				Conditions: []v1.PodCondition{
+					{
+						Type:   corev1.PodReady,
+						Status: corev1.ConditionTrue,
+					},
+				},
+			},
+		},
+	}
+	podsWithFailover := append(pods, &v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "failover-pd-2",
+			Namespace: v1.NamespaceDefault,
+		},
+		Status: v1.PodStatus{
+			Conditions: []v1.PodCondition{
+				{
+					Type:   corev1.PodReady,
+					Status: corev1.ConditionFalse,
+				},
+			},
+		},
+	})
+	tests := []struct {
+		name string
+		tc   *v1alpha1.TidbCluster
+		pods []*v1.Pod
+		want bool
+	}{
+		{
+			name: "should not recover if no failure members",
+			tc: &v1alpha1.TidbCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "failover",
+					Namespace: v1.NamespaceDefault,
+				},
+				Status: v1alpha1.TidbClusterStatus{},
+			},
+			pods: pods,
+			want: false,
+		},
+		{
+			name: "should not recover if a member is not healthy",
+			tc: &v1alpha1.TidbCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "failover",
+					Namespace: v1.NamespaceDefault,
+				},
+				Spec: v1alpha1.TidbClusterSpec{
+					PD: v1alpha1.PDSpec{
+						Replicas: 2,
+					},
+				},
+				Status: v1alpha1.TidbClusterStatus{
+					PD: v1alpha1.PDStatus{
+						Members: map[string]v1alpha1.PDMember{
+							"failover-pd-0": {
+								Name:   "failover-pd-0",
+								Health: false,
+							},
+							"failover-pd-1": {
+								Name:   "failover-pd-1",
+								Health: true,
+							},
+						},
+						FailureMembers: map[string]v1alpha1.PDFailureMember{
+							"failover-pd-0": {
+								PodName: "failover-pd-0",
+							},
+						},
+					},
+				},
+			},
+			pods: pods,
+			want: false,
+		},
+		{
+			name: "should recover if all members are ready and healthy",
+			tc: &v1alpha1.TidbCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "failover",
+					Namespace: v1.NamespaceDefault,
+				},
+				Spec: v1alpha1.TidbClusterSpec{
+					PD: v1alpha1.PDSpec{
+						Replicas: 2,
+					},
+				},
+				Status: v1alpha1.TidbClusterStatus{
+					PD: v1alpha1.PDStatus{
+						Members: map[string]v1alpha1.PDMember{
+							"failover-pd-0": {
+								Name:   "failover-pd-0",
+								Health: true,
+							},
+							"failover-pd-1": {
+								Name:   "failover-pd-1",
+								Health: true,
+							},
+						},
+						FailureMembers: map[string]v1alpha1.PDFailureMember{
+							"failover-pd-0": {
+								PodName: "failover-pd-0",
+							},
+						},
+					},
+				},
+			},
+			pods: pods,
+			want: true,
+		},
+		{
+			name: "should recover if all members are ready and healthy (ignore auto-created failover pods)",
+			tc: &v1alpha1.TidbCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "failover",
+					Namespace: v1.NamespaceDefault,
+				},
+				Spec: v1alpha1.TidbClusterSpec{
+					PD: v1alpha1.PDSpec{
+						Replicas: 2,
+					},
+				},
+				Status: v1alpha1.TidbClusterStatus{
+					PD: v1alpha1.PDStatus{
+						Members: map[string]v1alpha1.PDMember{
+							"failover-pd-0": {
+								Name:   "failover-pd-0",
+								Health: true,
+							},
+							"failover-pd-1": {
+								Name:   "failover-pd-1",
+								Health: true,
+							},
+							"failover-pd-2": {
+								Name:   "failover-pd-1",
+								Health: false,
+							},
+						},
+						FailureMembers: map[string]v1alpha1.PDFailureMember{
+							"failover-pd-0": {
+								PodName: "failover-pd-0",
+							},
+						},
+					},
+				},
+			},
+			pods: podsWithFailover,
+			want: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+			client := kubefake.NewSimpleClientset()
+			for _, pod := range tt.pods {
+				client.CoreV1().Pods(pod.Namespace).Create(pod)
+			}
+			kubeInformerFactory := kubeinformers.NewSharedInformerFactory(client, 0)
+			podLister := kubeInformerFactory.Core().V1().Pods().Lister()
+			kubeInformerFactory.Start(ctx.Done())
+			kubeInformerFactory.WaitForCacheSync(ctx.Done())
+			pdMemberManager := &pdMemberManager{podLister: podLister}
+			got := pdMemberManager.shouldRecover(tt.tc)
+			if got != tt.want {
+				t.Fatalf("wants %v, got %v", tt.want, got)
+			}
+		})
+	}
+}