Skip to content

[Fix] Gate Jupyter Ready on HTTP /lab serving - #306

Open
royischoss wants to merge 4 commits into
mlrun:developmentfrom
royischoss:ceml-730-jupyter-fix
Open

[Fix] Gate Jupyter Ready on HTTP /lab serving#306
royischoss wants to merge 4 commits into
mlrun:developmentfrom
royischoss:ceml-730-jupyter-fix

Conversation

@royischoss

@royischoss royischoss commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

📝 Description

On-prem CE installs could report success while mlrun-jupyter was still not serving HTTP.

The pod was marked Ready in ~1s because the CE chart had no probes, even though mlce-start.sh can take minutes to extract basehome.tar before Jupyter binds :8888.

Downstream naipi then hit /lab and got 502 Bad Gateway.

This PR adds an HTTP readinessProbe on GET /lab so Kubernetes Ready and helm --wait only succeed once Jupyter is actually serving. A startupProbe/livenessProbe combo was tried first but dropped in favor of readinessProbe-only — readiness alone holds the pod out of rotation during the slow cold-start extract without risking a restart loop if extraction runs long.

Complements the ML-12950 jupyter image fix that reduces cold-start extract time


🛠️ Changes Made

charts/mlrun-ce/values.yaml — added configurable jupyterNotebook.readinessProbe defaults (HTTP GET /lab on port http, periodSeconds 10 / timeoutSeconds 5 / failureThreshold 3). Note: startupProbe/livenessProbe value blocks are still present in values.yaml but are no longer wired into the deployment (see below) — left as-is, can be removed in a follow-up if not needed.
charts/mlrun-ce/templates/jupyter-notebook/deployment.yaml — wired the readinessProbe into the jupyter-notebook container
charts/mlrun-ce/Chart.yaml — bumped version 0.12.0-rc.6 → 0.12.0-rc.7


✅ Checklist

  • I have tested the changes in this PR
  • I confirmed whether my changes require a change in documentation and if so, I created another PR in MLRun for the relevant documentation.
  • I confirmed whether my changes require a changes in QA tests, for example: credentials changes, resources naming change and if so, I updated the relevant Jira ticket for QA.
  • I increased the Chart version in charts/mlrun-ce/Chart.yaml.
  • I confirmed that the installation works both on a local Docker Desktop environment and on a real cluster when using the required prerequisites.
    • If installation issues were found, I updated the relevant Jira ticket with the issue and steps to reproduce, or updated the prerequisites documentation if the issue is related to missing or outdated prerequisites.
  • If needed, update https://github.com/mlrun/ce/blob/development/charts/mlrun-ce/README.md with the relevant installation instructions and version Matrix.
  • If needed, update the following values files for multi namespace support:

🧪 Testing

helm template mlrun charts/mlrun-ce -f charts/mlrun-ce/values.yaml --show-only templates/jupyter-notebook/deployment.yaml — probe renders with expected path/port/thresholds
./tests/helm-template-test.sh — 82/82 passed

Real-cluster verification (2026-08-06, vmdev137ig4 / 192.168.236.51): packaged the chart locally (make package, mlrun-ce-0.12.0-rc.7.tgz), scp'd it to the lab, removed the prior mlrun-ce release + PVCs, then helm install my-mlrun <tgz> -n mlrun --wait --timeout 15m. Install completed successfully. mlrun-jupyter pod: container started, then took ~2m29s (19 failed readiness probes on /lab, connection refused while mlce-start.sh extracted basehome.tar) before passing readiness and going 1/1 Ready — with 0 restarts throughout. Confirms the fix: helm --wait now blocks until Jupyter is actually serving, and the pod isn't killed while cold-starting.

Not yet re-verified on local Docker Desktop.


🔗 References

Ticket link: https://ecliptos.atlassian.net/browse/CEML-730
Related: https://ecliptos.atlassian.net/browse/ML-12950 (jupyter basehome.tar shrink, mlrun#9990)
Related: #304 (TimescaleDB startupProbe precedent)
External links: #336 (progressDeadlineSeconds — exposed this latent gap)


🚨 Breaking Changes?

  • Yes (explain below)
  • No

🔍️ Additional Notes

values.yaml still declares jupyterNotebook.startupProbe and jupyterNotebook.livenessProbe blocks that are no longer referenced by the template (removed from deployment.yaml in a follow-up commit on this branch). Left in place for now since they're harmless unused config, but worth pruning in a follow-up if we're confident readiness-only is the final shape.

@github-actions github-actions Bot added the bug Something isn't working label Aug 4, 2026
@royischoss
royischoss marked this pull request as ready for review August 4, 2026 13:40
Comment thread charts/mlrun-ce/values.yaml Outdated
Comment on lines +345 to +351
livenessProbe:
httpGet:
path: /lab
port: http
periodSeconds: 30
timeoutSeconds: 5
failureThreshold: 6

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note that if jupyter doesnt answer within this time - it means it kills the pod completely. perhaps make it more graceful

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@tal-haim
tal-haim self-requested a review August 5, 2026 11:25
Comment thread charts/mlrun-ce/values.yaml Outdated
port: http
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 3

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

one small, maybe worth increasing to (1056) = 300 ~ 5m

@GiladShapira94

Copy link
Copy Markdown
Collaborator

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants