Skip to content

Commit 892f016

Browse files
AbhineetJainAbhineetJain
committed
Merge branch 'main' into abhineetjain/template-versions
2 parents 3bec160 + 5e32468 commit 892f016

File tree

64 files changed

+1466
-379
lines changed

Some content is hidden

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

64 files changed

+1466
-379
lines changed

‎.github/workflows/coder.yaml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,10 @@ jobs:
233233
test-go-postgres:
234234
name: "test/go/postgres"
235235
runs-on: ubuntu-latest
236-
# This timeout must be greater than go test -timeout.
236+
# This timeout must be greater than the timeout set by `go test` in
237+
# `make test-postgres` to ensure we receive a trace of running
238+
# goroutines. Setting this to the timeout +5m should work quite well
239+
# even if some of the preceding steps are slow.
237240
timeout-minutes: 25
238241
steps:
239242
- uses: actions/checkout@v3

‎Makefile

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,11 @@ test: test-clean
171171
gotestsum -- -v -short ./...
172172
.PHONY: test
173173

174+
# When updating -timeout for this test, keep in sync with
175+
# test-go-postgres (.github/workflows/coder.yaml).
174176
test-postgres: test-clean test-postgres-docker
175177
DB=ci DB_FROM=$(shell go run scripts/migrate-ci/main.go) gotestsum --junitfile="gotests.xml" --packages="./..." -- \
176-
-covermode=atomic -coverprofile="gotests.coverage" -timeout=30m \
178+
-covermode=atomic -coverprofile="gotests.coverage" -timeout=20m \
177179
-coverpkg=./...,github.com/coder/coder/codersdk \
178180
-count=1 -race -failfast
179181
.PHONY: test-postgres

‎agent/agent_test.go

Lines changed: 5 additions & 8 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),
@@ -224,7 +224,9 @@ func TestAgent(t *testing.T) {
224224
if runtime.GOOS == "windows" {
225225
// Windows uses UTF16! 🪟🪟🪟
226226
content, _, err = transform.Bytes(unicode.UTF16(unicode.LittleEndian, unicode.UseBOM).NewDecoder(), content)
227-
require.NoError(t, err)
227+
if !assert.NoError(t, err) {
228+
return false
229+
}
228230
}
229231
gotContent = string(content)
230232
return true
@@ -324,12 +326,7 @@ func TestAgent(t *testing.T) {
324326
t.Skip("Unix socket forwarding isn't supported on Windows")
325327
}
326328

327-
tmpDir, err := os.MkdirTemp("", "coderd_agent_test_")
328-
require.NoError(t, err, "create temp dir for unix listener")
329-
t.Cleanup(func() {
330-
_ = os.RemoveAll(tmpDir)
331-
})
332-
329+
tmpDir := t.TempDir()
333330
l, err := net.Listen("unix", filepath.Join(tmpDir, "test.sock"))
334331
require.NoError(t, err, "create UDP listener")
335332
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/resetpassword_test.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,15 @@ func TestResetPassword(t *testing.T) {
4444
err = serverCmd.ExecuteContext(ctx)
4545
assert.ErrorIs(t, err, context.Canceled)
4646
}()
47-
var client *codersdk.Client
47+
var rawURL string
4848
require.Eventually(t, func() bool {
49-
rawURL, err := cfg.URL().Read()
50-
if err != nil {
51-
return false
52-
}
53-
accessURL, err := url.Parse(rawURL)
54-
require.NoError(t, err)
55-
client = codersdk.New(accessURL)
56-
return true
49+
rawURL, err = cfg.URL().Read()
50+
return err == nil && rawURL != ""
5751
}, 15*time.Second, 25*time.Millisecond)
52+
accessURL, err := url.Parse(rawURL)
53+
require.NoError(t, err)
54+
client := codersdk.New(accessURL)
55+
5856
_, err = client.CreateFirstUser(ctx, codersdk.CreateFirstUserRequest{
5957
Email: email,
6058
Username: username,

‎cli/server.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -796,8 +796,8 @@ func configureGithubOAuth2(accessURL *url.URL, clientID, clientSecret string, al
796796
})
797797
return memberships, err
798798
},
799-
Team: func(ctx context.Context, client *http.Client, org, teamSlug string) (*github.Team, error) {
800-
team, _, err := github.NewClient(client).Teams.GetTeamBySlug(ctx, org, teamSlug)
799+
TeamMembership: func(ctx context.Context, client *http.Client, org, teamSlug, username string) (*github.Membership, error) {
800+
team, _, err := github.NewClient(client).Teams.GetTeamMembershipBySlug(ctx, org, teamSlug, username)
801801
return team, err
802802
},
803803
}, nil

‎cli/server_test.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,17 +50,15 @@ func TestServer(t *testing.T) {
5050
go func() {
5151
errC <- root.ExecuteContext(ctx)
5252
}()
53-
var client *codersdk.Client
53+
var rawURL string
5454
require.Eventually(t, func() bool {
55-
rawURL, err := cfg.URL().Read()
56-
if err != nil {
57-
return false
58-
}
59-
accessURL, err := url.Parse(rawURL)
60-
assert.NoError(t, err)
61-
client = codersdk.New(accessURL)
62-
return true
55+
rawURL, err = cfg.URL().Read()
56+
return err == nil && rawURL != ""
6357
}, time.Minute, 50*time.Millisecond)
58+
accessURL, err := url.Parse(rawURL)
59+
require.NoError(t, err)
60+
client := codersdk.New(accessURL)
61+
6462
_, err = client.CreateFirstUser(ctx, codersdk.CreateFirstUserRequest{
6563
Email: "some@one.com",
6664
Username: "example",

‎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/coderd_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
153153
// so we wait for it to occur.
154154
require.Eventually(t, func() bool {
155155
provisionerds, err := client.ProvisionerDaemons(ctx)
156-
require.NoError(t, err)
157-
return len(provisionerds) > 0
156+
return assert.NoError(t, err) && len(provisionerds) > 0
158157
}, time.Second*10, time.Second)
159158

160159
provisionerds, err := client.ProvisionerDaemons(ctx)

0 commit comments

Comments
 (0)