fix(etcd-shield): raise thresholds, add total_size check, and fix severity — production - #11211
Conversation
|
Hi @peet-rh. Thanks for your PR. I'm waiting for a redhat-appstudio member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Review Summary by QodoFix etcd-shield alert severity and add total_size metric check
WalkthroughsDescription• Add etcd_mvcc_db_total_size_in_bytes metric to trigger rule - Catches fragmentation scenarios where total size exceeds quota - Previously only checked in-use size, missing high-fragmentation cases • Change alert severity from warning to critical - Fixes hysteresis condition that expected critical severity - Aligns with impact: etcd-shield denial causes cluster-wide PipelineRun rejection • Apply changes to both base and production alert configurations Diagramflowchart LR
A["etcd_shield_trigger rule"] -->|"add OR condition"| B["Check total_size_in_bytes"]
A -->|"existing condition"| C["Check total_size_in_use_in_bytes"]
B -->|"80% threshold"| D["Trigger alert"]
C -->|"80% threshold"| D
D -->|"severity update"| E["critical instead of warning"]
E -->|"fixes hysteresis"| F["70% sticky threshold active"]
File Changes1. components/etcd-shield/base/etcd_shield_alerts.yaml
|
Code Review by Qodo
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11211 +/- ##
=======================================
Coverage 58.73% 58.73%
=======================================
Files 24 24
Lines 1546 1546
=======================================
Hits 908 908
Misses 559 559
Partials 79 79
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Kustomize Render DiffComparing No render differences detected. |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| (((etcd_mvcc_db_total_size_in_use_in_bytes >= bool (etcd_server_quota_backend_bytes * 0.80)) == 1) or | ||
| ((etcd_mvcc_db_total_size_in_bytes >= bool (etcd_server_quota_backend_bytes * 0.80)) == 1) or | ||
| ((((etcd_mvcc_db_total_size_in_use_in_bytes) >= bool (etcd_server_quota_backend_bytes * 0.70)) == 1) and | ||
| (count without (alertname, alertstate, severity) |
There was a problem hiding this comment.
1. Hysteresis ignores total_size 🐞 Bug ≡ Correctness
etcd_shield_trigger can be triggered by etcd_mvcc_db_total_size_in_bytes at the 80% threshold, but the 70% hysteresis branch still only checks etcd_mvcc_db_total_size_in_use_in_bytes. This makes alerts triggered by fragmentation/total-size resolve immediately when total-size drops below 80% (even if it remains between 70–80%), defeating the intended hysteresis behavior.
Agent Prompt
### Issue description
The recording rule `etcd_shield_trigger` now triggers at 80% based on either `etcd_mvcc_db_total_size_in_use_in_bytes` or `etcd_mvcc_db_total_size_in_bytes`, but the 70% hysteresis (sticky) branch still only checks the in-use metric.
This causes alerts that fired due to `total_size_in_bytes` (fragmentation) to stop firing as soon as `total_size_in_bytes` falls below 80%, even if it is still above the intended 70% hysteresis threshold.
### Issue Context
You want: trigger at 80% based on either metric, and *remain active down to 70%* based on the same metric(s) that can trigger.
### Fix Focus Areas
- components/etcd-shield/base/etcd_shield_alerts.yaml[19-29]
- components/etcd-shield/production/base/etcd_shield_alerts.yaml[19-29]
### Suggested change (one of these approaches)
1) Extend the 70% hysteresis branch to OR in the `total_size_in_bytes` 70% check as well.
2) Alternatively, compute a single "effective size" (e.g., `max(etcd_mvcc_db_total_size_in_use_in_bytes, etcd_mvcc_db_total_size_in_bytes)`) and apply both the 80% and 70% thresholds to that, so trigger/hysteresis are symmetric.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
3ebc4bc to
3563f40
Compare
|
/review |
PR Reviewer Guide 🔍(Review updated until commit c7a91af)Here are some key observations to aid the review process:
|
gcpsoares
left a comment
gcpsoares
left a comment
There was a problem hiding this comment.
Review recording rule.
| annotations: | ||
| summary: etcd-shield is denying admission | ||
| description: Etcd is nearing capacity limits, so etcd-shield is denying admission | ||
| - record: etcd_shield_trigger | ||
| expr: | | ||
| (((etcd_mvcc_db_total_size_in_use_in_bytes >= bool (etcd_server_quota_backend_bytes * 0.80)) == 1) or | ||
| ((((etcd_mvcc_db_total_size_in_use_in_bytes) >= bool (etcd_server_quota_backend_bytes * 0.70)) == 1) and | ||
| ((etcd_mvcc_db_total_size_in_bytes >= bool (etcd_server_quota_backend_bytes * 0.80)) == 1) or | ||
| ((((etcd_mvcc_db_total_size_in_use_in_bytes >= bool (etcd_server_quota_backend_bytes * 0.70)) or |
There was a problem hiding this comment.
I'm not sure if that's ok, it might be good to test the recording rule before this merge/change.
There was a problem hiding this comment.
@gcpsoares — tested the recording rule against live Prometheus on stone-prd-rh01:
Current state (all 3 etcd members):
| Metric | Max Value | % of 8 GiB quota |
|---|---|---|
etcd_mvcc_db_total_size_in_bytes |
3.9 GiB | 49.2% |
etcd_mvcc_db_total_size_in_use_in_bytes |
2.9 GiB | 37.1% |
Condition 1 — in_use >= 80%: empty (correct, 37% < 80%)
Condition 2 — total_size >= 80%: empty (correct, 49% < 80%)
Condition 3 — hysteresis (in_use >= 70% OR total_size >= 70%) AND alert firing: returns 0 for all members (correct — neither at
70%, no alert active)
All expressions parse correctly, >= bool / == 1 semantics work as expected, and label sets match across all three metrics
(instance, job, namespace). No vector matching issues.
Severity change from warning → critical: during ITN-2026-00103, etcd-shield denying admission blocked all PipelineRuns cluster-wide —
that's critical-level impact. The change also fixes the hysteresis, which was checking ALERTS{severity="critical"} but the alert was
labeled warning, making the 70% sticky threshold dead code.
|
/review |
|
Persistent review updated to latest commit 3563f40 |
3abd1b2 to
bc33dc2
Compare
peet-rh
changed the title
bc33dc2 to
c7a91af
Compare
|
/review |
|
Persistent review updated to latest commit c7a91af |
Production Approval Record
Approved. Qodo reported issue is not an actual problem. |
Production Approval Record
|
37b0968 to
c990b30
Compare
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require review |
c990b30 to
dc3f745
Compare
|
@peet-rh this PR needs at least one GitHub reviewer approval before it can be approved for production. Please request a review from a team member using the Reviewers panel on the right, or ask someone to submit an approving review on this PR. |
dc3f745 to
a24ccf7
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: peet-rh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Code Review by QodoSorry, something went wrongWe weren't able to complete the code review on our side. Please try again |
da7f64d to
b03ecb4
Compare
|
Code review by qodo was updated up to the latest commit b03ecb4 |
b03ecb4 to
ae258ad
Compare
peet-rh
changed the title
ae258ad to
249015c
Compare
github-actions
Bot
added
environment/none
and removed
environment/production
infra/hold-production
prod/needs-approval
labels
Code Review by Qodo
Context used✅ Compliance rules (platform):
3 rules 1.
|
![qodo-for-redhat-appstudio[bot]](https://avatars.githubusercontent.com/u/89927105?s=60&v=4){kind=small}
…erity — production
249015c to
21da3b4
Compare
|
Code review by qodo was updated up to the latest commit 21da3b4 |
🤖 Pipeline Failure AnalysisCategory: Configuration The pipeline failed during the Konflux installation step because core ArgoCD applications were unable to sync due to missing Custom Resource Definitions (CRDs) required for their functionality. 📋 Technical DetailsImmediate CauseThe Konflux installation failed because critical ArgoCD applications, including Contributing FactorsThe unhealthiness of the ArgoCD applications was directly attributable to the absence of essential Kubernetes Custom Resource Definitions (CRDs), such as ImpactThe inability to successfully install Konflux rendered the entire AppStudio E2E test environment unusable, preventing any subsequent tests from executing and leading to the overall pipeline failure. 🔍 Evidenceappstudio-e2e-tests/konflux-ci-install-konfluxCategory: Logs:
|
|
Code review by qodo was updated up to the latest commit d728548 |
|
@peet-rh: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
What
Raise etcd-shield recording rule thresholds, add
etcd_mvcc_db_total_size_in_bytes(physical DB size) check, and fix alert severity — production overlay only.Changes
in_usetotal_size(new)Why
in_use, causing unnecessary shield activation at the old 80%/70% thresholds (per staging review feedback).total_size) can exceed quota undetected when onlyin_useis checked — fragmentation fills the gap.ALERTS{severity="critical"}but alert was labeledwarning, making the 70% deactivation threshold dead code.Validation
Risk Assessment
Risk Level: Low — validated in staging, single revert, thresholds are more conservative than quota limits.