Fix pd deleting admission webhook bug (#1568) (#1574)

pingcap · Jan 16, 2020 · 82a41cd · 82a41cd
1 parent 6cbf92e
commit 82a41cd
Show file tree

Hide file tree

Showing 6 changed files with 130 additions and 36 deletions.
diff --git a/pkg/webhook/pod/pd_deleter.go b/pkg/webhook/pod/pd_deleter.go
@@ -20,6 +20,7 @@ import (
 	operatorUtils "github.com/pingcap/tidb-operator/pkg/util"
 	"github.com/pingcap/tidb-operator/pkg/webhook/util"
 	admission "k8s.io/api/admission/v1beta1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog"
 )
@@ -121,9 +122,12 @@ func (pc *PodAdmissionControl) admitDeleteNonPDMemberPod(payload *admitPayload)
 		// it would be existed an pd-3 instance with its deferDeleting label Annotations PVC.
 		// And the pvc can be deleted during upgrading if we use create pod webhook in future.
 		if !isInOrdinal {
-			pvcName := operatorUtils.OrdinalPVCName(v1alpha1.TiKVMemberType, payload.ownerStatefulSet.Name, ordinal)
+			pvcName := operatorUtils.OrdinalPVCName(v1alpha1.PDMemberType, payload.ownerStatefulSet.Name, ordinal)
 			pvc, err := pc.kubeCli.CoreV1().PersistentVolumeClaims(namespace).Get(pvcName, meta.GetOptions{})
 			if err != nil {
+				if errors.IsNotFound(err) {
+					return util.ARSuccess()
+				}
 				return util.ARFail(err)
 			}
 			err = addDeferDeletingToPVC(pvc, pc.kubeCli, payload.tc)

diff --git a/pkg/webhook/pod/pd_deleter_test.go b/pkg/webhook/pod/pd_deleter_test.go
@@ -14,6 +14,9 @@
 package pod
 
 import (
+	"fmt"
+	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
 	"testing"
 
 	. "github.com/onsi/gomega"
@@ -29,6 +32,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kubefake "k8s.io/client-go/kubernetes/fake"
+	k8sTesting "k8s.io/client-go/testing"
 )
 
 var (
@@ -49,6 +53,7 @@ func TestPDDeleterDelete(t *testing.T) {
 		isStatefulSetUpgrading bool
 		isLeader               bool
 		UpdatePVCErr           bool
+		PVCNotFound            bool
 		expectFn               func(g *GomegaWithT, response *admission.AdmissionResponse)
 	}
 
@@ -65,7 +70,20 @@ func TestPDDeleterDelete(t *testing.T) {
 		tc := newTidbClusterForPodAdmissionControl()
 		kubeCli := kubefake.NewSimpleClientset()
 
-		podAdmissionControl := newPodAdmissionControl()
+		if test.UpdatePVCErr {
+
+			if test.PVCNotFound {
+				kubeCli.PrependReactor("get", "persistentvolumeclaims", func(action k8sTesting.Action) (handled bool, ret runtime.Object, err error) {
+					return true, nil, errors.NewNotFound(action.GetResource().GroupResource(), "name")
+				})
+			} else {
+				kubeCli.PrependReactor("get", "persistentvolumeclaims", func(action k8sTesting.Action) (handled bool, ret runtime.Object, err error) {
+					return true, nil, fmt.Errorf("some errors")
+				})
+			}
+		}
+
+		podAdmissionControl := newPodAdmissionControl(kubeCli)
 		pdControl := pdapi.NewFakePDControl(kubeCli)
 		fakePDClient := controller.NewFakePDClient(pdControl, tc)
 
@@ -159,8 +177,9 @@ func TestPDDeleterDelete(t *testing.T) {
 			isStatefulSetUpgrading: true,
 			isLeader:               false,
 			UpdatePVCErr:           false,
+			PVCNotFound:            false,
 			expectFn: func(g *GomegaWithT, response *admission.AdmissionResponse) {
-				g.Expect(response.Allowed, false)
+				g.Expect(response.Allowed).Should(Equal(false))
 			},
 		},
 		{
@@ -171,8 +190,9 @@ func TestPDDeleterDelete(t *testing.T) {
 			isStatefulSetUpgrading: true,
 			isLeader:               false,
 			UpdatePVCErr:           false,
+			PVCNotFound:            false,
 			expectFn: func(g *GomegaWithT, response *admission.AdmissionResponse) {
-				g.Expect(response.Allowed, true)
+				g.Expect(response.Allowed).Should(Equal(true))
 			},
 		},
 		{
@@ -183,8 +203,9 @@ func TestPDDeleterDelete(t *testing.T) {
 			isStatefulSetUpgrading: true,
 			isLeader:               false,
 			UpdatePVCErr:           false,
+			PVCNotFound:            false,
 			expectFn: func(g *GomegaWithT, response *admission.AdmissionResponse) {
-				g.Expect(response.Allowed, false)
+				g.Expect(response.Allowed).Should(Equal(false))
 			},
 		},
 		{
@@ -195,8 +216,9 @@ func TestPDDeleterDelete(t *testing.T) {
 			isStatefulSetUpgrading: true,
 			isLeader:               true,
 			UpdatePVCErr:           false,
+			PVCNotFound:            false,
 			expectFn: func(g *GomegaWithT, response *admission.AdmissionResponse) {
-				g.Expect(response.Allowed, false)
+				g.Expect(response.Allowed).Should(Equal(false))
 			},
 		},
 		{
@@ -206,8 +228,10 @@ func TestPDDeleterDelete(t *testing.T) {
 			isOutOfOrdinal:         true,
 			isStatefulSetUpgrading: false,
 			isLeader:               false,
+			UpdatePVCErr:           false,
+			PVCNotFound:            false,
 			expectFn: func(g *GomegaWithT, response *admission.AdmissionResponse) {
-				g.Expect(response.Allowed, false)
+				g.Expect(response.Allowed).Should(Equal(false))
 			},
 		},
 		{
@@ -218,8 +242,9 @@ func TestPDDeleterDelete(t *testing.T) {
 			isStatefulSetUpgrading: false,
 			isLeader:               false,
 			UpdatePVCErr:           false,
+			PVCNotFound:            false,
 			expectFn: func(g *GomegaWithT, response *admission.AdmissionResponse) {
-				g.Expect(response.Allowed, true)
+				g.Expect(response.Allowed).Should(Equal(true))
 			},
 		},
 		{
@@ -230,9 +255,22 @@ func TestPDDeleterDelete(t *testing.T) {
 			isStatefulSetUpgrading: false,
 			isLeader:               false,
 			UpdatePVCErr:           true,
+			PVCNotFound:            false,
+			expectFn: func(g *GomegaWithT, response *admission.AdmissionResponse) {
+				g.Expect(response.Allowed).Should(Equal(false))
+			},
+		},
+		{
+			name:                   "final scale in,update pvc error,pvc not found",
+			isMember:               false,
+			isDeferDeleting:        true,
+			isOutOfOrdinal:         true,
+			isStatefulSetUpgrading: false,
+			isLeader:               false,
+			UpdatePVCErr:           true,
+			PVCNotFound:            true,
 			expectFn: func(g *GomegaWithT, response *admission.AdmissionResponse) {
-				g.Expect(response.Allowed, false)
-				g.Expect(response.Result.Message, "update pvc error")
+				g.Expect(response.Allowed).Should(Equal(true))
 			},
 		},
 	}

diff --git a/pkg/webhook/pod/pods_test.go b/pkg/webhook/pod/pods_test.go
@@ -16,6 +16,8 @@ package pod
 import (
 	"testing"
 
+	"k8s.io/client-go/kubernetes"
+
 	. "github.com/onsi/gomega"
 	"github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1"
 	operatorClifake "github.com/pingcap/tidb-operator/pkg/client/clientset/versioned/fake"
@@ -53,7 +55,8 @@ func TestAdmitPod(t *testing.T) {
 	testFn := func(test *testcase) {
 		t.Log(test.name)
 
-		podAdmissionControl := newPodAdmissionControl()
+		kubeCli := kubefake.NewSimpleClientset()
+		podAdmissionControl := newPodAdmissionControl(kubeCli)
 		ar := newAdmissionRequest()
 		pod := newNormalPod()
 		if test.isDelete {
@@ -124,8 +127,7 @@ func newAdmissionRequest() *admission.AdmissionRequest {
 	return &request
 }
 
-func newPodAdmissionControl() *PodAdmissionControl {
-	kubeCli := kubefake.NewSimpleClientset()
+func newPodAdmissionControl(kubeCli kubernetes.Interface) *PodAdmissionControl {
 	operatorCli := operatorClifake.NewSimpleClientset()
 	pdControl := pdapi.NewFakePDControl(kubeCli)
 	return &PodAdmissionControl{

diff --git a/pkg/webhook/pod/tikv_deleter.go b/pkg/webhook/pod/tikv_deleter.go
@@ -15,6 +15,7 @@ package pod
 
 import (
 	"fmt"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"strconv"
 	"strings"
 	"time"
@@ -131,6 +132,9 @@ func (pc *PodAdmissionControl) admitDeleteUselessTiKVPod(payload *admitPayload)
 		pvcName := operatorUtils.OrdinalPVCName(v1alpha1.TiKVMemberType, payload.ownerStatefulSet.Name, ordinal)
 		pvc, err := pc.kubeCli.CoreV1().PersistentVolumeClaims(namespace).Get(pvcName, meta.GetOptions{})
 		if err != nil {
+			if errors.IsNotFound(err) {
+				return util.ARSuccess()
+			}
 			return util.ARFail(err)
 		}
 		err = addDeferDeletingToPVC(pvc, pc.kubeCli, payload.tc)