Add z-stream testing support to CI doctor pipeline#212
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:
WalkthroughChangesLVMS CI PR reporting
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
fab25d4 to
10ddcf3
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/lvms-ci/scripts/zstream-trigger.sh`:
- Around line 186-190: The snapshot extraction in extract_snapshot is causing
the script to abort when no snapshot line is found because grep returns non-zero
under pipefail. Update the extract_snapshot pipeline so a missing match is
treated as an empty/absent result instead of a failure, and let the later
snapshot handling code in the main flow continue so it can write the intended
error summary. Keep the fix localized to the extract_snapshot logic and its
caller that assigns snapshot=$(extract_snapshot ...).
- Around line 427-430: The digest state update in the trigger flow should not
run when only some Gangway jobs were triggered successfully. In the
zstream-trigger.sh update-state block, change the condition around save_digest
so it only persists after all intended triggers succeed, using the existing
triggered count plus whatever total/expected trigger count is already available
in this script to detect partial failures. Keep the DRY_RUN guard, and make the
logic in the trigger path ensure a digest is marked tested only when every
Gangway trigger completes successfully.
- Around line 80-86: The option parsing in zstream-trigger.sh reads $2 for
--releases, --workdir, and --token-file without first confirming a value exists,
which breaks under set -u. Update the argument handling in the main while/case
loop to validate that each of these options has a following operand before
assigning from "${2}", and route missing operands through the script’s existing
usage/error path. Keep the fix localized to the option parser so the script
fails fast with a clear message instead of an unbound-variable error.
In `@plugins/lvms-ci/skills/doctor/SKILL.md`:
- Line 150: The SKILL.md description for the Periodics tab overstates when the
Z-Stream checkbox appears; update the wording to match the actual behavior in
the report generation flow so it only says the checkbox is shown when
zstream-summary.json contains skipped z-stream releases. Refer to the Periodics
tab/Z-Stream checkbox description in SKILL.md and revise that sentence to avoid
implying the file’s mere presence is enough.
In `@plugins/shared/scripts/create-report.py`:
- Around line 1608-1617: The filterZstream() logic is hiding too many list items
because the selector `#tab-periodics` ul > li:not(.toc-zstream) matches nested
issue-detail bullets as well as the TOC. Update filterZstream so the TOC
visibility toggle only targets the top-level TOC list for the periodics tab,
while leaving nested lists inside z-stream sections unaffected. Keep the
existing z-stream section and release-section toggles, but scope the li selector
to the TOC container using the existing filterZstream and toc-zstream
identifiers.
In `@plugins/shared/scripts/doctor.sh`:
- Line 130: The loops in doctor.sh are iterating over unquoted command-output
scalars, which causes word splitting and can break paths with spaces. Update the
zrel loop and the finalize-related loop block to use quoted expansions or a
safer iteration pattern, and keep the changes aligned with the existing
shellcheck-friendly style in doctor.sh and the finalize logic so WORKDIR values
are handled correctly.
- Around line 149-155: The z-stream collection failure path in the release job
gathering logic is masking errors by writing an empty jobs array and still
adding the patch version. Update the failure branch in the z-stream handling
around prow-jobs-for-release.sh to match the regular release error flow:
preserve the error state instead of emitting "[]", avoid appending the patch
version on failure, and let the report surface that collection failed. Make the
same change in the other z-stream failure branch mentioned by the review so both
paths behave consistently.
🪄 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: Enterprise
Run ID: a1302976-e23b-4964-86c3-aa0bf5bc3ec1
📒 Files selected for processing (6)
.claude-plugin/marketplace.jsonplugins/lvms-ci/.claude-plugin/plugin.jsonplugins/lvms-ci/scripts/zstream-trigger.shplugins/lvms-ci/skills/doctor/SKILL.mdplugins/shared/scripts/create-report.pyplugins/shared/scripts/doctor.sh
|
/hold |
10ddcf3 to
976fceb
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/lvms-ci/scripts/zstream-trigger.sh`:
- Line 102: The shell boolean checks are invoking `${DRY_RUN}` and similar
variables as commands instead of quoting them, which violates the shell quoting
guideline. Update the conditional logic in the zstream trigger script to use
quoted variable comparisons or a small helper for boolean evaluation, and apply
the same fix to the other affected checks involving DRY_RUN and FORCE. Use the
existing shell conditions in the script as the places to adjust, keeping all
variable expansions quoted.
- Around line 151-158: `find_zstream_pr` and `extract_snapshot` are treating
GitHub/API or jq failures like “no release” instead of an error state. Update
these helpers to validate the GitHub response before parsing, detect curl/jq
failures explicitly, and write the per-release JSON with status: "error" when
the API call or parsing fails. Make sure the downstream doctor/reporting path
can distinguish these failures from a real empty result, while keeping the
existing no_pr path only for successful queries that return no matching release.
- Line 31: The Quay repository namespace in zstream-trigger.sh is misspelled,
causing digest lookup to target the wrong catalog image path. Update the
QUAY_REPO value to use the correct logical-volume-manage-tenant namespace so the
repository reference resolves properly.
- Line 444: The jobs array serialization in the zstream trigger pipeline is
producing a phantom empty entry when triggered_jobs is empty, so update the
jq/printf handling to emit an actual empty array instead of [""] and keep
summaries aligned with []; adjust the --argjson jobs construction in
zstream-trigger.sh so the code path that builds the jobs payload correctly
handles the empty triggered_jobs case.
🪄 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: Enterprise
Run ID: 8b0164e9-a3e3-4ecf-997f-8bf567430641
📒 Files selected for processing (6)
.claude-plugin/marketplace.jsonplugins/lvms-ci/.claude-plugin/plugin.jsonplugins/lvms-ci/scripts/zstream-trigger.shplugins/lvms-ci/skills/doctor/SKILL.mdplugins/shared/scripts/create-report.pyplugins/shared/scripts/doctor.sh
✅ Files skipped from review due to trivial changes (3)
- plugins/lvms-ci/.claude-plugin/plugin.json
- .claude-plugin/marketplace.json
- plugins/lvms-ci/skills/doctor/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/shared/scripts/doctor.sh
- plugins/shared/scripts/create-report.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/lvms-ci/scripts/zstream-trigger.sh (1)
281-286: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
-o /dev/stderris nullified by the trailing2>/dev/null.
/dev/stderrresolves to fd 2, which the shell has already redirected to/dev/null, so the response body is discarded rather than surfaced. On a failed trigger (warn "failed (HTTP ${http_code})") there's no body to explain the failure. If surfacing the body was the intent, drop2>/dev/null(-sSalready suppresses progress and only prints curl errors on failure); otherwise-o /dev/nullis clearer.🤖 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/lvms-ci/scripts/zstream-trigger.sh` around lines 281 - 286, The curl call in zstream-trigger.sh is discarding the response body because `-o /dev/stderr` is immediately overridden by `2>/dev/null`, so failures in the execution trigger path lose useful error details. Update the `curl` invocation in the `http_code=$(...)` block to either remove the trailing redirection so the body can be surfaced, or redirect explicitly to `/dev/null` if the body is intentionally ignored; keep the behavior consistent with the later `warn "failed (HTTP ${http_code})"` handling.
🤖 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/lvms-ci/scripts/zstream-trigger.sh`:
- Around line 237-243: The tag चयन logic in zstream-trigger.sh should ignore
Quay tags that do not have last_modified before calling strptime(), because a
null value can abort the whole digest lookup. Update the jq pipeline in the
result assignment to filter out entries with missing last_modified before
computing .tag_epoch, while keeping the existing tag-name prefix and regex
checks intact. Use the existing tags_json processing block and the .abs_delta
sorting flow as the place to apply the guard.
---
Nitpick comments:
In `@plugins/lvms-ci/scripts/zstream-trigger.sh`:
- Around line 281-286: The curl call in zstream-trigger.sh is discarding the
response body because `-o /dev/stderr` is immediately overridden by
`2>/dev/null`, so failures in the execution trigger path lose useful error
details. Update the `curl` invocation in the `http_code=$(...)` block to either
remove the trailing redirection so the body can be surfaced, or redirect
explicitly to `/dev/null` if the body is intentionally ignored; keep the
behavior consistent with the later `warn "failed (HTTP ${http_code})"` 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: a611ae28-3746-4dc1-9b24-ced486f3bc7c
📒 Files selected for processing (1)
plugins/lvms-ci/scripts/zstream-trigger.sh
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kasturinarra, pacevedom 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 |
|
/unhold |
|
/hold |
|
I would like us to make an effort to separate trigger invocation similar to how we do this with MicroShift rebase PR failed job restarts. |
✅ Action performedReview finished.
|
- Add zstream-trigger.sh for detecting and triggering z-stream e2e jobs - Integrate z-stream into doctor prepare/finalize using unified release-* naming - Render z-stream sections in report with patch version labels (e.g., 4.19.3) - Add Z-Stream checkbox to toggle between regular and z-stream views - Make report component-agnostic via COMPONENT_REPOS mapping - GCS-based state persistence for CI step-registry trigger script Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Switch from -on-push tags (which expire) to v{version}-{commit} merge tags
- Fix empty triggered_jobs producing [""] instead of []
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The v{version}-{commit} merge tags can appear before or after the
snapshot timestamp, so pick the closest match regardless of direction
instead of requiring a narrow 30-minute window.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Retry up to 3 times on HTTP 429 with exponential backoff (15s, 30s). Increase base delay between triggers from 5s to 30s. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…apshot content Filter GitHub PR search to base=release-management branch (all LVMS release PRs target this branch), eliminating the need for title regex matching. Instead, match PRs by checking if their catalog files contain a snapshot referencing the target version. This handles varying title formats and reduces API calls (5 PRs vs 100). The snapshot is now extracted during PR discovery and returned alongside the PR number/title, removing the need for a separate extract_snapshot call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Skip tags with missing last_modified before calling strptime to prevent jq pipeline abort during digest resolution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move z-stream artifact collection out of the shared doctor.sh into zstream-trigger.sh so doctor stays reporting-only with no LVMS-specific logic. For skipped releases (same digest), the trigger script now collects failed periodic jobs and downloads artifacts into the workdir using the same release-*-jobs.json naming, so doctor's finalize discovers them automatically. Also make --releases a required argument instead of hardcoding the release list. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Z-stream entries with PRs now display alongside rebase PRs in the Pull Requests tab using release version headers (e.g. "Release 4.18.5"). Entries without PRs (no_pr, completed) are omitted. Removes the z-stream checkbox, filterZstream JS, and zstream-section CSS from the Periodics tab. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove zstream-trigger.sh (Gangway API triggering) — z-stream tests now run as Prow presubmits on release-management branch. Add LVMS-specific prow-jobs-for-pull-requests.sh to collect open PRs and their presubmit results from GCS. Make doctor.sh component-aware for PR collection (--author for MicroShift, --base for LVMS) and add --release-prs alias. Remove all z-stream rendering from create-report.py (keeping _pick_zstream_tag used by Images tab). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The find-based discovery of release-*-jobs.json was needed to pick up z-stream patch version files created by zstream-trigger.sh. Now that the trigger is removed, only CLI-passed releases have job files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
aggregate.py handles the no-reports case gracefully, so the find check for report files before calling it is unnecessary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Restore the original `for repo in ${repos}` loop from main — the
while/read change was introduced on this branch but is unrelated to
z-stream work.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove cosmetic and scaffolding changes introduced on this branch that are not needed for the z-stream PR collection work: JS selector tweak, whitespace changes, js_content variable rename, TOC div wrapper, and display/css_class/style variables left over from z-stream rendering removal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Show the catalog index image metadata (image ref, digest, build date, source commit) per PR in the Pull Requests tab, matched by extracting the release version from the PR title or presubmit job names. Also rename "Rebase Pull Requests" headings to "Pull Requests" in the error and empty states to match the overview card label. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
f9a64ab to
691a563
Compare
|
@coderabbitai summary |
✅ Action performedSummary regeneration triggered. |
|
@coderabbitai generate summary |
|
Regenerating the PR summary. ✅ Action performedSummary regeneration triggered. |
b964b85 to
b29c46d
Compare
Move duplicated script from plugins/lvms-ci and plugins/microshift-ci into plugins/shared/scripts with a --component flag that maps component names to GitHub repos and GCS paths internally. Both plugin directories now symlink to the shared copy, matching the pattern used by all other shared scripts. Also rename doctor.sh --rebase|--release-prs flag to --pull-requests since it now covers all PR types, not just rebase PRs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
b29c46d to
51b506c
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
f2482c1 to
e3dd029
Compare
Report rendering now :
Summary by CodeRabbit
New Features
Improvements
Chores
lvms-ciplugin version to 1.2.0.