From de266464a42e6cbc024ddfb4aaf33c39e59932a4 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Mon, 10 May 2021 21:34:13 +0000 Subject: [PATCH 1/5] chore: Test RTC listener reconnects --- internal/cmd/tunnel.go | 2 +- wsnet/listen.go | 9 +++++-- wsnet/listen_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 wsnet/listen_test.go diff --git a/internal/cmd/tunnel.go b/internal/cmd/tunnel.go index 6194891a..858544b2 100644 --- a/internal/cmd/tunnel.go +++ b/internal/cmd/tunnel.go @@ -72,7 +72,7 @@ coder tunnel my-dev 3000 3000 } c := &tunnneler{ - log: log.Leveled(slog.LevelDebug), + log: log, brokerAddr: &baseURL, token: sdk.Token(), workspaceID: envID, diff --git a/wsnet/listen.go b/wsnet/listen.go index 55d6d019..9a9a30db 100644 --- a/wsnet/listen.go +++ b/wsnet/listen.go @@ -18,6 +18,8 @@ import ( "cdr.dev/coder-cli/coder-sdk" ) +var keepAliveInterval = 5 * time.Second + // Listen connects to the broker proxies connections to the local net. // Close will end all RTC connections. func Listen(ctx context.Context, broker string) (io.Closer, error) { @@ -35,7 +37,7 @@ func Listen(ctx context.Context, broker string) (io.Closer, error) { go func() { for { err := <-ch - if errors.Is(err, io.EOF) { + if errors.Is(err, io.EOF) || errors.Is(err, yamux.ErrKeepAliveTimeout) { // If we hit an EOF, then the connection to the broker // was interrupted. We'll take a short break then dial // again. @@ -76,7 +78,10 @@ func (l *listener) dial(ctx context.Context) (<-chan error, error) { } l.ws = conn nconn := websocket.NetConn(ctx, conn, websocket.MessageBinary) - session, err := yamux.Server(nconn, nil) + config := yamux.DefaultConfig() + config.KeepAliveInterval = keepAliveInterval + config.LogOutput = io.Discard + session, err := yamux.Server(nconn, config) if err != nil { return nil, fmt.Errorf("create multiplex: %w", err) } diff --git a/wsnet/listen_test.go b/wsnet/listen_test.go new file mode 100644 index 00000000..dd1033db --- /dev/null +++ b/wsnet/listen_test.go @@ -0,0 +1,59 @@ +package wsnet + +import ( + "context" + "fmt" + "net" + "net/http" + "testing" + "time" + + "nhooyr.io/websocket" +) + +func TestListen(t *testing.T) { + t.Run("Reconnect", func(t *testing.T) { + keepAliveInterval = 50 * time.Millisecond + + var ( + connCh = make(chan interface{}) + mux = http.NewServeMux() + srv = http.Server{ + Handler: mux, + } + ) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + _, err := websocket.Accept(w, r, nil) + if err != nil { + t.Error(err) + return + } + connCh <- struct{}{} + }) + + listener, err := net.Listen("tcp4", "127.0.0.1:0") + if err != nil { + t.Error(err) + return + } + go srv.Serve(listener) + + addr := listener.Addr() + broker := fmt.Sprintf("http://%s/", addr.String()) + + _, err = Listen(context.Background(), broker) + if err != nil { + t.Error(err) + return + } + <-connCh + _ = listener.Close() + listener, err = net.Listen("tcp4", addr.String()) + if err != nil { + t.Error(err) + return + } + go srv.Serve(listener) + <-connCh + }) +} From 49df5fdcd583d1d5305d9d0300f355749b1dd4e8 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Mon, 10 May 2021 21:35:07 +0000 Subject: [PATCH 2/5] Remove version check for tunnel --- internal/cmd/agent.go | 2 +- internal/cmd/auth.go | 12 ++++++++---- internal/cmd/configssh.go | 2 +- internal/cmd/envs.go | 12 ++++++------ internal/cmd/images.go | 2 +- internal/cmd/providers.go | 12 ++++++------ internal/cmd/rebuild.go | 4 ++-- internal/cmd/resourcemanager.go | 2 +- internal/cmd/ssh.go | 4 ++-- internal/cmd/sync.go | 2 +- internal/cmd/tags.go | 6 +++--- internal/cmd/tokens.go | 8 ++++---- internal/cmd/tunnel.go | 2 +- internal/cmd/urls.go | 6 +++--- internal/cmd/users.go | 2 +- 15 files changed, 41 insertions(+), 37 deletions(-) diff --git a/internal/cmd/agent.go b/internal/cmd/agent.go index eef86f05..38853dd1 100644 --- a/internal/cmd/agent.go +++ b/internal/cmd/agent.go @@ -51,7 +51,7 @@ coder agent start --coder-url https://my-coder.com --token xxxx-xxxx var ok bool coderURL, ok = os.LookupEnv("CODER_URL") if !ok { - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return xerrors.New("must login, pass --coder-url flag, or set the CODER_URL env variable") } diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index a6e73d16..56aaef80 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -23,7 +23,7 @@ var errNeedLogin = clog.Fatal( const tokenEnv = "CODER_TOKEN" const urlEnv = "CODER_URL" -func newClient(ctx context.Context) (coder.Client, error) { +func newClient(ctx context.Context, checkVersion bool) (coder.Client, error) { var ( err error sessionToken = os.Getenv(tokenEnv) @@ -55,10 +55,14 @@ func newClient(ctx context.Context) (coder.Client, error) { return nil, xerrors.Errorf("failed to create new coder.Client: %w", err) } - apiVersion, err := c.APIVersion(ctx) - if apiVersion != "" && !version.VersionsMatch(apiVersion) { - logVersionMismatchError(apiVersion) + if checkVersion { + var apiVersion string + apiVersion, err = c.APIVersion(ctx) + if apiVersion != "" && !version.VersionsMatch(apiVersion) { + logVersionMismatchError(apiVersion) + } } + if err != nil { var he *coder.HTTPError if xerrors.As(err, &he) { diff --git a/internal/cmd/configssh.go b/internal/cmd/configssh.go index 0853a32e..7abe2c2b 100644 --- a/internal/cmd/configssh.go +++ b/internal/cmd/configssh.go @@ -88,7 +88,7 @@ func configSSH(configpath *string, remove *bool, p2p *bool) func(cmd *cobra.Comm return nil } - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } diff --git a/internal/cmd/envs.go b/internal/cmd/envs.go index c33db37d..352b2567 100644 --- a/internal/cmd/envs.go +++ b/internal/cmd/envs.go @@ -59,7 +59,7 @@ func lsEnvsCommand() *cobra.Command { Long: "List all Coder environments owned by the active user.", RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -128,7 +128,7 @@ coder envs --user charlie@coder.com ls -o json \ Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return xerrors.Errorf("new client: %w", err) } @@ -189,7 +189,7 @@ coder envs create my-new-powerful-env --cpu 12 --disk 100 --memory 16 --image ub return xerrors.New("image unset") } - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -315,7 +315,7 @@ coder envs create-from-config --name="dev-env" -f coder.yaml`, ) } - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -438,7 +438,7 @@ func editEnvCmd() *cobra.Command { coder envs edit back-end-env --disk 20`, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -532,7 +532,7 @@ func rmEnvsCmd() *cobra.Command { Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } diff --git a/internal/cmd/images.go b/internal/cmd/images.go index 70364a59..ccb68ee5 100644 --- a/internal/cmd/images.go +++ b/internal/cmd/images.go @@ -38,7 +38,7 @@ func lsImgsCommand(user *string) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } diff --git a/internal/cmd/providers.go b/internal/cmd/providers.go index 46074933..5081df2b 100644 --- a/internal/cmd/providers.go +++ b/internal/cmd/providers.go @@ -50,7 +50,7 @@ coder providers create my-provider --hostname=https://provider.example.com --clu RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -143,7 +143,7 @@ coder providers ls`, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -174,7 +174,7 @@ func deleteProviderCmd() *cobra.Command { coder providers rm my-workspace-provider`, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -232,7 +232,7 @@ func cordonProviderCmd() *cobra.Command { coder providers cordon my-workspace-provider --reason "limit cloud clost"`, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -265,7 +265,7 @@ func unCordonProviderCmd() *cobra.Command { coder providers uncordon my-workspace-provider`, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -296,7 +296,7 @@ func renameProviderCmd() *cobra.Command { coder providers rename build-in us-east-1`, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } diff --git a/internal/cmd/rebuild.go b/internal/cmd/rebuild.go index fe6c3dff..4df7d8e9 100644 --- a/internal/cmd/rebuild.go +++ b/internal/cmd/rebuild.go @@ -29,7 +29,7 @@ func rebuildEnvCommand() *cobra.Command { coder envs rebuild backend-env --force`, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -166,7 +166,7 @@ func watchBuildLogCommand() *cobra.Command { Args: xcobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } diff --git a/internal/cmd/resourcemanager.go b/internal/cmd/resourcemanager.go index 5611594f..85d39c98 100644 --- a/internal/cmd/resourcemanager.go +++ b/internal/cmd/resourcemanager.go @@ -62,7 +62,7 @@ coder resources top --sort-by memory --show-empty`, func runResourceTop(options *resourceTopOptions) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } diff --git a/internal/cmd/ssh.go b/internal/cmd/ssh.go index f062fa11..34478e0a 100644 --- a/internal/cmd/ssh.go +++ b/internal/cmd/ssh.go @@ -37,7 +37,7 @@ coder ssh my-dev pwd`, func shell(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -99,7 +99,7 @@ func shValidArgs(cmd *cobra.Command, args []string) error { ctx := cmd.Context() err := cobra.MinimumNArgs(1)(cmd, args) if err != nil { - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return clog.Error("missing [environment_name] argument") } diff --git a/internal/cmd/sync.go b/internal/cmd/sync.go index fc059ac0..922f2679 100644 --- a/internal/cmd/sync.go +++ b/internal/cmd/sync.go @@ -54,7 +54,7 @@ func makeRunSync(init *bool) func(cmd *cobra.Command, args []string) error { remote = args[1] ) - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } diff --git a/internal/cmd/tags.go b/internal/cmd/tags.go index 91d7ba19..201133b0 100644 --- a/internal/cmd/tags.go +++ b/internal/cmd/tags.go @@ -41,7 +41,7 @@ func tagsCreateCmd() *cobra.Command { Args: xcobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -86,7 +86,7 @@ func tagsLsCmd() *cobra.Command { Example: `coder tags ls --image ubuntu --org default --output json`, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -143,7 +143,7 @@ func tagsRmCmd() *cobra.Command { Args: xcobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } diff --git a/internal/cmd/tokens.go b/internal/cmd/tokens.go index 21fd478f..0763a5b0 100644 --- a/internal/cmd/tokens.go +++ b/internal/cmd/tokens.go @@ -36,7 +36,7 @@ func lsTokensCmd() *cobra.Command { Short: "show the user's active API tokens", RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -79,7 +79,7 @@ func createTokensCmd() *cobra.Command { Args: xcobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -102,7 +102,7 @@ func rmTokenCmd() *cobra.Command { Args: xcobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -121,7 +121,7 @@ func regenTokenCmd() *cobra.Command { Args: xcobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } diff --git a/internal/cmd/tunnel.go b/internal/cmd/tunnel.go index 858544b2..09463589 100644 --- a/internal/cmd/tunnel.go +++ b/internal/cmd/tunnel.go @@ -49,7 +49,7 @@ coder tunnel my-dev 3000 3000 } } - sdk, err := newClient(ctx) + sdk, err := newClient(ctx, false) if err != nil { return xerrors.Errorf("getting coder client: %w", err) } diff --git a/internal/cmd/urls.go b/internal/cmd/urls.go index 94f90357..909662a7 100644 --- a/internal/cmd/urls.go +++ b/internal/cmd/urls.go @@ -81,7 +81,7 @@ func accessLevelIsValid(level string) bool { func listDevURLsCmd(outputFmt *string) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -147,7 +147,7 @@ func createDevURLCmd() *cobra.Command { if urlname != "" && !devURLNameValidRx.MatchString(urlname) { return xerrors.New("update devurl: name must be < 64 chars in length, begin with a letter and only contain letters or digits.") } - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } @@ -230,7 +230,7 @@ func removeDevURL(cmd *cobra.Command, args []string) error { return xerrors.Errorf("validate port: %w", err) } - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } diff --git a/internal/cmd/users.go b/internal/cmd/users.go index 03929366..c9a00343 100644 --- a/internal/cmd/users.go +++ b/internal/cmd/users.go @@ -32,7 +32,7 @@ coder users ls -o json | jq .[] | jq -r .email`, func listUsers(outputFmt *string) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - client, err := newClient(ctx) + client, err := newClient(ctx, true) if err != nil { return err } From 6254e8fbb8c336804b2872c635670027f8c8962f Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 11 May 2021 03:02:12 +0000 Subject: [PATCH 3/5] Fix linting issues --- wsnet/listen_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/wsnet/listen_test.go b/wsnet/listen_test.go index dd1033db..d228bd09 100644 --- a/wsnet/listen_test.go +++ b/wsnet/listen_test.go @@ -36,8 +36,9 @@ func TestListen(t *testing.T) { t.Error(err) return } - go srv.Serve(listener) - + go func() { + _ = srv.Serve(listener) + }() addr := listener.Addr() broker := fmt.Sprintf("http://%s/", addr.String()) @@ -53,7 +54,9 @@ func TestListen(t *testing.T) { t.Error(err) return } - go srv.Serve(listener) + go func() { + _ = srv.Serve(listener) + }() <-connCh }) } From d2b0a6da21c25f66a4a65c3c2220df7ddedd5047 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 11 May 2021 03:04:14 +0000 Subject: [PATCH 4/5] Update Go version --- ci/image/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/image/Dockerfile b/ci/image/Dockerfile index f814866d..08dde84b 100644 --- a/ci/image/Dockerfile +++ b/ci/image/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1 +FROM golang:1.16.3 ENV GOFLAGS="-mod=readonly" ENV CI=true From 57f4f1dbffe5f7b64e865887c63b862113237caf Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 11 May 2021 03:08:18 +0000 Subject: [PATCH 5/5] Pin to 1.16.3 --- .github/workflows/build.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index cb2e37a7..a0d76acf 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -7,6 +7,9 @@ jobs: steps: - name: Checkout uses: actions/checkout@v1 + - uses: actions/setup-go@v2 + with: + go-version: '^1.16.3' - name: Build run: make -j build/linux build/windows - name: Upload @@ -19,6 +22,9 @@ jobs: steps: - name: Checkout uses: actions/checkout@v1 + - uses: actions/setup-go@v2 + with: + go-version: '^1.16.3' - name: Install Gon run: | brew tap mitchellh/gon