OSAC-866: Consolidate 8 periodic vmaas jobs into 2#80146
OSAC-866: Consolidate 8 periodic vmaas jobs into 2#80146omer-vishlitzky wants to merge 2 commits into
Conversation
Replace 8 separate periodic vmaas CI jobs (each installing a full OCP cluster to run a single test file) with two consolidated periodics: - e2e-vmaas-periodic: runs every 12h via cluster-tool snapshot boot, executes all vmaas tests in one job - e2e-vmaas-full-install: runs nightly with full OCP installation (no cluster-tool), deploys OSAC via osac-installer, runs all vmaas tests Changes: - Delete 8 e2e-metal-vmaas-* periodic entries from ci-operator config - Modify osac-project-baremetal-test step: replace single-file TEST env var with TEST_SUITE (default: vmaas) that runs the full suite - Add osac-project-notify to post steps of both workflows for Slack notifications (skipped for non-periodic jobs) - Add NOTIFY_LABEL env var to notify step for custom Slack headers - Change osac-project-ofcir-baremetal CLUSTERTYPE to assisted_large_el9 - Regenerate Prow job configs via make jobs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@omer-vishlitzky: This pull request references OSAC-866 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
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 ignored due to path filters (1)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughOSAC test infra switches from single-file test selection to TEST_SUITE directories, the baremetal test script adds artifact collection and pytest JUnit output, installer and workflows gain DEPLOY_MODE/VALUES_FILE and Helm support, and workflows add notification/gather post-steps with configurable Slack labels. ChangesOSAC Test Infrastructure Modernization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
approved
|
/pj-rehearse pull-ci-osac-project-osac-test-infra-main-e2e-vmaas pull-ci-osac-project-osac-aap-main-e2e-vmaas pull-ci-osac-project-fulfillment-service-main-e2e-vmaas pull-ci-osac-project-osac-operator-main-e2e-vmaas pull-ci-osac-project-osac-installer-main-e2e-vmaas periodic-ci-osac-project-osac-test-infra-main-e2e-vmaas-full-install periodic-ci-osac-project-osac-test-infra-main-e2e-vmaas-periodic |
|
@omer-vishlitzky: 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}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/osac-project/baremetal/test/osac-project-baremetal-test-commands.sh (1)
19-27: ⚡ Quick winValidate
TEST_SUITEbefore using it in paths.
TEST_SUITEis interpolated into file paths and the pytest target without format checks. A simple allowlist regex avoids unintended path expansion (for example../...) and keeps suite selection bounded.Suggested patch
+if [[ ! "${TEST_SUITE}" =~ ^[a-zA-Z0-9_-]+$ ]]; then + echo "Invalid TEST_SUITE: ${TEST_SUITE}" + exit 1 +fi + echo "Running OSAC E2E tests: suite=${TEST_SUITE}"Also applies to: 30-57
🤖 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/osac-project/baremetal/test/osac-project-baremetal-test-commands.sh` around lines 19 - 27, Validate and sanitize TEST_SUITE before it is used in paths and passed to pytest: add an allowlist check (e.g. /^[A-Za-z0-9._-]+$/ or similar) at the top of the script that rejects or exits on any value not matching the regex, normalize/strip dangerous sequences (e.g. remove leading ../ or path separators) and use the validated variable in the ssh command invocation and any file/path concatenations (referencing TEST_SUITE in the ssh -F ... ci_machine bash -s invocation and any later uses of TEST_SUITE in file paths/pytest targets) so untrusted values cannot cause path traversal or command injection.
🤖 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.
Nitpick comments:
In
`@ci-operator/step-registry/osac-project/baremetal/test/osac-project-baremetal-test-commands.sh`:
- Around line 19-27: Validate and sanitize TEST_SUITE before it is used in paths
and passed to pytest: add an allowlist check (e.g. /^[A-Za-z0-9._-]+$/ or
similar) at the top of the script that rejects or exits on any value not
matching the regex, normalize/strip dangerous sequences (e.g. remove leading ../
or path separators) and use the validated variable in the ssh command invocation
and any file/path concatenations (referencing TEST_SUITE in the ssh -F ...
ci_machine bash -s invocation and any later uses of TEST_SUITE in file
paths/pytest targets) so untrusted values cannot cause path traversal or command
injection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 0baa4d92-b663-431d-b733-7400c21f237c
⛔ Files ignored due to path filters (1)
ci-operator/jobs/osac-project/osac-test-infra/osac-project-osac-test-infra-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (7)
ci-operator/config/osac-project/osac-test-infra/osac-project-osac-test-infra-main.yamlci-operator/step-registry/osac-project/baremetal/test/osac-project-baremetal-test-commands.shci-operator/step-registry/osac-project/baremetal/test/osac-project-baremetal-test-ref.yamlci-operator/step-registry/osac-project/cluster-tool/vmaas/osac-project-cluster-tool-vmaas-workflow.yamlci-operator/step-registry/osac-project/notify/osac-project-notify-commands.shci-operator/step-registry/osac-project/notify/osac-project-notify-ref.yamlci-operator/step-registry/osac-project/ofcir/baremetal/osac-project-ofcir-baremetal-workflow.yaml
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: omer-vishlitzky 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 |
|
/pj-rehearse |
|
@omer-vishlitzky: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/config/osac-project/osac-installer/osac-project-osac-installer-main.yaml`:
- Around line 30-32: The Dockerfile RUN that fetches and extracts Helm (the RUN
line using curl to download https://get.helm.sh/helm-v3.17.3-linux-amd64.tar.gz
and then tar/mv) must verify the artifact before extraction: download both the
tarball and its published SHA256 file (or the .tar.gz.sha256sum), compute the
local sha256 (sha256sum) and compare to the published value, aborting the build
if they differ; only after successful verification proceed to tar -xzf and mv
/tmp/linux-amd64/helm /usr/local/bin/helm. Ensure the verification is done in
the same RUN step so intermediate layers don’t keep an unverified artifact and
clean up downloaded files afterwards.
🪄 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: 4b09841d-8ac4-43e5-ba8f-e38838e53110
⛔ Files ignored due to path filters (1)
ci-operator/jobs/osac-project/osac-test-infra/osac-project-osac-test-infra-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (5)
ci-operator/config/osac-project/osac-installer/osac-project-osac-installer-main.yamlci-operator/config/osac-project/osac-test-infra/osac-project-osac-test-infra-main.yamlci-operator/step-registry/osac-project/installer/osac-project-installer-commands.shci-operator/step-registry/osac-project/installer/osac-project-installer-ref.yamlci-operator/step-registry/osac-project/ofcir/baremetal/osac-project-ofcir-baremetal-workflow.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/config/osac-project/osac-test-infra/osac-project-osac-test-infra-main.yaml
| RUN dnf install -y git jq && \ | ||
| curl -fsSL https://get.helm.sh/helm-v3.17.3-linux-amd64.tar.gz | tar xz -C /tmp && \ | ||
| mv /tmp/linux-amd64/helm /usr/local/bin/helm |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify Helm release artifact has published checksums for the pinned version.
ver="v3.17.3"
curl -fsSL "https://get.helm.sh/helm-${ver}-linux-amd64.tar.gz.sha256" | sed -n '1,3p'
curl -fsSL "https://get.helm.sh/helm-${ver}-linux-amd64.tar.gz.sha256sum" | sed -n '1,3p'Repository: openshift/release
Length of output: 224
Add checksum verification for downloaded Helm artifact
The Docker build installs Helm fetched from the network (curl ... | tar) without checksum/signature verification, leaving a supply-chain integrity gap. Helm v3.17.3 publishes SHA256 checksums, so downloading the tarball and verifying it with sha256sum before extracting/install is straightforward.
Suggested hardening diff
- RUN dnf install -y git jq && \
- curl -fsSL https://get.helm.sh/helm-v3.17.3-linux-amd64.tar.gz | tar xz -C /tmp && \
- mv /tmp/linux-amd64/helm /usr/local/bin/helm
+ ARG HELM_VERSION=v3.17.3
+ ARG HELM_SHA256
+ RUN dnf install -y git jq && \
+ curl -fsSLo /tmp/helm.tar.gz "https://get.helm.sh/helm-${HELM_VERSION}-linux-amd64.tar.gz" && \
+ echo "${HELM_SHA256} /tmp/helm.tar.gz" | sha256sum -c - && \
+ tar xzf /tmp/helm.tar.gz -C /tmp && \
+ install -m 0755 /tmp/linux-amd64/helm /usr/local/bin/helm🤖 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/config/osac-project/osac-installer/osac-project-osac-installer-main.yaml`
around lines 30 - 32, The Dockerfile RUN that fetches and extracts Helm (the RUN
line using curl to download https://get.helm.sh/helm-v3.17.3-linux-amd64.tar.gz
and then tar/mv) must verify the artifact before extraction: download both the
tarball and its published SHA256 file (or the .tar.gz.sha256sum), compute the
local sha256 (sha256sum) and compare to the published value, aborting the build
if they differ; only after successful verification proceed to tar -xzf and mv
/tmp/linux-amd64/helm /usr/local/bin/helm. Ensure the verification is done in
the same RUN step so intermediate layers don’t keep an unverified artifact and
clean up downloaded files afterwards.
|
/pj-rehearse pull-ci-osac-project-osac-test-infra-main-e2e-vmaas pull-ci-osac-project-fulfillment-service-main-e2e-vmaas pull-ci-osac-project-osac-installer-main-e2e-vmaas pull-ci-osac-project-osac-installer-main-images pull-ci-osac-project-osac-operator-main-e2e-vmaas pull-ci-osac-project-osac-aap-main-e2e-vmaas periodic-ci-osac-project-osac-test-infra-main-e2e-vmaas-full-setup-kustomize periodic-ci-osac-project-osac-test-infra-main-e2e-vmaas-full-setup-helm periodic-ci-osac-project-osac-test-infra-main-e2e-vmaas-periodic |
|
@omer-vishlitzky: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-osac-project-osac-test-infra-main-e2e-vmaas-full-setup-helm pull-ci-osac-project-osac-installer-main-e2e-vmaas |
|
@omer-vishlitzky: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
- Add helm binary to osac-installer dockerfile_literal - Add DEPLOY_MODE and VALUES_FILE env vars to osac-project-installer step (default: kustomize, values/vmaas-ci.yaml) - Rename e2e-vmaas-full-install to e2e-vmaas-full-setup-kustomize - Add e2e-vmaas-full-setup-helm nightly periodic (DEPLOY_MODE=helm) - Update notify label to "Full Setup vmaas (kustomize)" - Regenerate Prow job configs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
omer-vishlitzky
force-pushed
the
osac-866-consolidate-vmaas-periodics
branch
from
2bfb9e4 to
82900be
Compare
|
/pj-rehearse pull-ci-osac-project-osac-test-infra-main-e2e-vmaas pull-ci-osac-project-fulfillment-service-main-e2e-vmaas pull-ci-osac-project-osac-installer-main-e2e-vmaas pull-ci-osac-project-osac-installer-main-images pull-ci-osac-project-osac-operator-main-e2e-vmaas pull-ci-osac-project-osac-aap-main-e2e-vmaas periodic-ci-osac-project-osac-test-infra-main-e2e-vmaas-full-setup-kustomize periodic-ci-osac-project-osac-test-infra-main-e2e-vmaas-full-setup-helm periodic-ci-osac-project-osac-test-infra-main-e2e-vmaas-periodic |
|
@omer-vishlitzky: 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 pull-ci-osac-project-osac-test-infra-main-e2e-vmaas pull-ci-osac-project-osac-operator-main-e2e-vmaas |
|
@omer-vishlitzky: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@omer-vishlitzky: 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. |
2 participants
Summary
Replaces 8 separate periodic vmaas CI jobs — each provisioning a full OCP cluster just to run a single test file — with 3 consolidated periodics:
e2e-vmaas-periodic(every 12h): boots from cluster-tool snapshot, runs all vmaas tests in one job. Same workflow as the presubmite2e-vmaas.e2e-vmaas-full-setup-kustomize(nightly): full OCP installation via assisted-installer, deploys OSAC via kustomize, runs all vmaas tests.e2e-vmaas-full-setup-helm(nightly): full OCP installation via assisted-installer, deploys OSAC via helm, runs all vmaas tests.All periodics send Slack notifications via
osac-project-notifywith custom labels.Changes
ci-operator config (osac-test-infra):
e2e-metal-vmaas-*periodic entriesci-operator config (osac-installer):
helmbinary toosac-installerdockerfile_literalStep registry:
osac-project-baremetal-test: replace single-fileTESTenv var withTEST_SUITE(default:vmaas) — runs fullpytest tests/vmaas/with JUnit XMLosac-project-installer: addDEPLOY_MODE(default:kustomize) andVALUES_FILE(default:values/vmaas-ci.yaml) env vars, pass them to setup.shosac-project-cluster-tool-vmaasworkflow: addosac-project-notifyto post stepsosac-project-ofcir-baremetalworkflow: change CLUSTERTYPE toassisted_large_el9, addosac-project-notifyto post stepsosac-project-notify: addNOTIFY_LABELenv var for custom Slack message headersNet: 8 jobs → 3 jobs, -596 lines in the first commit
Jira
Test plan
e2e-vmaas-periodic— boots from snapshot, runs all vmaas testse2e-vmaas-full-setup-kustomize— full OCP install + kustomize deploy + all vmaas testse2e-vmaas-full-setup-helm— full OCP install + helm deploy + all vmaas testse2e-vmaasis unaffectedSummary by CodeRabbit
This PR updates OSAC's OpenShift CI configuration to consolidate multiple vmaas periodic jobs and improve deployment/test workflows and notifications across the OSAC CI repo(s). It replaces the previous set of e2e-metal-vmaas periodic jobs with three consolidated periodics and adds Helm deployment support, improved test-suite handling, and customizable Slack notification labels.
What changed in practical terms
Key implementation details
Test runner and artifacts
Deployment mode / Helm support
Notification and workflow changes
Files/areas touched (high level)
Impact
Jira: OSAC-866