Skip to content

feat: wire manifest readinessProbe to remote session controller#1135

Open
SalimKayal wants to merge 3 commits into
mainfrom
salimkayal-feat-allow-readiness-probe-on-http
Open

feat: wire manifest readinessProbe to remote session controller#1135
SalimKayal wants to merge 3 commits into
mainfrom
salimkayal-feat-allow-readiness-probe-on-http

Conversation

@SalimKayal

@SalimKayal SalimKayal commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

The AmaltheaSession CRD already exposes spec.session.readinessProbe, spec.session.port, and spec.session.urlPath. These fields were previously only honored for local sessions (through Kubernetes container probes). This PR forwards the same values into the remote session controller sidecar so its /ready HTTP endpoint reports readiness based on whether the actual remote session is reachable.

Motivation and Context

For remote sessions managed via FirecREST / RunAI / Slurm, the controller pod was always reporting ready immediately, even if the IDE on the remote compute node had not started yet. We need the Kubernetes readiness state to reflect real user-facing session availability so that ingress and service routing only start once the session is actually usable.

Changes

  • Injects RSC_SESSION_PORT, RSC_SESSION_URL_PATH, and RSC_READINESS_PROBE_TYPE as environment variables into the remote session controller container (sessionContainerRemote).
  • The sidecar's /ready handler now interprets these values:
    • none — immediately returns 200 OK.
    • tcp — attempts a TCP dial to 127.0.0.1:<RSC_SESSION_PORT> (the local wstunnel endpoint); returns 200 on success, 503 otherwise.
    • http — performs an HTTP GET to 127.0.0.1:<RSC_SESSION_PORT><RSC_SESSION_URL_PATH>; returns 200 on 2xx/3xx responses, 503 otherwise.
  • Fully backward compatible: an empty or unknown probe type, as well as port 0, fall back to the previous behavior of returning 200 OK.

Tests

  • Added TestConfigNewFlags covering CLI flags, environment variables, and validation of the readiness probe type.
  • Added TestReadyEndpoint covering all probe modes and edge cases: open/closed TCP ports, HTTP 200/302/500, connection refused, and port-0 fallback.

Compatibility

  • No CRD changes required — the fields already exist in amaltheasession_types.go.
  • Existing remote sessions without these values continue to behave exactly as before.

Notes

Scope: Core feature — the RSC /ready endpoint becomes aware of the remote session's serving state.

- api/v1alpha1/amaltheasession_children.go: inject RSC_SESSION_PORT, RSC_SESSION_URL_PATH, and RSC_READINESS_PROBE_TYPE into the RSC container
- internal/remote/config/config.go: add SessionPort, SessionURLPath, ReadinessProbeType to RemoteSessionControllerConfig; register flags/env vars
- internal/remote/server/server.go: rework /ready handler with switch on ReadinessProbeType (none/tcp/http/default)
- internal/remote/server/server_test.go: add TestReadyEndpoint with 11 test cases
@SalimKayal SalimKayal requested review from leafty and sambuc June 10, 2026 14:10
@SalimKayal SalimKayal requested review from a team and olevski as code owners June 10, 2026 14:10
Scope: Prevent invalid probe types from being silently accepted; test the new config surface.

- internal/remote/config/config.go: add ReadinessProbeType enum validation in Validate() (reject values other than "", none, tcp, http)
- internal/remote/config/config_test.go: add TestConfigNewFlags covering defaults, CLI flags, env vars (RSC_READINESS_PROBE_TYPE, RSC_SESSION_PORT, RSC_SESSION_URL_PATH), and validation errors
@SalimKayal SalimKayal force-pushed the salimkayal-feat-allow-readiness-probe-on-http branch from 589f4a5 to c09c795 Compare June 11, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant