[WIP] extend the baremetal-ove-compact-konflux job with CNV validation steps#79895
Open
orenc1 wants to merge 1 commit into
Open
[WIP] extend the baremetal-ove-compact-konflux job with CNV validation steps#79895orenc1 wants to merge 1 commit into
baremetal-ove-compact-konflux job with CNV validation steps#79895orenc1 wants to merge 1 commit into
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
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 (19)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (11)
WalkthroughAdds a disconnected OVE bare-metal CI flow: creates a CatalogSource, installs Local Storage Operator, configures local storage with loopback and block LocalVolumeSets, deploys ODF StorageCluster, integrates steps into the Konflux workflow, and pins a nightly job SNAPSHOT to a specific OVE ISO tag. ChangesDisconnected OVE LSO and ODF Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 3 warnings)
✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: orenc1 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 |
Contributor
|
@orenc1, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
![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: 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/agent-qe/baremetal/install/ove/disconnected/configure-lso/agent-qe-baremetal-install-ove-disconnected-configure-lso-commands.sh`:
- Around line 40-44: The early oc wait for Updating can spuriously fail; make
the first wait best-effort so it won't abort the step if the pool never enters
Updating. Modify the oc wait mcp/master --for=condition=Updating --timeout=5m
invocation (the first of the two waits) so its non-zero exit is ignored (e.g.,
run it in a subshell or append a no-op like "|| true" or otherwise catch and
ignore errors), but keep the oc wait mcp/master --for=condition=Updated
--timeout=1h (the second wait) as the real, fatal success gate.
In
`@ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-commands.sh`:
- Around line 23-58: The check_marketplace invocation is being masked by "||
exit 0" which turns real failures from check_marketplace into successes; change
the call site to invoke check_marketplace directly (remove the "|| exit 0"
fallback) so a non-zero return from the check_marketplace function propagates
and fails the job (rely on set -e or standard error handling), leaving the
existing check_olm_capability || exit 0 behavior unchanged.
In
`@ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-odf/agent-qe-baremetal-install-ove-disconnected-install-odf-commands.sh`:
- Around line 183-184: The oc wait against StorageCluster/ocs-storagecluster is
using a non-existent condition=Available; update the wait to check status.phase
== "Ready" instead (use oc wait with --for=jsonpath to target .status.phase) and
adjust the echo message accordingly so the script waits for the StorageCluster
to report status.phase: Ready (locate the oc wait call for
StorageCluster/ocs-storagecluster in the script).
🪄 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: 3b039384-5160-4480-978c-53be058ef0a4
📒 Files selected for processing (19)
ci-operator/config/openshift-eng/agent-qe-infra/openshift-eng-agent-qe-infra-release-4.21__amd64-nightly.yamlci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/configure-lso/OWNERSci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/configure-lso/agent-qe-baremetal-install-ove-disconnected-configure-lso-commands.shci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/configure-lso/agent-qe-baremetal-install-ove-disconnected-configure-lso-ref.metadata.jsonci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/configure-lso/agent-qe-baremetal-install-ove-disconnected-configure-lso-ref.yamlci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/OWNERSci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-commands.shci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-ref.metadata.jsonci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-ref.yamlci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-lso/OWNERSci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-lso/agent-qe-baremetal-install-ove-disconnected-install-lso-commands.shci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-lso/agent-qe-baremetal-install-ove-disconnected-install-lso-ref.metadata.jsonci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-lso/agent-qe-baremetal-install-ove-disconnected-install-lso-ref.yamlci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-odf/OWNERSci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-odf/agent-qe-baremetal-install-ove-disconnected-install-odf-commands.shci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-odf/agent-qe-baremetal-install-ove-disconnected-install-odf-ref.metadata.jsonci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-odf/agent-qe-baremetal-install-ove-disconnected-install-odf-ref.yamlci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/konflux/agent-qe-baremetal-install-ove-disconnected-konflux-chain.yamlci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/konflux/agent-qe-baremetal-install-ove-disconnected-konflux-workflow.yaml
orenc1
force-pushed
the
ove_installer_connected_cnv_validation
branch
from
af46cf0 to
d10c542
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-commands.sh (1)
58-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPropagate
check_marketplacefailures instead of exiting successfully.Line 58 still converts a real marketplace setup failure into success, which hides the root cause and defers breakage to later steps.
Suggested fix
-check_marketplace || exit 0 +check_marketplace🤖 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/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-commands.sh` at line 58, The line currently swallows failures by using "check_marketplace || exit 0"; update the call to let errors propagate (remove the "|| exit 0" or replace it with a non-zero exit) so that a failing check_marketplace returns failure to the pipeline; locate the invocation of check_marketplace (the "check_marketplace || exit 0" command) and either call it directly or use "check_marketplace || exit 1" so the script fails fast on marketplace setup errors.
🤖 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/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-commands.sh`:
- Line 45: The step currently calls run_command "oc version -o yaml" which
prints full cluster/version YAML (may include cluster URL/token); replace that
invocation so it only reveals safe client version info (for example call
run_command "oc version --client" or an equivalent that does not output
cluster/kubeconfig details). Update the call site where run_command is invoked
with the "oc version -o yaml" argument and ensure no other step in this script
echoes full cluster/version YAML, tokens, or kubeconfig contents.
---
Duplicate comments:
In
`@ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-commands.sh`:
- Line 58: The line currently swallows failures by using "check_marketplace ||
exit 0"; update the call to let errors propagate (remove the "|| exit 0" or
replace it with a non-zero exit) so that a failing check_marketplace returns
failure to the pipeline; locate the invocation of check_marketplace (the
"check_marketplace || exit 0" command) and either call it directly or use
"check_marketplace || exit 1" so the script fails fast on marketplace setup
errors.
🪄 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: e02087fe-7f4b-4a52-9197-8e5ca07e339e
📒 Files selected for processing (19)
ci-operator/config/openshift-eng/agent-qe-infra/openshift-eng-agent-qe-infra-release-4.21__amd64-nightly.yamlci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/configure-lso/OWNERSci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/configure-lso/agent-qe-baremetal-install-ove-disconnected-configure-lso-commands.shci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/configure-lso/agent-qe-baremetal-install-ove-disconnected-configure-lso-ref.metadata.jsonci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/configure-lso/agent-qe-baremetal-install-ove-disconnected-configure-lso-ref.yamlci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/OWNERSci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-commands.shci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-ref.metadata.jsonci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-ref.yamlci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-lso/OWNERSci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-lso/agent-qe-baremetal-install-ove-disconnected-install-lso-commands.shci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-lso/agent-qe-baremetal-install-ove-disconnected-install-lso-ref.metadata.jsonci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-lso/agent-qe-baremetal-install-ove-disconnected-install-lso-ref.yamlci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-odf/OWNERSci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-odf/agent-qe-baremetal-install-ove-disconnected-install-odf-commands.shci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-odf/agent-qe-baremetal-install-ove-disconnected-install-odf-ref.metadata.jsonci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-odf/agent-qe-baremetal-install-ove-disconnected-install-odf-ref.yamlci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/konflux/agent-qe-baremetal-install-ove-disconnected-konflux-chain.yamlci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/konflux/agent-qe-baremetal-install-ove-disconnected-konflux-workflow.yaml
✅ Files skipped from review due to trivial changes (7)
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-lso/OWNERS
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-lso/agent-qe-baremetal-install-ove-disconnected-install-lso-ref.metadata.json
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/konflux/agent-qe-baremetal-install-ove-disconnected-konflux-workflow.yaml
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-ref.metadata.json
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/configure-lso/agent-qe-baremetal-install-ove-disconnected-configure-lso-ref.metadata.json
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-odf/OWNERS
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/OWNERS
🚧 Files skipped from review as they are similar to previous changes (8)
- ci-operator/config/openshift-eng/agent-qe-infra/openshift-eng-agent-qe-infra-release-4.21__amd64-nightly.yaml
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-odf/agent-qe-baremetal-install-ove-disconnected-install-odf-ref.metadata.json
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/konflux/agent-qe-baremetal-install-ove-disconnected-konflux-chain.yaml
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/configure-lso/agent-qe-baremetal-install-ove-disconnected-configure-lso-ref.yaml
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/configure-lso/OWNERS
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/configure-lso/agent-qe-baremetal-install-ove-disconnected-configure-lso-commands.sh
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-odf/agent-qe-baremetal-install-ove-disconnected-install-odf-commands.sh
- ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/install-lso/agent-qe-baremetal-install-ove-disconnected-install-lso-commands.sh
| } | ||
|
|
||
| run_command "oc whoami" | ||
| run_command "oc version -o yaml" |
Contributor
There was a problem hiding this comment.
Avoid logging full cluster/version YAML in CI output.
Line 45 prints full oc version -o yaml, which can include cluster URL/details that should not be emitted in step-registry logs.
Suggested fix
-run_command "oc version -o yaml"
+run_command "oc version --client"As per coding guidelines: in ci-operator/step-registry/**/*-commands.sh, never echo or print passwords, tokens, API keys, cluster URLs, or kubeconfig contents.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| run_command "oc version -o yaml" | |
| run_command "oc version --client" |
🤖 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/agent-qe/baremetal/install/ove/disconnected/create-catalogsource/agent-qe-baremetal-install-ove-disconnected-create-catalogsource-commands.sh`
at line 45, The step currently calls run_command "oc version -o yaml" which
prints full cluster/version YAML (may include cluster URL/token); replace that
invocation so it only reveals safe client version info (for example call
run_command "oc version --client" or an equivalent that does not output
cluster/kubeconfig details). Update the call site where run_command is invoked
with the "oc version -o yaml" argument and ensure no other step in this script
echoes full cluster/version YAML, tokens, or kubeconfig contents.
Add a connected baremetal-ove-compact-konflux periodic job that installs an OVE cluster using Konflux-built ISOs on a connected (non-disconnected) baremetal lab, then provisions LSO and ODF storage. Four new step-registry steps are introduced and appended to the existing agent-qe-baremetal-install-ove-disconnected-konflux chain: - create-catalogsource: disables default catalog sources and creates a version-pinned CatalogSource (redhat-operators-full) from registry.redhat.io/redhat/redhat-operator-index:v<major>.<minor>, auto-detecting the OCP version from the cluster. - install-lso: subscribes and installs local-storage-operator from the new catalog source. - configure-lso: labels nodes, creates a MachineConfig for Ceph MON loop devices, and provisions localblock-mon (Filesystem) and localblock (Block) LocalVolumeSets with the required PVs. - install-odf: subscribes and installs odf-operator, then creates an ocs-storagecluster backed by the LSO-provisioned local storage. The new job overrides DISCONNECTED=false so the firewall step is skipped and the cluster retains internet access for pulling the operator index directly from registry.redhat.io. Signed-off-by: Oren Cohen <ocohen@redhat.com>
orenc1
force-pushed
the
ove_installer_connected_cnv_validation
branch
from
d10c542 to
354035b
Compare
Contributor
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
Contributor
|
@orenc1: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
Author
|
/pj-rehearse periodic-ci-openshift-eng-agent-qe-infra-release-4.21-amd64-nightly-baremetal-ove-compact-konflux |
Contributor
|
@orenc1: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
Author
|
/pj-rehearse periodic-ci-openshift-eng-agent-qe-infra-release-4.21-amd64-nightly-baremetal-ove-compact-konflux |
Contributor
|
@orenc1: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
|
@orenc1: The following test 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. |
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
1 participant
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.
Add a connected baremetal-ove-compact-konflux periodic job that installs an OVE cluster using Konflux-built ISOs on a connected (non-disconnected) baremetal lab, then provisions LSO and ODF storage.
Four new step-registry steps are introduced and appended to the existing agent-qe-baremetal-install-ove-disconnected-konflux chain:
The new job overrides
DISCONNECTED=falseso the firewall step is skipped and the cluster retains internet access for pulling the operator index directly from registry.redhat.io.Summary by CodeRabbit
This PR extends the OpenShift CI infrastructure with a new connected periodic test job for OVE (OpenShift Virtualization Engine) cluster deployments that includes storage provisioning capabilities.
Key Changes
The PR adds four new step-registry steps to the
agent-qe-baremetal-install-ove-disconnectedchain:create-catalogsource: Disables default OLM catalog sources and provisions a version-pinned CatalogSource pointing to the Red Hat operator index. It auto-detects the cluster's OCP version and handles OLMv1
clustercatalogresources for newer cluster versions.install-lso: Installs the Local Storage Operator (LSO) from the newly created CatalogSource with polling to ensure the operator CSV reaches a "Succeeded" state.
configure-lso: Configures local storage by labeling nodes, applying a MachineConfig to create loop devices for Ceph MON storage, and provisioning two LocalVolumeSets—one for MON storage (filesystem) and one for block storage (XFS)—with comprehensive polling and diagnostics to verify PV creation.
install-odf: Installs the OpenShift Data Foundation (ODF) operator and creates an OCS StorageCluster backed by the LSO-provisioned local volumes, with error handling to gather ODF-related diagnostics on failure.
These steps are integrated into the existing chain workflow, and the job configuration overrides
DISCONNECTED=falseto allow the cluster to retain internet connectivity for pulling operator images directly from registry.redhat.io—contrasting with the disconnected variant.Additionally, the SNAPSHOT environment variable is updated to reference a specific Konflux-built ISO, and OWNERS files are established for the new step directories with approvers and reviewers designated.
Infrastructure Impact
This addition enables a new CI validation pathway for OVE clusters with integrated storage operator deployment on connected baremetal infrastructure, extending the existing agent-based installer test coverage to include LSO and ODF provisioning workflows.