-
Notifications
You must be signed in to change notification settings - Fork 719
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
Expand upgrading ECK doc + add annotator script #2797
Conversation
kubectl annotate elasticsearch quickstart $RM_ANNOTATION | ||
---- | ||
|
||
<1> Since ECK 1.1.0, this annotation has been superseded by `eck.k8s.elastic.co/managed-` |
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.
The final dash seems unexpected.
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.
It is the way to remove an annotation though. Any suggestions to make it look like it's not a typo?
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.
In this case, it is legitimate and I think it's okay.
I'm not sure it will be more clear, we could keep the dash in the command like this:
kubectl annotate elasticsearch quickstart ${RM_ANNOTATION}-
.
I'm fine with keeping it like this.
|
||
set -euo pipefail | ||
|
||
ANN_KEY=${ANN_KEY:-"common.k8s.elastic.co/pause"} |
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.
Let's use the latest annotation eck.k8s.elastic.co/managed
?
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.
The reason I went with the old annotation is because the documentation is targeted at people who are trying to upgrade from 1.0.0-beta1. The new annotation is only available from 1.1.0 onwards.
Co-Authored-By: Thibault Richard <thbkrkr@users.noreply.github.com>
*) | ||
echo "Unknown action '$1'" | ||
usage | ||
;; |
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 wonder if for a production deployment you would want to have more control, i.e. create certain cohorts of Elastic stack apps and remove labels not all at once by cohort by cohort.
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.
That would be pretty hard for us to generalise and put into a script, wouldn't it? I reckon the grouping logic would be unique to each user (grouping based on custom label combinations for example). My intention was to provide a starting point for users that can then be adapted to fit their particular requirements. WDYT?
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.
Adds more information to the "Upgrading ECK" document to inform users about the rolling restart of all resources and how to manage them effectively.
Also adds a script to help with annotating all Elastic resources in a cluster.
Fixes: #2595
Depends on: #2783
Preview: http://cloud-on-k8s_2797.docs-preview.app.elstc.co/guide/en/cloud-on-k8s/master/k8s-upgrading-eck.html