fix(tests): probe / instead of /api for OpenShift port-forward readiness - #2685
Conversation
RHAIENG-6036 (08f4fe3) already fixed this correctly: probe / with a persistent session (cookies are required across RStudio's multi-hop redirect chain), because RStudio doesn't support /api the way Jupyter and code-server do. The hermetic-codeserver backport (da9540c) later silently reverted this hunk back to a bare 30s/localhost/no-session probe, and a same-day follow-up fix (7487358) only noticed the regressed timeout, re-fixing it by copying main's /api-only probe -- but main dropped RStudio entirely, so that path never worked here. Confirmed against real ghcr.io rhoai-2.25 images: RStudio's nginx redirects /api to a hardcoded http://127.0.0.1:8888/api/, which is unreachable through a port-forward where the local port isn't literally 8888. / works for rstudio, jupyter-minimal, and codeserver alike, but only with a redirect-following session (a bare GET redirect-loops on RStudio's auth flow without persisted cookies). See #2684 for the CI failure history this caused (rstudio/cuda-rstudio amd64 timing out on nearly every push). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe Kubernetes readiness check now probes the root URL and uses shared timeout constants. The container workflow adds matrix-based OpenShift tests with Podman, Kubernetes provisioning, GHCR authentication, JUnit output, and conditional Codecov uploads. ChangesContainer test execution
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
CI status [antigravity]Run: Build Notebooks (pr) #30715956755 — 2/2 complete · 1 passed · 1 skipped No workbench image jobs ran; all matrix jobs were skipped. |
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 `@tests/containers/kubernetes_utils.py`:
- Around line 277-285: Update the readiness flow around _ready and Wait.until so
a single requests.Session is created before polling, reused by every _ready
invocation, and closed after Wait.until completes, including when the wait
raises. Add a regression test covering cookie-dependent readiness to verify
repeated polls use the same session and eventually succeed.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 952a7542-eba9-4c99-99ba-7bb7103c49b0
📒 Files selected for processing (1)
tests/containers/kubernetes_utils.py
…tests The existing self-test workflow only runs `pytest tests/containers -m 'not openshift and not cuda and not rocm'`, explicitly skipping the OpenShift/Kubernetes deployment path (ImageDeployment / test_image_run_on_openshift). That's exactly the code path that silently regressed and went unnoticed for weeks (see #2684, #2685) -- nothing in this repo's CI actually exercised it outside of the full image build workflow. Add openshift-container-tests, reusing find-images' matrix, mirroring the provisioning steps build-notebooks-TEMPLATE.yaml already uses for the same purpose: rootful podman (install-podman-action, sharing image storage with cri-o) + a kubeadm cluster (provision-k8s), then `pytest -m 'openshift and not cuda and not rocm'` against the pulled image. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI status [antigravity]Run: Build Notebooks (push) #30716162558 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
CI status [antigravity]Run: Build Notebooks (pr) #30716163834 — 2/2 complete · 1 passed · 1 skipped No workbench image jobs ran; all matrix jobs were skipped. |
📋 Review SummaryThis pull request correctly addresses the RStudio/cuda-rstudio timeout issue on OpenShift by reverting to a session-based root ( 🔍 General Feedback
I did not find any issues requiring inline comments. |
install-podman-action's storage.conf enables partial-image pulls (enable_partial_images = "true"), which install-podman-action's own callers rely on for speed when podman build pulls base images. But this job instead `podman pull`s an already-published ghcr.io image, and ghcr.io's blob endpoint returns "501 Unsupported client range" for the resulting ranged fetch, failing the pull outright (observed on 3 consecutive CI runs of PR #2685): Error: unable to copy from source docker://ghcr.io/.../codeserver...: partial pull of blob sha256:...: read zstd:chunked manifest: fetching partial blob: received unexpected HTTP status: 501 Unsupported client range Disable it for this job only (not the shared ci/cached-builds/storage.conf, which other build jobs still benefit from) by patching /etc/containers/ storage.conf after install-podman-action runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI status [antigravity]Run: Build Notebooks (push) #30716642515 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
CI status [antigravity]Run: Build Notebooks (pr) #30716643675 — 2/2 complete · 1 passed · 1 skipped No workbench image jobs ran; all matrix jobs were skipped. |
Follow-up: why only
|
|
Tip For best results, initiate chat on the files or code changes.
Disabling 🐇🔍 ✏️ Learnings added
You are interacting with an AI system. |
ide-developer
left a comment
There was a problem hiding this comment.
Reviewed all three commits.
- RStudio readiness probe fix — restores the RHAIENG-6036 fix that got silently reverted by the codeserver-backport merge. The root cause (RStudio's nginx redirects
/apito a hardcodedhttp://127.0.0.1:8888/api/, unreachable through any port-forward) is well-documented in #2684 and verified empirically against real pulled images (jupyter-minimal, rstudio, codeserver), not just inferred from logs. - New openshift-container-tests job — closes the coverage gap that let the above regression go unnoticed for weeks; sensibly reuses the existing
find-imagesmatrix and the same provisioning building blocks (install-podman-action+provision-k8s) the main build workflow already relies on. - Disable partial zstd:chunked pulls for this job — real failure caught by the new job in its own first CI runs (codeserver 3/3), root-caused to a
501 Unsupported client rangeon ghcr.io's blob endpoint for ranged fetches, scoped narrowly to this job rather than touching the sharedstorage.conf.
All CI checks are green, including all 5 openshift: matrix legs. Nice find-and-fix loop — the new test job earning its keep on the very PR that introduces it is a good sign.
Approving.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ide-developer 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 |
…tests (#4259) * ci(test-containers): add a job to run the openshift-marked container tests The existing self-test workflow only runs `pytest tests/containers -m 'not openshift and not cuda and not rocm'`, explicitly skipping the OpenShift/Kubernetes deployment path (ImageDeployment / test_image_run_on_openshift). That's exactly the code path that silently regressed and went unnoticed for weeks (see red-hat-data-services#2684, red-hat-data-services#2685) -- nothing in this repo's CI actually exercised it outside of the full image build workflow. Add openshift-container-tests, reusing find-images' matrix, mirroring the provisioning steps build-notebooks-TEMPLATE.yaml already uses for the same purpose: rootful podman (install-podman-action, sharing image storage with cri-o) + a kubeadm cluster (provision-k8s), then `pytest -m 'openshift and not cuda and not rocm'` against the pulled image. Fixes #4257. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> (cherry picked from commit c342da8) * fix(ci): disable zstd:chunked partial pulls in openshift-container-tests install-podman-action's storage.conf enables partial-image pulls (enable_partial_images = "true"), which install-podman-action's own callers rely on for speed when podman build pulls base images. But this job instead `podman pull`s an already-published ghcr.io image, and ghcr.io's blob endpoint returns "501 Unsupported client range" for the resulting ranged fetch, failing the pull outright (observed on 3 consecutive CI runs of PR #2685): Error: unable to copy from source docker://ghcr.io/.../codeserver...: partial pull of blob sha256:...: read zstd:chunked manifest: fetching partial blob: received unexpected HTTP status: 501 Unsupported client range Disable it for this job only (not the shared ci/cached-builds/storage.conf, which other build jobs still benefit from) by patching /etc/containers/ storage.conf after install-podman-action runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> (cherry picked from commit a624f5a) * fix(ci): disable credential persistence in openshift-container-tests checkout CodeRabbit (artipacked): the new job's checkout step ran repository code (pytest) without persist-credentials: false, leaving GITHUB_TOKEN in the local git config where a compromised dependency or test could read and exfiltrate it. The job already authenticates to GHCR separately via docker/login-action, so nothing needs the persisted credential. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Three commits, in order:
fix(tests): probe / instead of /api for OpenShift port-forward readiness— restores a previously-working fix that got silently reverted, fixing the recurring RStudio/cuda-rstudiotest_image_run_on_openshifttimeout tracked in [rhoai-2.25] Flaky test_image_run_on_openshift: rstudio/cuda-rstudio (amd64) hit 'Connecting to pod succeeds' timeout on nearly every push #2684.ci(test-containers): add a job to run the openshift-marked container tests— addsopenshift-container-teststo the fast, PR-scoped self-test workflow (test-containers.yaml), so this exact code path gets exercised on every PR instead of only in the full, expensive image-build workflow. This is what would have caught both regressions described in commit 1 immediately instead of weeks later. See test-containers.yaml self-test doesn't exercise the openshift-marked tests (ImageDeployment/test_image_run_on_openshift) opendatahub-io/notebooks#4257 for the equivalent tracking issue onmain.fix(ci): disable zstd:chunked partial pulls in openshift-container-tests— fixes a failure the new job in commit 2 immediately hit in this PR's own CI (3 consecutive runs):podman pullof thecodeserverimage failed with501 Unsupported client range, because that image's layers happen to bezstd:chunked-compressed (a known buildah bug means whether a given build's layers end upzstd:chunkedvs. plaingzipdepends on--cache-from/--cache-tocache hit/miss, not on the image itself) andinstall-podman-action'sstorage.confenables partial/ranged fetches for such layers, whichghcr.io's blob endpoint doesn't support. Disabled for this job only.Commit 1: RStudio readiness probe fix
Restores a previously-working fix that got silently reverted, fixing the recurring RStudio/cuda-rstudio
test_image_run_on_openshifttimeout tracked in #2684.RHAIENG-6036(08f4fe374, merged 2026-07-11) fixed this correctly: probe/with a persistentrequests.Session()(cookies are required across RStudio's multi-hop redirect chain), because RStudio doesn't support/apithe way Jupyter and code-server do.da9540c47) later silently reverted this hunk back to a bare30s/localhost/no-session probe (almost certainly because that backport branch was cut before08f4fe374merged).74873588f) noticed only the regressed timeout and re-fixed it by copyingmain's/api-only probe — butmaindropped RStudio entirely, so that path never actually worked here.Evidence
Confirmed against real
ghcr.iorhoai-2.25images (jupyter-minimal, rstudio, codeserver) pulled and run directly:RStudio's nginx redirects
/apito a hardcodedhttp://127.0.0.1:8888/api/— its own internal container port, not the locally-forwarded port — so it's unreachable through any port-forward/proxy./(session-based, redirects followed) returns 200 for all three:(a bare GET without a persistent cookie jar redirect-loops on RStudio's
/, which is why the session matters, not just the path.)Commit 2: add openshift-container-tests to the self-test workflow
.github/workflows/test-containers.yamlonly ranpytest -m 'not openshift and not cuda and not rocm', explicitly skippingtest_image_run_on_openshift. That's exactly the code path that regressed twice without anyone noticing (commit 1's history). Adds a job that reusesfind-images's matrix and mirrorsbuild-notebooks-TEMPLATE.yaml's own provisioning steps (install-podman-action+provision-k8s) to runpytest -m 'openshift and not cuda and not rocm'against each pulled image in fast, PR-scoped CI.Commit 3: fix a pull failure the new job surfaced
The new job in commit 2 failed 3/3 times in this PR's own CI on
codeserverspecifically:codeserver's layers happened to be pushed aszstd:chunked(the other 4 images in the matrix are plaingzip) — this is a known buildah issue where--cache-to/--cache-fromused to ignore the configuredcompression_formatfor cache-derived (hit) layers, so which images end upzstd:chunkedvs.gzipdepends on build-cache hit/miss, not on the image itself.install-podman-action'sstorage.confenables partial/ranged pulls forzstd:chunkedlayers, andghcr.io's blob endpoint returns501for the resulting ranged request instead of falling back to a full pull. Disabledenable_partial_imagesfor this job only (not the sharedci/cached-builds/storage.conf, which other build jobs still benefit from).Test plan
ruff check/pyrightclean on the changed files/+ session works for jupyter-minimal, rstudio, and codeserver images pulled fromghcr.io(rhoai-2.25 build)actionlint/ YAML-valid ontest-containers.yamlopenshift-container-testspasses for all 5 images (rstudio/cuda-rstudio no longer time out; codeserver's partial-pull failure fixed) after commit 3🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests