Add z-stream testing#81376
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:
WalkthroughThis PR adds a new LVMS z-stream CI path with CI config, step-registry wiring, ownership metadata, and a Bash trigger that resolves snapshots, triggers nightly jobs through Gangway, and writes per-release summary state. ChangesLVMS z-stream trigger
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kasturinarra The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
ci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-commands.sh (1)
14-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider making
RELEASES/DRY_RUN/FORCEoverridable via environment.These are hardcoded, so enabling a dry run or forcing a re-trigger (e.g. to debug a production incident) requires a code change/PR rather than a job parameter.
♻️ Proposed refactor
-RELEASES="4.14,4.16,4.18,4.19,4.20,4.21" -DRY_RUN=false -FORCE=false +RELEASES="${RELEASES:-4.14,4.16,4.18,4.19,4.20,4.21}" +DRY_RUN="${DRY_RUN:-false}" +FORCE="${FORCE:-false}"🤖 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 `@ci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-commands.sh` around lines 14 - 16, The lvms-zstream-trigger-commands.sh defaults for RELEASES, DRY_RUN, and FORCE are hardcoded, so make them overridable from the environment instead of fixed constants. Update the variable initialization near the top of the script to respect externally supplied values while preserving the current defaults when unset, and keep the existing behavior in the trigger command path that consumes these settings.
🤖 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
`@ci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-commands.sh`:
- Around line 290-320: The process_release flow calls get_last_pr, but that
helper is missing and will cause the script to fail under set -euo pipefail
whenever no open PR is found. Add a get_last_pr implementation alongside the
existing get_last_digest logic, or refactor process_release to use the correct
helper, and make sure the returned value is safely handled before the merged
check in process_release.
- Around line 219-250: The trigger_job helper is leaking shell tracing state and
is sending the job identifier in the URL instead of the request body. In
trigger_job, preserve the prior xtrace setting around the curl call so set -x is
restored only if it was already enabled, and update the Gangway request to POST
to the executions endpoint with job_name included in the JSON payload alongside
the existing pod_spec_options. Use the existing trigger_job, http_code, and body
logic as the place to make both fixes.
- Around line 71-105: The prowjobs.js response still includes a JavaScript
variable assignment, so the jq pipeline in load_previous_summary cannot parse it
and prev_build_id never gets set. Update the curl/jq handling in
load_previous_summary to strip the leading var allBuilds = prefix before jq
processes the payload, so the previous successful build ID can be extracted
correctly and the unchanged-release path in get_last_digest can be reached.
---
Nitpick comments:
In
`@ci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-commands.sh`:
- Around line 14-16: The lvms-zstream-trigger-commands.sh defaults for RELEASES,
DRY_RUN, and FORCE are hardcoded, so make them overridable from the environment
instead of fixed constants. Update the variable initialization near the top of
the script to respect externally supplied values while preserving the current
defaults when unset, and keep the existing behavior in the trigger command path
that consumes these settings.
🪄 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: 03d11887-f605-4cc0-bb30-e98cf7d6f2bd
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/lvm-operator/openshift-lvm-operator-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (6)
ci-operator/config/openshift/lvm-operator/openshift-lvm-operator-main__zstream.yamlci-operator/step-registry/lvms/zstream/OWNERSci-operator/step-registry/lvms/zstream/trigger/OWNERSci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-commands.shci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-ref.metadata.jsonci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-ref.yaml
|
/test core-valid |
|
|
||
| local tags_json | ||
| tags_json=$(curl -sSL --connect-timeout 10 --max-time 30 \ | ||
| "${QUAY_API}/repository/${QUAY_REPO}/tag/?limit=50&filter_tag_name=like:${version_prefix}-on-push") |
There was a problem hiding this comment.
We need to use a different tag, the -on-push images are from PRs and not merge commits, meaning they have an expiry annotation that renders them stale. This would need to match to v{version}-{commit} instead.
We dont know the commit because the snapshot doesnt surface that info in the name (and doing the 1:1 exact mapping requires access to the konflux tenant) so the version filtering should do.
We would need to list tags, then exclude all auxiliary tags and we are left with raw commit tags and then the delta time logic would select the right one.
There was a problem hiding this comment.
Updated, can you help check again ?
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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
`@ci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-commands.sh`:
- Around line 291-297: The fetch_last_runs path in
lvms-zstream-trigger-commands.sh is selecting the first matching Prow job from
prow_json without ordering, so it can return a stale run. Update the jq logic in
the fetch_last_runs entry construction to sort the filtered .items by a time
field before choosing the result, matching the approach used by
load_previous_summary and ensuring the latest job is used. Keep the existing
name/state/url/started fields, but make the selection deterministic by ordering
on the job completion or start timestamp before taking the final item.
- Around line 415-419: The zstream summary JSON generation in
lvms-zstream-trigger-commands.sh is serializing an empty triggered_jobs array as
[""] because the jq pipeline always consumes a blank line from printf. Update
the jobs construction near the jq object in the zstream summary block to
explicitly handle the empty-array case, so jobs becomes [] when triggered_jobs
has no entries and otherwise contains the collected job names.
- Around line 46-52: The github_curl helper in lvms-zstream-trigger-commands.sh
is making unauthenticated GitHub API requests unless GITHUB_TOKEN is present,
which can trigger rate limits and false “no PR” behavior. Update the
lvms-zstream-trigger step so it injects a GitHub token into the environment
before github_curl runs, and verify the existing github_curl function uses that
token for Authorization when available.
🪄 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: 260bc542-6a94-4bc7-8256-c9209e5514a3
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/lvm-operator/openshift-lvm-operator-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (6)
ci-operator/config/openshift/lvm-operator/openshift-lvm-operator-main__zstream.yamlci-operator/step-registry/lvms/zstream/OWNERSci-operator/step-registry/lvms/zstream/trigger/OWNERSci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-commands.shci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-ref.metadata.jsonci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-ref.yaml
✅ Files skipped from review due to trivial changes (3)
- ci-operator/step-registry/lvms/zstream/OWNERS
- ci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-ref.metadata.json
- ci-operator/step-registry/lvms/zstream/trigger/OWNERS
🚧 Files skipped from review as they are similar to previous changes (2)
- ci-operator/config/openshift/lvm-operator/openshift-lvm-operator-main__zstream.yaml
- ci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger-ref.yaml
- Switch from -on-push tags (which expire) to v{version}-{commit} merge tags
- Fix prowjobs.js parsing by stripping var/semicolon wrapper before jq
- Add missing get_last_pr() function
- Fix tracing state leak in trigger_job() with save/restore pattern
- 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>
|
/pj-rehearse periodic-ci-openshift-lvm-operator-main-zstream-trigger |
|
@kasturinarra: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-lvm-operator-main-zstream-trigger |
|
@kasturinarra: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
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>
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-openshift-lvm-operator-main-zstream-trigger |
|
@kasturinarra: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@kasturinarra: 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. |
2 participants
Summary by CodeRabbit
This PR extends OpenShift CI to add automated “z-stream” nightly e2e testing for the
openshift/lvm-operator.ci-operator/config/openshift/lvm-operator/openshift-lvm-operator-main__zstream.yaml, defining anocpnightly candidate at version4.21. It schedules a nightlyteststrigger (cron:0 6 * * *) that runs thelvms-zstream-triggerstep and sets default target resource requests to 100m CPU / 200Mi memory.ci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-trigger(plus ref/metadata and OWNERS/approval routing). The step runslvms-zstream-trigger-commands.shin theocp/cli-jq:latestimage, mounts Gangway credentials fromtest-credentialsat/etc/gangway, and triggers nightly LVMS e2e jobs via Gangway withMULTISTAGE_PARAM_OVERRIDE_LVM_INDEX_IMAGEset to a resolved Quay image digest.Trigger behavior (practical impact):
4.14, 4.16, 4.18–4.21), the script:openshift/lvm-operator.snapshot:token from PR file diffs.v{version}Quay tag prefixzstream-summary.jsonfrom GCS (discovered via the most recent successful Prow build id for the trigger job).FORCEisn’t set), it skips triggering and records “skipped” job state; otherwise it triggers the configured nightly Gangway executions for the release, overridingMULTISTAGE_PARAM_OVERRIDE_LVM_INDEX_IMAGEwith the resolved digest.${WORKDIR}/zstream-summary.json, prints a concise summary, and removes the downloaded previous summary artifact when applicable.Script correctness/hardening incorporated:
get_last_pr()helper to check/track whether a previously tracked release PR was merged and mark the release as completed.set -xduring the Gangway curl and restoring it only when tracing was previously enabled.jobsoutput is consistently an array ([]when nothing was triggered) by buildingtriggered_jobsas an array and emitting[]when empty.prowjobs.jsper job to fetch last run results (sorting bystartTime) for the “skipped” case.