Skip to content

fix(ci): harden GKE readiness via /healthcheck probe - #5215

Merged
openshift-merge-bot[bot] merged 1 commit into
redhat-developer:mainfrom
zdrapela:fix/rhdhbugs-3508-gke-healthcheck-main
Aug 5, 2026
Merged

fix(ci): harden GKE readiness via /healthcheck probe#5215
openshift-merge-bot[bot] merged 1 commit into
redhat-developer:mainfrom
zdrapela:fix/rhdhbugs-3508-gke-healthcheck-main

Conversation

@zdrapela

@zdrapela zdrapela commented Aug 5, 2026

Copy link
Copy Markdown
Member

Description

Slim CI-only forward-port of the /healthcheck readiness probe from #5211 (release-1.10) to fix RHDHBUGS-3508 without touching Playwright globalSetup.

  • Add testing::probe_rhdh_healthcheck (GET /healthcheck, curl --connect-timeout 5 --max-time 15, jq -e '.status == "ok"', _TESTING_LAST_HEALTH_DETAIL)
  • Switch testing::check_backstage_running from HEAD / to that probe
  • Warn-only pre-Playwright re-probe via common::retry (~30s) after yarn install

Complementary to #5083 (Paul): that PR owns Playwright wait-for-rhdh-ready / globalSetup. This PR owns only the shared CI shell gate. They do not conflict.

Which issue(s) does this PR fix

PR acceptance criteria

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing (N/A — shell only)
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (N/A)
  • Add a screenshot if the change is UX/UI related (N/A)

How to test changes / Special notes to the reviewer

