cisco-open / cluster-registry-controller

An operator that automatically synchronizes Kubernetes resources across multiple clusters
Apache License 2.0
22 stars 8 forks source link

Add readiness probe to check webhook cert ready #33

Closed nishantapatil3 closed 1 year ago

nishantapatil3 commented 1 year ago
Q A
Bug fix? no yes
New feature? yes (/readyz as readiness probe)
API breaks? yes (/metrics as readiness probe)
Deprecations? yes (/metrics as readiness probe)
Related tickets
License Apache 2.0

What's in this PR?

Add readiness probe to mark cluster-registry as "ready" only after webhook configuration certificate is updated successfully

Why?

cluster-registry generates self signed certificate to inject into webhook which take approx ~30 sec after deploy.

If we are running an automation script that waits for cluster-registrty to be ready then /metrics end point marks container as ready even before caBundle in injected into ValidatingWebhookConfig.

As a result, the automation script would fail with

x509: certificate is not valid for any names, but wanted to match cluster-registry-controller.cluster-registry.svc

so we need to wait until the webhook is ready, then mark cluster-registry as "ready"

Additional context

This PR changes the readiness probe from /metrics to /readyz which would potentially break API that depends on readiness probe

Checklist

To Do

Tests

nishantapatil3 commented 1 year ago

CC: @pregnor @waynz0r @Laci21 @tiswanso (I couldnt assign reviewers)

nishantapatil3 commented 1 year ago

@pregnor thanks for the review.

Added manual test cases in PR summary to verify pod readiness after deploy using kubectl get pod -n cluster-registry cluster-registry-controller-controller-xxx -o yaml from status field

nishantapatil3 commented 1 year ago

Thanks for the review guys, this is my first pull request so i am unable to merge.. please merge when possible, I'll create another PR to update charts and image versions

nishantapatil3 commented 1 year ago

Thanks for the review and merge @Laci21 @pregnor @waynz0r