Add z-stream testing#81376
Conversation
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds a new LVMS z-stream CI path, including CI config, step-registry wiring, ownership metadata, and a trigger script that resolves snapshots, starts 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.
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. |
|
[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, introducing anocpnightly stream candidate targeting version 4.21, with a scheduledteststrigger (cron:0 6 * * *) that runs thelvms-zstream-triggerstep. It also sets default resource requests for all targets (*) to 100m CPU / 200Mi memory.ci-operator/step-registry/lvms/zstream/trigger/lvms-zstream-triggerplus ref/metadata and ownership/routing files. The trigger runslvms-zstream-trigger-commands.shin theocp/cli-jq:latestimage with Gangway credentials mounted fromtest-credentialsand executes nightly jobs via the Gangway API.Trigger behavior (practical impact):
z-streamPR inopenshift/lvm-operator.snapshot:catalog token from the PR diff, resolves it to the best matchingquay.iocatalog manifest tag near the snapshot timestamp, and derives the corresponding image digest.zstream-summary.jsonartifact (via the most recent successful trigger build id). If the resolved digest is unchanged (andFORCEis not set), it skips triggering for that release and records per-release skipped state.MULTISTAGE_PARAM_OVERRIDE_LVM_INDEX_IMAGEwith the resolved digest (with optionalDRY_RUNsupport), writes per-release outputs, and merges them into a newzstream-summary.jsonfor the run.Script hardening/fixes included:
prowjobs.jsparsing, adding a missingget_last_pr()helper, preventing a tracing state leak intrigger_job()via save/restore, and correcting emptytriggered_jobsoutput from[""]to[]).