fix bug that tikv status is always upgrade (#598)

pingcap · Jun 21, 2019 · fa143ce · fa143ce
1 parent b95df21
commit fa143ce
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 96 deletions.
diff --git a/pkg/manager/member/tikv_member_manager.go b/pkg/manager/member/tikv_member_manager.go
@@ -620,12 +620,7 @@ func tikvStatefulSetIsUpgrading(podLister corelisters.PodLister, pdControl contr
 		}
 	}
 
-	evictLeaderSchedulers, err := pdControl.GetPDClient(tc).GetEvictLeaderSchedulers()
-	if err != nil {
-		return false, err
-	}
-
-	return evictLeaderSchedulers != nil && len(evictLeaderSchedulers) > 0, nil
+	return false, nil
 }
 
 type FakeTiKVMemberManager struct {

diff --git a/pkg/manager/member/tikv_member_manager_test.go b/pkg/manager/member/tikv_member_manager_test.go
@@ -377,18 +377,16 @@ func TestTiKVMemberManagerSyncUpdate(t *testing.T) {
 func TestTiKVMemberManagerTiKVStatefulSetIsUpgrading(t *testing.T) {
 	g := NewGomegaWithT(t)
 	type testcase struct {
-		name             string
-		setUpdate        func(*apps.StatefulSet)
-		hasPod           bool
-		updatePod        func(*corev1.Pod)
-		errWhenGetLeader bool
-		hasLeader        bool
-		errExpectFn      func(*GomegaWithT, error)
-		expectUpgrading  bool
+		name            string
+		setUpdate       func(*apps.StatefulSet)
+		hasPod          bool
+		updatePod       func(*corev1.Pod)
+		errExpectFn     func(*GomegaWithT, error)
+		expectUpgrading bool
 	}
 	testFn := func(test *testcase, t *testing.T) {
 		tc := newTidbClusterForPD()
-		pmm, _, _, pdClient, podIndexer, _ := newFakeTiKVMemberManager(tc)
+		pmm, _, _, _, podIndexer, _ := newFakeTiKVMemberManager(tc)
 		tc.Status.TiKV.StatefulSet = &apps.StatefulSetStatus{
 			UpdateRevision: "v3",
 		}
@@ -417,19 +415,6 @@ func TestTiKVMemberManagerTiKVStatefulSetIsUpgrading(t *testing.T) {
 			}
 			podIndexer.Add(pod)
 		}
-		if test.errWhenGetLeader {
-			pdClient.AddReaction(controller.GetEvictLeaderSchedulersActionType, func(action *controller.Action) (interface{}, error) {
-				return []string{}, fmt.Errorf("failed to get leader")
-			})
-		} else if test.hasLeader {
-			pdClient.AddReaction(controller.GetEvictLeaderSchedulersActionType, func(action *controller.Action) (interface{}, error) {
-				return []string{"leader"}, nil
-			})
-		} else {
-			pdClient.AddReaction(controller.GetEvictLeaderSchedulersActionType, func(action *controller.Action) (interface{}, error) {
-				return []string{}, nil
-			})
-		}
 		b, err := pmm.tikvStatefulSetIsUpgradingFn(pmm.podLister, pmm.pdControl, set, tc)
 		if test.errExpectFn != nil {
 			test.errExpectFn(g, err)
@@ -448,22 +433,18 @@ func TestTiKVMemberManagerTiKVStatefulSetIsUpgrading(t *testing.T) {
 				set.Status.UpdateRevision = "v2"
 				set.Status.ObservedGeneration = func() *int64 { var i int64; i = 1000; return &i }()
 			},
-			hasPod:           false,
-			updatePod:        nil,
-			errWhenGetLeader: false,
-			hasLeader:        false,
-			errExpectFn:      errExpectNil,
-			expectUpgrading:  true,
+			hasPod:          false,
+			updatePod:       nil,
+			errExpectFn:     errExpectNil,
+			expectUpgrading: true,
 		},
 		{
-			name:             "pod don't have revision hash",
-			setUpdate:        nil,
-			hasPod:           true,
-			updatePod:        nil,
-			errWhenGetLeader: false,
-			hasLeader:        false,
-			errExpectFn:      errExpectNil,
-			expectUpgrading:  false,
+			name:            "pod don't have revision hash",
+			setUpdate:       nil,
+			hasPod:          true,
+			updatePod:       nil,
+			errExpectFn:     errExpectNil,
+			expectUpgrading: false,
 		},
 		{
 			name:      "pod have revision hash, not equal statefulset's",
@@ -472,10 +453,8 @@ func TestTiKVMemberManagerTiKVStatefulSetIsUpgrading(t *testing.T) {
 			updatePod: func(pod *corev1.Pod) {
 				pod.Labels[apps.ControllerRevisionHashLabelKey] = "v2"
 			},
-			errWhenGetLeader: false,
-			hasLeader:        false,
-			errExpectFn:      errExpectNil,
-			expectUpgrading:  true,
+			errExpectFn:     errExpectNil,
+			expectUpgrading: true,
 		},
 		{
 			name:      "pod have revision hash, equal statefulset's",
@@ -484,50 +463,9 @@ func TestTiKVMemberManagerTiKVStatefulSetIsUpgrading(t *testing.T) {
 			updatePod: func(pod *corev1.Pod) {
 				pod.Labels[apps.ControllerRevisionHashLabelKey] = "v3"
 			},
-			errWhenGetLeader: false,
-			hasLeader:        false,
-			errExpectFn:      errExpectNil,
-			expectUpgrading:  false,
-		},
-		{
-			name:      "error when get leader schedulers",
-			setUpdate: nil,
-			hasPod:    true,
-			updatePod: func(pod *corev1.Pod) {
-				pod.Labels[apps.ControllerRevisionHashLabelKey] = "v3"
-			},
-			errWhenGetLeader: true,
-			hasLeader:        false,
-			errExpectFn: func(g *GomegaWithT, err error) {
-				g.Expect(err).To(HaveOccurred())
-				g.Expect(strings.Contains(err.Error(), "failed to get leader")).To(Equal(true))
-			},
+			errExpectFn:     errExpectNil,
 			expectUpgrading: false,
 		},
-		{
-			name:      "has no leader",
-			setUpdate: nil,
-			hasPod:    true,
-			updatePod: func(pod *corev1.Pod) {
-				pod.Labels[apps.ControllerRevisionHashLabelKey] = "v3"
-			},
-			errWhenGetLeader: false,
-			hasLeader:        false,
-			errExpectFn:      errExpectNil,
-			expectUpgrading:  false,
-		},
-		{
-			name:      "has leader",
-			setUpdate: nil,
-			hasPod:    true,
-			updatePod: func(pod *corev1.Pod) {
-				pod.Labels[apps.ControllerRevisionHashLabelKey] = "v3"
-			},
-			errWhenGetLeader: false,
-			hasLeader:        true,
-			errExpectFn:      errExpectNil,
-			expectUpgrading:  true,
-		},
 	}
 
 	for i := range tests {

diff --git a/pkg/manager/member/tikv_upgrader.go b/pkg/manager/member/tikv_upgrader.go
@@ -95,10 +95,7 @@ func (tku *tikvUpgrader) Upgrade(tc *v1alpha1.TidbCluster, oldSet *apps.Stateful
 			if store.State != v1alpha1.TiKVStateUp {
 				return controller.RequeueErrorf("tidbcluster: [%s/%s]'s upgraded tikv pod: [%s] is not all ready", ns, tcName, podName)
 			}
-			err := tku.endEvictLeader(tc, i)
-			if err != nil {
-				return err
-			}
+
 			continue
 		}
 
@@ -126,6 +123,10 @@ func (tku *tikvUpgrader) upgradeTiKVPod(tc *v1alpha1.TidbCluster, ordinal int32,
 			_, evicting := upgradePod.Annotations[EvictLeaderBeginTime]
 
 			if tku.readyToUpgrade(upgradePod, store) {
+				err := tku.endEvictLeader(tc, ordinal)
+				if err != nil {
+					return err
+				}
 				setUpgradePartition(newSet, ordinal)
 				return nil
 			}
@@ -193,6 +194,7 @@ func (tku *tikvUpgrader) endEvictLeader(tc *v1alpha1.TidbCluster, ordinal int32)
 	if err != nil {
 		return err
 	}
+
 	return nil
 }
 

diff --git a/pkg/manager/member/tikv_upgrader_test.go b/pkg/manager/member/tikv_upgrader_test.go
@@ -384,7 +384,7 @@ func TestTiKVUpgraderUpgrade(t *testing.T) {
 			},
 			changePods: func(pods []*corev1.Pod) {
 				for _, pod := range pods {
-					if pod.GetName() == tikvPodName(upgradeTcName, 2) {
+					if pod.GetName() == tikvPodName(upgradeTcName, 1) {
 						pod.Annotations = map[string]string{EvictLeaderBeginTime: time.Now().Format(time.RFC3339)}
 					}
 				}
@@ -407,9 +407,6 @@ func TestTiKVUpgraderUpgrade(t *testing.T) {
 				tc.Status.TiKV.Synced = true
 				tc.Status.TiKV.StatefulSet.CurrentReplicas = 2
 				tc.Status.TiKV.StatefulSet.UpdatedReplicas = 1
-				// set leader to 0
-				store := tc.Status.TiKV.Stores["2"]
-				tc.Status.TiKV.Stores["2"] = store
 			},
 			changeOldSet: func(oldSet *apps.StatefulSet) {
 				SetLastAppliedConfigAnnotation(oldSet)