Switch CI deploy readiness from HEAD / to GET /healthcheck with curl
timeouts, jq validation, and a warn-only pre-Playwright re-probe so
transient TLS disconnects (RHDHBUGS-3508) can be retried without
touching Playwright globalSetup (redhat-developer#5083).
@openshift-ci
openshift-ci Bot requested review from gustavolira and kadel August 5, 2026 07:48
@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Harden CI readiness checks using a validated /healthcheck probe

🐞 Bug fix 🕐 20-40 Minutes

Grey Divider

AI Description

• Switch CI readiness check from HEAD / to validated GET /healthcheck.
• Add curl+jq probe with timeouts and _TESTING_LAST_HEALTH_DETAIL diagnostics.
• Warn-only retry probe before Playwright to ride out transient GKE TLS drops.
Diagram

graph TD
  CI["CI E2E job"] --> TS["testing.sh"] --> CBS["check_backstage_running()"] --> PROBE["probe_rhdh_healthcheck()"] --> CURL["curl GET /healthcheck"] --> JQ["jq status==ok"]
  TS --> RETRY["common::retry()"] --> PROBE
  PROBE --> ENDPOINT[["RHDH /healthcheck"]]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Rely solely on Playwright globalSetup readiness
2. Gate on Kubernetes readiness (rollout/status/endpoints) instead of HTTP
  • ➕ Avoids TLS/application-layer flakiness entirely
  • ➕ Can detect readiness without external ingress exposure
  • ➖ Does not guarantee the public route/ingress is actually serving traffic
  • ➖ More cluster/provider-specific logic and edge cases (routes vs ingresses)

Recommendation: Keep the PR’s approach: a lightweight, CI-local HTTP probe of /healthcheck with strict timeouts and JSON validation, plus a warn-only retry before Playwright. It improves determinism for GKE/TLS flake scenarios while staying scoped to the shared shell gate and avoiding interference with the Playwright readiness work owned elsewhere.

Files changed (1) +54 / -9

Bug fix (1) +54 / -9
testing.shUse /healthcheck readiness probe with retries and JSON validation +54/-9

Use /healthcheck readiness probe with retries and JSON validation

• Adds a dedicated testing::probe_rhdh_healthcheck() helper that calls GET /healthcheck with curl connect/max timeouts and validates the JSON response via jq, recording failure details in _TESTING_LAST_HEALTH_DETAIL. Switches testing::check_backstage_running() to use this probe (instead of HEAD /) and improves log messages accordingly. Adds a warn-only, post-yarn-install re-probe using common::retry to smooth transient TLS disconnects before Playwright starts.

.ci/pipelines/lib/testing.sh

@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.96%. Comparing base (e3b3f0b) to head (030160d).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5215      +/-   ##
==========================================
- Coverage   63.69%   59.96%   -3.74%     
==========================================
  Files         123      111      -12     
  Lines        2424     2198     -226     
  Branches      573      522      -51     
==========================================
- Hits         1544     1318     -226     
- Misses        878      879       +1     
+ Partials        2        1       -1     
Flag Coverage Δ
rhdh 59.96% <ø> (-3.74%) ⬇️
Components Coverage Δ
Backend plugins ∅ <ø> (∅)
Backend app 66.66% <ø> (ø)
Frontend app 58.89% <ø> (ø)
Plugin utils ∅ <ø> (∅)
Dynamic plugins utils ∅ <ø> (∅)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e3b3f0b...030160d. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rhdh-qodo-merge

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Warn-only retry logs errors 🐞 Bug ◔ Observability
Description
testing::run_tests intends the pre-Playwright /healthcheck re-probe to be warn-only, but it calls
common::retry which emits log::error when retries are exhausted. This produces ERROR logs even
though the script continues to run tests, making CI output misleading.
Code

.ci/pipelines/lib/testing.sh[R88-92]

+    if common::retry "${health_retries}" "${health_backoff_seconds}" testing::probe_rhdh_healthcheck "${url}"; then
+      log::success "Pre-Playwright /healthcheck OK"
+    else
+      log::warn "Pre-Playwright /healthcheck still flaky after ~$((health_retries * health_backoff_seconds))s; continuing to tests"
+    fi
Relevance

●●● Strong

Team tends to align CI output semantics; reducing misleading ERROR logs is a straightforward
observability fix.

PR-#5027

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new pre-Playwright check continues on failure (warn-only), but the retry helper unconditionally
logs errors when it gives up, creating contradictory severity in the logs.

.ci/pipelines/lib/testing.sh[84-93]
.ci/pipelines/lib/common.sh[179-203]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`testing::run_tests` performs a warn-only pre-Playwright healthcheck retry, but it uses `common::retry`, which always logs `ERROR` when it exhausts attempts. This results in non-fatal flakiness being logged as errors.

### Issue Context
The caller explicitly continues to tests after the retry fails, so the retry helper should either be configurable (warn-only) or the call site should use a quieter retry loop.

### Fix Focus Areas
- .ci/pipelines/lib/testing.sh[84-93]
- .ci/pipelines/lib/common.sh[179-203]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Probe hides curl/jq errors 🐞 Bug ☼ Reliability
Description
testing::probe_rhdh_healthcheck discards curl stderr and masks curl failures with || true,
collapsing distinct transport problems (DNS/TLS/timeout/missing binary) into generic HTTP 000. It
also suppresses jq diagnostics, making readiness failures harder to troubleshoot and potentially
wasting many retry attempts.
Code

.ci/pipelines/lib/testing.sh[R315-318]

+  # Append http_code on its own line so connect/TLS failures can be retried.
+  # Connect failures typically yield "\n000"; bounds avoid hung probes.
+  response=$(curl --insecure -s --connect-timeout 5 --max-time 15 -w "\n%{http_code}" "${health_url}" 2> /dev/null || true)
+
Relevance

●● Moderate

Better curl/jq diagnostics helps debugging, but team sometimes prefers suppressing noisy logs in CI
scripts.

PR-#4267
PR-#4798

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The probe explicitly discards stderr and ignores curl’s exit status, then maps failures into coarse
_TESTING_LAST_HEALTH_DETAIL values, which removes actionable debugging context from readiness
gating.

.ci/pipelines/lib/testing.sh[310-333]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The healthcheck probe suppresses curl/jq diagnostics (`2> /dev/null`, `> /dev/null 2>&1`) and also forces curl success with `|| true`. This prevents callers/logs from distinguishing between HTTP-not-ready vs. transport failures vs. tooling issues.

### Issue Context
The probe already exposes `_TESTING_LAST_HEALTH_DETAIL`; it can be extended to include a short, safe reason (e.g., curl exit code, timeout vs. TLS) without dumping full bodies.

### Fix Focus Areas
- .ci/pipelines/lib/testing.sh[310-333]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. Retry duration message underestimates 🐞 Bug ➹ Performance
Description
The pre-Playwright warning message estimates total retry time as retries*backoff (30s), but each
probe can also run up to 15s due to curl --max-time 15. This makes the logged duration misleading;
worst-case wall time is ~6*15s + 5*5s = 115s.
Code

.ci/pipelines/lib/testing.sh[R90-92]

+    else
+      log::warn "Pre-Playwright /healthcheck still flaky after ~$((health_retries * health_backoff_seconds))s; continuing to tests"
+    fi
Relevance

●● Moderate

Purely improving an approximate log message can be seen as nit; similar “clarify log” feedback was
rejected before.

PR-#4798

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The message uses only backoff math, while the probe has a 15s max-time and the retry helper adds
sleeps between attempts, so wall time can be far higher than the message indicates.

.ci/pipelines/lib/testing.sh[84-93]
.ci/pipelines/lib/testing.sh[315-318]
.ci/pipelines/lib/common.sh[179-203]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The log message after a failed pre-Playwright retry claims an approximate duration that ignores the per-attempt probe time (`curl --max-time 15`). This misrepresents how long the script may have actually waited.

### Issue Context
`common::retry` sleeps between attempts but also includes the execution time of the command itself; `testing::probe_rhdh_healthcheck` can consume up to 15s per attempt.

### Fix Focus Areas
- .ci/pipelines/lib/testing.sh[84-93]
- .ci/pipelines/lib/testing.sh[315-318]
- .ci/pipelines/lib/common.sh[179-203]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials
✅ Compliance rules (platform): 46 rules
✅ Cross-repo context
  Not relevant to this PR: redhat-developer/rhdh-plugins
  Not relevant to this PR: redhat-developer/rhdh-chart
  Not relevant to this PR: redhat-developer/rhdh-operator
  Not relevant to this PR: redhat-developer/rhdh-local

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Image was built and published successfully. It is available at:

@zdrapela

zdrapela commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm

@zdrapela

zdrapela commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

/test e2e-gke-helm-nightly

@openshift-ci openshift-ci Bot added the lgtm label Aug 5, 2026
@openshift-merge-bot
openshift-merge-bot Bot merged commit 98ed929 into redhat-developer:main Aug 5, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants