From 9f402fa27f75889c56acae6a114244e35911dfa4 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Wed, 18 May 2022 09:33:33 -0700 Subject: [PATCH] Spike/222 workspace build order (#1534) * chore: refactor before_id/after_id to build_number Signed-off-by: Spike Curtis * pagination of workspace_builds Signed-off-by: Spike Curtis * Disable parallel on postgres tests Signed-off-by: Spike Curtis * Fix lint Signed-off-by: Spike Curtis * Fix workspace build postgres query Signed-off-by: Spike Curtis * Fix JS tests Signed-off-by: Spike Curtis * Fix workspace builds postgres query Signed-off-by: Spike Curtis --- cli/resetpassword_test.go | 4 +- cli/server_test.go | 5 +- .../autobuild/executor/lifecycle_executor.go | 28 +- .../executor/lifecycle_executor_test.go | 9 +- coderd/coderdtest/coderdtest.go | 14 + coderd/database/databasefake/databasefake.go | 85 ++++-- coderd/database/dump.sql | 6 +- coderd/database/migrations/000004_jobs.up.sql | 6 +- coderd/database/models.go | 3 +- coderd/database/postgres/postgres_test.go | 4 +- coderd/database/pubsub_test.go | 8 +- coderd/database/querier.go | 6 +- coderd/database/queries.sql.go | 252 ++++++++++-------- coderd/database/queries/workspacebuilds.sql | 60 ++++- coderd/workspaceagents.go | 2 +- coderd/workspacebuilds.go | 46 ++-- coderd/workspacebuilds_test.go | 44 ++- coderd/workspaceresourceauth.go | 2 +- coderd/workspaces.go | 21 +- coderd/workspaces_test.go | 10 +- codersdk/workspacebuilds.go | 5 +- codersdk/workspaces.go | 10 +- site/src/api/typesGenerated.ts | 16 +- site/src/testHelpers/entities.ts | 3 +- 24 files changed, 410 insertions(+), 239 deletions(-) diff --git a/cli/resetpassword_test.go b/cli/resetpassword_test.go index eafa097e3e..20f69cda7e 100644 --- a/cli/resetpassword_test.go +++ b/cli/resetpassword_test.go @@ -15,8 +15,10 @@ import ( "github.com/coder/coder/pty/ptytest" ) +// nolint:paralleltest func TestResetPassword(t *testing.T) { - t.Parallel() + // postgres.Open() seems to be creating race conditions when run in parallel. + // t.Parallel() if runtime.GOOS != "linux" || testing.Short() { // Skip on non-Linux because it spawns a PostgreSQL instance. diff --git a/cli/server_test.go b/cli/server_test.go index f209443678..ef0d72a1cd 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -32,10 +32,11 @@ import ( ) // This cannot be ran in parallel because it uses a signal. -// nolint:tparallel +// nolint:paralleltest func TestServer(t *testing.T) { t.Run("Production", func(t *testing.T) { - t.Parallel() + // postgres.Open() seems to be creating race conditions when run in parallel. + // t.Parallel() if runtime.GOOS != "linux" || testing.Short() { // Skip on non-Linux because it spawns a PostgreSQL instance. t.SkipNow() diff --git a/coderd/autobuild/executor/lifecycle_executor.go b/coderd/autobuild/executor/lifecycle_executor.go index 3fd1eb7fc2..f402e7cedc 100644 --- a/coderd/autobuild/executor/lifecycle_executor.go +++ b/coderd/autobuild/executor/lifecycle_executor.go @@ -57,7 +57,7 @@ func (e *Executor) runOnce(t time.Time) error { for _, ws := range eligibleWorkspaces { // Determine the workspace state based on its latest build. - priorHistory, err := db.GetWorkspaceBuildByWorkspaceIDWithoutAfter(e.ctx, ws.ID) + priorHistory, err := db.GetLatestWorkspaceBuildByWorkspaceID(e.ctx, ws.ID) if err != nil { e.log.Warn(e.ctx, "get latest workspace build", slog.F("workspace_id", ws.ID), @@ -152,12 +152,8 @@ func build(ctx context.Context, store database.Store, workspace database.Workspa return xerrors.Errorf("get workspace template: %w", err) } - priorHistoryID := uuid.NullUUID{ - UUID: priorHistory.ID, - Valid: true, - } + priorBuildNumber := priorHistory.BuildNumber - var newWorkspaceBuild database.WorkspaceBuild // This must happen in a transaction to ensure history can be inserted, and // the prior history can update it's "after" column to point at the new. workspaceBuildID := uuid.New() @@ -186,13 +182,13 @@ func build(ctx context.Context, store database.Store, workspace database.Workspa if err != nil { return xerrors.Errorf("insert provisioner job: %w", err) } - newWorkspaceBuild, err = store.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{ + _, err = store.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{ ID: workspaceBuildID, CreatedAt: now, UpdatedAt: now, WorkspaceID: workspace.ID, TemplateVersionID: priorHistory.TemplateVersionID, - BeforeID: priorHistoryID, + BuildNumber: priorBuildNumber + 1, Name: namesgenerator.GetRandomName(1), ProvisionerState: priorHistory.ProvisionerState, InitiatorID: workspace.OwnerID, @@ -202,21 +198,5 @@ func build(ctx context.Context, store database.Store, workspace database.Workspa if err != nil { return xerrors.Errorf("insert workspace build: %w", err) } - - if priorHistoryID.Valid { - // Update the prior history entries "after" column. - err = store.UpdateWorkspaceBuildByID(ctx, database.UpdateWorkspaceBuildByIDParams{ - ID: priorHistory.ID, - ProvisionerState: priorHistory.ProvisionerState, - UpdatedAt: now, - AfterID: uuid.NullUUID{ - UUID: newWorkspaceBuild.ID, - Valid: true, - }, - }) - if err != nil { - return xerrors.Errorf("update prior workspace build: %w", err) - } - } return nil } diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index 445be2d888..38642cd541 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -419,10 +419,17 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { require.NotEqual(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected a workspace build to occur") require.Equal(t, codersdk.ProvisionerJobSucceeded, ws.LatestBuild.Job.Status, "expected provisioner job to have succeeded") require.Equal(t, database.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected latest transition to be start") - builds, err := client.WorkspaceBuilds(ctx, ws.ID) + builds, err := client.WorkspaceBuilds(ctx, codersdk.WorkspaceBuildsRequest{WorkspaceID: ws.ID}) require.NoError(t, err, "fetch list of workspace builds from primary") // One build to start, one stop transition, and one autostart. No more. + require.Equal(t, database.WorkspaceTransitionStart, builds[0].Transition) + require.Equal(t, database.WorkspaceTransitionStop, builds[1].Transition) + require.Equal(t, database.WorkspaceTransitionStart, builds[2].Transition) require.Len(t, builds, 3, "unexpected number of builds for workspace from primary") + + // Builds are returned most recent first. + require.True(t, builds[0].CreatedAt.After(builds[1].CreatedAt)) + require.True(t, builds[1].CreatedAt.After(builds[2].CreatedAt)) } func mustProvisionWorkspace(t *testing.T, client *codersdk.Client) codersdk.Workspace { diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index b9c043075f..117e96e6ac 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -295,6 +295,20 @@ func CreateTemplateVersion(t *testing.T, client *codersdk.Client, organizationID return templateVersion } +// CreateWorkspaceBuild creates a workspace build for the given workspace and transition. +func CreateWorkspaceBuild( + t *testing.T, + client *codersdk.Client, + workspace codersdk.Workspace, + transition database.WorkspaceTransition) codersdk.WorkspaceBuild { + req := codersdk.CreateWorkspaceBuildRequest{ + Transition: transition, + } + build, err := client.CreateWorkspaceBuild(context.Background(), workspace.ID, req) + require.NoError(t, err) + return build +} + // CreateTemplate creates a template with the "echo" provisioner for // compatibility with testing. The name assigned is randomly generated. func CreateTemplate(t *testing.T, client *codersdk.Client, organization uuid.UUID, version uuid.UUID) codersdk.Template { diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 6f213048d8..e876a54fda 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -441,50 +441,100 @@ func (q *fakeQuerier) GetWorkspaceBuildByJobID(_ context.Context, jobID uuid.UUI return database.WorkspaceBuild{}, sql.ErrNoRows } -func (q *fakeQuerier) GetWorkspaceBuildByWorkspaceIDWithoutAfter(_ context.Context, workspaceID uuid.UUID) (database.WorkspaceBuild, error) { +func (q *fakeQuerier) GetLatestWorkspaceBuildByWorkspaceID(_ context.Context, workspaceID uuid.UUID) (database.WorkspaceBuild, error) { q.mutex.RLock() defer q.mutex.RUnlock() + var row database.WorkspaceBuild + var buildNum int32 for _, workspaceBuild := range q.workspaceBuilds { - if workspaceBuild.WorkspaceID.String() != workspaceID.String() { - continue - } - if !workspaceBuild.AfterID.Valid { - return workspaceBuild, nil + if workspaceBuild.WorkspaceID.String() == workspaceID.String() && workspaceBuild.BuildNumber > buildNum { + row = workspaceBuild + buildNum = workspaceBuild.BuildNumber } } - return database.WorkspaceBuild{}, sql.ErrNoRows + if buildNum == 0 { + return database.WorkspaceBuild{}, sql.ErrNoRows + } + return row, nil } -func (q *fakeQuerier) GetWorkspaceBuildsByWorkspaceIDsWithoutAfter(_ context.Context, ids []uuid.UUID) ([]database.WorkspaceBuild, error) { +func (q *fakeQuerier) GetLatestWorkspaceBuildsByWorkspaceIDs(_ context.Context, ids []uuid.UUID) ([]database.WorkspaceBuild, error) { q.mutex.RLock() defer q.mutex.RUnlock() - builds := make([]database.WorkspaceBuild, 0) + builds := make(map[uuid.UUID]database.WorkspaceBuild) + buildNumbers := make(map[uuid.UUID]int32) for _, workspaceBuild := range q.workspaceBuilds { for _, id := range ids { - if id.String() != workspaceBuild.WorkspaceID.String() { - continue + if id.String() == workspaceBuild.WorkspaceID.String() && workspaceBuild.BuildNumber > buildNumbers[id] { + builds[id] = workspaceBuild + buildNumbers[id] = workspaceBuild.BuildNumber } - builds = append(builds, workspaceBuild) } } - if len(builds) == 0 { + var returnBuilds []database.WorkspaceBuild + for i, n := range buildNumbers { + if n > 0 { + b := builds[i] + returnBuilds = append(returnBuilds, b) + } + } + if len(returnBuilds) == 0 { return nil, sql.ErrNoRows } - return builds, nil + return returnBuilds, nil } -func (q *fakeQuerier) GetWorkspaceBuildByWorkspaceID(_ context.Context, workspaceID uuid.UUID) ([]database.WorkspaceBuild, error) { +func (q *fakeQuerier) GetWorkspaceBuildByWorkspaceID(_ context.Context, + params database.GetWorkspaceBuildByWorkspaceIDParams) ([]database.WorkspaceBuild, error) { q.mutex.RLock() defer q.mutex.RUnlock() history := make([]database.WorkspaceBuild, 0) for _, workspaceBuild := range q.workspaceBuilds { - if workspaceBuild.WorkspaceID.String() == workspaceID.String() { + if workspaceBuild.WorkspaceID.String() == params.WorkspaceID.String() { history = append(history, workspaceBuild) } } + + // Order by build_number + slices.SortFunc(history, func(a, b database.WorkspaceBuild) bool { + // use greater than since we want descending order + return a.BuildNumber > b.BuildNumber + }) + + if params.AfterID != uuid.Nil { + found := false + for i, v := range history { + if v.ID == params.AfterID { + // We want to return all builds after index i. + history = history[i+1:] + found = true + break + } + } + + // If no builds after the time, then we return an empty list. + if !found { + return nil, sql.ErrNoRows + } + } + + if params.OffsetOpt > 0 { + if int(params.OffsetOpt) > len(history)-1 { + return nil, sql.ErrNoRows + } + history = history[params.OffsetOpt:] + } + + if params.LimitOpt > 0 { + if int(params.LimitOpt) > len(history) { + params.LimitOpt = int32(len(history)) + } + history = history[:params.LimitOpt] + } + if len(history) == 0 { return nil, sql.ErrNoRows } @@ -1429,7 +1479,7 @@ func (q *fakeQuerier) InsertWorkspaceBuild(_ context.Context, arg database.Inser WorkspaceID: arg.WorkspaceID, Name: arg.Name, TemplateVersionID: arg.TemplateVersionID, - BeforeID: arg.BeforeID, + BuildNumber: arg.BuildNumber, Transition: arg.Transition, InitiatorID: arg.InitiatorID, JobID: arg.JobID, @@ -1641,7 +1691,6 @@ func (q *fakeQuerier) UpdateWorkspaceBuildByID(_ context.Context, arg database.U continue } workspaceBuild.UpdatedAt = arg.UpdatedAt - workspaceBuild.AfterID = arg.AfterID workspaceBuild.ProvisionerState = arg.ProvisionerState q.workspaceBuilds[index] = workspaceBuild return nil diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index b69ca30566..f1041e3519 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -288,8 +288,7 @@ CREATE TABLE workspace_builds ( workspace_id uuid NOT NULL, template_version_id uuid NOT NULL, name character varying(64) NOT NULL, - before_id uuid, - after_id uuid, + build_number integer NOT NULL, transition workspace_transition NOT NULL, initiator_id uuid NOT NULL, provisioner_state bytea, @@ -389,6 +388,9 @@ ALTER TABLE ONLY workspace_builds ALTER TABLE ONLY workspace_builds ADD CONSTRAINT workspace_builds_pkey PRIMARY KEY (id); +ALTER TABLE ONLY workspace_builds + ADD CONSTRAINT workspace_builds_workspace_id_build_number_key UNIQUE (workspace_id, build_number); + ALTER TABLE ONLY workspace_builds ADD CONSTRAINT workspace_builds_workspace_id_name_key UNIQUE (workspace_id, name); diff --git a/coderd/database/migrations/000004_jobs.up.sql b/coderd/database/migrations/000004_jobs.up.sql index d1c6633f09..cc87d95b95 100644 --- a/coderd/database/migrations/000004_jobs.up.sql +++ b/coderd/database/migrations/000004_jobs.up.sql @@ -165,8 +165,7 @@ CREATE TABLE workspace_builds ( workspace_id uuid NOT NULL REFERENCES workspaces (id) ON DELETE CASCADE, template_version_id uuid NOT NULL REFERENCES template_versions (id) ON DELETE CASCADE, name varchar(64) NOT NULL, - before_id uuid, - after_id uuid, + build_number integer NOT NULL, transition workspace_transition NOT NULL, initiator_id uuid NOT NULL, -- State stored by the provisioner @@ -174,5 +173,6 @@ CREATE TABLE workspace_builds ( -- Job ID of the action job_id uuid NOT NULL UNIQUE REFERENCES provisioner_jobs (id) ON DELETE CASCADE, PRIMARY KEY (id), - UNIQUE(workspace_id, name) + UNIQUE(workspace_id, name), + UNIQUE(workspace_id, build_number) ); diff --git a/coderd/database/models.go b/coderd/database/models.go index 6ac61e6da1..a705a02410 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -501,8 +501,7 @@ type WorkspaceBuild struct { WorkspaceID uuid.UUID `db:"workspace_id" json:"workspace_id"` TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` Name string `db:"name" json:"name"` - BeforeID uuid.NullUUID `db:"before_id" json:"before_id"` - AfterID uuid.NullUUID `db:"after_id" json:"after_id"` + BuildNumber int32 `db:"build_number" json:"build_number"` Transition WorkspaceTransition `db:"transition" json:"transition"` InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` ProvisionerState []byte `db:"provisioner_state" json:"provisioner_state"` diff --git a/coderd/database/postgres/postgres_test.go b/coderd/database/postgres/postgres_test.go index 178a434c3b..d11fdb21e8 100644 --- a/coderd/database/postgres/postgres_test.go +++ b/coderd/database/postgres/postgres_test.go @@ -17,8 +17,10 @@ func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } +// nolint:paralleltest func TestPostgres(t *testing.T) { - t.Parallel() + // postgres.Open() seems to be creating race conditions when run in parallel. + // t.Parallel() if testing.Short() { t.Skip() diff --git a/coderd/database/pubsub_test.go b/coderd/database/pubsub_test.go index 1312326a20..73bd96dd15 100644 --- a/coderd/database/pubsub_test.go +++ b/coderd/database/pubsub_test.go @@ -22,8 +22,10 @@ func TestPubsub(t *testing.T) { return } + // nolint:paralleltest t.Run("Postgres", func(t *testing.T) { - t.Parallel() + // postgres.Open() seems to be creating race conditions when run in parallel. + // t.Parallel() ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() @@ -52,8 +54,10 @@ func TestPubsub(t *testing.T) { assert.Equal(t, string(message), data) }) + // nolint:paralleltest t.Run("PostgresCloseCancel", func(t *testing.T) { - t.Parallel() + // postgres.Open() seems to be creating race conditions when run in parallel. + // t.Parallel() ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() connectionURL, closePg, err := postgres.Open() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 308c1a14ae..387a2c9a06 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -27,6 +27,8 @@ type querier interface { GetAuditLogsBefore(ctx context.Context, arg GetAuditLogsBeforeParams) ([]AuditLog, error) GetFileByHash(ctx context.Context, hash string) (File, error) GetGitSSHKey(ctx context.Context, userID uuid.UUID) (GitSSHKey, error) + GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, workspaceID uuid.UUID) (WorkspaceBuild, error) + GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceBuild, error) GetOrganizationByID(ctx context.Context, id uuid.UUID) (Organization, error) GetOrganizationByName(ctx context.Context, name string) (Organization, error) GetOrganizationIDsByMemberIDs(ctx context.Context, ids []uuid.UUID) ([]GetOrganizationIDsByMemberIDsRow, error) @@ -61,10 +63,8 @@ type querier interface { GetWorkspaceAgentsByResourceIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgent, error) GetWorkspaceBuildByID(ctx context.Context, id uuid.UUID) (WorkspaceBuild, error) GetWorkspaceBuildByJobID(ctx context.Context, jobID uuid.UUID) (WorkspaceBuild, error) - GetWorkspaceBuildByWorkspaceID(ctx context.Context, workspaceID uuid.UUID) ([]WorkspaceBuild, error) + GetWorkspaceBuildByWorkspaceID(ctx context.Context, arg GetWorkspaceBuildByWorkspaceIDParams) ([]WorkspaceBuild, error) GetWorkspaceBuildByWorkspaceIDAndName(ctx context.Context, arg GetWorkspaceBuildByWorkspaceIDAndNameParams) (WorkspaceBuild, error) - GetWorkspaceBuildByWorkspaceIDWithoutAfter(ctx context.Context, workspaceID uuid.UUID) (WorkspaceBuild, error) - GetWorkspaceBuildsByWorkspaceIDsWithoutAfter(ctx context.Context, ids []uuid.UUID) ([]WorkspaceBuild, error) GetWorkspaceByID(ctx context.Context, id uuid.UUID) (Workspace, error) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg GetWorkspaceByOwnerIDAndNameParams) (Workspace, error) GetWorkspaceOwnerCountsByTemplateIDs(ctx context.Context, ids []uuid.UUID) ([]GetWorkspaceOwnerCountsByTemplateIDsRow, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 24e96f792c..5985afbf33 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2749,9 +2749,93 @@ func (q *sqlQuerier) UpdateWorkspaceAgentConnectionByID(ctx context.Context, arg return err } +const getLatestWorkspaceBuildByWorkspaceID = `-- name: GetLatestWorkspaceBuildByWorkspaceID :one +SELECT + id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id +FROM + workspace_builds +WHERE + workspace_id = $1 +ORDER BY + build_number desc +LIMIT + 1 +` + +func (q *sqlQuerier) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, workspaceID uuid.UUID) (WorkspaceBuild, error) { + row := q.db.QueryRowContext(ctx, getLatestWorkspaceBuildByWorkspaceID, workspaceID) + var i WorkspaceBuild + err := row.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.WorkspaceID, + &i.TemplateVersionID, + &i.Name, + &i.BuildNumber, + &i.Transition, + &i.InitiatorID, + &i.ProvisionerState, + &i.JobID, + ) + return i, err +} + +const getLatestWorkspaceBuildsByWorkspaceIDs = `-- name: GetLatestWorkspaceBuildsByWorkspaceIDs :many +SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.name, wb.build_number, wb.transition, wb.initiator_id, wb.provisioner_state, wb.job_id +FROM ( + SELECT + workspace_id, MAX(build_number) as max_build_number + FROM + workspace_builds + WHERE + workspace_id = ANY($1 :: uuid [ ]) + GROUP BY + workspace_id +) m +JOIN + workspace_builds wb +ON m.workspace_id = wb.workspace_id AND m.max_build_number = wb.build_number +` + +func (q *sqlQuerier) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceBuild, error) { + rows, err := q.db.QueryContext(ctx, getLatestWorkspaceBuildsByWorkspaceIDs, pq.Array(ids)) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceBuild + for rows.Next() { + var i WorkspaceBuild + if err := rows.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.WorkspaceID, + &i.TemplateVersionID, + &i.Name, + &i.BuildNumber, + &i.Transition, + &i.InitiatorID, + &i.ProvisionerState, + &i.JobID, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getWorkspaceBuildByID = `-- name: GetWorkspaceBuildByID :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, name, before_id, after_id, transition, initiator_id, provisioner_state, job_id + id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id FROM workspace_builds WHERE @@ -2770,8 +2854,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByID(ctx context.Context, id uuid.UUID) (W &i.WorkspaceID, &i.TemplateVersionID, &i.Name, - &i.BeforeID, - &i.AfterID, + &i.BuildNumber, &i.Transition, &i.InitiatorID, &i.ProvisionerState, @@ -2782,7 +2865,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByID(ctx context.Context, id uuid.UUID) (W const getWorkspaceBuildByJobID = `-- name: GetWorkspaceBuildByJobID :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, name, before_id, after_id, transition, initiator_id, provisioner_state, job_id + id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id FROM workspace_builds WHERE @@ -2801,8 +2884,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByJobID(ctx context.Context, jobID uuid.UU &i.WorkspaceID, &i.TemplateVersionID, &i.Name, - &i.BeforeID, - &i.AfterID, + &i.BuildNumber, &i.Transition, &i.InitiatorID, &i.ProvisionerState, @@ -2813,15 +2895,51 @@ func (q *sqlQuerier) GetWorkspaceBuildByJobID(ctx context.Context, jobID uuid.UU const getWorkspaceBuildByWorkspaceID = `-- name: GetWorkspaceBuildByWorkspaceID :many SELECT - id, created_at, updated_at, workspace_id, template_version_id, name, before_id, after_id, transition, initiator_id, provisioner_state, job_id + id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id FROM workspace_builds WHERE - workspace_id = $1 + workspace_builds.workspace_id = $1 + AND CASE + -- This allows using the last element on a page as effectively a cursor. + -- This is an important option for scripts that need to paginate without + -- duplicating or missing data. + WHEN $2 :: uuid != '00000000-00000000-00000000-00000000' THEN ( + -- The pagination cursor is the last ID of the previous page. + -- The query is ordered by the build_number field, so select all + -- rows after the cursor. + build_number > ( + SELECT + build_number + FROM + workspace_builds + WHERE + id = $2 + ) + ) + ELSE true +END +ORDER BY + build_number desc OFFSET $3 +LIMIT + -- A null limit means "no limit", so -1 means return all + NULLIF($4 :: int, -1) ` -func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceID(ctx context.Context, workspaceID uuid.UUID) ([]WorkspaceBuild, error) { - rows, err := q.db.QueryContext(ctx, getWorkspaceBuildByWorkspaceID, workspaceID) +type GetWorkspaceBuildByWorkspaceIDParams struct { + WorkspaceID uuid.UUID `db:"workspace_id" json:"workspace_id"` + AfterID uuid.UUID `db:"after_id" json:"after_id"` + OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` + LimitOpt int32 `db:"limit_opt" json:"limit_opt"` +} + +func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceID(ctx context.Context, arg GetWorkspaceBuildByWorkspaceIDParams) ([]WorkspaceBuild, error) { + rows, err := q.db.QueryContext(ctx, getWorkspaceBuildByWorkspaceID, + arg.WorkspaceID, + arg.AfterID, + arg.OffsetOpt, + arg.LimitOpt, + ) if err != nil { return nil, err } @@ -2836,8 +2954,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceID(ctx context.Context, workspa &i.WorkspaceID, &i.TemplateVersionID, &i.Name, - &i.BeforeID, - &i.AfterID, + &i.BuildNumber, &i.Transition, &i.InitiatorID, &i.ProvisionerState, @@ -2858,7 +2975,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceID(ctx context.Context, workspa const getWorkspaceBuildByWorkspaceIDAndName = `-- name: GetWorkspaceBuildByWorkspaceIDAndName :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, name, before_id, after_id, transition, initiator_id, provisioner_state, job_id + id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id FROM workspace_builds WHERE @@ -2881,8 +2998,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceIDAndName(ctx context.Context, &i.WorkspaceID, &i.TemplateVersionID, &i.Name, - &i.BeforeID, - &i.AfterID, + &i.BuildNumber, &i.Transition, &i.InitiatorID, &i.ProvisionerState, @@ -2891,84 +3007,6 @@ func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceIDAndName(ctx context.Context, return i, err } -const getWorkspaceBuildByWorkspaceIDWithoutAfter = `-- name: GetWorkspaceBuildByWorkspaceIDWithoutAfter :one -SELECT - id, created_at, updated_at, workspace_id, template_version_id, name, before_id, after_id, transition, initiator_id, provisioner_state, job_id -FROM - workspace_builds -WHERE - workspace_id = $1 - AND after_id IS NULL -LIMIT - 1 -` - -func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceIDWithoutAfter(ctx context.Context, workspaceID uuid.UUID) (WorkspaceBuild, error) { - row := q.db.QueryRowContext(ctx, getWorkspaceBuildByWorkspaceIDWithoutAfter, workspaceID) - var i WorkspaceBuild - err := row.Scan( - &i.ID, - &i.CreatedAt, - &i.UpdatedAt, - &i.WorkspaceID, - &i.TemplateVersionID, - &i.Name, - &i.BeforeID, - &i.AfterID, - &i.Transition, - &i.InitiatorID, - &i.ProvisionerState, - &i.JobID, - ) - return i, err -} - -const getWorkspaceBuildsByWorkspaceIDsWithoutAfter = `-- name: GetWorkspaceBuildsByWorkspaceIDsWithoutAfter :many -SELECT - id, created_at, updated_at, workspace_id, template_version_id, name, before_id, after_id, transition, initiator_id, provisioner_state, job_id -FROM - workspace_builds -WHERE - workspace_id = ANY($1 :: uuid [ ]) - AND after_id IS NULL -` - -func (q *sqlQuerier) GetWorkspaceBuildsByWorkspaceIDsWithoutAfter(ctx context.Context, ids []uuid.UUID) ([]WorkspaceBuild, error) { - rows, err := q.db.QueryContext(ctx, getWorkspaceBuildsByWorkspaceIDsWithoutAfter, pq.Array(ids)) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceBuild - for rows.Next() { - var i WorkspaceBuild - if err := rows.Scan( - &i.ID, - &i.CreatedAt, - &i.UpdatedAt, - &i.WorkspaceID, - &i.TemplateVersionID, - &i.Name, - &i.BeforeID, - &i.AfterID, - &i.Transition, - &i.InitiatorID, - &i.ProvisionerState, - &i.JobID, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const insertWorkspaceBuild = `-- name: InsertWorkspaceBuild :one INSERT INTO workspace_builds ( @@ -2977,7 +3015,7 @@ INSERT INTO updated_at, workspace_id, template_version_id, - before_id, + "build_number", "name", transition, initiator_id, @@ -2985,7 +3023,7 @@ INSERT INTO provisioner_state ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) RETURNING id, created_at, updated_at, workspace_id, template_version_id, name, before_id, after_id, transition, initiator_id, provisioner_state, job_id + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) RETURNING id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id ` type InsertWorkspaceBuildParams struct { @@ -2994,7 +3032,7 @@ type InsertWorkspaceBuildParams struct { UpdatedAt time.Time `db:"updated_at" json:"updated_at"` WorkspaceID uuid.UUID `db:"workspace_id" json:"workspace_id"` TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` - BeforeID uuid.NullUUID `db:"before_id" json:"before_id"` + BuildNumber int32 `db:"build_number" json:"build_number"` Name string `db:"name" json:"name"` Transition WorkspaceTransition `db:"transition" json:"transition"` InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` @@ -3009,7 +3047,7 @@ func (q *sqlQuerier) InsertWorkspaceBuild(ctx context.Context, arg InsertWorkspa arg.UpdatedAt, arg.WorkspaceID, arg.TemplateVersionID, - arg.BeforeID, + arg.BuildNumber, arg.Name, arg.Transition, arg.InitiatorID, @@ -3024,8 +3062,7 @@ func (q *sqlQuerier) InsertWorkspaceBuild(ctx context.Context, arg InsertWorkspa &i.WorkspaceID, &i.TemplateVersionID, &i.Name, - &i.BeforeID, - &i.AfterID, + &i.BuildNumber, &i.Transition, &i.InitiatorID, &i.ProvisionerState, @@ -3039,26 +3076,19 @@ UPDATE workspace_builds SET updated_at = $2, - after_id = $3, - provisioner_state = $4 + provisioner_state = $3 WHERE id = $1 ` type UpdateWorkspaceBuildByIDParams struct { - ID uuid.UUID `db:"id" json:"id"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - AfterID uuid.NullUUID `db:"after_id" json:"after_id"` - ProvisionerState []byte `db:"provisioner_state" json:"provisioner_state"` + ID uuid.UUID `db:"id" json:"id"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + ProvisionerState []byte `db:"provisioner_state" json:"provisioner_state"` } func (q *sqlQuerier) UpdateWorkspaceBuildByID(ctx context.Context, arg UpdateWorkspaceBuildByIDParams) error { - _, err := q.db.ExecContext(ctx, updateWorkspaceBuildByID, - arg.ID, - arg.UpdatedAt, - arg.AfterID, - arg.ProvisionerState, - ) + _, err := q.db.ExecContext(ctx, updateWorkspaceBuildByID, arg.ID, arg.UpdatedAt, arg.ProvisionerState) return err } diff --git a/coderd/database/queries/workspacebuilds.sql b/coderd/database/queries/workspacebuilds.sql index 8b9220e725..733725131e 100644 --- a/coderd/database/queries/workspacebuilds.sql +++ b/coderd/database/queries/workspacebuilds.sql @@ -33,27 +33,60 @@ SELECT FROM workspace_builds WHERE - workspace_id = $1; + workspace_builds.workspace_id = $1 + AND CASE + -- This allows using the last element on a page as effectively a cursor. + -- This is an important option for scripts that need to paginate without + -- duplicating or missing data. + WHEN @after_id :: uuid != '00000000-00000000-00000000-00000000' THEN ( + -- The pagination cursor is the last ID of the previous page. + -- The query is ordered by the build_number field, so select all + -- rows after the cursor. + build_number > ( + SELECT + build_number + FROM + workspace_builds + WHERE + id = @after_id + ) + ) + ELSE true +END +ORDER BY + build_number desc OFFSET @offset_opt +LIMIT + -- A null limit means "no limit", so -1 means return all + NULLIF(@limit_opt :: int, -1); --- name: GetWorkspaceBuildByWorkspaceIDWithoutAfter :one +-- name: GetLatestWorkspaceBuildByWorkspaceID :one SELECT * FROM workspace_builds WHERE workspace_id = $1 - AND after_id IS NULL +ORDER BY + build_number desc LIMIT 1; --- name: GetWorkspaceBuildsByWorkspaceIDsWithoutAfter :many -SELECT - * -FROM - workspace_builds -WHERE - workspace_id = ANY(@ids :: uuid [ ]) - AND after_id IS NULL; +-- name: GetLatestWorkspaceBuildsByWorkspaceIDs :many +SELECT wb.* +FROM ( + SELECT + workspace_id, MAX(build_number) as max_build_number + FROM + workspace_builds + WHERE + workspace_id = ANY(@ids :: uuid [ ]) + GROUP BY + workspace_id +) m +JOIN + workspace_builds wb +ON m.workspace_id = wb.workspace_id AND m.max_build_number = wb.build_number; + -- name: InsertWorkspaceBuild :one INSERT INTO @@ -63,7 +96,7 @@ INSERT INTO updated_at, workspace_id, template_version_id, - before_id, + "build_number", "name", transition, initiator_id, @@ -78,7 +111,6 @@ UPDATE workspace_builds SET updated_at = $2, - after_id = $3, - provisioner_state = $4 + provisioner_state = $3 WHERE id = $1; diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index c7843b4a3d..ec26001b99 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -212,7 +212,7 @@ func (api *api) workspaceAgentListen(rw http.ResponseWriter, r *http.Request) { // Ensure the resource is still valid! // We only accept agents for resources on the latest build. ensureLatestBuild := func() error { - latestBuild, err := api.Database.GetWorkspaceBuildByWorkspaceIDWithoutAfter(r.Context(), build.WorkspaceID) + latestBuild, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), build.WorkspaceID) if err != nil { return err } diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index e9e908bc66..b3cf97462d 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -34,7 +34,17 @@ func (api *api) workspaceBuild(rw http.ResponseWriter, r *http.Request) { func (api *api) workspaceBuilds(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) - builds, err := api.Database.GetWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) + paginationParams, ok := parsePagination(rw, r) + if !ok { + return + } + req := database.GetWorkspaceBuildByWorkspaceIDParams{ + WorkspaceID: workspace.ID, + AfterID: paginationParams.AfterID, + OffsetOpt: int32(paginationParams.Offset), + LimitOpt: int32(paginationParams.Limit), + } + builds, err := api.Database.GetWorkspaceBuildByWorkspaceID(r.Context(), req) if xerrors.Is(err, sql.ErrNoRows) { err = nil } @@ -116,7 +126,7 @@ func (api *api) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { return } if createBuild.TemplateVersionID == uuid.Nil { - latestBuild, err := api.Database.GetWorkspaceBuildByWorkspaceIDWithoutAfter(r.Context(), workspace.ID) + latestBuild, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ Message: fmt.Sprintf("get latest workspace build: %s", err), @@ -176,9 +186,9 @@ func (api *api) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { return } - // Store prior history ID if it exists to update it after we create new! - priorHistoryID := uuid.NullUUID{} - priorHistory, err := api.Database.GetWorkspaceBuildByWorkspaceIDWithoutAfter(r.Context(), workspace.ID) + // Store prior build number to compute new build number + var priorBuildNum int32 + priorHistory, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) if err == nil { priorJob, err := api.Database.GetProvisionerJobByID(r.Context(), priorHistory.JobID) if err == nil && convertProvisionerJob(priorJob).Status.Active() { @@ -188,10 +198,7 @@ func (api *api) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { return } - priorHistoryID = uuid.NullUUID{ - UUID: priorHistory.ID, - Valid: true, - } + priorBuildNum = priorHistory.BuildNumber } else if !errors.Is(err, sql.ErrNoRows) { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ Message: fmt.Sprintf("get prior workspace build: %s", err), @@ -237,7 +244,7 @@ func (api *api) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { UpdatedAt: database.Now(), WorkspaceID: workspace.ID, TemplateVersionID: templateVersion.ID, - BeforeID: priorHistoryID, + BuildNumber: priorBuildNum + 1, Name: namesgenerator.GetRandomName(1), ProvisionerState: state, InitiatorID: apiKey.UserID, @@ -248,22 +255,6 @@ func (api *api) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { return xerrors.Errorf("insert workspace build: %w", err) } - if priorHistoryID.Valid { - // Update the prior history entries "after" column. - err = db.UpdateWorkspaceBuildByID(r.Context(), database.UpdateWorkspaceBuildByIDParams{ - ID: priorHistory.ID, - ProvisionerState: priorHistory.ProvisionerState, - UpdatedAt: database.Now(), - AfterID: uuid.NullUUID{ - UUID: workspaceBuild.ID, - Valid: true, - }, - }) - if err != nil { - return xerrors.Errorf("update prior workspace build: %w", err) - } - } - return nil }) if err != nil { @@ -355,8 +346,7 @@ func convertWorkspaceBuild(workspaceBuild database.WorkspaceBuild, job codersdk. UpdatedAt: workspaceBuild.UpdatedAt, WorkspaceID: workspaceBuild.WorkspaceID, TemplateVersionID: workspaceBuild.TemplateVersionID, - BeforeID: workspaceBuild.BeforeID.UUID, - AfterID: workspaceBuild.AfterID.UUID, + BuildNumber: workspaceBuild.BuildNumber, Name: workspaceBuild.Name, Transition: workspaceBuild.Transition, InitiatorID: workspaceBuild.InitiatorID, diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index a9cec2cf33..e50c2281f7 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner/echo" "github.com/coder/coder/provisionersdk/proto" @@ -38,9 +39,50 @@ func TestWorkspaceBuilds(t *testing.T) { template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - _, err := client.WorkspaceBuilds(context.Background(), workspace.ID) + builds, err := client.WorkspaceBuilds(context.Background(), + codersdk.WorkspaceBuildsRequest{WorkspaceID: workspace.ID}) + require.Len(t, builds, 1) + require.Equal(t, int32(1), builds[0].BuildNumber) require.NoError(t, err) }) + + t.Run("PaginateLimitOffset", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + coderdtest.NewProvisionerDaemon(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + var expectedBuilds []codersdk.WorkspaceBuild + extraBuilds := 4 + for i := 0; i < extraBuilds; i++ { + b := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStart) + expectedBuilds = append(expectedBuilds, b) + coderdtest.AwaitWorkspaceBuildJob(t, client, b.ID) + } + + pageSize := 3 + firstPage, err := client.WorkspaceBuilds(context.Background(), codersdk.WorkspaceBuildsRequest{ + WorkspaceID: workspace.ID, + Pagination: codersdk.Pagination{Limit: pageSize, Offset: 0}, + }) + require.NoError(t, err) + require.Len(t, firstPage, pageSize) + for i := 0; i < pageSize; i++ { + require.Equal(t, expectedBuilds[extraBuilds-i-1].ID, firstPage[i].ID) + } + secondPage, err := client.WorkspaceBuilds(context.Background(), codersdk.WorkspaceBuildsRequest{ + WorkspaceID: workspace.ID, + Pagination: codersdk.Pagination{Limit: pageSize, Offset: pageSize}, + }) + require.NoError(t, err) + require.Len(t, secondPage, 2) + require.Equal(t, expectedBuilds[0].ID, secondPage[0].ID) + require.Equal(t, workspace.LatestBuild.ID, secondPage[1].ID) // build created while creating workspace + }) } func TestPatchCancelWorkspaceBuild(t *testing.T) { diff --git a/coderd/workspaceresourceauth.go b/coderd/workspaceresourceauth.go index a2784b9553..17a713b651 100644 --- a/coderd/workspaceresourceauth.go +++ b/coderd/workspaceresourceauth.go @@ -137,7 +137,7 @@ func (api *api) handleAuthInstanceID(rw http.ResponseWriter, r *http.Request, in // This token should only be exchanged if the instance ID is valid // for the latest history. If an instance ID is recycled by a cloud, // we'd hate to leak access to a user's workspace. - latestHistory, err := api.Database.GetWorkspaceBuildByWorkspaceIDWithoutAfter(r.Context(), resourceHistory.WorkspaceID) + latestHistory, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), resourceHistory.WorkspaceID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ Message: fmt.Sprintf("get latest workspace build: %s", err), diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 37b8c7d054..4ef3b61301 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -25,7 +25,7 @@ import ( func (api *api) workspace(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) - build, err := api.Database.GetWorkspaceBuildByWorkspaceIDWithoutAfter(r.Context(), workspace.ID) + build, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ Message: fmt.Sprintf("get workspace build: %s", err), @@ -244,7 +244,7 @@ func (api *api) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request) return } - build, err := api.Database.GetWorkspaceBuildByWorkspaceIDWithoutAfter(r.Context(), workspace.ID) + build, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ Message: fmt.Sprintf("get workspace build: %s", err), @@ -446,6 +446,7 @@ func (api *api) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req InitiatorID: apiKey.UserID, Transition: database.WorkspaceTransitionStart, JobID: provisionerJob.ID, + BuildNumber: 1, // First build! }) if err != nil { return xerrors.Errorf("insert workspace build: %w", err) @@ -543,7 +544,7 @@ func convertWorkspaces(ctx context.Context, db database.Store, workspaces []data templateIDs = append(templateIDs, workspace.TemplateID) ownerIDs = append(ownerIDs, workspace.OwnerID) } - workspaceBuilds, err := db.GetWorkspaceBuildsByWorkspaceIDsWithoutAfter(ctx, workspaceIDs) + workspaceBuilds, err := db.GetLatestWorkspaceBuildsByWorkspaceIDs(ctx, workspaceIDs) if errors.Is(err, sql.ErrNoRows) { err = nil } @@ -575,7 +576,19 @@ func convertWorkspaces(ctx context.Context, db database.Store, workspaces []data buildByWorkspaceID := map[uuid.UUID]database.WorkspaceBuild{} for _, workspaceBuild := range workspaceBuilds { - buildByWorkspaceID[workspaceBuild.WorkspaceID] = workspaceBuild + buildByWorkspaceID[workspaceBuild.WorkspaceID] = database.WorkspaceBuild{ + ID: workspaceBuild.ID, + CreatedAt: workspaceBuild.CreatedAt, + UpdatedAt: workspaceBuild.UpdatedAt, + WorkspaceID: workspaceBuild.WorkspaceID, + TemplateVersionID: workspaceBuild.TemplateVersionID, + Name: workspaceBuild.Name, + BuildNumber: workspaceBuild.BuildNumber, + Transition: workspaceBuild.Transition, + InitiatorID: workspaceBuild.InitiatorID, + ProvisionerState: workspaceBuild.ProvisionerState, + JobID: workspaceBuild.JobID, + } } templateByID := map[uuid.UUID]database.Template{} for _, template := range templates { diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 724be6106d..58140b1d00 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -248,7 +248,7 @@ func TestPostWorkspaceBuild(t *testing.T) { require.Equal(t, http.StatusConflict, apiErr.StatusCode()) }) - t.Run("UpdatePriorAfterField", func(t *testing.T) { + t.Run("IncrementBuildNumber", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) user := coderdtest.CreateFirstUser(t, client) @@ -263,11 +263,7 @@ func TestPostWorkspaceBuild(t *testing.T) { Transition: database.WorkspaceTransitionStart, }) require.NoError(t, err) - require.Equal(t, workspace.LatestBuild.ID.String(), build.BeforeID.String()) - - firstBuild, err := client.WorkspaceBuild(context.Background(), workspace.LatestBuild.ID) - require.NoError(t, err) - require.Equal(t, build.ID.String(), firstBuild.AfterID.String()) + require.Equal(t, workspace.LatestBuild.BuildNumber+1, build.BuildNumber) }) t.Run("WithState", func(t *testing.T) { @@ -307,7 +303,7 @@ func TestPostWorkspaceBuild(t *testing.T) { Transition: database.WorkspaceTransitionDelete, }) require.NoError(t, err) - require.Equal(t, workspace.LatestBuild.ID.String(), build.BeforeID.String()) + require.Equal(t, workspace.LatestBuild.BuildNumber+1, build.BuildNumber) coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID) workspaces, err := client.WorkspacesByOwner(context.Background(), user.OrganizationID, user.UserID.String()) diff --git a/codersdk/workspacebuilds.go b/codersdk/workspacebuilds.go index 3c1ee5154b..83e2a4b535 100644 --- a/codersdk/workspacebuilds.go +++ b/codersdk/workspacebuilds.go @@ -14,15 +14,14 @@ import ( ) // WorkspaceBuild is an at-point representation of a workspace state. -// Iterate on before/after to determine a chronological history. +// BuildNumbers start at 1 and increase by 1 for each subsequent build type WorkspaceBuild struct { ID uuid.UUID `json:"id"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` WorkspaceID uuid.UUID `json:"workspace_id"` TemplateVersionID uuid.UUID `json:"template_version_id"` - BeforeID uuid.UUID `json:"before_id"` - AfterID uuid.UUID `json:"after_id"` + BuildNumber int32 `json:"build_number"` Name string `json:"name"` Transition database.WorkspaceTransition `json:"transition"` InitiatorID uuid.UUID `json:"initiator_id"` diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index a7bcff37b4..6e4ab7afd6 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -52,8 +52,14 @@ func (c *Client) Workspace(ctx context.Context, id uuid.UUID) (Workspace, error) return workspace, json.NewDecoder(res.Body).Decode(&workspace) } -func (c *Client) WorkspaceBuilds(ctx context.Context, workspace uuid.UUID) ([]WorkspaceBuild, error) { - res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/workspaces/%s/builds", workspace), nil) +type WorkspaceBuildsRequest struct { + WorkspaceID uuid.UUID + Pagination +} + +func (c *Client) WorkspaceBuilds(ctx context.Context, req WorkspaceBuildsRequest) ([]WorkspaceBuild, error) { + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/workspaces/%s/builds", req.WorkspaceID), + nil, req.Pagination.asRequestOption()) if err != nil { return nil, err } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index e45461e50f..792ef39613 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -292,12 +292,12 @@ export interface UpdateUserProfileRequest { readonly username: string } -// From codersdk/workspaces.go:96:6 +// From codersdk/workspaces.go:102:6 export interface UpdateWorkspaceAutostartRequest { readonly schedule: string } -// From codersdk/workspaces.go:116:6 +// From codersdk/workspaces.go:122:6 export interface UpdateWorkspaceAutostopRequest { readonly schedule: string } @@ -421,8 +421,7 @@ export interface WorkspaceBuild { readonly updated_at: string readonly workspace_id: string readonly template_version_id: string - readonly before_id: string - readonly after_id: string + readonly build_number: number readonly name: string // This is likely an enum in an external package ("github.com/coder/coder/coderd/database.WorkspaceTransition") readonly transition: string @@ -430,10 +429,15 @@ export interface WorkspaceBuild { readonly job: ProvisionerJob } -// From codersdk/workspaces.go:135:6 +// From codersdk/workspaces.go:55:6 +export interface WorkspaceBuildsRequest extends Pagination { + readonly WorkspaceID: string +} + +// From codersdk/workspaces.go:141:6 export interface WorkspaceFilter { readonly OrganizationID: string - readonly OwnerID: string + readonly Owner: string } // From codersdk/workspaceresources.go:23:6 diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 777c9cdbb7..4f42e854c2 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -110,8 +110,7 @@ export const MockWorkspaceAutostopEnabled: TypesGen.UpdateWorkspaceAutostartRequ } export const MockWorkspaceBuild: TypesGen.WorkspaceBuild = { - after_id: "", - before_id: "", + build_number: 1, created_at: new Date().toString(), id: "1", initiator_id: "",