-
Notifications
You must be signed in to change notification settings - Fork 499
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
Monitoring: enable alert rules persistence #898
Monitoring: enable alert rules persistence #898
Conversation
@@ -121,6 +125,8 @@ spec: | |||
imagePullPolicy: {{ .Values.monitor.reloader.imagePullPolicy | default "IfNotPresent" }} | |||
command: | |||
- /bin/reload | |||
- --root-store-path=/data | |||
- --sub-store-path={{ .Values.tidb.image }} |
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.
This command-line argument looks odd, can we rename it to make the name and value match?
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.
reloader 想做成和 TIDB 没什么关联独立存在的 prometheus ui reload 工具,所以取了和 TIDB 无关的名字
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.
Yes, this is OK. But maybe we can convert the .Values.tidb.image
to the expected path in Helm template.
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
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
But, the release-note is not related to this PR fully.
/run-e2e-in-kind |
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
/run-e2e-in-kind |
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
/run-e2e-in-kind |
cherry pick to release-1.0 failed |
…upstream-release-1.0
* update tkctl debug usage * Update en/use-tkctl.md * Update en/use-tkctl.md Co-authored-by: Ran <[email protected]>
What problem does this PR solve?
Previously, The new alert rules added by monitoring-reloader will be loss if prometheus restart.
It is unreasonable and we update the logic following rules
TiDB Version is not changed, the updated rules should be persistence even though prometheus restart
TiDB Version is changed, the updated rules will be loss (It is also stored PV).
The Cluster is deleted, the updated rules will be loss (It is also stored PV).
It is related the PR
What is changed and how does it work?
Check List
Tests
Upgrade TIDB Cluster but do not change the TIDB version , the updated files will be persisted
Upgrade TIDB Version, Updated rules is loss
Code changes
Does this PR introduce a user-facing change?: