diff --git a/pkg/controller/tidbcluster/tidb_cluster_control.go b/pkg/controller/tidbcluster/tidb_cluster_control.go index 267ef19fb45..553e7998d90 100644 --- a/pkg/controller/tidbcluster/tidb_cluster_control.go +++ b/pkg/controller/tidbcluster/tidb_cluster_control.go @@ -17,6 +17,7 @@ import ( "github.com/pingcap/tidb-operator/pkg/apis/pingcap.com/v1alpha1" "github.com/pingcap/tidb-operator/pkg/controller" "github.com/pingcap/tidb-operator/pkg/manager" + "github.com/pingcap/tidb-operator/pkg/manager/member" apiequality "k8s.io/apimachinery/pkg/api/equality" errorutils "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/tools/record" @@ -39,6 +40,7 @@ func NewDefaultTidbClusterControl( tidbMemberManager manager.Manager, reclaimPolicyManager manager.Manager, metaManager manager.Manager, + orphanPodsCleaner member.OrphanPodsCleaner, recorder record.EventRecorder) ControlInterface { return &defaultTidbClusterControl{ tcControl, @@ -47,6 +49,7 @@ func NewDefaultTidbClusterControl( tidbMemberManager, reclaimPolicyManager, metaManager, + orphanPodsCleaner, recorder, } } @@ -58,6 +61,7 @@ type defaultTidbClusterControl struct { tidbMemberManager manager.Manager reclaimPolicyManager manager.Manager metaManager manager.Manager + orphanPodsCleaner member.OrphanPodsCleaner recorder record.EventRecorder } @@ -92,6 +96,11 @@ func (tcc *defaultTidbClusterControl) updateTidbCluster(tc *v1alpha1.TidbCluster return err } + _, err = tcc.orphanPodsCleaner.Clean(tc) + if err != nil { + return err + } + // PD err = tcc.pdMemberManager.Sync(tc) if err != nil { diff --git a/pkg/controller/tidbcluster/tidb_cluster_control_test.go b/pkg/controller/tidbcluster/tidb_cluster_control_test.go index 436ba825397..c5ab6e12709 100644 --- a/pkg/controller/tidbcluster/tidb_cluster_control_test.go +++ b/pkg/controller/tidbcluster/tidb_cluster_control_test.go @@ -361,7 +361,8 @@ func newFakeTidbClusterControl() (ControlInterface, *meta.FakeReclaimPolicyManag tidbMemberManager := mm.NewFakeTiDBMemberManager() reclaimPolicyManager := meta.NewFakeReclaimPolicyManager() metaManager := meta.NewFakeMetaManager() - control := NewDefaultTidbClusterControl(tcControl, pdMemberManager, tikvMemberManager, tidbMemberManager, reclaimPolicyManager, metaManager, recorder) + opc := mm.NewFakeOrphanPodsCleaner() + control := NewDefaultTidbClusterControl(tcControl, pdMemberManager, tikvMemberManager, tidbMemberManager, reclaimPolicyManager, metaManager, opc, recorder) return control, reclaimPolicyManager, pdMemberManager, tikvMemberManager, tidbMemberManager, metaManager } diff --git a/pkg/controller/tidbcluster/tidb_cluster_controller.go b/pkg/controller/tidbcluster/tidb_cluster_controller.go index 42808cd482f..677802af012 100644 --- a/pkg/controller/tidbcluster/tidb_cluster_controller.go +++ b/pkg/controller/tidbcluster/tidb_cluster_controller.go @@ -162,6 +162,11 @@ func NewController( podInformer.Lister(), podControl, ), + mm.NewOrphanPodsCleaner( + podInformer.Lister(), + podControl, + pvcInformer.Lister(), + ), recorder, ), queue: workqueue.NewNamedRateLimitingQueue( diff --git a/pkg/controller/tidbcluster/tidb_cluster_controller_test.go b/pkg/controller/tidbcluster/tidb_cluster_controller_test.go index ee4df26c574..c9a442bb0a2 100644 --- a/pkg/controller/tidbcluster/tidb_cluster_controller_test.go +++ b/pkg/controller/tidbcluster/tidb_cluster_controller_test.go @@ -319,6 +319,7 @@ func newFakeTidbClusterController() (*Controller, cache.Indexer, cache.Indexer) podInformer.Lister(), podControl, ), + mm.NewFakeOrphanPodsCleaner(), recorder, ) diff --git a/pkg/label/label_test.go b/pkg/label/label_test.go index 2856e71a913..a13d6f43a41 100644 --- a/pkg/label/label_test.go +++ b/pkg/label/label_test.go @@ -16,9 +16,8 @@ package label import ( "testing" - "k8s.io/apimachinery/pkg/labels" - . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/labels" ) func TestLabelNew(t *testing.T) { diff --git a/pkg/manager/member/orphan_pods_cleaner.go b/pkg/manager/member/orphan_pods_cleaner.go new file mode 100644 index 00000000000..3ea725a024a --- /dev/null +++ b/pkg/manager/member/orphan_pods_cleaner.go @@ -0,0 +1,109 @@ +// Copyright 2018 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package member + +import ( + "github.com/pingcap/tidb-operator/pkg/apis/pingcap.com/v1alpha1" + "github.com/pingcap/tidb-operator/pkg/controller" + "github.com/pingcap/tidb-operator/pkg/label" + "k8s.io/apimachinery/pkg/api/errors" + corelisters "k8s.io/client-go/listers/core/v1" +) + +const ( + skipReasonOrphanPodsCleanerIsNotPDOrTiKV = "orphan pods cleaner: member type is not pd or tikv" + skipReasonOrphanPodsCleanerPVCNameIsEmpty = "orphan pods cleaner: pvcName is empty" + skipReasonOrphanPodsCleanerPVCIsFound = "orphan pods cleaner: pvc is found" +) + +// OrphanPodsCleaner implements the logic for cleaning the orphan pods(has no pvc) +type OrphanPodsCleaner interface { + Clean(*v1alpha1.TidbCluster) (map[string]string, error) +} + +type orphanPodsCleaner struct { + podLister corelisters.PodLister + podControl controller.PodControlInterface + pvcLister corelisters.PersistentVolumeClaimLister +} + +// NewOrphanPodsCleaner returns a OrphanPodsCleaner +func NewOrphanPodsCleaner(podLister corelisters.PodLister, + podControl controller.PodControlInterface, + pvcLister corelisters.PersistentVolumeClaimLister) OrphanPodsCleaner { + return &orphanPodsCleaner{podLister, podControl, pvcLister} +} + +func (opc *orphanPodsCleaner) Clean(tc *v1alpha1.TidbCluster) (map[string]string, error) { + ns := tc.GetNamespace() + // for unit test + skipReason := map[string]string{} + + selector, err := label.New().Instance(tc.GetLabels()[label.InstanceLabelKey]).Selector() + if err != nil { + return skipReason, err + } + pods, err := opc.podLister.Pods(ns).List(selector) + if err != nil { + return skipReason, err + } + + for _, pod := range pods { + podName := pod.GetName() + l := label.Label(pod.Labels) + if !(l.IsPD() || l.IsTiKV()) { + skipReason[podName] = skipReasonOrphanPodsCleanerIsNotPDOrTiKV + continue + } + + var pvcName string + for _, vol := range pod.Spec.Volumes { + if vol.PersistentVolumeClaim != nil { + pvcName = vol.PersistentVolumeClaim.ClaimName + break + } + } + if pvcName == "" { + skipReason[podName] = skipReasonOrphanPodsCleanerPVCNameIsEmpty + continue + } + + _, err := opc.pvcLister.PersistentVolumeClaims(ns).Get(pvcName) + if err == nil { + skipReason[podName] = skipReasonOrphanPodsCleanerPVCIsFound + continue + } + if !errors.IsNotFound(err) { + return skipReason, err + } + + err = opc.podControl.DeletePod(tc, pod) + if err != nil { + return skipReason, err + } + } + + return skipReason, nil +} + +type fakeOrphanPodsCleaner struct{} + +// NewFakeOrphanPodsCleaner returns a fake orphan pods cleaner +func NewFakeOrphanPodsCleaner() OrphanPodsCleaner { + return &fakeOrphanPodsCleaner{} +} + +func (fopc *fakeOrphanPodsCleaner) Clean(_ *v1alpha1.TidbCluster) (map[string]string, error) { + return nil, nil +} diff --git a/pkg/manager/member/orphan_pods_cleaner_test.go b/pkg/manager/member/orphan_pods_cleaner_test.go new file mode 100644 index 00000000000..7735bc17a5c --- /dev/null +++ b/pkg/manager/member/orphan_pods_cleaner_test.go @@ -0,0 +1,366 @@ +// Copyright 2018 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package member + +import ( + "fmt" + "strings" + "testing" + + . "github.com/onsi/gomega" + "github.com/pingcap/tidb-operator/pkg/controller" + "github.com/pingcap/tidb-operator/pkg/label" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kubeinformers "k8s.io/client-go/informers" + kubefake "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/cache" +) + +func TestOrphanPodsCleanerClean(t *testing.T) { + g := NewGomegaWithT(t) + + tc := newTidbClusterForPD() + type testcase struct { + name string + pods []*corev1.Pod + pvcs []*corev1.PersistentVolumeClaim + deletePodFailed bool + expectFn func(*GomegaWithT, map[string]string, *orphanPodsCleaner, error) + } + testFn := func(test *testcase, t *testing.T) { + t.Log(test.name) + + opc, podIndexer, pvcIndexer, podControl := newFakeOrphanPodsCleaner() + if test.pods != nil { + for _, pod := range test.pods { + podIndexer.Add(pod) + } + } + if test.pvcs != nil { + for _, pvc := range test.pvcs { + pvcIndexer.Add(pvc) + } + } + if test.deletePodFailed { + podControl.SetDeletePodError(fmt.Errorf("delete pod failed"), 0) + } + + skipReason, err := opc.Clean(tc) + test.expectFn(g, skipReason, opc, err) + } + tests := []testcase{ + { + name: "no pods", + pods: []*corev1.Pod{}, + pvcs: nil, + expectFn: func(g *GomegaWithT, skipReason map[string]string, _ *orphanPodsCleaner, err error) { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(len(skipReason)).To(Equal(0)) + }, + }, + { + name: "not pd or tikv pods", + pods: []*corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: metav1.NamespaceDefault, + Labels: label.New().Instance(tc.GetLabels()[label.InstanceLabelKey]).TiDB().Labels(), + }, + }, + }, + pvcs: nil, + expectFn: func(g *GomegaWithT, skipReason map[string]string, _ *orphanPodsCleaner, err error) { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(len(skipReason)).To(Equal(1)) + g.Expect(skipReason["pod-1"]).To(Equal(skipReasonOrphanPodsCleanerIsNotPDOrTiKV)) + }, + }, + { + name: "has no spec.volumes", + pods: []*corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: metav1.NamespaceDefault, + Labels: label.New().Instance(tc.GetLabels()[label.InstanceLabelKey]).PD().Labels(), + }, + }, + }, + pvcs: nil, + expectFn: func(g *GomegaWithT, skipReason map[string]string, _ *orphanPodsCleaner, err error) { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(len(skipReason)).To(Equal(1)) + g.Expect(skipReason["pod-1"]).To(Equal(skipReasonOrphanPodsCleanerPVCNameIsEmpty)) + }, + }, + { + name: "claimName is empty", + pods: []*corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: metav1.NamespaceDefault, + Labels: label.New().Instance(tc.GetLabels()[label.InstanceLabelKey]).PD().Labels(), + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "pd", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "", + }, + }, + }, + }, + }, + }, + }, + pvcs: nil, + expectFn: func(g *GomegaWithT, skipReason map[string]string, _ *orphanPodsCleaner, err error) { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(len(skipReason)).To(Equal(1)) + g.Expect(skipReason["pod-1"]).To(Equal(skipReasonOrphanPodsCleanerPVCNameIsEmpty)) + }, + }, + { + name: "pvc is found", + pods: []*corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: metav1.NamespaceDefault, + Labels: label.New().Instance(tc.GetLabels()[label.InstanceLabelKey]).PD().Labels(), + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "pd", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pvc-1", + }, + }, + }, + }, + }, + }, + }, + pvcs: []*corev1.PersistentVolumeClaim{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pvc-1", + Namespace: metav1.NamespaceDefault, + }, + }, + }, + expectFn: func(g *GomegaWithT, skipReason map[string]string, _ *orphanPodsCleaner, err error) { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(len(skipReason)).To(Equal(1)) + g.Expect(skipReason["pod-1"]).To(Equal(skipReasonOrphanPodsCleanerPVCIsFound)) + }, + }, + { + name: "pvc is not found", + pods: []*corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: metav1.NamespaceDefault, + Labels: label.New().Instance(tc.GetLabels()[label.InstanceLabelKey]).PD().Labels(), + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "pd", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pvc-1", + }, + }, + }, + }, + }, + }, + }, + pvcs: []*corev1.PersistentVolumeClaim{}, + expectFn: func(g *GomegaWithT, skipReason map[string]string, opc *orphanPodsCleaner, err error) { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(len(skipReason)).To(Equal(0)) + _, err = opc.podLister.Pods("default").Get("pod-1") + g.Expect(err).To(HaveOccurred()) + g.Expect(strings.Contains(err.Error(), "not found")).To(BeTrue()) + }, + }, + { + name: "pod delete failed", + pods: []*corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: metav1.NamespaceDefault, + Labels: label.New().Instance(tc.GetLabels()[label.InstanceLabelKey]).PD().Labels(), + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "pd", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pvc-1", + }, + }, + }, + }, + }, + }, + }, + pvcs: []*corev1.PersistentVolumeClaim{}, + deletePodFailed: true, + expectFn: func(g *GomegaWithT, skipReason map[string]string, opc *orphanPodsCleaner, err error) { + g.Expect(len(skipReason)).To(Equal(0)) + g.Expect(err).To(HaveOccurred()) + g.Expect(strings.Contains(err.Error(), "delete pod failed")).To(BeTrue()) + }, + }, + { + name: "multiple pods", + pods: []*corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: metav1.NamespaceDefault, + Labels: label.New().Instance(tc.GetLabels()[label.InstanceLabelKey]).PD().Labels(), + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "pd", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pvc-1", + }, + }, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-2", + Namespace: metav1.NamespaceDefault, + Labels: label.New().Instance(tc.GetLabels()[label.InstanceLabelKey]).TiKV().Labels(), + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "pd", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "", + }, + }, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-3", + Namespace: metav1.NamespaceDefault, + Labels: label.New().Instance(tc.GetLabels()[label.InstanceLabelKey]).TiKV().Labels(), + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "pd", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pvc-3", + }, + }, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-4", + Namespace: metav1.NamespaceDefault, + Labels: label.New().Instance(tc.GetLabels()[label.InstanceLabelKey]).TiDB().Labels(), + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "pd", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pvc-4", + }, + }, + }, + }, + }, + }, + }, + pvcs: []*corev1.PersistentVolumeClaim{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pvc-2", + Namespace: metav1.NamespaceDefault, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pvc-3", + Namespace: metav1.NamespaceDefault, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pvc-4", + Namespace: metav1.NamespaceDefault, + }, + }, + }, + deletePodFailed: false, + expectFn: func(g *GomegaWithT, skipReason map[string]string, opc *orphanPodsCleaner, err error) { + g.Expect(len(skipReason)).To(Equal(3)) + g.Expect(skipReason["pod-2"]).To(Equal(skipReasonOrphanPodsCleanerPVCNameIsEmpty)) + g.Expect(skipReason["pod-3"]).To(Equal(skipReasonOrphanPodsCleanerPVCIsFound)) + g.Expect(skipReason["pod-4"]).To(Equal(skipReasonOrphanPodsCleanerIsNotPDOrTiKV)) + g.Expect(err).NotTo(HaveOccurred()) + _, err = opc.podLister.Pods("default").Get("pod-1") + g.Expect(err).To(HaveOccurred()) + g.Expect(strings.Contains(err.Error(), "not found")).To(BeTrue()) + }, + }, + } + for i := range tests { + testFn(&tests[i], t) + } +} + +func newFakeOrphanPodsCleaner() (*orphanPodsCleaner, cache.Indexer, cache.Indexer, *controller.FakePodControl) { + kubeCli := kubefake.NewSimpleClientset() + kubeInformerFactory := kubeinformers.NewSharedInformerFactory(kubeCli, 0) + podInformer := kubeInformerFactory.Core().V1().Pods() + pvcInformer := kubeInformerFactory.Core().V1().PersistentVolumeClaims() + podControl := controller.NewFakePodControl(podInformer) + + return &orphanPodsCleaner{podInformer.Lister(), podControl, pvcInformer.Lister()}, + podInformer.Informer().GetIndexer(), pvcInformer.Informer().GetIndexer(), podControl +} diff --git a/pkg/manager/member/pd_failover.go b/pkg/manager/member/pd_failover.go index a8b952762b2..b7eef977a9d 100644 --- a/pkg/manager/member/pd_failover.go +++ b/pkg/manager/member/pd_failover.go @@ -57,36 +57,6 @@ func NewPDFailover(cli versioned.Interface, pvLister} } -func (pf *pdFailover) cleanOrphanPods(tc *v1alpha1.TidbCluster) error { - ns := tc.GetNamespace() - tcName := tc.GetName() - - for _, pdMember := range tc.Status.PD.FailureMembers { - if !pdMember.MemberDeleted { - continue - } - - podName := pdMember.PodName - ordinal, err := util.GetOrdinalFromPodName(podName) - if err != nil { - return err - } - pvcName := ordinalPVCName(v1alpha1.PDMemberType, controller.PDMemberName(tcName), ordinal) - _, err = pf.pvcLister.PersistentVolumeClaims(ns).Get(pvcName) - if errors.IsNotFound(err) { - pod, err := pf.podLister.Pods(ns).Get(podName) - if err == nil { - err = pf.podControl.DeletePod(tc, pod) - if err != nil { - return err - } - } - } - } - - return nil -} - func (pf *pdFailover) Failover(tc *v1alpha1.TidbCluster) error { ns := tc.GetNamespace() tcName := tc.GetName() @@ -94,14 +64,9 @@ func (pf *pdFailover) Failover(tc *v1alpha1.TidbCluster) error { if !tc.Status.PD.Synced { return fmt.Errorf("TidbCluster: %s/%s's pd status sync failed, can't failover", ns, tcName) } - if tc.Status.PD.FailureMembers == nil { tc.Status.PD.FailureMembers = map[string]v1alpha1.PDFailureMember{} } - err := pf.cleanOrphanPods(tc) - if err != nil { - return err - } healthCount := 0 for _, pdMember := range tc.Status.PD.Members { @@ -155,7 +120,7 @@ func (pf *pdFailover) Failover(tc *v1alpha1.TidbCluster) error { } // invoke deleteMember api to delete a member from the pd cluster - err = pf.pdControl.GetPDClient(tc).DeleteMember(failureMember.PodName) + err := pf.pdControl.GetPDClient(tc).DeleteMember(failureMember.PodName) if err != nil { return err } diff --git a/pkg/manager/member/pd_scaler.go b/pkg/manager/member/pd_scaler.go index f5b7b065c90..1fe1379dd48 100644 --- a/pkg/manager/member/pd_scaler.go +++ b/pkg/manager/member/pd_scaler.go @@ -45,7 +45,7 @@ func (psd *pdScaler) ScaleOut(tc *v1alpha1.TidbCluster, oldSet *apps.StatefulSet return nil } - _, err := psd.deleteAllDeferDeletingPVC(tc, oldSet.GetName(), v1alpha1.PDMemberType, *oldSet.Spec.Replicas, *newSet.Spec.Replicas) + _, err := psd.deleteDeferDeletingPVC(tc, oldSet.GetName(), v1alpha1.PDMemberType, *oldSet.Spec.Replicas) if err != nil { resetReplicas(newSet, oldSet) return err diff --git a/pkg/manager/member/scaler.go b/pkg/manager/member/scaler.go index 2474f555687..02759e62f74 100644 --- a/pkg/manager/member/scaler.go +++ b/pkg/manager/member/scaler.go @@ -25,9 +25,9 @@ import ( ) const ( - skipReasonPVCNotFound = "pvc is not found" - skipReasonAnnIsNil = "pvc annotations is nil" - skipReasonAnnDeferDeletingIsEmpty = "pvc annotations defer deleting is empty" + skipReasonScalerPVCNotFound = "scaler: pvc is not found" + skipReasonScalerAnnIsNil = "scaler: pvc annotations is nil" + skipReasonScalerAnnDeferDeletingIsEmpty = "scaler: pvc annotations defer deleting is empty" ) // Scaler implements the logic for scaling out or scaling in the cluster. @@ -44,39 +44,32 @@ type generalScaler struct { pvcControl controller.PVCControlInterface } -func (gs *generalScaler) deleteAllDeferDeletingPVC(tc *v1alpha1.TidbCluster, - setName string, memberType v1alpha1.MemberType, from, to int32) (map[int32]string, error) { +func (gs *generalScaler) deleteDeferDeletingPVC(tc *v1alpha1.TidbCluster, + setName string, memberType v1alpha1.MemberType, ordinal int32) (map[int32]string, error) { ns := tc.GetNamespace() // for unit test skipReason := map[int32]string{} - for i := from; i < to; i++ { - pvcName := ordinalPVCName(memberType, setName, i) - pvc, err := gs.pvcLister.PersistentVolumeClaims(ns).Get(pvcName) - if errors.IsNotFound(err) { - skipReason[i] = skipReasonPVCNotFound - continue - } - if err != nil { - return skipReason, err - } - - if pvc.Annotations == nil { - skipReason[i] = skipReasonAnnIsNil - continue - } - if _, ok := pvc.Annotations[label.AnnPVCDeferDeleting]; !ok { - skipReason[i] = skipReasonAnnDeferDeletingIsEmpty - continue - } + pvcName := ordinalPVCName(memberType, setName, ordinal) + pvc, err := gs.pvcLister.PersistentVolumeClaims(ns).Get(pvcName) + if errors.IsNotFound(err) { + skipReason[ordinal] = skipReasonScalerPVCNotFound + return skipReason, nil + } + if err != nil { + return skipReason, err + } - err = gs.pvcControl.DeletePVC(tc, pvc) - if err != nil { - return skipReason, err - } + if pvc.Annotations == nil { + skipReason[ordinal] = skipReasonScalerAnnIsNil + return skipReason, nil + } + if _, ok := pvc.Annotations[label.AnnPVCDeferDeleting]; !ok { + skipReason[ordinal] = skipReasonScalerAnnDeferDeletingIsEmpty + return skipReason, nil } - return skipReason, nil + return skipReason, gs.pvcControl.DeletePVC(tc, pvc) } func resetReplicas(newSet *apps.StatefulSet, oldSet *apps.StatefulSet) { diff --git a/pkg/manager/member/scaler_test.go b/pkg/manager/member/scaler_test.go index 4726b09ef81..1a63e2aaed9 100644 --- a/pkg/manager/member/scaler_test.go +++ b/pkg/manager/member/scaler_test.go @@ -31,13 +31,12 @@ import ( func TestGeneralScalerDeleteAllDeferDeletingPVC(t *testing.T) { type testcase struct { - name string - memberType v1alpha1.MemberType - from int32 - to int32 - pvcs []*corev1.PersistentVolumeClaim - deleteFailedIdx []int - expectFn func(*GomegaWithT, map[int32]string, error) + name string + memberType v1alpha1.MemberType + ordinal int32 + pvc *corev1.PersistentVolumeClaim + deleteFailed bool + expectFn func(*GomegaWithT, map[int32]string, error) } tc := newTidbClusterForPD() setName := controller.PDMemberName(tc.GetName()) @@ -46,270 +45,97 @@ func TestGeneralScalerDeleteAllDeferDeletingPVC(t *testing.T) { g := NewGomegaWithT(t) gs, pvcIndexer, pvcControl := newFakeGeneralScaler() - for _, pvc := range test.pvcs { - pvcIndexer.Add(pvc) + if test.pvc != nil { + pvcIndexer.Add(test.pvc) } - for _, idx := range test.deleteFailedIdx { - pvcControl.SetDeletePVCError(fmt.Errorf("delete pvc failed"), idx) + if test.deleteFailed { + pvcControl.SetDeletePVCError(fmt.Errorf("delete pvc failed"), 0) } - skipReason, err := gs.deleteAllDeferDeletingPVC(tc, setName, test.memberType, test.from, test.to) + skipReason, err := gs.deleteDeferDeletingPVC(tc, setName, test.memberType, test.ordinal) test.expectFn(g, skipReason, err) } tests := []testcase{ { name: "normal", memberType: v1alpha1.PDMemberType, - from: 3, - to: 5, - pvcs: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 3), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{ - label.AnnPVCDeferDeleting: "deleting-3", - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 4), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{ - label.AnnPVCDeferDeleting: "deleting-4", - }, + ordinal: 3, + pvc: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 3), + Namespace: corev1.NamespaceDefault, + Annotations: map[string]string{ + label.AnnPVCDeferDeleting: "deleting-3", }, }, }, - deleteFailedIdx: nil, + deleteFailed: false, expectFn: func(g *GomegaWithT, skipReason map[int32]string, err error) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(len(skipReason)).To(Equal(0)) }, }, { - name: "from 3 to 5, but pvc 3 is not found", - memberType: v1alpha1.PDMemberType, - from: 3, - to: 5, - pvcs: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 4), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{ - label.AnnPVCDeferDeleting: "deleting-4", - }, - }, - }, - }, - deleteFailedIdx: nil, + name: "pvc is not found", + memberType: v1alpha1.PDMemberType, + ordinal: 3, + pvc: nil, + deleteFailed: false, expectFn: func(g *GomegaWithT, skipReason map[int32]string, err error) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(len(skipReason)).To(Equal(1)) - g.Expect(skipReason[3]).To(Equal(skipReasonPVCNotFound)) + g.Expect(skipReason[3]).To(Equal(skipReasonScalerPVCNotFound)) }, }, { - name: "from 3 to 5, but pvc 4 is not found", + name: "pvc annotations is nil", memberType: v1alpha1.PDMemberType, - from: 3, - to: 5, - pvcs: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 3), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{ - label.AnnPVCDeferDeleting: "deleting-3", - }, - }, + ordinal: 3, + pvc: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 3), + Namespace: corev1.NamespaceDefault, }, }, - deleteFailedIdx: nil, + deleteFailed: false, expectFn: func(g *GomegaWithT, skipReason map[int32]string, err error) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(len(skipReason)).To(Equal(1)) - g.Expect(skipReason[4]).To(Equal(skipReasonPVCNotFound)) + g.Expect(skipReason[3]).To(Equal(skipReasonScalerAnnIsNil)) }, }, { - name: "from 3 to 5, but pvc 3 annotations is nil", + name: "pvc annotations defer deleting is empty", memberType: v1alpha1.PDMemberType, - from: 3, - to: 5, - pvcs: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 3), - Namespace: corev1.NamespaceDefault, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 4), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{ - label.AnnPVCDeferDeleting: "deleting-4", - }, - }, + ordinal: 3, + pvc: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 3), + Namespace: corev1.NamespaceDefault, + Annotations: map[string]string{}, }, }, - deleteFailedIdx: nil, + deleteFailed: false, expectFn: func(g *GomegaWithT, skipReason map[int32]string, err error) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(len(skipReason)).To(Equal(1)) - g.Expect(skipReason[3]).To(Equal(skipReasonAnnIsNil)) + g.Expect(skipReason[3]).To(Equal(skipReasonScalerAnnDeferDeletingIsEmpty)) }, }, { - name: "from 3 to 5, but pvc 4 annotations is nil", + name: "pvc delete failed", memberType: v1alpha1.PDMemberType, - from: 3, - to: 5, - pvcs: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 3), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{ - label.AnnPVCDeferDeleting: "deleting-3", - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 4), - Namespace: corev1.NamespaceDefault, - }, - }, - }, - deleteFailedIdx: nil, - expectFn: func(g *GomegaWithT, skipReason map[int32]string, err error) { - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(len(skipReason)).To(Equal(1)) - g.Expect(skipReason[4]).To(Equal(skipReasonAnnIsNil)) - }, - }, - { - name: "from 3 to 5, but pvc 3 annotations defer deleting is empty", - memberType: v1alpha1.PDMemberType, - from: 3, - to: 5, - pvcs: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 3), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{}, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 4), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{ - label.AnnPVCDeferDeleting: "deleting-4", - }, - }, - }, - }, - deleteFailedIdx: nil, - expectFn: func(g *GomegaWithT, skipReason map[int32]string, err error) { - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(len(skipReason)).To(Equal(1)) - g.Expect(skipReason[3]).To(Equal(skipReasonAnnDeferDeletingIsEmpty)) - }, - }, - { - name: "from 3 to 5, but pvc 4 annotations defer deleting is empty", - memberType: v1alpha1.PDMemberType, - from: 3, - to: 5, - pvcs: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 3), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{ - label.AnnPVCDeferDeleting: "deleting-3", - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 4), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{}, - }, - }, - }, - deleteFailedIdx: nil, - expectFn: func(g *GomegaWithT, skipReason map[int32]string, err error) { - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(len(skipReason)).To(Equal(1)) - g.Expect(skipReason[4]).To(Equal(skipReasonAnnDeferDeletingIsEmpty)) - }, - }, - { - name: "from 3 to 5, but pvc 3 delete failed", - memberType: v1alpha1.PDMemberType, - from: 3, - to: 5, - pvcs: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 3), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{ - label.AnnPVCDeferDeleting: "deleting-3", - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 4), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{ - label.AnnPVCDeferDeleting: "deleting-4", - }, - }, - }, - }, - deleteFailedIdx: []int{0}, - expectFn: func(g *GomegaWithT, skipReason map[int32]string, err error) { - g.Expect(err).To(HaveOccurred()) - g.Expect(strings.Contains(err.Error(), "delete pvc failed")).To(BeTrue()) - g.Expect(len(skipReason)).To(Equal(0)) - }, - }, - { - name: "from 3 to 5, but pvc 4 delete failed", - memberType: v1alpha1.PDMemberType, - from: 3, - to: 5, - pvcs: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 3), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{ - label.AnnPVCDeferDeleting: "deleting-3", - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 4), - Namespace: corev1.NamespaceDefault, - Annotations: map[string]string{ - label.AnnPVCDeferDeleting: "deleting-4", - }, + ordinal: 3, + pvc: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: ordinalPVCName(v1alpha1.PDMemberType, setName, 3), + Namespace: corev1.NamespaceDefault, + Annotations: map[string]string{ + label.AnnPVCDeferDeleting: "deleting-3", }, }, }, - deleteFailedIdx: []int{1}, + deleteFailed: true, expectFn: func(g *GomegaWithT, skipReason map[int32]string, err error) { g.Expect(err).To(HaveOccurred()) g.Expect(strings.Contains(err.Error(), "delete pvc failed")).To(BeTrue()) diff --git a/pkg/manager/member/tikv_scaler.go b/pkg/manager/member/tikv_scaler.go index 78cd8f3961b..8c4c7bb028e 100644 --- a/pkg/manager/member/tikv_scaler.go +++ b/pkg/manager/member/tikv_scaler.go @@ -45,7 +45,7 @@ func (tsd *tikvScaler) ScaleOut(tc *v1alpha1.TidbCluster, oldSet *apps.StatefulS return nil } - _, err := tsd.deleteAllDeferDeletingPVC(tc, oldSet.GetName(), v1alpha1.TiKVMemberType, *oldSet.Spec.Replicas, *newSet.Spec.Replicas) + _, err := tsd.deleteDeferDeletingPVC(tc, oldSet.GetName(), v1alpha1.TiKVMemberType, *oldSet.Spec.Replicas) if err != nil { resetReplicas(newSet, oldSet) return err diff --git a/tests/e2e/upgrade.go b/tests/e2e/upgrade.go index eea5a63fdd2..7cf8c5f22fd 100644 --- a/tests/e2e/upgrade.go +++ b/tests/e2e/upgrade.go @@ -38,7 +38,7 @@ func testUpgrade(ns, clusterName string) { Expect(err).NotTo(HaveOccurred()) By("When upgrade TiDB cluster to newer version") - err = wait.Poll(5*time.Second, 5*time.Minute, func() (bool, error) { + err = wait.Poll(5*time.Second, 10*time.Minute, func() (bool, error) { return upgrade(ns, clusterName) }) Expect(err).NotTo(HaveOccurred())