fix(deployment): bound TCP readiness attempts by the readiness timeout

commit: d0140016b42cee2e9127302f5a36239f7a186813
author: brice · 5d ago · 2026-10-16 20:22 UTC
parent: a2f0e01
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

3 files · +71 -12

M internal/usecase/deployment/probes.go +3 -3


@@ -76,12 +76,12 @@ func tcpProbeOnce(ctx context.Context, addr string, settle time.Duration) error
76	76		defer stop()
77	77		var buf [1]byte
78	78		n, err := conn.Read(buf[:])
79		-	if n > 0 {
80		-	return nil
81		-	}
82	79		if ctxErr := ctx.Err(); ctxErr != nil {
83	80		return ctxErr
84	81		}
	82	+	if n > 0 {
	83	+	return nil
	84	+	}
85	85		var netErr net.Error
86	86		if errors.As(err, &netErr) && netErr.Timeout() {
87	87		return nil

 	defer stop()
 	var buf [1]byte
 	n, err := conn.Read(buf[:])
-	if n > 0 {
-		return nil
-	}
 	if ctxErr := ctx.Err(); ctxErr != nil {
 		return ctxErr
 	}
+	if n > 0 {
+		return nil
+	}
 	var netErr net.Error
 	if errors.As(err, &netErr) && netErr.Timeout() {
 		return nil

M internal/usecase/deployment/probes_tcp_test.go +54 -1


@@ -8,6 +8,8 @@ import (
8	8
9	9		"github.com/stretchr/testify/assert"
10	10		"github.com/stretchr/testify/require"
	11	+
	12	+	"github.com/bnema/gordon/internal/domain"
11	13		)
12	14
13	15		// serveTCP accepts connections on loopback and hands each to handle.

@@ -28,7 +30,8 @@ func serveTCP(t *testing.T, handle func(net.Conn)) string {
28	30		return ln.Addr().String()
29	31		}
30	32
31		-	const testSettle = 100 * time.Millisecond
	33	+	// testSettle leaves a wide margin for the server goroutine to close first.
	34	+	const testSettle = 500 * time.Millisecond
32	35
33	36		func TestTCPProbeOnce_RelayAcceptThenCloseIsNotReady(t *testing.T) {
34	37		// A host port relay with no listener behind it accepts and closes at once.

@@ -91,6 +94,56 @@ func TestTCPProbeOnce_NoListenerFails(t *testing.T) {
91	94		require.Error(t, tcpProbeOnce(context.Background(), addr, testSettle))
92	95		}
93	96
	97	+	func TestTCPProbeOnce_CanceledContextWinsOverData(t *testing.T) {
	98	+	addr := serveTCP(t, func(c net.Conn) {
	99	+	_, _ = c.Write([]byte("banner"))
	100	+	_ = c.Close()
	101	+	})
	102	+	ctx, cancel := context.WithCancel(context.Background())
	103	+	cancel()
	104	+
	105	+	require.Error(t, tcpProbeOnce(ctx, addr, testSettle))
	106	+	}
	107	+
	108	+	func TestWaitTCPReady_SettleAfterBudgetIsNotReady(t *testing.T) {
	109	+	// Each attempt settles only after the readiness budget expired: the wait
	110	+	// must report a timeout instead of accepting the late success.
	111	+	slowDial := func(ctx context.Context, _ string) error {
	112	+	select {
	113	+	case <-ctx.Done():
	114	+	case <-time.After(400 * time.Millisecond):
	115	+	}
	116	+	return nil
	117	+	}
	118	+	deps := NewTestProbeDeps(nil, nil, slowDial)
	119	+	spec := domain.AppService{
	120	+	Name: "relay",
	121	+	TCP: []domain.AppTCPInterface{{Entrypoint: "tcp", Port: 9000}},
	122	+	Readiness: domain.AppReadiness{Type: domain.AppReadinessTCP, Timeout: 200 * time.Millisecond},
	123	+	}
	124	+
	125	+	start := time.Now()
	126	+	err := waitTCPReadyWithDeps(context.Background(), deps, spec, map[int]int{9000: 40000})
	127	+
	128	+	require.ErrorContains(t, err, "TCP readiness timeout")
	129	+	assert.Less(t, time.Since(start), 5*time.Second)
	130	+	}
	131	+
	132	+	func TestWaitTCPReady_CallerCancelIsReturned(t *testing.T) {
	133	+	deps := NewTestProbeDeps(nil, nil, func(context.Context, string) error { return errTCPProbeClosed })
	134	+	spec := domain.AppService{
	135	+	Name: "relay",
	136	+	TCP: []domain.AppTCPInterface{{Entrypoint: "tcp", Port: 9000}},
	137	+	Readiness: domain.AppReadiness{Type: domain.AppReadinessTCP, Timeout: 10 * time.Second},
	138	+	}
	139	+	ctx, cancel := context.WithCancel(context.Background())
	140	+	cancel()
	141	+
	142	+	err := waitTCPReadyWithDeps(ctx, deps, spec, map[int]int{9000: 40000})
	143	+
	144	+	require.ErrorIs(t, err, context.Canceled)
	145	+	}
	146	+
94	147		func TestTCPProbeOnce_ContextCancelStopsTheSettleWait(t *testing.T) {
95	148		release := make(chan struct{})
96	149		t.Cleanup(func() { close(release) })


@@ -8,6 +8,8 @@ import (
8			8
9		"github.com/stretchr/testify/assert"	9		"github.com/stretchr/testify/assert"
10		"github.com/stretchr/testify/require"	10		"github.com/stretchr/testify/require"
			11	+
			12	+	"github.com/bnema/gordon/internal/domain"
11		)	13		)
12			14
13		// serveTCP accepts connections on loopback and hands each to handle.	15		// serveTCP accepts connections on loopback and hands each to handle.

@@ -28,7 +30,8 @@ func serveTCP(t *testing.T, handle func(net.Conn)) string {
28		return ln.Addr().String()	30		return ln.Addr().String()
29		}	31		}
30			32
31	-	const testSettle = 100 * time.Millisecond	33	+	// testSettle leaves a wide margin for the server goroutine to close first.
			34	+	const testSettle = 500 * time.Millisecond
32			35
33		func TestTCPProbeOnce_RelayAcceptThenCloseIsNotReady(t *testing.T) {	36		func TestTCPProbeOnce_RelayAcceptThenCloseIsNotReady(t *testing.T) {
34		// A host port relay with no listener behind it accepts and closes at once.	37		// A host port relay with no listener behind it accepts and closes at once.

@@ -91,6 +94,56 @@ func TestTCPProbeOnce_NoListenerFails(t *testing.T) {
91		require.Error(t, tcpProbeOnce(context.Background(), addr, testSettle))	94		require.Error(t, tcpProbeOnce(context.Background(), addr, testSettle))
92		}	95		}
93			96
			97	+	func TestTCPProbeOnce_CanceledContextWinsOverData(t *testing.T) {
			98	+	addr := serveTCP(t, func(c net.Conn) {
			99	+	_, _ = c.Write([]byte("banner"))
			100	+	_ = c.Close()
			101	+	})
			102	+	ctx, cancel := context.WithCancel(context.Background())
			103	+	cancel()
			104	+
			105	+	require.Error(t, tcpProbeOnce(ctx, addr, testSettle))
			106	+	}
			107	+
			108	+	func TestWaitTCPReady_SettleAfterBudgetIsNotReady(t *testing.T) {
			109	+	// Each attempt settles only after the readiness budget expired: the wait
			110	+	// must report a timeout instead of accepting the late success.
			111	+	slowDial := func(ctx context.Context, _ string) error {
			112	+	select {
			113	+	case <-ctx.Done():
			114	+	case <-time.After(400 * time.Millisecond):
			115	+	}
			116	+	return nil
			117	+	}
			118	+	deps := NewTestProbeDeps(nil, nil, slowDial)
			119	+	spec := domain.AppService{
			120	+	Name: "relay",
			121	+	TCP: []domain.AppTCPInterface{{Entrypoint: "tcp", Port: 9000}},
			122	+	Readiness: domain.AppReadiness{Type: domain.AppReadinessTCP, Timeout: 200 * time.Millisecond},
			123	+	}
			124	+
			125	+	start := time.Now()
			126	+	err := waitTCPReadyWithDeps(context.Background(), deps, spec, map[int]int{9000: 40000})
			127	+
			128	+	require.ErrorContains(t, err, "TCP readiness timeout")
			129	+	assert.Less(t, time.Since(start), 5*time.Second)
			130	+	}
			131	+
			132	+	func TestWaitTCPReady_CallerCancelIsReturned(t *testing.T) {
			133	+	deps := NewTestProbeDeps(nil, nil, func(context.Context, string) error { return errTCPProbeClosed })
			134	+	spec := domain.AppService{
			135	+	Name: "relay",
			136	+	TCP: []domain.AppTCPInterface{{Entrypoint: "tcp", Port: 9000}},
			137	+	Readiness: domain.AppReadiness{Type: domain.AppReadinessTCP, Timeout: 10 * time.Second},
			138	+	}
			139	+	ctx, cancel := context.WithCancel(context.Background())
			140	+	cancel()
			141	+
			142	+	err := waitTCPReadyWithDeps(ctx, deps, spec, map[int]int{9000: 40000})
			143	+
			144	+	require.ErrorIs(t, err, context.Canceled)
			145	+	}
			146	+
94		func TestTCPProbeOnce_ContextCancelStopsTheSettleWait(t *testing.T) {	147		func TestTCPProbeOnce_ContextCancelStopsTheSettleWait(t *testing.T) {
95		release := make(chan struct{})	148		release := make(chan struct{})
96		t.Cleanup(func() { close(release) })	149		t.Cleanup(func() { close(release) })

M internal/usecase/deployment/readiness.go +14 -8


@@ -344,20 +344,26 @@ func waitTCPReadyWithDeps(ctx context.Context, deps ProbeDeps, spec domain.AppSe
344	344		return err
345	345		}
346	346		addr := net.JoinHostPort("127.0.0.1", itoa(port))
347		-	deadline := time.Now().Add(timeout)
348		-	for {
	347	+	// Every attempt and retry wait runs under the readiness budget, so a
	348	+	// connection that only settles after the budget expired is never accepted.
	349	+	attemptCtx, cancel := context.WithTimeout(ctx, timeout)
	350	+	defer cancel()
	351	+	timedOut := func() error {
349	352		if err := ctx.Err(); err != nil {
350	353		return err
351	354		}
352		-	if err := deps.tcpDial(ctx, addr); err == nil {
353		-	return nil
	355	+	return fmt.Errorf("deployment: TCP readiness timeout after %s: %s not reachable", timeout, addr)
	356	+	}
	357	+	for {
	358	+	if attemptCtx.Err() != nil {
	359	+	return timedOut()
354	360		}
355		-	if time.Now().After(deadline) {
356		-	return fmt.Errorf("deployment: TCP readiness timeout after %s: %s not reachable", timeout, addr)
	361	+	if err := deps.tcpDial(attemptCtx, addr); err == nil && attemptCtx.Err() == nil {
	362	+	return nil
357	363		}
358	364		select {
359		-	case <-ctx.Done():
360		-	return ctx.Err()
	365	+	case <-attemptCtx.Done():
	366	+	return timedOut()
361	367		case <-time.After(500 * time.Millisecond):
362	368		}
363	369		}

 		return err
 	}
 	addr := net.JoinHostPort("127.0.0.1", itoa(port))
-	deadline := time.Now().Add(timeout)
+	// Every attempt and retry wait runs under the readiness budget, so a
-	for {
+	// connection that only settles after the budget expired is never accepted.
+	attemptCtx, cancel := context.WithTimeout(ctx, timeout)
+	defer cancel()
+	timedOut := func() error {
 		if err := ctx.Err(); err != nil {
 			return err
 		}
-		if err := deps.tcpDial(ctx, addr); err == nil {
+		return fmt.Errorf("deployment: TCP readiness timeout after %s: %s not reachable", timeout, addr)
-			return nil
+	}
+	for {
+		if attemptCtx.Err() != nil {
+			return timedOut()
 		}
-		if time.Now().After(deadline) {
+		if err := deps.tcpDial(attemptCtx, addr); err == nil && attemptCtx.Err() == nil {
-			return fmt.Errorf("deployment: TCP readiness timeout after %s: %s not reachable", timeout, addr)
+			return nil
 		}
 		select {
-		case <-ctx.Done():
+		case <-attemptCtx.Done():
-			return ctx.Err()
+			return timedOut()
 		case <-time.After(500 * time.Millisecond):
 		}
 	}