From 21e650c5b4cba7b2fa77b6ab91f0bc438f88ebcc Mon Sep 17 00:00:00 2001 From: Song Gao Date: Wed, 18 Mar 2020 13:27:44 +0800 Subject: [PATCH] Fix some webhook error (#1963) * fix webhook error --- .../admission-webhook-deployment.yaml | 5 +++++ cmd/admission-webhook/main.go | 8 +++++++ pkg/webhook/pod/pods.go | 9 +++++--- pkg/webhook/pod/tikv_deleter.go | 22 ++++++++++++++++--- pkg/webhook/pod/tikv_deleter_test.go | 4 ++-- 5 files changed, 40 insertions(+), 8 deletions(-) diff --git a/charts/tidb-operator/templates/admission/admission-webhook-deployment.yaml b/charts/tidb-operator/templates/admission/admission-webhook-deployment.yaml index 228cdb590be..eb3b46bc85e 100644 --- a/charts/tidb-operator/templates/admission/admission-webhook-deployment.yaml +++ b/charts/tidb-operator/templates/admission/admission-webhook-deployment.yaml @@ -53,6 +53,11 @@ spec: scheme: HTTPS initialDelaySeconds: 5 timeoutSeconds: 5 + env: + - name: NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace {{- if eq .Values.admissionWebhook.apiservice.insecureSkipTLSVerify false }} volumeMounts: - mountPath: /var/serving-cert diff --git a/cmd/admission-webhook/main.go b/cmd/admission-webhook/main.go index 9df37ba7523..600f86c7585 100644 --- a/cmd/admission-webhook/main.go +++ b/cmd/admission-webhook/main.go @@ -15,6 +15,7 @@ package main import ( "flag" + "fmt" "os" "time" @@ -22,6 +23,7 @@ import ( "github.com/pingcap/tidb-operator/pkg/features" "github.com/pingcap/tidb-operator/pkg/version" "github.com/pingcap/tidb-operator/pkg/webhook" + "github.com/pingcap/tidb-operator/pkg/webhook/pod" "k8s.io/component-base/logs" "k8s.io/klog" ) @@ -59,5 +61,11 @@ func main() { ExtraServiceAccounts: extraServiceAccounts, EvictRegionLeaderTimeout: evictRegionLeaderTimeout, } + ns := os.Getenv("NAMESPACE") + if len(ns) < 1 { + klog.Fatal("ENV NAMESPACE should be set.") + } + pod.AstsControllerServiceAccounts = fmt.Sprintf("system:serviceaccount:%s:advanced-statefulset-controller", ns) + cmd.RunAdmissionServer(ah) } diff --git a/pkg/webhook/pod/pods.go b/pkg/webhook/pod/pods.go index 774d154214e..c4ac21050a2 100644 --- a/pkg/webhook/pod/pods.go +++ b/pkg/webhook/pod/pods.go @@ -47,8 +47,11 @@ type PodAdmissionControl struct { } const ( - stsControllerServiceAccounts = "system:serviceaccount:kube-system:statefulset-controller" - astsControllerServiceAccounts = "system:serviceaccount:pingcap:advanced-statefulset-controller" + stsControllerServiceAccounts = "system:serviceaccount:kube-system:statefulset-controller" +) + +var ( + AstsControllerServiceAccounts string ) func NewPodAdmissionControl(kubeCli kubernetes.Interface, operatorCli versioned.Interface, PdControl pdapi.PDControlInterface, extraServiceAccounts []string, evictRegionLeaderTimeout time.Duration) *PodAdmissionControl { @@ -58,7 +61,7 @@ func NewPodAdmissionControl(kubeCli kubernetes.Interface, operatorCli versioned. serviceAccounts.Insert(sa) } if features.DefaultFeatureGate.Enabled(features.AdvancedStatefulSet) { - serviceAccounts.Insert(astsControllerServiceAccounts) + serviceAccounts.Insert(AstsControllerServiceAccounts) } EvictLeaderTimeout = evictRegionLeaderTimeout return &PodAdmissionControl{ diff --git a/pkg/webhook/pod/tikv_deleter.go b/pkg/webhook/pod/tikv_deleter.go index 60ea1a65e20..fb5fe443bdb 100644 --- a/pkg/webhook/pod/tikv_deleter.go +++ b/pkg/webhook/pod/tikv_deleter.go @@ -100,9 +100,9 @@ func (pc *PodAdmissionControl) admitDeleteTiKVPods(payload *admitPayload) *admis case v1alpha1.TiKVStateTombstone: return pc.admitDeleteUselessTiKVPod(payload) case v1alpha1.TiKVStateOffline: - return pc.admitDeleteOfflineTiKVPod() + return pc.rejectDeleteTiKVPod() case v1alpha1.TiKVStateDown: - return pc.admitDeleteUselessTiKVPod(payload) + return pc.admitDeleteDownTikvPod(payload) case v1alpha1.TiKVStateUp: return pc.admitDeleteUpTiKVPod(payload, storeInfo, storesInfo) default: @@ -147,7 +147,7 @@ func (pc *PodAdmissionControl) admitDeleteUselessTiKVPod(payload *admitPayload) return util.ARSuccess() } -func (pc *PodAdmissionControl) admitDeleteOfflineTiKVPod() *admission.AdmissionResponse { +func (pc *PodAdmissionControl) rejectDeleteTiKVPod() *admission.AdmissionResponse { return &admission.AdmissionResponse{ Allowed: false, } @@ -217,3 +217,19 @@ func (pc *PodAdmissionControl) admitDeleteUpTiKVPodDuringUpgrading(payload *admi return util.ARSuccess() } + +// When the target tikv's store is DOWN, we would not pass the deleting request during scale-in, otherwise it would cause +// the duplicated id problem for the newly tikv pod. +// Users should offline the target tikv into tombstone first, then scale-in it. +// In other cases, we would admit to delete the down tikv pod like upgrading. +func (pc *PodAdmissionControl) admitDeleteDownTikvPod(payload *admitPayload) *admission.AdmissionResponse { + + isInOrdinal, err := operatorUtils.IsPodOrdinalNotExceedReplicas(payload.pod, payload.ownerStatefulSet) + if err != nil { + return util.ARFail(err) + } + if !isInOrdinal { + return pc.rejectDeleteTiKVPod() + } + return util.ARSuccess() +} diff --git a/pkg/webhook/pod/tikv_deleter_test.go b/pkg/webhook/pod/tikv_deleter_test.go index bf3b42a11ad..b0e7d0c8d3e 100644 --- a/pkg/webhook/pod/tikv_deleter_test.go +++ b/pkg/webhook/pod/tikv_deleter_test.go @@ -212,7 +212,7 @@ func TestTiKVDeleterDelete(t *testing.T) { UpdatePVCErr: false, PVCNotFound: false, expectFn: func(g *GomegaWithT, response *admission.AdmissionResponse) { - g.Expect(response.Allowed).Should(Equal(true)) + g.Expect(response.Allowed).Should(Equal(false)) }, }, { @@ -236,7 +236,7 @@ func TestTiKVDeleterDelete(t *testing.T) { UpdatePVCErr: true, PVCNotFound: true, expectFn: func(g *GomegaWithT, response *admission.AdmissionResponse) { - g.Expect(response.Allowed).Should(Equal(true)) + g.Expect(response.Allowed).Should(Equal(false)) }, }, {