Skip to content

Add readiness checks before running ocp-virt-validation-checkup #83157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
orenc1 wants to merge 1 commit into
base: main
Choose a base branch
from
+30 −0
Open

Add readiness checks before running ocp-virt-validation-checkup #83157

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 30 additions & 0 deletions ...on-checkup/agent-qe-baremetal-install-ove-disconnected-run-validation-checkup-commands.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,41 @@

set -euo pipefail

echo "=== Waiting for ODF StorageClass ${STORAGE_CLASS} ==="
COUNTER=0
while [ $COUNTER -lt 300 ]; do
if oc get storageclass "${STORAGE_CLASS}" &>/dev/null; then
echo "StorageClass ${STORAGE_CLASS} found"
break
fi
sleep 10
COUNTER=$((COUNTER + 10))
echo "Waiting ${COUNTER}s for StorageClass ${STORAGE_CLASS}..."
done

echo "=== Setting ODF StorageClass as default ==="
oc patch storageclass "${STORAGE_CLASS}" \
-p '{"metadata":{"annotations":{"storageclass.kubernetes.io/is-default-class":"true"}}}'
echo "StorageClass ${STORAGE_CLASS} set as default"

echo "=== Waiting for HCO CLI download route ==="
CLI_ROUTE=$(oc get route hyperconverged-cluster-cli-download -n "${TARGET_NAMESPACE}" \
-o jsonpath='{.status.ingress[0].host}' 2>/dev/null || echo "")
if [[ -n "${CLI_ROUTE}" ]]; then
COUNTER=0
while [ $COUNTER -lt 120 ]; do
if curl -sk -o /dev/null -w '%{http_code}' "https://${CLI_ROUTE}/" 2>/dev/null | grep -qE '^(200|301|302)'; then

@coderabbitai coderabbitai Bot Aug 10, 2026

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

curl_help="$(curl --help all)"
grep -q -- '--connect-timeout' <<<"${curl_help}"
grep -q -- '--max-time' <<<"${curl_help}"

Repository: openshift/release

Length of output: 155

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/run-validation-checkup/agent-qe-baremetal-install-ove-disconnected-run-validation-checkup-commands.sh'

printf '%s\n' '--- target script ---'
cat -n "$file" | sed -n '1,90p'

printf '%s\n' '--- diff summary ---'
git diff --stat -- "$file"

printf '%s\n' '--- timeout and curl references ---'
rg -n -C 3 'curl|120|CLI_ROUTE|set -' "$file" ci-operator/step-registry/agent-qe/baremetal/install/ove/disconnected/run-validation-checkup

Repository: openshift/release

Length of output: 11763

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- curl timeout documentation ---'
curl --manual 2>/dev/null |
  grep -i -E -A 4 -B 4 'maximum time.*complete|default.*timeout|--max-time|--connect-timeout' |
  head -80

printf '%s\n' '--- no curl deadline: external timeout is required ---'
set +e
timeout 1s curl -sS -o /dev/null -w '%{http_code}' 'file:///dev/zero' >/tmp/curl-no-max-time.out 2>/dev/null
status_no_max_time=$?
set -e
printf 'status=%s output_bytes=%s\n' \
  "$status_no_max_time" "$(wc -c </tmp/curl-no-max-time.out)"

printf '%s\n' '--- curl max-time enforces a request deadline ---'
set +e
curl -sS --max-time 0.2 -o /dev/null -w '%{http_code}' 'file:///dev/zero' >/tmp/curl-max-time.out 2>/dev/null
status_max_time=$?
set -e
printf 'status=%s output_bytes=%s\n' \
  "$status_max_time" "$(wc -c </tmp/curl-max-time.out)"

test "$status_no_max_time" -eq 124
test "$status_max_time" -eq 28

Repository: openshift/release

Length of output: 190

🏁 Script executed:

#!/bin/bash
set -u

printf '%s\n' '--- curl without --max-time ---'
timeout 1s curl -sS -o /dev/null -w '%{http_code}' \
  'file:///dev/zero' >/tmp/curl-no-max-time.out 2>/dev/null
status_no_max_time=$?
printf 'status=%s output_bytes=%s\n' \
  "$status_no_max_time" "$(wc -c </tmp/curl-no-max-time.out)"

printf '%s\n' '--- curl with --max-time ---'
curl -sS --max-time 0.2 -o /dev/null -w '%{http_code}' \
  'file:///dev/zero' >/tmp/curl-max-time.out 2>/dev/null
status_max_time=$?
printf 'status=%s output_bytes=%s\n' \
  "$status_max_time" "$(wc -c </tmp/curl-max-time.out)"

test "$status_no_max_time" -eq 124
test "$status_max_time" -eq 28

Repository: openshift/release

Length of output: 267

🏁 Script executed:

#!/bin/bash
set -u

printf '%s\n' '--- curl without --max-time ---'
timeout 1s curl -sS -o /dev/null -w '%{http_code}' \
  'file:///dev/zero' >/tmp/curl-no-max-time.out 2>/dev/null
status_no_max_time=$?
printf 'status=%s output_bytes=%s\n' \
  "$status_no_max_time" "$(wc -c </tmp/curl-no-max-time.out)"

printf '%s\n' '--- curl with --max-time ---'
curl -sS --max-time 0.2 -o /dev/null -w '%{http_code}' \
  'file:///dev/zero' >/tmp/curl-max-time.out 2>/dev/null
status_max_time=$?
printf 'status=%s output_bytes=%s\n' \
  "$status_no_max_time" "$(wc -c </tmp/curl-no-max-time.out)"

