-
Notifications
You must be signed in to change notification settings - Fork 197
Elastic Stack Helm chart upgrade and configuration changes documentation #4388
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
Conversation
✅ Vale Linting ResultsNo issues found on modified lines! The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
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.
although I don't know too much about kubernetes / helm, none of this struck me as superfluous information. I think you did a good job of adding the right amount of info but keeping it task-based for the ECK context.
approving because this is nearly perfect - whatever you do from here is mergeable from my pov.
one thing that might be missing: do I need a specific version of the helm chart to access later versions of the stack? e.g. is upgrading the helm chart version a prerequisite? If that is the case, I believe it is not clear.
deploy-manage/deploy/cloud-on-k8s/managing-deployments-using-helm-chart.md
Outdated
Show resolved
Hide resolved
deploy-manage/deploy/cloud-on-k8s/managing-deployments-using-helm-chart.md
Outdated
Show resolved
Hide resolved
deploy-manage/deploy/cloud-on-k8s/managing-deployments-using-helm-chart.md
Outdated
Show resolved
Hide resolved
deploy-manage/deploy/cloud-on-k8s/managing-deployments-using-helm-chart.md
Outdated
Show resolved
Hide resolved
deploy-manage/deploy/cloud-on-k8s/managing-deployments-using-helm-chart.md
Outdated
Show resolved
Hide resolved
deploy-manage/deploy/cloud-on-k8s/managing-deployments-using-helm-chart.md
Outdated
Show resolved
Hide resolved
deploy-manage/deploy/cloud-on-k8s/managing-deployments-using-helm-chart.md
Outdated
Show resolved
Hide resolved
deploy-manage/deploy/cloud-on-k8s/managing-deployments-using-helm-chart.md
Outdated
Show resolved
Hide resolved
deploy-manage/deploy/cloud-on-k8s/managing-deployments-using-helm-chart.md
Outdated
Show resolved
Hide resolved
Co-authored-by: shainaraskas <58563081+shainaraskas@users.noreply.github.com>
shainaraskas
left a comment
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.
changes look great. two tiniest suggestions
Co-authored-by: shainaraskas <58563081+shainaraskas@users.noreply.github.com>
|
This PR is now ready for review by ECK team! cc: @elastic/cloud-k8s ;) |
pebrc
left a comment
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. My only note is that we are venturing a bit far into the realm of explaining Helm which is a third-party tool and mastering it is not something Elastic can necessarily support to the full extent.
|
thanks @pebrc
Completely agreed, that’s a fair point. I’ve tried to keep the scope limited to Helm usage as it relates to the Anyway I've updated a bit the intro of the document to state that we expect familiarity with Helm and we are not providing a comprehensive introduction to Helm itself. And I've also updated the On the other hand, I think this type of guide (trying not to be a helm guide but providing some basics when needed) could also help reduce requests for help (SDHs). |
shainaraskas
left a comment
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.
changes look good to me with a couple of small notes that are mostly just my opinion
| # {{stack}} Helm chart [k8s-stack-helm-chart] | ||
|
|
||
| Starting from ECK 2.4.0, a Helm chart is available for managing {{stack}} resources using the ECK Operator. It is available from the Elastic Helm repository and can be added to your Helm repository list by running the following command: | ||
| Starting with ECK 2.4.0, a Helm chart is available to deploy and manage {{stack}} resources using the ECK Operator. This guide focuses on using the `eck-stack` Helm chart and assumes basic familiarity with [Helm](https://helm.sh/docs/); it does not provide a comprehensive introduction to Helm itself. |
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 suggest moving this scope note out of this intro paragraph because it's not core to the doc. maybe put it as a bullet in the tip. Also avoid semicolons
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.
ok, reverted intro to original text, and improved the paragraph that talked about the document, now this fits in the right place:
The chart enables you to deploy the core components ({{es}} and {{kib}}) together, along with other {{stack}} applications if needed, under the same chart release. The following sections guide you through common installation, configuration, and upgrade use cases and assume basic familiarity with Helm. It does not provide a comprehensive introduction to Helm itself.
| To upgrade your {{stack}} components to a new version or modify the configuration of your existing installation (known as a `release`), use the [`helm upgrade`](https://helm.sh/docs/helm/helm_upgrade/) command. | ||
|
|
||
| The `helm upgrade` command requires the following arguments: | ||
| When upgrading an existing release with `helm upgrade`, you must specify, at least: |
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.
why is this different from required arguments? I think the old sentence was clearer
This PR adds documentation for Elastic Stack Helm chart (provided by ECK) users to know how to upgrade and apply configuration changes.
As you'll see, certain parts of the content are not purely about ECK and Elastic Stack specifics, but a bit about how Helm works and educates the reader in areas that are not purely ours.
I've tried to fit the content to our exact domain (Elastic Stack), but giving enough information to understand the basics.
Closes #3591
After admin-docs review we will need to find devs to review the content.
Update: ready for review by ECK team.