Support to deploy v1 admission webhook (#4478)

charts/tidb-operator/templates/admission/admission-webhook-registration.yaml

@@ -1,5 +1,5 @@
 {{- if .Values.admissionWebhook.create }}
-apiVersion: apiregistration.k8s.io/v1beta1
+apiVersion: apiregistration.k8s.io/v1
 kind: APIService
 metadata:
   name: v1alpha1.admission.tidb.pingcap.com
@@ -19,13 +19,13 @@ spec:
   group: admission.tidb.pingcap.com
   groupPriorityMinimum: 1000
   versionPriority: 15
+  version: v1alpha1
   service:
     name: tidb-admission-webhook
     namespace: {{ .Release.Namespace }}
-  version: v1alpha1
 ---
 {{- if .Values.admissionWebhook.validation.statefulSets }}
-apiVersion: admissionregistration.k8s.io/v1beta1
+apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
   name: validation-tidb-statefulset-webhook-cfg
@@ -38,12 +38,12 @@ metadata:
     helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
 webhooks:
   - name: stsadmission.tidb.pingcap.com
-    {{- if semverCompare ">=1.15-0" .Capabilities.KubeVersion.GitVersion }}
     objectSelector:
       matchLabels:
         "app.kubernetes.io/managed-by": "tidb-operator"
-    {{- end }}
-    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.validation | default "Ignore" }}
+    admissionReviewVersions: ["v1beta1"]
+    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.validation | default "Fail" }}
+    sideEffects: None
     clientConfig:
       service:
         name: kubernetes
@@ -66,7 +66,7 @@ webhooks:
 {{- end }}
 ---
 {{- if .Values.admissionWebhook.validation.pingcapResources }}
-apiVersion: admissionregistration.k8s.io/v1beta1
+apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
   name: pingcap-tidb-resources-validating
@@ -79,7 +79,9 @@ metadata:
     helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
 webhooks:
   - name: validating.admission.tidb.pingcap.com
-    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.validation | default "Ignore" }}
+    admissionReviewVersions: ["v1beta1"]
+    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.validation | default "Fail" }}
+    sideEffects: None
     clientConfig:
       service:
         name: kubernetes
@@ -98,7 +100,7 @@ webhooks:
 {{- end }}
 ---
 {{- if .Values.admissionWebhook.mutation.pingcapResources }}
-apiVersion: admissionregistration.k8s.io/v1beta1
+apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
 metadata:
   name: pingcap-tidb-resources-defaulitng
@@ -111,7 +113,9 @@ metadata:
     helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+"  "_" }}
 webhooks:
   - name: defaulting.admission.tidb.pingcap.com
-    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.mutation | default "Ignore" }}
+    admissionReviewVersions: ["v1beta1"]
+    failurePolicy: {{ .Values.admissionWebhook.failurePolicy.mutation | default "Fail" }}
+    sideEffects: None
     clientConfig:
       service:
         name: kubernetes

charts/tidb-operator/values.yaml

@@ -230,11 +230,9 @@ admissionWebhook:
   ## refer to https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#failure-policy
   failurePolicy:
     ## the validation webhook would check the request of the given resources.
-    ## If the kubernetes api-server version >= 1.15.0, we recommend the failurePolicy as Fail, otherwise, as Ignore.
-    validation: Ignore
+    validation: Fail
     ## the mutation webhook would mutate the request of the given resources.
-    ## If the kubernetes api-server version >= 1.15.0, we recommend the failurePolicy as Fail, otherwise, as Ignore.
-    mutation: Ignore
+    mutation: Fail
   ## tidb-admission-webhook deployed as kubernetes apiservice server
   ## refer to https://github.com/openshift/generic-admission-server
   apiservice:

cmd/admission-webhook/main.go

