Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ tests:
OCP_VERSION: "4.22"
ODF_OPERATOR_CHANNEL: stable-4.20
ODF_VERSION_MAJOR_MINOR: "4.20"
REPORTPORTAL_CMP: CNV-lp-interop
REPORTPORTAL_CMP: lp-ocp-compat--CNV
USER_TAGS: |
scenario cnv
test:
Expand Down Expand Up @@ -109,7 +109,7 @@ tests:
ODF_OPERATOR_CHANNEL: stable-4.20
ODF_VERSION_MAJOR_MINOR: "4.20"
REPORT_TO_DR: "true"
REPORTPORTAL_CMP: CNV-lp-interop
REPORTPORTAL_CMP: lp-ocp-compat--CNV
USER_TAGS: |
scenario cnv
test:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,48 +1,29 @@
#!/bin/bash

set -o nounset
set -o errexit
set -o pipefail

# Ensure Requirements.
# yq should be installed within the container of openshift-cnv/cnv-ci
# as fallback we check if it is missing and install it if necessary
PATH="$(exec 3>&1 1>&2
typeset binDir="/tmp/bin"
mkdir -p "${binDir}"
yq --version || {
curl -sLo "${binDir}/yq" \
"https://github.com/mikefarah/yq/releases/latest/download/yq_$(
uname -s | tr '[:upper:]' '[:lower:]'
)_$(
uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/'
)" &&
chmod a+x "${binDir}/yq"
"${binDir}/yq" --version
}
echo "${binDir}" 1>&3):${PATH}"

function mapTestsForComponentReadiness() {
if [[ $MAP_TESTS == "true" ]]; then
results_file="${1}"
echo "Patching Tests Result File: ${results_file}"
if [ -f "${results_file}" ]; then
echo "Mapping Test Suite Name To: CNV-lp-interop"
yq eval -px -ox -iI0 '.testsuite."+@name" = "CNV-lp-interop"' "${results_file}"
fi
fi
}
set -euxo pipefail; shopt -s inherit_errexit

# Map results by setting identifier prefix in tests suites names for reporting tools
# Merge original results into a single file and compress
# Send modified file to shared dir for Data Router Reporter step
if [ "${MAP_TESTS}" = "true" ]; then
eval "$(
curl -fsSL \
https://raw.githubusercontent.com/RedHatQE/OpenShift-LP-QE--Tools/refs/heads/main/libs/bash/ci-operator/interop/common/ExitTrap--PostProcessPrep.sh
)"; trap '
LP_IO__ET_PPP__NEW_TS_NAME="${REPORTPORTAL_CMP}--%s" \
ExitTrap--PostProcessPrep junit--cnv__interop-tests__cnv-tests-e2e-deploy.xml
' EXIT
fi
Comment on lines +7 to +15
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n ci-operator/step-registry/interop-tests/cnv-tests-e2e-deploy/interop-tests-cnv-tests-e2e-deploy-commands.sh

Repository: openshift/release

Length of output: 3352


Don't let the helper fetch gate make deploy_test.

Lines 7-15 fetch and eval a script from refs/heads/main before the test run at line 58. With set -e enabled, any GitHub/raw outage or curl failure will fail this step before make deploy_test executes, even though this block only provides post-processing. Additionally, the mutable ref (refs/heads/main) makes the step non-reproducible. Defer this fetch to exit time with proper error handling and pin to an immutable commit hash instead.

