Reduce sensitivity of component readiness for GA releases#3441
Reduce sensitivity of component readiness for GA releases#3441dgoodwin wants to merge 3 commits intoopenshift:mainfrom
Conversation
The noise of component readiness is presently so overwhelming for z-stream releases that no one actually monitors them. On the belief that churn is lower in these releases, this change reduces the levels at which we flag a regression to be less sensitive. Allow up to a 10% pass rate drop instead of 5. Require at least 4 failures in the week instead of 3. Stop doing multi-release fallback looking for the highest pass rate. Require 90% pass rate for new tests in that release instead of 95%.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
WalkthroughTightened component-readiness thresholds in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 14✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/views.yaml`:
- Around line 3617-3619: The 4.17-main and 4.16-main view blocks currently only
set minimum_failure/confidence/pity_factor and omit
pass_rate_required_new_tests, causing Advanced.PassRateRequiredNewTests to
default to 0; update those view entries (the blocks that contain
minimum_failure, confidence, pity_factor for 4.17-main and 4.16-main) to
explicitly add pass_rate_required_new_tests: 90 so they match the 4.18+ policy
and ensure consistency with Advanced.PassRateRequiredNewTests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 27e4d123-0a29-4219-b15b-d4ff0baa92d0
📒 Files selected for processing (1)
config/views.yaml
| minimum_failure: 4 | ||
| confidence: 95 | ||
| pity_factor: 5 | ||
| pity_factor: 10 |
There was a problem hiding this comment.
Make the older *-main policy explicit too.
4.17-main and 4.16-main only pick up the looser minimum_failure/pity_factor settings. They still omit pass_rate_required_new_tests, so Advanced.PassRateRequiredNewTests falls back to 0 here rather than the new explicit 90 used in 4.18+. If these views are still active, that makes the GA policy inconsistent and more permissive than intended.
Suggested follow-up
advanced_options:
minimum_failure: 4
confidence: 95
pity_factor: 10
ignore_missing: false
ignore_disruption: true
flake_as_failure: false
+ pass_rate_required_new_tests: 90
+ include_multi_release_analysis: falseAlso applies to: 3673-3675
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/views.yaml` around lines 3617 - 3619, The 4.17-main and 4.16-main view
blocks currently only set minimum_failure/confidence/pity_factor and omit
pass_rate_required_new_tests, causing Advanced.PassRateRequiredNewTests to
default to 0; update those view entries (the blocks that contain
minimum_failure, confidence, pity_factor for 4.17-main and 4.16-main) to
explicitly add pass_rate_required_new_tests: 90 so they match the 4.18+ policy
and ensure consistency with Advanced.PassRateRequiredNewTests.
There was a problem hiding this comment.
Those releases did not have this feature enabled at all.
There was a problem hiding this comment.
@dgoodwin, thanks for the clarification — that makes sense. The omission of pass_rate_required_new_tests in 4.17-main and 4.16-main is intentional since those older releases never had that feature enabled, rather than being an inconsistency to fix.
📝 Learnings were identified and not saved (knowledge base disabled). Enable
|
Scheduling required tests: |
| - crun | ||
| advanced_options: | ||
| minimum_failure: 3 | ||
| minimum_failure: 4 |
There was a problem hiding this comment.
As noted by @sosiouxme - could this be reflected in https://github.com/dgoodwin/sippy/blob/f89eeb7482efe296c45a26f71046cc32152b9749/.claude/commands/sippy-update-ga-release-views.md
|
/test security |
|
Scheduling required tests: |
|
/test e2e |
1 similar comment
|
/test e2e |
|
|
||
| **D. Increase pity_factor** (for views of the GA release itself): | ||
| - Find views where `sample_release.release` equals the GA release | ||
| - If `pity_factor` is set to `5` (i.e. 5%), change it to `10` (i.e. 10%) |
There was a problem hiding this comment.
I wonder if this is too rigid of direction. Maybe it is set to 6 or something in the future, and now this command is ignored and it is not changed to 10 as desired.
Same with other directions here
|
/lgtm |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dgoodwin, smg247, sosiouxme The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.claude/commands/sippy-update-ga-release-views.md (2)
187-187:⚠️ Potential issue | 🟡 MinorFix inconsistent field name.
Same issue as line 42: the documentation refers to
multi_release_analysis, but the actual field name isinclude_multi_release_analysis.📝 Proposed fix
- - Views where `sample_release.release` equals the GA release (sensitivity reductions: automate_jira, multi_release_analysis, pity_factor, minimum_failure, pass_rate_required_new_tests) + - Views where `sample_release.release` equals the GA release (sensitivity reductions: automate_jira, include_multi_release_analysis, pity_factor, minimum_failure, pass_rate_required_new_tests)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/sippy-update-ga-release-views.md at line 187, The doc uses the wrong field name `multi_release_analysis`; update instances (including the line that reads "Views where `sample_release.release` equals the GA release (sensitivity reductions: automate_jira, multi_release_analysis, pity_factor, minimum_failure, pass_rate_required_new_tests)") to the correct field `include_multi_release_analysis` so the documentation matches the actual schema; search for `multi_release_analysis` and replace with `include_multi_release_analysis` (also check the earlier occurrence noted around line 42) to keep names consistent.
151-165:⚠️ Potential issue | 🟠 MajorAvoid hardcoding old values in the selection criteria.
The selection logic for Types D, E, and F hardcodes specific old values (
pity_factor: 5,minimum_failure: 3,pass_rate_required_new_tests: 95). This creates several problems:
- Non-idempotent: Running the command twice will fail on the second run because the first run already changed these values, so no views will match the old value criteria.
- Brittle: If the default values change in the future (e.g., the default
pity_factorbecomes6instead of5), the command will silently fail to update views.- Inconsistent with Types B and C: Types B and C check for boolean states (
enabled: true), while Types D, E, and F check for specific numeric values.Consider revising the logic to either:
- Set the values unconditionally for all views where
sample_release.releaseequals the GA release, OR- Check if the current value differs from the target value and update only if needed
This aligns with smg247's earlier concern about rigid hardcoded values in the command logic.
📋 Proposed revision to the selection logic
**Type D: Increase pity_factor (for views of the GA release itself)** -1. **Find Views**: Identify all views where `sample_release.release` equals the GA release AND `pity_factor: 5` +1. **Find Views**: Identify all views where `sample_release.release` equals the GA release 2. **Update**: - - `pity_factor: 5` → `pity_factor: 10` + - Set `pity_factor: 10` (regardless of current value) **Type E: Increase minimum_failure (for views of the GA release itself)** -1. **Find Views**: Identify all views where `sample_release.release` equals the GA release AND `minimum_failure: 3` +1. **Find Views**: Identify all views where `sample_release.release` equals the GA release 2. **Update**: - - `minimum_failure: 3` → `minimum_failure: 4` + - Set `minimum_failure: 4` (regardless of current value) **Type F: Decrease pass_rate_required_new_tests (for views of the GA release itself)** -1. **Find Views**: Identify all views where `sample_release.release` equals the GA release AND `pass_rate_required_new_tests: 95` +1. **Find Views**: Identify all views where `sample_release.release` equals the GA release 2. **Update**: - - `pass_rate_required_new_tests: 95` → `pass_rate_required_new_tests: 90` + - Set `pass_rate_required_new_tests: 90` (regardless of current value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/sippy-update-ga-release-views.md around lines 151 - 165, Update the selection logic for Types D, E, and F to stop matching on hardcoded old numeric values; instead locate views by sample_release.release == GA release and then either (a) set pity_factor, minimum_failure, and pass_rate_required_new_tests unconditionally to the new target values, or (b) read the current value and update only when it differs from the target; specifically modify the code that implements "Type D/Type E/Type F" selection so it no longer filters by pity_factor:5, minimum_failure:3, or pass_rate_required_new_tests:95 but operates on all views for the GA release (or compares current vs target before writing) to make the command idempotent and robust.
🧹 Nitpick comments (1)
.claude/commands/sippy-update-ga-release-views.md (1)
169-182: Expand Implementation Approach to cover all update types.The Implementation Approach section only describes how to update
base_releasedates (Type A updates), but does not provide guidance for implementing the sensitivity parameter updates (Types B-F: automate_jira, include_multi_release_analysis, pity_factor, minimum_failure, pass_rate_required_new_tests).Consider adding implementation guidance for these additional update types to make this section complete and more useful for implementers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/sippy-update-ga-release-views.md around lines 169 - 182, The Implementation Approach only covers replacing base_release relative_start/relative_end values but omits guidance for changing sensitivity parameters; update the section to include step-by-step Edit-tool instructions for each additional update type: open config/views.yaml, locate views where base_release.release equals the GA release and then (Type B) set automate_jira: true/false as required, (Type C) set include_multi_release_analysis: true/false, (Type D) update pity_factor to the specified numeric value, (Type E) update minimum_failure to the specified integer, and (Type F) update pass_rate_required_new_tests to the specified percentage/float; emphasize using the Edit tool for each individual field change to preserve exact YAML formatting and include examples of exact key names (automate_jira, include_multi_release_analysis, pity_factor, minimum_failure, pass_rate_required_new_tests) and the same caution about not using yaml.dump().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/sippy-update-ga-release-views.md:
- Line 42: The documentation line that lists views uses the wrong field name
"multi_release_analysis"; update it to the actual field name
"include_multi_release_analysis" to match the config (as used in
config/views.yaml and elsewhere); locate the reference in the sentence that
lists sensitivity reductions (currently mentioning automate_jira,
multi_release_analysis, pity_factor, minimum_failure,
pass_rate_required_new_tests) and replace "multi_release_analysis" with
"include_multi_release_analysis" so the docs and config stay consistent.
---
Duplicate comments:
In @.claude/commands/sippy-update-ga-release-views.md:
- Line 187: The doc uses the wrong field name `multi_release_analysis`; update
instances (including the line that reads "Views where `sample_release.release`
equals the GA release (sensitivity reductions: automate_jira,
multi_release_analysis, pity_factor, minimum_failure,
pass_rate_required_new_tests)") to the correct field
`include_multi_release_analysis` so the documentation matches the actual schema;
search for `multi_release_analysis` and replace with
`include_multi_release_analysis` (also check the earlier occurrence noted around
line 42) to keep names consistent.
- Around line 151-165: Update the selection logic for Types D, E, and F to stop
matching on hardcoded old numeric values; instead locate views by
sample_release.release == GA release and then either (a) set pity_factor,
minimum_failure, and pass_rate_required_new_tests unconditionally to the new
target values, or (b) read the current value and update only when it differs
from the target; specifically modify the code that implements "Type D/Type
E/Type F" selection so it no longer filters by pity_factor:5, minimum_failure:3,
or pass_rate_required_new_tests:95 but operates on all views for the GA release
(or compares current vs target before writing) to make the command idempotent
and robust.
---
Nitpick comments:
In @.claude/commands/sippy-update-ga-release-views.md:
- Around line 169-182: The Implementation Approach only covers replacing
base_release relative_start/relative_end values but omits guidance for changing
sensitivity parameters; update the section to include step-by-step Edit-tool
instructions for each additional update type: open config/views.yaml, locate
views where base_release.release equals the GA release and then (Type B) set
automate_jira: true/false as required, (Type C) set
include_multi_release_analysis: true/false, (Type D) update pity_factor to the
specified numeric value, (Type E) update minimum_failure to the specified
integer, and (Type F) update pass_rate_required_new_tests to the specified
percentage/float; emphasize using the Edit tool for each individual field change
to preserve exact YAML formatting and include examples of exact key names
(automate_jira, include_multi_release_analysis, pity_factor, minimum_failure,
pass_rate_required_new_tests) and the same caution about not using yaml.dump().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: cf995b96-84ba-415e-8b64-b2eda2e9ad3d
📒 Files selected for processing (1)
.claude/commands/sippy-update-ga-release-views.md
| - Read `config/views.yaml` | ||
| - Find all views where `base_release.release` equals the GA release (for date updates) | ||
| - Find all views where `sample_release.release` equals the GA release AND have `automate_jira.enabled: true` (for JIRA automation disabling) | ||
| - Find all views where `sample_release.release` equals the GA release (for sensitivity reductions: automate_jira, multi_release_analysis, pity_factor, minimum_failure, pass_rate_required_new_tests) |
There was a problem hiding this comment.
Fix inconsistent field name.
The documentation refers to multi_release_analysis, but the actual field name is include_multi_release_analysis (as shown in lines 64, 147, and confirmed by the context snippets from config/views.yaml). Use the complete field name for consistency.
📝 Proposed fix
- - Find all views where `sample_release.release` equals the GA release (for sensitivity reductions: automate_jira, multi_release_analysis, pity_factor, minimum_failure, pass_rate_required_new_tests)
+ - Find all views where `sample_release.release` equals the GA release (for sensitivity reductions: automate_jira, include_multi_release_analysis, pity_factor, minimum_failure, pass_rate_required_new_tests)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/sippy-update-ga-release-views.md at line 42, The
documentation line that lists views uses the wrong field name
"multi_release_analysis"; update it to the actual field name
"include_multi_release_analysis" to match the config (as used in
config/views.yaml and elsewhere); locate the reference in the sentence that
lists sensitivity reductions (currently mentioning automate_jira,
multi_release_analysis, pity_factor, minimum_failure,
pass_rate_required_new_tests) and replace "multi_release_analysis" with
"include_multi_release_analysis" so the docs and config stay consistent.
|
Scheduling required tests: |
|
@dgoodwin: all tests passed! 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. |
|
/hold cancel |
4 participants
The noise of component readiness is presently so overwhelming for
z-stream releases that no one actually monitors them. On the belief that
churn is lower in these releases, this change reduces the levels at
which we flag a regression to be less sensitive.
Allow up to a 10% pass rate drop instead of 5.
Require at least 4 failures in the week instead of 3.
Stop doing multi-release fallback looking for the highest pass rate.
Require 90% pass rate for new tests in that release instead of 95%.
For 4.21 this cuts the current list of 30 down to 15 and still shows some clear problems. For 4.20 it cuts 48 down to 28.
Summary by CodeRabbit