-
Notifications
You must be signed in to change notification settings - Fork 3
fix(observability): fix expr for alerts like PodIsNotReady #1553
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,7 +1,10 @@ | ||||||||||||||||||
- name: kubernetes.internal.virtualization.cdi_operator_state | ||||||||||||||||||
rules: | ||||||||||||||||||
- alert: D8InternalVirtualizationCDIOperatorPodIsNotReady | ||||||||||||||||||
expr: min by (pod) (kube_pod_status_ready{condition="true", namespace="d8-virtualization", pod=~"cdi-operator-.*"}) != 1 | ||||||||||||||||||
expr: | | ||||||||||||||||||
(kube_pod_status_ready{condition="true", namespace="d8-virtualization", pod=~"cdi-operator-.*"} == 0) | ||||||||||||||||||
and on (namespace,pod) | ||||||||||||||||||
(kube_pod_status_phase{namespace="d8-virtualization", phase=~"Running|Succeeded", pod=~"cdi-operator-.*"} == 1) | ||||||||||||||||||
Comment on lines
+4
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Consider using consistent aggregation for pod readiness and phase checks. Aggregating over pods will ensure the alert works correctly when multiple pods match the pattern, preventing missed readiness or phase issues.
Suggested change
|
||||||||||||||||||
labels: | ||||||||||||||||||
severity_level: "6" | ||||||||||||||||||
tier: cluster | ||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,7 +1,10 @@ | ||||||||||||||||||||||||||||||||||||||
- name: kubernetes.internal.virtualization.virt_api_state | ||||||||||||||||||||||||||||||||||||||
rules: | ||||||||||||||||||||||||||||||||||||||
- alert: D8InternalVirtualizationVirtAPIPodIsNotReady | ||||||||||||||||||||||||||||||||||||||
expr: min by (pod) (kube_pod_status_ready{condition="true", namespace="d8-virtualization", pod=~"virt-api-.*"}) != 1 | ||||||||||||||||||||||||||||||||||||||
expr: | | ||||||||||||||||||||||||||||||||||||||
(kube_pod_status_ready{condition="true", namespace="d8-virtualization", pod=~"virt-api-.*"} == 0) | ||||||||||||||||||||||||||||||||||||||
and on (namespace,pod) | ||||||||||||||||||||||||||||||||||||||
(kube_pod_status_phase{namespace="d8-virtualization", phase=~"Running|Succeeded", pod=~"virt-api-.*"} == 1) | ||||||||||||||||||||||||||||||||||||||
Comment on lines
+4
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: The alert logic may not handle multiple pods correctly due to equality checks. Consider replacing strict equality checks with aggregation or range-based logic to ensure the alert accurately reflects pod readiness in deployments with multiple pods.
Suggested change
|
||||||||||||||||||||||||||||||||||||||
labels: | ||||||||||||||||||||||||||||||||||||||
severity_level: "6" | ||||||||||||||||||||||||||||||||||||||
tier: cluster | ||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,7 +1,10 @@ | ||||||||||||||||||||||
- name: kubernetes.internal.virtualization.virt_controller_state | ||||||||||||||||||||||
rules: | ||||||||||||||||||||||
- alert: D8InternalVirtualizationVirtControllerPodIsNotReady | ||||||||||||||||||||||
expr: min by (pod) (kube_pod_status_ready{condition="true", namespace="d8-virtualization", pod=~"virt-controller-.*"}) != 1 | ||||||||||||||||||||||
expr: | | ||||||||||||||||||||||
(kube_pod_status_ready{condition="true", namespace="d8-virtualization", pod=~"virt-controller-.*"} == 0) | ||||||||||||||||||||||
and on (namespace,pod) | ||||||||||||||||||||||
(kube_pod_status_phase{namespace="d8-virtualization", phase=~"Running|Succeeded", pod=~"virt-controller-.*"} == 1) | ||||||||||||||||||||||
Comment on lines
+4
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (bug_risk): Equality checks for pod readiness and phase may not be robust for deployments with multiple pods. Consider updating the logic to aggregate pod readiness, ensuring the alert triggers if any pod is not ready, rather than relying on equality checks.
Suggested change
|
||||||||||||||||||||||
labels: | ||||||||||||||||||||||
severity_level: "6" | ||||||||||||||||||||||
tier: 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.
suggestion: The alert triggers only if exactly one pod matches the phase condition, which may miss cases with multiple pods.
Consider replacing '== 1' with '> 0' or a suitable aggregation to handle scenarios with multiple pods in the specified phases.