feat: wire manifest readinessProbe to remote session controller by SalimKayal · Pull Request #1135 · SwissDataScienceCenter/amalthea

SalimKayal · 2026-06-10T14:10:17Z

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

Companion PR: feat: Use an HTTP readiness probe for remote interactive sessions renku-data-services#1350 selects the probe type per session; this PR acts on it in the sidecar. Merge/release together.

olevski

Thanks Salim. Just a few minor things. You can ignore the note about finding a free port if you wish.

olevski · 2026-06-15T12:09:29Z

+		{
+			name: "interactive tcp open port returns 200",
+			cfg: func() config.RemoteSessionControllerConfig {
+				c := baseCfg
+				c.ReadinessProbeType = string(amaltheadevv1alpha1.TCP)
+				c.SessionPort = 18000
+				return c
+			}(),
+			setupBackend: func() func() {
+				ln, err := net.Listen("tcp", "127.0.0.1:18000")
+				if err != nil {
+					panic(err)
+				}
+				return func() { _ = ln.Close() }
+			},
+			wantStatus: http.StatusOK,
+		},


This is a way to not hardcode the port to run the test server on.

func getFreePortOrDie() int { var a *net.TCPAddr var err error if a, err = net.ResolveTCPAddr("tcp", "localhost:0"); err == nil { var l *net.TCPListener if l, err = net.ListenTCP("tcp", a); err == nil { defer func() { err := l.Close() if err != nil { panic(err) } }() return l.Addr().(*net.TCPAddr).Port } panic(err) } panic(err) }

From https://github.com/SwissDataScienceCenter/renku-frontend-buildpacks/blob/main/tests/e2e/utils.go#L162C1-L179C2

thanks, added

leafty · 2026-06-15T13:22:12Z

+		return err
+	}
+
+	cmd.Flags().String(readinessProbeTypeFlag, "", "readiness probe type: none, tcp, or http")


Why is the default "" and not "none"?

leafty · 2026-06-15T13:23:29Z

+			if cfg.SessionPort == 0 {
+				return c.NoContent(http.StatusOK)
+			}


This should not happen in my opinion: using a port value of 0 is invalid. The default values should map to amaltheadevv1alpha1.None.

leafty · 2026-06-15T13:24:01Z

+		case string(amaltheadevv1alpha1.None):
+			return c.NoContent(http.StatusOK)


Since we also have a default case, you can remove the None case.

leafty · 2026-06-15T13:25:09Z

+		cfg.ReadinessProbeType != string(amaltheadevv1alpha1.TCP) &&
+		cfg.ReadinessProbeType != string(amaltheadevv1alpha1.HTTP) {
+		return fmt.Errorf("invalid readiness probe type: %s", cfg.ReadinessProbeType)
+	}


Is there any reason to not update the configuration to have none as the default? This way you can remove the defensive code in the endpoint implementation.

added default as none

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

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

Co-authored-by: Tasko Olevski <16360283+olevski@users.noreply.github.com>

Co-authored-by: Flora Thiebaut <flora@leafty.dev>

are the changes ok ?

SalimKayal requested review from leafty and sambuc June 10, 2026 14:10

SalimKayal requested review from a team and olevski as code owners June 10, 2026 14:10

SalimKayal mentioned this pull request Jun 10, 2026

feat: Use an HTTP readiness probe for remote interactive sessions SwissDataScienceCenter/renku-data-services#1350

Merged

SalimKayal force-pushed the salimkayal-feat-allow-readiness-probe-on-http branch 2 times, most recently from 589f4a5 to c09c795 Compare June 11, 2026 14:17

olevski previously requested changes Jun 15, 2026

View reviewed changes

leafty reviewed Jun 15, 2026

View reviewed changes

SalimKayal force-pushed the salimkayal-feat-allow-readiness-probe-on-http branch from 05e26bf to c5b20d9 Compare June 16, 2026 10:59

SalimKayal requested a review from olevski June 16, 2026 14:24

SalimKayal and others added 8 commits June 17, 2026 11:57

fix: set publishNotReadyAddresses on remote sessions

ed9ab0a

refactor: use request context for probes

b3e4324

Co-authored-by: Tasko Olevski <16360283+olevski@users.noreply.github.com>

fix: use none as default

d11035f

fix: tests for refactor

f350305

feat: get free port for server tests

05efe8a

squashme: match port type to crd

a107f62

SalimKayal force-pushed the salimkayal-feat-allow-readiness-probe-on-http branch from 3f6ecab to a107f62 Compare June 17, 2026 09:57

leafty reviewed Jun 17, 2026

View reviewed changes

Comment thread internal/remote/server/server.go Outdated

leafty reviewed Jun 17, 2026

View reviewed changes

Comment thread internal/remote/server/server.go Outdated

fix: remove case for SessionPort set to 0, fix comment

8658bbb

Co-authored-by: Flora Thiebaut <flora@leafty.dev>

SalimKayal force-pushed the salimkayal-feat-allow-readiness-probe-on-http branch from 6eafe8d to 8658bbb Compare June 17, 2026 10:34

leafty approved these changes Jun 17, 2026

View reviewed changes

SalimKayal merged commit 1488d77 into main Jun 17, 2026
8 checks passed

SalimKayal deleted the salimkayal-feat-allow-readiness-probe-on-http branch June 17, 2026 11:53

		case string(amaltheadevv1alpha1.None):
		return c.NoContent(http.StatusOK)

Uh oh!

Conversation

SalimKayal commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation and Context

Changes

Notes

Uh oh!

olevski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SalimKayal commented Jun 10, 2026 •

edited

Loading