Skip to content

Commit 8035265

Browse files
kylecarbskylecarbs
authored
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.
1 parent 0f0e3d1 commit 8035265

File tree

6 files changed

+50
-20
lines changed

6 files changed

+50
-20
lines changed

‎.github/workflows/release.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ jobs:
4747
uses: docker/login-action@v2
4848
with:
4949
registry: ghcr.io
50-
username: ${{ github.repository_owner }}
50+
username: ${{ github.actor }}
5151
password: ${{ secrets.GITHUB_TOKEN }}
5252

5353
- uses: actions/setup-go@v3

‎agent/agent.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,11 @@ func (a *agent) runTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) {
270270
a.logger.Debug(ctx, "speedtest listener failed", slog.Error(err))
271271
return
272272
}
273+
a.closeMutex.Lock()
274+
a.connCloseWait.Add(1)
275+
a.closeMutex.Unlock()
273276
go func() {
277+
defer a.connCloseWait.Done()
274278
_ = speedtest.ServeConn(conn)
275279
}()
276280
}

‎cli/speedtest.go

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,23 @@ import (
55
"fmt"
66
"time"
77

8-
"cdr.dev/slog"
9-
"github.com/coder/coder/cli/cliflag"
10-
"github.com/coder/coder/cli/cliui"
11-
"github.com/coder/coder/codersdk"
128
"github.com/jedib0t/go-pretty/v6/table"
139
"github.com/spf13/cobra"
1410
"golang.org/x/xerrors"
1511
tsspeedtest "tailscale.com/net/speedtest"
12+
13+
"cdr.dev/slog"
14+
"github.com/coder/coder/agent"
15+
"github.com/coder/coder/cli/cliflag"
16+
"github.com/coder/coder/cli/cliui"
17+
"github.com/coder/coder/codersdk"
1618
)
1719

