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
Refactor monitoring rules #1757
Refactor monitoring rules #1757
Conversation
@@ -0,0 +1,101 @@ | |||
package main |
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.
Is this file should be added for all operators?
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.
here they already had the rules unit tests, on kubevirt they already had something similar, so it really depends
@@ -3,10 +3,11 @@ package main | |||
import ( |
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 file should move under pkg/monitoring/tools
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.
Tools should be in the pkg/monitoring! Many component's Dockefiles when building the operator copy pkg/ into the final image, and it is not necessary for the final build, so it should be in a separate base directory like just tools/
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 cdi we already moved the metrics tools under monitoring, and also I was able to move them in hco and hpp.
Basically it was Shirly's request so adding here @sradco
@@ -26,7 +26,6 @@ import ( | |||
"github.com/kubevirt/monitoring/pkg/metrics/parser" |
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 file should move under pkg/monitoring/tools
@@ -1,9 +1,10 @@ | |||
--- | |||
rule_files: | |||
- /tmp/rules.verify.yaml | |||
- /tmp/rules.verify |
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 think also this folder prom-rule-ci need to move under pkg/monitoring/tests or to to pkg/monitoring/tools
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.
Hey @machadovilaca - This PR lacks commit descriptions on all the commits. Please add these so I can review.
@@ -23,14 +23,14 @@ require ( | |||
github.com/openshift/origin v4.1.0+incompatible | |||
github.com/operator-framework/operator-sdk v1.12.0 | |||
github.com/pkg/errors v0.9.1 | |||
github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.64.1 |
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.
did you run make vendor afterwards, and add the necessary files?
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, no additional changes
Update PrometheusRule generation approach based on the changes proposed in kubevirt/community#219 Signed-off-by: João Vilaça <jvilaca@redhat.com>
Simplify the collection of metrics by using the utility methods added in the previous commit Signed-off-by: João Vilaça <jvilaca@redhat.com>
61a0217
to
0b46568
Compare
Add recording rules to the collector that gathers and list all metrics for the linter tool Signed-off-by: João Vilaça <jvilaca@redhat.com>
Convert verify-rules to Golang and take advantage of metrics and recording rules listing utilities added in the previous commits Signed-off-by: João Vilaça <jvilaca@redhat.com>
github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring is no longer a indirect dependency, update go.mod accordingly Signed-off-by: João Vilaça <jvilaca@redhat.com>
0b46568
to
e9e195b
Compare
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
/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
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RamLavi 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 |
What this PR does / why we need it:
Following the work started in kubevirt/kubevirt#10700, and according to the kubevirt/community#219 proposal, this PR refactors monitoring rules
jira-ticket: https://issues.redhat.com/browse/CNV-34289
Special notes for your reviewer:
Release note: