Add z-stream testing support to CI doctor pipeline#212
Conversation
openshift-ci
Bot
added
the
approved
|
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: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR bumps lvms-ci metadata, adds a z-stream trigger script and related doctor guidance, updates shared doctor job aggregation, and extends report generation to load and render z-stream summaries. ChangesZ-Stream integration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 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/lvms-ci/skills/doctor/SKILL.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0) ... [truncated 1086 characters] ... node:internal/modules/esm/resolve:271:11) 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 |
kasturinarra
force-pushed
the
add-zstream-trigger
branch
2 times, most recently
from
fab25d4 to
10ddcf3
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 |
openshift-ci
Bot
added
the
do-not-merge/hold
- 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>
kasturinarra
force-pushed
the
add-zstream-trigger
branch
from
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
- 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>
openshift-ci
Bot
added
the
ready-for-human-review
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>
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
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>
|
[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 |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/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. |
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>
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/shared/scripts/doctor.sh (1)
359-368: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSplit
_component_reposoutput before the loop
_component_reposreturns a space-separated list, butwhile IFS= read -r repo; do ... done <<< "${repos}"only reads a single newline-delimited record. That makes${repo}contain the entire repo list for microshift and lvm-operator, so the catalog lookup and image fetches run against one malformed repo string. Split into an array (or emit one repo per line) so each repository is processed independently.🤖 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/shared/scripts/doctor.sh` around lines 359 - 368, The `_component_repos` result is space-separated, but the `while IFS= read -r repo` loop in `doctor.sh` treats it as a single newline-delimited record, so each repo is not processed independently. Update the `repos` handling near `_component_repos` and the loop that collects container image health to split the repos into separate items first, then iterate over each repository one at a time so the catalog lookup and image fetch logic runs per repo instead of against one combined string.
🤖 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/shared/scripts/doctor.sh`:
- Around line 359-368: The `_component_repos` result is space-separated, but the
`while IFS= read -r repo` loop in `doctor.sh` treats it as a single
newline-delimited record, so each repo is not processed independently. Update
the `repos` handling near `_component_repos` and the loop that collects
container image health to split the repos into separate items first, then
iterate over each repository one at a time so the catalog lookup and image fetch
logic runs per repo instead of against one combined string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 3c5effb9-909a-4485-833e-34436d4bba59
📒 Files selected for processing (3)
plugins/lvms-ci/scripts/zstream-trigger.shplugins/lvms-ci/skills/doctor/SKILL.mdplugins/shared/scripts/doctor.sh
✅ Files skipped from review due to trivial changes (1)
- plugins/lvms-ci/skills/doctor/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/lvms-ci/scripts/zstream-trigger.sh
|
@coderabbitai review |
✅ Action performedReview finished.
|
3 participants
Report rendering when z-stream is not selected:
Report rendering when z-stream is selected:
Summary by CodeRabbit