diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go index 01ad6942d6b10..872ce87e9049a 100644 --- a/coderd/database/dbfake/databasefake.go +++ b/coderd/database/dbfake/databasefake.go @@ -370,8 +370,10 @@ func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg datab stopTimes []float64 deleteTimes []float64 ) + q.mutex.RLock() + defer q.mutex.RUnlock() for _, wb := range q.workspaceBuilds { - version, err := q.GetTemplateVersionByID(ctx, wb.TemplateVersionID) + version, err := q.getTemplateVersionByIDNoLock(ctx, wb.TemplateVersionID) if err != nil { return emptyRow, err } @@ -379,17 +381,18 @@ func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg datab continue } - job, err := q.GetProvisionerJobByID(ctx, wb.JobID) + job, err := q.getProvisionerJobByIDNoLock(ctx, wb.JobID) if err != nil { return emptyRow, err } if job.CompletedAt.Valid { took := job.CompletedAt.Time.Sub(job.StartedAt.Time).Seconds() - if wb.Transition == database.WorkspaceTransitionStart { + switch wb.Transition { + case database.WorkspaceTransitionStart: startTimes = append(startTimes, took) - } else if wb.Transition == database.WorkspaceTransitionStop { + case database.WorkspaceTransitionStop: stopTimes = append(stopTimes, took) - } else if wb.Transition == database.WorkspaceTransitionDelete { + case database.WorkspaceTransitionDelete: deleteTimes = append(deleteTimes, took) } } @@ -1797,10 +1800,14 @@ func (q *fakeQuerier) GetTemplateVersionParameters(_ context.Context, templateVe return parameters, nil } -func (q *fakeQuerier) GetTemplateVersionByID(_ context.Context, templateVersionID uuid.UUID) (database.TemplateVersion, error) { +func (q *fakeQuerier) GetTemplateVersionByID(ctx context.Context, templateVersionID uuid.UUID) (database.TemplateVersion, error) { q.mutex.RLock() defer q.mutex.RUnlock() + return q.getTemplateVersionByIDNoLock(ctx, templateVersionID) +} + +func (q *fakeQuerier) getTemplateVersionByIDNoLock(_ context.Context, templateVersionID uuid.UUID) (database.TemplateVersion, error) { for _, templateVersion := range q.templateVersions { if templateVersion.ID != templateVersionID { continue @@ -2232,10 +2239,14 @@ func (q *fakeQuerier) GetWorkspaceAppByAgentIDAndSlug(_ context.Context, arg dat return database.WorkspaceApp{}, sql.ErrNoRows } -func (q *fakeQuerier) GetProvisionerJobByID(_ context.Context, id uuid.UUID) (database.ProvisionerJob, error) { +func (q *fakeQuerier) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (database.ProvisionerJob, error) { q.mutex.RLock() defer q.mutex.RUnlock() + return q.getProvisionerJobByIDNoLock(ctx, id) +} + +func (q *fakeQuerier) getProvisionerJobByIDNoLock(_ context.Context, id uuid.UUID) (database.ProvisionerJob, error) { for _, provisionerJob := range q.provisionerJobs { if provisionerJob.ID != id { continue diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 89e22cfd84707..d059b7dd12600 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -1529,19 +1529,25 @@ func TestWorkspaceWatcher(t *testing.T) { // Wait events are easier to debug with timestamped logs. logger := slogtest.Make(t, nil).Named(t.Name()).Leveled(slog.LevelDebug) - wait := func(event string) { - select { - case <-ctx.Done(): - require.FailNow(t, "timed out waiting for event", event) - case <-wc: - logger.Info(ctx, "done waiting for event", slog.F("event", event)) + wait := func(event string, ready func(w codersdk.Workspace) bool) { + for { + select { + case <-ctx.Done(): + require.FailNow(t, "timed out waiting for event", event) + case w, ok := <-wc: + require.True(t, ok, "watch channel closed: %s", event) + if ready == nil || ready(w) { + logger.Info(ctx, "done waiting for event", slog.F("event", event)) + return + } + } } } coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStart) - wait("workspace build being created") - wait("workspace build being acquired") - wait("workspace build completing") + wait("workspace build being created", nil) + wait("workspace build being acquired", nil) + wait("workspace build completing", nil) // Unfortunately, this will add ~1s to the test due to the granularity // of agent timeout seconds. However, if we don't do this we won't know @@ -1549,8 +1555,8 @@ func TestWorkspaceWatcher(t *testing.T) { // // Note that the first timeout is from `coderdtest.CreateWorkspace` and // the latter is from `coderdtest.CreateWorkspaceBuild`. - wait("agent timeout after create") - wait("agent timeout after start") + wait("agent timeout after create", nil) + wait("agent timeout after start", nil) agentClient := agentsdk.New(client.URL) agentClient.SetSessionToken(authToken) @@ -1562,28 +1568,33 @@ func TestWorkspaceWatcher(t *testing.T) { _ = agentCloser.Close() }() - wait("agent connected") + wait("agent connected/ready", func(w codersdk.Workspace) bool { + return w.LatestBuild.Resources[0].Agents[0].Status == codersdk.WorkspaceAgentConnected && + w.LatestBuild.Resources[0].Agents[0].LifecycleState == codersdk.WorkspaceAgentLifecycleReady + }) agentCloser.Close() - wait("agent disconnected") + wait("agent disconnected", func(w codersdk.Workspace) bool { + return w.LatestBuild.Resources[0].Agents[0].Status == codersdk.WorkspaceAgentDisconnected + }) closeFunc.Close() build := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStart) - wait("first is for the workspace build itself") + wait("first is for the workspace build itself", nil) err = client.CancelWorkspaceBuild(ctx, build.ID) require.NoError(t, err) - wait("second is for the build cancel") + wait("second is for the build cancel", nil) err = client.UpdateWorkspace(ctx, workspace.ID, codersdk.UpdateWorkspaceRequest{ Name: "another", }) require.NoError(t, err) - wait("update workspace name") + wait("update workspace name", nil) err = client.UpdateActiveTemplateVersion(ctx, template.ID, codersdk.UpdateActiveTemplateVersion{ ID: template.ActiveVersionID, }) require.NoError(t, err) - wait("update active template version") + wait("update active template version", nil) cancel() } diff --git a/go.mod b/go.mod index 41e96913ce6a2..8f96266282f52 100644 --- a/go.mod +++ b/go.mod @@ -68,7 +68,7 @@ require ( github.com/charmbracelet/glamour v0.6.0 github.com/charmbracelet/lipgloss v0.6.0 github.com/cli/safeexec v1.0.0 - github.com/coder/retry v1.3.0 + github.com/coder/retry v1.3.1-0.20230210155434-e90a2e1e091d github.com/coder/terraform-provider-coder v0.6.11 github.com/coreos/go-oidc/v3 v3.4.0 github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf diff --git a/go.sum b/go.sum index eab133fb40aaa..f71ec500d1f65 100644 --- a/go.sum +++ b/go.sum @@ -364,8 +364,8 @@ github.com/cockroachdb/errors v1.2.4/go.mod h1:rQD95gz6FARkaKkQXUksEje/d9a6wBJoC github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f/go.mod h1:i/u985jwjWRlyHXQbwatDASoW0RMlZ/3i9yJHE2xLkI= github.com/coder/glog v1.0.1-0.20220322161911-7365fe7f2cd1 h1:UqBrPWSYvRI2s5RtOul20JukUEpu4ip9u7biBL+ntgk= github.com/coder/glog v1.0.1-0.20220322161911-7365fe7f2cd1/go.mod h1:EWib/APOK0SL3dFbYqvxE3UYd8E6s1ouQ7iEp/0LWV4= -github.com/coder/retry v1.3.0 h1:5lAAwt/2Cm6lVmnfBY7sOMXcBOwcwJhmV5QGSELIVWY= -github.com/coder/retry v1.3.0/go.mod h1:tXuRgZgWjUnU5LZPT4lJh4ew2elUhexhlnXzrJWdyFY= +github.com/coder/retry v1.3.1-0.20230210155434-e90a2e1e091d h1:09JG37IgTB6n3ouX9BXdUiibGzkGGbslFuDZO9Ru9aw= +github.com/coder/retry v1.3.1-0.20230210155434-e90a2e1e091d/go.mod h1:r+1J5i/989wt6CUeNSuvFKKA9hHuKKPMxdzDbTuvwwk= github.com/coder/ssh v0.0.0-20220811105153-fcea99919338 h1:tN5GKFT68YLVzJoA8AHuiMNJ0qlhoD3pGN3JY9gxSko= github.com/coder/ssh v0.0.0-20220811105153-fcea99919338/go.mod h1:ZSS+CUoKHDrqVakTfTWUlKSr9MtMFkC4UvtQKD7O914= github.com/coder/tailscale v1.1.1-0.20230210022917-446fc10e755a h1:fTXoK+Yikz8Rl0v0nHxO+lTV0y8Q7wdmGFq1CqLgznE=