Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion reaper.go
Original file line number Diff line number Diff line change
Expand Up @@ -536,8 +536,13 @@ func (r *Reaper) useTermSignal() chan bool {
// It returns a channel that can be sent true to terminate the connection.
// Returns an error if config.RyukDisabled is true.
func (r *Reaper) connect(ctx context.Context) (chan bool, error) {
// Bound the dial: with dropped SYNs it would sit in the kernel's ~2min
// connect timeout and eat the spawner's 20s backoff budget in one attempt.
dialCtx, cancel := context.WithTimeout(ctx, time.Second*5)
defer cancel()

var d net.Dialer
conn, err := d.DialContext(ctx, "tcp", r.Endpoint)
conn, err := d.DialContext(dialCtx, "tcp", r.Endpoint)
if err != nil {
return nil, fmt.Errorf("dial reaper %s: %w", r.Endpoint, err)
}
Expand Down
33 changes: 33 additions & 0 deletions reaper_dial_bound_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package testcontainers

import (
"context"
"errors"
"testing"
"time"

"github.com/cenkalti/backoff/v4"
)

// A blackholed endpoint (SYNs dropped, no RST) must fail within the 5s dial
// bound and be classified retryable, so the spawner's 20s backoff can retry.
func TestReaperConnectDialBounded(t *testing.T) {
r := &Reaper{Endpoint: "192.0.2.1:8080"} // TEST-NET-1, not routed
start := time.Now()
_, err := r.connect(context.Background())
elapsed := time.Since(start)

if err == nil {
t.Fatal("expected dial error")
}
if elapsed > 10*time.Second {
t.Fatalf("dial not bounded: took %v", elapsed)
}
s := &reaperSpawner{}
rerr := s.retryError(err)
var perm *backoff.PermanentError
if errors.As(rerr, &perm) {
t.Fatalf("timeout classified permanent, backoff would stop: %v", rerr)
Comment on lines +15 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Make the timeout regression test deterministic and enforce the five-second bound.

192.0.2.1 does not guarantee a SYN timeout on every CI host. The dial can return ENETUNREACH or ECONNREFUSED immediately. reaper.go Lines 237-248 can classify ENETUNREACH as permanent, and an immediate ECONNREFUSED lets an implementation without the new timeout pass this test.

Inject the dialer or dial function and block it until the child context expires. Assert a deadline or timeout error. Set the elapsed limit to five seconds plus a small scheduling margin instead of ten seconds.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@reaper_dial_bound_test.go` around lines 15 - 30, Make the connect timeout
regression test deterministic by injecting a dialer or dial function into
Reaper.connect that blocks until the derived child context expires, rather than
dialing 192.0.2.1. Assert that the returned error is a context deadline or
timeout error, retain the retryError classification check, and tighten the
elapsed-time assertion to five seconds plus a small scheduling margin.

}
t.Logf("dial failed after %v, retryable: %v", elapsed, err)
}