ROSAENG-23861: Add ROSA gap-analysis test workflows for Classic and HCP clusters - #82780
ROSAENG-23861: Add ROSA gap-analysis test workflows for Classic and HCP clusters#82780TheUndeadKing wants to merge 0 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@TheUndeadKing: This pull request references ROSAENG-23861 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:
WalkthroughAdds ROSA Classic and HCP gap-analysis workflows, a validation step, diagnostic reporting, cleanup actions, metadata, CI scheduling, and ownership configuration. ChangesROSA gap-analysis CI
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ROSACIConfig
participant ROSAWorkflow
participant ROSACluster
participant GapAnalysisTest
participant OCM
ROSACIConfig->>ROSAWorkflow: schedule Classic or HCP gap-analysis
ROSAWorkflow->>ROSACluster: provision cluster and wait for readiness
ROSAWorkflow->>GapAnalysisTest: run validation step
GapAnalysisTest->>OCM: authenticate and retrieve cluster credentials
GapAnalysisTest->>ROSACluster: validate operators and nodes
GapAnalysisTest->>ROSACluster: collect diagnostics and write report
GapAnalysisTest-->>ROSAWorkflow: return validation status
ROSAWorkflow->>ROSACluster: perform best-effort cleanup
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 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 |
openshift-merge-bot
Bot
added
the
rehearsals-ack
![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/rosa/gap-analysis/test/rosa-gap-analysis-test-commands.sh`:
- Line 100: Replace the TIMEOUT_SECONDS calculation in the VALIDATION_TIMEOUT
parsing flow with a small parser that recognizes minute and hour suffixes,
multiplying minute values by 60 and hour values by 3600. Reject unsupported or
malformed timeout formats explicitly instead of silently producing an invalid
value.
- Around line 36-41: Update both credential branches in the login flow before
the first rosa describe cluster invocation to authenticate the ROSA CLI with
rosa login, using the available SSO credentials in the
SSO_CLIENT_ID/SSO_CLIENT_SECRET branch and the ROSA_TOKEN in the offline-token
branch. Keep the existing ocm login commands and ensure each supported branch
performs rosa login before cluster inspection.
🪄 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: Pro Plus
Run ID: 1de9089a-16e6-4d1e-914f-3b4be4050cd3
📒 Files selected for processing (10)
ci-operator/step-registry/rosa/gap-analysis/classic/OWNERSci-operator/step-registry/rosa/gap-analysis/classic/rosa-gap-analysis-classic-workflow.metadata.jsonci-operator/step-registry/rosa/gap-analysis/classic/rosa-gap-analysis-classic-workflow.yamlci-operator/step-registry/rosa/gap-analysis/hcp/OWNERSci-operator/step-registry/rosa/gap-analysis/hcp/rosa-gap-analysis-hcp-workflow.metadata.jsonci-operator/step-registry/rosa/gap-analysis/hcp/rosa-gap-analysis-hcp-workflow.yamlci-operator/step-registry/rosa/gap-analysis/test/OWNERSci-operator/step-registry/rosa/gap-analysis/test/rosa-gap-analysis-test-commands.shci-operator/step-registry/rosa/gap-analysis/test/rosa-gap-analysis-test-ref.metadata.jsonci-operator/step-registry/rosa/gap-analysis/test/rosa-gap-analysis-test-ref.yaml
| ########################################### | ||
| log "Validation 1: Waiting for cluster ready state (timeout: ${VALIDATION_TIMEOUT})..." | ||
| CLUSTER_READY=false | ||
| TIMEOUT_SECONDS=$(($(echo "${VALIDATION_TIMEOUT}" | sed 's/m$//' | sed 's/h$//' | awk '{print $1}') * 60)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ci-operator/step-registry/rosa/gap-analysis/test/rosa-gap-analysis-test-commands.sh"
if [ -f "$file" ]; then
echo "== file exists =="
wc -l "$file"
echo "== relevant lines 1-180 =="
cat -n "$file" | sed -n '1,180p'
else
echo "File not found: $file"
fi
echo "== shell arithmetic probe =="
python3 - <<'PY'
import subprocess, sys
for v in ["1m", "1h", "2h", "30m", "90m"]:
out = subprocess.check_output(["bash", "-c", f'VALIDATION_TIMEOUT={v}; TIMEOUT_SECONDS=$(($(echo "${{VALIDATION_TIMEOUT}}" | sed "s/m$"\\| sed "s/h$//" | awk "{{print $1}}") * 60)); echo "$TIMEOUT_SECONDS"'], text=True).strip()
print(f"VALIDATION_TIMEOUT={v} -> TIMEOUT_SECONDS={out}")
PYRepository: openshift/release
Length of output: 8449
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== fixed sed probe =="
python3 - <<'PY'
import subprocess
for v in ["1m", "1h", "2h", "30m", "90m", "2h30m", "abc"]:
try:
out = subprocess.check_output(
["bash", "-c", f'VALIDATION_TIMEOUT={v}; TIMEOUT_SECONDS=$(($(echo "${{VALIDATION_TIMEOUT}}" | sed "s/m$"\\| sed "s/h$//" | awk "{{print $1}}") * 60)); echo "$TIMEOUT_SECONDS"'],
text=True
).strip()
except subprocess.CalledProcessError as e:
out = f"exit {e.returncode}"
print(f"VALIDATION_TIMEOUT={v} -> TIMEOUT_SECONDS={out}")
PY
echo "== parse function candidates =="
rg -n "VALIDATION_TIMEOUT|TIMEOUT_SECONDS|seconds|hour|minute|timeout" ci-operator/step-registry/rosa/gap-analysis/test/rosa-gap-analysis-test-commands.sh ci-operator/step-registry/rosa/gap-analysis -g '!**/*.txt' || trueRepository: openshift/release
Length of output: 3291
Parse validation timeouts into seconds safely.
ci-operator/step-registry/rosa/gap-analysis/test/rosa-gap-analysis-test-commands.sh:100 currently fails for all VALIDATION_TIMEOUT forms because the sed replacement expression is malformed, and it also uses the wrong multiplier for hour values. Replace the malformed expression with a small parser that handles minutes by multiplying by 60 and hours by multiplying by 3600, plus an explicit rejection for unsupported formats.
🤖 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/rosa/gap-analysis/test/rosa-gap-analysis-test-commands.sh`
at line 100, Replace the TIMEOUT_SECONDS calculation in the VALIDATION_TIMEOUT
parsing flow with a small parser that recognizes minute and hour suffixes,
multiplying minute values by 60 and hour values by 3600. Reject unsupported or
malformed timeout formats explicitly instead of silently producing an invalid
value.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
| # Disable tracing due to kubeconfig handling | ||
| [[ $- == *x* ]] && WAS_TRACING=true || WAS_TRACING=false | ||
| set +x | ||
| ocm get /api/clusters_mgmt/v1/clusters/"${CLUSTER_ID}"/credentials 2>/dev/null | jq -r '.kubeconfig' > "${KUBECONFIG_FILE}" || { |
There was a problem hiding this comment.
Chain already creates a working admin kubeconfig in rosa-conf-idp-htpasswd (oc login as rosa-admin, then cat ${KUBECONFIG} > "${SHARED_DIR}/kubeconfig"). Later steps (rosa-cluster-wait-ready-operators, rosa-cluster-wait-ready-nodes) rely on that file. This line always overwrites ${SHARED_DIR}/kubeconfig. Is it possible to use existing kubeconfig when present; only fetch as fallback.
rbhilare
left a comment
rbhilare
left a comment
There was a problem hiding this comment.
Confirm whether job wiring (periodics) is a follow-up; as-is these workflows won’t run after merge.
openshift-merge-bot
Bot
removed
the
rehearsals-ack
| log "Running gap-analysis validation for cluster: ${CLUSTER_ID}" | ||
|
|
||
| # Initialize validation report | ||
| VALIDATION_REPORT="${ARTIFACT_DIR}/gap-analysis-validation-report.txt" |
There was a problem hiding this comment.
What kind of validation report it is? Are we not executing/running actual GAP analysis here. I do not understand this step.
There was a problem hiding this comment.
It only collect co, no, install log as per card[ROSAENG-23861] Acceptance Criteria.
There was a problem hiding this comment.
This should not be here. Please check feasibility to add this to rosa-gap-analysis repository.
| @@ -0,0 +1,40 @@ | |||
| base_images: | |||
There was a problem hiding this comment.
I think you need to check wiring again. You have used incorrect folder to place config and job.
As I mentioned above, config and job for rosa gap analysis already present and running
Job: https://github.com/openshift/release/tree/main/ci-operator/jobs/openshift-online/rosa-gap-analysis
Config: https://github.com/openshift/release/tree/main/ci-operator/config/openshift-online/rosa-gap-analysis.
I saw that you have written 2 different jobs, one for classic and one for HCP. Here are our current jobs independent of product type - https://prow.ci.openshift.org/?type=periodic&job=*rosa-gap-analysis-main-periodics*
rbhilare
left a comment
rbhilare
left a comment
There was a problem hiding this comment.
I think you need to check wiring again.
|
@TheUndeadKing: |
| log "Running gap-analysis validation for cluster: ${CLUSTER_ID}" | ||
|
|
||
| # Initialize validation report | ||
| VALIDATION_REPORT="${ARTIFACT_DIR}/gap-analysis-validation-report.txt" |
There was a problem hiding this comment.
This should not be here. Please check feasibility to add this to rosa-gap-analysis repository.
|
/pj-rehearse |
|
@TheUndeadKing: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@TheUndeadKing: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@TheUndeadKing: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@TheUndeadKing: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/retest |
|
@TheUndeadKing: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@TheUndeadKing: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@TheUndeadKing: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse abort |
|
@TheUndeadKing: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
/retest all |
|
@TheUndeadKing: The The following commands are available to trigger optional jobs: Use 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 kubernetes-sigs/prow repository. |
|
/pj-rehearse |
|
@TheUndeadKing: your |
1 similar comment
|
@TheUndeadKing: your |
eed89b6 to
fda17fe
Compare
fda17fe to
5fe0732
Compare
openshift-merge-bot
Bot
added
the
rehearsals-ack
|
@TheUndeadKing: your |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: TheUndeadKing 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 |
|
[REHEARSALNOTIFIER] Note: If this PR includes changes to step registry files ( Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@TheUndeadKing: 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. |
3 participants
Summary
Adds gap-analysis validation test workflows for ROSA Classic STS and HCP clusters to validate cluster health after provisioning.
This PR introduces automated gap-analysis testing for ROSA clusters that:
JIRA
https://redhat.atlassian.net/browse/ROSAENG-23861
Summary by CodeRabbit
openshift/rosamaster configuration.