From 33c0b77a372a2f2253f0159658f733f05cc53fe5 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 6 Sep 2022 21:38:22 -0500 Subject: [PATCH] fix: Improve speedtest by adding direct connection toggle It's weird to test connection speeds over DERP, because most connections will eventually migrate to direct. --- .github/workflows/release.yaml | 2 +- agent/agent.go | 4 +++ cli/speedtest.go | 55 ++++++++++++++++++++++++---------- cli/speedtest_test.go | 3 +- go.mod | 2 +- go.sum | 4 +-- 6 files changed, 50 insertions(+), 20 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index fd89c988c3954..a7808865a3fbd 100644 --- a/.github/workflows/release.yaml +++ b/.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 diff --git a/agent/agent.go b/agent/agent.go index 1274f4e5a76fb..c477122e7b39a 100644 --- a/agent/agent.go +++ b/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) }() } diff --git a/cli/speedtest.go b/cli/speedtest.go index c9e69a2ddf3fb..ab5a402977cb5 100644 --- a/cli/speedtest.go +++ b/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 } diff --git a/cli/speedtest_test.go b/cli/speedtest_test.go index 0eccd69171dee..3431c5508ee40 100644 --- a/cli/speedtest_test.go +++ b/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) { diff --git a/go.mod b/go.mod index 2604f1436694e..3bb1d54380041 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index bcd1e0c1d1242..7c1c33c284767 100644 --- a/go.sum +++ b/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=