Skip to content

Commit 7892c5a

Browse files
deansheatherdeansheather
committed
Merge branch 'main' into dean/schedule-max-ttl
2 parents 4c6a501 + f05609b commit 7892c5a

File tree

367 files changed

+14683
-3720
lines changed

Some content is hidden

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

367 files changed

+14683
-3720
lines changed

‎.github/workflows/ci.yaml

-3
Original file line numberDiff line numberDiff line change
@@ -583,9 +583,6 @@ jobs:
583583
- run: yarn playwright:install
584584
working-directory: site
585585

586-
- run: yarn playwright:install-deps
587-
working-directory: site
588-
589586
- run: yarn playwright:test
590587
env:
591588
DEBUG: pw:api

‎Makefile

+7-1
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,15 @@ install: build/coder_$(VERSION)_$(GOOS)_$(GOARCH)$(GOOS_BIN_EXT)
368368
cp "$<" "$$output_file"
369369
.PHONY: install
370370

371-
fmt: fmt/prettier fmt/terraform fmt/shfmt
371+
fmt: fmt/prettier fmt/terraform fmt/shfmt fmt/go
372372
.PHONY: fmt
373373

374+
fmt/go:
375+
# VS Code users should check out
376+
# https://github.com/mvdan/gofumpt#visual-studio-code
377+
go run mvdan.cc/gofumpt@v0.4.0 -w -l .
378+
.PHONY: fmt/go
379+
374380
fmt/prettier:
375381
echo "--- prettier"
376382
cd site

‎agent/agent.go

