OCPBUGS-84873: Fix to agent integration test finding agent-tui files#10530
Conversation
|
@bfournie: This pull request references Jira Issue OCPBUGS-84873, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.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)
Walkthrough
Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. 🔧 golangci-lint (2.11.4)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
|
/jira refresh |
|
@bfournie: This pull request references Jira Issue OCPBUGS-84873, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
|
@bfournie: This pull request references Jira Issue OCPBUGS-84873, which is valid. 3 validation(s) were run on this bug
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. |
|
/test ? |
|
/test integration-tests |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/tshelpers/custom_commands.go`:
- Around line 602-610: The code currently masks Seek/I/O failures by breaking
out of loops (e.g., after initRdImg.Seek calls near the lookForCpioFiles flow)
which leads to misleading NotFound returns; change those break paths to return
the actual error (propagate the err) so seek/IO errors are diagnosable —
specifically, in the blocks where newPos, err := initRdImg.Seek(...) and the
subsequent seek inside the error branch after lookForCpioFiles(...) are handled,
replace the break statements with returns of err (or wrap/contextualize the
error) so the caller receives the real failure instead of continuing and
eventually returning NotFound; do the same for the other similar seek error path
later in the function.
- Around line 591-593: The loop that scans buff for the gzip magic (checking
buff[idx+0], buff[idx+1], buff[idx+2] using gzipID1, gzipID2, gzipDeflate) can
miss a magic sequence split across reads; modify the read loop to preserve a
2-byte overlap between iterations by copying the last two bytes of the previous
buffer to the start of buff before reading new data, adjust n to account for
those carried bytes, and then run the existing scan (for idx := 0; idx < n-2;
idx++) so boundary-spanning sequences like 0x1f 0x8b 0x08 are detected.
- Around line 585-588: The loop reading from initRdImg using
initRdImg.Read(buff) incorrectly breaks on io.EOF before processing a possible
final partial buffer and ignores non-EOF errors; change the loop around
initRdImg.Read(buff) (the call and its surrounding loop) to first handle the
bytes read (use n to process/write the valid data), then check err: if err ==
io.EOF break after processing, else if err != nil return the error. Ensure you
reference and update the read loop that uses initRdImg.Read(buff) so final
partial reads are handled and real errors are propagated.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 58f0f640-e3ce-4a2e-91f7-109da43ae67a
📒 Files selected for processing (1)
internal/tshelpers/custom_commands.go
|
/cc @zaneb @andfasano |
598af99 to
63468a3
Compare
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 `@internal/tshelpers/custom_commands.go`:
- Around line 586-587: The build breaks because the name errors is currently
bound to github.com/go-openapi/errors, but the code calls errors.Is() from the
standard library; change the import to bring in the stdlib errors under a
distinct alias (e.g. stderrors) and update the Is() calls (the checks around the
error handling in custom_commands.go that currently call errors.Is(err, io.EOF))
to use that alias (stderrors.Is). Make the same replacement for both occurrences
(the two checks that use errors.Is) so imports and calls are consistent.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 320f92d7-4c70-41f8-b094-acf48470f580
📒 Files selected for processing (1)
internal/tshelpers/custom_commands.go
Fixed two scanner bugs in checkFileFromInitrdImg: - Use actual bytes read instead of buffer size - Seek past gzip header after each archive to prevent gzip.Reader buffering from skipping nearby archives This caused the agent integration test to fail when the appended agent-tui cpio archive landed close to the previous archive.
63468a3 to
289800d
Compare
|
/test integration-tests |
|
@bfournie: 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zaneb 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 |
|
/verified by @bfournie |
|
@zaneb: This PR has been marked as verified by 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. |
|
@bfournie: Jira Issue Verification Checks: Jira Issue OCPBUGS-84873 Jira Issue OCPBUGS-84873 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
|
Fix included in release 5.0.0-0.nightly-2026-05-04-090050 |
Fixed two scanner bugs in checkFileFromInitrdImg:
This caused the agent integration test to fail when the appended agent-tui cpio archive landed close to the previous archive.
Summary by CodeRabbit