-
Notifications
You must be signed in to change notification settings - Fork 607
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
pkg/server: get rules/alerts from thanos #5941
pkg/server: get rules/alerts from thanos #5941
Conversation
/lgtm |
the |
/hold |
We need a new version of Thanos in github.com/openshift/thanos that includes thanos-io/thanos#2888 |
Waiting on openshift/thanos#32 to merge. |
/unhold |
/approve |
/assign @spadgett |
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.
Thanks @simonpasquier
/retest |
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.
/approve
/lgtm
Thanks 👍
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/hold The test failure looks legitimate. It's expecting there to be no Monitoring nav item: We forgot to update this test when we changed the default perspective for normal users in #5911 |
I've rebased on top of master and the failure is gone. Not sure if it's related. |
It looks like the nav test failure is a flake we introduced in #5911 and is unrelated to the PR. I've seen it on a different PR: |
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
/hold cancel
/lgtm cancel |
The console backend doesn't need to access the prometheus-k8s service anymore. All requests are done against the Thanos querier service instead. Signed-off-by: Simon Pasquier <spasquie@redhat.com>
change rebased |
/retest |
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kyoto, lilic, simonpasquier, spadgett The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest Please review the full test history for this PR and help us cut down flakes. |
It's a rebase of #5881 on top of master because the previous PR had conflicts. Since @s-urbaniak isn't available, I'm taking over the change.
This enables user workload alerting rules to be shown in the admin perspective.
As discussed in the arch call it is favorable to implement some filtering knobs first in the UI.
Additionally this requires thanos-io/thanos#2834 to be pulled in downstream first./cc @openshift/openshift-team-monitoring