Suggested direction
-if [ "${MAP_TESTS}" = "true" ]; then
-    eval "$(
-        curl -fsSL \
-https://raw.githubusercontent.com/RedHatQE/OpenShift-LP-QE--Tools/refs/heads/main/libs/bash/ci-operator/interop/common/ExitTrap--PostProcessPrep.sh
-    )"; trap '
-        LP_IO__ET_PPP__NEW_TS_NAME="${REPORTPORTAL_CMP}--%s" \
-            ExitTrap--PostProcessPrep junit--cnv__interop-tests__cnv-tests-e2e-deploy.xml
-    ' EXIT
-fi
+post_process_junit() {
+    [ "${MAP_TESTS}" = "true" ] || return 0
+
+    if eval "$(
+        curl -fsSL \
+https://raw.githubusercontent.com/RedHatQE/OpenShift-LP-QE--Tools/<pinned-commit>/libs/bash/ci-operator/interop/common/ExitTrap--PostProcessPrep.sh
+    )"; then
+        LP_IO__ET_PPP__NEW_TS_NAME="${REPORTPORTAL_CMP}--%s" \
+            ExitTrap--PostProcessPrep junit--cnv__interop-tests__cnv-tests-e2e-deploy.xml || \
+            echo "Warning: junit post-processing failed; leaving results unmodified" >&2
+    else
+        echo "Warning: unable to load ExitTrap--PostProcessPrep; leaving results unmodified" >&2
+    fi
+}
+
+trap post_process_junit EXIT
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@ci-operator/step-registry/interop-tests/cnv-tests-e2e-deploy/interop-tests-cnv-tests-e2e-deploy-commands.sh`
around lines 7 - 15, The startup block currently evals a remote script
immediately (when MAP_TESTS="true") which can fail under set -e and prevents
running make deploy_test; instead, remove the early eval and change the trap to
fetch-and-source the helper at EXIT time with robust error handling: in the trap
callback (referencing ExitTrap--PostProcessPrep, REPORTPORTAL_CMP and
junit--cnv__interop-tests__cnv-tests-e2e-deploy.xml) curl the raw URL pinned to
an immutable commit hash to a temp file, check curl's exit status and only
source the file if the download succeeded (log a warning but do not abort if it
failed), then call ExitTrap--PostProcessPrep and
junit--cnv__interop-tests__cnv-tests-e2e-deploy.xml processing; ensure no
commands in the startup path use eval of remote content so make deploy_test
always runs even if the fetch fails.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai
This has been discussed over and over again. You are supposed to learn this. See my comment and in any other PR about the the test nature is non-deterministic to begin with.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
gh api repos/openshift/release/pulls/comments/3183387185 --jq '.body'

Length of output: 404


@etirta, thank you for the clarification and apologies for the repeated noise. I now understand: ExitTrap--PostProcessPrep is mandatory for Component Readiness (CR) to pick up results — if the network fetch fails, failing the entire step is the correct and intentional behavior, because silently showing a "successful" test in CR without the post-processing would be misleading and harmful. The early eval+curl placement is a deliberate design decision.

I've added a learning to avoid raising this concern in future reviews.


📝 Learnings were identified and not saved (knowledge base disabled). Enable

Comment thread
oharan2 marked this conversation as resolved.

# Set cluster variables
# CLUSTER_NAME=$(cat "${SHARED_DIR}/CLUSTER_NAME")
# CLUSTER_DOMAIN="${CLUSTER_DOMAIN:-release-ci.cnv-qe.rhood.us}"
BIN_FOLDER=$(mktemp -d /tmp/bin.XXXX)
typeset binFolder=''
typeset exit_code=0
binFolder="$(mktemp -d /tmp/bin.XXXX)"

# Exports
# export CLUSTER_NAME CLUSTER_DOMAIN
export ARTIFACTS="${ARTIFACT_DIR}"
export PATH="${BIN_FOLDER}:${PATH}"
export KUBEVIRT_TESTING_CONFIGURATION_FILE=${KUBEVIRT_TESTING_CONFIGURATION_FILE:-'kubevirt-tier1-ocs.json'}
export PATH="${binFolder}:${PATH}"

# Unset the following environment variables to avoid issues with oc command
unset KUBERNETES_SERVICE_PORT_HTTPS
Expand All @@ -58,8 +39,8 @@ unset KUBERNETES_PORT_443_TCP_PORT
set -x

# Get oc binary
# curl -sL "${OC_URL}" | tar -C "${BIN_FOLDER}" -xzvf - oc
curl -L "https://github.com/openshift-cnv/cnv-ci/tarball/release-${OCP_VERSION}" -o /tmp/cnv-ci.tgz
# curl -sL "${OC_URL}" | tar -C "${binFolder}" -xzvf - oc
curl -fsSL "https://github.com/openshift-cnv/cnv-ci/tarball/release-${OCP_VERSION}" -o /tmp/cnv-ci.tgz
mkdir -p /tmp/cnv-ci
tar -xvzf /tmp/cnv-ci.tgz -C /tmp/cnv-ci --strip-components=1
cd /tmp/cnv-ci || exit 1
Expand All @@ -68,8 +49,8 @@ cd /tmp/cnv-ci || exit 1
# If KUBEVIRT_TESTING_CONFIGURATION is set and not empty, is has higher priority over KUBEVIRT_TESTING_CONFIGURATION_FILE
if [[ -n "${KUBEVIRT_TESTING_CONFIGURATION:-}" ]]; then
export KUBEVIRT_TESTING_CONFIGURATION_FILE="${ARTIFACT_DIR}/kubevirt-testing-configuration.json"
echo "${KUBEVIRT_TESTING_CONFIGURATION}" | tee "${KUBEVIRT_TESTING_CONFIGURATION_FILE}"
echo "🔄 KUBEVIRT_TESTING_CONFIGURATION_FILE set to ${KUBEVIRT_TESTING_CONFIGURATION_FILE}"
# Write inline JSON to the artifact path (avoid echo for xtrace log noise; tee preserves a traceable redirect).
tee "${KUBEVIRT_TESTING_CONFIGURATION_FILE}" <<< "${KUBEVIRT_TESTING_CONFIGURATION}"
fi


Expand All @@ -78,24 +59,18 @@ make deploy_test || exit_code=$?

set +x

# Map tests if needed for related use cases
mapTestsForComponentReadiness "${ARTIFACT_DIR}/junit.functest.xml"

# Send junit files to shared dir for Data Router Reporter step
cp "${ARTIFACT_DIR}"/*.xml "${SHARED_DIR}"


if [ "${exit_code:-0}" -ne 0 ]; then
echo "deploy_test failed with exit code $exit_code"
# deploy_test failed; exit status is propagated below (xtrace is off in this block).
if [[ -n "${CNV_WAIT_FOR_LIVE_DEBUG:-}" ]]; then
echo "Waiting to allow interactive debugging"
if ((CNV_WAIT_FOR_LIVE_DEBUG > 0)); then
# Hold for live debugging: positive CNV_WAIT_FOR_LIVE_DEBUG = bounded sleep; otherwise sleep inf.
if ((${CNV_WAIT_FOR_LIVE_DEBUG} > 0)); then
sleep "${CNV_WAIT_FOR_LIVE_DEBUG}"
else
sleep inf
fi
Comment thread
oharan2 marked this conversation as resolved.
fi
exit "${exit_code}"
else
echo "deploy_test succeeded"
fi

# deploy_test succeeded when execution reaches here (step outcome from exit status).
true
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ ref:
from: cnv-ci
commands: interop-tests-cnv-tests-e2e-deploy-commands.sh
timeout: 4h0m0s
grace_period: 10m
credentials:
- namespace: test-credentials
name: cluster-secrets-aws-interop-qe
Expand Down Expand Up @@ -57,3 +58,6 @@ ref:
- name: CNV_WAIT_FOR_LIVE_DEBUG
default: "0"
documentation: Specify whether to wait for live debug
- name: REPORTPORTAL_CMP
default: "lp-ocp-compat--CNV"
documentation: If test mapping is applied, differentiate between the CNV and ODF jobs
Original file line number Diff line number Diff line change
@@ -1,17 +1,42 @@
#!/bin/bash

set -o nounset
set -o errexit
set -o pipefail

start_time=$SECONDS
set -euo pipefail; shopt -s inherit_errexit

# This trap will be executed when the script exits for any reason (successful, error, or signal).
trap 'debug_on_exit' EXIT
if [ "${MAP_TESTS}" = "true" ]; then
# Map results by setting identifier prefix in tests suites names for reporting tools
# Merge original results into a single file and compress
# Send modified file to shared dir for Data Router Reporter step (run here so EXIT stays debug_on_exit).
eval "$(
curl -fsSL \
https://raw.githubusercontent.com/RedHatQE/OpenShift-LP-QE--Tools/refs/heads/main/libs/bash/ci-operator/interop/common/ExitTrap--PostProcessPrep.sh
)"; trap '
typeset scriptExit=$?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scriptExit is a somewhat peculiar name for storing the actual exit status. Terms like es, ec, or rc (corresponding to exit status, exit code, and return code, respectively) are more commonly used.

LP_IO__ET_PPP__NEW_TS_NAME="${REPORTPORTAL_CMP}--%s" \
ExitTrap--PostProcessPrep junit--cnv__interop-tests__openshift-virtualization-tests.xml || true
debug_on_exit "${scriptExit}"
' EXIT
Comment thread
coderabbitai[bot] marked this conversation as resolved.
else
trap 'debug_on_exit' EXIT
fi

typeset start_time=${SECONDS}
typeset binFolder=''
typeset ocUrl=''
typeset hcoSubscription=''
typeset rc=0
typeset ARTIFACTORY_USER=''
typeset ARTIFACTORY_TOKEN=''
typeset ARTIFACTORY_SERVER=''
typeset ACCESS_TOKEN=''
typeset ORGANIZATION_ID=''
typeset -x OPENSHIFT_PYTHON_WRAPPER_LOG_FILE="${ARTIFACT_DIR}/openshift_python_wrapper.log"
typeset -x JUNIT_RESULTS_FILE="${ARTIFACT_DIR}/junit_results.xml"
typeset -x HTML_RESULTS_FILE="${ARTIFACT_DIR}/report.html"

# shellcheck disable=SC2329
debug_on_exit() {
local exit_code=$?
# First argument is the script exit status when the EXIT trap ran other handlers first (e.g. ExitTrap--PostProcessPrep).
local exit_code="${1:-$?}"
local end_time=$SECONDS
local execution_time=$((end_time - start_time))
local debug_threshold=720 # 12 minutes in seconds
Expand Down Expand Up @@ -63,7 +88,6 @@ function getMustGatherImage() {
| .spec.relatedImages[]
| select(.name | contains("must-gather"))
| .image'

}

# shellcheck disable=SC2329
Expand All @@ -90,11 +114,11 @@ function retry() {
until "$@"; do
exit_code=$?
count=$((count + 1))
if [ $count -lt $max_retries ]; then
echo "Command failed. Attempt $count/$max_retries. Retrying in $delay seconds..."
sleep $delay
if [ "${count}" -lt "${max_retries}" ]; then
# Command failed. Attempt ${count}/${max_retries}.
sleep "${delay}"
else
echo "Command failed after $max_retries attempts."
# Command failed after $max_retries attempts.
return $exit_code
fi
done
Expand Down Expand Up @@ -146,6 +170,7 @@ function cnv::reimport_datavolumes() {
local retry_count=0
local max_retries=10
local interval=30
local volumesnapshotcontent_name=''
while [[ $retry_count -lt $max_retries ]]; do
echo "Attempting to delete all volumesnapshots in namespace ${dvnamespace} (Attempt $((retry_count + 1)) of ${max_retries})..."

Expand Down Expand Up @@ -182,42 +207,11 @@ function cnv::reimport_datavolumes() {
oc get pvc -n "${dvnamespace}"
}

function install_yq_if_not_exists() {
# Install yq manually if not found in image
echo "Checking if yq exists"
cmd_yq="$(yq --version 2>/dev/null || true)"
if [ -n "$cmd_yq" ]; then
echo "yq version: $cmd_yq"
else
echo "Installing yq"
mkdir -p /tmp/bin
export PATH=$PATH:/tmp/bin/
curl -L "https://github.com/mikefarah/yq/releases/latest/download/yq_linux_$(uname -m | sed 's/aarch64/arm64/;s/x86_64/amd64/')" \
-o /tmp/bin/yq && chmod +x /tmp/bin/yq
fi
}

function mapTestsForComponentReadiness() {

[[ ${MAP_TESTS:-false} != "true" ]] && return

results_file="${1}"
echo "Patching Tests Result File: ${results_file}"
if [ -f "${results_file}" ]; then
install_yq_if_not_exists
echo "Mapping Test Suite Name To: CNV-lp-interop"
yq eval -px -ox -iI0 '.testsuites.testsuite.+@name="CNV-lp-interop"' $results_file
fi
}

BIN_FOLDER=$(mktemp -d /tmp/bin.XXXX)
OC_URL="https://mirror.openshift.com/pub/openshift-v4/amd64/clients/ocp/latest/openshift-client-linux.tar.gz"
binFolder="$(mktemp -d /tmp/bin.XXXX)"
ocUrl='https://mirror.openshift.com/pub/openshift-v4/amd64/clients/ocp/latest/openshift-client-linux.tar.gz'

# Exports
export PATH="${BIN_FOLDER}:${PATH}"
export OPENSHIFT_PYTHON_WRAPPER_LOG_FILE="${ARTIFACT_DIR}/openshift_python_wrapper.log"
export JUNIT_RESULTS_FILE="${ARTIFACT_DIR}/junit_results.xml"
export HTML_RESULTS_FILE="${ARTIFACT_DIR}/report.html"
export PATH="${binFolder}:${PATH}"
set +x # We don't want to see it in the logs
ARTIFACTORY_USER=$(head -1 "${BW_PATH}"/artifactory-user || printf ci-read-only-user)
ARTIFACTORY_TOKEN=$(head -1 "${BW_PATH}"/artifactory-token)
Expand All @@ -239,10 +233,10 @@ unset KUBERNETES_PORT_443_TCP_PORT

###########################################################################
# Get oc binary
curl -sL "${OC_URL}" | tar -C "${BIN_FOLDER}" -xzvf - oc
curl -sL "${ocUrl}" | tar -C "${binFolder}" -xzvf - oc
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Harden oc download against transient network failures.

curl -sL here has no fail-fast, retry, or timeout controls. A flaky network can make the step fail intermittently or hang longer than needed.

Suggested change
-curl -sL "${ocUrl}" | tar -C "${binFolder}" -xzvf - oc
+curl --fail --silent --show-error --location \
+  --retry 5 --retry-all-errors --connect-timeout 30 --max-time 300 \
+  "${ocUrl}" | tar -C "${binFolder}" -xzvf - oc
📝 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
curl -sL "${ocUrl}" | tar -C "${binFolder}" -xzvf - oc
curl --fail --silent --show-error --location \
--retry 5 --retry-all-errors --connect-timeout 30 --max-time 300 \
"${ocUrl}" | tar -C "${binFolder}" -xzvf - oc
🤖 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/interop-tests/openshift-virtualization-tests/interop-tests-openshift-virtualization-tests-commands.sh`
at line 236, The curl pipeline that downloads and extracts oc using ocUrl and
binFolder is not resilient to transient network issues; update the download step
to use curl with fail-fast and retry/timeouts (e.g., add --fail, --retry with a
small count, --retry-connrefused, and connection/total timeouts like
--connect-timeout/--max-time) and ensure the command exits non-zero on failure
before piping to tar (so tar doesn't hang on an empty stream). Apply these
options to the existing usage of ocUrl and preserve extraction into binFolder
(targeting the same oc binary).


oc whoami --show-console
HCO_SUBSCRIPTION=$(oc get subscription.operators.coreos.com -n openshift-cnv -o jsonpath='{.items[0].metadata.name}')
hcoSubscription="$(oc get subscription.operators.coreos.com -n openshift-cnv -o jsonpath='{.items[0].metadata.name}')"

oc get sc # Before
setDefaultStorageClass 'ocs-storagecluster-ceph-rbd-virtualization'
Expand All @@ -262,7 +256,7 @@ uv --verbose --cache-dir /tmp/uv-cache \
--tb=native \
--tc default_storage_class:ocs-storagecluster-ceph-rbd-virtualization \
--tc default_volume_mode:Block \
--tc "hco_subscription:${HCO_SUBSCRIPTION}" \
--tc "hco_subscription:${hcoSubscription}" \
--latest-rhel \
--storage-class-matrix=ocs-storagecluster-ceph-rbd-virtualization \
--leftovers-collector \
Expand All @@ -277,10 +271,4 @@ uv --verbose --cache-dir /tmp/uv-cache \
# | xmllint --format - > "${JUNIT_RESULTS_FILE}"
# fi

# Map tests if needed for related use cases
mapTestsForComponentReadiness "${JUNIT_RESULTS_FILE}"

# Send junit file to shared dir for Data Router Reporter step
cp "${JUNIT_RESULTS_FILE}" "${SHARED_DIR}"

exit ${rc}
exit "${rc}"
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,6 @@ ref:
- name: MAP_TESTS
default: "false"
documentation: Specify whether to update the test suite name for reporting tools
- name: REPORTPORTAL_CMP
default: "lp-ocp-compat--CNV"
documentation: If test mapping is applied, differentiate between the CNV and ODF jobs