fix: Improve speedtest by adding direct connection toggle (#3919)

It's weird to test connection speeds over DERP, because most
connections will eventually migrate to direct.

.github/workflows/release.yaml

@@ -47,7 +47,7 @@ jobs:
         uses: docker/login-action@v2
         with:
           registry: ghcr.io
-          username: ${{ github.repository_owner }}
+          username: ${{ github.actor }}
           password: ${{ secrets.GITHUB_TOKEN }}
 
       - uses: actions/setup-go@v3

agent/agent.go

@@ -270,7 +270,11 @@ func (a *agent) runTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) {
 				a.logger.Debug(ctx, "speedtest listener failed", slog.Error(err))
 				return
 			}
+			a.closeMutex.Lock()
+			a.connCloseWait.Add(1)
+			a.closeMutex.Unlock()
 			go func() {
+				defer a.connCloseWait.Done()
 				_ = speedtest.ServeConn(conn)
 			}()
 		}

cli/speedtest.go

@@ -5,20 +5,23 @@ import (
 	"fmt"
 	"time"
 
-	"cdr.dev/slog"
-	"github.com/coder/coder/cli/cliflag"
-	"github.com/coder/coder/cli/cliui"
-	"github.com/coder/coder/codersdk"
 	"github.com/jedib0t/go-pretty/v6/table"
 	"github.com/spf13/cobra"
 	"golang.org/x/xerrors"
 	tsspeedtest "tailscale.com/net/speedtest"
+
+	"cdr.dev/slog"
+	"github.com/coder/coder/agent"
+	"github.com/coder/coder/cli/cliflag"
+	"github.com/coder/coder/cli/cliui"
+	"github.com/coder/coder/codersdk"
 )
 
 func speedtest() *cobra.Command {
 	var (
-		reverse bool
-		timeStr string
+		direct   bool
+		duration time.Duration
+		reverse  bool
 	)
 	cmd := &cobra.Command{
 		Annotations: workspaceCommand,
@@ -28,11 +31,6 @@ func speedtest() *cobra.Command {
 			ctx, cancel := context.WithCancel(cmd.Context())
 			defer cancel()
 
-			dur, err := time.ParseDuration(timeStr)
-			if err != nil {
-				return err
-			}
-
 			client, err := CreateClient(cmd)
 			if err != nil {
 				return xerrors.Errorf("create codersdk client: %w", err)
@@ -57,13 +55,38 @@ func speedtest() *cobra.Command {
 				return err
 			}
 			defer conn.Close()
-			_, _ = conn.Ping()
+			if direct {
+				ticker := time.NewTicker(time.Second)
+				defer ticker.Stop()
+				for {
+					select {
+					case <-ctx.Done():
+						return ctx.Err()
+					case <-ticker.C:
+					}
+					dur, err := conn.Ping()
+					if err != nil {
+						continue
+					}
+					tc, _ := conn.(*agent.TailnetConn)
+					status := tc.Status()
+					if len(status.Peers()) != 1 {
+						continue
+					}
+					peer := status.Peer[status.Peers()[0]]
+					if peer.CurAddr == "" {
+						cmd.Printf("Waiting for a direct connection... (%dms via %s)\n", dur.Milliseconds(), peer.Relay)
+						continue
+					}
+					break
+				}
+			}
 			dir := tsspeedtest.Download
 			if reverse {
 				dir = tsspeedtest.Upload
 			}
-			cmd.Printf("Starting a %ds %s test...\n", int(dur.Seconds()), dir)
-			results, err := conn.Speedtest(dir, dur)
+			cmd.Printf("Starting a %ds %s test...\n", int(duration.Seconds()), dir)
+			results, err := conn.Speedtest(dir, duration)
 			if err != nil {
 				return err
 			}
@@ -83,9 +106,11 @@ func speedtest() *cobra.Command {
 			return err
 		},
 	}
+	cliflag.BoolVarP(cmd.Flags(), &direct, "direct", "d", "", false,
+		"Specifies whether to wait for a direct connection before testing speed.")
 	cliflag.BoolVarP(cmd.Flags(), &reverse, "reverse", "r", "", false,
 		"Specifies whether to run in reverse mode where the client receives and the server sends.")
-	cliflag.StringVarP(cmd.Flags(), &timeStr, "time", "t", "", "5s",
+	cmd.Flags().DurationVarP(&duration, "time", "t", tsspeedtest.DefaultDuration,
 		"Specifies the duration to monitor traffic.")
 	return cmd
 }

cli/speedtest_test.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	"cdr.dev/slog/sloggers/slogtest"
 	"github.com/coder/coder/agent"
 	"github.com/coder/coder/cli/clitest"
@@ -11,7 +13,6 @@ import (
 	"github.com/coder/coder/codersdk"
 	"github.com/coder/coder/pty/ptytest"
 	"github.com/coder/coder/testutil"
-	"github.com/stretchr/testify/assert"
 )
 
 func TestSpeedtest(t *testing.T) {

go.mod

@@ -49,7 +49,7 @@ replace github.com/tcnksm/go-httpstat => github.com/kylecarbs/go-httpstat v0.0.0
 
 // There are a few minor changes we make to Tailscale that we're slowly upstreaming. Compare here:
 // https://github.com/tailscale/tailscale/compare/main...coder:tailscale:main
-replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220905194158-291661887d25
+replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220907030728-c713fe41e3e6
 
 require (
 	cdr.dev/slog v1.4.2-0.20220525200111-18dce5c2cd5f

go.sum

@@ -352,8 +352,8 @@ github.com/coder/glog v1.0.1-0.20220322161911-7365fe7f2cd1 h1:UqBrPWSYvRI2s5RtOu
 github.com/coder/glog v1.0.1-0.20220322161911-7365fe7f2cd1/go.mod h1:EWib/APOK0SL3dFbYqvxE3UYd8E6s1ouQ7iEp/0LWV4=
 github.com/coder/retry v1.3.0 h1:5lAAwt/2Cm6lVmnfBY7sOMXcBOwcwJhmV5QGSELIVWY=
 github.com/coder/retry v1.3.0/go.mod h1:tXuRgZgWjUnU5LZPT4lJh4ew2elUhexhlnXzrJWdyFY=
-github.com/coder/tailscale v1.1.1-0.20220905194158-291661887d25 h1:XOloZLgDkAmVBVYXSQBLY+a/Vd2c+dWRBMKNJMWSAWo=
-github.com/coder/tailscale v1.1.1-0.20220905194158-291661887d25/go.mod h1:MO+tWkQp2YIF3KBnnej/mQvgYccRS5Xk/IrEpZ4Z3BU=
+github.com/coder/tailscale v1.1.1-0.20220907030728-c713fe41e3e6 h1:N1n4dt29RNITCOIbzEMyDgJooAhhVvCbU9OT045HKag=
+github.com/coder/tailscale v1.1.1-0.20220907030728-c713fe41e3e6/go.mod h1:MO+tWkQp2YIF3KBnnej/mQvgYccRS5Xk/IrEpZ4Z3BU=
 github.com/coder/wireguard-go/tun/netstack v0.0.0-20220823170024-a78136eb0cab h1:9yEvRWXXfyKzXu8AqywCi+tFZAoqCy4wVcsXwuvZNMc=
 github.com/coder/wireguard-go/tun/netstack v0.0.0-20220823170024-a78136eb0cab/go.mod h1:TCJ66NtXh3urJotTdoYQOHHkyE899vOQl5TuF+WLSes=
 github.com/containerd/aufs v0.0.0-20200908144142-dab0cbea06f4/go.mod h1:nukgQABAEopAHvB6j7cnP5zJ+/3aVcE7hCYqvIwAHyE=