From 851e509b2b3e6932933620d14522306672b8e042 Mon Sep 17 00:00:00 2001 From: Salim Kayal Date: Wed, 10 Jun 2026 15:55:19 +0200 Subject: [PATCH 1/9] feat: add tunable readiness probe to RSC /ready endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- api/v1alpha1/amaltheasession_children.go | 12 ++ internal/remote/config/config.go | 45 ++++- internal/remote/server/server.go | 50 +++++- internal/remote/server/server_test.go | 206 +++++++++++++++++++++++ 4 files changed, 308 insertions(+), 5 deletions(-) create mode 100644 internal/remote/server/server_test.go diff --git a/api/v1alpha1/amaltheasession_children.go b/api/v1alpha1/amaltheasession_children.go index 5e8d9a8f..0f4a763d 100644 --- a/api/v1alpha1/amaltheasession_children.go +++ b/api/v1alpha1/amaltheasession_children.go @@ -991,6 +991,18 @@ func (cr *AmaltheaSession) sessionContainerRemote(volumeMounts []v1.VolumeMount) Name: "RSC_SERVER_PORT", Value: fmt.Sprintf("%d", RemoteSessionControllerPort), }, + v1.EnvVar{ + Name: "RSC_SESSION_PORT", + Value: fmt.Sprintf("%d", cr.Spec.Session.Port), + }, + v1.EnvVar{ + Name: "RSC_SESSION_URL_PATH", + Value: cr.Spec.Session.URLPath, + }, + v1.EnvVar{ + Name: "RSC_READINESS_PROBE_TYPE", + Value: string(cr.Spec.Session.ReadinessProbe.Type), + }, v1.EnvVar{ Name: "RSC_WSTUNNEL_SECRET", ValueFrom: ptr.To(v1.EnvVarSource{ diff --git a/internal/remote/config/config.go b/internal/remote/config/config.go index e14eb722..5310952b 100644 --- a/internal/remote/config/config.go +++ b/internal/remote/config/config.go @@ -18,6 +18,8 @@ limitations under the License. package config import ( + "fmt" + amaltheadevv1alpha1 "github.com/SwissDataScienceCenter/amalthea/api/v1alpha1" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -35,8 +37,11 @@ const ( ) const ( - serverPortFlag = "server-port" - fakeStartFlag = "fake-start" + serverPortFlag = "server-port" + fakeStartFlag = "fake-start" + sessionPortFlag = "session-port" + sessionURLPathFlag = "session-url-path" + readinessProbeTypeFlag = "readiness-probe-type" ) type RemoteSessionControllerConfig struct { @@ -53,6 +58,15 @@ type RemoteSessionControllerConfig struct { // FakeStart if true, do not start the remote session and print debug information FakeStart bool + + // SessionPort is the port where the remote session is expected to be serving + SessionPort int32 + + // SessionURLPath is the URL path for the HTTP readiness probe + SessionURLPath string + + // ReadinessProbeType is "none", "tcp", or "http" + ReadinessProbeType string } func SetFlags(cmd *cobra.Command) error { @@ -72,6 +86,30 @@ func SetFlags(cmd *cobra.Command) error { return err } + cmd.Flags().Int32(sessionPortFlag, 0, "port the remote session is expected to be serving on") + if err := viper.BindPFlag(sessionPortFlag, cmd.Flags().Lookup(sessionPortFlag)); err != nil { + return err + } + if err := viper.BindEnv(sessionPortFlag, configUtils.AsEnvVarFlag(sessionPortFlag)); err != nil { + return err + } + + cmd.Flags().String(sessionURLPathFlag, "/", "URL path for the HTTP readiness probe") + if err := viper.BindPFlag(sessionURLPathFlag, cmd.Flags().Lookup(sessionURLPathFlag)); err != nil { + return err + } + if err := viper.BindEnv(sessionURLPathFlag, configUtils.AsEnvVarFlag(sessionURLPathFlag)); err != nil { + return err + } + + cmd.Flags().String(readinessProbeTypeFlag, "", "readiness probe type: none, tcp, or http") + if err := viper.BindPFlag(readinessProbeTypeFlag, cmd.Flags().Lookup(readinessProbeTypeFlag)); err != nil { + return err + } + if err := viper.BindEnv(readinessProbeTypeFlag, configUtils.AsEnvVarFlag(readinessProbeTypeFlag)); err != nil { + return err + } + // Set up shared flags if err := configUtils.SetFlags(cmd); err != nil { return err @@ -97,6 +135,9 @@ func GetConfig() (cfg RemoteSessionControllerConfig, err error) { cfg.ServerPort = viper.GetInt32(serverPortFlag) cfg.FakeStart = viper.GetBool(fakeStartFlag) + cfg.SessionPort = viper.GetInt32(sessionPortFlag) + cfg.SessionURLPath = viper.GetString(sessionURLPathFlag) + cfg.ReadinessProbeType = viper.GetString(readinessProbeTypeFlag) return cfg, nil } diff --git a/internal/remote/server/server.go b/internal/remote/server/server.go index 363cf237..5a51ece3 100644 --- a/internal/remote/server/server.go +++ b/internal/remote/server/server.go @@ -20,11 +20,13 @@ import ( "context" "fmt" "log/slog" + "net" "net/http" "os" "os/signal" "time" + amaltheadevv1alpha1 "github.com/SwissDataScienceCenter/amalthea/api/v1alpha1" "github.com/SwissDataScienceCenter/amalthea/internal/common" "github.com/SwissDataScienceCenter/amalthea/internal/remote/config" "github.com/SwissDataScienceCenter/amalthea/internal/remote/controller" @@ -56,7 +58,7 @@ func Start() { os.Exit(1) } - server := newServer(controller) + server := newServer(controller, cfg) address := fmt.Sprintf(":%d", cfg.ServerPort) @@ -95,7 +97,7 @@ func Start() { var logLevel *slog.LevelVar = new(slog.LevelVar) var jsonLogger *slog.Logger = slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: logLevel})) -func newServer(controller controller.RemoteSessionController) (server *echo.Echo) { +func newServer(controller controller.RemoteSessionController, cfg config.RemoteSessionControllerConfig) (server *echo.Echo) { e := echo.New() e.HideBanner = true @@ -114,7 +116,49 @@ func newServer(controller controller.RemoteSessionController) (server *echo.Echo // Readiness endpoint e.GET("/ready", func(c echo.Context) error { - return c.NoContent(http.StatusOK) + switch cfg.ReadinessProbeType { + case string(amaltheadevv1alpha1.None): + return c.NoContent(http.StatusOK) + case string(amaltheadevv1alpha1.TCP): + if cfg.SessionPort == 0 { + return c.NoContent(http.StatusOK) + } + conn, err := net.DialTimeout("tcp", fmt.Sprintf("127.0.0.1:%d", cfg.SessionPort), 5*time.Second) + if err != nil { + return c.NoContent(http.StatusServiceUnavailable) + } + if err := conn.Close(); err != nil { + slog.Error("failed to close readiness probe connection", "error", err) + } + return c.NoContent(http.StatusOK) + case string(amaltheadevv1alpha1.HTTP): + if cfg.SessionPort == 0 { + return c.NoContent(http.StatusOK) + } + client := &http.Client{ + Timeout: 5 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } + url := fmt.Sprintf("http://127.0.0.1:%d%s", cfg.SessionPort, cfg.SessionURLPath) + resp, err := client.Get(url) + if err != nil { + return c.NoContent(http.StatusServiceUnavailable) + } + defer func() { + if err := resp.Body.Close(); err != nil { + slog.Error("failed to close readiness probe response body", "error", err) + } + }() + if resp.StatusCode >= 200 && resp.StatusCode < 400 { + return c.NoContent(http.StatusOK) + } + return c.NoContent(http.StatusServiceUnavailable) + default: + // Unconfigured / unknown: preserve old behavior for backward compatibility + return c.NoContent(http.StatusOK) + } }) // Status endpoint diff --git a/internal/remote/server/server_test.go b/internal/remote/server/server_test.go new file mode 100644 index 00000000..79cfdbe6 --- /dev/null +++ b/internal/remote/server/server_test.go @@ -0,0 +1,206 @@ +package server + +import ( + "context" + "net" + "net/http" + "net/http/httptest" + "testing" + + amaltheadevv1alpha1 "github.com/SwissDataScienceCenter/amalthea/api/v1alpha1" + "github.com/SwissDataScienceCenter/amalthea/internal/remote/config" + "github.com/SwissDataScienceCenter/amalthea/internal/remote/models" + "github.com/stretchr/testify/assert" +) + +type mockController struct{} + +func (m *mockController) Status(ctx context.Context) (models.RemoteSessionState, error) { + return models.Running, nil +} +func (m *mockController) Start(ctx context.Context) error { return nil } +func (m *mockController) Stop(ctx context.Context) error { return nil } + +func TestReadyEndpoint(t *testing.T) { + baseCfg := config.RemoteSessionControllerConfig{ + ServerPort: 65532, + SessionPort: 0, + SessionURLPath: "/", + ReadinessProbeType: "", + } + + tests := []struct { + name string + cfg config.RemoteSessionControllerConfig + setupBackend func() (cleanup func()) + wantStatus int + }{ + { + name: "none probe returns 200", + cfg: func() config.RemoteSessionControllerConfig { + c := baseCfg + c.ReadinessProbeType = string(amaltheadevv1alpha1.None) + return c + }(), + wantStatus: http.StatusOK, + }, + { + name: "interactive with empty probe type defaults to 200", + cfg: func() config.RemoteSessionControllerConfig { + c := baseCfg + c.ReadinessProbeType = "" + return c + }(), + wantStatus: http.StatusOK, + }, + { + 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, + }, + { + name: "interactive tcp closed port returns 503", + cfg: func() config.RemoteSessionControllerConfig { + c := baseCfg + c.ReadinessProbeType = string(amaltheadevv1alpha1.TCP) + c.SessionPort = 18001 + return c + }(), + wantStatus: http.StatusServiceUnavailable, + }, + { + name: "interactive tcp port 0 defaults to 200", + cfg: func() config.RemoteSessionControllerConfig { + c := baseCfg + c.ReadinessProbeType = string(amaltheadevv1alpha1.TCP) + c.SessionPort = 0 + return c + }(), + wantStatus: http.StatusOK, + }, + { + name: "interactive http serving 200 returns 200", + cfg: func() config.RemoteSessionControllerConfig { + c := baseCfg + c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) + c.SessionPort = 18002 + c.SessionURLPath = "/lab" + return c + }(), + setupBackend: func() func() { + ln, err := net.Listen("tcp", "127.0.0.1:18002") + if err != nil { + panic(err) + } + srv := &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/lab" { + w.WriteHeader(http.StatusOK) + return + } + w.WriteHeader(http.StatusNotFound) + }), + } + go func() { _ = srv.Serve(ln) }() + return func() { _ = srv.Close() } + }, + wantStatus: http.StatusOK, + }, + { + name: "interactive http redirect 302 returns 200", + cfg: func() config.RemoteSessionControllerConfig { + c := baseCfg + c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) + c.SessionPort = 18003 + c.SessionURLPath = "/" + return c + }(), + setupBackend: func() func() { + ln, err := net.Listen("tcp", "127.0.0.1:18003") + if err != nil { + panic(err) + } + srv := &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "/other", http.StatusFound) + }), + } + go func() { _ = srv.Serve(ln) }() + return func() { _ = srv.Close() } + }, + wantStatus: http.StatusOK, + }, + { + name: "interactive http serving 500 returns 503", + cfg: func() config.RemoteSessionControllerConfig { + c := baseCfg + c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) + c.SessionPort = 18004 + c.SessionURLPath = "/" + return c + }(), + setupBackend: func() func() { + ln, err := net.Listen("tcp", "127.0.0.1:18004") + if err != nil { + panic(err) + } + srv := &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }), + } + go func() { _ = srv.Serve(ln) }() + return func() { _ = srv.Close() } + }, + wantStatus: http.StatusServiceUnavailable, + }, + { + name: "interactive http connection refused returns 503", + cfg: func() config.RemoteSessionControllerConfig { + c := baseCfg + c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) + c.SessionPort = 18005 + c.SessionURLPath = "/" + return c + }(), + wantStatus: http.StatusServiceUnavailable, + }, + { + name: "interactive http port 0 defaults to 200", + cfg: func() config.RemoteSessionControllerConfig { + c := baseCfg + c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) + c.SessionPort = 0 + return c + }(), + wantStatus: http.StatusOK, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.setupBackend != nil { + cleanup := tt.setupBackend() + defer cleanup() + } + + e := newServer(&mockController{}, tt.cfg) + req := httptest.NewRequest(http.MethodGet, "/ready", nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + assert.Equal(t, tt.wantStatus, rec.Code) + }) + } +} From fe627d902bcc2022d896809622780744526aee02 Mon Sep 17 00:00:00 2001 From: Salim Kayal Date: Wed, 10 Jun 2026 16:26:55 +0200 Subject: [PATCH 2/9] test: add config validation and flag coverage 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 --- internal/remote/config/config.go | 7 ++ internal/remote/config/config_test.go | 110 ++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/internal/remote/config/config.go b/internal/remote/config/config.go index 5310952b..d2461b97 100644 --- a/internal/remote/config/config.go +++ b/internal/remote/config/config.go @@ -143,6 +143,13 @@ func GetConfig() (cfg RemoteSessionControllerConfig, err error) { } func (cfg *RemoteSessionControllerConfig) Validate() error { + if cfg.ReadinessProbeType != "" && + cfg.ReadinessProbeType != string(amaltheadevv1alpha1.None) && + cfg.ReadinessProbeType != string(amaltheadevv1alpha1.TCP) && + cfg.ReadinessProbeType != string(amaltheadevv1alpha1.HTTP) { + return fmt.Errorf("invalid readiness probe type: %s", cfg.ReadinessProbeType) + } + // FireCREST has priority over Runai cfg.RemoteKind = RemoteKindFirecrest firecrestConfigErr := cfg.Firecrest.Validate() diff --git a/internal/remote/config/config_test.go b/internal/remote/config/config_test.go index 77e541f3..e365e690 100644 --- a/internal/remote/config/config_test.go +++ b/internal/remote/config/config_test.go @@ -19,6 +19,7 @@ package config import ( "testing" + amaltheadevv1alpha1 "github.com/SwissDataScienceCenter/amalthea/api/v1alpha1" "github.com/spf13/cobra" "github.com/spf13/viper" "github.com/stretchr/testify/assert" @@ -86,3 +87,112 @@ func TestConfig(t *testing.T) { }) } } + +func TestConfigNewFlags(t *testing.T) { + tests := []struct { + name string + args []string + env map[string]string + wantProbeType string + wantSessionPort int32 + wantURLPath string + wantErr bool + }{ + { + name: "default with empty readiness probe", + args: []string{ + "--firecrest-api-url=https://firecrest.example.com", + "--firecrest-system-name=test-system", + "--auth-kind=client_credentials", + "--auth-token-uri=https://auth.example.com/token", + "--auth-firecrest-client-id=my-client", + "--auth-firecrest-client-secret=my-secret", + }, + wantProbeType: "", + wantSessionPort: 0, + wantURLPath: "/", + }, + { + name: "custom flags via CLI", + args: []string{ + "--readiness-probe-type=tcp", + "--session-port=8888", + "--session-url-path=/lab", + "--runai-base-url=https://runai.example.com", + "--runai-project=my-project", + "--auth-kind=client_credentials", + "--auth-token-uri=https://runai-auth.example.com/token", + "--auth-runai-client-id=runai-client", + "--auth-runai-client-secret=runai-secret", + }, + wantProbeType: string(amaltheadevv1alpha1.TCP), + wantSessionPort: 8888, + wantURLPath: "/lab", + }, + { + name: "custom flags via environment variables", + args: []string{ + "--firecrest-api-url=https://firecrest.example.com", + "--firecrest-system-name=test-system", + "--auth-kind=client_credentials", + "--auth-token-uri=https://auth.example.com/token", + "--auth-firecrest-client-id=my-client", + "--auth-firecrest-client-secret=my-secret", + }, + env: map[string]string{ + "RSC_READINESS_PROBE_TYPE": "http", + "RSC_SESSION_PORT": "9999", + "RSC_SESSION_URL_PATH": "/jupyter", + }, + wantProbeType: string(amaltheadevv1alpha1.HTTP), + wantSessionPort: 9999, + wantURLPath: "/jupyter", + }, + { + name: "invalid readiness probe type errors", + args: []string{ + "--readiness-probe-type=invalid", + "--firecrest-api-url=https://firecrest.example.com", + "--firecrest-system-name=test-system", + "--auth-kind=client_credentials", + "--auth-token-uri=https://auth.example.com/token", + "--auth-firecrest-client-id=my-client", + "--auth-firecrest-client-secret=my-secret", + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + viper.Reset() + for k, v := range tt.env { + t.Setenv(k, v) + } + + cmd := &cobra.Command{ + Use: "test", + Run: func(cmd *cobra.Command, args []string) {}, + } + err := SetFlags(cmd) + require.NoError(t, err) + + cmd.SetArgs(tt.args) + err = cmd.Execute() + require.NoError(t, err) + + cfg, err := GetConfig() + require.NoError(t, err) + + err = cfg.Validate() + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantProbeType, cfg.ReadinessProbeType) + assert.Equal(t, tt.wantSessionPort, cfg.SessionPort) + assert.Equal(t, tt.wantURLPath, cfg.SessionURLPath) + }) + } +} From ed9ab0a81a700579211a38f6fa05da1089057369 Mon Sep 17 00:00:00 2001 From: Salim Kayal Date: Thu, 11 Jun 2026 15:30:47 +0200 Subject: [PATCH 3/9] fix: set publishNotReadyAddresses on remote sessions --- api/v1alpha1/amaltheasession_children.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/api/v1alpha1/amaltheasession_children.go b/api/v1alpha1/amaltheasession_children.go index 0f4a763d..4dc959df 100644 --- a/api/v1alpha1/amaltheasession_children.go +++ b/api/v1alpha1/amaltheasession_children.go @@ -282,6 +282,10 @@ func (cr *AmaltheaSession) Service() v1.Service { }, } if cr.Spec.SessionLocation == Remote { + // NOTE: In order to connect through the reverse tunnel, + // we need to have the tunnel established so we publish + // the service without the pod being ready + svc.Spec.PublishNotReadyAddresses = true svc.Spec.Ports = append(svc.Spec.Ports, v1.ServicePort{ Protocol: v1.ProtocolTCP, Name: tunnelServiceName, From b3e432408e0c46c5fb52fd61c4a13e701e73be39 Mon Sep 17 00:00:00 2001 From: Salim Kayal Date: Tue, 16 Jun 2026 12:12:14 +0200 Subject: [PATCH 4/9] refactor: use request context for probes Co-authored-by: Tasko Olevski <16360283+olevski@users.noreply.github.com> --- internal/remote/server/server.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/internal/remote/server/server.go b/internal/remote/server/server.go index 5a51ece3..425029f8 100644 --- a/internal/remote/server/server.go +++ b/internal/remote/server/server.go @@ -123,7 +123,11 @@ func newServer(controller controller.RemoteSessionController, cfg config.RemoteS if cfg.SessionPort == 0 { return c.NoContent(http.StatusOK) } - conn, err := net.DialTimeout("tcp", fmt.Sprintf("127.0.0.1:%d", cfg.SessionPort), 5*time.Second) + dialer := net.Dialer{} + dialCtx, dialCtxCancel := context.WithTimeout(c.Request().Context(), 5*time.Second) + defer dialCtxCancel() + conn, err := dialer.DialContext(dialCtx, "tcp", fmt.Sprintf("127.0.0.1:%d", cfg.SessionPort)) + if err != nil { return c.NoContent(http.StatusServiceUnavailable) } @@ -142,7 +146,11 @@ func newServer(controller controller.RemoteSessionController, cfg config.RemoteS }, } url := fmt.Sprintf("http://127.0.0.1:%d%s", cfg.SessionPort, cfg.SessionURLPath) - resp, err := client.Get(url) + req, err := http.NewRequestWithContext(c.Request().Context(), "GET", url, nil) + if err != nil { + return c.NoContent(http.StatusServiceUnavailable) + } + resp, err := client.Do(req) if err != nil { return c.NoContent(http.StatusServiceUnavailable) } From d11035f7121b452cb2d5388d75b11eeb8f09f334 Mon Sep 17 00:00:00 2001 From: Salim Kayal Date: Tue, 16 Jun 2026 12:16:40 +0200 Subject: [PATCH 5/9] fix: use none as default --- internal/remote/config/config.go | 5 ++--- internal/remote/server/server.go | 5 ----- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/internal/remote/config/config.go b/internal/remote/config/config.go index d2461b97..c14af31d 100644 --- a/internal/remote/config/config.go +++ b/internal/remote/config/config.go @@ -102,7 +102,7 @@ func SetFlags(cmd *cobra.Command) error { return err } - cmd.Flags().String(readinessProbeTypeFlag, "", "readiness probe type: none, tcp, or http") + cmd.Flags().String(readinessProbeTypeFlag, "none", "readiness probe type: none, tcp, or http") if err := viper.BindPFlag(readinessProbeTypeFlag, cmd.Flags().Lookup(readinessProbeTypeFlag)); err != nil { return err } @@ -143,8 +143,7 @@ func GetConfig() (cfg RemoteSessionControllerConfig, err error) { } func (cfg *RemoteSessionControllerConfig) Validate() error { - if cfg.ReadinessProbeType != "" && - cfg.ReadinessProbeType != string(amaltheadevv1alpha1.None) && + if cfg.ReadinessProbeType != string(amaltheadevv1alpha1.None) && cfg.ReadinessProbeType != string(amaltheadevv1alpha1.TCP) && cfg.ReadinessProbeType != string(amaltheadevv1alpha1.HTTP) { return fmt.Errorf("invalid readiness probe type: %s", cfg.ReadinessProbeType) diff --git a/internal/remote/server/server.go b/internal/remote/server/server.go index 425029f8..b7a59774 100644 --- a/internal/remote/server/server.go +++ b/internal/remote/server/server.go @@ -117,12 +117,7 @@ func newServer(controller controller.RemoteSessionController, cfg config.RemoteS // Readiness endpoint e.GET("/ready", func(c echo.Context) error { switch cfg.ReadinessProbeType { - case string(amaltheadevv1alpha1.None): - return c.NoContent(http.StatusOK) case string(amaltheadevv1alpha1.TCP): - if cfg.SessionPort == 0 { - return c.NoContent(http.StatusOK) - } dialer := net.Dialer{} dialCtx, dialCtxCancel := context.WithTimeout(c.Request().Context(), 5*time.Second) defer dialCtxCancel() From f3503055996564a83a6b47972646b9535dc0f67f Mon Sep 17 00:00:00 2001 From: Salim Kayal Date: Tue, 16 Jun 2026 12:40:32 +0200 Subject: [PATCH 6/9] fix: tests for refactor --- internal/remote/config/config.go | 8 +++---- internal/remote/config/config_test.go | 4 ++-- internal/remote/server/server.go | 2 +- internal/remote/server/server_test.go | 31 +-------------------------- 4 files changed, 8 insertions(+), 37 deletions(-) diff --git a/internal/remote/config/config.go b/internal/remote/config/config.go index c14af31d..c0a39b56 100644 --- a/internal/remote/config/config.go +++ b/internal/remote/config/config.go @@ -54,13 +54,13 @@ type RemoteSessionControllerConfig struct { Runai runaiConfig.RunaiConfig // The port the server will listen to - ServerPort int32 + ServerPort int // FakeStart if true, do not start the remote session and print debug information FakeStart bool // SessionPort is the port where the remote session is expected to be serving - SessionPort int32 + SessionPort int // SessionURLPath is the URL path for the HTTP readiness probe SessionURLPath string @@ -133,9 +133,9 @@ func GetConfig() (cfg RemoteSessionControllerConfig, err error) { cfg.Firecrest = firecrestConfig.GetConfig() cfg.Runai = runaiConfig.GetConfig() - cfg.ServerPort = viper.GetInt32(serverPortFlag) + cfg.ServerPort = viper.GetInt(serverPortFlag) cfg.FakeStart = viper.GetBool(fakeStartFlag) - cfg.SessionPort = viper.GetInt32(sessionPortFlag) + cfg.SessionPort = viper.GetInt(sessionPortFlag) cfg.SessionURLPath = viper.GetString(sessionURLPathFlag) cfg.ReadinessProbeType = viper.GetString(readinessProbeTypeFlag) diff --git a/internal/remote/config/config_test.go b/internal/remote/config/config_test.go index e365e690..96d909f0 100644 --- a/internal/remote/config/config_test.go +++ b/internal/remote/config/config_test.go @@ -94,7 +94,7 @@ func TestConfigNewFlags(t *testing.T) { args []string env map[string]string wantProbeType string - wantSessionPort int32 + wantSessionPort int wantURLPath string wantErr bool }{ @@ -108,7 +108,7 @@ func TestConfigNewFlags(t *testing.T) { "--auth-firecrest-client-id=my-client", "--auth-firecrest-client-secret=my-secret", }, - wantProbeType: "", + wantProbeType: string(amaltheadevv1alpha1.None), wantSessionPort: 0, wantURLPath: "/", }, diff --git a/internal/remote/server/server.go b/internal/remote/server/server.go index b7a59774..22996655 100644 --- a/internal/remote/server/server.go +++ b/internal/remote/server/server.go @@ -118,7 +118,7 @@ func newServer(controller controller.RemoteSessionController, cfg config.RemoteS e.GET("/ready", func(c echo.Context) error { switch cfg.ReadinessProbeType { case string(amaltheadevv1alpha1.TCP): - dialer := net.Dialer{} + dialer := net.Dialer{} dialCtx, dialCtxCancel := context.WithTimeout(c.Request().Context(), 5*time.Second) defer dialCtxCancel() conn, err := dialer.DialContext(dialCtx, "tcp", fmt.Sprintf("127.0.0.1:%d", cfg.SessionPort)) diff --git a/internal/remote/server/server_test.go b/internal/remote/server/server_test.go index 79cfdbe6..809d0098 100644 --- a/internal/remote/server/server_test.go +++ b/internal/remote/server/server_test.go @@ -26,7 +26,7 @@ func TestReadyEndpoint(t *testing.T) { ServerPort: 65532, SessionPort: 0, SessionURLPath: "/", - ReadinessProbeType: "", + ReadinessProbeType: "none", } tests := []struct { @@ -44,15 +44,6 @@ func TestReadyEndpoint(t *testing.T) { }(), wantStatus: http.StatusOK, }, - { - name: "interactive with empty probe type defaults to 200", - cfg: func() config.RemoteSessionControllerConfig { - c := baseCfg - c.ReadinessProbeType = "" - return c - }(), - wantStatus: http.StatusOK, - }, { name: "interactive tcp open port returns 200", cfg: func() config.RemoteSessionControllerConfig { @@ -80,16 +71,6 @@ func TestReadyEndpoint(t *testing.T) { }(), wantStatus: http.StatusServiceUnavailable, }, - { - name: "interactive tcp port 0 defaults to 200", - cfg: func() config.RemoteSessionControllerConfig { - c := baseCfg - c.ReadinessProbeType = string(amaltheadevv1alpha1.TCP) - c.SessionPort = 0 - return c - }(), - wantStatus: http.StatusOK, - }, { name: "interactive http serving 200 returns 200", cfg: func() config.RemoteSessionControllerConfig { @@ -177,16 +158,6 @@ func TestReadyEndpoint(t *testing.T) { }(), wantStatus: http.StatusServiceUnavailable, }, - { - name: "interactive http port 0 defaults to 200", - cfg: func() config.RemoteSessionControllerConfig { - c := baseCfg - c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) - c.SessionPort = 0 - return c - }(), - wantStatus: http.StatusOK, - }, } for _, tt := range tests { From 05efe8aa981002f54677e2546d55d86180f11249 Mon Sep 17 00:00:00 2001 From: Salim Kayal Date: Tue, 16 Jun 2026 12:57:14 +0200 Subject: [PATCH 7/9] feat: get free port for server tests --- internal/remote/server/server_test.go | 87 +++++++++++++++++---------- 1 file changed, 55 insertions(+), 32 deletions(-) diff --git a/internal/remote/server/server_test.go b/internal/remote/server/server_test.go index 809d0098..b3dde7fd 100644 --- a/internal/remote/server/server_test.go +++ b/internal/remote/server/server_test.go @@ -2,6 +2,7 @@ package server import ( "context" + "fmt" "net" "net/http" "net/http/httptest" @@ -21,6 +22,25 @@ func (m *mockController) Status(ctx context.Context) (models.RemoteSessionState, func (m *mockController) Start(ctx context.Context) error { return nil } func (m *mockController) Stop(ctx context.Context) error { return nil } +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) +} + func TestReadyEndpoint(t *testing.T) { baseCfg := config.RemoteSessionControllerConfig{ ServerPort: 65532, @@ -31,29 +51,29 @@ func TestReadyEndpoint(t *testing.T) { tests := []struct { name string - cfg config.RemoteSessionControllerConfig - setupBackend func() (cleanup func()) + makeCfg func(port int) config.RemoteSessionControllerConfig + setupBackend func(port int) (cleanup func()) wantStatus int }{ { name: "none probe returns 200", - cfg: func() config.RemoteSessionControllerConfig { + makeCfg: func(int) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.None) return c - }(), + }, wantStatus: http.StatusOK, }, { name: "interactive tcp open port returns 200", - cfg: func() config.RemoteSessionControllerConfig { + makeCfg: func(port int) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.TCP) - c.SessionPort = 18000 + c.SessionPort = port return c - }(), - setupBackend: func() func() { - ln, err := net.Listen("tcp", "127.0.0.1:18000") + }, + setupBackend: func(port int) func() { + ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) if err != nil { panic(err) } @@ -63,25 +83,25 @@ func TestReadyEndpoint(t *testing.T) { }, { name: "interactive tcp closed port returns 503", - cfg: func() config.RemoteSessionControllerConfig { + makeCfg: func(port int) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.TCP) - c.SessionPort = 18001 + c.SessionPort = port return c - }(), + }, wantStatus: http.StatusServiceUnavailable, }, { name: "interactive http serving 200 returns 200", - cfg: func() config.RemoteSessionControllerConfig { + makeCfg: func(port int) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) - c.SessionPort = 18002 + c.SessionPort = port c.SessionURLPath = "/lab" return c - }(), - setupBackend: func() func() { - ln, err := net.Listen("tcp", "127.0.0.1:18002") + }, + setupBackend: func(port int) func() { + ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) if err != nil { panic(err) } @@ -101,15 +121,15 @@ func TestReadyEndpoint(t *testing.T) { }, { name: "interactive http redirect 302 returns 200", - cfg: func() config.RemoteSessionControllerConfig { + makeCfg: func(port int) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) - c.SessionPort = 18003 + c.SessionPort = port c.SessionURLPath = "/" return c - }(), - setupBackend: func() func() { - ln, err := net.Listen("tcp", "127.0.0.1:18003") + }, + setupBackend: func(port int) func() { + ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) if err != nil { panic(err) } @@ -125,15 +145,15 @@ func TestReadyEndpoint(t *testing.T) { }, { name: "interactive http serving 500 returns 503", - cfg: func() config.RemoteSessionControllerConfig { + makeCfg: func(port int) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) - c.SessionPort = 18004 + c.SessionPort = port c.SessionURLPath = "/" return c - }(), - setupBackend: func() func() { - ln, err := net.Listen("tcp", "127.0.0.1:18004") + }, + setupBackend: func(port int) func() { + ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) if err != nil { panic(err) } @@ -149,25 +169,28 @@ func TestReadyEndpoint(t *testing.T) { }, { name: "interactive http connection refused returns 503", - cfg: func() config.RemoteSessionControllerConfig { + makeCfg: func(port int) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) - c.SessionPort = 18005 + c.SessionPort = port c.SessionURLPath = "/" return c - }(), + }, wantStatus: http.StatusServiceUnavailable, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + port := getFreePortOrDie() + cfg := tt.makeCfg(port) + if tt.setupBackend != nil { - cleanup := tt.setupBackend() + cleanup := tt.setupBackend(port) defer cleanup() } - e := newServer(&mockController{}, tt.cfg) + e := newServer(&mockController{}, cfg) req := httptest.NewRequest(http.MethodGet, "/ready", nil) rec := httptest.NewRecorder() e.ServeHTTP(rec, req) From a107f629877762b6652e91877d0e2aeebe925df1 Mon Sep 17 00:00:00 2001 From: Salim Kayal Date: Tue, 16 Jun 2026 13:14:31 +0200 Subject: [PATCH 8/9] squashme: match port type to crd --- internal/remote/config/config.go | 8 +++---- internal/remote/config/config_test.go | 2 +- internal/remote/server/server_test.go | 30 +++++++++++++-------------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/internal/remote/config/config.go b/internal/remote/config/config.go index c0a39b56..c14af31d 100644 --- a/internal/remote/config/config.go +++ b/internal/remote/config/config.go @@ -54,13 +54,13 @@ type RemoteSessionControllerConfig struct { Runai runaiConfig.RunaiConfig // The port the server will listen to - ServerPort int + ServerPort int32 // FakeStart if true, do not start the remote session and print debug information FakeStart bool // SessionPort is the port where the remote session is expected to be serving - SessionPort int + SessionPort int32 // SessionURLPath is the URL path for the HTTP readiness probe SessionURLPath string @@ -133,9 +133,9 @@ func GetConfig() (cfg RemoteSessionControllerConfig, err error) { cfg.Firecrest = firecrestConfig.GetConfig() cfg.Runai = runaiConfig.GetConfig() - cfg.ServerPort = viper.GetInt(serverPortFlag) + cfg.ServerPort = viper.GetInt32(serverPortFlag) cfg.FakeStart = viper.GetBool(fakeStartFlag) - cfg.SessionPort = viper.GetInt(sessionPortFlag) + cfg.SessionPort = viper.GetInt32(sessionPortFlag) cfg.SessionURLPath = viper.GetString(sessionURLPathFlag) cfg.ReadinessProbeType = viper.GetString(readinessProbeTypeFlag) diff --git a/internal/remote/config/config_test.go b/internal/remote/config/config_test.go index 96d909f0..f69e68eb 100644 --- a/internal/remote/config/config_test.go +++ b/internal/remote/config/config_test.go @@ -94,7 +94,7 @@ func TestConfigNewFlags(t *testing.T) { args []string env map[string]string wantProbeType string - wantSessionPort int + wantSessionPort int32 wantURLPath string wantErr bool }{ diff --git a/internal/remote/server/server_test.go b/internal/remote/server/server_test.go index b3dde7fd..3a5b6026 100644 --- a/internal/remote/server/server_test.go +++ b/internal/remote/server/server_test.go @@ -22,7 +22,7 @@ func (m *mockController) Status(ctx context.Context) (models.RemoteSessionState, func (m *mockController) Start(ctx context.Context) error { return nil } func (m *mockController) Stop(ctx context.Context) error { return nil } -func getFreePortOrDie() int { +func getFreePortOrDie() int32 { var a *net.TCPAddr var err error if a, err = net.ResolveTCPAddr("tcp", "localhost:0"); err == nil { @@ -34,7 +34,7 @@ func getFreePortOrDie() int { panic(err) } }() - return l.Addr().(*net.TCPAddr).Port + return int32(l.Addr().(*net.TCPAddr).Port) } panic(err) } @@ -51,13 +51,13 @@ func TestReadyEndpoint(t *testing.T) { tests := []struct { name string - makeCfg func(port int) config.RemoteSessionControllerConfig - setupBackend func(port int) (cleanup func()) + makeCfg func(port int32) config.RemoteSessionControllerConfig + setupBackend func(port int32) (cleanup func()) wantStatus int }{ { name: "none probe returns 200", - makeCfg: func(int) config.RemoteSessionControllerConfig { + makeCfg: func(int32) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.None) return c @@ -66,13 +66,13 @@ func TestReadyEndpoint(t *testing.T) { }, { name: "interactive tcp open port returns 200", - makeCfg: func(port int) config.RemoteSessionControllerConfig { + makeCfg: func(port int32) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.TCP) c.SessionPort = port return c }, - setupBackend: func(port int) func() { + setupBackend: func(port int32) func() { ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) if err != nil { panic(err) @@ -83,7 +83,7 @@ func TestReadyEndpoint(t *testing.T) { }, { name: "interactive tcp closed port returns 503", - makeCfg: func(port int) config.RemoteSessionControllerConfig { + makeCfg: func(port int32) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.TCP) c.SessionPort = port @@ -93,14 +93,14 @@ func TestReadyEndpoint(t *testing.T) { }, { name: "interactive http serving 200 returns 200", - makeCfg: func(port int) config.RemoteSessionControllerConfig { + makeCfg: func(port int32) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) c.SessionPort = port c.SessionURLPath = "/lab" return c }, - setupBackend: func(port int) func() { + setupBackend: func(port int32) func() { ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) if err != nil { panic(err) @@ -121,14 +121,14 @@ func TestReadyEndpoint(t *testing.T) { }, { name: "interactive http redirect 302 returns 200", - makeCfg: func(port int) config.RemoteSessionControllerConfig { + makeCfg: func(port int32) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) c.SessionPort = port c.SessionURLPath = "/" return c }, - setupBackend: func(port int) func() { + setupBackend: func(port int32) func() { ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) if err != nil { panic(err) @@ -145,14 +145,14 @@ func TestReadyEndpoint(t *testing.T) { }, { name: "interactive http serving 500 returns 503", - makeCfg: func(port int) config.RemoteSessionControllerConfig { + makeCfg: func(port int32) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) c.SessionPort = port c.SessionURLPath = "/" return c }, - setupBackend: func(port int) func() { + setupBackend: func(port int32) func() { ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) if err != nil { panic(err) @@ -169,7 +169,7 @@ func TestReadyEndpoint(t *testing.T) { }, { name: "interactive http connection refused returns 503", - makeCfg: func(port int) config.RemoteSessionControllerConfig { + makeCfg: func(port int32) config.RemoteSessionControllerConfig { c := baseCfg c.ReadinessProbeType = string(amaltheadevv1alpha1.HTTP) c.SessionPort = port From 8658bbb9f5805083fe7873e41542d18d59b631fe Mon Sep 17 00:00:00 2001 From: Salim Kayal Date: Wed, 17 Jun 2026 12:32:36 +0200 Subject: [PATCH 9/9] fix: remove case for SessionPort set to 0, fix comment Co-authored-by: Flora Thiebaut --- internal/remote/server/server.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/remote/server/server.go b/internal/remote/server/server.go index 22996655..227f3d8c 100644 --- a/internal/remote/server/server.go +++ b/internal/remote/server/server.go @@ -131,9 +131,6 @@ func newServer(controller controller.RemoteSessionController, cfg config.RemoteS } return c.NoContent(http.StatusOK) case string(amaltheadevv1alpha1.HTTP): - if cfg.SessionPort == 0 { - return c.NoContent(http.StatusOK) - } client := &http.Client{ Timeout: 5 * time.Second, CheckRedirect: func(req *http.Request, via []*http.Request) error { @@ -159,7 +156,7 @@ func newServer(controller controller.RemoteSessionController, cfg config.RemoteS } return c.NoContent(http.StatusServiceUnavailable) default: - // Unconfigured / unknown: preserve old behavior for backward compatibility + // Case None or unset: preserve old behavior for backward compatibility return c.NoContent(http.StatusOK) } })