-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add logic to fetch prometheus rules from clusters #290
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
8971136
to
5637e64
Compare
5637e64
to
18bab94
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.
Good stuff! Thanks!
Prometheus rules are obtained from managed clusters to subsequently fill out the alarm dictionaries Signed-off-by: Marcelo Guerrero <[email protected]>
18bab94
to
45d8f36
Compare
secretTypeLabel = "hive.openshift.io/secret-type" | ||
secretTypeLabelValue = "kubeconfig" |
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.
could add a comment about where these are coming from just to make it clear for the future?
Also are the timeouts related to these two consts if not maybe create new const block (nit)?
managedClusterVersionLabel = "openshiftVersion-major-minor" | ||
localClusterLabel = "local-cluster" | ||
|
||
resourceTypeCluster = "Cluster" |
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.
- nit: maybe a separate block since it's unrelated to acm values (this is o-ran specific stuff)? Also maybe a comment about what this is how we will have more types in the future?
} | ||
} | ||
|
||
return rules, nil |
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.
maybe a log see say which cluster, the version and count?
versionSelector, _ := labels.NewRequirement(managedClusterVersionLabel, selection.Equals, []string{version}) | ||
localClusterRequirement, _ := labels.NewRequirement(localClusterLabel, selection.NotEquals, []string{"true"}) |
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.
can we return the errors?
var rules []monitoringv1.Rule | ||
for _, promRule := range promRules { | ||
for _, group := range promRule.Spec.Groups { | ||
rules = append(rules, group.Rules...) |
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.
Might be helpful to print out the alert names?
hubClient, err := clients.NewClientForHub() | ||
if err != nil { | ||
slog.Error("error creating client for hub", "error", err) | ||
os.Exit(1) | ||
} | ||
|
||
alarmsDict := dictionary.New(hubClient) | ||
alarmsDict.Load(ctx) |
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 there any clean up we need to do after this...like closing the client maybe?
Prometheus rules are obtained from managed clusters to subsequently fill out the alarm dictionaries