payload-snapshot: surface podman stderr on RPMDB extraction failure - #667
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe payload snapshot now reports Podman failures, validates process results during RPMDB extraction, includes payload tags in logs, and removes temporary containers. CI plugin metadata is updated from 0.0.78 to 0.0.79. ChangesCI payload snapshot
CI plugin version synchronization
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant RpmdbCollector
participant PodmanRunner
participant Podman
RpmdbCollector->>PodmanRunner: Run RPMDB extraction commands
PodmanRunner->>Podman: Execute Podman command
Podman-->>PodmanRunner: Return CompletedProcess
PodmanRunner-->>RpmdbCollector: Return result and diagnostics
RpmdbCollector->>Podman: Remove extraction container
Possibly related PRs
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)
Comment |
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/payload-snapshot/scripts/payload_snapshot.py`:
- Around line 1377-1386: Update the cleanup in the finally block of the podman
operation to inspect the return code from podman rm and log cleanup failures,
including exceptions. Ensure cleanup failure after a successful podman cp
follows an explicit policy and prevents the surrounding collection flow from
reporting success or writing .complete, while preserving the existing timeout
and startup-error handling.
🪄 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: e2a14e3b-67a0-4a74-9743-a722751b81c3
📒 Files selected for processing (4)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/ci/.claude-plugin/plugin.jsonplugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
RpmdbCollector previously swallowed subprocess stderr/exit codes when podman commands failed (e.g. reading image-references, create/cp), leaving only an unhelpful "failed to ..." warning with no diagnostic detail.
8c161f6 to
991df5d
Compare
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/payload-snapshot/scripts/payload_snapshot.py`:
- Around line 1391-1393: Update the TimeoutExpired handling in _extract_rpmdb
and _run_podman to read and normalize e.stderr whether it is bytes or str, log
it when non-empty, and retain the existing timeout failure behavior. Add
regression coverage verifying partial stderr is logged for both data types.
🪄 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: 9aff6f62-9c7c-4f5a-b6ba-551251abcbbe
📒 Files selected for processing (4)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/ci/.claude-plugin/plugin.jsonplugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
🚧 Files skipped from review as they are similar to previous changes (3)
- .claude-plugin/marketplace.json
- docs/index.html
- plugins/ci/.claude-plugin/plugin.json
_extract_rpmdb hand-rolled the same log-exit-code/stderr/timeout/OSError pattern three times across nested try/except blocks. Have _run_podman return the CompletedProcess and do the logging once, so callers just check .returncode.
b51cf33 to
7f0ca05
Compare
RpmdbCollector runs one instance per payload in the chain, concurrently across worker threads. Its per-variant log lines only named the RHCOS variant (e.g. "rhel-coreos: rpmdb.sqlite extracted"), so interleaved output from multiple payloads was indistinguishable. Thread the payload's chain tag through, matching the pattern JUnitCollector already uses.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: petr-muller, smg247 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 |
RpmdbCollector previously swallowed subprocess stderr/exit codes when podman commands failed (e.g. reading image-references, create/cp), leaving only an unhelpful "failed to ..." warning with no diagnostic detail.
Summary by CodeRabbit
Bug Fixes
Chores