1820
func speedtest() *cobra.Command {
1921
var (
20-
reverse bool
21-
timeStr string
22+
direct bool
23+
duration time.Duration
24+
reverse bool
2225
)
2326
cmd := &cobra.Command{
2427
Annotations: workspaceCommand,
@@ -28,11 +31,6 @@ func speedtest() *cobra.Command {
2831
ctx, cancel := context.WithCancel(cmd.Context())
2932
defer cancel()
3033

31-
dur, err := time.ParseDuration(timeStr)
32-
if err != nil {
33-
return err
34-
}
35-
3634
client, err := CreateClient(cmd)
3735
if err != nil {
3836
return xerrors.Errorf("create codersdk client: %w", err)
@@ -57,13 +55,38 @@ func speedtest() *cobra.Command {
5755
return err
5856
}
5957
defer conn.Close()
60-
_, _ = conn.Ping()
58+
if direct {
59+
ticker := time.NewTicker(time.Second)
60+
defer ticker.Stop()
61+
for {
62+
select {
63+
case <-ctx.Done():
64+
return ctx.Err()
65+
case <-ticker.C:
66+
}
67+
dur, err := conn.Ping()
68+
if err != nil {
69+
continue
70+
}
71+
tc, _ := conn.(*agent.TailnetConn)
72+
status := tc.Status()
73+
if len(status.Peers()) != 1 {
74+
continue
75+
}
76+
peer := status.Peer[status.Peers()[0]]
77+
if peer.CurAddr == "" {
78+
cmd.Printf("Waiting for a direct connection... (%dms via %s)\n", dur.Milliseconds(), peer.Relay)
79+
continue
80+
}
81+
break
82+
}
83+
}
6184
dir := tsspeedtest.Download
6285
if reverse {
6386
dir = tsspeedtest.Upload
6487
}
65-
cmd.Printf("Starting a %ds %s test...\n", int(dur.Seconds()), dir)
66-
results, err := conn.Speedtest(dir, dur)
88+
cmd.Printf("Starting a %ds %s test...\n", int(duration.Seconds()), dir)
89+
results, err := conn.Speedtest(dir, duration)
6790
if err != nil {
6891
return err
6992
}
@@ -83,9 +106,11 @@ func speedtest() *cobra.Command {
83106
return err
84107
},
85108
}
109+
cliflag.BoolVarP(cmd.Flags(), &direct, "direct", "d", "", false,
110+
"Specifies whether to wait for a direct connection before testing speed.")
86111
cliflag.BoolVarP(cmd.Flags(), &reverse, "reverse", "r", "", false,
87112
"Specifies whether to run in reverse mode where the client receives and the server sends.")
88-
cliflag.StringVarP(cmd.Flags(), &timeStr, "time", "t", "", "5s",
113+
cmd.Flags().DurationVarP(&duration, "time", "t", tsspeedtest.DefaultDuration,
89114
"Specifies the duration to monitor traffic.")
90115
return cmd
91116
}

‎cli/speedtest_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@ import (
44
"context"
55
"testing"
66

7+
"github.com/stretchr/testify/assert"
8+
79
"cdr.dev/slog/sloggers/slogtest"
810
"github.com/coder/coder/agent"
911
"github.com/coder/coder/cli/clitest"
1012
"github.com/coder/coder/coderd/coderdtest"
1113
"github.com/coder/coder/codersdk"
1214
"github.com/coder/coder/pty/ptytest"
1315
"github.com/coder/coder/testutil"
14-
"github.com/stretchr/testify/assert"
1516
)
1617

1718
func TestSpeedtest(t *testing.T) {

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ replace github.com/tcnksm/go-httpstat => github.com/kylecarbs/go-httpstat v0.0.0
4949

5050
// There are a few minor changes we make to Tailscale that we're slowly upstreaming. Compare here:
5151
// https://github.com/tailscale/tailscale/compare/main...coder:tailscale:main
52-
replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220905194158-291661887d25
52+
replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220907030728-c713fe41e3e6
5353

5454
require (
5555
cdr.dev/slog v1.4.2-0.20220525200111-18dce5c2cd5f

‎go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,8 @@ github.com/coder/glog v1.0.1-0.20220322161911-7365fe7f2cd1 h1:UqBrPWSYvRI2s5RtOu
352352
github.com/coder/glog v1.0.1-0.20220322161911-7365fe7f2cd1/go.mod h1:EWib/APOK0SL3dFbYqvxE3UYd8E6s1ouQ7iEp/0LWV4=
353353
github.com/coder/retry v1.3.0 h1:5lAAwt/2Cm6lVmnfBY7sOMXcBOwcwJhmV5QGSELIVWY=
354354
github.com/coder/retry v1.3.0/go.mod h1:tXuRgZgWjUnU5LZPT4lJh4ew2elUhexhlnXzrJWdyFY=
355-
github.com/coder/tailscale v1.1.1-0.20220905194158-291661887d25 h1:XOloZLgDkAmVBVYXSQBLY+a/Vd2c+dWRBMKNJMWSAWo=
356-
github.com/coder/tailscale v1.1.1-0.20220905194158-291661887d25/go.mod h1:MO+tWkQp2YIF3KBnnej/mQvgYccRS5Xk/IrEpZ4Z3BU=
355+
github.com/coder/tailscale v1.1.1-0.20220907030728-c713fe41e3e6 h1:N1n4dt29RNITCOIbzEMyDgJooAhhVvCbU9OT045HKag=
356+
github.com/coder/tailscale v1.1.1-0.20220907030728-c713fe41e3e6/go.mod h1:MO+tWkQp2YIF3KBnnej/mQvgYccRS5Xk/IrEpZ4Z3BU=
357357
github.com/coder/wireguard-go/tun/netstack v0.0.0-20220823170024-a78136eb0cab h1:9yEvRWXXfyKzXu8AqywCi+tFZAoqCy4wVcsXwuvZNMc=
358358
github.com/coder/wireguard-go/tun/netstack v0.0.0-20220823170024-a78136eb0cab/go.mod h1:TCJ66NtXh3urJotTdoYQOHHkyE899vOQl5TuF+WLSes=
359359
github.com/containerd/aufs v0.0.0-20200908144142-dab0cbea06f4/go.mod h1:nukgQABAEopAHvB6j7cnP5zJ+/3aVcE7hCYqvIwAHyE=

0 commit comments

Comments
 (0)