INTEROP-9417: Add acm-tests-clc-smoke step for reduced CLC scope - #83359
INTEROP-9417: Add acm-tests-clc-smoke step for reduced CLC scope#83359amp-rh wants to merge 4 commits into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@amp-rh: This pull request references INTEROP-9417 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. |
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
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: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe PR adds the ACM CLC smoke test step, its execution script, ownership metadata, and documentation. It replaces ChangesCLC smoke test integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to The PR adds a scoped smoke step with a shorter timeout and explicit failure propagation; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant AWSInterop
participant CLCSmokeStep
participant SmokeCommands
participant ArtifactStorage
AWSInterop->>CLCSmokeStep: invoke acm-tests-clc-smoke
CLCSmokeStep->>SmokeCommands: run configured command
SmokeCommands->>SmokeCommands: prepare credentials and test environment
SmokeCommands->>SmokeCommands: run CLC interop commands
SmokeCommands->>ArtifactStorage: copy reports
SmokeCommands-->>CLCSmokeStep: return interop status
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (14 passed)
✨ 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: amp-rh 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 |
The existing acm-tests-clc-create step already creates only 1 AWS managed cluster (~50 min runtime) but carries a 28800s (8h) timeout and suppresses failures with || :. This new step provides: - Right-sized timeout: 5400s (90 min) vs 28800s - Strict failure propagation: no || : so downstream steps fail fast if cluster creation does not succeed No CUSTOMER_TAGS or CLOUD_PROVIDERS changes needed; the existing test image already scopes to single-cluster creation via TEST_STAGE=OCPInterop-create internally. Update OPP interop configs (ocp4.22, ocp5.0) to use the new step. The acm-tests-clc-destroy post step remains unchanged.
c08f512 to
4886163
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/step-registry/acm/tests/clc-smoke/acm-tests-clc-smoke-commands.sh`:
- Line 2: Update the tracing setup around the test invocation in the shell
script so expanded CYPRESS_BASE_URL and CYPRESS_HUB_API_URL assignments are
never emitted by parent-shell tracing; disable tracing in the current shell
through the commands covering lines 47-51, and add a brief comment documenting
that tracing is disabled to protect cluster endpoints.
- Around line 47-55: Update the invocation of execute_clc_interop_commands.sh to
run within an if block that captures its exit status without terminating the
step immediately. Ensure reports are copied to ARTIFACT_DIR and
CYPRESS_OPTIONS_HUB_PASSWORD is unset before returning the captured failure
status.
🪄 Autofix
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: Pro Plus
Run ID: 79985e4f-a7ec-4729-8f6d-ed1b0a7ebe5d
📒 Files selected for processing (7)
ci-operator/config/stolostron/policy-collection/stolostron-policy-collection-main__ocp4.22.yamlci-operator/config/stolostron/policy-collection/stolostron-policy-collection-main__ocp5.0.yamlci-operator/step-registry/acm/tests/clc-smoke/OWNERSci-operator/step-registry/acm/tests/clc-smoke/README.mdci-operator/step-registry/acm/tests/clc-smoke/acm-tests-clc-smoke-commands.shci-operator/step-registry/acm/tests/clc-smoke/acm-tests-clc-smoke-ref.metadata.jsonci-operator/step-registry/acm/tests/clc-smoke/acm-tests-clc-smoke-ref.yaml
|
/test all |
- Keep tracing disabled through cluster endpoint assignments to prevent logging CYPRESS_BASE_URL and CYPRESS_HUB_API_URL in CI output - Capture test exit status so reports are always copied to ARTIFACT_DIR before propagating the failure - Add trailing newline to metadata.json
- Replace silent [ -n ] && [ -n ] with explicit error message on credential extraction failure for faster CI triage - Use jq --rawfile for AWS secret key to keep it off the process command line (awsAccKeyID kept as --arg since semi-public)
|
/pj-rehearse periodic-ci-stolostron-policy-collection-main-ocp4.22-interop-opp-aws |
|
@amp-rh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/test all |
- Revert trailing newline in metadata.json (auto-generated file must match generator output exactly) - Declare SKIP_OCP_DEPLOY in YAML env section for discoverability - Regenerate metadata via make registry-metadata
|
/test all |
|
/pj-rehearse periodic-ci-stolostron-policy-collection-main-ocp4.22-interop-opp-aws |
|
@amp-rh: 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: |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/pj-rehearse ack |
|
@amp-rh: 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
|
@amp-rh: 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. |
|
@amp-rh: This pull request references INTEROP-9417 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.1.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. |
2 participants
Summary
Adds
acm-tests-clc-smoke, a right-sized variant ofacm-tests-clc-createfor OPP interop jobs.Problem:
acm-tests-clc-createhas a 28800s (8h) timeout and suppresses all failures with|| :, even though it only creates 1 AWS managed cluster in ~50 minutes. When CLC fails silently, downstream steps (acm-fetch-managed-clusters,acm-opp-app) run against a missing cluster and produce confusing failures.Solution: New step with behavioral differences:
acm-tests-clc-createacm-tests-clc-smoke|| :(always passes)jq --rawfilekeeps secret off cmdlineReviewer Guide
Start here:
acm-tests-clc-smoke-ref.yamlshows the step definition (timeout, credentials, env vars).Core behavioral changes in
acm-tests-clc-smoke-commands.shvsacm-tests-clc-create:|| clcStatus=$?so reports are always copied toARTIFACT_DIRbefore propagating the failure.set +x) from password extraction through test execution, preventing cluster endpoint URLs from appearing in CI logs.echo "ERROR: ..." 1>&2; exit 1) instead of silent[ -n ] && [ -n ]that exits with no diagnostic.jq --rawfileto keep it off the process argument list.Config changes (2 files, 1-line each): swap
acm-tests-clc-create→acm-tests-clc-smokein the test section. Theacm-tests-clc-destroypost step is unchanged.Risk: If CLC has transient failures that the
|| :was masking, this step will now fail the job. That's intentional (fail fast so we triage the real issue instead of debugging missing-cluster errors in downstream steps). The 90-min timeout provides ~80% headroom over the observed ~50 min average.Changes
acm-tests-clc-smokeunderci-operator/step-registry/acm/tests/clc-smoke/acm-tests-clc-destroypost step unchangedTest Plan
acm-fetch-managed-clustersandacm-opp-appsucceed/cc @dtthuynh @vboulos
Rehearsal validation
Steps in this PR:
acm-tests-clc-smoke(new step)This step is not included in the batch PR (#83405). It was rehearsed independently via
opp-aws-4.22. The rehearsal failed at an infrastructure level unrelated to this step. Structural CI checks pass, and the step script follows established patterns from the existingacm-tests-clc-createstep.Rehearsal ack rationale: All rehearsal failures are caused by known infrastructure issues (ACS upstream scanner timeout, ACM S3 credential rotation) unrelated to this PR's changes. All structural CI checks (ci-operator-config, ci-operator-registry, step-registry-shellcheck, yamllint, generated-config) pass. See batch PR #83405 for full rehearsal results and analysis.