ROSAENG-1340: Consolidate OCM FVT configs into shared step registry ref - #79840
Conversation
|
@dustman9000: This pull request references ROSAENG-1340 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 story 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. |
|
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:
WalkthroughExtract a reusable ci-operator step ChangesOCM FVT Test Execution Refactor
Sequence Diagram(s)sequenceDiagram
participant Prow
participant ci_operator as ci-operator
participant ocm_fvt_step as ocm-fvt-test
participant Podman
Prow->>ci_operator: trigger periodic job
ci_operator->>ocm_fvt_step: run step with env (OCM_FVT_JOB_NAME, OCM_FVT_OCM_ENV, OCM_FVT_EXTRA_ENVS, OCM_FVT_GCP_CREDS)
ocm_fvt_step->>Podman: prepare env-file, mount creds, execute containerized ocmtest
Podman->>ocm_fvt_step: return test result/status
ocm_fvt_step->>ci_operator: emit job status and Jira/reporting
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__ocm-fvt-rosa-classic-integration.yaml`:
- Around line 30-34: The job is missing the OCM_FVT_OCM_ENV override so the
shared step falls back to staging; add an explicit OCM_FVT_OCM_ENV entry under
the same steps.env map (next to OCM_FVT_JOB_NAME) and set it to the intended
environment string (e.g., "production" or the correct OCM env for this
integration) so the ocm-fvt-test ref runs against the right environment.
In `@ci-operator/step-registry/ocm/fvt/test/ocm-fvt-test-commands.sh`:
- Around line 48-57: The podman_args array entries containing comma-separated
mount options (e.g., the -v ...:ro,z entries added to podman_args and the
podman_args+=(...) block under the OCM_FVT_GCP_CREDS check) trigger shellcheck
SC2054 because the comma is parsed as a separator; fix by quoting each entire -v
mount argument so the full string (for example the -v
/usr/local/cs-qe-credentials:/credentials:ro,z and the -v
/usr/local/cs-qe-credentials/osd-ccs-admin.json:/home/ci-user/.gcp/osd-ccs-admin.json:ro,z
entries) is a single array element in podman_args and podman_args+=(...).
🪄 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: a06639aa-379c-4093-8b5d-9d36efdec057
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (9)
ci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__ocm-fvt-osd-gcp-staging.yamlci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__ocm-fvt-rosa-classic-integration.yamlci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__ocm-fvt-rosa-classic-staging.yamlci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__ocm-fvt-rosa-hcp-integration.yamlci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__ocm-fvt-rosa-hcp-staging.yamlci-operator/step-registry/ocm/fvt/test/OWNERSci-operator/step-registry/ocm/fvt/test/ocm-fvt-test-commands.shci-operator/step-registry/ocm/fvt/test/ocm-fvt-test-ref.metadata.jsonci-operator/step-registry/ocm/fvt/test/ocm-fvt-test-ref.yaml
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci-operator/step-registry/ocm/fvt/test/ocm-fvt-test-commands.sh (1)
42-46: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winDisable inherited xtrace before sourcing credentials.
If tracing is already enabled by the caller, this block will log the assignments from
ocm-tokensandjira-cred. Wrap the sensitive section inset +xand restore the previous state afterward.As per coding guidelines, "In step registry scripts handling sensitive credentials, temporarily disable command tracing with `set +x` to prevent argument expansion from exposing values in logs".🔒 Proposed fix
+restore_xtrace=false +if [[ $- == *x* ]]; then + restore_xtrace=true + set +x +fi + env -i bash --norc --noprofile -c ' source /usr/local/cs-qe-credentials/ocm-tokens source /usr/local/cs-qe-credentials/jira-cred env | grep -v "^_=" ' >> "${podman_env_file}" + +if [[ "${restore_xtrace}" == "true" ]]; then + set -x +fi🤖 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/ocm/fvt/test/ocm-fvt-test-commands.sh` around lines 42 - 46, Temporarily disable shell xtrace around the sensitive sourcing block so credential values are not printed: before sourcing /usr/local/cs-qe-credentials/ocm-tokens and jira-cred, detect and save current xtrace state, run set +x, perform the two source commands and the env | grep -v "^_=" redirection to "${podman_env_file}", then restore the original xtrace state (re-enable with set -x only if it was enabled before). Ensure the change wraps the exact block that sources those files and writes to podman_env_file.
🤖 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.
Outside diff comments:
In `@ci-operator/step-registry/ocm/fvt/test/ocm-fvt-test-commands.sh`:
- Around line 42-46: Temporarily disable shell xtrace around the sensitive
sourcing block so credential values are not printed: before sourcing
/usr/local/cs-qe-credentials/ocm-tokens and jira-cred, detect and save current
xtrace state, run set +x, perform the two source commands and the env | grep -v
"^_=" redirection to "${podman_env_file}", then restore the original xtrace
state (re-enable with set -x only if it was enabled before). Ensure the change
wraps the exact block that sources those files and writes to podman_env_file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8d08e463-5604-4ead-a1df-dee4d1464a92
📒 Files selected for processing (2)
ci-operator/step-registry/ocm/fvt/OWNERSci-operator/step-registry/ocm/fvt/test/ocm-fvt-test-commands.sh
✅ Files skipped from review due to trivial changes (1)
- ci-operator/step-registry/ocm/fvt/OWNERS
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-rosa-hcp-staging-ocm-fvt-periodic-cs-rosa-hcp-ad-staging-main periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osd-gcp-staging-ocm-fvt-periodic-cs-osd-ccs-gcp-ad-staging-main periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-rosa-hcp-integration-ocm-fvt-periodic-cs-rosa-hcp-backup-restore-integration-main |
|
@dustman9000: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
Actionable comments posted: 0 |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-rosa-hcp-staging-ocm-fvt-periodic-cs-rosa-hcp-ad-staging-main periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-osd-gcp-staging-ocm-fvt-periodic-cs-osd-ccs-gcp-ad-staging-main periodic-ci-openshift-online-rosa-e2e-main-ocm-fvt-rosa-hcp-integration-ocm-fvt-periodic-cs-rosa-hcp-backup-restore-integration-main |
|
@dustman9000: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
4b57272 to
e5b6b15
Compare
e5b6b15 to
8a51dcf
Compare
|
/lgtm |
Create rosa-e2e-ocm-fvt ref at ci-operator/step-registry/rosa/e2e/ocm-fvt/ that encapsulates the common nested-podman + ocmtest pattern shared across all 26 OCM FVT periodic test entries. Each test entry is reduced from ~40 lines of inline bash boilerplate to ~10 lines referencing the shared ref with parameterized env vars: - OCM_FVT_JOB_NAME (required): ocmtest --job argument - OCM_FVT_OCM_ENV (optional): OCM environment override - OCM_FVT_EXTRA_ENVS (optional): additional KEY=VALUE env vars - OCM_FVT_GCP_CREDS (optional): mount GCP credentials for OSD GCP tests Also fixes env var inconsistency where one test used AWS_SHARED_VPC_CREDENTIALS_FILE instead of the standard SHARED_VPC_AWS_SHARED_CREDENTIALS_FILE. Jira: https://redhat.atlassian.net/browse/ROSAENG-1340
8a51dcf to
b91380a
Compare
|
[REHEARSALNOTIFIER]
A total of 26 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dustman9000, jfrazierRedHat 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 ack |
|
@dustman9000: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@dustman9000: 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. |
…ef (openshift#79840) Create rosa-e2e-ocm-fvt ref at ci-operator/step-registry/rosa/e2e/ocm-fvt/ that encapsulates the common nested-podman + ocmtest pattern shared across all 26 OCM FVT periodic test entries. Each test entry is reduced from ~40 lines of inline bash boilerplate to ~10 lines referencing the shared ref with parameterized env vars: - OCM_FVT_JOB_NAME (required): ocmtest --job argument - OCM_FVT_OCM_ENV (optional): OCM environment override - OCM_FVT_EXTRA_ENVS (optional): additional KEY=VALUE env vars - OCM_FVT_GCP_CREDS (optional): mount GCP credentials for OSD GCP tests Also fixes env var inconsistency where one test used AWS_SHARED_VPC_CREDENTIALS_FILE instead of the standard SHARED_VPC_AWS_SHARED_CREDENTIALS_FILE. Jira: https://redhat.atlassian.net/browse/ROSAENG-1340
…ef (openshift#79840) Create rosa-e2e-ocm-fvt ref at ci-operator/step-registry/rosa/e2e/ocm-fvt/ that encapsulates the common nested-podman + ocmtest pattern shared across all 26 OCM FVT periodic test entries. Each test entry is reduced from ~40 lines of inline bash boilerplate to ~10 lines referencing the shared ref with parameterized env vars: - OCM_FVT_JOB_NAME (required): ocmtest --job argument - OCM_FVT_OCM_ENV (optional): OCM environment override - OCM_FVT_EXTRA_ENVS (optional): additional KEY=VALUE env vars - OCM_FVT_GCP_CREDS (optional): mount GCP credentials for OSD GCP tests Also fixes env var inconsistency where one test used AWS_SHARED_VPC_CREDENTIALS_FILE instead of the standard SHARED_VPC_AWS_SHARED_CREDENTIALS_FILE. Jira: https://redhat.atlassian.net/browse/ROSAENG-1340
…ef (openshift#79840) Create rosa-e2e-ocm-fvt ref at ci-operator/step-registry/rosa/e2e/ocm-fvt/ that encapsulates the common nested-podman + ocmtest pattern shared across all 26 OCM FVT periodic test entries. Each test entry is reduced from ~40 lines of inline bash boilerplate to ~10 lines referencing the shared ref with parameterized env vars: - OCM_FVT_JOB_NAME (required): ocmtest --job argument - OCM_FVT_OCM_ENV (optional): OCM environment override - OCM_FVT_EXTRA_ENVS (optional): additional KEY=VALUE env vars - OCM_FVT_GCP_CREDS (optional): mount GCP credentials for OSD GCP tests Also fixes env var inconsistency where one test used AWS_SHARED_VPC_CREDENTIALS_FILE instead of the standard SHARED_VPC_AWS_SHARED_CREDENTIALS_FILE. Jira: https://redhat.atlassian.net/browse/ROSAENG-1340
…ef (openshift#79840) Create rosa-e2e-ocm-fvt ref at ci-operator/step-registry/rosa/e2e/ocm-fvt/ that encapsulates the common nested-podman + ocmtest pattern shared across all 26 OCM FVT periodic test entries. Each test entry is reduced from ~40 lines of inline bash boilerplate to ~10 lines referencing the shared ref with parameterized env vars: - OCM_FVT_JOB_NAME (required): ocmtest --job argument - OCM_FVT_OCM_ENV (optional): OCM environment override - OCM_FVT_EXTRA_ENVS (optional): additional KEY=VALUE env vars - OCM_FVT_GCP_CREDS (optional): mount GCP credentials for OSD GCP tests Also fixes env var inconsistency where one test used AWS_SHARED_VPC_CREDENTIALS_FILE instead of the standard SHARED_VPC_AWS_SHARED_CREDENTIALS_FILE. Jira: https://redhat.atlassian.net/browse/ROSAENG-1340
…ef (openshift#79840) Create rosa-e2e-ocm-fvt ref at ci-operator/step-registry/rosa/e2e/ocm-fvt/ that encapsulates the common nested-podman + ocmtest pattern shared across all 26 OCM FVT periodic test entries. Each test entry is reduced from ~40 lines of inline bash boilerplate to ~10 lines referencing the shared ref with parameterized env vars: - OCM_FVT_JOB_NAME (required): ocmtest --job argument - OCM_FVT_OCM_ENV (optional): OCM environment override - OCM_FVT_EXTRA_ENVS (optional): additional KEY=VALUE env vars - OCM_FVT_GCP_CREDS (optional): mount GCP credentials for OSD GCP tests Also fixes env var inconsistency where one test used AWS_SHARED_VPC_CREDENTIALS_FILE instead of the standard SHARED_VPC_AWS_SHARED_CREDENTIALS_FILE. Jira: https://redhat.atlassian.net/browse/ROSAENG-1340
…ef (openshift#79840) Create rosa-e2e-ocm-fvt ref at ci-operator/step-registry/rosa/e2e/ocm-fvt/ that encapsulates the common nested-podman + ocmtest pattern shared across all 26 OCM FVT periodic test entries. Each test entry is reduced from ~40 lines of inline bash boilerplate to ~10 lines referencing the shared ref with parameterized env vars: - OCM_FVT_JOB_NAME (required): ocmtest --job argument - OCM_FVT_OCM_ENV (optional): OCM environment override - OCM_FVT_EXTRA_ENVS (optional): additional KEY=VALUE env vars - OCM_FVT_GCP_CREDS (optional): mount GCP credentials for OSD GCP tests Also fixes env var inconsistency where one test used AWS_SHARED_VPC_CREDENTIALS_FILE instead of the standard SHARED_VPC_AWS_SHARED_CREDENTIALS_FILE. Jira: https://redhat.atlassian.net/browse/ROSAENG-1340
…ef (openshift#79840) Create rosa-e2e-ocm-fvt ref at ci-operator/step-registry/rosa/e2e/ocm-fvt/ that encapsulates the common nested-podman + ocmtest pattern shared across all 26 OCM FVT periodic test entries. Each test entry is reduced from ~40 lines of inline bash boilerplate to ~10 lines referencing the shared ref with parameterized env vars: - OCM_FVT_JOB_NAME (required): ocmtest --job argument - OCM_FVT_OCM_ENV (optional): OCM environment override - OCM_FVT_EXTRA_ENVS (optional): additional KEY=VALUE env vars - OCM_FVT_GCP_CREDS (optional): mount GCP credentials for OSD GCP tests Also fixes env var inconsistency where one test used AWS_SHARED_VPC_CREDENTIALS_FILE instead of the standard SHARED_VPC_AWS_SHARED_CREDENTIALS_FILE. Jira: https://redhat.atlassian.net/browse/ROSAENG-1340
Summary
rosa-e2e-ocm-fvtstep registry ref atci-operator/step-registry/rosa/e2e/ocm-fvt/that encapsulates the common nested-podman + ocmtest patternAWS_SHARED_VPC_CREDENTIALS_FILEinstead of the standardSHARED_VPC_AWS_SHARED_CREDENTIALS_FILEParameterized via env vars
OCM_FVT_JOB_NAMEcs-rosa-hcp-ad-staging-main)OCM_FVT_SERVICEcms--serviceflag (cms,osdfm)OCM_FVT_REPORT_JIRAtrue--reportJiraTicketand Jira credential sourcingOCM_FVT_OCM_ENV""integrationfor int tests)OCM_FVT_EXTRA_ENVS""KEY=VALUEpairs for additional env varsOCM_FVT_GCP_CREDSfalseExample: CMS test (default)
Example: OSDFM test
Jira: https://redhat.atlassian.net/browse/ROSAENG-1340
Test plan
make jobsandmake registry-metadatacomplete successfully