ci:payload-analysis: surface RHCOS RPM changes - #566
openshift-merge-bot[bot] merged 1 commit into
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
needs-rebase
WalkthroughThe PR adds end-to-end RHCOS RPM change correlation to the CI plugin. ChangesRHCOS RPM Suspect Correlation Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.22.1)plugins/ci/skills/payload-analysis/SKILL.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0) plugins/ci/skills/payload-snapshot/SKILL.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0) plugins/ci/skills/payload-autodl-json/SKILL.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0)
Comment |
e5b1a2a to
801b3e5
Compare
openshift-ci
Bot
removed
the
needs-rebase
fe98540 to
3a428b9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
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/payload-analysis/SKILL.md (1)
203-224: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
tagwhen looking up payload entries.
summary.json.payloads[]entries are written with atagfield, notpayload_tag, so this lookup will never match and the new RHCOS RPM correlation path will be skipped.Suggested fix
-> Read `<summary_json_path>` and find the entry in `payloads[]` whose `payload_tag` equals `<originating_payload_tag>`. +> Read `<summary_json_path>` and find the entry in `payloads[]` whose `tag` equals `<originating_payload_tag>`.🤖 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/payload-analysis/SKILL.md` around lines 203 - 224, In the RHCOS RPM changes section, the instruction references a non-existent field `payload_tag` when describing how to look up entries in the `payloads[]` array. Replace the phrase "whose `payload_tag` equals" with "whose `tag` equals" to correctly reference the actual JSON field name used in summary.json, ensuring the lookup logic will correctly match payload entries against the originating_payload value.
🤖 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/payload-results-yaml/scripts/validate.py`:
- Around line 64-78: The validation code for rhcos_suspects uses
data.get("rhcos_suspects") which cannot distinguish between a missing key and an
explicitly null value in YAML, allowing malformed null entries to pass
validation. Replace the data.get() call with a check using the in operator to
explicitly verify key presence, and add validation to reject cases where the key
exists but its value is None (from null in YAML). Ensure that if rhcos_suspects
is present, it must be a non-null list, treating explicit null values as an
error rather than treating them as an absent field.
In `@plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py`:
- Around line 1866-1880: The _extract_rhcos_changes() function is not tolerant
of None or empty changelogs since it calls changelog.get() without first
checking if changelog is None, and it also appends entries to results even when
the changed, added, and removed dictionaries are all empty. Add a guard clause
at the start of _extract_rhcos_changes() to return an empty list if changelog is
None or falsy, and add an additional condition before appending to results that
verifies at least one of the changed, added, or removed dictionaries contains
data to avoid creating empty/bogus entries.
---
Outside diff comments:
In `@plugins/ci/skills/payload-analysis/SKILL.md`:
- Around line 203-224: In the RHCOS RPM changes section, the instruction
references a non-existent field `payload_tag` when describing how to look up
entries in the `payloads[]` array. Replace the phrase "whose `payload_tag`
equals" with "whose `tag` equals" to correctly reference the actual JSON field
name used in summary.json, ensuring the lookup logic will correctly match
payload entries against the originating_payload value.
🪄 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: 771adddb-e790-4eee-a859-c8343081bdb4
📒 Files selected for processing (12)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/ci/.claude-plugin/plugin.jsonplugins/ci/skills/payload-analysis/SKILL.mdplugins/ci/skills/payload-autodl-json/SKILL.mdplugins/ci/skills/payload-results-yaml/SKILL.mdplugins/ci/skills/payload-results-yaml/scripts/test_validate.pyplugins/ci/skills/payload-results-yaml/scripts/testdata/invalid_rhcos_suspects.yamlplugins/ci/skills/payload-results-yaml/scripts/testdata/valid_with_rhcos_suspects.yamlplugins/ci/skills/payload-results-yaml/scripts/validate.pyplugins/ci/skills/payload-snapshot/SKILL.mdplugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
3a428b9 to
47d663a
Compare
petr-muller
changed the title
ci:payload-analysis: surface RHCOS RPM changes in payload analysis
petr-muller
changed the title
ci:payload-analysis: surface RHCOS RPM changes in payload analysisci:payload-analysis: surface RHCOS RPM changes
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
Actionable comments posted: 2
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/payload-analysis/SKILL.md (1)
203-225: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign the subagent prompt with the new OS-level correlation scope.
The verbatim Step 4 prompt omits
MCOandIgnition, but Step 6.1b now treats both as OS-level correlation signals. That gap can make subagents miss the new RHCOS suspect path for those failures.Suggested prompt fix
-> **RHCOS RPM changes**: Read `<summary_json_path>` and find the entry in `payloads[]` whose `tag` equals `<originating_payload_tag>`. If that entry has an `rhcos_changes[]` array, look up the RHCOS variant matching this job's `rhcos_version` using the tag mapping: `rhel-coreos` → `rhcos9`/`rhcos9-default`, `rhel-coreos-10` → `rhcos10`/`rhcos10-default`, both apply to `rhcos9_10`. Check whether any changed, added, or removed RPM packages overlap with the failure's root cause. If the failure involves OS-level components (kernel, bootloader, systemd, SELinux, rpm-ostree, cri-o, crun, runc, networking) and matching packages changed, note the potential correlation in your ANALYSIS_RESULT. +> **RHCOS RPM changes**: Read `<summary_json_path>` and find the entry in `payloads[]` whose `tag` equals `<originating_payload_tag>`. If that entry has an `rhcos_changes[]` array, look up the RHCOS variant matching this job's `rhcos_version` using the tag mapping: `rhel-coreos` → `rhcos9`/`rhcos9-default`, `rhel-coreos-10` → `rhcos10`/`rhcos10-default`, both apply to `rhcos9_10`. Check whether any changed, added, or removed RPM packages overlap with the failure's root cause. If the failure involves OS-level components (kernel, bootloader, systemd, SELinux, rpm-ostree, cri-o, crun, runc, networking, MCO, Ignition) and matching packages changed, note the potential correlation in your ANALYSIS_RESULT.🤖 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/payload-analysis/SKILL.md` around lines 203 - 225, The RHCOS RPM changes section lists OS-level components to check for correlation (kernel, bootloader, systemd, SELinux, rpm-ostree, cri-o, crun, runc, networking), but the RHCOS 10 context section later mentions MCO and Ignition as additional OS-level components to consider. Update the component list in the RHCOS RPM changes section to include MCO and Ignition so that subagents check these components when analyzing failures, ensuring consistency across all sections of the skill documentation.
🤖 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/payload-analysis/SKILL.md`:
- Around line 451-491: The RHCOS Changes section 7.3b currently states to
include RPM diffs from "any payload in the chain" but Step 3.7 only extracts
rhcos_changes[] from the failed job's originating payload. Update the
description in section 7.3b to clarify that both the suspected RHCOS RPM changes
and the full RPM diffs should be scoped only to the originating payload that
caused the failure, and update the variable placeholder from generic
originating_payload_tag to more specifically indicate this is the failed job's
originating payload, ensuring the documentation correctly reflects that
unrelated payload changes in the chain will not be surfaced in this report.
- Around line 616-617: The completeness checklist item for RHCOS RPM correlation
(currently listed as item 5 "Missing RHCOS RPM correlation") only covers the
variant-isolation and OS-level component overlap checks, but does not account
for the fallback PR-confidence case described in Step 6.1b. Add an explicit
checklist item or expand the existing item to include checking whether RHCOS
changes should be treated as the most plausible explanation when no PR candidate
scores reach the 50% confidence threshold. This ensures the reviewer evaluates
the fallback logic path that treats RHCOS changes as suspects when other
PR-based explanations fall below the confidence threshold.
---
Outside diff comments:
In `@plugins/ci/skills/payload-analysis/SKILL.md`:
- Around line 203-225: The RHCOS RPM changes section lists OS-level components
to check for correlation (kernel, bootloader, systemd, SELinux, rpm-ostree,
cri-o, crun, runc, networking), but the RHCOS 10 context section later mentions
MCO and Ignition as additional OS-level components to consider. Update the
component list in the RHCOS RPM changes section to include MCO and Ignition so
that subagents check these components when analyzing failures, ensuring
consistency across all sections of the skill documentation.
🪄 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: 1d0505c3-ac31-43a2-b354-5154cb20f43b
📒 Files selected for processing (12)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/ci/.claude-plugin/plugin.jsonplugins/ci/skills/payload-analysis/SKILL.mdplugins/ci/skills/payload-autodl-json/SKILL.mdplugins/ci/skills/payload-results-yaml/SKILL.mdplugins/ci/skills/payload-results-yaml/scripts/test_validate.pyplugins/ci/skills/payload-results-yaml/scripts/testdata/invalid_rhcos_suspects.yamlplugins/ci/skills/payload-results-yaml/scripts/testdata/valid_with_rhcos_suspects.yamlplugins/ci/skills/payload-results-yaml/scripts/validate.pyplugins/ci/skills/payload-snapshot/SKILL.mdplugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
✅ Files skipped from review due to trivial changes (5)
- plugins/ci/.claude-plugin/plugin.json
- .claude-plugin/marketplace.json
- docs/index.html
- plugins/ci/skills/payload-results-yaml/scripts/testdata/invalid_rhcos_suspects.yaml
- plugins/ci/skills/payload-snapshot/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (6)
- plugins/ci/skills/payload-results-yaml/scripts/test_validate.py
- plugins/ci/skills/payload-results-yaml/scripts/testdata/valid_with_rhcos_suspects.yaml
- plugins/ci/skills/payload-autodl-json/SKILL.md
- plugins/ci/skills/payload-results-yaml/scripts/validate.py
- plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
- plugins/ci/skills/payload-results-yaml/SKILL.md
The release controller API returns nodeImageStreams in changelog responses with RPM diffs showing which packages changed in RHCOS images between payloads. This data was already preserved in changelog.json but invisible to the analysis pipeline. Extract RHCOS RPM diffs into summary.json, pass them to subagents as failure investigation context, and surface correlated packages as "RHCOS RPM suspects" in reports. Unlike PR candidates, RHCOS changes cannot be reverted — they are informational for manual investigation by the RHCOS or platform team. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
47d663a to
d8446f2
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
plugins/ci/skills/payload-analysis/SKILL.md (2)
616-617: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd the fallback PR-confidence case to the completeness checklist.
Step 6.1b (line 323) documents a fourth condition for RHCOS RPM correlation: when no PR candidate scores reach 50%, RHCOS changes become the most plausible explanation. The completeness checklist at lines 616–617 currently covers only variant-isolation and OS-level overlap, missing this fallback path.
Suggested fix
- 5. **Missing RHCOS RPM correlation**: If RHCOS RPM changes exist in the originating payload and failures are variant-isolated or involve OS-level components, was the correlation checked? Were relevant packages surfaced as suspects? + 5. **Missing RHCOS RPM correlation**: If RHCOS RPM changes exist in the originating payload and failures are variant-isolated, involve OS-level components, or have no high-confidence PR candidates, was the correlation checked? Were relevant packages surfaced as suspects?🤖 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/payload-analysis/SKILL.md` around lines 616 - 617, The completeness checklist for Missing RHCOS RPM correlation (at lines 616-617) is incomplete and missing a documented condition. Add a fourth bullet point or condition to the checklist that covers the fallback case where no PR candidate scores reach 50%, making RHCOS changes the most plausible explanation. Reference the condition documented in step 6.1b (line 323) and ensure the checklist now covers all three conditions: variant-isolation, OS-level overlap, and this PR-confidence fallback scenario where low PR scores shift focus to RHCOS changes.
451-491: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winScope the RHCOS diff report to the originating payload that caused the failure.
Line 453 states "Include this section after the failed job details when any payload in the chain has RHCOS RPM changes." However, Step 3.7 (line 172–186) only extracts
rhcos_changes[]from the failed job's originating payload, and the HTML template at line 476 specifically references{originating_payload_tag}. This inconsistency can blur which payload actually introduced the suspect change.Update the description to clarify that RHCOS diffs are scoped to the originating payload only.
Suggested fix
- Include this section after the failed job details when any payload in the chain has RHCOS RPM changes. If RHCOS RPM suspects were identified (Step 6.1b), show them prominently first, then include the full RPM diff in a collapsible section. + Include this section after the failed job details when the failed job's originating payload has RHCOS RPM changes. If RHCOS RPM suspects were identified (Step 6.1b), show them prominently first, then include the full RPM diff in a collapsible section.Also update line 474 for clarity:
- Always include full RPM diffs when RHCOS changes exist in any originating payload + Always include full RPM diffs when RHCOS changes exist for the originating payload🤖 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/payload-analysis/SKILL.md` around lines 451 - 491, The description in section 7.3b (line 453) states that RHCOS RPM diffs are included when "any payload in the chain" has changes, which contradicts the actual implementation that only extracts rhcos_changes from the failed job's originating payload (Step 3.7) and references {originating_payload_tag} in the template. Update the description at line 453 to clarify that RHCOS RPM diffs are scoped specifically to the originating payload of the failed job, not all payloads in the chain. Also update line 474 for consistency to make the scope explicit and match the actual data extraction behavior.
🤖 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.
Duplicate comments:
In `@plugins/ci/skills/payload-analysis/SKILL.md`:
- Around line 616-617: The completeness checklist for Missing RHCOS RPM
correlation (at lines 616-617) is incomplete and missing a documented condition.
Add a fourth bullet point or condition to the checklist that covers the fallback
case where no PR candidate scores reach 50%, making RHCOS changes the most
plausible explanation. Reference the condition documented in step 6.1b (line
323) and ensure the checklist now covers all three conditions:
variant-isolation, OS-level overlap, and this PR-confidence fallback scenario
where low PR scores shift focus to RHCOS changes.
- Around line 451-491: The description in section 7.3b (line 453) states that
RHCOS RPM diffs are included when "any payload in the chain" has changes, which
contradicts the actual implementation that only extracts rhcos_changes from the
failed job's originating payload (Step 3.7) and references
{originating_payload_tag} in the template. Update the description at line 453 to
clarify that RHCOS RPM diffs are scoped specifically to the originating payload
of the failed job, not all payloads in the chain. Also update line 474 for
consistency to make the scope explicit and match the actual data extraction
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 099c8dd7-99b9-4e02-8c8e-344f8d13cacb
📒 Files selected for processing (12)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/ci/.claude-plugin/plugin.jsonplugins/ci/skills/payload-analysis/SKILL.mdplugins/ci/skills/payload-autodl-json/SKILL.mdplugins/ci/skills/payload-results-yaml/SKILL.mdplugins/ci/skills/payload-results-yaml/scripts/test_validate.pyplugins/ci/skills/payload-results-yaml/scripts/testdata/invalid_rhcos_suspects.yamlplugins/ci/skills/payload-results-yaml/scripts/testdata/valid_with_rhcos_suspects.yamlplugins/ci/skills/payload-results-yaml/scripts/validate.pyplugins/ci/skills/payload-snapshot/SKILL.mdplugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
✅ Files skipped from review due to trivial changes (7)
- plugins/ci/.claude-plugin/plugin.json
- plugins/ci/skills/payload-results-yaml/scripts/testdata/valid_with_rhcos_suspects.yaml
- .claude-plugin/marketplace.json
- plugins/ci/skills/payload-results-yaml/scripts/test_validate.py
- plugins/ci/skills/payload-snapshot/SKILL.md
- docs/index.html
- plugins/ci/skills/payload-results-yaml/scripts/testdata/invalid_rhcos_suspects.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
- plugins/ci/skills/payload-autodl-json/SKILL.md
- plugins/ci/skills/payload-results-yaml/SKILL.md
- plugins/ci/skills/payload-results-yaml/scripts/validate.py
|
/test ? |
|
/test eval-payload-analysis-minimal |
|
/test eval-payload-analysis |
|
There does not seem to be any good eval candidate available at the moment. I was looking into using TRT-2723 but it is hard to reconstruct what exactly happened there and when, because affected problematic payloads are now GC'd and the snapshots we have do not correspond with what is described in the card:
But that was a green payload and therefore we do not have its snapshot. Our first alert about TRT-2723 seems to be 5.0.0-0.ci-2026-06-18-115849 and we do have its snapshot but it strangely shows cri-o as downgraded: This makes it somewhat hard to reconstruct what actually happened and how exactly was cri-o part of the issue, I think we need to wait for something clearer. The evals show mistakes in cases 006 and 007, but they do not seem to be caused by the changes in this PR, the same flaws are present in the latest run from #556 as well. |
|
It is unfortunate evals are so expensive, I am concerned by the reduction in accuracy but the only way to prove it, is to spend a ridiculous amount of money - or just wait to watch the agent's performance over time. What are the chances the payload snapshot might be able to get the RPM changelog from the payload under test? I guess it won't work in Prow without nested podman, unless we can somehow extract the rpmdb and use rpm in the container itself |
|
/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 |
2 participants
The release controller API returns
nodeImageStreamsin changelog responses with RPM diffs showing which packages changed in RHCOS images between payloads. This data was already preserved inchangelog.jsonbut invisible to the analysis pipeline —summary.jsondidn't extract it, the payload-analysis skill didn't consider it, and reports didn't surface it.RHCOS RPM changes (systemd, kernel, glibc, SELinux, etc.) can cause failures, especially on variant-isolated jobs (rhcos10-only or rhcos9-only). When no PR candidate explains a failure, RHCOS changes are a likely culprit. Unlike PRs, they can't be reverted through the normal mechanism, so they're surfaced as informational suspects rather than revert candidates.
Changes:
payload_snapshot.py: extractnodeImageStreamsRPM diffs intosummary.jsonper-payloadrhcos_changes[]entriespayload-analysis/SKILL.md: pass RHCOS RPM changes to subagents as failure context, add RHCOS RPM correlation step (6.1b), add RHCOS changes section to HTML report (7.3b), extend completeness reviewpayload-results-yaml/SKILL.md: add optionalrhcos_suspects[]array separate from PRcandidates[]payload-autodl-json/SKILL.md: addrhcos_change_suspectedandrhcos_suspect_packagesfields🤖 Generated with Claude Code
Summary by CodeRabbit
rhcos_changes[]and new RHCOS suspect fields/sectionsrhcos_suspects, including missing-field failure cases