HIVE-3148: Adding HCMbundle tests using OTE#79862
Conversation
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/hold
|
@miyadav: This pull request references HIVE-3148 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: miyadav 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 |
|
/pj-rehearse periodic-ci-openshift-hive-master-periodic-e2e-ote-sd-rosa |
|
@miyadav: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals. Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
WalkthroughThis PR adds periodic CI testing for Hive with ROSA-specific OTE (OpenShift Test Extension) validation. It introduces a ChangesOTE Hive Periodic Testing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/openshift/hive/openshift-hive-master__periodic.yaml`:
- Around line 378-407: The step currently forces
/usr/bin/openshift-tests-extension run-suite to succeed with "|| true" and only
fails when extension_test_result_*.json exists, so if run-suite crashes before
writing JSON the job will incorrectly pass; change the logic in the run-suite
block to capture run-suite's exit code (e.g., RC=$?), remove the unconditional
"|| true" or immediately check RC, and if RC is non-zero and no RESULT_JSON was
produced (RESULT_JSON empty or not a file in ARTIFACT_DIR) then exit 1; keep the
existing JSON-to-junit conversion and FAIL_COUNT logic but add an explicit check
after running run-suite that fails the job when run-suite failed or when no
RESULT_JSON was created.
🪄 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: cac7ada3-f79c-4bdb-b8ed-0d05804f51e6
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/hive/openshift-hive-master-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/openshift/hive/openshift-hive-master__periodic.yaml
| /usr/bin/openshift-tests-extension run-suite -c 1 openshift/hive -j ${ARTIFACT_DIR}/junit_results.xml || true | ||
| RESULT_JSON=$(ls ${ARTIFACT_DIR}/extension_test_result_*.json 2>/dev/null | head -1) | ||
| if [[ -n "$RESULT_JSON" && -f "$RESULT_JSON" ]]; then | ||
| jq -r ' | ||
| (. // []) | | ||
| def pass: | ||
| .result == "passed" or | ||
| ((.output // "") | contains("SUCCESS! -- 1 Passed")); | ||
| "<?xml version=\"1.0\" encoding=\"UTF-8\"?>", | ||
| "<testsuite tests=\"\(length)\" failures=\"\([.[] | select(pass | not)] | length)\">", | ||
| (.[] | | ||
| if pass then | ||
| "<testcase name=\"\(.name | @html)\"/>" | ||
| else | ||
| "<testcase name=\"\(.name | @html)\"><failure><![CDATA[\((.error // "")[0:500])]]></failure></testcase>" | ||
| end | ||
| ), | ||
| "</testsuite>" | ||
| ' "$RESULT_JSON" > "${ARTIFACT_DIR}/junit_results.xml" | ||
| FAIL_COUNT=$(jq ' | ||
| (. // []) | | ||
| def pass: | ||
| .result == "passed" or | ||
| ((.output // "") | contains("SUCCESS! -- 1 Passed")); | ||
| [.[] | select(pass | not)] | length | ||
| ' "$RESULT_JSON") | ||
| TOTAL=$(jq '(. // []) | length' "$RESULT_JSON") | ||
| echo "Results: $((TOTAL - FAIL_COUNT)) passed, ${FAIL_COUNT} failed" | ||
| [[ $FAIL_COUNT -gt 0 ]] && exit 1 | ||
| fi |
There was a problem hiding this comment.
Fail the job when test execution fails before producing result JSON.
run-suite is forced to succeed with || true, and the script only asserts failures when a JSON file exists. If the runner fails early and writes no extension_test_result_*.json, this step passes incorrectly.
Proposed fix
- /usr/bin/openshift-tests-extension run-suite -c 1 openshift/hive -j ${ARTIFACT_DIR}/junit_results.xml || true
+ set +e
+ /usr/bin/openshift-tests-extension run-suite -c 1 openshift/hive -j ${ARTIFACT_DIR}/junit_results.xml
+ RUN_SUITE_RC=$?
+ set -e
RESULT_JSON=$(ls ${ARTIFACT_DIR}/extension_test_result_*.json 2>/dev/null | head -1)
- if [[ -n "$RESULT_JSON" && -f "$RESULT_JSON" ]]; then
+ if [[ -n "$RESULT_JSON" && -f "$RESULT_JSON" ]]; then
jq -r '
(. // []) |
def pass:
.result == "passed" or
((.output // "") | contains("SUCCESS! -- 1 Passed"));
@@
TOTAL=$(jq '(. // []) | length' "$RESULT_JSON")
echo "Results: $((TOTAL - FAIL_COUNT)) passed, ${FAIL_COUNT} failed"
[[ $FAIL_COUNT -gt 0 ]] && exit 1
+ elif [[ $RUN_SUITE_RC -ne 0 ]]; then
+ echo "openshift-tests-extension failed and produced no result JSON"
+ exit $RUN_SUITE_RC
fi📝 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.
| /usr/bin/openshift-tests-extension run-suite -c 1 openshift/hive -j ${ARTIFACT_DIR}/junit_results.xml || true | |
| RESULT_JSON=$(ls ${ARTIFACT_DIR}/extension_test_result_*.json 2>/dev/null | head -1) | |
| if [[ -n "$RESULT_JSON" && -f "$RESULT_JSON" ]]; then | |
| jq -r ' | |
| (. // []) | | |
| def pass: | |
| .result == "passed" or | |
| ((.output // "") | contains("SUCCESS! -- 1 Passed")); | |
| "<?xml version=\"1.0\" encoding=\"UTF-8\"?>", | |
| "<testsuite tests=\"\(length)\" failures=\"\([.[] | select(pass | not)] | length)\">", | |
| (.[] | | |
| if pass then | |
| "<testcase name=\"\(.name | @html)\"/>" | |
| else | |
| "<testcase name=\"\(.name | @html)\"><failure><![CDATA[\((.error // "")[0:500])]]></failure></testcase>" | |
| end | |
| ), | |
| "</testsuite>" | |
| ' "$RESULT_JSON" > "${ARTIFACT_DIR}/junit_results.xml" | |
| FAIL_COUNT=$(jq ' | |
| (. // []) | | |
| def pass: | |
| .result == "passed" or | |
| ((.output // "") | contains("SUCCESS! -- 1 Passed")); | |
| [.[] | select(pass | not)] | length | |
| ' "$RESULT_JSON") | |
| TOTAL=$(jq '(. // []) | length' "$RESULT_JSON") | |
| echo "Results: $((TOTAL - FAIL_COUNT)) passed, ${FAIL_COUNT} failed" | |
| [[ $FAIL_COUNT -gt 0 ]] && exit 1 | |
| fi | |
| set +e | |
| /usr/bin/openshift-tests-extension run-suite -c 1 openshift/hive -j ${ARTIFACT_DIR}/junit_results.xml | |
| RUN_SUITE_RC=$? | |
| set -e | |
| RESULT_JSON=$(ls ${ARTIFACT_DIR}/extension_test_result_*.json 2>/dev/null | head -1) | |
| if [[ -n "$RESULT_JSON" && -f "$RESULT_JSON" ]]; then | |
| jq -r ' | |
| (. // []) | | |
| def pass: | |
| .result == "passed" or | |
| ((.output // "") | contains("SUCCESS! -- 1 Passed")); | |
| "<?xml version=\"1.0\" encoding=\"UTF-8\"?>", | |
| "<testsuite tests=\"\(length)\" failures=\"\([.[] | select(pass | not)] | length)\">", | |
| (.[] | | |
| if pass then | |
| "<testcase name=\"\(.name | `@html`)\"/>" | |
| else | |
| "<testcase name=\"\(.name | `@html`)\"><failure><![CDATA[\((.error // "")[0:500])]]></failure></testcase>" | |
| end | |
| ), | |
| "</testsuite>" | |
| ' "$RESULT_JSON" > "${ARTIFACT_DIR}/junit_results.xml" | |
| FAIL_COUNT=$(jq ' | |
| (. // []) | | |
| def pass: | |
| .result == "passed" or | |
| ((.output // "") | contains("SUCCESS! -- 1 Passed")); | |
| [.[] | select(pass | not)] | length | |
| ' "$RESULT_JSON") | |
| TOTAL=$(jq '(. // []) | length' "$RESULT_JSON") | |
| echo "Results: $((TOTAL - FAIL_COUNT)) passed, ${FAIL_COUNT} failed" | |
| [[ $FAIL_COUNT -gt 0 ]] && exit 1 | |
| elif [[ $RUN_SUITE_RC -ne 0 ]]; then | |
| echo "openshift-tests-extension failed and produced no result JSON" | |
| exit $RUN_SUITE_RC | |
| 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/config/openshift/hive/openshift-hive-master__periodic.yaml`
around lines 378 - 407, The step currently forces
/usr/bin/openshift-tests-extension run-suite to succeed with "|| true" and only
fails when extension_test_result_*.json exists, so if run-suite crashes before
writing JSON the job will incorrectly pass; change the logic in the run-suite
block to capture run-suite's exit code (e.g., RC=$?), remove the unconditional
"|| true" or immediately check RC, and if RC is non-zero and no RESULT_JSON was
produced (RESULT_JSON empty or not a file in ARTIFACT_DIR) then exit 1; keep the
existing JSON-to-junit conversion and FAIL_COUNT logic but add an explicit check
after running run-suite that fails the job when run-suite failed or when no
RESULT_JSON was created.
|
@miyadav: 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. |
2 participants
/hold
Onces the tests looks stable we can retire the CI job using OTP and use this one for HCM bundle tests .
Summary by CodeRabbit
This PR establishes new CI infrastructure for testing Hive using the OpenShift Tests Extension (OTE) framework on ROSA (Red Hat OpenShift on AWS) deployments.
What changed:
Two additions to the Hive periodic CI configuration (
openshift-hive-master__periodic.yaml):New
hive-testsimage build: Created a container image that builds and packages the OTE test extension binary. This image is constructed by:make -C test/ote build)/usr/bin/openshift-tests-extensionNew
e2e-ote-sd-rosaperiodic job: A weekly scheduled test job (runs Saturdays at 6 AM) that:#team-hive-alertSlack channelPurpose: As noted in the PR comments, this adds a new CI validation path for HCM (Hive Cluster Manager) bundle functionality. Once these OTE-based tests prove stable, the team plans to retire the existing OTP-based CI job in favor of this new test pipeline.