Merge branch 'master' into yinliang/modify-br-tls

pingcap · Mar 23, 2020 · c4e7f34 · c4e7f34
2 parents 6aea916 + 1f09144
commit c4e7f34
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 18 deletions.
diff --git a/pkg/controller/pod_control.go b/pkg/controller/pod_control.go
@@ -191,7 +191,7 @@ func (rpc *realPodControl) DeletePod(tc *v1alpha1.TidbCluster, pod *corev1.Pod)
  ns := tc.GetNamespace()
  tcName := tc.GetName()
  podName := pod.GetName()
- preconditions := metav1.Preconditions{UID: &pod.UID}
+ preconditions := metav1.Preconditions{UID: &pod.UID, ResourceVersion: &pod.ResourceVersion}
  deleteOptions := metav1.DeleteOptions{Preconditions: &preconditions}
  err := rpc.kubeCli.CoreV1().Pods(ns).Delete(podName, &deleteOptions)
  if err != nil {

diff --git a/pkg/manager/member/orphan_pods_cleaner.go b/pkg/manager/member/orphan_pods_cleaner.go
@@ -17,7 +17,6 @@ import (
  "github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1"
  "github.com/pingcap/tidb-operator/pkg/controller"
  "github.com/pingcap/tidb-operator/pkg/label"
- v1 "k8s.io/api/core/v1"
  "k8s.io/apimachinery/pkg/api/errors"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
  "k8s.io/client-go/kubernetes"
@@ -26,12 +25,12 @@ import (
 )
 
 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"
- skipReasonOrphanPodsCleanerPodIsNotPending = "orphan pods cleaner: pod is not pending"
- skipReasonOrphanPodsCleanerPodIsNotFound = "orphan pods cleaner: pod does not exist anymore"
- skipReasonOrphanPodsCleanerPodChanged = "orphan pods cleaner: pod changed before deletion"
+ 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"
+ skipReasonOrphanPodsCleanerPodHasBeenScheduled = "orphan pods cleaner: pod has been scheduled"
+ skipReasonOrphanPodsCleanerPodIsNotFound  = "orphan pods cleaner: pod does not exist anymore"
+ skipReasonOrphanPodsCleanerPodChanged  = "orphan pods cleaner: pod changed before deletion"
 )
 
 // OrphanPodsCleaner implements the logic for cleaning the orphan pods(has no pvc)
@@ -88,8 +87,8 @@ func (opc *orphanPodsCleaner) Clean(tc *v1alpha1.TidbCluster) (map[string]string
  continue
  }
 
- if pod.Status.Phase != v1.PodPending {
- skipReason[podName] = skipReasonOrphanPodsCleanerPodIsNotPending
+ if len(pod.Spec.NodeName) > 0 {
+ skipReason[podName] = skipReasonOrphanPodsCleanerPodHasBeenScheduled
  continue
  }
 
@@ -128,7 +127,7 @@ func (opc *orphanPodsCleaner) Clean(tc *v1alpha1.TidbCluster) (map[string]string
  }
 
  // if the PVC is not found in apiserver (also informer cache) and the
- // phase of the Pod is Pending, delete it and let the stateful
+ // pod has not been scheduled, delete it and let the stateful
  // controller to create the pod and its PVC(s) again
  apiPod, err := opc.kubeCli.CoreV1().Pods(ns).Get(podName, metav1.GetOptions{})
  if errors.IsNotFound(err) {
@@ -138,8 +137,9 @@ func (opc *orphanPodsCleaner) Clean(tc *v1alpha1.TidbCluster) (map[string]string
  if err != nil {
  return skipReason, err
  }
- // try our best to avoid deleting wrong object in apiserver
- // TODO upgrade to use deleteOption.Preconditions.ResourceVersion in client-go 1.14+
+ // In pre-1.14, kube-apiserver does not support
+ // deleteOption.Preconditions.ResourceVersion, we try our best to avoid
+ // deleting wrong object in apiserver.
  if apiPod.UID != pod.UID || apiPod.ResourceVersion != pod.ResourceVersion {
  skipReason[podName] = skipReasonOrphanPodsCleanerPodChanged
  continue

diff --git a/pkg/manager/member/orphan_pods_cleaner_test.go b/pkg/manager/member/orphan_pods_cleaner_test.go
@@ -227,7 +227,9 @@ func TestOrphanPodsCleanerClean(t *testing.T) {
  },
  },
  {
- name: "pvc is not found but pod is not pending",
+ // in theory, this is is possible because we can't check the PVC
+ // and pod in an atomic operation.
+ name: "pvc is not found but pod has been scheduled",
  pods: []*corev1.Pod{
  {
  ObjectMeta: metav1.ObjectMeta{
@@ -246,17 +248,15 @@ func TestOrphanPodsCleanerClean(t *testing.T) {
  },
  },
  },
- },
- Status: corev1.PodStatus{
- Phase: corev1.PodRunning,
+ NodeName: "foobar",
  },
  },
  },
  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(1))
- g.Expect(skipReason["pod-1"]).To(Equal(skipReasonOrphanPodsCleanerPodIsNotPending))
+ g.Expect(skipReason["pod-1"]).To(Equal(skipReasonOrphanPodsCleanerPodHasBeenScheduled))
  },
  },
  {