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

Wrap the cert-manager re-configuration instructions for easier reading #1216

Conversation

wallrj
Copy link
Member

@wallrj wallrj commented Apr 26, 2023

Preview: https://deploy-preview-1216--cert-manager-website.netlify.app/docs/projects/approver-policy/

In the existing documentation , the helm upgrade command also installs and the command is one long line which is difficult to read.

The documentation assumes that if the user has already installed cert-manager, that they used the --set installCRDs=true flag,
which is not the recommended helm install method so I added a note about that.

I considered recommending helm upgrade --reuse-values to retain what ever values the user originally supplied when they installed cert-manager, but that's not safe either, because extraArgs may or may not have already been customized.
Instead I recommended that the user check existing values before reconfiguring.

@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 26, 2023
@netlify
Copy link

netlify bot commented Apr 26, 2023

Deploy Preview for cert-manager-website ready!

Name Link
🔨 Latest commit 08e028d
🔍 Latest deploy log https://app.netlify.com/sites/cert-manager-website/deploys/644a39e307891d0008506222
😎 Deploy Preview https://deploy-preview-1216--cert-manager-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@wallrj wallrj force-pushed the clearer-cert-manager-reconfiguration-instructions branch from 4ee90ad to 4695a37 Compare April 27, 2023 08:54
And explain that extraArgs and version must be specified to avoid changing the
version while reconfiguring cert-manager.

Signed-off-by: Richard Wall <richard.wall@jetstack.io>
@wallrj wallrj force-pushed the clearer-cert-manager-reconfiguration-instructions branch from 4695a37 to 08e028d Compare April 27, 2023 09:01
Copy link
Contributor

@irbekrm irbekrm left a comment

Choose a reason for hiding this comment

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

Thanks Richard, I have read through the changes and I think this is an improvement although I regret that we spend so much time on Helm specific stuff. I guess at some point it'd be good to have a user survey to understand what are the most common installation mechanisms and what the trend is to see whether this is reasonable.

/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 27, 2023
@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: irbekrm, wallrj

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jetstack-bot jetstack-bot merged commit 99373ef into cert-manager:master Apr 27, 2023
@wallrj wallrj deleted the clearer-cert-manager-reconfiguration-instructions branch April 27, 2023 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants