Webhook for PD upgrader #1040

Yisaer · 2019-10-23T06:33:46Z

What problem does this PR solve?

What is changed and how does it work?

Tidb-Operator will only need to edit statefulset spec during pd upgrading and to edit replicas during pd scale-in. The webhook server will ensure that each pd pod will be safely deleted.

And I also add one more component as an init job to auto-create CA cert and csr for webhook configuration which would be much user-friendly.

Currently I package tidb-operator-initializer in the same docker image as others and install openssl and kubectl in tidb-operator image which initializer use these in the script to manage CA cert.

Packaging all necessary process into 2 images would need us to change some e2e test script which would have huge influence for other e2e test.

After we moved our e2e CI script into code repo instead of jenkins server, I would pakcage tidb-operator-initializer in a different image so that other e2e test won't be influenced.

Check List

Tests

Unit test
E2E test
Manual test (add detailed scripts or steps below)

Code changes

Has Helm charts change
Has Go code change

Side effects
N/A

Related changes

Kubernetes 1.9.0 or above with the admissionregistration.k8s.io/v1beta1 API enabled.

Yisaer · 2019-10-29T13:12:55Z

/run-e2e-test

Yisaer · 2019-10-31T13:49:01Z

/run-e2e-in-kind

Yisaer · 2019-10-31T13:54:38Z

charts/tidb-operator/values.yaml

+      cpu: 80m
+      memory: 50Mi
+  ## Interval Hour to Refresh CA Cert
+  certRefreshIntervalHour: 336


AdmissionController will refresh CA cert before certRefreshIntervalHour hour on its expireDate.

Yisaer · 2019-10-31T14:02:14Z

cmd/admission-controller/main.go

+	}
+
+	/*
+	* currently we only use one replica for admission-controller-webhook


Currently we just set replica as 1 for admissioncontroller so we don't have lease lock from apiServer as we are using Lister here.

Yisaer · 2019-10-31T14:10:23Z

pkg/util/certUtil.go

+	return hex.EncodeToString(out)
+}
+
+func GenerateRSAKeyPair(namespace, serviceName string) ([]byte, []byte, error) {


CertUtil help us generate CA Cert for a certain service with ten years default validity period

Yisaer · 2019-10-31T14:12:02Z

pkg/webhook/pod/pods.go

+	}
+}
+
+func (pc *PodAdmissionControl) AdmitPods(ar v1beta1.AdmissionReview) *v1beta1.AdmissionResponse {


AdmitPods will only check AdmissionReview with Delete Operation.

Yisaer · 2019-10-31T14:13:26Z

pkg/webhook/pod/pods.go

+
+	if l.IsPD() {
+		pdClient := pc.pdControl.GetPDClient(pdapi.Namespace(namespace), tcName, tc.Spec.EnableTLSCluster)
+		return pc.admitDeletePdPods(pod, ownerStatefulSet, tc, pdClient)


Currently we only check Delete AdmissionReview For PD Pod.

Yisaer · 2019-10-31T14:15:10Z

pkg/webhook/route.go

@@ -89,9 +89,14 @@ func serve(w http.ResponseWriter, r *http.Request, admit admitFunc) {
 	responseAdmissionReview.Response.UID = requestedAdmissionReview.Request.UID

 	marshalAndWrite(responseAdmissionReview, w)
+	webhookHandler.RefreshCertPEMExpirationHandler(RefreshJobConfig)


WebHook Server will check whether CA cert would be expired at each serving request.

Yisaer · 2019-10-31T14:16:01Z

tests/cmd/e2e/main.go

@@ -133,7 +128,6 @@ func main() {
 		oa.CheckTidbClusterStatusOrDie(cluster)

 		// upgrade
-		oa.RegisterWebHookAndServiceOrDie(certCtx, ocfg)


Remove e2e webhook.

…-operator into Yisaer-pd_delete_pods_webhook

pkg/initializer/initializer.go

Yisaer · 2019-11-01T11:20:42Z

@tennix @weekface @cofyc @aylei PTAL.

cofyc · 2019-11-01T12:41:54Z

Can we divide the whole process into several steps?

implement the webhook controller and can use the webhook in tidb-operator optionally

we can use hardcoded self-signed certs in e2e tests
- it seems that we can use a custom ca bundle in clientConfig: https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#service-reference
add a feature switch to disable the logic in controller-manager to work with webhook admission controller

in this step, we focus on webhook logic

deploy/configure webhook controller

request/sign/configure/renew cert for webhook controller

in this step, we focus on webhook deployment and management

use the webhook controller by default

when it's stable, we can switch to use webhook controller by default but have an option to switch back

production-ready

when it's production-ready, we can remove the old logic in the controller and require the webhook to exist in our deployment

In each step, we introduce small changes to our code. It's easier to review and we can have enough tests before enabling the new feature by default.

Yisaer · 2019-11-01T12:55:16Z

/run-e2e-in-kind

DanielZhangQD · 2019-11-02T06:35:05Z

charts/tidb-operator/templates/scripts/_create_cert.sh.tpl

+spec:
+  groups:
+  - system:authenticated
+  request: $(cat ${tmpdir}/server.csr | base64 | tr -d '\n')


base64 | tr -d '\n' can be base64 -w0

DanielZhangQD · 2019-11-02T06:36:54Z

charts/tidb-operator/templates/scripts/_create_cert.sh.tpl

+    kubectl get csr ${csrName}
+    if [ "$?" -eq 0 ]; then
+        break
+    fi


Would it be better to sleep a while here?

charts/tidb-operator/templates/scripts/_create_cert.sh.tpl

DanielZhangQD · 2019-11-02T07:45:40Z

pkg/initializer/initializer.go

+	}
+
+	server.Spec.Template.Annotations = map[string]string{
+		"checksum/config": certUtils.Checksum(secret.Data["cert.pem"]),


Suggested change

"checksum/config": certUtils.Checksum(secret.Data["cert.pem"]),

"checksum/cert": certUtils.Checksum(secret.Data["cert.pem"]),

BTW, no need to preserve other annotations?

pkg/webhook/pod/pd_deleter.go

Co-Authored-By: DanielZhangQD <36026334+DanielZhangQD@users.noreply.github.com>

tennix · 2019-11-13T13:20:16Z

Is this PR still needed after #1101 is merged?

Yisaer · 2019-12-18T08:46:37Z

This request is no more needed now.

This comment has been minimized.

Sign in to view

Yisaer added the enhancement New feature or request label Oct 25, 2019

Yisaer force-pushed the pd_delete_pods_webhook branch from add10a2 to c1d1bd5 Compare October 25, 2019 09:53

tennix added the type/refactor label Oct 29, 2019

Yisaer force-pushed the pd_delete_pods_webhook branch 2 times, most recently from 790c92f to 563054c Compare October 29, 2019 10:48

Yisaer force-pushed the pd_delete_pods_webhook branch from 5623ac0 to 8ad32bf Compare October 29, 2019 13:46

aylei mentioned this pull request Oct 30, 2019

Install tidb-apiserver in tidb-operator chart #1083

Merged

Yisaer force-pushed the pd_delete_pods_webhook branch from 2ec236a to 404ede4 Compare October 31, 2019 06:07

Yisaer commented Oct 31, 2019

View reviewed changes

pd webhook refactor

cde02ab

Yisaer force-pushed the pd_delete_pods_webhook branch from 9529337 to cde02ab Compare November 1, 2019 04:36

Song Gao added 6 commits November 1, 2019 12:44

Merge branch 'pd_delete_pods_webhook' of git://github.com/Yisaer/tidb…

2460195

…-operator into Yisaer-pd_delete_pods_webhook

Update go.mod

0ec2855

Merge branch 'Yisaer-pd_delete_pods_webhook' into pd_delete_pods_webhook

2078d1b

Merge branch 'Yisaer-pd_delete_pods_webhook' into pd_delete_pods_webhook

e3a4272

fix certByte

7a7c283

fix cert

3481528

Yisaer commented Nov 1, 2019

View reviewed changes

pkg/initializer/initializer.go Outdated Show resolved Hide resolved

Song Gao and others added 3 commits November 1, 2019 18:37

cert-manager

9f9a4fc

Merge branch 'master' into pd_delete_pods_webhook

af61a1f

Update Makefile

7f85ca8

Song Gao added 2 commits November 1, 2019 18:52

initializer-image

76f8da4

go fmt

bfb27e1

Yisaer requested review from weekface, tennix, aylei and cofyc November 1, 2019 11:21

Song Gao added 5 commits November 1, 2019 19:40

fix image

ab647b1

fix lint

78f59dc

fix Dockerfile

f5fee7d

fix initializer rbac

88b6e0f

fix wait

0e475a7

Song Gao added 2 commits November 1, 2019 20:41

dig into

c34ad79

fix rbac

11552fa

DanielZhangQD reviewed Nov 2, 2019

View reviewed changes

Update charts/tidb-operator/templates/scripts/_create_cert.sh.tpl

4e10f5b

Co-Authored-By: DanielZhangQD <36026334+DanielZhangQD@users.noreply.github.com>

Yisaer mentioned this pull request Nov 4, 2019

Add admission to delete PD Pod handler for webhook #1101

Merged

aylei removed their request for review November 11, 2019 06:21

Yisaer closed this Dec 18, 2019

Yisaer deleted the pd_delete_pods_webhook branch February 26, 2020 04:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Webhook for PD upgrader #1040

Webhook for PD upgrader #1040

Yisaer commented Oct 23, 2019 •

edited

Loading

This comment has been minimized.

Yisaer commented Oct 29, 2019

Yisaer commented Oct 31, 2019

Yisaer Oct 31, 2019

Yisaer Oct 31, 2019

Yisaer Oct 31, 2019

Yisaer Oct 31, 2019

Yisaer Oct 31, 2019

Yisaer Oct 31, 2019

Yisaer Oct 31, 2019

Yisaer commented Nov 1, 2019

cofyc commented Nov 1, 2019 •

edited

Loading

Yisaer commented Nov 1, 2019

DanielZhangQD Nov 2, 2019

DanielZhangQD Nov 2, 2019

DanielZhangQD Nov 2, 2019

tennix commented Nov 13, 2019

Yisaer commented Dec 18, 2019

	"checksum/config": certUtils.Checksum(secret.Data["cert.pem"]),
	"checksum/cert": certUtils.Checksum(secret.Data["cert.pem"]),

Webhook for PD upgrader #1040

Webhook for PD upgrader #1040

Conversation

Yisaer commented Oct 23, 2019 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

This comment has been minimized.

Yisaer commented Oct 29, 2019

Yisaer commented Oct 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yisaer commented Nov 1, 2019

cofyc commented Nov 1, 2019 • edited Loading

implement the webhook controller and can use the webhook in tidb-operator optionally

deploy/configure webhook controller

use the webhook controller by default

production-ready

Yisaer commented Nov 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tennix commented Nov 13, 2019

Yisaer commented Dec 18, 2019

Yisaer commented Oct 23, 2019 •

edited

Loading

cofyc commented Nov 1, 2019 •

edited

Loading