From 15ba2b8e6e762934eba8e74c4f9055e69bbb3d9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= Date: Wed, 26 Jan 2022 14:51:06 +0100 Subject: [PATCH 1/3] Making TestDialWithBackoff work without special environment --- network/transports.go | 5 +- network/transports_test.go | 153 +++++++++++++++++++++++++------------ 2 files changed, 107 insertions(+), 51 deletions(-) diff --git a/network/transports.go b/network/transports.go index d96eda1177..c97d0052e1 100644 --- a/network/transports.go +++ b/network/transports.go @@ -55,6 +55,9 @@ var backOffTemplate = wait.Backoff{ Steps: 15, } +// ErrTimeoutDialing when the timeout is reached after set amount of time. +var ErrTimeoutDialing = errors.New("timed out dialing") + // DialWithBackOff executes `net.Dialer.DialContext()` with exponentially increasing // dial timeouts. In addition it sleeps with random jitter between tries. var DialWithBackOff = NewBackoffDialer(backOffTemplate) @@ -110,7 +113,7 @@ func dialBackOffHelper(ctx context.Context, network, address string, bo wait.Bac return c, nil } elapsed := time.Since(start) - return nil, fmt.Errorf("timed out dialing after %.2fs", elapsed.Seconds()) + return nil, fmt.Errorf("%w after %.2fs", ErrTimeoutDialing, elapsed.Seconds()) } func newHTTPTransport(disableKeepAlives, disableCompression bool, maxIdle, maxIdlePerHost int) http.RoundTripper { diff --git a/network/transports_test.go b/network/transports_test.go index 7691e16280..156fe3aaf2 100644 --- a/network/transports_test.go +++ b/network/transports_test.go @@ -20,20 +20,20 @@ import ( "context" "crypto/tls" "crypto/x509" + "errors" + "fmt" + "io" "net" "net/http" "net/http/httptest" "strings" + "syscall" "testing" + "time" "k8s.io/apimachinery/pkg/util/sets" ) -const ( - timeoutErr = "timed out dialing" - connectionRefusedErr = "connection refused" -) - func TestHTTPRoundTripper(t *testing.T) { wants := sets.NewString() frt := func(key string) http.RoundTripper { @@ -77,66 +77,119 @@ func TestHTTPRoundTripper(t *testing.T) { } func TestDialWithBackoff(t *testing.T) { - // Make the test short. - bo := backOffTemplate - bo.Steps = 2 - - // Nobody's listening on a random port. Usually. - c, err := dialBackOffHelper(context.Background(), "tcp4", "127.0.0.1:41482", bo, nil) - verifyFailedConnection(t, c, err, connectionRefusedErr) - - // Timeout. Use special testing IP address. - c, err = dialBackOffHelper(context.Background(), "tcp4", "198.18.0.254:8888", bo, nil) - verifyFailedConnection(t, c, err, timeoutErr) - - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) - defer s.Close() - - c, err = DialWithBackOff(context.Background(), "tcp4", strings.TrimPrefix(s.URL, "http://")) - if err != nil { - t.Fatal("Dial error =", err) - } - c.Close() + var tlsConf *tls.Config + t.Parallel() + t.Run("ConnectionRefused", testDialWithBackoffConnectionRefused(tlsConf)) + t.Run("Timeout", testDialWithBackoffTimeout(tlsConf)) + t.Run("Success", testDialWithBackoffSuccess(tlsConf)) } func TestDialTLSWithBackoff(t *testing.T) { - // Make the test short. - bo := backOffTemplate - bo.Steps = 2 - tlsConf := &tls.Config{ InsecureSkipVerify: false, ServerName: "example.com", MinVersion: tls.VersionTLS12, } + t.Parallel() + t.Run("ConnectionRefused", testDialWithBackoffConnectionRefused(tlsConf)) + t.Run("Timeout", testDialWithBackoffTimeout(tlsConf)) + t.Run("Success", testDialWithBackoffSuccess(tlsConf)) +} - // Nobody's listening on a random port. Usually. - c, err := dialBackOffHelper(context.Background(), "tcp4", "127.0.0.1:41482", bo, tlsConf) - verifyFailedConnection(t, c, err, connectionRefusedErr) +func testDialWithBackoffConnectionRefused(tlsConf *tls.Config) func(t *testing.T) { + ctx := context.TODO() + return func(t *testing.T) { + port := findUnusedPortOrFail(t) + addr := fmt.Sprintf("127.0.0.1:%d", port) + c, err := dialer(ctx, tlsConf)(addr) + closeOrFail(t, c) + if !errors.Is(err, syscall.ECONNREFUSED) { + t.Errorf("Unexpected error: %+v", err) + } + } +} - // Timeout. Use special testing IP address. - c, err = dialBackOffHelper(context.Background(), "tcp4", "198.18.0.254:8888", bo, tlsConf) - verifyFailedConnection(t, c, err, timeoutErr) +func testDialWithBackoffTimeout(tlsConf *tls.Config) func(t *testing.T) { + return func(t *testing.T) { + // Timeout. Use non-routable IP. See: https://stackoverflow.com/a/31581323/844449 + c, err := dialer(context.TODO(), tlsConf)("10.0.0.0:81") + if err == nil { + closeOrFail(t, c) + t.Error("Unexpected success dialing") + } + if !errors.Is(err, ErrTimeoutDialing) { + t.Errorf("Unexpected error: %+v", err) + } + } +} - s := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) - defer s.Close() +func testDialWithBackoffSuccess(tlsConf *tls.Config) func(t *testing.T) { + //goland:noinspection HttpUrlsUsage + const ( + prefixHTTP = "http://" + prefixHTTPS = "https://" + ) + ctx := context.TODO() + return func(t *testing.T) { + var s *httptest.Server + servFn := httptest.NewServer + if tlsConf != nil { + servFn = httptest.NewTLSServer + } + s = servFn(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + defer s.Close() + prefix := prefixHTTP + if tlsConf != nil { + servFn = httptest.NewTLSServer + prefix = prefixHTTPS + rootCAs := x509.NewCertPool() + rootCAs.AddCert(s.Certificate()) + tlsConf.RootCAs = rootCAs + } + addr := strings.TrimPrefix(s.URL, prefix) + + c, err := dialer(ctx, tlsConf)(addr) + if err != nil { + t.Fatal("Dial error =", err) + } + closeOrFail(t, c) + } +} + +func dialer(ctx context.Context, tlsConf *tls.Config) func(addr string) (net.Conn, error) { + // Make the test short. + bo := backOffTemplate + bo.Steps = 1 - rootCAs := x509.NewCertPool() - rootCAs.AddCert(s.Certificate()) - tlsConf.RootCAs = rootCAs + dialFn := func(addr string) (net.Conn, error) { + bo.Duration = time.Millisecond + return NewBackoffDialer(bo)(ctx, "tcp4", addr) + } + if tlsConf != nil { + dialFn = func(addr string) (net.Conn, error) { + bo.Duration = 10 * time.Millisecond + return NewTLSBackoffDialer(bo)(ctx, "tcp4", addr, tlsConf) + } + } + return dialFn +} - c, err = DialTLSWithBackOff(context.Background(), "tcp4", strings.TrimPrefix(s.URL, "https://"), tlsConf) - if err != nil { - t.Fatal("Dial error =", err) +func closeOrFail(tb testing.TB, con io.Closer) { + tb.Helper() + if con == nil { + return + } + if err := con.Close(); err != nil { + tb.Fatal(err) } - c.Close() } -func verifyFailedConnection(t *testing.T, c net.Conn, err error, prefix string) { - if err == nil { - c.Close() - t.Error("Unexpected success dialing") - } else if !strings.Contains(err.Error(), prefix) { - t.Errorf("Error = %v, want: %s(...)", err, prefix) +func findUnusedPortOrFail(tb testing.TB) int { + tb.Helper() + l, err := net.Listen("tcp", "localhost:0") + if err != nil { + tb.Fatal(err) } + defer closeOrFail(tb, l) + return l.Addr().(*net.TCPAddr).Port } From dbe1f752dce233bac66989bc57628f1c55129134 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Mon, 25 Apr 2022 04:58:09 -0700 Subject: [PATCH 2/3] Use listen to force a connection timeout (#1) --- network/transports_test.go | 47 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/network/transports_test.go b/network/transports_test.go index 156fe3aaf2..4ee37fd67f 100644 --- a/network/transports_test.go +++ b/network/transports_test.go @@ -111,8 +111,20 @@ func testDialWithBackoffConnectionRefused(tlsConf *tls.Config) func(t *testing.T func testDialWithBackoffTimeout(tlsConf *tls.Config) func(t *testing.T) { return func(t *testing.T) { - // Timeout. Use non-routable IP. See: https://stackoverflow.com/a/31581323/844449 - c, err := dialer(context.TODO(), tlsConf)("10.0.0.0:81") + // Create a listening socket with backlog 1, then occupy the backlog to force a timeout. + closer, addr, err := listenOne() + if err != nil { + t.Fatal("Unable to create listener:", err) + } + defer closer() + c1, err := net.Dial("tcp4", addr.String()) + if err != nil { + t.Fatalf("Unable to connect to server on %s: %s", addr, err) + } + defer c1.Close() + + // Since the backlog is full, the next request must time out. + c, err := dialer(context.TODO(), tlsConf)(addr.String()) if err == nil { closeOrFail(t, c) t.Error("Unexpected success dialing") @@ -193,3 +205,34 @@ func findUnusedPortOrFail(tb testing.TB) int { defer closeOrFail(tb, l) return l.Addr().(*net.TCPAddr).Port } + +// Golang doesn't allow us to set the backlog argument on syscall.Listen from +// net.ListenTCP, so we need to get directly into syscall land. +func listenOne() (func(), *net.TCPAddr, error) { + fd, err := syscall.Socket(syscall.AF_INET, syscall.SOCK_STREAM, 0) + if err != nil { + return nil, nil, fmt.Errorf("Couldn't get socket: %w", err) + } + sa := &syscall.SockaddrInet4{ + Port: 0, + Addr: [4]byte{127, 0, 0, 1}, + } + if err = syscall.Bind(fd, sa); err != nil { + return nil, nil, fmt.Errorf("Unable to bind: %w", err) + } + if err = syscall.Listen(fd, 0); err != nil { + return nil, nil, fmt.Errorf("Unable to Listen: %w", err) + } + closer := func() { syscall.Close(fd) } + listenaddr, err := syscall.Getsockname(fd) + if err != nil { + closer() + return nil, nil, fmt.Errorf("Could not get sockname: %w", err) + } + sa = listenaddr.(*syscall.SockaddrInet4) + addr := &net.TCPAddr{ + IP: sa.Addr[:], + Port: sa.Port, + } + return closer, addr, nil +} From abf66bbceb9c6c983f4d6634ddafead00b582841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= Date: Mon, 25 Apr 2022 15:16:26 +0200 Subject: [PATCH 3/3] Don't parallelize tests to avoid free port conflicts --- network/transports_test.go | 197 +++++++++++++++++++++---------------- 1 file changed, 111 insertions(+), 86 deletions(-) diff --git a/network/transports_test.go b/network/transports_test.go index 4ee37fd67f..4aeff2f085 100644 --- a/network/transports_test.go +++ b/network/transports_test.go @@ -32,6 +32,7 @@ import ( "time" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/wait" ) func TestHTTPRoundTripper(t *testing.T) { @@ -76,158 +77,177 @@ func TestHTTPRoundTripper(t *testing.T) { } } -func TestDialWithBackoff(t *testing.T) { - var tlsConf *tls.Config - t.Parallel() - t.Run("ConnectionRefused", testDialWithBackoffConnectionRefused(tlsConf)) - t.Run("Timeout", testDialWithBackoffTimeout(tlsConf)) - t.Run("Success", testDialWithBackoffSuccess(tlsConf)) +func TestDialWithBackoffConnectionRefused(t *testing.T) { + testDialWithBackoffConnectionRefused(nil, t) } -func TestDialTLSWithBackoff(t *testing.T) { - tlsConf := &tls.Config{ - InsecureSkipVerify: false, - ServerName: "example.com", - MinVersion: tls.VersionTLS12, - } - t.Parallel() - t.Run("ConnectionRefused", testDialWithBackoffConnectionRefused(tlsConf)) - t.Run("Timeout", testDialWithBackoffTimeout(tlsConf)) - t.Run("Success", testDialWithBackoffSuccess(tlsConf)) +func TestDialWithBackoffTimeout(t *testing.T) { + testDialWithBackoffTimeout(nil, t) +} + +func TestDialWithBackoffSuccess(t *testing.T) { + testDialWithBackoffSuccess(nil, t) +} + +func TestDialTLSWithBackoffConnectionRefused(t *testing.T) { + testDialWithBackoffConnectionRefused(exampleTlsConf(), t) +} + +func TestDialTLSWithBackoffTimeout(t *testing.T) { + testDialWithBackoffTimeout(exampleTlsConf(), t) +} + +func TestDialTLSWithBackoffSuccess(t *testing.T) { + testDialWithBackoffSuccess(exampleTlsConf(), t) } -func testDialWithBackoffConnectionRefused(tlsConf *tls.Config) func(t *testing.T) { +func testDialWithBackoffConnectionRefused(tlsConf *tls.Config, t testingT) { ctx := context.TODO() - return func(t *testing.T) { - port := findUnusedPortOrFail(t) - addr := fmt.Sprintf("127.0.0.1:%d", port) - c, err := dialer(ctx, tlsConf)(addr) - closeOrFail(t, c) - if !errors.Is(err, syscall.ECONNREFUSED) { - t.Errorf("Unexpected error: %+v", err) - } + port := findUnusedPortOrFail(t) + addr := fmt.Sprintf("127.0.0.1:%d", port) + dialer := newDialer(ctx, tlsConf) + c, err := dialer(addr) + closeOrFail(t, c) + if !errors.Is(err, syscall.ECONNREFUSED) { + t.Fatalf("Unexpected error: %+v", err) } } -func testDialWithBackoffTimeout(tlsConf *tls.Config) func(t *testing.T) { - return func(t *testing.T) { - // Create a listening socket with backlog 1, then occupy the backlog to force a timeout. - closer, addr, err := listenOne() - if err != nil { - t.Fatal("Unable to create listener:", err) - } - defer closer() - c1, err := net.Dial("tcp4", addr.String()) - if err != nil { - t.Fatalf("Unable to connect to server on %s: %s", addr, err) - } - defer c1.Close() +func testDialWithBackoffTimeout(tlsConf *tls.Config, t testingT) { + ctx := context.TODO() + closer, addr, err := listenOne() + if err != nil { + t.Fatal("Unable to create listener:", err) + } + defer closer() + c1, err := net.Dial("tcp4", addr.String()) + if err != nil { + t.Fatalf("Unable to connect to server on %s: %s", addr, err) + } + defer closeOrFail(t, c1) - // Since the backlog is full, the next request must time out. - c, err := dialer(context.TODO(), tlsConf)(addr.String()) - if err == nil { - closeOrFail(t, c) - t.Error("Unexpected success dialing") - } - if !errors.Is(err, ErrTimeoutDialing) { - t.Errorf("Unexpected error: %+v", err) - } + // Since the backlog is full, the next request must time out. + dialer := newDialer(ctx, tlsConf) + c, err := dialer(addr.String()) + if err == nil { + closeOrFail(t, c) + t.Fatal("Unexpected success dialing") + } + if !errors.Is(err, ErrTimeoutDialing) { + t.Fatalf("Unexpected error: %+v", err) } } -func testDialWithBackoffSuccess(tlsConf *tls.Config) func(t *testing.T) { +func testDialWithBackoffSuccess(tlsConf *tls.Config, t testingT) { //goland:noinspection HttpUrlsUsage const ( prefixHTTP = "http://" prefixHTTPS = "https://" ) ctx := context.TODO() - return func(t *testing.T) { - var s *httptest.Server - servFn := httptest.NewServer - if tlsConf != nil { - servFn = httptest.NewTLSServer - } - s = servFn(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) - defer s.Close() - prefix := prefixHTTP - if tlsConf != nil { - servFn = httptest.NewTLSServer - prefix = prefixHTTPS - rootCAs := x509.NewCertPool() - rootCAs.AddCert(s.Certificate()) - tlsConf.RootCAs = rootCAs - } - addr := strings.TrimPrefix(s.URL, prefix) + var s *httptest.Server + servFn := httptest.NewServer + if tlsConf != nil { + servFn = httptest.NewTLSServer + } + s = servFn(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + defer s.Close() + prefix := prefixHTTP + if tlsConf != nil { + prefix = prefixHTTPS + rootCAs := x509.NewCertPool() + rootCAs.AddCert(s.Certificate()) + tlsConf.RootCAs = rootCAs + } + addr := strings.TrimPrefix(s.URL, prefix) - c, err := dialer(ctx, tlsConf)(addr) - if err != nil { - t.Fatal("Dial error =", err) - } - closeOrFail(t, c) + dialer := newDialer(ctx, tlsConf) + c, err := dialer(addr) + if err != nil { + t.Fatal("Dial error =", err) } + closeOrFail(t, c) } -func dialer(ctx context.Context, tlsConf *tls.Config) func(addr string) (net.Conn, error) { +func exampleTlsConf() *tls.Config { + return &tls.Config{ + InsecureSkipVerify: false, + ServerName: "example.com", + MinVersion: tls.VersionTLS12, + } +} + +func newDialer(ctx context.Context, tlsConf *tls.Config) func(addr string) (net.Conn, error) { // Make the test short. - bo := backOffTemplate - bo.Steps = 1 + bo := wait.Backoff{ + Duration: time.Millisecond, + Factor: 1.4, + Jitter: 0.1, // At most 10% jitter. + Steps: 1, + } dialFn := func(addr string) (net.Conn, error) { - bo.Duration = time.Millisecond return NewBackoffDialer(bo)(ctx, "tcp4", addr) } if tlsConf != nil { dialFn = func(addr string) (net.Conn, error) { bo.Duration = 10 * time.Millisecond + bo.Steps = 3 return NewTLSBackoffDialer(bo)(ctx, "tcp4", addr, tlsConf) } } return dialFn } -func closeOrFail(tb testing.TB, con io.Closer) { - tb.Helper() +func closeOrFail(t testingT, con io.Closer) { if con == nil { return } if err := con.Close(); err != nil { - tb.Fatal(err) + t.Fatal(err) } } -func findUnusedPortOrFail(tb testing.TB) int { - tb.Helper() +func findUnusedPortOrFail(t testingT) int { l, err := net.Listen("tcp", "localhost:0") if err != nil { - tb.Fatal(err) + t.Fatal(err) } - defer closeOrFail(tb, l) + defer closeOrFail(t, l) return l.Addr().(*net.TCPAddr).Port } +var errTest = errors.New("testing") + +func newTestErr(msg string, err error) error { + return fmt.Errorf("%w: %s: %v", errTest, msg, err) +} + +// listenOne creates a socket with backlog of one, and use that socket, so +// any other connection will guarantee to timeout. +// // Golang doesn't allow us to set the backlog argument on syscall.Listen from // net.ListenTCP, so we need to get directly into syscall land. func listenOne() (func(), *net.TCPAddr, error) { fd, err := syscall.Socket(syscall.AF_INET, syscall.SOCK_STREAM, 0) if err != nil { - return nil, nil, fmt.Errorf("Couldn't get socket: %w", err) + return nil, nil, newTestErr("Couldn't get socket", err) } sa := &syscall.SockaddrInet4{ Port: 0, Addr: [4]byte{127, 0, 0, 1}, } if err = syscall.Bind(fd, sa); err != nil { - return nil, nil, fmt.Errorf("Unable to bind: %w", err) + return nil, nil, newTestErr("Unable to bind", err) } if err = syscall.Listen(fd, 0); err != nil { - return nil, nil, fmt.Errorf("Unable to Listen: %w", err) + return nil, nil, newTestErr("Unable to Listen", err) } - closer := func() { syscall.Close(fd) } + closer := func() { _ = syscall.Close(fd) } listenaddr, err := syscall.Getsockname(fd) if err != nil { closer() - return nil, nil, fmt.Errorf("Could not get sockname: %w", err) + return nil, nil, newTestErr("Could not get sockname", err) } sa = listenaddr.(*syscall.SockaddrInet4) addr := &net.TCPAddr{ @@ -236,3 +256,8 @@ func listenOne() (func(), *net.TCPAddr, error) { } return closer, addr, nil } + +type testingT interface { + Fatal(args ...interface{}) + Fatalf(format string, args ...interface{}) +}