test "$status_no_max_time" -eq 124
test "$status_max_time" -eq 28

Repository: openshift/release

Length of output: 268

🌐 Web query:

Official curl documentation --max-time default zero --connect-timeout timeout

💡 Result:

In curl, the --max-time option has no default timeout; it effectively defaults to zero, meaning there is no time limit on the operation [1]. When --max-time is set (or left at its default of zero), curl will allow the operation to continue indefinitely until completion or until another network-related error occurs [1]. Regarding --connect-timeout, this option limits only the connection phase, which includes DNS lookup and TCP, TLS, or QUIC handshakes [2][3]. If curl does not successfully connect within the time specified by --connect-timeout, it will exit with a timeout error (exit code 28) [3][4]. There is no default "timeout" value for --connect-timeout that forces a connection to fail; if the option is not used, curl relies on the operating system's default connection timeout [1][5]. Both options accept decimal values for sub-second precision (e.g., --max-time 0.5 or --connect-timeout 2.781) [1][5][4].

Citations:

Bound each route probe.

The retry counter advances only after curl returns. Without a default operation timeout, a stalled DNS lookup, connection, TLS handshake, or response can exceed the intended 120-second limit. Add per-request --connect-timeout and --max-time values, such as --connect-timeout 3 --max-time 5.

🤖 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/agent-qe/baremetal/install/ove/disconnected/run-validation-checkup/agent-qe-baremetal-install-ove-disconnected-run-validation-checkup-commands.sh`
at line 28, Update the curl invocation in the route probe to include bounded
connection and total request timeouts, using values such as --connect-timeout 3
and --max-time 5, so stalled DNS, connection, TLS, or response operations cannot
delay retry counter progress.

echo "HCO CLI download route is ready"
break
fi
sleep 10
COUNTER=$((COUNTER + 10))
echo "Waiting ${COUNTER}s for HCO CLI route to respond..."
done
else
echo "WARNING: HCO CLI download route not found, skipping readiness check"
fi
Comment on lines +26 to +38

@coderabbitai coderabbitai Bot Aug 10, 2026

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fail closed when the HCO CLI download route is not ready.

After COUNTER reaches 120, the loop exits and execution continues. No branch verifies that a probe succeeded. The validation flow can therefore proceed while the route still returns errors.

Track an explicit ROUTE_READY flag and exit with a clear error when the deadline expires. If continuing is intentional, make that fallback explicit and verify that the downstream validation flow supports it.

Proposed fix 
   COUNTER=0
+  ROUTE_READY=false
   while [ $COUNTER -lt 120 ]; do
     if curl -sk -o /dev/null -w '%{http_code}' "https://${CLI_ROUTE}/" 2>/dev/null | grep -qE '^(200|301|302)'; then
+      ROUTE_READY=true
       echo "HCO CLI download route is ready"
       break
@@
     echo "Waiting ${COUNTER}s for HCO CLI route to respond..."
   done
+  if [[ "${ROUTE_READY}" != true ]]; then
+    echo "ERROR: HCO CLI download route did not become ready within 120s"
+    exit 1
+  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.

Suggested change
COUNTER=0
while [ $COUNTER -lt 120 ]; do
if curl -sk -o /dev/null -w '%{http_code}' "https://${CLI_ROUTE}/" 2>/dev/null | grep -qE '^(200|301|302)'; then
echo "HCO CLI download route is ready"
break
fi
sleep 10
COUNTER=$((COUNTER + 10))
echo "Waiting ${COUNTER}s for HCO CLI route to respond..."
done
else
echo "WARNING: HCO CLI download route not found, skipping readiness check"
fi
COUNTER=0
ROUTE_READY=false
while [ $COUNTER -lt 120 ]; do
if curl -sk -o /dev/null -w '%{http_code}' "https://${CLI_ROUTE}/" 2>/dev/null | grep -qE '^(200|301|302)'; then
ROUTE_READY=true
echo "HCO CLI download route is ready"
break
fi
sleep 10
COUNTER=$((COUNTER + 10))
echo "Waiting ${COUNTER}s for HCO CLI route to respond..."
done
if [[ "${ROUTE_READY}" != true ]]; then
echo "ERROR: HCO CLI download route did not become ready within 120s"
exit 1
fi
else
echo "WARNING: HCO CLI download route not found, skipping readiness check"
fi
🧰 Tools
🪛 ast-grep (0.45.0)

[warning] 27-27: curl is invoked with -k/--insecure, which disables TLS certificate verification and exposes the connection to man-in-the-middle attacks. Remove the insecure flag and let curl validate the server certificate; if you need to trust a private CA, pin it with --cacert instead.
Context: curl -sk -o /dev/null -w '%{http_code}' "https://${CLI_ROUTE}/"
Note: [CWE-295] Improper Certificate Validation.

(curl-insecure-tls-bash)

🤖 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/agent-qe/baremetal/install/ove/disconnected/run-validation-checkup/agent-qe-baremetal-install-ove-disconnected-run-validation-checkup-commands.sh`
around lines 26 - 38, Update the HCO CLI route readiness loop around COUNTER to
track an explicit ROUTE_READY flag, set it only when the curl probe returns an
accepted status, and fail with a clear error after the 120-second deadline if it
remains unset. Ensure execution cannot continue into validation when the route
is unavailable.


echo "=== Discovering validation image from CNV CSV ==="
CSV_NAME=$(oc get csv -n "${TARGET_NAMESPACE}" -o json | \
jq -r '.items[] | select(.metadata.name | startswith("kubevirt-hyperconverged")).metadata.name')
Expand Down