Skip to content

Commit 7b7124d

Browse files
mafredrimafredri
authored
Merge branch 'main' into mafredri/fix-use-of-require-in-require-eventually
2 parents d54b923 + fa4361d commit 7b7124d

File tree

19 files changed

+373
-122
lines changed

19 files changed

+373
-122
lines changed

‎agent/agent_test.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ func TestAgent(t *testing.T) {
206206

207207
t.Run("StartupScript", func(t *testing.T) {
208208
t.Parallel()
209-
tempPath := filepath.Join(os.TempDir(), "content.txt")
209+
tempPath := filepath.Join(t.TempDir(), "content.txt")
210210
content := "somethingnice"
211211
setupAgent(t, agent.Metadata{
212212
StartupScript: fmt.Sprintf("echo %s > %s", content, tempPath),
@@ -326,12 +326,7 @@ func TestAgent(t *testing.T) {
326326
t.Skip("Unix socket forwarding isn't supported on Windows")
327327
}
328328

329-
tmpDir, err := os.MkdirTemp("", "coderd_agent_test_")
330-
require.NoError(t, err, "create temp dir for unix listener")
331-
t.Cleanup(func() {
332-
_ = os.RemoveAll(tmpDir)
333-
})
334-
329+
tmpDir := t.TempDir()
335330
l, err := net.Listen("unix", filepath.Join(tmpDir, "test.sock"))
336331
require.NoError(t, err, "create UDP listener")
337332
return l

‎cli/create_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ func TestCreate(t *testing.T) {
192192
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
193193
_ = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
194194
tempDir := t.TempDir()
195+
removeTmpDirUntilSuccessAfterTest(t, tempDir)
195196
parameterFile, _ := os.CreateTemp(tempDir, "testParameterFile*.yaml")
196197
_, _ = parameterFile.WriteString("region: \"bingo\"\nusername: \"boingo\"")
197198
cmd, root := clitest.New(t, "create", "", "--parameter-file", parameterFile.Name())
@@ -217,7 +218,6 @@ func TestCreate(t *testing.T) {
217218
pty.WriteLine(value)
218219
}
219220
<-doneChan
220-
removeTmpDirUntilSuccess(t, tempDir)
221221
})
222222

223223
t.Run("WithParameterFileNotContainingTheValue", func(t *testing.T) {
@@ -234,6 +234,7 @@ func TestCreate(t *testing.T) {
234234
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
235235
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
236236
tempDir := t.TempDir()
237+
removeTmpDirUntilSuccessAfterTest(t, tempDir)
237238
parameterFile, _ := os.CreateTemp(tempDir, "testParameterFile*.yaml")
238239
_, _ = parameterFile.WriteString("zone: \"bananas\"")
239240
cmd, root := clitest.New(t, "create", "my-workspace", "--template", template.Name, "--parameter-file", parameterFile.Name())
@@ -248,7 +249,6 @@ func TestCreate(t *testing.T) {
248249
assert.EqualError(t, err, "Parameter value absent in parameter file for \"region\"!")
249250
}()
250251
<-doneChan
251-
removeTmpDirUntilSuccess(t, tempDir)
252252
})
253253

254254
t.Run("FailedDryRun", func(t *testing.T) {

‎cli/list_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ func TestList(t *testing.T) {
1616
t.Parallel()
1717
t.Run("Single", func(t *testing.T) {
1818
t.Parallel()
19-
ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second)
20-
defer cancelFunc()
2119
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
2220
user := coderdtest.CreateFirstUser(t, client)
2321
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
@@ -30,6 +28,9 @@ func TestList(t *testing.T) {
3028
pty := ptytest.New(t)
3129
cmd.SetIn(pty.Input())
3230
cmd.SetOut(pty.Output())
31+
32+
ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second)
33+
defer cancelFunc()
3334
done := make(chan any)
3435
go func() {
3536
errC := cmd.ExecuteContext(ctx)

‎cli/portforward_test.go

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -119,23 +119,13 @@ func TestPortForward(t *testing.T) {
119119
t.Skip("Unix socket forwarding isn't supported on Windows")
120120
}
121121

122-
tmpDir, err := os.MkdirTemp("", "coderd_agent_test_")
123-
require.NoError(t, err, "create temp dir for unix listener")
124-
t.Cleanup(func() {
125-
_ = os.RemoveAll(tmpDir)
126-
})
127-
122+
tmpDir := t.TempDir()
128123
l, err := net.Listen("unix", filepath.Join(tmpDir, "test.sock"))
129124
require.NoError(t, err, "create UDP listener")
130125
return l
131126
},
132127
setupLocal: func(t *testing.T) (string, string) {
133-
tmpDir, err := os.MkdirTemp("", "coderd_agent_test_")
134-
require.NoError(t, err, "create temp dir for unix listener")
135-
t.Cleanup(func() {
136-
_ = os.RemoveAll(tmpDir)
137-
})
138-
128+
tmpDir := t.TempDir()
139129
path := filepath.Join(tmpDir, "test.sock")
140130
return path, path
141131
},

‎cli/templatecreate_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ func TestTemplateCreate(t *testing.T) {
132132
ProvisionDryRun: echo.ProvisionComplete,
133133
})
134134
tempDir := t.TempDir()
135+
removeTmpDirUntilSuccessAfterTest(t, tempDir)
135136
parameterFile, _ := os.CreateTemp(tempDir, "testParameterFile*.yaml")
136137
_, _ = parameterFile.WriteString("region: \"bananas\"")
137138
cmd, root := clitest.New(t, "templates", "create", "my-template", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--parameter-file", parameterFile.Name())
@@ -158,7 +159,6 @@ func TestTemplateCreate(t *testing.T) {
158159
}
159160

160161
require.NoError(t, <-execDone)
161-
removeTmpDirUntilSuccess(t, tempDir)
162162
})
163163

164164
t.Run("WithParameterFileNotContainingTheValue", func(t *testing.T) {
@@ -171,6 +171,7 @@ func TestTemplateCreate(t *testing.T) {
171171
ProvisionDryRun: echo.ProvisionComplete,
172172
})
173173
tempDir := t.TempDir()
174+
removeTmpDirUntilSuccessAfterTest(t, tempDir)
174175
parameterFile, _ := os.CreateTemp(tempDir, "testParameterFile*.yaml")
175176
_, _ = parameterFile.WriteString("zone: \"bananas\"")
176177
cmd, root := clitest.New(t, "templates", "create", "my-template", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--parameter-file", parameterFile.Name())
@@ -196,7 +197,6 @@ func TestTemplateCreate(t *testing.T) {
196197
}
197198

198199
require.EqualError(t, <-execDone, "Parameter value absent in parameter file for \"region\"!")
199-
removeTmpDirUntilSuccess(t, tempDir)
200200
})
201201

202202
t.Run("Recreate template with same name (create, delete, create)", func(t *testing.T) {
@@ -267,7 +267,7 @@ func createTestParseResponse() []*proto.Parse_Response {
267267

268268
// Need this for Windows because of a known issue with Go:
269269
// https://github.com/golang/go/issues/52986
270-
func removeTmpDirUntilSuccess(t *testing.T, tempDir string) {
270+
func removeTmpDirUntilSuccessAfterTest(t *testing.T, tempDir string) {
271271
t.Helper()
272272
t.Cleanup(func() {
273273
err := os.RemoveAll(tempDir)

‎coderd/autobuild/executor/lifecycle_executor.go

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -113,46 +113,11 @@ func (e *Executor) runOnce(t time.Time) Stats {
113113
continue
114114
}
115115

116-
if !priorJob.CompletedAt.Valid || priorJob.Error.String != "" {
117-
e.log.Debug(e.ctx, "last workspace build did not complete successfully, skipping",
118-
slog.F("workspace_id", ws.ID),
119-
slog.F("error", priorJob.Error.String),
120-
)
121-
continue
122-
}
123-
124-
var validTransition database.WorkspaceTransition
125-
var nextTransition time.Time
126-
switch priorHistory.Transition {
127-
case database.WorkspaceTransitionStart:
128-
validTransition = database.WorkspaceTransitionStop
129-
if priorHistory.Deadline.IsZero() {
130-
e.log.Debug(e.ctx, "latest workspace build has zero deadline, skipping",
131-
slog.F("workspace_id", ws.ID),
132-
slog.F("workspace_build_id", priorHistory.ID),
133-
)
134-
continue
135-
}
136-
// For stopping, do not truncate. This is inconsistent with autostart, but
137-
// it ensures we will not stop too early.
138-
nextTransition = priorHistory.Deadline
139-
case database.WorkspaceTransitionStop:
140-
validTransition = database.WorkspaceTransitionStart
141-
sched, err := schedule.Weekly(ws.AutostartSchedule.String)
142-
if err != nil {
143-
e.log.Debug(e.ctx, "workspace has invalid autostart schedule, skipping",
144-
slog.F("workspace_id", ws.ID),
145-
slog.F("autostart_schedule", ws.AutostartSchedule.String),
146-
)
147-
continue
148-
}
149-
// Round down to the nearest minute, as this is the finest granularity cron supports.
150-
// Truncate is probably not necessary here, but doing it anyway to be sure.
151-
nextTransition = sched.Next(priorHistory.CreatedAt).Truncate(time.Minute)
152-
default:
153-
e.log.Debug(e.ctx, "last transition not valid for autostart or autostop",
116+
validTransition, nextTransition, err := getNextTransition(ws, priorHistory, priorJob)
117+
if err != nil {
118+
e.log.Debug(e.ctx, "skipping workspace",
119+
slog.Error(err),
154120
slog.F("workspace_id", ws.ID),
155-
slog.F("latest_build_transition", priorHistory.Transition),
156121
)
157122
continue
158123
}
@@ -186,6 +151,41 @@ func (e *Executor) runOnce(t time.Time) Stats {
186151
return stats
187152
}
188153

154+
func getNextTransition(
155+
ws database.Workspace,
156+
priorHistory database.WorkspaceBuild,
157+
priorJob database.ProvisionerJob,
158+
) (
159+
validTransition database.WorkspaceTransition,
160+
nextTransition time.Time,
161+
err error,
162+
) {
163+
if !priorJob.CompletedAt.Valid || priorJob.Error.String != "" {
164+
return "", time.Time{}, xerrors.Errorf("last workspace build did not complete successfully")
165+
}
166+
167+
switch priorHistory.Transition {
168+
case database.WorkspaceTransitionStart:
169+
if priorHistory.Deadline.IsZero() {
170+
return "", time.Time{}, xerrors.Errorf("latest workspace build has zero deadline")
171+
}
172+
// For stopping, do not truncate. This is inconsistent with autostart, but
173+
// it ensures we will not stop too early.
174+
return database.WorkspaceTransitionStop, priorHistory.Deadline, nil
175+
case database.WorkspaceTransitionStop:
176+
sched, err := schedule.Weekly(ws.AutostartSchedule.String)
177+
if err != nil {
178+
return "", time.Time{}, xerrors.Errorf("workspace has invalid autostart schedule: %w", err)
179+
}
180+
// Round down to the nearest minute, as this is the finest granularity cron supports.
181+
// Truncate is probably not necessary here, but doing it anyway to be sure.
182+
nextTransition = sched.Next(priorHistory.CreatedAt).Truncate(time.Minute)
183+
return database.WorkspaceTransitionStart, nextTransition, nil
184+
default:
185+
return "", time.Time{}, xerrors.Errorf("last transition not valid for autostart or autostop")
186+
}
187+
}
188+
189189
// TODO(cian): this function duplicates most of api.postWorkspaceBuilds. Refactor.
190190
// See: https://github.com/coder/coder/issues/1401
191191
func build(ctx context.Context, store database.Store, workspace database.Workspace, trans database.WorkspaceTransition, priorHistory database.WorkspaceBuild, priorJob database.ProvisionerJob) error {

‎coderd/devtunnel/tunnel.go

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,19 @@ type Config struct {
3636
PublicKey device.NoisePublicKey `json:"public_key"`
3737

3838
Tunnel Node `json:"tunnel"`
39+
40+
// Used in testing. Normally this is nil, indicating to use DefaultClient.
41+
HTTPClient *http.Client `json:"-"`
3942
}
4043
type configExt struct {
4144
Version int `json:"-"`
4245
PrivateKey device.NoisePrivateKey `json:"-"`
4346
PublicKey device.NoisePublicKey `json:"public_key"`
4447

4548
Tunnel Node `json:"-"`
49+
50+
// Used in testing. Normally this is nil, indicating to use DefaultClient.
51+
HTTPClient *http.Client `json:"-"`
4652
}
4753

4854
// NewWithConfig calls New with the given config. For documentation, see New.
@@ -65,17 +71,23 @@ func NewWithConfig(ctx context.Context, logger slog.Logger, cfg Config) (*Tunnel
6571
if err != nil {
6672
return nil, nil, xerrors.Errorf("resolve endpoint: %w", err)
6773
}
74+
// In IPv6, we need to enclose the address to in [] before passing to wireguard's endpoint key, like
75+
// [2001:abcd::1]:8888. We'll use netip.AddrPort to correctly handle this.
76+
wgAddr, err := netip.ParseAddr(wgip.String())
77+
if err != nil {
78+
return nil, nil, xerrors.Errorf("parse address: %w", err)
79+
}
80+
wgEndpoint := netip.AddrPortFrom(wgAddr, cfg.Tunnel.WireguardPort)
6881

69-
dev := device.NewDevice(tun, conn.NewDefaultBind(), device.NewLogger(device.LogLevelSilent, ""))
82+
dev := device.NewDevice(tun, conn.NewDefaultBind(), device.NewLogger(device.LogLevelError, "devtunnel "))
7083
err = dev.IpcSet(fmt.Sprintf(`private_key=%s
7184
public_key=%s
72-
endpoint=%s:%d
85+
endpoint=%s
7386
persistent_keepalive_interval=21
7487
allowed_ip=%s/128`,
7588
hex.EncodeToString(cfg.PrivateKey[:]),
7689
server.ServerPublicKey,
77-
wgip.IP.String(),
78-
cfg.Tunnel.WireguardPort,
90+
wgEndpoint.String(),
7991
server.ServerIP.String(),
8092
))
8193
if err != nil {
@@ -97,6 +109,9 @@ allowed_ip=%s/128`,
97109
select {
98110
case <-ctx.Done():
99111
_ = wgListen.Close()
112+
// We need to remove peers before closing to avoid a race condition between dev.Close() and the peer
113+
// goroutines which results in segfault.
114+
dev.RemoveAllPeers()
100115
dev.Close()
101116
<-routineEnd
102117
close(ch)
@@ -174,7 +189,11 @@ func sendConfigToServer(ctx context.Context, cfg Config) (ServerResponse, error)
174189
return ServerResponse{}, xerrors.Errorf("new request: %w", err)
175190
}
176191

177-
res, err := http.DefaultClient.Do(req)
192+
client := http.DefaultClient
193+
if cfg.HTTPClient != nil {
194+
client = cfg.HTTPClient
195+
}
196+
res, err := client.Do(req)
178197
if err != nil {
179198
return ServerResponse{}, xerrors.Errorf("do request: %w", err)
180199
}

0 commit comments

Comments
 (0)