Skip to content

Commit

Permalink
don't failover if TiDB pod is not scheduled and perform recovery no m…
Browse files Browse the repository at this point in the history 
…atter what state failover pods are in (#2263) (#2358)

* skip auto failover if pod is not scheduled

* fix tidb recover

* share failover period in tests

Co-authored-by: Yecheng Fu <fuyecheng@pingcap.com>
  • Loading branch information
@sre-bot @cofyc
sre-bot and cofyc authored Apr 30, 2020
1 parent 9d383ac commit 8a7e46b
Show file tree
Hide file tree
Showing 8 changed files with 648 additions and 69 deletions.
2 changes: 1 addition & 1 deletion pkg/controller/tidbcluster/tidb_cluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func NewController(
tiflashScaler := mm.NewTiFlashScaler(pdControl, pvcInformer.Lister(), pvcControl, podInformer.Lister())
pdFailover := mm.NewPDFailover(cli, pdControl, pdFailoverPeriod, podInformer.Lister(), podControl, pvcInformer.Lister(), pvcControl, pvInformer.Lister(), recorder)
tikvFailover := mm.NewTiKVFailover(tikvFailoverPeriod, recorder)
tidbFailover := mm.NewTiDBFailover(tidbFailoverPeriod, recorder)
tidbFailover := mm.NewTiDBFailover(tidbFailoverPeriod, recorder, podInformer.Lister())
tiflashFailover := mm.NewTiFlashFailover(tiflashFailoverPeriod, recorder)
pdUpgrader := mm.NewPDUpgrader(pdControl, podControl, podInformer.Lister())
tikvUpgrader := mm.NewTiKVUpgrader(pdControl, podControl, podInformer.Lister())
Expand Down
52 changes: 35 additions & 17 deletions pkg/manager/member/tidb_failover.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,24 @@ import (
"github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
corelisters "k8s.io/client-go/listers/core/v1"
"k8s.io/client-go/tools/record"
"k8s.io/klog"
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
)

type tidbFailover struct {
tidbFailoverPeriod time.Duration
recorder record.EventRecorder
podLister corelisters.PodLister
}

// NewTiDBFailover returns a tidbFailover instance
func NewTiDBFailover(failoverPeriod time.Duration, recorder record.EventRecorder) Failover {
func NewTiDBFailover(failoverPeriod time.Duration, recorder record.EventRecorder, podLister corelisters.PodLister) Failover {
return &tidbFailover{
tidbFailoverPeriod: failoverPeriod,
recorder: recorder,
podLister: podLister,
}
}

Expand All @@ -50,24 +54,38 @@ func (tf *tidbFailover) Failover(tc *v1alpha1.TidbCluster) error {
}
}

if tc.Spec.TiDB.MaxFailoverCount != nil && *tc.Spec.TiDB.MaxFailoverCount > 0 {
maxFailoverCount := *tc.Spec.TiDB.MaxFailoverCount
if len(tc.Status.TiDB.FailureMembers) >= int(maxFailoverCount) {
klog.Warningf("the failure members count reached the limit:%d", tc.Spec.TiDB.MaxFailoverCount)
return nil
}
for _, tidbMember := range tc.Status.TiDB.Members {
_, exist := tc.Status.TiDB.FailureMembers[tidbMember.Name]
deadline := tidbMember.LastTransitionTime.Add(tf.tidbFailoverPeriod)
if !tidbMember.Health && time.Now().After(deadline) && !exist {
tc.Status.TiDB.FailureMembers[tidbMember.Name] = v1alpha1.TiDBFailureMember{
PodName: tidbMember.Name,
CreatedAt: metav1.Now(),
}
msg := fmt.Sprintf("tidb[%s] is unhealthy", tidbMember.Name)
tf.recorder.Event(tc, corev1.EventTypeWarning, unHealthEventReason, fmt.Sprintf(unHealthEventMsgPattern, "tidb", tidbMember.Name, msg))
if tc.Spec.TiDB.MaxFailoverCount == nil || *tc.Spec.TiDB.MaxFailoverCount <= 0 {
klog.Infof("tidb failover is disabled for %s/%s, skipped", tc.Namespace, tc.Name)
return nil
}

maxFailoverCount := *tc.Spec.TiDB.MaxFailoverCount
for _, tidbMember := range tc.Status.TiDB.Members {
_, exist := tc.Status.TiDB.FailureMembers[tidbMember.Name]
deadline := tidbMember.LastTransitionTime.Add(tf.tidbFailoverPeriod)
if !tidbMember.Health && time.Now().After(deadline) && !exist {
if len(tc.Status.TiDB.FailureMembers) >= int(maxFailoverCount) {
klog.Warningf("the failover count reachs the limit (%d), no more failover pods will be created", maxFailoverCount)
break
}
pod, err := tf.podLister.Pods(tc.Namespace).Get(tidbMember.Name)
if err != nil {
return err
}
_, condition := podutil.GetPodCondition(&pod.Status, corev1.PodScheduled)
if condition == nil || condition.Status != corev1.ConditionTrue {
// if a member is unheathy because it's not scheduled yet, we
// should not create failover pod for it
klog.Warningf("pod %s/%s is not scheduled yet, skipping failover", pod.Namespace, pod.Name)
continue
}
tc.Status.TiDB.FailureMembers[tidbMember.Name] = v1alpha1.TiDBFailureMember{
PodName: tidbMember.Name,
CreatedAt: metav1.Now(),
}
msg := fmt.Sprintf("tidb[%s] is unhealthy", tidbMember.Name)
tf.recorder.Event(tc, corev1.EventTypeWarning, unHealthEventReason, fmt.Sprintf(unHealthEventMsgPattern, "tidb", tidbMember.Name, msg))
break
}
}

Expand Down
196 changes: 157 additions & 39 deletions pkg/manager/member/tidb_failover_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,40 +14,31 @@
package member

import (
"context"
"testing"
"time"

. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/types"
kubeinformers "k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/record"
"k8s.io/utils/pointer"

"github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
)

func TestFakeTiDBFailoverFailover(t *testing.T) {
type testcase struct {
func TestTiDBFailoverFailover(t *testing.T) {
tests := []struct {
name string
pods []*corev1.Pod
update func(*v1alpha1.TidbCluster)
errExpectFn func(*GomegaWithT, error)
expectFn func(*GomegaWithT, *v1alpha1.TidbCluster)
}

testFn := func(test *testcase, t *testing.T) {
t.Logf(test.name)
g := NewGomegaWithT(t)
tidbFailover := newTiDBFailover()
tc := newTidbClusterForTiDBFailover()
test.update(tc)

err := tidbFailover.Failover(tc)
test.errExpectFn(g, err)
test.expectFn(g, tc)
}

tests := []testcase{
}{
{
name: "all tidb members are ready",
update: func(tc *v1alpha1.TidbCluster) {
Expand All @@ -72,6 +63,36 @@ func TestFakeTiDBFailoverFailover(t *testing.T) {
},
{
name: "one tidb member failed",
pods: []*corev1.Pod{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Name: "failover-tidb-0",
},
Status: corev1.PodStatus{
Conditions: []corev1.PodCondition{
{
Type: corev1.PodScheduled,
Status: corev1.ConditionTrue,
},
},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Name: "failover-tidb-1",
},
Status: corev1.PodStatus{
Conditions: []corev1.PodCondition{
{
Type: corev1.PodScheduled,
Status: corev1.ConditionTrue,
},
},
},
},
},
update: func(tc *v1alpha1.TidbCluster) {
tc.Status.TiDB.Members = map[string]v1alpha1.TiDBMember{
"failover-tidb-0": {
Expand All @@ -92,8 +113,90 @@ func TestFakeTiDBFailoverFailover(t *testing.T) {
t.Expect(int(tc.Spec.TiDB.Replicas)).To(Equal(2))
},
},
{
name: "one tidb member failed but not scheduled yet",
pods: []*corev1.Pod{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Name: "failover-tidb-0",
},
Status: corev1.PodStatus{
Conditions: []corev1.PodCondition{
{
Type: corev1.PodScheduled,
Status: corev1.ConditionUnknown,
},
},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Name: "failover-tidb-1",
},
Status: corev1.PodStatus{
Conditions: []corev1.PodCondition{
{
Type: corev1.PodScheduled,
Status: corev1.ConditionTrue,
},
},
},
},
},
update: func(tc *v1alpha1.TidbCluster) {
tc.Status.TiDB.Members = map[string]v1alpha1.TiDBMember{
"failover-tidb-0": {
Name: "failover-tidb-0",
Health: false,
},
"failover-tidb-1": {
Name: "failover-tidb-1",
Health: true,
},
}
},
errExpectFn: func(t *GomegaWithT, err error) {
t.Expect(err).NotTo(HaveOccurred())
},
expectFn: func(t *GomegaWithT, tc *v1alpha1.TidbCluster) {
t.Expect(len(tc.Status.TiDB.FailureMembers)).To(Equal(0))
t.Expect(int(tc.Spec.TiDB.Replicas)).To(Equal(2))
},
},
{
name: "two tidb members failed",
pods: []*corev1.Pod{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Name: "failover-tidb-0",
},
Status: corev1.PodStatus{
Conditions: []corev1.PodCondition{
{
Type: corev1.PodScheduled,
Status: corev1.ConditionTrue,
},
},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Name: "failover-tidb-1",
},
Status: corev1.PodStatus{
Conditions: []corev1.PodCondition{
{
Type: corev1.PodScheduled,
Status: corev1.ConditionTrue,
},
},
},
},
},
update: func(tc *v1alpha1.TidbCluster) {
tc.Status.TiDB.Members = map[string]v1alpha1.TiDBMember{
"failover-tidb-0": {
Expand Down Expand Up @@ -207,30 +310,31 @@ func TestFakeTiDBFailoverFailover(t *testing.T) {
},
}

for i := range tests {
testFn(&tests[i], t)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewGomegaWithT(t)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
fakeClient := fake.NewSimpleClientset()
for _, pod := range test.pods {
fakeClient.CoreV1().Pods(pod.Namespace).Create(pod)
}
tidbFailover := newTiDBFailover(ctx, fakeClient)
tc := newTidbClusterForTiDBFailover()
test.update(tc)
err := tidbFailover.Failover(tc)
test.errExpectFn(g, err)
test.expectFn(g, tc)
})
}
}

func TestFakeTiDBFailoverRecover(t *testing.T) {
type testcase struct {
func TestTiDBFailoverRecover(t *testing.T) {
tests := []struct {
name string
update func(*v1alpha1.TidbCluster)
expectFn func(*GomegaWithT, *v1alpha1.TidbCluster)
}

testFn := func(test *testcase, t *testing.T) {
t.Log(test.name)
g := NewGomegaWithT(t)
tidbFailover := newTiDBFailover()
tc := newTidbClusterForTiDBFailover()
test.update(tc)

tidbFailover.Recover(tc)
test.expectFn(g, tc)
}

tests := []testcase{
}{
{
name: "have not failure tidb member to recover",
update: func(tc *v1alpha1.TidbCluster) {
Expand Down Expand Up @@ -377,14 +481,28 @@ func TestFakeTiDBFailoverRecover(t *testing.T) {
},
}

for i := range tests {
testFn(&tests[i], t)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewGomegaWithT(t)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
fakeClient := fake.NewSimpleClientset()
tidbFailover := newTiDBFailover(ctx, fakeClient)
tc := newTidbClusterForTiDBFailover()
test.update(tc)
tidbFailover.Recover(tc)
test.expectFn(g, tc)
})
}
}

func newTiDBFailover() Failover {
func newTiDBFailover(ctx context.Context, client kubernetes.Interface) Failover {
kubeInformerFactory := kubeinformers.NewSharedInformerFactory(client, 0)
podLister := kubeInformerFactory.Core().V1().Pods().Lister()
kubeInformerFactory.Start(ctx.Done())
kubeInformerFactory.WaitForCacheSync(ctx.Done())
recorder := record.NewFakeRecorder(100)
return &tidbFailover{tidbFailoverPeriod: time.Duration(5 * time.Minute), recorder: recorder}
return NewTiDBFailover(time.Duration(5*time.Minute), recorder, podLister)
}

func newTidbClusterForTiDBFailover() *v1alpha1.TidbCluster {
Expand Down
Loading

0 comments on commit 8a7e46b

Please sign in to comment.