Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: optionally disable the webhook server #272

Merged
merged 2 commits into from
Jul 16, 2024
Merged

Conversation

wparr-circle
Copy link
Contributor

@wparr-circle wparr-circle commented Jul 8, 2024

Fixes #271

When .Values.webhook.pvcMutatingWebhook.enabled is set false, ensure that any of the configuration related to the webhook server is not rendered. ie. cert manager certificate / issuer, deployment volume mount from secret etc.
This is supplemented with a new option --webhook-enabled - which by default is set true, to mirror existing behaviour. When it is explicitly set false, for example when .Values.webhook.pvcMutatingWebhook.enabled is set false the webhook server will not be configured.

ie.

helm install pvc-autoresizer ./charts/pvc-autoresizer --create-namespace --namespace=pvc-autoresizer --set "controller.args.prometheusURL=http://prom-kube-prometheus-stack-prometheus.default.svc:9090/" --set "image.tag=devel" --set "webhook.pvcMutatingWebhook.enabled=false"
NAME: pvc-autoresizer
LAST DEPLOYED: Mon Jul  8 11:51:45 2024
NAMESPACE: pvc-autoresizer
STATUS: deployed
REVISION: 1
TEST SUITE: None
kubectl -n pvc-autoresizer logs -l app.kubernetes.io/name=pvc-autoresizer --since 24h -f
{"level":"info","ts":"2024-07-08T10:51:46Z","logger":"setup","msg":"starting manager"}
{"level":"info","ts":"2024-07-08T10:51:46Z","logger":"controller-runtime.metrics","msg":"Starting metrics server"}
{"level":"info","ts":"2024-07-08T10:51:46Z","logger":"controller-runtime.metrics","msg":"Serving metrics server","bindAddress":":8080","secure":false}
{"level":"info","ts":"2024-07-08T10:51:46Z","msg":"starting server","kind":"health probe","addr":"[::]:8081"}
I0708 10:51:46.417637       1 leaderelection.go:250] attempting to acquire leader lease pvc-autoresizer/49e22f61.topolvm.io...
I0708 10:51:46.422308       1 leaderelection.go:260] successfully acquired lease pvc-autoresizer/49e22f61.topolvm.io

daichimukai
daichimukai previously approved these changes Jul 9, 2024
cmd/main.go Outdated Show resolved Hide resolved
@daichimukai daichimukai dismissed their stale review July 9, 2024 05:20

I mistakenly clicked the approve button.

@pluser
Copy link
Contributor

pluser commented Jul 10, 2024

Once mukai's point is corrected, there are no other concerns for me.

Signed-off-by: wparr-circle <william.parr@circle.com>
Signed-off-by: wparr-circle <william.parr@circle.com>
@wparr-circle
Copy link
Contributor Author

Last force push there, to trigger a re-run of some failing tests

Copy link
Contributor

@daichimukai daichimukai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pluser pluser merged commit 4310bb9 into topolvm:main Jul 16, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Cannot disable webhooks via .Values.webhook.pvcMutatingWebhook.enabled
3 participants