40bf8ea refactor: address CodeRabbit review and fix lint gate · gordon

M internal/usecase/container/readiness.go +22 -3


@@ -13,6 +13,7 @@ import (
13	13		// it verifies the process is at least accepting connections.
14	14		func tcpProbe(ctx context.Context, addr string, timeout time.Duration) error {
15	15		deadline := time.Now().Add(timeout)
	16	+	dialer := &net.Dialer{}
16	17		for {
17	18		if err := ctx.Err(); err != nil {
18	19		return err

@@ -21,7 +22,14 @@ func tcpProbe(ctx context.Context, addr string, timeout time.Duration) error {
21	22		return fmt.Errorf("TCP probe timeout after %s: %s not reachable", timeout, addr)
22	23		}
23	24
24		-	conn, err := net.DialTimeout("tcp", addr, time.Second)
	25	+	remaining := time.Until(deadline)
	26	+	attemptTimeout := time.Second
	27	+	if remaining < attemptTimeout {
	28	+	attemptTimeout = remaining
	29	+	}
	30	+	attemptCtx, cancel := context.WithTimeout(ctx, attemptTimeout)
	31	+	conn, err := dialer.DialContext(attemptCtx, "tcp", addr)
	32	+	cancel()
25	33		if err == nil {
26	34		conn.Close()
27	35		return nil

@@ -39,7 +47,6 @@ func tcpProbe(ctx context.Context, addr string, timeout time.Duration) error {
39	47		// 2xx/3xx response or timeout. Used when gordon.health label is set.
40	48		func httpProbe(ctx context.Context, url string, timeout time.Duration) error {
41	49		client := &http.Client{
42		-	Timeout: 2 * time.Second,
43	50		CheckRedirect: func(req http.Request, via []http.Request) error {
44	51		// Don't follow redirects — treat 3xx as a successful response.
45	52		return http.ErrUseLastResponse

@@ -56,7 +63,19 @@ func httpProbe(ctx context.Context, url string, timeout time.Duration) error {
56	63		return fmt.Errorf("HTTP probe timeout after %s: last status %d from %s", timeout, lastStatus, url)
57	64		}
58	65
59		-	resp, err := client.Get(url)
	66	+	remaining := time.Until(deadline)
	67	+	attemptTimeout := 2 * time.Second
	68	+	if remaining < attemptTimeout {
	69	+	attemptTimeout = remaining
	70	+	}
	71	+	req, reqErr := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
	72	+	if reqErr != nil {
	73	+	return reqErr
	74	+	}
	75	+	attemptCtx, cancel := context.WithTimeout(ctx, attemptTimeout)
	76	+	req = req.WithContext(attemptCtx)
	77	+	resp, err := client.Do(req)
	78	+	cancel()
60	79		if err == nil {
61	80		lastStatus = resp.StatusCode
62	81		resp.Body.Close()

 // it verifies the process is at least accepting connections.
 func tcpProbe(ctx context.Context, addr string, timeout time.Duration) error {
 	deadline := time.Now().Add(timeout)
+	dialer := &net.Dialer{}
 	for {
 		if err := ctx.Err(); err != nil {
 			return err
 			return fmt.Errorf("TCP probe timeout after %s: %s not reachable", timeout, addr)
 		}
-		conn, err := net.DialTimeout("tcp", addr, time.Second)
+		remaining := time.Until(deadline)
+		attemptTimeout := time.Second
+		if remaining < attemptTimeout {
+			attemptTimeout = remaining
+		}
+		attemptCtx, cancel := context.WithTimeout(ctx, attemptTimeout)
+		conn, err := dialer.DialContext(attemptCtx, "tcp", addr)
+		cancel()
 		if err == nil {
 			conn.Close()
 			return nil
 // 2xx/3xx response or timeout. Used when gordon.health label is set.
 func httpProbe(ctx context.Context, url string, timeout time.Duration) error {
 	client := &http.Client{
-		Timeout: 2 * time.Second,
 		CheckRedirect: func(req *http.Request, via []*http.Request) error {
 			// Don't follow redirects — treat 3xx as a successful response.
 			return http.ErrUseLastResponse
 			return fmt.Errorf("HTTP probe timeout after %s: last status %d from %s", timeout, lastStatus, url)
 		}
-		resp, err := client.Get(url)
+		remaining := time.Until(deadline)
+		attemptTimeout := 2 * time.Second
+		if remaining < attemptTimeout {
+			attemptTimeout = remaining
+		}
+		req, reqErr := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
+		if reqErr != nil {
+			return reqErr
+		}
+		attemptCtx, cancel := context.WithTimeout(ctx, attemptTimeout)
+		req = req.WithContext(attemptCtx)
+		resp, err := client.Do(req)
+		cancel()
 		if err == nil {
 			lastStatus = resp.StatusCode
 			resp.Body.Close()

M internal/usecase/container/readiness_test.go +8 -2


@@ -37,9 +37,15 @@ func TestTCPProbe_Success(t *testing.T) {
37	37		}
38	38
39	39		func TestTCPProbe_Timeout(t *testing.T) {
40		-	// Use a port that nothing listens on (port 1 is privileged, won't have a listener)
	40	+	// Reserve an ephemeral port and close it immediately to get a deterministic
	41	+	// unreachable target, avoiding assumptions about privileged ports.
	42	+	ln, err := net.Listen("tcp", "127.0.0.1:0")
	43	+	require.NoError(t, err)
	44	+	addr := ln.Addr().String()
	45	+	ln.Close()
	46	+
41	47		ctx := testContext()
42		-	err := tcpProbe(ctx, "127.0.0.1:1", 200*time.Millisecond)
	48	+	err = tcpProbe(ctx, addr, 200*time.Millisecond)
43	49		assert.Error(t, err)
44	50		assert.Contains(t, err.Error(), "TCP probe timeout")
45	51		}

 }
 func TestTCPProbe_Timeout(t *testing.T) {
-	// Use a port that nothing listens on (port 1 is privileged, won't have a listener)
+	// Reserve an ephemeral port and close it immediately to get a deterministic
+	// unreachable target, avoiding assumptions about privileged ports.
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	addr := ln.Addr().String()
+	ln.Close()
 	ctx := testContext()
-	err := tcpProbe(ctx, "127.0.0.1:1", 200*time.Millisecond)
+	err = tcpProbe(ctx, addr, 200*time.Millisecond)
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "TCP probe timeout")
 }

M internal/usecase/container/service.go +74 -53


@@ -259,24 +259,7 @@ func (s Service) Deploy(ctx context.Context, route domain.Route) (domain.Conta
259	259		log := zerowrap.FromCtx(ctx)
260	260
261	261		// Record deploy metrics and trace status
262		-	defer func() {
263		-	if err != nil {
264		-	span.RecordError(err)
265		-	span.SetStatus(codes.Error, err.Error())
266		-	}
267		-	if s.metrics == nil {
268		-	return
269		-	}
270		-	attrs := metric.WithAttributes(
271		-	attribute.String("domain", route.Domain),
272		-	attribute.String("image", route.Image),
273		-	)
274		-	s.metrics.DeployTotal.Add(ctx, 1, attrs)
275		-	s.metrics.DeployDuration.Record(ctx, time.Since(deployStart).Seconds(), attrs)
276		-	if err != nil {
277		-	s.metrics.DeployErrors.Add(ctx, 1, attrs)
278		-	}
279		-	}()
	262	+	defer s.recordDeployMetrics(ctx, span, route, deployStart, &err)
280	263
281	264		existing, hasExisting := s.resolveExistingContainer(ctx, route.Domain)
282	265

@@ -337,6 +320,29 @@ func (s Service) Deploy(ctx context.Context, route domain.Route) (domain.Conta
337	320		return newContainer, nil
338	321		}
339	322
	323	+	// recordDeployMetrics records span error status and deploy metrics at the end of a Deploy call.
	324	+	// It is called via defer and receives a pointer to the named return error so it can observe
	325	+	// the final error value after all deferred functions have run.
	326	+	func (s Service) recordDeployMetrics(ctx context.Context, span trace.Span, route domain.Route, start time.Time, errPtr error) {
	327	+	err := *errPtr
	328	+	if err != nil {
	329	+	span.RecordError(err)
	330	+	span.SetStatus(codes.Error, err.Error())
	331	+	}
	332	+	if s.metrics == nil {
	333	+	return
	334	+	}
	335	+	attrs := metric.WithAttributes(
	336	+	attribute.String("domain", route.Domain),
	337	+	attribute.String("image", route.Image),
	338	+	)
	339	+	s.metrics.DeployTotal.Add(ctx, 1, attrs)
	340	+	s.metrics.DeployDuration.Record(ctx, time.Since(start).Seconds(), attrs)
	341	+	if err != nil {
	342	+	s.metrics.DeployErrors.Add(ctx, 1, attrs)
	343	+	}
	344	+	}
	345	+
340	346		// skipRedundantDeploy checks whether the existing container is already running
341	347		// the same image (by Docker image ID) as the one we are about to deploy.
342	348		// When a push triggers both an event-based deploy and an explicit CLI deploy,

@@ -397,13 +403,6 @@ type deployResources struct {
397	403		volumes map[string]string
398	404		}
399	405
400		-	func (s Service) getTrackedContainer(domainName string) (domain.Container, bool) {
401		-	s.mu.RLock()
402		-	container, ok := s.containers[domainName]
403		-	s.mu.RUnlock()
404		-	return container, ok
405		-	}
406		-
407	406		// resolveExistingContainer returns the currently running container for a domain.
408	407		// It first checks the in-memory map (fast path). If not found, it queries the
409	408		// runtime for a running container with matching name and managed label. This

@@ -2285,37 +2284,13 @@ func (s *Service) readinessCascade(ctx context.Context, containerID string, cont
2285	2284		}
2286	2285
2287	2286		// 2. HTTP probe via gordon.health label
2288		-	if containerConfig != nil {
2289		-	if healthPath, ok := containerConfig.Labels[domain.LabelHealth]; ok && healthPath != "" {
2290		-	ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID)
2291		-	if probeErr == nil && ip != "" && port > 0 {
2292		-	url := fmt.Sprintf("http://%s:%d%s", ip, port, healthPath)
2293		-	timeout := cfg.HTTPProbeTimeout
2294		-	if timeout == 0 {
2295		-	timeout = 60 * time.Second
2296		-	}
2297		-	log.Info().Str("url", url).Dur("timeout", timeout).Msg("readiness cascade: using HTTP probe")
2298		-	return httpProbe(ctx, url, timeout)
2299		-	}
2300		-	// Could not resolve endpoint; fall through to next level
2301		-	log.Debug().Err(probeErr).Msg("readiness cascade: HTTP probe skipped, could not resolve container endpoint")
2302		-	}
	2287	+	if probed, probeErr := s.tryHTTPProbe(ctx, containerID, containerConfig, cfg); probed {
	2288	+	return probeErr
2303	2289		}
2304	2290
2305	2291		// 3. TCP probe (if port info available)
2306		-	if containerConfig != nil && len(containerConfig.Ports) > 0 {
2307		-	ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID)
2308		-	if probeErr == nil && ip != "" && port > 0 {
2309		-	addr := fmt.Sprintf("%s:%d", ip, port)
2310		-	timeout := cfg.TCPProbeTimeout
2311		-	if timeout == 0 {
2312		-	timeout = 30 * time.Second
2313		-	}
2314		-	log.Info().Str("addr", addr).Dur("timeout", timeout).Msg("readiness cascade: using TCP probe")
2315		-	return tcpProbe(ctx, addr, timeout)
2316		-	}
2317		-	// Could not resolve endpoint; fall through to delay
2318		-	log.Debug().Err(probeErr).Msg("readiness cascade: TCP probe skipped, could not resolve container endpoint")
	2292	+	if probed, probeErr := s.tryTCPProbe(ctx, containerID, containerConfig, cfg); probed {
	2293	+	return probeErr
2319	2294		}
2320	2295
2321	2296		// 4. Delay fallback

@@ -2323,6 +2298,52 @@ func (s *Service) readinessCascade(ctx context.Context, containerID string, cont
2323	2298		return s.waitForReadyByDelay(ctx, containerID)
2324	2299		}
2325	2300
	2301	+	// tryHTTPProbe attempts an HTTP probe if the gordon.health label is set.
	2302	+	// Returns (true, err) if the probe was attempted, (false, nil) if skipped.
	2303	+	func (s Service) tryHTTPProbe(ctx context.Context, containerID string, containerConfig domain.ContainerConfig, cfg Config) (bool, error) {
	2304	+	log := zerowrap.FromCtx(ctx)
	2305	+	if containerConfig == nil {
	2306	+	return false, nil
	2307	+	}
	2308	+	healthPath, ok := containerConfig.Labels[domain.LabelHealth]
	2309	+	if !ok \|\| healthPath == "" {
	2310	+	return false, nil
	2311	+	}
	2312	+	ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID)
	2313	+	if probeErr != nil \|\| ip == "" \|\| port <= 0 {
	2314	+	log.Debug().Err(probeErr).Msg("readiness cascade: HTTP probe skipped, could not resolve container endpoint")
	2315	+	return false, nil
	2316	+	}
	2317	+	url := fmt.Sprintf("http://%s:%d%s", ip, port, healthPath)
	2318	+	timeout := cfg.HTTPProbeTimeout
	2319	+	if timeout == 0 {
	2320	+	timeout = 60 * time.Second
	2321	+	}
	2322	+	log.Info().Str("url", url).Dur("timeout", timeout).Msg("readiness cascade: using HTTP probe")
	2323	+	return true, httpProbe(ctx, url, timeout)
	2324	+	}
	2325	+
	2326	+	// tryTCPProbe attempts a TCP probe if port info is available.
	2327	+	// Returns (true, err) if the probe was attempted, (false, nil) if skipped.
	2328	+	func (s Service) tryTCPProbe(ctx context.Context, containerID string, containerConfig domain.ContainerConfig, cfg Config) (bool, error) {
	2329	+	log := zerowrap.FromCtx(ctx)
	2330	+	if containerConfig == nil \|\| len(containerConfig.Ports) == 0 {
	2331	+	return false, nil
	2332	+	}
	2333	+	ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID)
	2334	+	if probeErr != nil \|\| ip == "" \|\| port <= 0 {
	2335	+	log.Debug().Err(probeErr).Msg("readiness cascade: TCP probe skipped, could not resolve container endpoint")
	2336	+	return false, nil
	2337	+	}
	2338	+	addr := fmt.Sprintf("%s:%d", ip, port)
	2339	+	timeout := cfg.TCPProbeTimeout
	2340	+	if timeout == 0 {
	2341	+	timeout = 30 * time.Second
	2342	+	}
	2343	+	log.Info().Str("addr", addr).Dur("timeout", timeout).Msg("readiness cascade: using TCP probe")
	2344	+	return true, tcpProbe(ctx, addr, timeout)
	2345	+	}
	2346	+
2326	2347		// resolveContainerEndpoint retrieves the container's IP and first port via
2327	2348		// GetContainerNetworkInfo. The result can be used for HTTP or TCP probes.
2328	2349		func (s *Service) resolveContainerEndpoint(ctx context.Context, containerID string) (string, int, error) {

 	log := zerowrap.FromCtx(ctx)
 	// Record deploy metrics and trace status
-	defer func() {
+	defer s.recordDeployMetrics(ctx, span, route, deployStart, &err)
-		if err != nil {
-			span.RecordError(err)
-			span.SetStatus(codes.Error, err.Error())
-		}
-		if s.metrics == nil {
-			return
-		}
-		attrs := metric.WithAttributes(
-			attribute.String("domain", route.Domain),
-			attribute.String("image", route.Image),
-		)
-		s.metrics.DeployTotal.Add(ctx, 1, attrs)
-		s.metrics.DeployDuration.Record(ctx, time.Since(deployStart).Seconds(), attrs)
-		if err != nil {
-			s.metrics.DeployErrors.Add(ctx, 1, attrs)
-		}
-	}()
 	existing, hasExisting := s.resolveExistingContainer(ctx, route.Domain)
 	return newContainer, nil
 }
+// recordDeployMetrics records span error status and deploy metrics at the end of a Deploy call.
+// It is called via defer and receives a pointer to the named return error so it can observe
+// the final error value after all deferred functions have run.
+func (s *Service) recordDeployMetrics(ctx context.Context, span trace.Span, route domain.Route, start time.Time, errPtr *error) {
+	err := *errPtr
+	if err != nil {
+		span.RecordError(err)
+		span.SetStatus(codes.Error, err.Error())
+	}
+	if s.metrics == nil {
+		return
+	}
+	attrs := metric.WithAttributes(
+		attribute.String("domain", route.Domain),
+		attribute.String("image", route.Image),
+	)
+	s.metrics.DeployTotal.Add(ctx, 1, attrs)
+	s.metrics.DeployDuration.Record(ctx, time.Since(start).Seconds(), attrs)
+	if err != nil {
+		s.metrics.DeployErrors.Add(ctx, 1, attrs)
+	}
+}
 // skipRedundantDeploy checks whether the existing container is already running
 // the same image (by Docker image ID) as the one we are about to deploy.
 // When a push triggers both an event-based deploy and an explicit CLI deploy,
 	volumes        map[string]string
 }
-func (s *Service) getTrackedContainer(domainName string) (*domain.Container, bool) {
-	s.mu.RLock()
-	container, ok := s.containers[domainName]
-	s.mu.RUnlock()
-	return container, ok
-}
 // resolveExistingContainer returns the currently running container for a domain.
 // It first checks the in-memory map (fast path). If not found, it queries the
 // runtime for a running container with matching name and managed label. This
 	}
 	// 2. HTTP probe via gordon.health label
-	if containerConfig != nil {
+	if probed, probeErr := s.tryHTTPProbe(ctx, containerID, containerConfig, cfg); probed {
-		if healthPath, ok := containerConfig.Labels[domain.LabelHealth]; ok && healthPath != "" {
+		return probeErr
-			ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID)
-			if probeErr == nil && ip != "" && port > 0 {
-				url := fmt.Sprintf("http://%s:%d%s", ip, port, healthPath)
-				timeout := cfg.HTTPProbeTimeout
-				if timeout == 0 {
-					timeout = 60 * time.Second
-				}
-				log.Info().Str("url", url).Dur("timeout", timeout).Msg("readiness cascade: using HTTP probe")
-				return httpProbe(ctx, url, timeout)
-			}
-			// Could not resolve endpoint; fall through to next level
-			log.Debug().Err(probeErr).Msg("readiness cascade: HTTP probe skipped, could not resolve container endpoint")
-		}
 	}
 	// 3. TCP probe (if port info available)
-	if containerConfig != nil && len(containerConfig.Ports) > 0 {
+	if probed, probeErr := s.tryTCPProbe(ctx, containerID, containerConfig, cfg); probed {
-		ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID)
+		return probeErr
-		if probeErr == nil && ip != "" && port > 0 {
-			addr := fmt.Sprintf("%s:%d", ip, port)
-			timeout := cfg.TCPProbeTimeout
-			if timeout == 0 {
-				timeout = 30 * time.Second
-			}
-			log.Info().Str("addr", addr).Dur("timeout", timeout).Msg("readiness cascade: using TCP probe")
-			return tcpProbe(ctx, addr, timeout)
-		}
-		// Could not resolve endpoint; fall through to delay
-		log.Debug().Err(probeErr).Msg("readiness cascade: TCP probe skipped, could not resolve container endpoint")
 	}
 	// 4. Delay fallback
 	return s.waitForReadyByDelay(ctx, containerID)
 }
+// tryHTTPProbe attempts an HTTP probe if the gordon.health label is set.
+// Returns (true, err) if the probe was attempted, (false, nil) if skipped.
+func (s *Service) tryHTTPProbe(ctx context.Context, containerID string, containerConfig *domain.ContainerConfig, cfg Config) (bool, error) {
+	log := zerowrap.FromCtx(ctx)
+	if containerConfig == nil {
+		return false, nil
+	}
+	healthPath, ok := containerConfig.Labels[domain.LabelHealth]
+	if !ok || healthPath == "" {
+		return false, nil
+	}
+	ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID)
+	if probeErr != nil || ip == "" || port <= 0 {
+		log.Debug().Err(probeErr).Msg("readiness cascade: HTTP probe skipped, could not resolve container endpoint")
+		return false, nil
+	}
+	url := fmt.Sprintf("http://%s:%d%s", ip, port, healthPath)
+	timeout := cfg.HTTPProbeTimeout
+	if timeout == 0 {
+		timeout = 60 * time.Second
+	}
+	log.Info().Str("url", url).Dur("timeout", timeout).Msg("readiness cascade: using HTTP probe")
+	return true, httpProbe(ctx, url, timeout)
+}
+// tryTCPProbe attempts a TCP probe if port info is available.
+// Returns (true, err) if the probe was attempted, (false, nil) if skipped.
+func (s *Service) tryTCPProbe(ctx context.Context, containerID string, containerConfig *domain.ContainerConfig, cfg Config) (bool, error) {
+	log := zerowrap.FromCtx(ctx)
+	if containerConfig == nil || len(containerConfig.Ports) == 0 {
+		return false, nil
+	}
+	ip, port, probeErr := s.resolveContainerEndpoint(ctx, containerID)
+	if probeErr != nil || ip == "" || port <= 0 {
+		log.Debug().Err(probeErr).Msg("readiness cascade: TCP probe skipped, could not resolve container endpoint")
+		return false, nil
+	}
+	addr := fmt.Sprintf("%s:%d", ip, port)
+	timeout := cfg.TCPProbeTimeout
+	if timeout == 0 {
+		timeout = 30 * time.Second
+	}
+	log.Info().Str("addr", addr).Dur("timeout", timeout).Msg("readiness cascade: using TCP probe")
+	return true, tcpProbe(ctx, addr, timeout)
+}
 // resolveContainerEndpoint retrieves the container's IP and first port via
 // GetContainerNetworkInfo. The result can be used for HTTP or TCP probes.
 func (s *Service) resolveContainerEndpoint(ctx context.Context, containerID string) (string, int, error) {

M internal/usecase/container/service_readiness_test.go +28 -8

@@ -1,17 +1,31 @@
1	1		package container
2	2
3	3		import (
	4	+	"net"
4	5		"strings"
5	6		"testing"
6	7		"time"
7	8
8	9		"github.com/stretchr/testify/assert"
9	10		"github.com/stretchr/testify/mock"
	11	+	"github.com/stretchr/testify/require"
10	12
11	13		"github.com/bnema/gordon/internal/boundaries/out/mocks"
12	14		"github.com/bnema/gordon/internal/domain"
13	15		)
14	16
	17	+	// reserveEphemeralAddr binds on 127.0.0.1:0, captures the assigned port, closes
	18	+	// the listener, and returns the address string. The port is guaranteed to be
	19	+	// unreachable (no process listening) when the probe runs.
	20	+	func reserveEphemeralAddr(t *testing.T) (ip string, port int) {
	21	+	t.Helper()
	22	+	ln, err := net.Listen("tcp", "127.0.0.1:0")
	23	+	require.NoError(t, err)
	24	+	addr := ln.Addr().(*net.TCPAddr)
	25	+	ln.Close()
	26	+	return addr.IP.String(), addr.Port
	27	+	}
	28	+
15	29		func TestService_WaitForReady_AutoFallsBackToDelayWhenNoHealthcheck(t *testing.T) {
16	30		runtime := mocks.NewMockContainerRuntime(t)
17	31		svc := NewService(runtime, nil, nil, nil, Config{

@@ -47,21 +61,24 @@ func TestService_WaitForReady_AutoCascadeUsesTCPWhenNoHealthcheckAndNoHealthLabe
47	61		Labels: map[string]string{},
48	62		}
49	63
	64	+	// Reserve an ephemeral loopback port that is guaranteed to be unreachable.
	65	+	probeIP, probePort := reserveEphemeralAddr(t)
	66	+
50	67		// Initial running poll
51	68		runtime.EXPECT().IsContainerRunning(mock.Anything, containerID).Return(true, nil).Once()
52	69		// No Docker healthcheck
53	70		runtime.EXPECT().GetContainerHealthStatus(mock.Anything, containerID).Return("", false, nil).Once()
54	71		// Cascade resolves container endpoint for TCP probe
55		-	runtime.EXPECT().GetContainerNetworkInfo(mock.Anything, containerID).Return("172.17.0.5", 8080, nil).Once()
	72	+	runtime.EXPECT().GetContainerNetworkInfo(mock.Anything, containerID).Return(probeIP, probePort, nil).Once()
56	73
57		-	// TCP probe will try to connect to 172.17.0.5:8080 — which will fail since
58		-	// there's nothing listening. Use a short health timeout so the test doesn't hang.
	74	+	// TCP probe will try to connect — which will fail since the port is closed.
	75	+	// Use a short health timeout so the test doesn't hang.
59	76		svc.mu.Lock()
60		-	svc.config.HealthTimeout = 50 * time.Millisecond
	77	+	svc.config.TCPProbeTimeout = 50 * time.Millisecond
61	78		svc.mu.Unlock()
62	79
63	80		err := svc.waitForReady(ctx, containerID, containerConfig)
64		-	// Expect TCP probe timeout (no server listening at 172.17.0.5:8080)
	81	+	// Expect TCP probe timeout (no server listening)
65	82		assert.Error(t, err)
66	83		assert.Contains(t, err.Error(), "TCP probe timeout")
67	84		}

@@ -69,8 +86,8 @@ func TestService_WaitForReady_AutoCascadeUsesTCPWhenNoHealthcheckAndNoHealthLabe
69	86		func TestService_WaitForReady_AutoCascadeUsesHTTPProbeWhenHealthLabelSet(t *testing.T) {
70	87		runtime := mocks.NewMockContainerRuntime(t)
71	88		svc := NewService(runtime, nil, nil, nil, Config{
72		-	ReadinessMode: "auto",
73		-	HealthTimeout: 50 * time.Millisecond,
	89	+	ReadinessMode: "auto",
	90	+	HTTPProbeTimeout: 50 * time.Millisecond,
74	91		})
75	92
76	93		ctx := testContext()

@@ -83,12 +100,15 @@ func TestService_WaitForReady_AutoCascadeUsesHTTPProbeWhenHealthLabelSet(t *test
83	100		},
84	101		}
85	102
	103	+	// Reserve an ephemeral loopback port that is guaranteed to be unreachable.
	104	+	probeIP, probePort := reserveEphemeralAddr(t)
	105	+
86	106		// Initial running poll
87	107		runtime.EXPECT().IsContainerRunning(mock.Anything, containerID).Return(true, nil).Once()
88	108		// No Docker healthcheck
89	109		runtime.EXPECT().GetContainerHealthStatus(mock.Anything, containerID).Return("", false, nil).Once()
90	110		// Cascade resolves container endpoint for HTTP probe
91		-	runtime.EXPECT().GetContainerNetworkInfo(mock.Anything, containerID).Return("172.17.0.5", 8080, nil).Once()
	111	+	runtime.EXPECT().GetContainerNetworkInfo(mock.Anything, containerID).Return(probeIP, probePort, nil).Once()
92	112
93	113		err := svc.waitForReady(ctx, containerID, containerConfig)
94	114		// Expect HTTP probe timeout (no server listening)

 package container
 import (
+	"net"
 	"strings"
 	"testing"
 	"time"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 	"github.com/bnema/gordon/internal/boundaries/out/mocks"
 	"github.com/bnema/gordon/internal/domain"
 )
+// reserveEphemeralAddr binds on 127.0.0.1:0, captures the assigned port, closes
+// the listener, and returns the address string. The port is guaranteed to be
+// unreachable (no process listening) when the probe runs.
+func reserveEphemeralAddr(t *testing.T) (ip string, port int) {
+	t.Helper()
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	addr := ln.Addr().(*net.TCPAddr)
+	ln.Close()
+	return addr.IP.String(), addr.Port
+}
 func TestService_WaitForReady_AutoFallsBackToDelayWhenNoHealthcheck(t *testing.T) {
 	runtime := mocks.NewMockContainerRuntime(t)
 	svc := NewService(runtime, nil, nil, nil, Config{
 		Labels: map[string]string{},
 	}
+	// Reserve an ephemeral loopback port that is guaranteed to be unreachable.
+	probeIP, probePort := reserveEphemeralAddr(t)
 	// Initial running poll
 	runtime.EXPECT().IsContainerRunning(mock.Anything, containerID).Return(true, nil).Once()
 	// No Docker healthcheck
 	runtime.EXPECT().GetContainerHealthStatus(mock.Anything, containerID).Return("", false, nil).Once()
 	// Cascade resolves container endpoint for TCP probe
-	runtime.EXPECT().GetContainerNetworkInfo(mock.Anything, containerID).Return("172.17.0.5", 8080, nil).Once()
+	runtime.EXPECT().GetContainerNetworkInfo(mock.Anything, containerID).Return(probeIP, probePort, nil).Once()
-	// TCP probe will try to connect to 172.17.0.5:8080 — which will fail since
+	// TCP probe will try to connect — which will fail since the port is closed.
-	// there's nothing listening. Use a short health timeout so the test doesn't hang.
+	// Use a short health timeout so the test doesn't hang.
 	svc.mu.Lock()
-	svc.config.HealthTimeout = 50 * time.Millisecond
+	svc.config.TCPProbeTimeout = 50 * time.Millisecond
 	svc.mu.Unlock()
 	err := svc.waitForReady(ctx, containerID, containerConfig)
-	// Expect TCP probe timeout (no server listening at 172.17.0.5:8080)
+	// Expect TCP probe timeout (no server listening)
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "TCP probe timeout")
 }
 func TestService_WaitForReady_AutoCascadeUsesHTTPProbeWhenHealthLabelSet(t *testing.T) {
 	runtime := mocks.NewMockContainerRuntime(t)
 	svc := NewService(runtime, nil, nil, nil, Config{
-		ReadinessMode: "auto",
+		ReadinessMode:    "auto",
-		HealthTimeout: 50 * time.Millisecond,
+		HTTPProbeTimeout: 50 * time.Millisecond,
 	})
 	ctx := testContext()
 		},
 	}
+	// Reserve an ephemeral loopback port that is guaranteed to be unreachable.
+	probeIP, probePort := reserveEphemeralAddr(t)
 	// Initial running poll
 	runtime.EXPECT().IsContainerRunning(mock.Anything, containerID).Return(true, nil).Once()
 	// No Docker healthcheck
 	runtime.EXPECT().GetContainerHealthStatus(mock.Anything, containerID).Return("", false, nil).Once()
 	// Cascade resolves container endpoint for HTTP probe
-	runtime.EXPECT().GetContainerNetworkInfo(mock.Anything, containerID).Return("172.17.0.5", 8080, nil).Once()
+	runtime.EXPECT().GetContainerNetworkInfo(mock.Anything, containerID).Return(probeIP, probePort, nil).Once()
 	err := svc.waitForReady(ctx, containerID, containerConfig)
 	// Expect HTTP probe timeout (no server listening)

M internal/usecase/container/service_test.go +36 -24


@@ -2644,42 +2644,45 @@ func TestService_Deploy_SkipRedundantDeploy_ContainerNotRunning(t *testing.T) {
2644	2644		}
2645	2645
2646	2646		// TestService_Deploy_OrphanCleanup_NeverKillsRunningCanonical verifies that orphan
2647		-	// cleanup never stops a running container with the canonical name, even if it is not
2648		-	// tracked in memory and was not resolved by resolveExistingContainer. Only stopped
	2647	+	// cleanup never stops a running container with the canonical name. Only stopped
2649	2648		// canonical containers and temp containers (-new/-next) should be cleaned up.
	2649	+	// The active canonical container is resolved via resolveExistingContainer (it has the
	2650	+	// managed label), so the new container is created with the "-new" suffix — Docker
	2651	+	// disallows creating a container with the same name as an existing running container.
2650	2652		func TestService_Deploy_OrphanCleanup_NeverKillsRunningCanonical(t *testing.T) {
2651	2653		runtime := mocks.NewMockContainerRuntime(t)
2652	2654		envLoader := mocks.NewMockEnvLoader(t)
2653	2655		eventBus := mocks.NewMockEventPublisher(t)
2654	2656
2655	2657		config := Config{
2656		-	ReadinessDelay: time.Millisecond,
2657		-	DrainDelay: time.Millisecond,
	2658	+	ReadinessDelay: time.Millisecond,
	2659	+	DrainDelay: time.Millisecond,
	2660	+	StabilizationDelay: time.Millisecond,
2658	2661		}
2659	2662		svc := NewService(runtime, envLoader, eventBus, nil, config)
2660	2663		ctx := testContext()
2661	2664
2662		-	// NO tracked container in memory. resolveExistingContainer will NOT find it either
2663		-	// because the running container lacks the managed label in this edge case.
	2665	+	// The canonical container is running and has the managed label — it will be
	2666	+	// discovered by resolveExistingContainer's slow path (no in-memory state).
	2667	+	existingContainer := &domain.Container{
	2668	+	ID: "active-canonical",
	2669	+	Name: "gordon-test.example.com",
	2670	+	Status: "running",
	2671	+	Labels: map[string]string{domain.LabelManaged: "true"},
	2672	+	}
2664	2673
2665	2674		route := domain.Route{
2666	2675		Domain: "test.example.com",
2667	2676		Image: "myapp:v2",
2668	2677		}
2669	2678
2670		-	// resolveExistingContainer: ListContainers(ctx, false) returns empty —
2671		-	// the canonical container is running but not discovered (e.g., missing label)
2672		-	runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{}, nil)
	2679	+	// resolveExistingContainer: slow path finds the running canonical container via label.
	2680	+	runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{existingContainer}, nil)
2673	2681
2674		-	// cleanupOrphanedContainers: ListContainers(ctx, true) finds both the running
2675		-	// canonical container AND a stale -new leftover. The canonical container should
2676		-	// NOT be killed because it's running and has the canonical name.
	2682	+	// cleanupOrphanedContainers: finds the running canonical AND a stale -new leftover.
	2683	+	// The canonical container must NOT be killed because it's running.
2677	2684		runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{
2678		-	{
2679		-	ID: "active-canonical",
2680		-	Name: "gordon-test.example.com",
2681		-	Status: "running",
2682		-	},
	2685	+	existingContainer,
2683	2686		{
2684	2687		ID: "stale-leftover",
2685	2688		Name: "gordon-test.example.com-new",

@@ -2687,11 +2690,11 @@ func TestService_Deploy_OrphanCleanup_NeverKillsRunningCanonical(t *testing.T) {
2687	2690		},
2688	2691		}, nil)
2689	2692
2690		-	// Only the stale -new container should be stopped and removed during orphan cleanup
	2693	+	// Only the stale -new container should be stopped and removed during orphan cleanup.
2691	2694		runtime.EXPECT().StopContainer(mock.Anything, "stale-leftover").Return(nil).Once()
2692	2695		runtime.EXPECT().RemoveContainer(mock.Anything, "stale-leftover", true).Return(nil).Once()
2693	2696
2694		-	// Image operations
	2697	+	// Image operations: pull new image (existing.Image is empty so redundancy check is skipped)
2695	2698		runtime.EXPECT().ListImages(mock.Anything).Return([]string{}, nil)
2696	2699		runtime.EXPECT().PullImage(mock.Anything, "myapp:v2").Return(nil)
2697	2700		runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:v2").Return([]int{8080}, nil)

@@ -2700,15 +2703,17 @@ func TestService_Deploy_OrphanCleanup_NeverKillsRunningCanonical(t *testing.T) {
2700	2703		envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil)
2701	2704		runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:v2").Return([]string{}, nil)
2702	2705
2703		-	// Create container — no existing tracked, so canonical name is used
2704		-	newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com", Status: "created"}
	2706	+	// Create container — existing is the canonical container, so the new one gets "-new" suffix.
	2707	+	// Docker enforces unique container names; using "-new" avoids a name collision.
	2708	+	newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-new", Status: "created"}
2705	2709		runtime.EXPECT().CreateContainer(mock.Anything, mock.MatchedBy(func(cfg *domain.ContainerConfig) bool {
2706		-	return cfg.Name == "gordon-test.example.com"
	2710	+	return cfg.Name == "gordon-test.example.com-new"
2707	2711		})).Return(newContainer, nil)
2708	2712		runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil)
2709	2713
2710		-	// Wait for ready
2711		-	runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2)
	2714	+	// Wait for ready (delay mode): pollContainerRunning (1×) + waitForReadyByDelay (1×)
	2715	+	// Post-switch stabilization: 1× — total 3 calls.
	2716	+	runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(3)
2712	2717
2713	2718		// Inspect after ready
2714	2719		runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{

@@ -2717,6 +2722,13 @@ func TestService_Deploy_OrphanCleanup_NeverKillsRunningCanonical(t *testing.T) {
2717	2722		Ports: []int{8080},
2718	2723		}, nil)
2719	2724
	2725	+	// Rename new container to canonical name after stabilization.
	2726	+	runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil).Once()
	2727	+
	2728	+	// Stop and remove old canonical container.
	2729	+	runtime.EXPECT().StopContainer(mock.Anything, "active-canonical").Return(nil).Once()
	2730	+	runtime.EXPECT().RemoveContainer(mock.Anything, "active-canonical", true).Return(nil).Once()
	2731	+
2720	2732		// Publish event
2721	2733		eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil)
2722	2734

 }
 // TestService_Deploy_OrphanCleanup_NeverKillsRunningCanonical verifies that orphan
-// cleanup never stops a running container with the canonical name, even if it is not
+// cleanup never stops a running container with the canonical name. Only stopped
-// tracked in memory and was not resolved by resolveExistingContainer. Only stopped
 // canonical containers and temp containers (-new/-next) should be cleaned up.
+// The active canonical container is resolved via resolveExistingContainer (it has the
+// managed label), so the new container is created with the "-new" suffix — Docker
+// disallows creating a container with the same name as an existing running container.
 func TestService_Deploy_OrphanCleanup_NeverKillsRunningCanonical(t *testing.T) {
 	runtime := mocks.NewMockContainerRuntime(t)
 	envLoader := mocks.NewMockEnvLoader(t)
 	eventBus := mocks.NewMockEventPublisher(t)
 	config := Config{
-		ReadinessDelay: time.Millisecond,
+		ReadinessDelay:     time.Millisecond,
-		DrainDelay:     time.Millisecond,
+		DrainDelay:         time.Millisecond,
+		StabilizationDelay: time.Millisecond,
 	}
 	svc := NewService(runtime, envLoader, eventBus, nil, config)
 	ctx := testContext()
-	// NO tracked container in memory. resolveExistingContainer will NOT find it either
+	// The canonical container is running and has the managed label — it will be
-	// because the running container lacks the managed label in this edge case.
+	// discovered by resolveExistingContainer's slow path (no in-memory state).
+	existingContainer := &domain.Container{
+		ID:     "active-canonical",
+		Name:   "gordon-test.example.com",
+		Status: "running",
+		Labels: map[string]string{domain.LabelManaged: "true"},
+	}
 	route := domain.Route{
 		Domain: "test.example.com",
 		Image:  "myapp:v2",
 	}
-	// resolveExistingContainer: ListContainers(ctx, false) returns empty —
+	// resolveExistingContainer: slow path finds the running canonical container via label.
-	// the canonical container is running but not discovered (e.g., missing label)
+	runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{existingContainer}, nil)
-	runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{}, nil)
-	// cleanupOrphanedContainers: ListContainers(ctx, true) finds both the running
+	// cleanupOrphanedContainers: finds the running canonical AND a stale -new leftover.
-	// canonical container AND a stale -new leftover. The canonical container should
+	// The canonical container must NOT be killed because it's running.
-	// NOT be killed because it's running and has the canonical name.
 	runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{
-		{
+		existingContainer,
-			ID:     "active-canonical",
-			Name:   "gordon-test.example.com",
-			Status: "running",
-		},
 		{
 			ID:     "stale-leftover",
 			Name:   "gordon-test.example.com-new",
 		},
 	}, nil)
-	// Only the stale -new container should be stopped and removed during orphan cleanup
+	// Only the stale -new container should be stopped and removed during orphan cleanup.
 	runtime.EXPECT().StopContainer(mock.Anything, "stale-leftover").Return(nil).Once()
 	runtime.EXPECT().RemoveContainer(mock.Anything, "stale-leftover", true).Return(nil).Once()
-	// Image operations
+	// Image operations: pull new image (existing.Image is empty so redundancy check is skipped)
 	runtime.EXPECT().ListImages(mock.Anything).Return([]string{}, nil)
 	runtime.EXPECT().PullImage(mock.Anything, "myapp:v2").Return(nil)
 	runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:v2").Return([]int{8080}, nil)
 	envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil)
 	runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:v2").Return([]string{}, nil)
-	// Create container — no existing tracked, so canonical name is used
+	// Create container — existing is the canonical container, so the new one gets "-new" suffix.
-	newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com", Status: "created"}
+	// Docker enforces unique container names; using "-new" avoids a name collision.
+	newContainer := &domain.Container{ID: "new-container", Name: "gordon-test.example.com-new", Status: "created"}
 	runtime.EXPECT().CreateContainer(mock.Anything, mock.MatchedBy(func(cfg *domain.ContainerConfig) bool {
-		return cfg.Name == "gordon-test.example.com"
+		return cfg.Name == "gordon-test.example.com-new"
 	})).Return(newContainer, nil)
 	runtime.EXPECT().StartContainer(mock.Anything, "new-container").Return(nil)
-	// Wait for ready
+	// Wait for ready (delay mode): pollContainerRunning (1×) + waitForReadyByDelay (1×)
-	runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(2)
+	// Post-switch stabilization: 1×  — total 3 calls.
+	runtime.EXPECT().IsContainerRunning(mock.Anything, "new-container").Return(true, nil).Times(3)
 	// Inspect after ready
 	runtime.EXPECT().InspectContainer(mock.Anything, "new-container").Return(&domain.Container{
 		Ports:  []int{8080},
 	}, nil)
+	// Rename new container to canonical name after stabilization.
+	runtime.EXPECT().RenameContainer(mock.Anything, "new-container", "gordon-test.example.com").Return(nil).Once()
+	// Stop and remove old canonical container.
+	runtime.EXPECT().StopContainer(mock.Anything, "active-canonical").Return(nil).Once()
+	runtime.EXPECT().RemoveContainer(mock.Anything, "active-canonical", true).Return(nil).Once()
 	// Publish event
 	eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil)