-
Notifications
You must be signed in to change notification settings - Fork 5k
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
[kube-prometheus-stack] Add upgrade path from stable/prometheus-operator #119
[kube-prometheus-stack] Add upgrade path from stable/prometheus-operator #119
Conversation
7576c5b
to
d2a10a6
Compare
Signed-off-by: fktkrt <[email protected]>
Signed-off-by: fktkrt <[email protected]>
Signed-off-by: fktkrt <[email protected]>
Signed-off-by: fktkrt <[email protected]>
d2a10a6
to
de00619
Compare
Signed-off-by: fktkrt <[email protected]>
409d910
to
09cfc5f
Compare
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, let wait for one more reviewer.
Thanks 😊 This is great if a user needs to upgrade the release name or namespace. But given that users can do a full name override to match their existing release, would you be willing to add a similar scenario for retaining PVs for an in-place upgrade? Also is it clear to everyone what data is being persisted here, and how exactly to do that? Users can persist the grafana subchart user account and dashboard data as well, right? |
Alternatively, we could merge this as-is, and allow other members of the community to document how to do so for an in-place upgrade. WDYT? |
@scottrigby I like your second option. Merge this as-is and support the @fktkrt for his contribution and others (or may be @fktkrt would be interested as well) might be interested in contributing the other documentation. |
Before this is merged, let's have someone try these steps to ensure they work properly. I'm not sure when I'll get to that but once that's verified by at least one other person we can merge this. I created #121 to make sure we don't forget to address scenarios and details I asked about above. |
Signed-off-by: fktkrt <[email protected]>
91eeb1c
to
080e5d6
Compare
Just performed it in another cluster, added a missing step and cleaned up the second patch syntax. |
@m42u Please see my recent changes to this PR based on your comments.
@desaintmartin, there's this option that might solves your comment, any help is welcomed to help validate it as I have limited time at the moment. |
@fktkrt Path requires me to uninstall previous chart release wouldn't this cause any downtimes ? |
@flouthoc Yes, from to point the old is removed and until the new is installed (1-2m), you won't have metrics. Do you maybe want to give it a try? :) |
@fktkrt Thanks i guess 1-2m wont do any damage. I am performing a drill on dummy cluster to record delta for downtime, I guess after that we are good to go. Any reason why is this guide not merged to master yet ? |
@scottrigby said that first let's wait for a few more successful migration report then we can merge this. |
I just did a migration from JSONNET version of kube-prometheus to the helm chart, using those instructions more or less, and it worked perfectly, I think that should get merged :-) |
Signed-off-by: fktkrt <[email protected]>
856abac
to
2ec1163
Compare
We have some good user validation of the steps, so let's merge this. It just needs conflicts to be resolved. |
I have resolved the conflict |
…k option Signed-off-by: Scott Rigby <[email protected]>
Fixed markdownlint on your branch @fktkrt |
The prometheus.prometheusSpec.serviceMonitorSelector changes with the new chart from |
I had no issues regarding this with the following setup: I am using ServiceMonitors and PodMonitors, and labeling those this way:
This is my prometheusSpec:
additionally, for my custom environments I am overriding these with externalLabels like this:
My Prometheus instance has default labels:
and the new release can continue scraping all my existing Pod/ServiceMonitors created with the legacy chart. If you have access to a test cluster (or can deploy a playground with k3s or similar), you can easily doublecheck it, just to make sure nothing breaks. |
By turning off the rule/serviceMonitor/podMonitor selectors, doesn't that make it difficult to tell an instance of prometheus to only monitor things that are related to that instance? What if you have multiple prometheuses (promethei?) running in your cluster? |
You can use additional labels to tie apps to specific prometheus instances. (By the way, you were close with promethei, https://prometheus.io/docs/introduction/faq/#what-is-the-plural-of-prometheus :) ) |
…tor (prometheus-community#119) * add upgrade path from stable/prometheus-operator Signed-off-by: fktkrt <[email protected]> * bump chart Signed-off-by: fktkrt <[email protected]> * fix lint Signed-off-by: fktkrt <[email protected]> * fix lint Signed-off-by: fktkrt <[email protected]> * generalize object references Signed-off-by: fktkrt <[email protected]> * fix kubectl patch syntax, add missing step Signed-off-by: fktkrt <[email protected]> * bump Chart version Signed-off-by: fktkrt <[email protected]> * add step to remove legacy kubelet service Signed-off-by: fktkrt <[email protected]> * add instructions to specify AZ trough labels Signed-off-by: fktkrt <[email protected]> * remove unnecessary namespace reference, mention CRD provisioning, add missing zone identifier Signed-off-by: fktkrt <[email protected]> * add volumeClaimTemplate example Signed-off-by: fktkrt <[email protected]> * Fix markdownlint. Also change 'bash' to appropriate linguist codeblock option Signed-off-by: Scott Rigby <[email protected]> Co-authored-by: Scott Rigby <[email protected]>
What this PR does / why we need it:
I had to migrate our stable/prometheus-operator to the new chart while keeping the existing PersistentVolume.
To do this I had to perform the steps provided below. After the procedure I was able to access the old time-series data stored in the PV.
Which issue this PR fixes
Special notes for your reviewer:
This guide covers a fresh installation of the new chart, so not persisted configurations (e.g. in values.yaml, custom dashboards) can be lost after the process is finished.
Checklist
[prometheus-couchdb-exporter]
)