-
Notifications
You must be signed in to change notification settings - Fork 110
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
[EV-5302] Add set licenseKeyContent file to helm upgrade #1716
Conversation
✅ Deploy Preview for calico-docs-preview-next ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview succeeded!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@ctauchen could I please get your review? There a number of failures, but from the looks of it they are unrelated to this change. |
@@ -144,11 +144,13 @@ These steps differ based on your cluster type. If you are unsure of your cluster | |||
? ( | |||
`helm upgrade calico-enterprise --reuse-values --values=<path-to-values-yaml> tigera-operator-v0.0.tgz \\ | |||
--set-file imagePullSecrets.tigera-pull-secret=<path-to-pull-secret>,tigera-prometheus-operator.imagePullSecrets.tigera-pull-secret=<path-to-pull-secret> \\ | |||
--set-file licenseKeyContent=<path/to/license/file/yaml> \\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--set-file licenseKeyContent=<path/to/license/file/yaml> \\ | |
--set-file licenseKeyContent=<path-to-license-file-yaml> \\ |
Just to follow the convention already in this code block and elsewhere in the documentation. Otherwise LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ctauchen Thanks! The paths were updated to using the convention. The commits were squashed
1d21b25
to
031dd8c
Compare
LGTM. Ready to merge? |
Yes please! |
As part of our automation efforts, we eliminated the manual step of adding license.yaml and instead embedded the licenseKey directly into the
helm install
command. To ensure compatibility, a small template was added to convert the license from v3 to v1, since the CRD is installed before the API server is available. This change made the license resource Helm-managed. Consequently, if the license is not provided during any subsequent helm upgrade, Helm will delete the resource, assuming it's no longer needed.To address this, the following line has been added to the
helm upgrade
command in our documentation:Going forward, the license file must be specified during upgrades.
I considered using the
keep
annotation, which instructs Helm to ignore specific files. However, this requires manually updating the license file prior to upgrade, and it introduces side effects—such as Helm disregarding the file duringhelm uninstall
. Ultimately, updating our documentation proved to be the cleanest solution.Product Version(s):
[Main][v3.20]
Issue:
https://tigera.atlassian.net/browse/EV-5302
Link to docs preview:
SME review:
DOCS review:
Additional information:
Merge checklist: