Skip to content

Commit 4c2338c

Browse files
ammarioammario
committed
Merge remote-tracking branch 'origin/main' into quotas-v3
2 parents b030d47 + f017548 commit 4c2338c

32 files changed

+272
-160
lines changed

‎.github/workflows/coder.yaml

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ jobs:
8989
style-lint-golangci:
9090
name: style/lint/golangci
9191
timeout-minutes: 5
92-
runs-on: ${{ github.repository_owner == 'coder' && 'buildjet-8vcpu-ubuntu-2204' || 'ubuntu-latest' }}
92+
runs-on: ${{ github.repository_owner == 'coder' && 'ubuntu-latest-16-cores' || 'ubuntu-latest' }}
9393
steps:
9494
- uses: actions/checkout@v3
9595
- uses: actions/setup-go@v3
@@ -171,7 +171,7 @@ jobs:
171171
gen:
172172
name: "style/gen"
173173
timeout-minutes: 8
174-
runs-on: ${{ github.repository_owner == 'coder' && 'buildjet-8vcpu-ubuntu-2204' || 'ubuntu-latest' }}
174+
runs-on: ${{ github.repository_owner == 'coder' && 'ubuntu-latest-16-cores' || 'ubuntu-latest' }}
175175
needs: changes
176176
if: needs.changes.outputs.docs-only == 'false'
177177
steps:
@@ -276,7 +276,7 @@ jobs:
276276
277277
test-go:
278278
name: "test/go"
279-
runs-on: ${{ matrix.os == 'ubuntu-latest' && github.repository_owner == 'coder' && 'buildjet-8vcpu-ubuntu-2204' || matrix.os }}
279+
runs-on: ${{ matrix.os == 'ubuntu-latest' && github.repository_owner == 'coder' && 'ubuntu-latest-16-cores' || matrix.os == 'windows-2022' && github.repository_owner == 'coder' && 'windows-latest-8-cores'|| matrix.os }}
280280
timeout-minutes: 20
281281
strategy:
282282
matrix:
@@ -336,11 +336,7 @@ jobs:
336336
echo ::set-output name=cover::false
337337
fi
338338
set -x
339-
test_timeout=5m
340-
if [[ "${{ matrix.os }}" == windows* ]]; then
341-
test_timeout=10m
342-
fi
343-
gotestsum --junitfile="gotests.xml" --packages="./..." -- -parallel=8 -timeout=$test_timeout -short -failfast $COVERAGE_FLAGS
339+
gotestsum --junitfile="gotests.xml" --packages="./..." -- -parallel=8 -timeout=3m -short -failfast $COVERAGE_FLAGS
344340
345341
- uses: codecov/codecov-action@v3
346342
# This action has a tendency to error out unexpectedly, it has
@@ -356,7 +352,7 @@ jobs:
356352

357353
test-go-postgres:
358354
name: "test/go/postgres"
359-
runs-on: ${{ github.repository_owner == 'coder' && 'buildjet-8vcpu-ubuntu-2204' || 'ubuntu-latest' }}
355+
runs-on: ${{ github.repository_owner == 'coder' && 'ubuntu-latest-16-cores' || 'ubuntu-latest' }}
360356
# This timeout must be greater than the timeout set by `go test` in
361357
# `make test-postgres` to ensure we receive a trace of running
362358
# goroutines. Setting this to the timeout +5m should work quite well
@@ -417,7 +413,7 @@ jobs:
417413

418414
deploy:
419415
name: "deploy"
420-
runs-on: ${{ github.repository_owner == 'coder' && 'buildjet-8vcpu-ubuntu-2204' || 'ubuntu-latest' }}
416+
runs-on: ${{ github.repository_owner == 'coder' && 'ubuntu-latest-16-cores' || 'ubuntu-latest' }}
421417
timeout-minutes: 30
422418
needs: changes
423419
if: |
@@ -514,7 +510,7 @@ jobs:
514510

515511
test-js:
516512
name: "test/js"
517-
runs-on: ubuntu-latest
513+
runs-on: ${{ github.repository_owner == 'coder' && 'ubuntu-latest-16-cores' || 'ubuntu-latest' }}
518514
timeout-minutes: 20
519515
steps:
520516
- uses: actions/checkout@v3

‎.github/workflows/release.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ env:
2828

2929
jobs:
3030
release:
31-
runs-on: ${{ github.repository_owner == 'coder' && 'buildjet-8vcpu-ubuntu-2204' || 'ubuntu-latest' }}
31+
runs-on: ${{ github.repository_owner == 'coder' && 'ubuntu-latest-16-cores' || 'ubuntu-latest' }}
3232
env:
3333
# Necessary for Docker manifest
3434
DOCKER_CLI_EXPERIMENTAL: "enabled"

‎Makefile

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,8 +463,11 @@ test: test-clean
463463
# When updating -timeout for this test, keep in sync with
464464
# test-go-postgres (.github/workflows/coder.yaml).
465465
test-postgres: test-clean test-postgres-docker
466+
# The postgres test is prone to failure, so we limit parallelism for
467+
# more consistent execution.
466468
DB=ci DB_FROM=$(shell go run scripts/migrate-ci/main.go) gotestsum --junitfile="gotests.xml" --packages="./..." -- \
467469
-covermode=atomic -coverprofile="gotests.coverage" -timeout=20m \
470+
-parallel=4 \
468471
-coverpkg=./... \
469472
-count=1 -race -failfast
470473
.PHONY: test-postgres

‎agent/agent.go

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ const (
5656

5757
type Options struct {
5858
Filesystem afero.Fs
59+
TempDir string
5960
ExchangeToken func(ctx context.Context) (string, error)
6061
Client Client
6162
ReconnectingPTYTimeout time.Duration
@@ -78,6 +79,9 @@ func New(options Options) io.Closer {
7879
if options.Filesystem == nil {
7980
options.Filesystem = afero.NewOsFs()
8081
}
82+
if options.TempDir == "" {
83+
options.TempDir = os.TempDir()
84+
}
8185
if options.ExchangeToken == nil {
8286
options.ExchangeToken = func(ctx context.Context) (string, error) {
8387
return "", nil
@@ -93,6 +97,7 @@ func New(options Options) io.Closer {
9397
client: options.Client,
9498
exchangeToken: options.ExchangeToken,
9599
filesystem: options.Filesystem,
100+
tempDir: options.TempDir,
96101
stats: &Stats{},
97102
}
98103
server.init(ctx)
@@ -104,6 +109,7 @@ type agent struct {
104109
client Client
105110
exchangeToken func(ctx context.Context) (string, error)
106111
filesystem afero.Fs
112+
tempDir string
107113

108114
reconnectingPTYs sync.Map
109115
reconnectingPTYTimeout time.Duration
@@ -375,14 +381,14 @@ func (a *agent) runStartupScript(ctx context.Context, script string) error {
375381
return nil
376382
}
377383

378-
writer, err := os.OpenFile(filepath.Join(os.TempDir(), "coder-startup-script.log"), os.O_CREATE|os.O_RDWR, 0o600)
384+
a.logger.Info(ctx, "running startup script", slog.F("script", script))
385+
writer, err := a.filesystem.OpenFile(filepath.Join(a.tempDir, "coder-startup-script.log"), os.O_CREATE|os.O_RDWR, 0o600)
379386
if err != nil {
380387
return xerrors.Errorf("open startup script log file: %w", err)
381388
}
382389
defer func() {
383390
_ = writer.Close()
384391
}()
385-
386392
cmd, err := a.createCommand(ctx, script, nil)
387393
if err != nil {
388394
return xerrors.Errorf("create command: %w", err)
@@ -470,6 +476,12 @@ func (a *agent) init(ctx context.Context) {
470476
},
471477
SubsystemHandlers: map[string]ssh.SubsystemHandler{
472478
"sftp": func(session ssh.Session) {
479+
ctx := session.Context()
480+
481+
// Typically sftp sessions don't request a TTY, but if they do,
482+
// we must ensure the gliderlabs/ssh CRLF emulation is disabled.
483+
// Otherwise sftp will be broken. This can happen if a user sets
484+
// `RequestTTY force` in their SSH config.
473485
session.DisablePTYEmulation()
474486

475487
var opts []sftp.ServerOption
@@ -478,22 +490,33 @@ func (a *agent) init(ctx context.Context) {
478490
// https://github.com/coder/coder/issues/3620
479491
u, err := user.Current()
480492
if err != nil {
481-
a.logger.Warn(ctx, "get sftp working directory failed, unable to get current user", slog.Error(err))
493+
sshLogger.Warn(ctx, "get sftp working directory failed, unable to get current user", slog.Error(err))
482494
} else {
483495
opts = append(opts, sftp.WithServerWorkingDirectory(u.HomeDir))
484496
}
485497

486498
server, err := sftp.NewServer(session, opts...)
487499
if err != nil {
488-
a.logger.Debug(session.Context(), "initialize sftp server", slog.Error(err))
500+
sshLogger.Debug(ctx, "initialize sftp server", slog.Error(err))
489501
return
490502
}
491503
defer server.Close()
504+
492505
err = server.Serve()
493506
if errors.Is(err, io.EOF) {
507+
// Unless we call `session.Exit(0)` here, the client won't
508+
// receive `exit-status` because `(*sftp.Server).Close()`
509+
// calls `Close()` on the underlying connection (session),
510+
// which actually calls `channel.Close()` because it isn't
511+
// wrapped. This causes sftp clients to receive a non-zero
512+
// exit code. Typically sftp clients don't echo this exit
513+
// code but `scp` on macOS does (when using the default
514+
// SFTP backend).
515+
_ = session.Exit(0)
494516
return
495517
}
496-
a.logger.Debug(session.Context(), "sftp server exited with error", slog.Error(err))
518+
sshLogger.Warn(ctx, "sftp server closed with error", slog.Error(err))
519+
_ = session.Exit(1)
497520
},
498521
},
499522
}

‎agent/agent_test.go

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func TestAgent(t *testing.T) {
6161
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
6262
defer cancel()
6363

64-
conn, stats := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
64+
conn, stats, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
6565

6666
sshClient, err := conn.SSHClient(ctx)
6767
require.NoError(t, err)
@@ -81,7 +81,7 @@ func TestAgent(t *testing.T) {
8181
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
8282
defer cancel()
8383

84-
conn, stats := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
84+
conn, stats, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
8585

8686
ptyConn, err := conn.ReconnectingPTY(ctx, uuid.NewString(), 128, 128, "/bin/bash")
8787
require.NoError(t, err)
@@ -231,7 +231,7 @@ func TestAgent(t *testing.T) {
231231
if runtime.GOOS == "windows" {
232232
home = "/" + strings.ReplaceAll(home, "\\", "/")
233233
}
234-
conn, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
234+
conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
235235
sshClient, err := conn.SSHClient(ctx)
236236
require.NoError(t, err)
237237
defer sshClient.Close()
@@ -261,7 +261,7 @@ func TestAgent(t *testing.T) {
261261
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
262262
defer cancel()
263263

264-
conn, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
264+
conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
265265
sshClient, err := conn.SSHClient(ctx)
266266
require.NoError(t, err)
267267
defer sshClient.Close()
@@ -360,19 +360,23 @@ func TestAgent(t *testing.T) {
360360

361361
t.Run("StartupScript", func(t *testing.T) {
362362
t.Parallel()
363-
tempPath := filepath.Join(t.TempDir(), "content.txt")
364-
content := "somethingnice"
365-
setupAgent(t, codersdk.WorkspaceAgentMetadata{
366-
StartupScript: fmt.Sprintf("echo %s > %s", content, tempPath),
363+
if runtime.GOOS == "windows" {
364+
t.Skip("This test doesn't work on Windows for some reason...")
365+
}
366+
content := "output"
367+
_, _, fs := setupAgent(t, codersdk.WorkspaceAgentMetadata{
368+
StartupScript: "echo " + content,
367369
}, 0)
368-
369370
var gotContent string
370371
require.Eventually(t, func() bool {
371-
content, err := os.ReadFile(tempPath)
372+
outputPath := filepath.Join(os.TempDir(), "coder-startup-script.log")
373+
content, err := afero.ReadFile(fs, outputPath)
372374
if err != nil {
375+
t.Logf("read file %q: %s", outputPath, err)
373376
return false
374377
}
375378
if len(content) == 0 {
379+
t.Logf("no content in %q", outputPath)
376380
return false
377381
}
378382
if runtime.GOOS == "windows" {
@@ -384,7 +388,7 @@ func TestAgent(t *testing.T) {
384388
}
385389
gotContent = string(content)
386390
return true
387-
}, testutil.WaitMedium, testutil.IntervalMedium)
391+
}, testutil.WaitShort, testutil.IntervalMedium)
388392
require.Equal(t, content, strings.TrimSpace(gotContent))
389393
})
390394

@@ -400,7 +404,7 @@ func TestAgent(t *testing.T) {
400404
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
401405
defer cancel()
402406

403-
conn, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
407+
conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
404408
id := uuid.NewString()
405409
netConn, err := conn.ReconnectingPTY(ctx, id, 100, 100, "/bin/bash")
406410
require.NoError(t, err)
@@ -497,13 +501,8 @@ func TestAgent(t *testing.T) {
497501
}
498502
}()
499503

500-
conn, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
501-
require.Eventually(t, func() bool {
502-
ctx, cancelFunc := context.WithTimeout(context.Background(), testutil.IntervalFast)
503-
defer cancelFunc()
504-
_, err := conn.Ping(ctx)
505-
return err == nil
506-
}, testutil.WaitMedium, testutil.IntervalFast)
504+
conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
505+
require.True(t, conn.AwaitReachable(context.Background()))
507506
conn1, err := conn.DialContext(context.Background(), l.Addr().Network(), l.Addr().String())
508507
require.NoError(t, err)
509508
defer conn1.Close()
@@ -523,7 +522,7 @@ func TestAgent(t *testing.T) {
523522
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
524523
defer cancel()
525524
derpMap := tailnettest.RunDERPAndSTUN(t)
526-
conn, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{
525+
conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{
527526
DERPMap: derpMap,
528527
}, 0)
529528
defer conn.Close()
@@ -606,7 +605,7 @@ func TestAgent(t *testing.T) {
606605
}
607606

608607
func setupSSHCommand(t *testing.T, beforeArgs []string, afterArgs []string) *exec.Cmd {
609-
agentConn, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
608+
agentConn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
610609
listener, err := net.Listen("tcp", "127.0.0.1:0")
611610
require.NoError(t, err)
612611
waitGroup := sync.WaitGroup{}
@@ -649,7 +648,7 @@ func setupSSHCommand(t *testing.T, beforeArgs []string, afterArgs []string) *exe
649648
func setupSSHSession(t *testing.T, options codersdk.WorkspaceAgentMetadata) *ssh.Session {
650649
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
651650
defer cancel()
652-
conn, _ := setupAgent(t, options, 0)
651+
conn, _, _ := setupAgent(t, options, 0)
653652
sshClient, err := conn.SSHClient(ctx)
654653
require.NoError(t, err)
655654
t.Cleanup(func() {
@@ -669,13 +668,15 @@ func (c closeFunc) Close() error {
669668
func setupAgent(t *testing.T, metadata codersdk.WorkspaceAgentMetadata, ptyTimeout time.Duration) (
670669
*codersdk.AgentConn,
671670
<-chan *codersdk.AgentStats,
671+
afero.Fs,
672672
) {
673673
if metadata.DERPMap == nil {
674674
metadata.DERPMap = tailnettest.RunDERPAndSTUN(t)
675675
}
676676
coordinator := tailnet.NewCoordinator()
677677
agentID := uuid.New()
678678
statsCh := make(chan *codersdk.AgentStats)
679+
fs := afero.NewMemMapFs()
679680
closer := agent.New(agent.Options{
680681
Client: &client{
681682
t: t,
@@ -684,6 +685,7 @@ func setupAgent(t *testing.T, metadata codersdk.WorkspaceAgentMetadata, ptyTimeo
684685
statsChan: statsCh,
685686
coordinator: coordinator,
686687
},
688+
Filesystem: fs,
687689
Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug),
688690
ReconnectingPTYTimeout: ptyTimeout,
689691
})
@@ -709,7 +711,7 @@ func setupAgent(t *testing.T, metadata codersdk.WorkspaceAgentMetadata, ptyTimeo
709711
conn.SetNodeCallback(sendNode)
710712
return &codersdk.AgentConn{
711713
Conn: conn,
712-
}, statsCh
714+
}, statsCh, fs
713715
}
714716

715717
var dialTestPayload = []byte("dean-was-here123")

‎cli/agent_test.go

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/coder/coder/coderd/coderdtest"
1515
"github.com/coder/coder/provisioner/echo"
1616
"github.com/coder/coder/provisionersdk/proto"
17-
"github.com/coder/coder/testutil"
1817
)
1918

2019
func TestWorkspaceAgent(t *testing.T) {
@@ -71,12 +70,7 @@ func TestWorkspaceAgent(t *testing.T) {
7170
dialer, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, nil)
7271
require.NoError(t, err)
7372
defer dialer.Close()
74-
require.Eventually(t, func() bool {
75-
ctx, cancelFunc := context.WithTimeout(ctx, testutil.IntervalFast)
76-
defer cancelFunc()
77-
_, err := dialer.Ping(ctx)
78-
return err == nil
79-
}, testutil.WaitMedium, testutil.IntervalFast)
73+
require.True(t, dialer.AwaitReachable(context.Background()))
8074
cancelFunc()
8175
err = <-errC
8276
require.NoError(t, err)
@@ -134,12 +128,7 @@ func TestWorkspaceAgent(t *testing.T) {
134128
dialer, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, nil)
135129
require.NoError(t, err)
136130
defer dialer.Close()
137-
require.Eventually(t, func() bool {
138-
ctx, cancelFunc := context.WithTimeout(ctx, testutil.IntervalFast)
139-
defer cancelFunc()
140-
_, err := dialer.Ping(ctx)
141-
return err == nil
142-
}, testutil.WaitMedium, testutil.IntervalFast)
131+
require.True(t, dialer.AwaitReachable(context.Background()))
143132
cancelFunc()
144133
err = <-errC
145134
require.NoError(t, err)
@@ -197,13 +186,7 @@ func TestWorkspaceAgent(t *testing.T) {
197186
dialer, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, nil)
198187
require.NoError(t, err)
199188
defer dialer.Close()
200-
require.Eventually(t, func() bool {
201-
ctx, cancelFunc := context.WithTimeout(ctx, testutil.IntervalFast)
202-
defer cancelFunc()
203-
_, err := dialer.Ping(ctx)
204-
return err == nil
205-
}, testutil.WaitMedium, testutil.IntervalFast)
206-
189+
require.True(t, dialer.AwaitReachable(context.Background()))
207190
sshClient, err := dialer.SSHClient(ctx)
208191
require.NoError(t, err)
209192
defer sshClient.Close()

0 commit comments

Comments
 (0)