+20-10
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ type Options struct {
7272
type Client interface {
7373
Metadata(ctx context.Context) (agentsdk.Metadata, error)
7474
Listen(ctx context.Context) (net.Conn, error)
75-
ReportStats(ctx context.Context, log slog.Logger, stats func() *agentsdk.Stats) (io.Closer, error)
75+
ReportStats(ctx context.Context, log slog.Logger, statsChan <-chan *agentsdk.Stats, setInterval func(time.Duration)) (io.Closer, error)
7676
PostLifecycle(ctx context.Context, state agentsdk.PostLifecycleRequest) error
7777
PostAppHealth(ctx context.Context, req agentsdk.PostAppHealthsRequest) error
7878
PostStartup(ctx context.Context, req agentsdk.PostStartupRequest) error
@@ -112,6 +112,7 @@ func New(options Options) io.Closer {
112112
logDir: options.LogDir,
113113
tempDir: options.TempDir,
114114
lifecycleUpdate: make(chan struct{}, 1),
115+
connStatsChan: make(chan *agentsdk.Stats, 1),
115116
}
116117
a.init(ctx)
117118
return a
@@ -143,7 +144,8 @@ type agent struct {
143144
lifecycleMu sync.Mutex // Protects following.
144145
lifecycleState codersdk.WorkspaceAgentLifecycle
145146

146-
network *tailnet.Conn
147+
network *tailnet.Conn
148+
connStatsChan chan *agentsdk.Stats
147149
}
148150

149151
// runLoop attempts to start the agent in a retry loop.
@@ -351,11 +353,20 @@ func (a *agent) run(ctx context.Context) error {
351353
return xerrors.New("agent is closed")
352354
}
353355

356+
setStatInterval := func(d time.Duration) {
357+
network.SetConnStatsCallback(d, 2048,
358+
func(_, _ time.Time, virtual, _ map[netlogtype.Connection]netlogtype.Counts) {
359+
select {
360+
case a.connStatsChan <- convertAgentStats(virtual):
361+
default:
362+
a.logger.Warn(ctx, "network stat dropped")
363+
}
364+
},
365+
)
366+
}
367+
354368
// Report statistics from the created network.
355-
cl, err := a.client.ReportStats(ctx, a.logger, func() *agentsdk.Stats {
356-
stats := network.ExtractTrafficStats()
357-
return convertAgentStats(stats)
358-
})
369+
cl, err := a.client.ReportStats(ctx, a.logger, a.connStatsChan, setStatInterval)
359370
if err != nil {
360371
a.logger.Error(ctx, "report stats", slog.Error(err))
361372
} else {
@@ -399,10 +410,9 @@ func (a *agent) trackConnGoroutine(fn func()) error {
399410

400411
func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (_ *tailnet.Conn, err error) {
401412
network, err := tailnet.NewConn(&tailnet.Options{
402-
Addresses: []netip.Prefix{netip.PrefixFrom(codersdk.WorkspaceAgentIP, 128)},
403-
DERPMap: derpMap,
404-
Logger: a.logger.Named("tailnet"),
405-
EnableTrafficStats: true,
413+
Addresses: []netip.Prefix{netip.PrefixFrom(codersdk.WorkspaceAgentIP, 128)},
414+
DERPMap: derpMap,
415+
Logger: a.logger.Named("tailnet"),
406416
})
407417
if err != nil {
408418
return nil, xerrors.Errorf("create tailnet: %w", err)

‎agent/agent_test.go

+20-21
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ import (
2222
"testing"
2323
"time"
2424

25-
"golang.org/x/xerrors"
26-
"tailscale.com/net/speedtest"
27-
"tailscale.com/tailcfg"
28-
2925
scp "github.com/bramvdbogaerde/go-scp"
3026
"github.com/google/uuid"
3127
"github.com/pion/udp"
@@ -37,6 +33,9 @@ import (
3733
"golang.org/x/crypto/ssh"
3834
"golang.org/x/text/encoding/unicode"
3935
"golang.org/x/text/transform"
36+
"golang.org/x/xerrors"
37+
"tailscale.com/net/speedtest"
38+
"tailscale.com/tailcfg"
4039

4140
"cdr.dev/slog"
4241
"cdr.dev/slog/sloggers/slogtest"
@@ -53,6 +52,8 @@ func TestMain(m *testing.M) {
5352
goleak.VerifyTestMain(m)
5453
}
5554

55+
// NOTE: These tests only work when your default shell is bash for some reason.
56+
5657
func TestAgent_Stats_SSH(t *testing.T) {
5758
t.Parallel()
5859
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
@@ -1153,17 +1154,16 @@ func setupAgent(t *testing.T, metadata agentsdk.Metadata, ptyTimeout time.Durati
11531154
closer := agent.New(agent.Options{
11541155
Client: c,
11551156
Filesystem: fs,
1156-
Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug),
1157+
Logger: slogtest.Make(t, nil).Named("agent").Leveled(slog.LevelDebug),
11571158
ReconnectingPTYTimeout: ptyTimeout,
11581159
})
11591160
t.Cleanup(func() {
11601161
_ = closer.Close()
11611162
})
11621163
conn, err := tailnet.NewConn(&tailnet.Options{
1163-
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
1164-
DERPMap: metadata.DERPMap,
1165-
Logger: slogtest.Make(t, nil).Named("client").Leveled(slog.LevelDebug),
1166-
EnableTrafficStats: true,
1164+
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
1165+
DERPMap: metadata.DERPMap,
1166+
Logger: slogtest.Make(t, nil).Named("client").Leveled(slog.LevelDebug),
11671167
})
11681168
require.NoError(t, err)
11691169
clientConn, serverConn := net.Pipe()
@@ -1251,28 +1251,27 @@ func (c *client) Listen(_ context.Context) (net.Conn, error) {
12511251
return clientConn, nil
12521252
}
12531253

1254-
func (c *client) ReportStats(ctx context.Context, _ slog.Logger, stats func() *agentsdk.Stats) (io.Closer, error) {
1254+
func (c *client) ReportStats(ctx context.Context, _ slog.Logger, statsChan <-chan *agentsdk.Stats, setInterval func(time.Duration)) (io.Closer, error) {
12551255
doneCh := make(chan struct{})
12561256
ctx, cancel := context.WithCancel(ctx)
12571257

12581258
go func() {
12591259
defer close(doneCh)
12601260

1261-
t := time.NewTicker(500 * time.Millisecond)
1262-
defer t.Stop()
1261+
setInterval(500 * time.Millisecond)
12631262
for {
12641263
select {
12651264
case <-ctx.Done():
12661265
return
1267-
case <-t.C:
1268-
}
1269-
select {
1270-
case c.statsChan <- stats():
1271-
case <-ctx.Done():
1272-
return
1273-
default:
1274-
// We don't want to send old stats.
1275-
continue
1266+
case stat := <-statsChan:
1267+
select {
1268+
case c.statsChan <- stat:
1269+
case <-ctx.Done():
1270+
return
1271+
default:
1272+
// We don't want to send old stats.
1273+
continue
1274+
}
12761275
}
12771276
}
12781277
}()

‎agent/apphealth_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func TestAppHealth_NotSpamming(t *testing.T) {
158158
// Ensure we haven't made more than 2 (expected 1 + 1 for buffer) requests in the last second.
159159
// if there is a bug where we are spamming the healthcheck route this will catch it.
160160
time.Sleep(time.Second)
161-
require.LessOrEqual(t, *counter, int32(2))
161+
require.LessOrEqual(t, atomic.LoadInt32(counter), int32(2))
162162
}
163163

164164
func setupAppReporter(ctx context.Context, t *testing.T, apps []codersdk.WorkspaceApp, handlers []http.Handler) (agent.WorkspaceAgentApps, func()) {

‎agent/reaper/reaper.go

+16-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package reaper
22

3-
import "github.com/hashicorp/go-reap"
3+
import (
4+
"os"
5+
6+
"github.com/hashicorp/go-reap"
7+
)
48

59
type Option func(o *options)
610

@@ -22,7 +26,16 @@ func WithPIDCallback(ch reap.PidCh) Option {
2226
}
2327
}
2428

29+
// WithCatchSignals sets the signals that are caught and forwarded to the
30+
// child process. By default no signals are forwarded.
31+
func WithCatchSignals(sigs ...os.Signal) Option {
32+
return func(o *options) {
33+
o.CatchSignals = sigs
34+
}
35+
}
36+
2537
type options struct {
26-
ExecArgs []string
27-
PIDs reap.PidCh
38+
ExecArgs []string
39+
PIDs reap.PidCh
40+
CatchSignals []os.Signal
2841
}

‎agent/reaper/reaper_test.go

+42-3
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33
package reaper_test
44

55
import (
6+
"fmt"
67
"os"
78
"os/exec"
9+
"os/signal"
10+
"syscall"
811
"testing"
912
"time"
1013

@@ -15,9 +18,8 @@ import (
1518
"github.com/coder/coder/testutil"
1619
)
1720

21+
//nolint:paralleltest // Non-parallel subtest.
1822
func TestReap(t *testing.T) {
19-
t.Parallel()
20-
2123
// Don't run the reaper test in CI. It does weird
2224
// things like forkexecing which may have unintended
2325
// consequences in CI.
@@ -28,8 +30,9 @@ func TestReap(t *testing.T) {
2830
// OK checks that's the reaper is successfully reaping
2931
// exited processes and passing the PIDs through the shared
3032
// channel.
33+
34+
//nolint:paralleltest // Signal handling.
3135
t.Run("OK", func(t *testing.T) {
32-
t.Parallel()
3336
pids := make(reap.PidCh, 1)
3437
err := reaper.ForkReap(
3538
reaper.WithPIDCallback(pids),
@@ -64,3 +67,39 @@ func TestReap(t *testing.T) {
6467
}
6568
})
6669
}
70+
71+
//nolint:paralleltest // Signal handling.
72+
func TestReapInterrupt(t *testing.T) {
73+
// Don't run the reaper test in CI. It does weird
74+
// things like forkexecing which may have unintended
75+
// consequences in CI.
76+
if _, ok := os.LookupEnv("CI"); ok {
77+
t.Skip("Detected CI, skipping reaper tests")
78+
}
79+
80+
errC := make(chan error, 1)
81+
pids := make(reap.PidCh, 1)
82+
83+
// Use signals to notify when the child process is ready for the
84+
// next step of our test.
85+
usrSig := make(chan os.Signal, 1)
86+
signal.Notify(usrSig, syscall.SIGUSR1, syscall.SIGUSR2)
87+
defer signal.Stop(usrSig)
88+
89+
go func() {
90+
errC <- reaper.ForkReap(
91+
reaper.WithPIDCallback(pids),
92+
reaper.WithCatchSignals(os.Interrupt),
93+
// Signal propagation does not extend to children of children, so
94+
// we create a little bash script to ensure sleep is interrupted.
95+
reaper.WithExecArgs("/bin/sh", "-c", fmt.Sprintf("pid=0; trap 'kill -USR2 %d; kill -TERM $pid' INT; sleep 10 &\npid=$!; kill -USR1 %d; wait", os.Getpid(), os.Getpid())),
96+
)
97+
}()
98+
99+
require.Equal(t, <-usrSig, syscall.SIGUSR1)
100+
err := syscall.Kill(os.Getpid(), syscall.SIGINT)
101+
require.NoError(t, err)
102+
require.Equal(t, <-usrSig, syscall.SIGUSR2)
103+
104+
require.NoError(t, <-errC)
105+
}

‎agent/reaper/reaper_unix.go

+26-3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package reaper
44

55
import (
66
"os"
7+
"os/signal"
78
"syscall"
89

910
"github.com/hashicorp/go-reap"
@@ -15,6 +16,24 @@ func IsInitProcess() bool {
1516
return os.Getpid() == 1
1617
}
1718

19+
func catchSignals(pid int, sigs []os.Signal) {
20+
if len(sigs) == 0 {
21+
return
22+
}
23+
24+
sc := make(chan os.Signal, 1)
25+
signal.Notify(sc, sigs...)
26+
defer signal.Stop(sc)
27+
28+
for {
29+
s := <-sc
30+
sig, ok := s.(syscall.Signal)
31+
if ok {
32+
_ = syscall.Kill(pid, sig)
33+
}
34+
}
35+
}
36+
1837
// ForkReap spawns a goroutine that reaps children. In order to avoid
1938
// complications with spawning `exec.Commands` in the same process that
2039
// is reaping, we forkexec a child process. This prevents a race between
@@ -51,13 +70,17 @@ func ForkReap(opt ...Option) error {
5170
}
5271

5372
//#nosec G204
54-
pid, _ := syscall.ForkExec(opts.ExecArgs[0], opts.ExecArgs, pattrs)
73+
pid, err := syscall.ForkExec(opts.ExecArgs[0], opts.ExecArgs, pattrs)
74+
if err != nil {
75+
return xerrors.Errorf("fork exec: %w", err)
76+
}
77+
78+
go catchSignals(pid, opts.CatchSignals)
5579

5680
var wstatus syscall.WaitStatus
5781
_, err = syscall.Wait4(pid, &wstatus, 0, nil)
5882
for xerrors.Is(err, syscall.EINTR) {
5983
_, err = syscall.Wait4(pid, &wstatus, 0, nil)
6084
}
61-
62-
return nil
85+
return err
6386
}

‎agent/ssh.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (h *forwardedUnixHandler) HandleSSHRequest(ctx ssh.Context, _ *ssh.Server,
7070

7171
// Create socket parent dir if not exists.
7272
parentDir := filepath.Dir(addr)
73-
err = os.MkdirAll(parentDir, 0700)
73+
err = os.MkdirAll(parentDir, 0o700)
7474
if err != nil {
7575
h.log.Warn(ctx, "create parent dir for SSH unix forward request",
7676
slog.F("parent_dir", parentDir),

‎cli/agent.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,10 @@ func workspaceAgent() *cobra.Command {
6868
// Do not start a reaper on the child process. It's important
6969
// to do this else we fork bomb ourselves.
7070
args := append(os.Args, "--no-reap")
71-
err := reaper.ForkReap(reaper.WithExecArgs(args...))
71+
err := reaper.ForkReap(
72+
reaper.WithExecArgs(args...),
73+
reaper.WithCatchSignals(InterruptSignals...),
74+
)
7275
if err != nil {
7376
logger.Error(ctx, "failed to reap", slog.Error(err))
7477
return xerrors.Errorf("fork reap: %w", err)

‎cli/clitest/clitest.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func extractTar(t *testing.T, data []byte, directory string) {
7878
path := filepath.Join(directory, header.Name)
7979
mode := header.FileInfo().Mode()
8080
if mode == 0 {
81-
mode = 0600
81+
mode = 0o600
8282
}
8383
switch header.Typeflag {
8484
case tar.TypeDir:

0 commit comments

Comments
 (0)