TRT-2612: Make payload analysis RHCOS-version-aware - #514
openshift-merge-bot[bot] merged 4 commits into
Conversation
Add an "Upgrade Jobs" section explaining micro/minor/major upgrade types and why the install-time OCP version matters for determining initial cluster state. Replace the outdated "RHCOS 10 (Tech Preview)" section with a comprehensive "RHCOS Versions" section covering both variants, detection rules from job names, and analysis implications of variant isolation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci
Bot
added
the
approved
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
WalkthroughAdds RHCOS-aware detection and reporting to analyze-payload: documents upgrade job naming and RHCOS variants, determines per-job ChangesRHCOS-Aware Upgrade Job Failure Analysis
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| - OCP 4.x → **RHCOS 9 (default)** | ||
| - OCP 5.x → **RHCOS 9 (default)** (current default; will change to RHCOS 10) | ||
|
|
||
| For upgrade jobs, use the **install-time** OCP version (see "Upgrade Jobs" in `jobs.md`), not the payload/target version. This matters for major upgrades: a major upgrade job in a 5.x payload installs OCP 4.x, so its RHCOS default follows OCP 4.x rules. |
There was a problem hiding this comment.
The canonical place is references/, is docs today in the plugin human or agent documentation?
https://agentskills.io/home
You should give a relative path here regardless and make it a link
There was a problem hiding this comment.
It apparently targets agents but the doc seems to be be plugin-wide and not skill-wide. renaming probably does not hurt but not sure if we actually need to symlink this from skills or something.
petr-muller
changed the title
|
@petr-muller: This pull request references TRT-2612 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/ci/skills/analyze-payload/SKILL.md (1)
113-127: ⚡ Quick winConsider adding explicit logic for determining install-time OCP version for upgrade jobs.
The documentation correctly states that upgrade jobs should use install-time OCP version for RHCOS defaults, but the implementation doesn't provide explicit logic for deriving the install-time version from job names.
For example:
- Minor upgrade job in 4.22 payload containing "upgrade-from-stable-4.21" → installs 4.21 (install-time = 4.21)
- Major upgrade job in 5.0 payload → installs 4.x (install-time = 4.x)
The current text assumes the implementer will know how to determine this, but adding a concrete algorithm would improve clarity:
4. No fragment → default based on the OCP major version **at install time**: - Determine install-time version: * If job name contains "upgrade-from-stable-X.Y", install-time version is X.Y * If job name contains "upgrade" and payload is 5.x but no "stable-5.x" fragment, install-time is 4.x (major upgrade) * Otherwise, install-time version equals payload version - Apply default: OCP 4.x → RHCOS 9, OCP 5.x → RHCOS 9🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/ci/skills/analyze-payload/SKILL.md` around lines 113 - 127, Extend the "RHCOS Version Determination" logic to explicitly derive the install-time OCP version from the job name for upgrade jobs: parse job names for patterns like "upgrade-from-stable-X.Y" to set install-time = X.Y; if job name contains "upgrade" with a 5.x payload but no "stable-5.x" fragment treat install-time as 4.x (major upgrade); otherwise default install-time to the payload version; then use that install-time major (4.x vs 5.x) to select the RHCOS default per the existing rules and pass the determined RHCOS to subagents.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@plugins/ci/skills/analyze-payload/SKILL.md`:
- Around line 113-127: Extend the "RHCOS Version Determination" logic to
explicitly derive the install-time OCP version from the job name for upgrade
jobs: parse job names for patterns like "upgrade-from-stable-X.Y" to set
install-time = X.Y; if job name contains "upgrade" with a 5.x payload but no
"stable-5.x" fragment treat install-time as 4.x (major upgrade); otherwise
default install-time to the payload version; then use that install-time major
(4.x vs 5.x) to select the RHCOS default per the existing rules and pass the
determined RHCOS to subagents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 02a641b9-e290-4cc9-86f4-16573aa0c0da
📒 Files selected for processing (2)
plugins/ci/docs/jobs.mdplugins/ci/skills/analyze-payload/SKILL.md
Extend the analyze-payload skill to determine and pass RHCOS version context to each subagent, detect variant-isolated failures across jobs, and surface RHCOS information in the HTML report. Subagents now receive the cluster's RHCOS version and return it in ANALYSIS_RESULT, enabling cross-job pattern recognition to flag failures isolated to one RHCOS variant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6643251 to
edbc77b
Compare
Rename plugins/ci/docs/ to plugins/ci/references/ to follow the plugin convention for reference documentation. Update all internal links and path references. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/ci/skills/analyze-payload/SKILL.md`:
- Line 124: Update the broken relative link in SKILL.md where the text contains
"[jobs.md](../docs/jobs.md)" (the sentence starting "For upgrade jobs, use the
**install-time** OCP version...") to use the correct path
"[jobs.md](../../docs/jobs.md)"; locate the markdown link text in SKILL.md and
replace "../docs/jobs.md" with "../../docs/jobs.md" so the link resolves to the
docs folder.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2540370b-c2ba-4fb4-92d6-e50b8fda2afc
📒 Files selected for processing (1)
plugins/ci/skills/analyze-payload/SKILL.md
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/ci/skills/analyze-payload/SKILL.md (1)
120-122:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnum value
rhcos10-defaultin schema is unreachable from documented determination logic.The RHCOS determination algorithm (lines 120-122) assigns RHCOS 9 as the default for both OCP 4.x and 5.x, with a note that 5.x "will change to RHCOS 10" in the future. However, the
ANALYSIS_RESULTschema on line 173 already includesrhcos10-defaultas a valid enum value that the current logic never produces.Either:
- Document when
rhcos10-defaultwill be returned (e.g., "For OCP 5.x starting from version 5.N, the default will be RHCOS 10"), or- Remove
rhcos10-defaultfrom the schema until the logic is updated to assign itThis prevents confusion about when subagents should return this value and ensures the schema matches the documented behavior.
Also applies to: 173-173
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/ci/skills/analyze-payload/SKILL.md` around lines 120 - 122, The schema includes an enum value `rhcos10-default` in ANALYSIS_RESULT that the RHCOS determination logic in SKILL.md (the "No fragment → default based on the OCP major version" rules) never emits; either remove `rhcos10-default` from the ANALYSIS_RESULT enum until the algorithm is updated, or update the SKILL.md rules to document explicitly when `rhcos10-default` will be returned (e.g., specify the OCP 5.x version threshold and wording like "For OCP 5.x starting with 5.N, default → RHCOS 10"), and update any consumers/tests that rely on ANALYSIS_RESULT to match the chosen change. Ensure the change is applied consistently by editing the ANALYSIS_RESULT enum and the RHCOS determination text together so schema and documentation align.
🧹 Nitpick comments (1)
plugins/ci/skills/analyze-payload/SKILL.md (1)
344-344: ⚡ Quick winClarify the mapping from
rhcos_versionenum values to badge CSS classes.The schema (line 173) defines five enum values:
rhcos9,rhcos10,rhcos9_10,rhcos9-default,rhcos10-default. The CSS (lines 528-530) defines three badge classes:badge-rhcos9,badge-rhcos10,badge-rhcos-mixed.The mapping is implicit:
rhcos9andrhcos9-default→badge-rhcos9rhcos10andrhcos10-default→badge-rhcos10rhcos9_10→badge-rhcos-mixedConsider adding an explicit note in section 7.2 or 7.3 documenting this mapping so implementers know how to render badges from the
ANALYSIS_RESULT.rhcos_versionfield.📝 Suggested clarification
After line 344 or in a new subsection, add:
**RHCOS Badge Rendering**: Map the subagent's `rhcos_version` value to CSS classes as follows: - `rhcos9` or `rhcos9-default` → `badge-rhcos9` with label "RHCOS 9" - `rhcos10` or `rhcos10-default` → `badge-rhcos10` with label "RHCOS 10" - `rhcos9_10` → `badge-rhcos-mixed` with label "RHCOS 9+10" When a failure is variant-isolated (see Step 5 Cross-Job Pattern Recognition), add the `variant-isolated` class to the badge.Also applies to: 359-359
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/ci/skills/analyze-payload/SKILL.md` at line 344, Add an explicit mapping in section 7.2 or 7.3 that documents how the ANALYSIS_RESULT.rhcos_version enum maps to badge CSS classes and labels: state that rhcos9 and rhcos9-default map to badge-rhcos9 with label "RHCOS 9", rhcos10 and rhcos10-default map to badge-rhcos10 with label "RHCOS 10", and rhcos9_10 maps to badge-rhcos-mixed with label "RHCOS 9+10"; also note that when a failure is variant-isolated the renderer should add the variant-isolated class to the badge and reference the rhcos_version field name so implementers can find it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@plugins/ci/skills/analyze-payload/SKILL.md`:
- Around line 120-122: The schema includes an enum value `rhcos10-default` in
ANALYSIS_RESULT that the RHCOS determination logic in SKILL.md (the "No fragment
→ default based on the OCP major version" rules) never emits; either remove
`rhcos10-default` from the ANALYSIS_RESULT enum until the algorithm is updated,
or update the SKILL.md rules to document explicitly when `rhcos10-default` will
be returned (e.g., specify the OCP 5.x version threshold and wording like "For
OCP 5.x starting with 5.N, default → RHCOS 10"), and update any consumers/tests
that rely on ANALYSIS_RESULT to match the chosen change. Ensure the change is
applied consistently by editing the ANALYSIS_RESULT enum and the RHCOS
determination text together so schema and documentation align.
---
Nitpick comments:
In `@plugins/ci/skills/analyze-payload/SKILL.md`:
- Line 344: Add an explicit mapping in section 7.2 or 7.3 that documents how the
ANALYSIS_RESULT.rhcos_version enum maps to badge CSS classes and labels: state
that rhcos9 and rhcos9-default map to badge-rhcos9 with label "RHCOS 9", rhcos10
and rhcos10-default map to badge-rhcos10 with label "RHCOS 10", and rhcos9_10
maps to badge-rhcos-mixed with label "RHCOS 9+10"; also note that when a failure
is variant-isolated the renderer should add the variant-isolated class to the
badge and reference the rhcos_version field name so implementers can find it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a1225faa-2d02-4e13-b957-dfb761a80cf1
⛔ Files ignored due to path filters (1)
plugins/ci/references/payload-agent-flowchart.pngis excluded by!**/*.png
📒 Files selected for processing (5)
plugins/ci/commands/analyze-regression.mdplugins/ci/references/jobs.mdplugins/ci/references/sippy-apis.mdplugins/ci/references/tests.mdplugins/ci/skills/analyze-payload/SKILL.md
💤 Files with no reviewable changes (1)
- plugins/ci/references/jobs.md
openshift-ci
Bot
added
the
do-not-merge/hold
|
I'm happy to take this PR, Do you happen to know an RHCOS-related issue we had in the last ~3 months I could use to build an evaluation case for? |
|
OK
I do not remember anything recent but wanted to look something up |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: petr-muller, stbenjam 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 |
3 participants
Summary
jobs.mdexplaining micro/minor/major upgrade types and why the install-time OCP version determines initial cluster stateanalyze-payloadskill to determine RHCOS version per job, pass it to subagents, collect it inANALYSIS_RESULT, detect variant-isolated failures in cross-job pattern recognition, and surface RHCOS badges and callouts in the HTML report🤖 Generated with Claude Code
Summary by CodeRabbit