Add webhook switch for tidb-controller-manager (#1389)

charts/tidb-operator/templates/admission/pre-delete-job.yaml

@@ -0,0 +1,100 @@
+{{- if and ( .Values.admissionWebhook.create ) ( .Values.admissionWebhook.hooksEnabled.pods ) }}
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: {{ .Release.Name }}
+  labels:
+    app.kubernetes.io/name: {{ template "chart.name" . }}
+    app.kubernetes.io/managed-by: {{ .Release.Service }}
+    app.kubernetes.io/instance: {{ .Release.Name }}
+    app.kubernetes.io/component: admission-webhook
+    helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
+  annotations:
+    # This is what defines this resource as a hook. Without this line, the
+    # job is considered part of the release.
+    "helm.sh/hook": pre-delete
+    "helm.sh/hook-weight": "-1"
+    "helm.sh/hook-delete-policy": hook-succeeded,hook-failed
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: {{ .Release.Name }}
+  labels:
+    app.kubernetes.io/name: {{ template "chart.name" . }}
+    app.kubernetes.io/managed-by: {{ .Release.Service }}
+    app.kubernetes.io/instance: {{ .Release.Name }}
+    app.kubernetes.io/component: admission-webhook
+    helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
+  annotations:
+    # This is what defines this resource as a hook. Without this line, the
+    # job is considered part of the release.
+    "helm.sh/hook": pre-delete
+    "helm.sh/hook-weight": "-1"
+    "helm.sh/hook-delete-policy": hook-succeeded,hook-failed
+rules:
+  - apiGroups: ["admissionregistration.k8s.io"]
+    resources: ["validatingwebhookconfigurations"]
+    verbs: ["delete"]
+---
+
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: {{ .Release.Name }}
+  labels:
+    app.kubernetes.io/name: {{ template "chart.name" . }}
+    app.kubernetes.io/managed-by: {{ .Release.Service }}
+    app.kubernetes.io/instance: {{ .Release.Name }}
+    app.kubernetes.io/component: admission-webhook
+    helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
+  annotations:
+    # This is what defines this resource as a hook. Without this line, the
+    # job is considered part of the release.
+    "helm.sh/hook": pre-delete
+    "helm.sh/hook-weight": "-1"
+    "helm.sh/hook-delete-policy": hook-succeeded,hook-failed
+subjects:
+  - kind: ServiceAccount
+    name: {{ .Release.Name }}
+    namespace: {{ .Release.Namespace }}
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: {{ .Release.Name }}
+
+---
+apiVersion: batch/v1
+kind: Job
+metadata:
+  name: webhook-del
+  labels:
+    app.kubernetes.io/name: {{ template "chart.name" . }}
+    app.kubernetes.io/managed-by: {{ .Release.Service }}
+    app.kubernetes.io/instance: {{ .Release.Name }}
+    app.kubernetes.io/component: admission-webhook
+    helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
+  annotations:
+    # This is what defines this resource as a hook. Without this line, the
+    # job is considered part of the release.
+    "helm.sh/hook": pre-delete
+    "helm.sh/hook-weight": "-1"
+    "helm.sh/hook-delete-policy": hook-succeeded,hook-failed
+spec:
+  template:
+    metadata:
+      name: webhook-del
+    spec:
+      restartPolicy: Never
+      serviceAccountName: {{ .Release.Name }}
+      containers:
+        - name: "{{.Release.Name}}-job-del"
+          image: {{ .Values.admissionWebhook.jobImage }}
+          imagePullPolicy: IfNotPresent
+          command:
+            - "sh"
+            - "-c"
+            - |
+              set -e
+              kubectl delete validatingWebhookConfigurations.admissionregistration.k8s.io validation-delete-tidb-admission-webhook-cfg || true
+{{- end }}

charts/tidb-operator/templates/controller-manager-deployment.yaml

@@ -53,6 +53,9 @@ spec:
           {{- if .Values.features }}
           - -features={{ join "," .Values.features }}
           {{- end }}
+          {{- if and ( .Values.admissionWebhook.create ) ( .Values.admissionWebhook.hooksEnabled.pods ) }}
+          - -pod-webhook-enabled=true
+          {{- end }}
         env:
           - name: NAMESPACE
             valueFrom:

charts/tidb-operator/values.yaml

@@ -182,6 +182,10 @@ admissionWebhook:
   serviceAccount: tidb-admission-webhook
   rbac:
     create: true
+  ## jobImage is to indicate the image used in `pre-delete-job.yaml`
+  ## if admissionWebhook.create and admissionWebhook.hooksEnabled.pods are both enabled,
+  ## The pre-delete-job would delete the validationWebhookConfiguration using this image
+  jobImage: "bitnami/kubectl:latest"
   hooksEnabled:
     ## statefulsets hook would check requests for updating tidbcluster's statefulsets
     ## If enabled it, the statefulsets of tidbcluseter would update in partition by tidbcluster's annotation

cmd/controller-manager/main.go

@@ -75,6 +75,7 @@ func init() {
 	flag.StringVar(&controller.TidbBackupManagerImage, "tidb-backup-manager-image", "pingcap/tidb-backup-manager:latest", "The image of backup manager tool")
 	// TODO: actually we just want to use the same image with tidb-controller-manager, but DownwardAPI cannot get image ID, see if there is any better solution
 	flag.StringVar(&controller.TidbDiscoveryImage, "tidb-discovery-image", "pingcap/tidb-operator:latest", "The image of the tidb discovery service")
+	flag.BoolVar(&controller.PodWebhookEnabled, "pod-webhook-enabled", false, "Whether Pod admission webhook is enabled")
 	features.DefaultFeatureGate.AddFlag(flag.CommandLine)
 
 	flag.Parse()

pkg/controller/controller_utils.go

@@ -65,6 +65,9 @@ var (
 
 	// TidbDiscoveryImage is the image of tidb discovery service
 	TidbDiscoveryImage string
+
+	// PodWebhookEnabled is the key to indicate whether pod admission webhook is set up.
+	PodWebhookEnabled bool
 )
 
 const (

pkg/manager/member/pd_scaler.go

@@ -101,6 +101,11 @@ func (psd *pdScaler) ScaleIn(tc *v1alpha1.TidbCluster, oldSet *apps.StatefulSet,
 		return fmt.Errorf("TidbCluster: %s/%s's pd status sync failed,can't scale in now", ns, tcName)
 	}
 
+	if controller.PodWebhookEnabled {
+		decreaseReplicas(newSet, oldSet)
+		return nil
+	}
+
 	pdClient := controller.GetPDClient(psd.pdControl, tc)
 	// If the pd pod was pd leader during scale-in, we would transfer pd leader to pd-0 directly
 	// If the pd statefulSet would be scale-in to zero and the pd-0 was going to be deleted,

pkg/manager/member/pd_upgrader.go

@@ -71,6 +71,10 @@ func (pu *pdUpgrader) gracefulUpgrade(tc *v1alpha1.TidbCluster, oldSet *apps.Sta
 		return nil
 	}
 
+	if controller.PodWebhookEnabled {
+		setUpgradePartition(newSet, 0)
+	}
+
 	setUpgradePartition(newSet, *oldSet.Spec.UpdateStrategy.RollingUpdate.Partition)
 	for i := tc.PDStsActualReplicas() - 1; i >= 0; i-- {
 		podName := PdPodName(tcName, i)

pkg/manager/member/tikv_scaler.go

@@ -80,6 +80,11 @@ func (tsd *tikvScaler) ScaleIn(tc *v1alpha1.TidbCluster, oldSet *apps.StatefulSe
 		return err
 	}
 
+	if controller.PodWebhookEnabled {
+		decreaseReplicas(newSet, oldSet)
+		return nil
+	}
+
 	for _, store := range tc.Status.TiKV.Stores {
 		if store.PodName == podName {
 			state := store.State

pkg/manager/member/tikv_upgrader.go

@@ -86,6 +86,10 @@ func (tku *tikvUpgrader) Upgrade(tc *v1alpha1.TidbCluster, oldSet *apps.Stateful
 		return nil
 	}
 
+	if controller.PodWebhookEnabled {
+		setUpgradePartition(newSet, 0)
+	}
+
 	setUpgradePartition(newSet, *oldSet.Spec.UpdateStrategy.RollingUpdate.Partition)
 	for i := tc.TiKVStsActualReplicas() - 1; i >= 0; i-- {
 		store := tku.getStoreByOrdinal(tc, i)

tests/e2e/tidbcluster/serial.go

@@ -15,8 +15,11 @@ package tidbcluster
 
 import (
 	"context"
+	"fmt"
 	_ "net/http/pprof"
 
+	"k8s.io/klog"
+
 	"github.com/onsi/ginkgo"
 	asclientset "github.com/pingcap/advanced-statefulset/pkg/client/clientset/versioned"
 	"github.com/pingcap/tidb-operator/pkg/client/clientset/versioned"
@@ -96,10 +99,10 @@ var _ = ginkgo.Describe("[tidb-operator][Serial]", func() {
 		})
 
 		ginkgo.AfterEach(func() {
-			ginkgo.By("Uninstalling CRDs")
-			oa.CleanCRDOrDie()
 			ginkgo.By("Uninstall tidb-operator")
 			oa.CleanOperatorOrDie(ocfg)
+			ginkgo.By("Uninstalling CRDs")
+			oa.CleanCRDOrDie()
 		})
 
 		ginkgo.It("able to deploy TiDB Cluster with advanced statefulset", func() {
@@ -109,4 +112,60 @@ var _ = ginkgo.Describe("[tidb-operator][Serial]", func() {
 		})
 	})
 
+	// tidb-operator with pod admission webhook enabled
+	ginkgo.Context("[Feature: PodAdmissionWebhook]", func() {
+
+		var ocfg *tests.OperatorConfig
+		var oa tests.OperatorActions
+
+		ginkgo.BeforeEach(func() {
+			ocfg = &tests.OperatorConfig{
+				Namespace:         "pingcap",
+				ReleaseName:       "operator",
+				Image:             cfg.OperatorImage,
+				Tag:               cfg.OperatorTag,
+				SchedulerImage:    "k8s.gcr.io/kube-scheduler",
+				LogLevel:          "4",
+				ImagePullPolicy:   v1.PullIfNotPresent,
+				TestMode:          true,
+				WebhookEnabled:    true,
+				PodWebhookEnabled: true,
+				StsWebhookEnabled: false,
+			}
+			oa = tests.NewOperatorActions(cli, c, asCli, tests.DefaultPollInterval, ocfg, e2econfig.TestConfig, nil, fw, f)
+			ginkgo.By("Installing CRDs")
+			oa.CleanCRDOrDie()
+			oa.InstallCRDOrDie()
+			ginkgo.By("Installing tidb-operator")
+			oa.CleanOperatorOrDie(ocfg)
+			oa.DeployOperatorOrDie(ocfg)
+		})
+
+		ginkgo.AfterEach(func() {
+			ginkgo.By("Uninstall tidb-operator")
+			oa.CleanOperatorOrDie(ocfg)
+			ginkgo.By("Uninstalling CRDs")
+			oa.CleanCRDOrDie()
+		})
+
+		ginkgo.It("able to upgrade TiDB Cluster with pod admission webhook", func() {
+			klog.Info("start to upgrade tidbcluster with pod admission webhook")
+			// deploy new cluster and test upgrade and scale-in/out with pod admission webhook
+			cluster := newTidbClusterConfig(e2econfig.TestConfig, ns, "admission", "", "")
+			cluster.Resources["pd.replicas"] = "3"
+			cluster.Resources["tikv.replicas"] = "3"
+			cluster.Resources["tidb.replicas"] = "2"
+
+			oa.DeployTidbClusterOrDie(&cluster)
+			oa.CheckTidbClusterStatusOrDie(&cluster)
+			upgradeVersions := cfg.GetUpgradeTidbVersionsOrDie()
+			ginkgo.By(fmt.Sprintf("Upgrading tidb cluster from %s to %s", cluster.ClusterVersion, upgradeVersions[0]))
+			cluster.UpgradeAll(upgradeVersions[0])
+			oa.UpgradeTidbClusterOrDie(&cluster)
+			// TODO: find a more graceful way to check tidbcluster during upgrading
+			oa.CheckTidbClusterStatusOrDie(&cluster)
+			oa.CleanTidbClusterOrDie(&cluster)
+		})
+	})
+
 })