diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 16bc364a42..4397ddfee3 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1512,6 +1512,13 @@ func (q *querier) CountInProgressPrebuilds(ctx context.Context) ([]database.Coun return q.db.CountInProgressPrebuilds(ctx) } +func (q *querier) CountPendingNonActivePrebuilds(ctx context.Context) ([]database.CountPendingNonActivePrebuildsRow, error) { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceWorkspace.All()); err != nil { + return nil, err + } + return q.db.CountPendingNonActivePrebuilds(ctx) +} + func (q *querier) CountUnreadInboxNotificationsByUserID(ctx context.Context, userID uuid.UUID) (int64, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceInboxNotification.WithOwner(userID.String())); err != nil { return 0, err @@ -4875,6 +4882,14 @@ func (q *querier) UpdateOrganizationDeletedByID(ctx context.Context, arg databas return deleteQ(q.log, q.auth, q.db.GetOrganizationByID, deleteF)(ctx, arg.ID) } +func (q *querier) UpdatePrebuildProvisionerJobWithCancel(ctx context.Context, arg database.UpdatePrebuildProvisionerJobWithCancelParams) ([]uuid.UUID, error) { + // Prebuild operation for canceling pending prebuild jobs from non-active template versions + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourcePrebuiltWorkspace); err != nil { + return []uuid.UUID{}, err + } + return q.db.UpdatePrebuildProvisionerJobWithCancel(ctx, arg) +} + func (q *querier) UpdatePresetPrebuildStatus(ctx context.Context, arg database.UpdatePresetPrebuildStatusParams) error { preset, err := q.db.GetPresetByID(ctx, arg.PresetID) if err != nil { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index ca82390984..465bd67905 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -641,6 +641,16 @@ func (s *MethodTestSuite) TestProvisionerJob() { dbm.EXPECT().UpdateProvisionerJobWithCancelByID(gomock.Any(), arg).Return(nil).AnyTimes() check.Args(arg).Asserts(v.RBACObject(tpl), []policy.Action{policy.ActionRead, policy.ActionUpdate}).Returns() })) + s.Run("UpdatePrebuildProvisionerJobWithCancel", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + arg := database.UpdatePrebuildProvisionerJobWithCancelParams{ + PresetID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, + Now: dbtime.Now(), + } + jobIDs := []uuid.UUID{uuid.New(), uuid.New()} + + dbm.EXPECT().UpdatePrebuildProvisionerJobWithCancel(gomock.Any(), arg).Return(jobIDs, nil).AnyTimes() + check.Args(arg).Asserts(rbac.ResourcePrebuiltWorkspace, policy.ActionUpdate).Returns(jobIDs) + })) s.Run("GetProvisionerJobsByIDs", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { org := testutil.Fake(s.T(), faker, database.Organization{}) org2 := testutil.Fake(s.T(), faker, database.Organization{}) @@ -3758,6 +3768,10 @@ func (s *MethodTestSuite) TestPrebuilds() { dbm.EXPECT().CountInProgressPrebuilds(gomock.Any()).Return([]database.CountInProgressPrebuildsRow{}, nil).AnyTimes() check.Args().Asserts(rbac.ResourceWorkspace.All(), policy.ActionRead) })) + s.Run("CountPendingNonActivePrebuilds", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { + dbm.EXPECT().CountPendingNonActivePrebuilds(gomock.Any()).Return([]database.CountPendingNonActivePrebuildsRow{}, nil).AnyTimes() + check.Args().Asserts(rbac.ResourceWorkspace.All(), policy.ActionRead) + })) s.Run("GetPresetsAtFailureLimit", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { dbm.EXPECT().GetPresetsAtFailureLimit(gomock.Any(), int64(0)).Return([]database.GetPresetsAtFailureLimitRow{}, nil).AnyTimes() check.Args(int64(0)).Asserts(rbac.ResourceTemplate.All(), policy.ActionViewInsights) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 9eaf1afb93..cf94812cfd 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -55,14 +55,10 @@ type WorkspaceBuildBuilder struct { resources []*sdkproto.Resource params []database.WorkspaceBuildParameter agentToken string - dispo workspaceBuildDisposition + jobStatus database.ProvisionerJobStatus taskAppID uuid.UUID } -type workspaceBuildDisposition struct { - starting bool -} - // WorkspaceBuild generates a workspace build for the provided workspace. // Pass a database.Workspace{} with a nil ID to also generate a new workspace. // Omitting the template ID on a workspace will also generate a new template @@ -145,8 +141,17 @@ func (b WorkspaceBuildBuilder) WithTask(seed *sdkproto.App) WorkspaceBuildBuilde } func (b WorkspaceBuildBuilder) Starting() WorkspaceBuildBuilder { - //nolint: revive // returns modified struct - b.dispo.starting = true + b.jobStatus = database.ProvisionerJobStatusRunning + return b +} + +func (b WorkspaceBuildBuilder) Pending() WorkspaceBuildBuilder { + b.jobStatus = database.ProvisionerJobStatusPending + return b +} + +func (b WorkspaceBuildBuilder) Canceled() WorkspaceBuildBuilder { + b.jobStatus = database.ProvisionerJobStatusCanceled return b } @@ -231,7 +236,11 @@ func (b WorkspaceBuildBuilder) Do() WorkspaceResponse { require.NoError(b.t, err, "insert job") b.logger.Debug(context.Background(), "inserted provisioner job", slog.F("job_id", job.ID)) - if b.dispo.starting { + switch b.jobStatus { + case database.ProvisionerJobStatusPending: + // Provisioner jobs are created in 'pending' status + b.logger.Debug(context.Background(), "pending the provisioner job") + case database.ProvisionerJobStatusRunning: // might need to do this multiple times if we got a template version // import job as well b.logger.Debug(context.Background(), "looping to acquire provisioner job") @@ -255,7 +264,23 @@ func (b WorkspaceBuildBuilder) Do() WorkspaceResponse { break } } - } else { + case database.ProvisionerJobStatusCanceled: + // Set provisioner job status to 'canceled' + b.logger.Debug(context.Background(), "canceling the provisioner job") + err = b.db.UpdateProvisionerJobWithCancelByID(ownerCtx, database.UpdateProvisionerJobWithCancelByIDParams{ + ID: jobID, + CanceledAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + CompletedAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + }) + require.NoError(b.t, err, "cancel job") + default: + // By default, consider jobs in 'succeeded' status b.logger.Debug(context.Background(), "completing the provisioner job") err = b.db.UpdateProvisionerJobWithCompleteByID(ownerCtx, database.UpdateProvisionerJobWithCompleteByIDParams{ ID: job.ID, @@ -571,6 +596,12 @@ func (t TemplateVersionBuilder) Do() TemplateVersionResponse { t.params[i] = dbgen.TemplateVersionParameter(t.t, t.db, param) } + // Update response with template and version + if resp.Template.ID == uuid.Nil && version.TemplateID.Valid { + template, err := t.db.GetTemplateByID(ownerCtx, version.TemplateID.UUID) + require.NoError(t.t, err) + resp.Template = template + } resp.TemplateVersion = version return resp } diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 79c8a01d67..9f0589178f 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -214,6 +214,13 @@ func (m queryMetricsStore) CountInProgressPrebuilds(ctx context.Context) ([]data return r0, r1 } +func (m queryMetricsStore) CountPendingNonActivePrebuilds(ctx context.Context) ([]database.CountPendingNonActivePrebuildsRow, error) { + start := time.Now() + r0, r1 := m.s.CountPendingNonActivePrebuilds(ctx) + m.queryLatencies.WithLabelValues("CountPendingNonActivePrebuilds").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) CountUnreadInboxNotificationsByUserID(ctx context.Context, userID uuid.UUID) (int64, error) { start := time.Now() r0, r1 := m.s.CountUnreadInboxNotificationsByUserID(ctx, userID) @@ -3000,6 +3007,13 @@ func (m queryMetricsStore) UpdateOrganizationDeletedByID(ctx context.Context, ar return r0 } +func (m queryMetricsStore) UpdatePrebuildProvisionerJobWithCancel(ctx context.Context, arg database.UpdatePrebuildProvisionerJobWithCancelParams) ([]uuid.UUID, error) { + start := time.Now() + r0, r1 := m.s.UpdatePrebuildProvisionerJobWithCancel(ctx, arg) + m.queryLatencies.WithLabelValues("UpdatePrebuildProvisionerJobWithCancel").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) UpdatePresetPrebuildStatus(ctx context.Context, arg database.UpdatePresetPrebuildStatusParams) error { start := time.Now() r0 := m.s.UpdatePresetPrebuildStatus(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index a16ba47e72..237fc3d0eb 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -352,6 +352,21 @@ func (mr *MockStoreMockRecorder) CountInProgressPrebuilds(ctx any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CountInProgressPrebuilds", reflect.TypeOf((*MockStore)(nil).CountInProgressPrebuilds), ctx) } +// CountPendingNonActivePrebuilds mocks base method. +func (m *MockStore) CountPendingNonActivePrebuilds(ctx context.Context) ([]database.CountPendingNonActivePrebuildsRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CountPendingNonActivePrebuilds", ctx) + ret0, _ := ret[0].([]database.CountPendingNonActivePrebuildsRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CountPendingNonActivePrebuilds indicates an expected call of CountPendingNonActivePrebuilds. +func (mr *MockStoreMockRecorder) CountPendingNonActivePrebuilds(ctx any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CountPendingNonActivePrebuilds", reflect.TypeOf((*MockStore)(nil).CountPendingNonActivePrebuilds), ctx) +} + // CountUnreadInboxNotificationsByUserID mocks base method. func (m *MockStore) CountUnreadInboxNotificationsByUserID(ctx context.Context, userID uuid.UUID) (int64, error) { m.ctrl.T.Helper() @@ -6451,6 +6466,21 @@ func (mr *MockStoreMockRecorder) UpdateOrganizationDeletedByID(ctx, arg any) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateOrganizationDeletedByID", reflect.TypeOf((*MockStore)(nil).UpdateOrganizationDeletedByID), ctx, arg) } +// UpdatePrebuildProvisionerJobWithCancel mocks base method. +func (m *MockStore) UpdatePrebuildProvisionerJobWithCancel(ctx context.Context, arg database.UpdatePrebuildProvisionerJobWithCancelParams) ([]uuid.UUID, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdatePrebuildProvisionerJobWithCancel", ctx, arg) + ret0, _ := ret[0].([]uuid.UUID) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdatePrebuildProvisionerJobWithCancel indicates an expected call of UpdatePrebuildProvisionerJobWithCancel. +func (mr *MockStoreMockRecorder) UpdatePrebuildProvisionerJobWithCancel(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePrebuildProvisionerJobWithCancel", reflect.TypeOf((*MockStore)(nil).UpdatePrebuildProvisionerJobWithCancel), ctx, arg) +} + // UpdatePresetPrebuildStatus mocks base method. func (m *MockStore) UpdatePresetPrebuildStatus(ctx context.Context, arg database.UpdatePresetPrebuildStatusParams) error { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index c6ae87dbc5..5744796abc 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -68,8 +68,10 @@ type sqlcQuerier interface { CountAuditLogs(ctx context.Context, arg CountAuditLogsParams) (int64, error) CountConnectionLogs(ctx context.Context, arg CountConnectionLogsParams) (int64, error) // CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped by preset ID and transition. - // Prebuild considered in-progress if it's in the "starting", "stopping", or "deleting" state. + // Prebuild considered in-progress if it's in the "pending", "starting", "stopping", or "deleting" state. CountInProgressPrebuilds(ctx context.Context) ([]CountInProgressPrebuildsRow, error) + // CountPendingNonActivePrebuilds returns the number of pending prebuilds for non-active template versions + CountPendingNonActivePrebuilds(ctx context.Context) ([]CountPendingNonActivePrebuildsRow, error) CountUnreadInboxNotificationsByUserID(ctx context.Context, userID uuid.UUID) (int64, error) CreateUserSecret(ctx context.Context, arg CreateUserSecretParams) (UserSecret, error) CustomRoles(ctx context.Context, arg CustomRolesParams) ([]CustomRole, error) @@ -647,6 +649,10 @@ type sqlcQuerier interface { UpdateOAuth2ProviderAppSecretByID(ctx context.Context, arg UpdateOAuth2ProviderAppSecretByIDParams) (OAuth2ProviderAppSecret, error) UpdateOrganization(ctx context.Context, arg UpdateOrganizationParams) (Organization, error) UpdateOrganizationDeletedByID(ctx context.Context, arg UpdateOrganizationDeletedByIDParams) error + // Cancels all pending provisioner jobs for prebuilt workspaces on a specific preset from an + // inactive template version. + // This is an optimization to clean up stale pending jobs. + UpdatePrebuildProvisionerJobWithCancel(ctx context.Context, arg UpdatePrebuildProvisionerJobWithCancelParams) ([]uuid.UUID, error) UpdatePresetPrebuildStatus(ctx context.Context, arg UpdatePresetPrebuildStatusParams) error UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg UpdateProvisionerDaemonLastSeenAtParams) error UpdateProvisionerJobByID(ctx context.Context, arg UpdateProvisionerJobByIDParams) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index c5b8caba37..94fdc03d32 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -7924,7 +7924,7 @@ type CountInProgressPrebuildsRow struct { } // CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped by preset ID and transition. -// Prebuild considered in-progress if it's in the "starting", "stopping", or "deleting" state. +// Prebuild considered in-progress if it's in the "pending", "starting", "stopping", or "deleting" state. func (q *sqlQuerier) CountInProgressPrebuilds(ctx context.Context) ([]CountInProgressPrebuildsRow, error) { rows, err := q.db.QueryContext(ctx, countInProgressPrebuilds) if err != nil { @@ -7954,6 +7954,58 @@ func (q *sqlQuerier) CountInProgressPrebuilds(ctx context.Context) ([]CountInPro return items, nil } +const countPendingNonActivePrebuilds = `-- name: CountPendingNonActivePrebuilds :many +SELECT + wpb.template_version_preset_id AS preset_id, + COUNT(*)::int AS count +FROM workspace_prebuild_builds wpb +INNER JOIN provisioner_jobs pj ON pj.id = wpb.job_id +INNER JOIN workspaces w ON w.id = wpb.workspace_id +INNER JOIN templates t ON t.id = w.template_id +WHERE + wpb.template_version_id != t.active_version_id + -- Only considers initial builds, i.e. created by the reconciliation loop + AND wpb.build_number = 1 + -- Only consider 'start' transitions (provisioning), not 'stop'/'delete' (deprovisioning) + -- Deprovisioning jobs should complete naturally as they're already cleaning up resources + AND wpb.transition = 'start'::workspace_transition + -- Pending jobs that have not yet been picked up by a provisioner + AND pj.job_status = 'pending'::provisioner_job_status + AND pj.worker_id IS NULL + AND pj.canceled_at IS NULL + AND pj.completed_at IS NULL +GROUP BY wpb.template_version_preset_id +` + +type CountPendingNonActivePrebuildsRow struct { + PresetID uuid.NullUUID `db:"preset_id" json:"preset_id"` + Count int32 `db:"count" json:"count"` +} + +// CountPendingNonActivePrebuilds returns the number of pending prebuilds for non-active template versions +func (q *sqlQuerier) CountPendingNonActivePrebuilds(ctx context.Context) ([]CountPendingNonActivePrebuildsRow, error) { + rows, err := q.db.QueryContext(ctx, countPendingNonActivePrebuilds) + if err != nil { + return nil, err + } + defer rows.Close() + var items []CountPendingNonActivePrebuildsRow + for rows.Next() { + var i CountPendingNonActivePrebuildsRow + if err := rows.Scan(&i.PresetID, &i.Count); 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 findMatchingPresetID = `-- name: FindMatchingPresetID :one WITH provided_params AS ( SELECT @@ -8396,6 +8448,65 @@ func (q *sqlQuerier) GetTemplatePresetsWithPrebuilds(ctx context.Context, templa return items, nil } +const updatePrebuildProvisionerJobWithCancel = `-- name: UpdatePrebuildProvisionerJobWithCancel :many +UPDATE provisioner_jobs +SET + canceled_at = $1::timestamptz, + completed_at = $1::timestamptz +WHERE id IN ( + SELECT pj.id + FROM provisioner_jobs pj + INNER JOIN workspace_prebuild_builds wpb ON wpb.job_id = pj.id + INNER JOIN workspaces w ON w.id = wpb.workspace_id + INNER JOIN templates t ON t.id = w.template_id + WHERE + wpb.template_version_id != t.active_version_id + AND wpb.template_version_preset_id = $2 + -- Only considers initial builds, i.e. created by the reconciliation loop + AND wpb.build_number = 1 + -- Only consider 'start' transitions (provisioning), not 'stop'/'delete' (deprovisioning) + -- Deprovisioning jobs should complete naturally as they're already cleaning up resources + AND wpb.transition = 'start'::workspace_transition + -- Pending jobs that have not yet been picked up by a provisioner + AND pj.job_status = 'pending'::provisioner_job_status + AND pj.worker_id IS NULL + AND pj.canceled_at IS NULL + AND pj.completed_at IS NULL +) +RETURNING id +` + +type UpdatePrebuildProvisionerJobWithCancelParams struct { + Now time.Time `db:"now" json:"now"` + PresetID uuid.NullUUID `db:"preset_id" json:"preset_id"` +} + +// Cancels all pending provisioner jobs for prebuilt workspaces on a specific preset from an +// inactive template version. +// This is an optimization to clean up stale pending jobs. +func (q *sqlQuerier) UpdatePrebuildProvisionerJobWithCancel(ctx context.Context, arg UpdatePrebuildProvisionerJobWithCancelParams) ([]uuid.UUID, error) { + rows, err := q.db.QueryContext(ctx, updatePrebuildProvisionerJobWithCancel, arg.Now, arg.PresetID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []uuid.UUID + for rows.Next() { + var id uuid.UUID + if err := rows.Scan(&id); err != nil { + return nil, err + } + items = append(items, id) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getActivePresetPrebuildSchedules = `-- name: GetActivePresetPrebuildSchedules :many SELECT tvpps.id, tvpps.preset_id, tvpps.cron_expression, tvpps.desired_instances diff --git a/coderd/database/queries/prebuilds.sql b/coderd/database/queries/prebuilds.sql index 2ad7f41d41..6c5520c9da 100644 --- a/coderd/database/queries/prebuilds.sql +++ b/coderd/database/queries/prebuilds.sql @@ -121,7 +121,7 @@ ORDER BY latest_prebuilds.id; -- name: CountInProgressPrebuilds :many -- CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped by preset ID and transition. --- Prebuild considered in-progress if it's in the "starting", "stopping", or "deleting" state. +-- Prebuild considered in-progress if it's in the "pending", "starting", "stopping", or "deleting" state. SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition)::int AS count, wlb.template_version_preset_id as preset_id FROM workspace_latest_builds wlb INNER JOIN workspace_prebuild_builds wpb ON wpb.id = wlb.id @@ -272,3 +272,56 @@ FROM preset_matches pm WHERE pm.total_preset_params = pm.matching_params -- All preset parameters must match ORDER BY pm.total_preset_params DESC -- Return the preset with the most parameters LIMIT 1; + +-- name: CountPendingNonActivePrebuilds :many +-- CountPendingNonActivePrebuilds returns the number of pending prebuilds for non-active template versions +SELECT + wpb.template_version_preset_id AS preset_id, + COUNT(*)::int AS count +FROM workspace_prebuild_builds wpb +INNER JOIN provisioner_jobs pj ON pj.id = wpb.job_id +INNER JOIN workspaces w ON w.id = wpb.workspace_id +INNER JOIN templates t ON t.id = w.template_id +WHERE + wpb.template_version_id != t.active_version_id + -- Only considers initial builds, i.e. created by the reconciliation loop + AND wpb.build_number = 1 + -- Only consider 'start' transitions (provisioning), not 'stop'/'delete' (deprovisioning) + -- Deprovisioning jobs should complete naturally as they're already cleaning up resources + AND wpb.transition = 'start'::workspace_transition + -- Pending jobs that have not yet been picked up by a provisioner + AND pj.job_status = 'pending'::provisioner_job_status + AND pj.worker_id IS NULL + AND pj.canceled_at IS NULL + AND pj.completed_at IS NULL +GROUP BY wpb.template_version_preset_id; + +-- name: UpdatePrebuildProvisionerJobWithCancel :many +-- Cancels all pending provisioner jobs for prebuilt workspaces on a specific preset from an +-- inactive template version. +-- This is an optimization to clean up stale pending jobs. +UPDATE provisioner_jobs +SET + canceled_at = @now::timestamptz, + completed_at = @now::timestamptz +WHERE id IN ( + SELECT pj.id + FROM provisioner_jobs pj + INNER JOIN workspace_prebuild_builds wpb ON wpb.job_id = pj.id + INNER JOIN workspaces w ON w.id = wpb.workspace_id + INNER JOIN templates t ON t.id = w.template_id + WHERE + wpb.template_version_id != t.active_version_id + AND wpb.template_version_preset_id = @preset_id + -- Only considers initial builds, i.e. created by the reconciliation loop + AND wpb.build_number = 1 + -- Only consider 'start' transitions (provisioning), not 'stop'/'delete' (deprovisioning) + -- Deprovisioning jobs should complete naturally as they're already cleaning up resources + AND wpb.transition = 'start'::workspace_transition + -- Pending jobs that have not yet been picked up by a provisioner + AND pj.job_status = 'pending'::provisioner_job_status + AND pj.worker_id IS NULL + AND pj.canceled_at IS NULL + AND pj.completed_at IS NULL +) +RETURNING id; diff --git a/coderd/prebuilds/global_snapshot.go b/coderd/prebuilds/global_snapshot.go index f8fb873739..3c7ec24f56 100644 --- a/coderd/prebuilds/global_snapshot.go +++ b/coderd/prebuilds/global_snapshot.go @@ -8,10 +8,9 @@ import ( "cdr.dev/slog" - "github.com/coder/quartz" - "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/util/slice" + "github.com/coder/quartz" ) // GlobalSnapshot represents a full point-in-time snapshot of state relating to prebuilds across all templates. @@ -20,6 +19,7 @@ type GlobalSnapshot struct { PrebuildSchedules []database.TemplateVersionPresetPrebuildSchedule RunningPrebuilds []database.GetRunningPrebuiltWorkspacesRow PrebuildsInProgress []database.CountInProgressPrebuildsRow + PendingPrebuilds []database.CountPendingNonActivePrebuildsRow Backoffs []database.GetPresetsBackoffRow HardLimitedPresetsMap map[uuid.UUID]database.GetPresetsAtFailureLimitRow clock quartz.Clock @@ -31,6 +31,7 @@ func NewGlobalSnapshot( prebuildSchedules []database.TemplateVersionPresetPrebuildSchedule, runningPrebuilds []database.GetRunningPrebuiltWorkspacesRow, prebuildsInProgress []database.CountInProgressPrebuildsRow, + pendingPrebuilds []database.CountPendingNonActivePrebuildsRow, backoffs []database.GetPresetsBackoffRow, hardLimitedPresets []database.GetPresetsAtFailureLimitRow, clock quartz.Clock, @@ -46,6 +47,7 @@ func NewGlobalSnapshot( PrebuildSchedules: prebuildSchedules, RunningPrebuilds: runningPrebuilds, PrebuildsInProgress: prebuildsInProgress, + PendingPrebuilds: pendingPrebuilds, Backoffs: backoffs, HardLimitedPresetsMap: hardLimitedPresetsMap, clock: clock, @@ -76,10 +78,20 @@ func (s GlobalSnapshot) FilterByPreset(presetID uuid.UUID) (*PresetSnapshot, err // Separate running workspaces into non-expired and expired based on the preset's TTL nonExpired, expired := filterExpiredWorkspaces(preset, running) + // Includes in-progress prebuilds only for active template versions. + // In-progress prebuilds correspond to workspace statuses: 'pending', 'starting', 'stopping', and 'deleting' inProgress := slice.Filter(s.PrebuildsInProgress, func(prebuild database.CountInProgressPrebuildsRow) bool { return prebuild.PresetID.UUID == preset.ID }) + // Includes count of pending prebuilds only for non-active template versions + pendingCount := 0 + if found, ok := slice.Find(s.PendingPrebuilds, func(prebuild database.CountPendingNonActivePrebuildsRow) bool { + return prebuild.PresetID.UUID == preset.ID + }); ok { + pendingCount = int(found.Count) + } + var backoffPtr *database.GetPresetsBackoffRow backoff, found := slice.Find(s.Backoffs, func(row database.GetPresetsBackoffRow) bool { return row.PresetID == preset.ID @@ -96,6 +108,7 @@ func (s GlobalSnapshot) FilterByPreset(presetID uuid.UUID) (*PresetSnapshot, err nonExpired, expired, inProgress, + pendingCount, backoffPtr, isHardLimited, s.clock, diff --git a/coderd/prebuilds/preset_snapshot.go b/coderd/prebuilds/preset_snapshot.go index be9299c8f5..04f4cd1a83 100644 --- a/coderd/prebuilds/preset_snapshot.go +++ b/coderd/prebuilds/preset_snapshot.go @@ -34,6 +34,9 @@ const ( // ActionTypeBackoff indicates that prebuild creation should be delayed. ActionTypeBackoff + + // ActionTypeCancelPending indicates that pending prebuilds should be canceled. + ActionTypeCancelPending ) // PresetSnapshot is a filtered view of GlobalSnapshot focused on a single preset. @@ -49,6 +52,7 @@ type PresetSnapshot struct { Running []database.GetRunningPrebuiltWorkspacesRow Expired []database.GetRunningPrebuiltWorkspacesRow InProgress []database.CountInProgressPrebuildsRow + PendingCount int Backoff *database.GetPresetsBackoffRow IsHardLimited bool clock quartz.Clock @@ -61,6 +65,7 @@ func NewPresetSnapshot( running []database.GetRunningPrebuiltWorkspacesRow, expired []database.GetRunningPrebuiltWorkspacesRow, inProgress []database.CountInProgressPrebuildsRow, + pendingCount int, backoff *database.GetPresetsBackoffRow, isHardLimited bool, clock quartz.Clock, @@ -72,6 +77,7 @@ func NewPresetSnapshot( Running: running, Expired: expired, InProgress: inProgress, + PendingCount: pendingCount, Backoff: backoff, IsHardLimited: isHardLimited, clock: clock, @@ -115,7 +121,7 @@ type ReconciliationActions struct { } func (ra *ReconciliationActions) IsNoop() bool { - return ra.Create == 0 && len(ra.DeleteIDs) == 0 && ra.BackoffUntil.IsZero() + return ra.ActionType != ActionTypeCancelPending && ra.Create == 0 && len(ra.DeleteIDs) == 0 && ra.BackoffUntil.IsZero() } // MatchesCron interprets a cron spec as a continuous time range, @@ -345,18 +351,30 @@ func (p PresetSnapshot) handleActiveTemplateVersion() (actions []*Reconciliation return actions, nil } -// handleInactiveTemplateVersion deletes all running prebuilds except those already being deleted -// to avoid duplicate deletion attempts. -func (p PresetSnapshot) handleInactiveTemplateVersion() ([]*ReconciliationActions, error) { - prebuildsToDelete := len(p.Running) - deleteIDs := p.getOldestPrebuildIDs(prebuildsToDelete) +// handleInactiveTemplateVersion handles prebuilds from inactive template versions: +// 1. If the preset has pending prebuild jobs from an inactive template version, create a cancel reconciliation action. +// This cancels all pending prebuild jobs for this preset's template version. +// 2. If the preset has prebuilt workspaces currently running from an inactive template version, +// create a delete reconciliation action to remove all running prebuilt workspaces. +func (p PresetSnapshot) handleInactiveTemplateVersion() (actions []*ReconciliationActions, err error) { + // Cancel pending initial prebuild jobs from inactive version + if p.PendingCount > 0 { + actions = append(actions, + &ReconciliationActions{ + ActionType: ActionTypeCancelPending, + }) + } - return []*ReconciliationActions{ - { - ActionType: ActionTypeDelete, - DeleteIDs: deleteIDs, - }, - }, nil + // Delete prebuilds running in inactive version + deleteIDs := p.getOldestPrebuildIDs(len(p.Running)) + if len(deleteIDs) > 0 { + actions = append(actions, + &ReconciliationActions{ + ActionType: ActionTypeDelete, + DeleteIDs: deleteIDs, + }) + } + return actions, nil } // needsBackoffPeriod checks if we should delay prebuild creation due to recent failures. diff --git a/coderd/prebuilds/preset_snapshot_test.go b/coderd/prebuilds/preset_snapshot_test.go index 8a1a104513..c32a84777d 100644 --- a/coderd/prebuilds/preset_snapshot_test.go +++ b/coderd/prebuilds/preset_snapshot_test.go @@ -6,16 +6,14 @@ import ( "testing" "time" - "github.com/coder/coder/v2/testutil" - "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/coder/quartz" - "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/prebuilds" + "github.com/coder/coder/v2/testutil" + "github.com/coder/quartz" ) type options struct { @@ -86,7 +84,7 @@ func TestNoPrebuilds(t *testing.T) { preset(true, 0, current), } - snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil, nil, nil, clock, testutil.Logger(t)) + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil, nil, nil, nil, clock, testutil.Logger(t)) ps, err := snapshot.FilterByPreset(current.presetID) require.NoError(t, err) @@ -108,7 +106,7 @@ func TestNetNew(t *testing.T) { preset(true, 1, current), } - snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil, nil, nil, clock, testutil.Logger(t)) + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil, nil, nil, nil, clock, testutil.Logger(t)) ps, err := snapshot.FilterByPreset(current.presetID) require.NoError(t, err) @@ -150,7 +148,7 @@ func TestOutdatedPrebuilds(t *testing.T) { var inProgress []database.CountInProgressPrebuildsRow // WHEN: calculating the outdated preset's state. - snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, quartz.NewMock(t), testutil.Logger(t)) + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, nil, quartz.NewMock(t), testutil.Logger(t)) ps, err := snapshot.FilterByPreset(outdated.presetID) require.NoError(t, err) @@ -216,7 +214,7 @@ func TestDeleteOutdatedPrebuilds(t *testing.T) { } // WHEN: calculating the outdated preset's state. - snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, quartz.NewMock(t), testutil.Logger(t)) + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, nil, quartz.NewMock(t), testutil.Logger(t)) ps, err := snapshot.FilterByPreset(outdated.presetID) require.NoError(t, err) @@ -238,6 +236,74 @@ func TestDeleteOutdatedPrebuilds(t *testing.T) { }, actions) } +func TestCancelPendingPrebuilds(t *testing.T) { + t.Parallel() + + // Setup + current := opts[optionSet3] + clock := quartz.NewMock(t) + + t.Run("CancelPendingPrebuildsNonActiveVersion", func(t *testing.T) { + t.Parallel() + + // Given: a preset from a non-active version + defaultPreset := preset(false, 0, current) + presets := []database.GetTemplatePresetsWithPrebuildsRow{ + defaultPreset, + } + + // Given: 2 pending prebuilt workspaces for the preset + pending := []database.CountPendingNonActivePrebuildsRow{{ + PresetID: uuid.NullUUID{ + UUID: defaultPreset.ID, + Valid: true, + }, + Count: 2, + }} + + // When: calculating the current preset's state + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil, pending, nil, nil, clock, testutil.Logger(t)) + ps, err := snapshot.FilterByPreset(current.presetID) + require.NoError(t, err) + + // Then: it should create a cancel reconciliation action + actions, err := ps.CalculateActions(backoffInterval) + require.NoError(t, err) + expectedAction := []*prebuilds.ReconciliationActions{{ActionType: prebuilds.ActionTypeCancelPending}} + require.Equal(t, expectedAction, actions) + }) + + t.Run("NotCancelPendingPrebuildsActiveVersion", func(t *testing.T) { + t.Parallel() + + // Given: a preset from an active version + defaultPreset := preset(true, 0, current) + presets := []database.GetTemplatePresetsWithPrebuildsRow{ + defaultPreset, + } + + // Given: 2 pending prebuilt workspaces for the preset + pending := []database.CountPendingNonActivePrebuildsRow{{ + PresetID: uuid.NullUUID{ + UUID: defaultPreset.ID, + Valid: true, + }, + Count: 2, + }} + + // When: calculating the current preset's state + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil, pending, nil, nil, clock, testutil.Logger(t)) + ps, err := snapshot.FilterByPreset(current.presetID) + require.NoError(t, err) + + // Then: it should not create a cancel reconciliation action + actions, err := ps.CalculateActions(backoffInterval) + require.NoError(t, err) + var expectedAction []*prebuilds.ReconciliationActions + require.Equal(t, expectedAction, actions) + }) +} + // A new template version is created with a preset with prebuilds configured; while a prebuild is provisioning up or down, // the calculated actions should indicate the state correctly. func TestInProgressActions(t *testing.T) { @@ -460,7 +526,7 @@ func TestInProgressActions(t *testing.T) { } // WHEN: calculating the current preset's state. - snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, quartz.NewMock(t), testutil.Logger(t)) + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, nil, quartz.NewMock(t), testutil.Logger(t)) ps, err := snapshot.FilterByPreset(current.presetID) require.NoError(t, err) @@ -503,7 +569,7 @@ func TestExtraneous(t *testing.T) { var inProgress []database.CountInProgressPrebuildsRow // WHEN: calculating the current preset's state. - snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, quartz.NewMock(t), testutil.Logger(t)) + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, nil, quartz.NewMock(t), testutil.Logger(t)) ps, err := snapshot.FilterByPreset(current.presetID) require.NoError(t, err) @@ -683,7 +749,7 @@ func TestExpiredPrebuilds(t *testing.T) { } // WHEN: calculating the current preset's state. - snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, nil, nil, nil, clock, testutil.Logger(t)) + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, nil, nil, nil, nil, clock, testutil.Logger(t)) ps, err := snapshot.FilterByPreset(current.presetID) require.NoError(t, err) @@ -719,7 +785,7 @@ func TestDeprecated(t *testing.T) { var inProgress []database.CountInProgressPrebuildsRow // WHEN: calculating the current preset's state. - snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, quartz.NewMock(t), testutil.Logger(t)) + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, nil, nil, quartz.NewMock(t), testutil.Logger(t)) ps, err := snapshot.FilterByPreset(current.presetID) require.NoError(t, err) @@ -772,7 +838,7 @@ func TestLatestBuildFailed(t *testing.T) { } // WHEN: calculating the current preset's state. - snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, backoffs, nil, clock, testutil.Logger(t)) + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, running, inProgress, nil, backoffs, nil, clock, testutil.Logger(t)) psCurrent, err := snapshot.FilterByPreset(current.presetID) require.NoError(t, err) @@ -865,7 +931,7 @@ func TestMultiplePresetsPerTemplateVersion(t *testing.T) { }, } - snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, inProgress, nil, nil, clock, testutil.Logger(t)) + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, inProgress, nil, nil, nil, clock, testutil.Logger(t)) // Nothing has to be created for preset 1. { @@ -985,7 +1051,7 @@ func TestPrebuildScheduling(t *testing.T) { schedule(presets[1].ID, "* 14-16 * * 1-5", 5), } - snapshot := prebuilds.NewGlobalSnapshot(presets, schedules, nil, nil, nil, nil, clock, testutil.Logger(t)) + snapshot := prebuilds.NewGlobalSnapshot(presets, schedules, nil, nil, nil, nil, nil, clock, testutil.Logger(t)) // Check 1st preset. { @@ -1093,6 +1159,7 @@ func TestCalculateDesiredInstances(t *testing.T) { nil, nil, nil, + 0, nil, false, quartz.NewMock(t), diff --git a/enterprise/coderd/prebuilds/reconcile.go b/enterprise/coderd/prebuilds/reconcile.go index 214d1643bb..ceb16061bd 100644 --- a/enterprise/coderd/prebuilds/reconcile.go +++ b/enterprise/coderd/prebuilds/reconcile.go @@ -12,10 +12,13 @@ import ( "sync/atomic" "time" + "github.com/google/uuid" "github.com/hashicorp/go-multierror" "github.com/prometheus/client_golang/prometheus" + "golang.org/x/sync/errgroup" + "golang.org/x/xerrors" - "github.com/coder/quartz" + "cdr.dev/slog" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" @@ -30,12 +33,7 @@ import ( "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/coder/v2/codersdk" sdkproto "github.com/coder/coder/v2/provisionersdk/proto" - - "cdr.dev/slog" - - "github.com/google/uuid" - "golang.org/x/sync/errgroup" - "golang.org/x/xerrors" + "github.com/coder/quartz" ) type StoreReconciler struct { @@ -412,6 +410,11 @@ func (c *StoreReconciler) SnapshotState(ctx context.Context, store database.Stor return xerrors.Errorf("failed to get prebuilds in progress: %w", err) } + allPendingPrebuilds, err := db.CountPendingNonActivePrebuilds(ctx) + if err != nil { + return xerrors.Errorf("failed to get pending prebuilds: %w", err) + } + presetsBackoff, err := db.GetPresetsBackoff(ctx, c.clock.Now().Add(-c.cfg.ReconciliationBackoffLookback.Value())) if err != nil { return xerrors.Errorf("failed to get backoffs for presets: %w", err) @@ -427,6 +430,7 @@ func (c *StoreReconciler) SnapshotState(ctx context.Context, store database.Stor presetPrebuildSchedules, allRunningPrebuilds, allPrebuildsInProgress, + allPendingPrebuilds, presetsBackoff, hardLimitedPresets, c.clock, @@ -581,6 +585,8 @@ func (c *StoreReconciler) executeReconciliationAction(ctx context.Context, logge levelFn = logger.Info case action.ActionType == prebuilds.ActionTypeDelete && len(action.DeleteIDs) > 0: levelFn = logger.Info + case action.ActionType == prebuilds.ActionTypeCancelPending: + levelFn = logger.Info } switch action.ActionType { @@ -635,6 +641,36 @@ func (c *StoreReconciler) executeReconciliationAction(ctx context.Context, logge return multiErr.ErrorOrNil() + case prebuilds.ActionTypeCancelPending: + // Cancel pending prebuild jobs from non-active template versions to avoid + // provisioning obsolete workspaces that would immediately be deprovisioned. + // This uses a criteria-based update to ensure only jobs that are still pending + // at execution time are canceled, avoiding race conditions where jobs may have + // transitioned to running status between query and update. + canceledJobs, err := c.store.UpdatePrebuildProvisionerJobWithCancel( + ctx, + database.UpdatePrebuildProvisionerJobWithCancelParams{ + Now: c.clock.Now(), + PresetID: uuid.NullUUID{ + UUID: ps.Preset.ID, + Valid: true, + }, + }) + if err != nil { + logger.Error(ctx, "failed to cancel pending prebuild jobs", + slog.F("template_version_id", ps.Preset.TemplateVersionID.String()), + slog.F("preset_id", ps.Preset.ID), + slog.Error(err)) + return err + } + if len(canceledJobs) > 0 { + logger.Info(ctx, "canceled pending prebuild jobs for inactive version", + slog.F("template_version_id", ps.Preset.TemplateVersionID.String()), + slog.F("preset_id", ps.Preset.ID), + slog.F("count", len(canceledJobs))) + } + return nil + default: return xerrors.Errorf("unknown action type: %v", action.ActionType) } diff --git a/enterprise/coderd/prebuilds/reconcile_test.go b/enterprise/coderd/prebuilds/reconcile_test.go index 413d61ddbb..33b99145d8 100644 --- a/enterprise/coderd/prebuilds/reconcile_test.go +++ b/enterprise/coderd/prebuilds/reconcile_test.go @@ -9,36 +9,35 @@ import ( "testing" "time" + "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" - "golang.org/x/xerrors" - - "github.com/coder/coder/v2/coderd/coderdtest" - "github.com/coder/coder/v2/coderd/database/dbtime" - "github.com/coder/coder/v2/coderd/files" - "github.com/coder/coder/v2/coderd/notifications" - "github.com/coder/coder/v2/coderd/notifications/notificationstest" - "github.com/coder/coder/v2/coderd/util/slice" - "github.com/coder/coder/v2/coderd/wsbuilder" - sdkproto "github.com/coder/coder/v2/provisionersdk/proto" - - "github.com/google/uuid" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" "tailscale.com/types/ptr" "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" - "github.com/coder/quartz" - - "github.com/coder/serpent" + "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/pubsub" + "github.com/coder/coder/v2/coderd/files" + "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/notifications/notificationstest" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/util/slice" + "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/prebuilds" + sdkproto "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/testutil" + "github.com/coder/quartz" + "github.com/coder/serpent" ) func TestNoReconciliationActionsIfNoPresets(t *testing.T) { @@ -1783,6 +1782,552 @@ func TestExpiredPrebuildsMultipleActions(t *testing.T) { } } +func TestCancelPendingPrebuilds(t *testing.T) { + t.Parallel() + + t.Run("CancelPendingPrebuilds", func(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + name string + setupBuild func( + t *testing.T, + db database.Store, + client *codersdk.Client, + orgID uuid.UUID, + templateID uuid.UUID, + templateVersionID uuid.UUID, + presetID uuid.NullUUID, + ) dbfake.WorkspaceResponse + activeTemplateVersion bool + previouslyCanceled bool + previouslyCompleted bool + shouldCancel bool + }{ + // Should cancel pending prebuild-related jobs from a non-active template version + { + name: "CancelsPendingPrebuildJobNonActiveVersion", + // Given: a pending prebuild job + setupBuild: func(t *testing.T, + db database.Store, + client *codersdk.Client, + orgID uuid.UUID, + templateID uuid.UUID, + templateVersionID uuid.UUID, + presetID uuid.NullUUID, + ) dbfake.WorkspaceResponse { + return dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + OrganizationID: orgID, + TemplateID: templateID, + }).Pending().Seed(database.WorkspaceBuild{ + InitiatorID: database.PrebuildsSystemUserID, + TemplateVersionID: templateVersionID, + TemplateVersionPresetID: presetID, + }).Do() + }, + activeTemplateVersion: false, + previouslyCanceled: false, + previouslyCompleted: false, + shouldCancel: true, + }, + // Should not cancel pending prebuild-related jobs from an active template version + { + name: "DoesNotCancelPendingPrebuildJobActiveVersion", + // Given: a pending prebuild job + setupBuild: func(t *testing.T, + db database.Store, + client *codersdk.Client, + orgID uuid.UUID, + templateID uuid.UUID, + templateVersionID uuid.UUID, + presetID uuid.NullUUID, + ) dbfake.WorkspaceResponse { + return dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + OrganizationID: orgID, + TemplateID: templateID, + }).Pending().Seed(database.WorkspaceBuild{ + InitiatorID: database.PrebuildsSystemUserID, + TemplateVersionID: templateVersionID, + TemplateVersionPresetID: presetID, + }).Do() + }, + activeTemplateVersion: true, + previouslyCanceled: false, + previouslyCompleted: false, + shouldCancel: false, + }, + // Should not cancel pending prebuild-related jobs associated to a second workspace build + { + name: "DoesNotCancelPendingPrebuildJobSecondBuild", + // Given: a pending prebuild job associated to a second workspace build + setupBuild: func(t *testing.T, + db database.Store, + client *codersdk.Client, + orgID uuid.UUID, + templateID uuid.UUID, + templateVersionID uuid.UUID, + presetID uuid.NullUUID, + ) dbfake.WorkspaceResponse { + return dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + OrganizationID: orgID, + TemplateID: templateID, + }).Pending().Seed(database.WorkspaceBuild{ + InitiatorID: database.PrebuildsSystemUserID, + BuildNumber: int32(2), + TemplateVersionID: templateVersionID, + TemplateVersionPresetID: presetID, + }).Do() + }, + activeTemplateVersion: false, + previouslyCanceled: false, + previouslyCompleted: false, + shouldCancel: false, + }, + // Should not cancel pending prebuild-related jobs of a different template + { + name: "DoesNotCancelPrebuildJobDifferentTemplate", + // Given: a pending prebuild job belonging to a different template + setupBuild: func( + t *testing.T, + db database.Store, + client *codersdk.Client, + orgID uuid.UUID, + templateID uuid.UUID, + templateVersionID uuid.UUID, + presetID uuid.NullUUID, + ) dbfake.WorkspaceResponse { + return dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + OrganizationID: orgID, + TemplateID: uuid.Nil, + }).Pending().Seed(database.WorkspaceBuild{ + InitiatorID: database.PrebuildsSystemUserID, + TemplateVersionID: templateVersionID, + TemplateVersionPresetID: presetID, + }).Do() + }, + activeTemplateVersion: false, + previouslyCanceled: false, + previouslyCompleted: false, + shouldCancel: false, + }, + // Should not cancel pending user workspace build jobs + { + name: "DoesNotCancelUserWorkspaceJob", + // Given: a pending user workspace build job + setupBuild: func( + t *testing.T, + db database.Store, + client *codersdk.Client, + orgID uuid.UUID, + templateID uuid.UUID, + templateVersionID uuid.UUID, + presetID uuid.NullUUID, + ) dbfake.WorkspaceResponse { + _, member := coderdtest.CreateAnotherUser(t, client, orgID, rbac.RoleMember()) + return dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: member.ID, + OrganizationID: orgID, + TemplateID: uuid.Nil, + }).Pending().Seed(database.WorkspaceBuild{ + InitiatorID: member.ID, + TemplateVersionID: templateVersionID, + TemplateVersionPresetID: presetID, + }).Do() + }, + activeTemplateVersion: false, + previouslyCanceled: false, + previouslyCompleted: false, + shouldCancel: false, + }, + // Should not cancel pending prebuild-related jobs with a delete transition + { + name: "DoesNotCancelPrebuildJobDeleteTransition", + // Given: a pending prebuild job with a delete transition + setupBuild: func( + t *testing.T, + db database.Store, + client *codersdk.Client, + orgID uuid.UUID, + templateID uuid.UUID, + templateVersionID uuid.UUID, + presetID uuid.NullUUID, + ) dbfake.WorkspaceResponse { + return dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + OrganizationID: orgID, + TemplateID: templateID, + }).Pending().Seed(database.WorkspaceBuild{ + InitiatorID: database.PrebuildsSystemUserID, + Transition: database.WorkspaceTransitionDelete, + TemplateVersionID: templateVersionID, + TemplateVersionPresetID: presetID, + }).Do() + }, + activeTemplateVersion: false, + previouslyCanceled: false, + previouslyCompleted: false, + shouldCancel: false, + }, + // Should not cancel prebuild-related jobs already being processed by a provisioner + { + name: "DoesNotCancelRunningPrebuildJob", + // Given: a running prebuild job + setupBuild: func( + t *testing.T, + db database.Store, + client *codersdk.Client, + orgID uuid.UUID, + templateID uuid.UUID, + templateVersionID uuid.UUID, + presetID uuid.NullUUID, + ) dbfake.WorkspaceResponse { + return dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + OrganizationID: orgID, + TemplateID: templateID, + }).Starting().Seed(database.WorkspaceBuild{ + InitiatorID: database.PrebuildsSystemUserID, + TemplateVersionID: templateVersionID, + TemplateVersionPresetID: presetID, + }).Do() + }, + activeTemplateVersion: false, + previouslyCanceled: false, + previouslyCompleted: false, + shouldCancel: false, + }, + // Should not cancel already canceled prebuild-related jobs + { + name: "DoesNotCancelCanceledPrebuildJob", + // Given: a canceled prebuild job + setupBuild: func( + t *testing.T, + db database.Store, + client *codersdk.Client, + orgID uuid.UUID, + templateID uuid.UUID, + templateVersionID uuid.UUID, + presetID uuid.NullUUID, + ) dbfake.WorkspaceResponse { + return dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + OrganizationID: orgID, + TemplateID: templateID, + }).Canceled().Seed(database.WorkspaceBuild{ + InitiatorID: database.PrebuildsSystemUserID, + TemplateVersionID: templateVersionID, + TemplateVersionPresetID: presetID, + }).Do() + }, + activeTemplateVersion: false, + shouldCancel: false, + previouslyCanceled: true, + previouslyCompleted: true, + }, + // Should not cancel completed prebuild-related jobs + { + name: "DoesNotCancelCompletedPrebuildJob", + // Given: a completed prebuild job + setupBuild: func( + t *testing.T, + db database.Store, + client *codersdk.Client, + orgID uuid.UUID, + templateID uuid.UUID, + templateVersionID uuid.UUID, + presetID uuid.NullUUID, + ) dbfake.WorkspaceResponse { + return dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + OrganizationID: orgID, + TemplateID: templateID, + }).Seed(database.WorkspaceBuild{ + InitiatorID: database.PrebuildsSystemUserID, + TemplateVersionID: templateVersionID, + TemplateVersionPresetID: presetID, + }).Do() + }, + activeTemplateVersion: false, + shouldCancel: false, + previouslyCanceled: false, + previouslyCompleted: true, + }, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Set the clock to Monday, January 1st, 2024 at 8:00 AM UTC to keep the test deterministic + clock := quartz.NewMock(t) + clock.Set(time.Date(2024, 1, 1, 8, 0, 0, 0, time.UTC)) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // Setup + db, ps := dbtestutil.NewDB(t) + client, _, _ := coderdtest.NewWithAPI(t, &coderdtest.Options{ + // Explicitly not including provisioner daemons, as we don't want the jobs to be processed + // Jobs operations will be simulated via the database model + IncludeProvisionerDaemon: false, + Database: db, + Pubsub: ps, + Clock: clock, + }) + fakeEnqueuer := newFakeEnqueuer() + registry := prometheus.NewRegistry() + cache := files.New(registry, &coderdtest.FakeAuthorizer{}) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: false}).Leveled(slog.LevelDebug) + reconciler := prebuilds.NewStoreReconciler(db, ps, cache, codersdk.PrebuildsConfig{}, logger, clock, registry, fakeEnqueuer, newNoopUsageCheckerPtr()) + owner := coderdtest.CreateFirstUser(t, client) + + // Given: a template with a version containing a preset with 1 prebuild instance + nonActivePresetID := uuid.NullUUID{ + UUID: uuid.New(), + Valid: true, + } + nonActiveTemplateVersion := dbfake.TemplateVersion(t, db).Seed(database.TemplateVersion{ + OrganizationID: owner.OrganizationID, + CreatedBy: owner.UserID, + }).Preset(database.TemplateVersionPreset{ + ID: nonActivePresetID.UUID, + DesiredInstances: sql.NullInt32{ + Int32: 1, + Valid: true, + }, + }).Do() + templateID := nonActiveTemplateVersion.Template.ID + + // Given: a new active template version + activePresetID := uuid.NullUUID{ + UUID: uuid.New(), + Valid: true, + } + activeTemplateVersion := dbfake.TemplateVersion(t, db).Seed(database.TemplateVersion{ + OrganizationID: owner.OrganizationID, + CreatedBy: owner.UserID, + TemplateID: uuid.NullUUID{ + UUID: templateID, + Valid: true, + }, + }).Preset(database.TemplateVersionPreset{ + ID: activePresetID.UUID, + DesiredInstances: sql.NullInt32{ + Int32: 1, + Valid: true, + }, + }).SkipCreateTemplate().Do() + + var workspace dbfake.WorkspaceResponse + if tt.activeTemplateVersion { + // Given: a prebuilt workspace, workspace build and respective provisioner job from an + // active template version + workspace = tt.setupBuild(t, db, client, + owner.OrganizationID, templateID, activeTemplateVersion.TemplateVersion.ID, activePresetID) + } else { + // Given: a prebuilt workspace, workspace build and respective provisioner job from a + // non-active template version + workspace = tt.setupBuild(t, db, client, + owner.OrganizationID, templateID, nonActiveTemplateVersion.TemplateVersion.ID, nonActivePresetID) + } + + // Given: the new template version is promoted to active + err := db.UpdateTemplateActiveVersionByID(ctx, database.UpdateTemplateActiveVersionByIDParams{ + ID: templateID, + ActiveVersionID: activeTemplateVersion.TemplateVersion.ID, + }) + require.NoError(t, err) + + // When: the reconciliation loop is triggered + require.NoError(t, reconciler.ReconcileAll(ctx)) + + if tt.shouldCancel { + // Then: the prebuild related jobs from non-active version should be canceled + cancelledJob, err := db.GetProvisionerJobByID(ctx, workspace.Build.JobID) + require.NoError(t, err) + require.Equal(t, clock.Now().UTC(), cancelledJob.CanceledAt.Time.UTC()) + require.Equal(t, clock.Now().UTC(), cancelledJob.CompletedAt.Time.UTC()) + require.Equal(t, database.ProvisionerJobStatusCanceled, cancelledJob.JobStatus) + } else { + // Then: the provisioner job should not be canceled + job, err := db.GetProvisionerJobByID(ctx, workspace.Build.JobID) + require.NoError(t, err) + if !tt.previouslyCanceled { + require.Zero(t, job.CanceledAt.Time.UTC()) + require.NotEqual(t, database.ProvisionerJobStatusCanceled, job.JobStatus) + } + if !tt.previouslyCompleted { + require.Zero(t, job.CompletedAt.Time.UTC()) + } + } + }) + } + }) + + t.Run("CancelPendingPrebuildsMultipleTemplates", func(t *testing.T) { + t.Parallel() + + createTemplateVersionWithPreset := func( + t *testing.T, + db database.Store, + orgID uuid.UUID, + userID uuid.UUID, + templateID uuid.UUID, + prebuiltInstances int32, + ) (uuid.UUID, uuid.UUID, uuid.UUID) { + templatePreset := uuid.NullUUID{ + UUID: uuid.New(), + Valid: true, + } + templateVersion := dbfake.TemplateVersion(t, db).Seed(database.TemplateVersion{ + OrganizationID: orgID, + CreatedBy: userID, + TemplateID: uuid.NullUUID{ + UUID: templateID, + Valid: true, + }, + }).Preset(database.TemplateVersionPreset{ + ID: templatePreset.UUID, + DesiredInstances: sql.NullInt32{ + Int32: prebuiltInstances, + Valid: true, + }, + }).Do() + + return templateVersion.Template.ID, templateVersion.TemplateVersion.ID, templatePreset.UUID + } + + setupPrebuilds := func( + t *testing.T, + db database.Store, + orgID uuid.UUID, + templateID uuid.UUID, + versionID uuid.UUID, + presetID uuid.UUID, + count int, + pending bool, + ) []dbfake.WorkspaceResponse { + prebuilds := make([]dbfake.WorkspaceResponse, count) + for i := range count { + builder := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + OrganizationID: orgID, + TemplateID: templateID, + }) + + if pending { + builder = builder.Pending() + } + + prebuilds[i] = builder.Seed(database.WorkspaceBuild{ + InitiatorID: database.PrebuildsSystemUserID, + TemplateVersionID: versionID, + TemplateVersionPresetID: uuid.NullUUID{ + UUID: presetID, + Valid: true, + }, + }).Do() + } + + return prebuilds + } + + checkIfJobCanceled := func( + t *testing.T, + clock *quartz.Mock, + ctx context.Context, + db database.Store, + shouldBeCanceled bool, + prebuilds []dbfake.WorkspaceResponse, + ) { + for _, prebuild := range prebuilds { + job, err := db.GetProvisionerJobByID(ctx, prebuild.Build.JobID) + require.NoError(t, err) + + if shouldBeCanceled { + require.Equal(t, database.ProvisionerJobStatusCanceled, job.JobStatus) + require.Equal(t, clock.Now().UTC(), job.CanceledAt.Time.UTC()) + require.Equal(t, clock.Now().UTC(), job.CompletedAt.Time.UTC()) + } else { + require.NotEqual(t, database.ProvisionerJobStatusCanceled, job.JobStatus) + require.Zero(t, job.CanceledAt.Time.UTC()) + } + } + } + + // Set the clock to Monday, January 1st, 2024 at 8:00 AM UTC to keep the test deterministic + clock := quartz.NewMock(t) + clock.Set(time.Date(2024, 1, 1, 8, 0, 0, 0, time.UTC)) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // Setup + db, ps := dbtestutil.NewDB(t) + client, _, _ := coderdtest.NewWithAPI(t, &coderdtest.Options{ + // Explicitly not including provisioner daemons, as we don't want the jobs to be processed + // Jobs operations will be simulated via the database model + IncludeProvisionerDaemon: false, + Database: db, + Pubsub: ps, + Clock: clock, + }) + fakeEnqueuer := newFakeEnqueuer() + registry := prometheus.NewRegistry() + cache := files.New(registry, &coderdtest.FakeAuthorizer{}) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: false}).Leveled(slog.LevelDebug) + reconciler := prebuilds.NewStoreReconciler(db, ps, cache, codersdk.PrebuildsConfig{}, logger, clock, registry, fakeEnqueuer, newNoopUsageCheckerPtr()) + owner := coderdtest.CreateFirstUser(t, client) + + // Given: template A with 2 versions + // Given: template A version v1: with a preset with 5 instances (2 running, 3 pending) + templateAID, templateAVersion1ID, templateAVersion1PresetID := createTemplateVersionWithPreset(t, db, owner.OrganizationID, owner.UserID, uuid.Nil, 5) + templateAVersion1Running := setupPrebuilds(t, db, owner.OrganizationID, templateAID, templateAVersion1ID, templateAVersion1PresetID, 2, false) + templateAVersion1Pending := setupPrebuilds(t, db, owner.OrganizationID, templateAID, templateAVersion1ID, templateAVersion1PresetID, 3, true) + // Given: template A version v2 (active version): with a preset with 2 instances (1 running, 1 pending) + _, templateAVersion2ID, templateAVersion2PresetID := createTemplateVersionWithPreset(t, db, owner.OrganizationID, owner.UserID, templateAID, 2) + templateAVersion2Running := setupPrebuilds(t, db, owner.OrganizationID, templateAID, templateAVersion2ID, templateAVersion2PresetID, 1, false) + templateAVersion2Pending := setupPrebuilds(t, db, owner.OrganizationID, templateAID, templateAVersion2ID, templateAVersion2PresetID, 1, true) + + // Given: template B with 3 versions + // Given: template B version v1: with a preset with 3 instances (1 running, 2 pending) + templateBID, templateBVersion1ID, templateBVersion1PresetID := createTemplateVersionWithPreset(t, db, owner.OrganizationID, owner.UserID, uuid.Nil, 3) + templateBVersion1Running := setupPrebuilds(t, db, owner.OrganizationID, templateBID, templateBVersion1ID, templateBVersion1PresetID, 1, false) + templateBVersion1Pending := setupPrebuilds(t, db, owner.OrganizationID, templateBID, templateBVersion1ID, templateBVersion1PresetID, 2, true) + // Given: template B version v2: with a preset with 2 instances (2 pending) + _, templateBVersion2ID, templateBVersion2PresetID := createTemplateVersionWithPreset(t, db, owner.OrganizationID, owner.UserID, templateBID, 2) + templateBVersion2Pending := setupPrebuilds(t, db, owner.OrganizationID, templateBID, templateBVersion2ID, templateBVersion2PresetID, 2, true) + // Given: template B version v3 (active version): with a preset with 2 instances (1 running, 1 pending) + _, templateBVersion3ID, templateBVersion3PresetID := createTemplateVersionWithPreset(t, db, owner.OrganizationID, owner.UserID, templateBID, 2) + templateBVersion3Running := setupPrebuilds(t, db, owner.OrganizationID, templateBID, templateBVersion3ID, templateBVersion3PresetID, 1, false) + templateBVersion3Pending := setupPrebuilds(t, db, owner.OrganizationID, templateBID, templateBVersion3ID, templateBVersion3PresetID, 1, true) + + // When: the reconciliation loop is executed + require.NoError(t, reconciler.ReconcileAll(ctx)) + + // Then: template A version 1 running workspaces should not be canceled + checkIfJobCanceled(t, clock, ctx, db, false, templateAVersion1Running) + // Then: template A version 1 pending workspaces should be canceled + checkIfJobCanceled(t, clock, ctx, db, true, templateAVersion1Pending) + // Then: template A version 2 running and pending workspaces should not be canceled + checkIfJobCanceled(t, clock, ctx, db, false, templateAVersion2Running) + checkIfJobCanceled(t, clock, ctx, db, false, templateAVersion2Pending) + + // Then: template B version 1 running workspaces should not be canceled + checkIfJobCanceled(t, clock, ctx, db, false, templateBVersion1Running) + // Then: template B version 1 pending workspaces should be canceled + checkIfJobCanceled(t, clock, ctx, db, true, templateBVersion1Pending) + // Then: template B version 2 pending workspaces should be canceled + checkIfJobCanceled(t, clock, ctx, db, true, templateBVersion2Pending) + // Then: template B version 3 running and pending workspaces should not be canceled + checkIfJobCanceled(t, clock, ctx, db, false, templateBVersion3Running) + checkIfJobCanceled(t, clock, ctx, db, false, templateBVersion3Pending) + }) +} + func newNoopEnqueuer() *notifications.NoopEnqueuer { return notifications.NewNoopEnqueuer() }