diff --git a/charts/tidb-operator/templates/admission/admission-webhook-registration.yaml b/charts/tidb-operator/templates/admission/admission-webhook-registration.yaml index 53a8d6fbdc..27d00773f5 100644 --- a/charts/tidb-operator/templates/admission/admission-webhook-registration.yaml +++ b/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 diff --git a/charts/tidb-operator/values.yaml b/charts/tidb-operator/values.yaml index f9c163188b..8e70b45c2d 100644 --- a/charts/tidb-operator/values.yaml +++ b/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: diff --git a/cmd/admission-webhook/main.go b/cmd/admission-webhook/main.go index 27748b4ff0..baea62b1d0 100644 --- a/cmd/admission-webhook/main.go +++ b/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::") diff --git a/tests/e2e/tidbcluster/serial.go b/tests/e2e/tidbcluster/serial.go index ce57176030..30221d126f 100644 --- a/tests/e2e/tidbcluster/serial.go +++ b/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() { diff --git a/tests/e2e/util/pod/pod.go b/tests/e2e/util/pod/pod.go index 8670fc9df8..ec2d6cb9e6 100644 --- a/tests/e2e/util/pod/pod.go +++ b/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 +} diff --git a/tests/webhook.go b/tests/webhook.go index cd296afa35..98f9e7fdab 100644 --- a/tests/webhook.go +++ b/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 +}