@@ -35,6 +35,9 @@ var (
 
 func init() {
 	flag.CommandLine.Init(os.Args[0], flag.ContinueOnError)
+	// Define the flag "secure-port" to avoid the `flag.Parse()` reporting error
+	// TODO: remove this flag after we don't use the lib "github.com/openshift/generic-admission-server"
+	flag.Int("secure-port", 6443, "The port on which to serve HTTPS with authentication and authorization. If 0, don't serve HTTPS at all.")
 	flag.BoolVar(&printVersion, "V", false, "Show version and quit")
 	flag.BoolVar(&printVersion, "version", false, "Show version and quit")
 	flag.StringVar(&extraServiceAccounts, "extraServiceAccounts", "", "comma-separated, extra Service Accounts the Webhook should control. The full pattern for each common service account is system:serviceaccount:<namespace>:<serviceaccount-name>")

tests/e2e/tidbcluster/serial.go

@@ -166,13 +166,10 @@ var _ = ginkgo.Describe("[Serial]", func() {
 			tc.Spec.PD.Replicas = 3
 			tc.Spec.TiKV.Replicas = 3
 			tc.Spec.TiDB.Replicas = 2
-			tc, err := cli.PingcapV1alpha1().TidbClusters(tc.Namespace).Create(context.TODO(), tc, metav1.CreateOptions{})
-			framework.ExpectNoError(err, "failed to create TidbCluster: %v", tc)
-			err = oa.WaitForTidbClusterReady(tc, 30*time.Minute, 5*time.Second)
-			framework.ExpectNoError(err, "failed to wait for TidbCluster ready: %v", tc)
+			utiltc.MustCreateTCWithComponentsReady(genericCli, oa, tc, 6*time.Minute, 5*time.Second)
 
 			ginkgo.By("Set tikv partition annotation to 1")
-			err = setPartitionAnnotation(ns, tc.Name, label.TiKVLabelVal, 1)
+			err := setPartitionAnnotation(ns, tc.Name, label.TiKVLabelVal, 1)
 			framework.ExpectNoError(err, "set tikv Partition annotation failed")
 
 			ginkgo.By(fmt.Sprintf("Upgrade TidbCluster version to %q", utilimage.TiDBLatest))
@@ -632,9 +629,10 @@ var _ = ginkgo.Describe("[Serial]", func() {
 
 	ginkgo.Describe("Upgrade TiDB Operator", func() {
 		var (
-			operatorVersion string
-			oa              *tests.OperatorActions
-			ocfg            *tests.OperatorConfig
+			operatorVersion   string
+			oa                *tests.OperatorActions
+			ocfg              *tests.OperatorConfig
+			setOperatorConfig func(oa *tests.OperatorConfig)
 		)
 
 		ginkgo.JustBeforeEach(func() {
@@ -646,6 +644,10 @@ var _ = ginkgo.Describe("[Serial]", func() {
 				Image:       fmt.Sprintf("pingcap/tidb-operator:%s", operatorVersion),
 			}
 			oa = tests.NewOperatorActions(cli, c, asCli, aggrCli, apiExtCli, tests.DefaultPollInterval, ocfg, e2econfig.TestConfig, fw, f)
+			if setOperatorConfig != nil {
+				setOperatorConfig(ocfg)
+			}
+
 			ginkgo.By("Installing CRDs")
 			oa.CleanCRDOrDie()
 			oa.CreateReleasedCRDOrDie(operatorVersion)
@@ -693,6 +695,32 @@ var _ = ginkgo.Describe("[Serial]", func() {
 				err := utilpod.WaitForPodsAreChanged(c, pods, time.Minute*5)
 				framework.ExpectEqual(err, wait.ErrWaitTimeout, "pods should not change in 5 minutes")
 			})
+
+			ginkgo.Context("Admission Webhook", func() {
+				ginkgo.BeforeEach(func() {
+					setOperatorConfig = func(oa *tests.OperatorConfig) {
+						oa.TestMode = true
+						oa.WebhookEnabled = true
+						oa.StsWebhookEnabled = true
+					}
+				})
+
+				ginkgo.It("should be able to be deployed", func() {
+					ginkgo.By("Wait old webhook pods to become ready")
+					err := oa.WaitAdmissionWebhookReady(ocfg, time.Minute*3, time.Second*10)
+					framework.ExpectNoError(err, "failed to wait old webhook pods to become ready")
+
+					ginkgo.By("Upgrade TiDB Operator and CRDs to current version")
+					ocfg.Tag = cfg.OperatorTag
+					ocfg.Image = cfg.OperatorImage
+					oa.ReplaceCRDOrDie(ocfg)
+					oa.UpgradeOperatorOrDie(ocfg)
+
+					ginkgo.By("Wait webhook pods to become ready")
+					err = oa.WaitAdmissionWebhookReady(ocfg, time.Minute*3, time.Second*10)
+					framework.ExpectNoError(err, "failed to wait webhook pods to become ready")
+				})
+			})
 		})
 
 		ginkgo.Context("From prev major version", func() {

tests/e2e/util/pod/pod.go

@@ -84,3 +84,13 @@ func MustListPods(labelSelector string, ns string, c clientset.Interface) []v1.P
 
 	return pods
 }
+
+func ListContainerFromPod(spec v1.PodSpec, pred func(container v1.Container) bool) []v1.Container {
+	list := make([]v1.Container, 0)
+	for _, container := range spec.Containers {
+		if pred(container) {
+			list = append(list, container)
+		}
+	}
+	return list
+}

tests/webhook.go

@@ -17,9 +17,14 @@ import (
 	"context"
 	"encoding/base64"
 	"fmt"
+	"time"
 
+	podutil "github.com/pingcap/tidb-operator/tests/e2e/util/pod"
+
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilversion "k8s.io/apimachinery/pkg/util/version"
+	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/kubernetes/test/e2e/framework/log"
 )
 
@@ -48,3 +53,50 @@ func (oa *OperatorActions) setCabundleFromApiServer(info *OperatorConfig) error
 	}
 	return nil
 }
+
+func (oa *OperatorActions) WaitAdmissionWebhookReady(info *OperatorConfig, timeout, pollInterval time.Duration) error {
+	var lastErr, err error
+	err = wait.PollImmediate(pollInterval, timeout, func() (done bool, err error) {
+		deploymentName := "tidb-admission-webhook"
+		deploymentID := fmt.Sprintf("%s/%s", info.Namespace, deploymentName)
+
+		deployment, err := oa.kubeCli.AppsV1().Deployments(info.Namespace).Get(context.TODO(), deploymentName, metav1.GetOptions{})
+		if err != nil {
+			lastErr = fmt.Errorf("failed to get deployment %q: %v", deploymentID, err)
+			return false, nil
+		}
+
+		containers := podutil.ListContainerFromPod(deployment.Spec.Template.Spec, func(container v1.Container) bool {
+			if container.Name != "admission-webhook" {
+				return false
+			}
+			if container.Image != info.Image {
+				return false
+			}
+			return true
+		})
+		if len(containers) == 0 {
+			lastErr = fmt.Errorf("failed to find container for deployment %q", deploymentID)
+			return false, nil
+		}
+
+		if deployment.Status.UpdatedReplicas != *deployment.Spec.Replicas {
+			lastErr = fmt.Errorf("not all replication are updated for deployement %q, ready: %d, spec: %d",
+				deploymentID, deployment.Status.ReadyReplicas, *deployment.Spec.Replicas)
+			return false, nil
+		}
+
+		if deployment.Status.ReadyReplicas != *deployment.Spec.Replicas {
+			lastErr = fmt.Errorf("not all replication are ready for deployment %q, ready: %d, spec: %d",
+				deploymentID, deployment.Status.ReadyReplicas, *deployment.Spec.Replicas)
+			return false, nil
+		}
+
+		return true, nil
+	})
+
+	if err == wait.ErrWaitTimeout {
+		return lastErr
+	}
+	return err
+}