-
Notifications
You must be signed in to change notification settings - Fork 215
K8s: helm upgrade changes #1640
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
base: main
Are you sure you want to change the base?
Conversation
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
|
||
2. Install the Helm chart, overriding specific value defaults using `--set`. | ||
|
||
```sh | ||
helm install <operator-name> redis/redis-enterprise-operator \ | ||
helm install <operator-name> <repo-name>/redis-enterprise-operator \ | ||
--version <release-name> \ |
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.
not related to the change, but should be chart-version instead of release-name here.
|
||
2. Create a YAML file to specify the values you want to configure. | ||
|
||
3. Install the chart with the `--values` option. | ||
|
||
```sh | ||
helm install <operator-name> redis/redis-enterprise-operator \ | ||
helm install <operator-name> <repo-name>/redis-enterprise-operator \ | ||
--version <release-name> \ |
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.
similarly here
kubectl delete validatingwebhookconfiguration redis-enterprise-admission | ||
``` | ||
|
||
This step is only needed when the `admission.limitToNamespace` chart value is set to `true` (the default). In this case, the webhook object installed by the chart is named `redis-enterprise-admission-<namespace>`. If `admission.limitToNamespace` is set to `false`, the webhook installed by the chart is named `redis-enterprise-admission`, and the existing webhook object is reused. |
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.
I'm wondering whether it's useful to include a little more information as to why this is needed.
Something like (new text in bold):
In this case, the webhook object installed by the chart is named
redis-enterprise-admission-<namespace>
, and the original webhook object, namedredis-enterprise-admission
, becomes redundant. Ifadmission.limitToNamespace
is set tofalse
...
Not sure if this is "too much information" though.
2. [Install](#install) the Helm chart adding the `--take-ownership` flag: | ||
|
||
```sh | ||
helm install <release-name> <path-to-chart> --take-ownership |
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.
Maybe this should use the Helm repo syntax (helm install <repo-name>/redis-enterprise-operator...
), being that:
- This is the primary method used throughout this article (we only mention that local directory option in one specific section).
- This is anyway the typical method Helm is used...
```sh | ||
helm upgrade my-redis-enterprise . | ||
``` | ||
|
||
You can also upgrade using the Redis Helm repository: | ||
|
||
```sh | ||
helm upgrade <release-name> <repo-name>/redis-enterprise-operator --version <chart-version> |
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.
Here too, maybe just use the Helm repo method, or at least inverse the order?
To upgrade the chart on **OpenShift**, include the `openshift.mode=true` parameter: | ||
|
||
```sh | ||
helm upgrade <release-name> <path-to-chart> \ | ||
--set openshift.mode=true | ||
``` |
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.
I'm wondering whether this merits an explicit note like this.
Maybe just add the following sentence, like we do in the other install sections:
To install with Openshift, add
--set openshift.mode=true
.
To upgrade using Helm on OpenShift: | ||
|
||
```sh | ||
helm upgrade <release-name> redis/redis-enterprise-operator --version <chart-version> \ |
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.
Use <repo-name>/
instead of redis/
?
@@ -57,6 +57,28 @@ See [Upgrade modules]({{<relref "/operate/oss_and_stack/stack-with-enterprise/in | |||
|
|||
Use `kubectl get rec` and verify the `LICENSE STATE` is valid on your REC before you start the upgrade process. | |||
|
|||
## Upgrade with Helm charts |
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.
Users installing via OLM and not installing via Helm, kinda by definition.
I think we don't need this section here within the OLM docs.
DOC-4924
DOC-5314