Skip to content

Conversation

@beraldoleal
Copy link
Member

Webhook requires cert-manager and is enabled by default. But users can disable installation if have them already installed.

@beraldoleal beraldoleal requested a review from a team as a code owner January 22, 2026 15:07
@beraldoleal
Copy link
Member Author

@wainersm @stevenhorsman I was planning to update the approach on the previous PR that just got merged, but you guys are on fire and merged before I send a push :) (thanks for that).

So I will let this here for review... I could rebase here, after @wainersm 's PR is merged first.

> [!WARNING]
> The webhook is enabled by default and requires cert-manager for TLS certificates.
> By default, cert-manager will be installed automatically (`webhook.certManager.install=true`).
> If cert-manager is already installed in your cluster, set `--set webhook.certManager.install=false`.
Copy link
Member

Choose a reason for hiding this comment

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

Hi @beraldoleal ,

What's the consequence of trying to install the cert-manager if it is already in the cluster? Will it assume the dependency is match and move on? Or try to update the installed version and move on? Or will do something else? (hope it won't put the cluster in bad state)

Copy link
Member Author

@beraldoleal beraldoleal Jan 23, 2026

Choose a reason for hiding this comment

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

My understanding is that this has some cluster wide resources (like CRDs) and that will fail / conflict. The helm installation would abort, could let the peerpods chart in partial state. You would need to do helm uninstall and try again with the flag set to false.

claude, told-me that it has a flag called "--atomic" that would do that rollback automatically but I never used. From the --help:

" if set, the installation process deletes the installation on failure. The --wait flag will be set automatically if --atomic is used"

(So maybe we should test and document that flag)

@wainersm
Copy link
Member

@wainersm @stevenhorsman I was planning to update the approach on the previous PR that just got merged, but you guys are on fire and merged before I send a push :) (thanks for that).

So I will let this here for review... I could rebase here, after @wainersm 's PR is merged first.

I can test it in my PR. However, I will wait on your reply to my comment (double install cert-manager)...just in case...things my change :)

Webhook requires cert-manager and is enabled by default. But users can
disable installation if have them already installed.

Signed-off-by: Beraldo Leal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants