Skip to content

Commit e6036c5

Browse files
coadlercoadler
committed
Merge branch 'main' into colin/job-log-idx
2 parents 453731b + 2a46702 commit e6036c5

File tree

224 files changed

+5556
-3127
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

224 files changed

+5556
-3127
lines changed

‎.github/ISSUE_TEMPLATE/external_bug_report.md

Lines changed: 0 additions & 9 deletions
This file was deleted.

‎.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/packages.yaml

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
name: Submit Packages
2+
on:
3+
release:
4+
types: [published]
5+
6+
env:
7+
CODER_VERSION: "${{ github.event.release.tag_name }}"
8+
9+
jobs:
10+
winget:
11+
runs-on: windows-latest
12+
steps:
13+
- name: Install wingetcreate
14+
run: |
15+
Invoke-WebRequest https://aka.ms/wingetcreate/latest -OutFile wingetcreate.exe
16+
17+
# the package version is the same as the release tag without the leading
18+
# "v", and with a trailing ".0" (e.g. "v1.2.3" -> "1.2.3.0")
19+
- name: Calculate package version
20+
id: version
21+
run: |
22+
$version = $env:CODER_VERSION -replace "^v", ""
23+
$version += ".0"
24+
echo "::set-output name=version::$version"
25+
26+
- name: Submit updated manifest to winget-pkgs
27+
run: |
28+
$release_assets = gh release view --repo coder/coder "$env:CODER_VERSION" --json assets | `
29+
ConvertFrom-Json
30+
31+
$installer_url = $release_assets.assets | `
32+
Where-Object name -Match ".*_windows_amd64_installer.exe$" | `
33+
Select -ExpandProperty url
34+
35+
echo "Installer URL: $installer_url"
36+
37+
# version should be extracted from the installer
38+
wingetcreate update Coder.Coder `
39+
--submit `
40+
--version "${{ steps.version.outputs.version }}" `
41+
--urls "$installer_url" `
42+
--token "${{ secrets.CDRCI_GITHUB_TOKEN }}"
43+
44+
- name: Comment on PR
45+
run: |
46+
# find the PR that wingetcreate just made
47+
$pr_list = gh pr list --repo microsoft/winget-pkgs --search "author:cdrci Coder.Coder version ${{ steps.version.outputs.version }}" --limit 1 --json number | `
48+
ConvertFrom-Json`
49+
$pr_number = $pr_list[0].number
50+
51+
gh pr comment --repo microsoft/winget-pkgs "$pr_number" --body "🤖 cc: @deansheather"

‎.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: 43 additions & 19 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
@@ -168,7 +174,7 @@ func (a *agent) run(ctx context.Context) error {
168174
if err != nil {
169175
return xerrors.Errorf("fetch metadata: %w", err)
170176
}
171-
a.logger.Info(context.Background(), "fetched metadata")
177+
a.logger.Info(ctx, "fetched metadata")
172178
oldMetadata := a.metadata.Swap(metadata)
173179

174180
// The startup script should only execute on the first run!
@@ -202,7 +208,7 @@ func (a *agent) run(ctx context.Context) error {
202208
a.closeMutex.Lock()
203209
network := a.network
204210
a.closeMutex.Unlock()
205-
if a.network == nil {
211+
if network == nil {
206212
a.logger.Debug(ctx, "creating tailnet")
207213
network, err = a.createTailnet(ctx, metadata.DERPMap)
208214
if err != nil {
@@ -359,7 +365,7 @@ func (a *agent) runCoordinator(ctx context.Context, network *tailnet.Conn) error
359365
return err
360366
}
361367
defer coordinator.Close()
362-
a.logger.Info(context.Background(), "connected to coordination server")
368+
a.logger.Info(ctx, "connected to coordination server")
363369
sendNodes, errChan := tailnet.ServeCoordinator(coordinator, network.UpdateNodes)
364370
network.SetNodeCallback(sendNodes)
365371
select {
@@ -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
}
@@ -538,25 +561,26 @@ func (a *agent) createCommand(ctx context.Context, rawCommand string, env []stri
538561
return nil, xerrors.Errorf("metadata is the wrong type: %T", metadata)
539562
}
540563

564+
// OpenSSH executes all commands with the users current shell.
565+
// We replicate that behavior for IDE support.
566+
caller := "-c"
567+
if runtime.GOOS == "windows" {
568+
caller = "/c"
569+
}
570+
args := []string{caller, rawCommand}
571+
541572
// gliderlabs/ssh returns a command slice of zero
542573
// when a shell is requested.
543-
command := rawCommand
544-
if len(command) == 0 {
545-
command = shell
574+
if len(rawCommand) == 0 {
575+
args = []string{}
546576
if runtime.GOOS != "windows" {
547577
// On Linux and macOS, we should start a login
548578
// shell to consume juicy environment variables!
549-
command += " -l"
579+
args = append(args, "-l")
550580
}
551581
}
552582

553-
// OpenSSH executes all commands with the users current shell.
554-
// We replicate that behavior for IDE support.
555-
caller := "-c"
556-
if runtime.GOOS == "windows" {
557-
caller = "/c"
558-
}
559-
cmd := exec.CommandContext(ctx, shell, caller, command)
583+
cmd := exec.CommandContext(ctx, shell, args...)
560584
cmd.Dir = metadata.Directory
561585
if cmd.Dir == "" {
562586
// Default to $HOME if a directory is not set!

0 commit comments

Comments
 (0)