FLPATH-4054 Add nightly periodic jobs with ReportPortal launch names for CoP chart#78041
Merged
openshift-merge-bot[bot] merged 11 commits intoopenshift:mainfrom Apr 23, 2026
Conversation
…onprem-chart Implements FLPATH-4054: scheduled CI jobs for testing released and RC chart versions with clear observability in ReportPortal. Signed-off-by: Thomas Stetson <tstetson@redhat.com>
openshift-ci
Bot
added
the
approved
Contributor
|
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:
WalkthroughAdds periodic CI and Prow job definitions and an image build for insights-onprem/cost-onprem-chart, implements CHART_REF resolution with conditional --devel handling in e2e steps, extends ReportPortal metadata with dynamic launch naming, and adds a presubmit for building the periodics image. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as Prow Scheduler
participant CIOp as ci-operator
participant ImageBuilder as Image Builder
participant E2E as E2E Step
participant RP as ReportPortal
Scheduler->>CIOp: Trigger periodic job (cron)
CIOp->>ImageBuilder: Build cost-onprem-test image
ImageBuilder-->>CIOp: Image available
CIOp->>E2E: Start E2E step with CHART_REF
E2E->>E2E: Resolve CHART_REF -> set USE_LOCAL_CHART / USE_HELM_DEVEL / CHART_REF_RESOLVED
alt CHART_REF == rc and supports --devel
E2E->>E2E: USE_HELM_DEVEL=true, add --devel to deploy args
else CHART_REF == rc and no --devel
E2E->>E2E: Checkout latest RC tag, set CHART_REF_RESOLVED
else CHART_REF == release
E2E->>E2E: Use Helm repo resolution (CHART_REF_RESOLVED=release)
end
E2E->>E2E: Deploy chart, run tests, collect artifacts
E2E->>RP: Send datarouter/ReportPortal metadata (dynamic launch name, chart_version, run/release type)
RP-->>RP: Record test run
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 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)
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 |
Contributor
Author
|
/pj-rehearse periodic-ci-insights-onprem-cost-onprem-chart-main-periodics-nightly-release |
Contributor
|
@testetson22: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yaml (1)
8-31: Consider pinning versions in the test image to improve reproducibility.A few optional hardening notes on the
cost-onprem-testDockerfile:
curl ... /releases/latest/download/yq_linux_amd64andopenshift-client-linux.tar.gz(stable) both float, so every image rebuild is a moving target — identical to nightly flake risk. Pin to explicit versions like you already do forhelm-v4.0.4.pip install ... pytest-playwright+playwright install chromiumpulls browser binaries from Playwright's CDN at image-build time. A transient CDN failure breaks the periodics-images presubmit. Pinningpytest-playwright==x.y.zand considering a retry onplaywright installreduces flakes.- No
--no-cache-dir/ cleanup afterplaywright install— the chromium download is ~150MB and lives in~/.cache/ms-playwright, which is fine for a test image but worth knowing re: image size.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yaml` around lines 8 - 31, The Dockerfile (dockerfile_literal) used to build the cost-onprem-test image currently pulls floating resources; pin the downloaded CLI artifacts and Python packages: replace the yq curl target (currently .../releases/latest/download/yq_linux_amd64) with a specific yq release URL, pin the openshift-client tarball to a fixed version instead of "stable/openshift-client-linux.tar.gz", and pin pytest-playwright (and any other pip packages) to exact versions in the RUN python3 -m pip install ... command; also make the playwright install step more resilient by retrying the playwright install chromium command (or pin Playwright/browser artifact versions if available) and optionally clean up Playwright caches after install to reduce image size.ci-operator/jobs/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main-periodics.yaml (1)
1-262: Add Slack alerting to periodic jobs.The new periodic jobs lack
reporter_configunlike the existing e2e presubmit (insights-onprem-cost-onprem-chart-main-presubmits.yamllines 17–24), which notifies#fp-cos-onprem-qe-cion failures. Without a reporter, nightly-rc, nightly-release, and weekly-full failures will only surface if someone actively monitors Prow—which undermines the monitoring goal. Other periodic jobs in the repo (e.g., windup, oadp-qe) demonstrate thatreporter_configis supported in periodics.Add
reporter_configwith Slack notification to the test stanzas inci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yaml(nightly-release, nightly-rc, weekly-full) to match the presubmit alerting pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/jobs/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main-periodics.yaml` around lines 1 - 262, The periodic jobs periodic-ci-insights-onprem-cost-onprem-chart-main-periodics-nightly-rc, -nightly-release, and -weekly-full are missing reporter_config so failures aren't Slack-notified; add a reporter_config block to each job's stanza by copying the reporter_config used in insights-onprem-cost-onprem-chart-main-presubmits.yaml (the Slack notification to `#fp-cos-onprem-qe-ci`) so these periodics send alerts on failure (place the reporter_config at the same level as cron/decorate/spec in each periodic job).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-commands.sh`:
- Around line 197-277: resolve_chart_ref is emitting informational logs to
stdout which are being captured into RESOLVED_REF; change all
informational/diagnostic echo calls inside resolve_chart_ref (and inside
check_devel_support) to write to stderr instead (e.g., echo "..." >&2) so that
only the final token (the intended resolved ref or special marker like
"USE_DEVEL_FLAG") is printed to stdout and captured by RESOLVED_REF; ensure the
final successful resolution lines (the actual ref or "USE_DEVEL_FLAG" or empty
string) remain on stdout, and leave the logic that tests RESOLVED_REF, sets
USE_HELM_DEVEL, and performs git checkout unchanged.
- Around line 261-270: The script calls git (git fetch/git checkout/git
describe) from the cost-onprem-test image but that image (cost-onprem-test) does
not install git, and the script writes to ${ARTIFACT_DIR}/test_status.txt
without guarding ARTIFACT_DIR; fix by adding git to the cost-onprem-test
Dockerfile used in the periodics config (install the distro package for git in
the RUN layer that builds the cost-onprem-test image) and harden the script
around ARTIFACT_DIR: ensure ARTIFACT_DIR is set or default it (e.g., default to
a safe temp/artifacts path), mkdir -p the directory and then write
test_status.txt into that directory so writes to ${ARTIFACT_DIR}/test_status.txt
cannot accidentally target /test_status.txt; update references to git commands
(git fetch/git checkout/git describe) remain unchanged but will succeed once git
is installed.
In
`@ci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh`:
- Around line 62-78: The determine_release_type function currently infers
release type solely from HELM_CHART_VERSION/helm_chart_version which
misclassifies main-branch runs as "release"; change determine_release_type to
prefer the authoritative flags CHART_REF_RESOLVED and USE_HELM_DEVEL (already
exported by the e2e step) first: if USE_HELM_DEVEL is true or CHART_REF_RESOLVED
contains an explicit rc indicator, return "rc"; if CHART_REF_RESOLVED explicitly
equals "main" (or indicates a branch ref), return "main"; otherwise fall back to
the existing HELM_CHART_VERSION / version_info.json logic in
determine_release_type to return "release" as before.
- Around line 21-60: The function determine_launch_name is being invoked in a
subshell (via command substitution), so its export of RUN_TYPE is lost; change
the approach so the function sets globals instead of relying on
export-through-subshell: modify determine_launch_name to assign the computed
launch name to a global variable (e.g., LAUNCH_NAME_RESULT) and still set/export
RUN_TYPE inside the function, then call determine_launch_name directly (not via
dynamic_launch_name=$(determine_launch_name)) and replace uses of
dynamic_launch_name with the global LAUNCH_NAME_RESULT; alternatively split
logic into determine_run_type (which sets RUN_TYPE) and build_launch_name (which
echoes/sets LAUNCH_NAME_RESULT) and ensure both are invoked in the parent shell
before using RUN_TYPE in generate_datarouter_metadata and when building the jq
payload.
---
Nitpick comments:
In
`@ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yaml`:
- Around line 8-31: The Dockerfile (dockerfile_literal) used to build the
cost-onprem-test image currently pulls floating resources; pin the downloaded
CLI artifacts and Python packages: replace the yq curl target (currently
.../releases/latest/download/yq_linux_amd64) with a specific yq release URL, pin
the openshift-client tarball to a fixed version instead of
"stable/openshift-client-linux.tar.gz", and pin pytest-playwright (and any other
pip packages) to exact versions in the RUN python3 -m pip install ... command;
also make the playwright install step more resilient by retrying the playwright
install chromium command (or pin Playwright/browser artifact versions if
available) and optionally clean up Playwright caches after install to reduce
image size.
In
`@ci-operator/jobs/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main-periodics.yaml`:
- Around line 1-262: The periodic jobs
periodic-ci-insights-onprem-cost-onprem-chart-main-periodics-nightly-rc,
-nightly-release, and -weekly-full are missing reporter_config so failures
aren't Slack-notified; add a reporter_config block to each job's stanza by
copying the reporter_config used in
insights-onprem-cost-onprem-chart-main-presubmits.yaml (the Slack notification
to `#fp-cos-onprem-qe-ci`) so these periodics send alerts on failure (place the
reporter_config at the same level as cron/decorate/spec in each periodic job).
🪄 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: Pro Plus
Run ID: 381c2f1e-6c7c-4929-891b-b276030e1dca
📒 Files selected for processing (6)
ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yamlci-operator/jobs/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main-periodics.yamlci-operator/jobs/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main-presubmits.yamlci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-commands.shci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-ref.yamlci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh
- Added git installation to the Dockerfile for chart tag resolution. - Updated e2e command scripts to redirect output to stderr for better visibility. - Implemented artifact directory creation to ensure test status files are saved correctly. Signed-off-by: Thomas Stetson <tstetson@redhat.com>
testetson22
changed the title
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
Author
|
/pj-rehearse periodic-ci-insights-onprem-cost-onprem-chart-main-periodics-nightly-release |
Contributor
|
@testetson22: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
- Changed the variable name for the artifact directory from 'artifact_dir' to '_artifact_dir' for consistency and clarity. - Ensured the directory creation and test status file writing logic remains intact. Signed-off-by: Thomas Stetson <tstetson@redhat.com>
- Increased timeout for periodic jobs from 2 hours to 3 hours. - Changed IQE_PROFILE from 'stable' to 'full' for enhanced testing coverage. - Adjusted cron schedule for nightly jobs from 3 AM to 5 AM. Signed-off-by: Thomas Stetson <tstetson@redhat.com>
Contributor
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
`@ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yaml`:
- Around line 45-79: Add a new periodic test entry mirroring the existing
nightly entries but with as: weekly-full and a Sunday cron; reuse cluster_claim
(architecture: amd64, cloud: aws, owner: openshift-ci, product: ocp, timeout:
3h0m0s, version: "4.20"), steps env (CHART_REF: release, IQE_PROFILE: full,
RUN_IQE: "true"), test ref insights-onprem-cost-onprem-chart-e2e, and workflow
insights-onprem-cost-onprem-chart-claim, and set cron to a Sunday schedule
(e.g., "0 0 * * 0").
In
`@ci-operator/jobs/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main-periodics.yaml`:
- Around line 51-53: The pods declare container mounts with name:
gcs-credentials and mountPath: /secrets/gcs but no corresponding volume is
defined, causing admission failures; add a volume entry named gcs-credentials
under each spec.template.spec.volumes for the affected periodic job PodSpecs and
point it to the appropriate Secret (e.g., secretName: gcs-credentials) so the
container mount (name: gcs-credentials, mountPath: /secrets/gcs) has a matching
volume. Ensure you update every affected job spec that includes the mount (the
blocks around the shown mount lines and the other ranges mentioned) so all pod
specs include the volumes array with the gcs-credentials secret volume.
In
`@ci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh`:
- Around line 51-78: The determine_release_type function must treat any
authoritative CHART_REF_RESOLVED that is non-empty, not "main", and not
containing "-rc" as a release; update the conditional chain in
determine_release_type so after the existing checks for USE_HELM_DEVEL and
CHART_REF_RESOLVED == "main" and CHART_REF_RESOLVED containing "-rc", add an
elif branch like "elif [[ -n "${CHART_REF_RESOLVED:-}" ]]; then
RELEASE_TYPE='release'" so resolved tags such as "cost-onprem-0.2.19" are
classified as release instead of falling back to get_chart_version; keep the
existing get_chart_version fallback only for cases where CHART_REF_RESOLVED is
empty.
🪄 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: Pro Plus
Run ID: 7303cce1-74be-4710-af6b-828b93a67472
📒 Files selected for processing (4)
ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yamlci-operator/jobs/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main-periodics.yamlci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-commands.shci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-commands.sh
Contributor
Author
|
/pj-rehearse periodic-ci-insights-onprem-cost-onprem-chart-main-periodics-nightly-release |
Contributor
|
@testetson22: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
- Updated Dockerfile to streamline installation commands. - Enhanced e2e command scripts to clarify chart source resolution logic, including handling for 'release' and 'rc' options. - Improved documentation for CHART_REF variable to better explain its usage in testing scenarios. Signed-off-by: Thomas Stetson <tstetson@redhat.com>
Contributor
Author
|
/pj-rehearse periodic-ci-insights-onprem-cost-onprem-chart-main-periodics-nightly-release |
Contributor
|
@testetson22: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh (1)
51-81:⚠️ Potential issue | 🟠 MajorResolved concrete (non-RC) chart tags still fall through to inference.
Lines 59/62/65 only match the literal strings
release/mainor refs containing-rc. A resolved explicit tag such ascost-onprem-0.2.19or0.2.19(the*)branch ininsights-onprem-cost-onprem-chart-e2e-commands.shexports these as-is) hits theelsefallback. Ifversion_info.jsonis missing or lackshelm_chart_version, this gets reported asmaindespite being a released tag. SinceCHART_REF_RESOLVEDis treated as authoritative, any non-empty non-mainvalue should bereleaseafter the-rccheck.🐛 Proposed fix
elif [[ -n "${CHART_REF_RESOLVED:-}" ]] && [[ "${CHART_REF_RESOLVED}" == *"-rc"* ]]; then # Resolved ref contains RC indicator (explicit version like 0.2.20-rc1) RELEASE_TYPE="rc" + elif [[ -n "${CHART_REF_RESOLVED:-}" ]]; then + # Any other non-empty resolved ref (explicit tag/branch) is a released chart + RELEASE_TYPE="release" else # Fall back to inferring from chart version (for PRs using local chart)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh` around lines 51 - 81, determine_release_type currently treats only literal "release"/"main" or values containing "-rc" as authoritative, so resolved concrete tags like "0.2.19" fall through to chart-version inference; change the logic in determine_release_type to treat any non-empty CHART_REF_RESOLVED that is not "main" and does not contain "-rc" as a released tag by setting RELEASE_TYPE="release" (i.e., after the "-rc" check add a branch like if [[ -n "${CHART_REF_RESOLVED:-}" ]]; then RELEASE_TYPE="release" ), leaving get_chart_version-based fallback only for empty/unresolved refs, and continue exporting RELEASE_TYPE.
🧹 Nitpick comments (1)
ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yaml (1)
22-27: Pin CLI tool versions for reproducible image builds.
yqdownloads fromreleases/latest/andoc/kubectlfromstable/; both drift independently of this repo. A silent upstream bump can turn a green nightly red with no traceable change. Helm is already pinned tov4.0.4— do the same for yq and the OpenShift client (e.g., pin to a specific yq version tag and an explicit OCP client release directory).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yaml` around lines 22 - 27, The Dockerfile RUN step currently pulls yq from releases/latest and the OpenShift client from stable which allows silent upstream bumps; update the curl URLs used in the RUN line that installs yq, helm, oc and kubectl so yq is fetched from a specific release tag (replace releases/latest with the chosen yq version tag) and the OpenShift client URL points to a pinned OCP release directory (replace stable with the explicit release version), keep helm pinned as-is to v4.0.4; update any related variables or comments to document the chosen yq and OCP versions so image builds are reproducible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh`:
- Around line 68-78: The current fallback uses get_chart_version and treats any
non-empty non-"unknown" version as "release", which misclassifies PR/local
builds (e.g., 0.2.20-dev); update the logic in the release-type determination to
first detect PR/local runs and set RELEASE_TYPE accordingly: if
CHART_REF_RESOLVED is empty and PULL_NUMBER is set (or USE_LOCAL_CHART is true),
set RELEASE_TYPE to "main" (or "pr"/"dev" as your convention) and skip the
chart_version-based checks; otherwise proceed to call get_chart_version and keep
the existing -rc / release / unknown branching for normal chart refs.
---
Duplicate comments:
In
`@ci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh`:
- Around line 51-81: determine_release_type currently treats only literal
"release"/"main" or values containing "-rc" as authoritative, so resolved
concrete tags like "0.2.19" fall through to chart-version inference; change the
logic in determine_release_type to treat any non-empty CHART_REF_RESOLVED that
is not "main" and does not contain "-rc" as a released tag by setting
RELEASE_TYPE="release" (i.e., after the "-rc" check add a branch like if [[ -n
"${CHART_REF_RESOLVED:-}" ]]; then RELEASE_TYPE="release" ), leaving
get_chart_version-based fallback only for empty/unresolved refs, and continue
exporting RELEASE_TYPE.
---
Nitpick comments:
In
`@ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yaml`:
- Around line 22-27: The Dockerfile RUN step currently pulls yq from
releases/latest and the OpenShift client from stable which allows silent
upstream bumps; update the curl URLs used in the RUN line that installs yq,
helm, oc and kubectl so yq is fetched from a specific release tag (replace
releases/latest with the chosen yq version tag) and the OpenShift client URL
points to a pinned OCP release directory (replace stable with the explicit
release version), keep helm pinned as-is to v4.0.4; update any related variables
or comments to document the chosen yq and OCP versions so image builds are
reproducible.
🪄 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: Pro Plus
Run ID: 7483e31e-7bf9-45d6-8d3a-1b1fe8efce20
📒 Files selected for processing (4)
ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yamlci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-commands.shci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-ref.yamlci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- ci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-ref.yaml
- ci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-commands.sh
- Added installation of git to the Dockerfile to support chart tag resolution. - Reformatted Dockerfile for improved readability. Signed-off-by: Thomas Stetson <tstetson@redhat.com>
Contributor
Author
|
/pj-rehearse periodic-ci-insights-onprem-cost-onprem-chart-main-periodics-nightly-release |
Contributor
|
@testetson22: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
… insights-onprem-cost-onprem-chart Signed-off-by: Thomas Stetson <tstetson@redhat.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
ci-operator/config/insights-onprem/cost-onprem-chart/.config.prowgen (1)
1-12: Nit: consider suppressingsuccessnotifications fornightly-rctoo, or align both blocks.Both reporter blocks include
successinjob_states_to_report, which will produce a Slack message every night per job even on green runs. That may be intentional (explicit "nightly passed" signal), but if the intent was only failure/error noise reduction, dropsuccess. Flagging so the choice is explicit rather than accidental — no change needed if this is by design.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/config/insights-onprem/cost-onprem-chart/.config.prowgen` around lines 1 - 12, The slack_reporter block currently lists "success" in job_states_to_report for the nightly job (symbol: job_states_to_report under slack_reporter for job_names: nightly-release); update this to either remove "success" from job_states_to_report to suppress green-run notifications or make the other reporter block (e.g., the nightly-rc reporter) match this block so both are aligned—ensure you modify the job_states_to_report array accordingly for the relevant reporter(s) (symbols: slack_reporter, job_states_to_report, job_names: nightly-release / nightly-rc).ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yaml (1)
25-29: Consider pinningyqandoc/kubectlversions instead of trackinglatest/stable.
yqusesreleases/latest/download/...andoc/kubectlcome fromclients/ocp/stable/.... For nightly CI that should be reproducible across consecutive days, pinning to known versions (and bumping deliberately) avoids "image rebuilt today now behaves differently" surprises and mitigates supply-chain risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yaml` around lines 25 - 29, The Dockerfile RUN that fetches tools uses floating tags ("releases/latest" for yq and "clients/ocp/stable" for oc/kubectl) which makes builds non-reproducible; pin the downloads to explicit versions by replacing the yq URL and the OpenShift client URL with specific release URLs (or introduce ARGs like YQ_VERSION and OCP_CLIENT_VERSION and use them in the RUN), and do the same for the helm tarball (replace helm-v4.0.4 if you want an ARG or verify the exact desired version); update the curl download lines that reference yq_linux_amd64, helm-v4.0.4-linux-amd64, and clients/ocp/stable to use those pinned version strings so nightly images are reproducible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ci-operator/config/insights-onprem/cost-onprem-chart/.config.prowgen`:
- Line 10: The Slack template currently hardcodes the ReportPortal launch URL
"/launches/724", so update the template to reference the per-run launch instead:
modify the link string in the Slack message (the token containing
"/launches/724") to either point to a filtered launches view (e.g.,
"/launches/all?filter.has.attributeKey=run_type&filter.has.attributeValue=nightly")
or replace it with a runtime-generated artifact path produced by the
send-reportportal step (publish the created launch URL as an artifact like
version_info.json and change the Slack template to link to that artifact);
ensure you update both places where the hardcoded "/launches/724" appears so
notifications point to the correct run.
In
`@ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yaml`:
- Around line 25-29: The Dockerfile RUN line that downloads and extracts tools
(the chained curl | tar and mv commands) must be hardened: enable shell pipefail
(set -o pipefail) at the start of the RUN, and change curl invocations to use
-fsSL so failures return non-zero and don't emit HTML; also add the -f option
when piping to tar where appropriate or split the commands so curl writes to a
file and then verify/extract it before mv. Update the RUN that fetches yq, helm,
and openshift-client (the curl | tar and mv sequence) to use these flags and
ensure each step fails fast on error.
---
Nitpick comments:
In `@ci-operator/config/insights-onprem/cost-onprem-chart/.config.prowgen`:
- Around line 1-12: The slack_reporter block currently lists "success" in
job_states_to_report for the nightly job (symbol: job_states_to_report under
slack_reporter for job_names: nightly-release); update this to either remove
"success" from job_states_to_report to suppress green-run notifications or make
the other reporter block (e.g., the nightly-rc reporter) match this block so
both are aligned—ensure you modify the job_states_to_report array accordingly
for the relevant reporter(s) (symbols: slack_reporter, job_states_to_report,
job_names: nightly-release / nightly-rc).
In
`@ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yaml`:
- Around line 25-29: The Dockerfile RUN that fetches tools uses floating tags
("releases/latest" for yq and "clients/ocp/stable" for oc/kubectl) which makes
builds non-reproducible; pin the downloads to explicit versions by replacing the
yq URL and the OpenShift client URL with specific release URLs (or introduce
ARGs like YQ_VERSION and OCP_CLIENT_VERSION and use them in the RUN), and do the
same for the helm tarball (replace helm-v4.0.4 if you want an ARG or verify the
exact desired version); update the curl download lines that reference
yq_linux_amd64, helm-v4.0.4-linux-amd64, and clients/ocp/stable to use those
pinned version strings so nightly images are reproducible.
🪄 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: Pro Plus
Run ID: 471887c5-da24-415e-a03b-a3a4cbb8c8d3
📒 Files selected for processing (5)
ci-operator/config/insights-onprem/cost-onprem-chart/.config.prowgenci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yamlci-operator/jobs/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main-periodics.yamlci-operator/jobs/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main-presubmits.yamlci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-commands.sh
✅ Files skipped from review due to trivial changes (1)
- ci-operator/jobs/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main-periodics.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-commands.sh
Contributor
Author
|
/pj-rehearse periodic-ci-insights-onprem-cost-onprem-chart-main-periodics-nightly-release |
Contributor
|
@testetson22: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
ci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh (1)
51-78:⚠️ Potential issue | 🟡 MinorTighten the release-type fallback for local builds and concrete refs.
The fallback still promotes PR/local dev chart versions to
release, and concrete resolved refs likecost-onprem-0.2.19can fall through tomainifversion_info.jsonis unavailable. Once the e2e chart context is persisted into this step, prefer that context before inferring from the version string.🐛 Proposed classifier adjustment
elif [[ -n "${CHART_REF_RESOLVED:-}" ]] && [[ "${CHART_REF_RESOLVED}" == *"-rc"* ]]; then # Resolved ref contains RC indicator (explicit version like 0.2.20-rc1) RELEASE_TYPE="rc" + elif [[ -n "${CHART_REF_RESOLVED:-}" ]]; then + # Resolved concrete non-RC ref means a released chart ref was selected + RELEASE_TYPE="release" + elif [[ "${USE_LOCAL_CHART:-false}" == "true" ]]; then + # PR/local chart builds should not be classified as release from dev SemVer alone + RELEASE_TYPE="main" else # Fall back to inferring from chart version (for PRs using local chart) local chart_version chart_version=$(get_chart_version)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh` around lines 51 - 78, The determine_release_type function currently can fall through to "main" when CHART_REF_RESOLVED is a concrete ref like "cost-onprem-0.2.19" or when version_info.json is missing; update the branching so that any non-empty CHART_REF_RESOLVED (after the explicit "rc"/"release"/"main" checks) is treated as a concrete release by setting RELEASE_TYPE="release" (i.e., add an elif [[ -n "${CHART_REF_RESOLVED:-}" ]]; then RELEASE_TYPE="release"); keep the existing get_chart_version-based fallback for truly local/PR charts and ensure unknown chart_version still maps to "main" rather than promoting to "release".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-commands.sh`:
- Around line 270-272: The exported variables USE_LOCAL_CHART, USE_HELM_DEVEL
and CHART_REF_RESOLVED do not persist across pipeline steps, so write their
values into files under SHARED_DIR (e.g., use_local_chart, use_helm_devel,
chart_ref_resolved) right after they are set in
insights-onprem-cost-onprem-chart-e2e-commands.sh and then update the
send-reportportal step to read those files into the same variables before
determine_release_type runs; reference the variable names USE_LOCAL_CHART,
USE_HELM_DEVEL, CHART_REF_RESOLVED and the shared-dir marker SHARED_DIR and
ensure send-reportportal loads them (e.g., check SHARED_DIR and read the files)
so ReportPortal sees the actual values.
- Around line 258-264: The git ref check using git rev-parse "${CHART_REF}" may
fail silently yet the script still sets USE_LOCAL_CHART="true" and
CHART_REF_RESOLVED="${CHART_REF}"; modify the if block around git
rev-parse/checkout so that if git rev-parse "${CHART_REF}" returns non-zero you
handle the error: add an else branch that prints a clear error message including
the CHART_REF value to stderr and exits non-zero (e.g., echo "ERROR: unable to
resolve CHART_REF '${CHART_REF}'" >&2; exit 1) instead of proceeding to set
USE_LOCAL_CHART or CHART_REF_RESOLVED, keeping the successful-branch behavior
(git checkout and setting CHART_REF_RESOLVED / USE_LOCAL_CHART) unchanged.
---
Duplicate comments:
In
`@ci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh`:
- Around line 51-78: The determine_release_type function currently can fall
through to "main" when CHART_REF_RESOLVED is a concrete ref like
"cost-onprem-0.2.19" or when version_info.json is missing; update the branching
so that any non-empty CHART_REF_RESOLVED (after the explicit
"rc"/"release"/"main" checks) is treated as a concrete release by setting
RELEASE_TYPE="release" (i.e., add an elif [[ -n "${CHART_REF_RESOLVED:-}" ]];
then RELEASE_TYPE="release"); keep the existing get_chart_version-based fallback
for truly local/PR charts and ensure unknown chart_version still maps to "main"
rather than promoting to "release".
🪄 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: Pro Plus
Run ID: 99274095-4279-4a7e-8082-9f44276a6243
📒 Files selected for processing (2)
ci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-commands.shci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.sh
Contributor
Author
|
/pj-rehearse periodic-ci-insights-onprem-cost-onprem-chart-main-periodics-nightly-release |
Contributor
|
@testetson22: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
Author
|
/pj-rehearse periodic-ci-insights-onprem-cost-onprem-chart-main-periodics-nightly-rc |
Contributor
|
@testetson22: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
…art that will allow triggering the same profile release and rc nightly jobs run on a per PR basis. This may fail; run responsibly. Signed-off-by: Thomas Stetson <tstetson@redhat.com>
Contributor
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
Contributor
Author
|
/pj-rehearse pull-ci-insights-onprem-cost-onprem-chart-main-e2e-iqe-stable |
Contributor
|
@testetson22: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
Author
|
/pj-rehearse pull-ci-insights-onprem-cost-onprem-chart-main-e2e-iqe-full |
Contributor
|
@testetson22: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
Author
|
This should be good for review. Just making sure the full profile job don't show any errors outside of test failures. |
testetson22
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
Contributor
Author
|
/pj-rehearse pull-ci-insights-onprem-cost-onprem-chart-main-e2e |
Contributor
|
@testetson22: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
|
@testetson22: The following tests failed, say
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. |
Contributor
Author
|
/pj-rehearse ack |
Contributor
|
@testetson22: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
the
rehearsals-ack
|
/lgtm |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jordigilh, testetson22 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 |
openshift-merge-bot
Bot
merged commit 835e4e3
into
openshift:main
19 of 21 checks passed
tbuskey
pushed a commit
to tbuskey/release
that referenced
this pull request
Apr 24, 2026
…for CoP chart (openshift#78041) * Add nightly periodic jobs with ReportPortal differentiation for cost-onprem-chart Implements FLPATH-4054: scheduled CI jobs for testing released and RC chart versions with clear observability in ReportPortal. Signed-off-by: Thomas Stetson <tstetson@redhat.com> * Enhance cost-onprem-chart CI configuration - Added git installation to the Dockerfile for chart tag resolution. - Updated e2e command scripts to redirect output to stderr for better visibility. - Implemented artifact directory creation to ensure test status files are saved correctly. Signed-off-by: Thomas Stetson <tstetson@redhat.com> * Refactor artifact directory variable in e2e command script - Changed the variable name for the artifact directory from 'artifact_dir' to '_artifact_dir' for consistency and clarity. - Ensured the directory creation and test status file writing logic remains intact. Signed-off-by: Thomas Stetson <tstetson@redhat.com> * Update periodic job configurations for insights-onprem-cost-onprem-chart - Increased timeout for periodic jobs from 2 hours to 3 hours. - Changed IQE_PROFILE from 'stable' to 'full' for enhanced testing coverage. - Adjusted cron schedule for nightly jobs from 3 AM to 5 AM. Signed-off-by: Thomas Stetson <tstetson@redhat.com> * Refactor chart source resolution in CI configuration - Updated Dockerfile to streamline installation commands. - Enhanced e2e command scripts to clarify chart source resolution logic, including handling for 'release' and 'rc' options. - Improved documentation for CHART_REF variable to better explain its usage in testing scenarios. Signed-off-by: Thomas Stetson <tstetson@redhat.com> * Enhance Dockerfile for insights-onprem-cost-onprem-chart - Added installation of git to the Dockerfile to support chart tag resolution. - Reformatted Dockerfile for improved readability. Signed-off-by: Thomas Stetson <tstetson@redhat.com> * Update slack configuration and fix artifact collection on failure for insights-onprem-cost-onprem-chart Signed-off-by: Thomas Stetson <tstetson@redhat.com> * Add debug output to ReportPortal step for SHARED_DIR investigation * Add more debug output for SHARED_DIR investigation * Remove debug output from e2e and ReportPortal steps * Add e2e-iqe-full job configuration for insights-onprem-cost-onprem-chart that will allow triggering the same profile release and rc nightly jobs run on a per PR basis. This may fail; run responsibly. Signed-off-by: Thomas Stetson <tstetson@redhat.com> --------- Signed-off-by: Thomas Stetson <tstetson@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds nightly periodic jobs to test released and RC versions of cost-onprem-chart, with dynamic ReportPortal launch naming to differentiate between nightly, PR, and ad-hoc test runs.
Changes
1. New Periodic Jobs Config
File:
ci-operator/config/insights-onprem/cost-onprem-chart/insights-onprem-cost-onprem-chart-main__periodics.yamlnightly-release- Runs daily at 2:00 AM UTCnightly-rc- Runs daily at 5:00 AM UTC--develflag or git tag fallback)2. E2E Step Updates
File:
ci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-commands.shNew
CHART_REFenvironment variable controls chart source:truereleasefalsercfalseortrue--develif supported, otherwise checks out latest RC git tag3. ReportPortal Differentiation
File:
ci-operator/step-registry/insights-onprem/cost-onprem-chart/send-reportportal/insights-onprem-cost-onprem-chart-send-reportportal-commands.shDynamic launch names based on run type:
insights-onprem-e2e-nightly-<version>- Nightly jobsinsights-onprem-e2e-pr-<PR#>- PR presubmitsinsights-onprem-e2e-adhoc-<version>- Manual triggersAttributes added:
run_type,release_type,chart_version4. Documentation Updates
File:
ci-operator/step-registry/insights-onprem/cost-onprem-chart/e2e/insights-onprem-cost-onprem-chart-e2e-ref.yamlUpdated
CHART_REFdocumentation to explain the new options.Test Plan
nightly-releaserehearsal job claims cluster and installs from Helm reponightly-rcrehearsal job uses git tag fallback (until--develsupport added)USE_LOCAL_CHART=trueRelated
--develsupport indeploy-test-cost-onprem.sh(optional, has fallback)Summary by CodeRabbit
New Features
Tests
Reporting
Changes