From 9a26f937cba3c633b296f5b2530fa0a1e10df31a Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 6 Jul 2023 02:35:36 +0000 Subject: [PATCH 1/2] feat: add deleting_at column to workspaces --- coderd/autobuild/lifecycle_executor.go | 6 +- coderd/database/dbauthz/dbauthz.go | 14 ++- coderd/database/dbfake/dbfake.go | 57 +++++++++- coderd/database/dbmetrics/dbmetrics.go | 13 ++- coderd/database/dbmock/dbmock.go | 26 ++++- coderd/database/dump.sql | 3 +- .../000140_workspace_deleting_at.down.sql | 1 + .../000140_workspace_deleting_at.up.sql | 1 + coderd/database/modelmethods.go | 2 + coderd/database/modelqueries.go | 1 + coderd/database/models.go | 1 + coderd/database/querier.go | 3 +- coderd/database/queries.sql.go | 67 ++++++++--- coderd/database/queries/workspaces.sql | 25 +++- coderd/workspaces.go | 28 ++--- coderd/workspaces_internal_test.go | 82 -------------- coderd/workspaces_test.go | 20 +++- docs/admin/audit-logs.md | 2 +- enterprise/audit/table.go | 1 + enterprise/coderd/templates_test.go | 72 ++++++++++++ enterprise/coderd/templateschedule.go | 107 ++++++++++++++++++ enterprise/coderd/workspaces_test.go | 96 +++++++++++++--- 22 files changed, 467 insertions(+), 161 deletions(-) create mode 100644 coderd/database/migrations/000140_workspace_deleting_at.down.sql create mode 100644 coderd/database/migrations/000140_workspace_deleting_at.up.sql delete mode 100644 coderd/workspaces_internal_test.go create mode 100644 enterprise/coderd/templateschedule.go diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 48942ede42aa3..f7176ae8cd721 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -178,7 +178,7 @@ func (e *Executor) runOnce(t time.Time) Stats { // Lock the workspace if it has breached the template's // threshold for inactivity. if reason == database.BuildReasonAutolock { - err = tx.UpdateWorkspaceLockedAt(e.ctx, database.UpdateWorkspaceLockedAtParams{ + err = tx.UpdateWorkspaceLockedDeletingAt(e.ctx, database.UpdateWorkspaceLockedDeletingAtParams{ ID: ws.ID, LockedAt: sql.NullTime{ Time: database.Now(), @@ -347,11 +347,11 @@ func isEligibleForLockedStop(ws database.Workspace, templateSchedule schedule.Te func isEligibleForDelete(ws database.Workspace, templateSchedule schedule.TemplateScheduleOptions, currentTick time.Time) bool { // Only attempt to delete locked workspaces. - return ws.LockedAt.Valid && + return ws.LockedAt.Valid && ws.DeletingAt.Valid && // Locked workspaces should only be deleted if a locked_ttl is specified. templateSchedule.LockedTTL > 0 && // The workspace must breach the locked_ttl. - currentTick.Sub(ws.LockedAt.Time) > templateSchedule.LockedTTL + currentTick.After(ws.DeletingAt.Time) } // isEligibleForFailedStop returns true if the workspace is eligible to be stopped diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 57bcd12c96354..891b322bc213d 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2488,11 +2488,11 @@ func (q *querier) UpdateWorkspaceLastUsedAt(ctx context.Context, arg database.Up return update(q.log, q.auth, fetch, q.db.UpdateWorkspaceLastUsedAt)(ctx, arg) } -func (q *querier) UpdateWorkspaceLockedAt(ctx context.Context, arg database.UpdateWorkspaceLockedAtParams) error { - fetch := func(ctx context.Context, arg database.UpdateWorkspaceLockedAtParams) (database.Workspace, error) { +func (q *querier) UpdateWorkspaceLockedDeletingAt(ctx context.Context, arg database.UpdateWorkspaceLockedDeletingAtParams) error { + fetch := func(ctx context.Context, arg database.UpdateWorkspaceLockedDeletingAtParams) (database.Workspace, error) { return q.db.GetWorkspaceByID(ctx, arg.ID) } - return update(q.log, q.auth, fetch, q.db.UpdateWorkspaceLockedAt)(ctx, arg) + return update(q.log, q.auth, fetch, q.db.UpdateWorkspaceLockedDeletingAt)(ctx, arg) } func (q *querier) UpdateWorkspaceProxy(ctx context.Context, arg database.UpdateWorkspaceProxyParams) (database.WorkspaceProxy, error) { @@ -2516,6 +2516,14 @@ func (q *querier) UpdateWorkspaceTTL(ctx context.Context, arg database.UpdateWor return update(q.log, q.auth, fetch, q.db.UpdateWorkspaceTTL)(ctx, arg) } +func (q *querier) UpdateWorkspacesDeletingAtByTemplateID(ctx context.Context, arg database.UpdateWorkspacesDeletingAtByTemplateIDParams) error { + fetch := func(ctx context.Context, arg database.UpdateWorkspacesDeletingAtByTemplateIDParams) (database.Template, error) { + return q.db.GetTemplateByID(ctx, arg.TemplateID) + } + + return fetchAndExec(q.log, q.auth, rbac.ActionUpdate, fetch, q.db.UpdateWorkspacesDeletingAtByTemplateID)(ctx, arg) +} + func (q *querier) UpsertAppSecurityKey(ctx context.Context, data string) error { // No authz checks as this is done during startup return q.db.UpsertAppSecurityKey(ctx, data) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index b1ae1e05b4cb4..99c2c9d10b489 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -339,6 +339,8 @@ func (q *FakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspac AutostartSchedule: w.AutostartSchedule, Ttl: w.Ttl, LastUsedAt: w.LastUsedAt, + LockedAt: w.LockedAt, + DeletingAt: w.DeletingAt, Count: count, } @@ -4851,24 +4853,42 @@ func (q *FakeQuerier) UpdateWorkspaceLastUsedAt(_ context.Context, arg database. return sql.ErrNoRows } -func (q *FakeQuerier) UpdateWorkspaceLockedAt(_ context.Context, arg database.UpdateWorkspaceLockedAtParams) error { +func (q *FakeQuerier) UpdateWorkspaceLockedDeletingAt(_ context.Context, arg database.UpdateWorkspaceLockedDeletingAtParams) error { if err := validateDatabaseType(arg); err != nil { return err } - q.mutex.Lock() defer q.mutex.Unlock() - for index, workspace := range q.workspaces { if workspace.ID != arg.ID { continue } workspace.LockedAt = arg.LockedAt - workspace.LastUsedAt = database.Now() + if workspace.LockedAt.Time.IsZero() { + workspace.LastUsedAt = database.Now() + workspace.DeletingAt = sql.NullTime{} + } + if !workspace.LockedAt.Time.IsZero() { + var template database.TemplateTable + for _, t := range q.templates { + if t.ID == workspace.TemplateID { + template = t + break + } + } + if template.ID == uuid.Nil { + return xerrors.Errorf("unable to find workspace template") + } + if template.LockedTTL > 0 { + workspace.DeletingAt = sql.NullTime{ + Valid: true, + Time: workspace.LockedAt.Time.Add(time.Duration(template.LockedTTL)), + } + } + } q.workspaces[index] = workspace return nil } - return sql.ErrNoRows } @@ -4932,6 +4952,32 @@ func (q *FakeQuerier) UpdateWorkspaceTTL(_ context.Context, arg database.UpdateW return sql.ErrNoRows } +func (q *FakeQuerier) UpdateWorkspacesDeletingAtByTemplateID(_ context.Context, arg database.UpdateWorkspacesDeletingAtByTemplateIDParams) error { + q.mutex.Lock() + defer q.mutex.Unlock() + + err := validateDatabaseType(arg) + if err != nil { + return err + } + + for i, ws := range q.workspaces { + if ws.LockedAt.Time.IsZero() { + continue + } + deletingAt := sql.NullTime{ + Valid: arg.LockedTtlMs > 0, + } + if arg.LockedTtlMs > 0 { + deletingAt.Time = ws.LockedAt.Time.Add(time.Duration(arg.LockedTtlMs) * time.Millisecond) + } + ws.DeletingAt = deletingAt + q.workspaces[i] = ws + } + + return nil +} + func (q *FakeQuerier) UpsertAppSecurityKey(_ context.Context, data string) error { q.mutex.Lock() defer q.mutex.Unlock() @@ -4951,7 +4997,6 @@ func (q *FakeQuerier) UpsertLastUpdateCheck(_ context.Context, data string) erro defer q.mutex.Unlock() q.lastUpdateCheck = []byte(data) - return nil } func (q *FakeQuerier) UpsertLogoURL(_ context.Context, data string) error { diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index e202f7bbc2992..2c8e18ab556c8 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -1516,10 +1516,10 @@ func (m metricsStore) UpdateWorkspaceLastUsedAt(ctx context.Context, arg databas return err } -func (m metricsStore) UpdateWorkspaceLockedAt(ctx context.Context, arg database.UpdateWorkspaceLockedAtParams) error { +func (m metricsStore) UpdateWorkspaceLockedDeletingAt(ctx context.Context, arg database.UpdateWorkspaceLockedDeletingAtParams) error { start := time.Now() - r0 := m.s.UpdateWorkspaceLockedAt(ctx, arg) - m.queryLatencies.WithLabelValues("UpdateWorkspaceLockedAt").Observe(time.Since(start).Seconds()) + r0 := m.s.UpdateWorkspaceLockedDeletingAt(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateWorkspaceLockedDeletingAt").Observe(time.Since(start).Seconds()) return r0 } @@ -1544,6 +1544,13 @@ func (m metricsStore) UpdateWorkspaceTTL(ctx context.Context, arg database.Updat return r0 } +func (m metricsStore) UpdateWorkspacesDeletingAtByTemplateID(ctx context.Context, arg database.UpdateWorkspacesDeletingAtByTemplateIDParams) error { + start := time.Now() + r0 := m.s.UpdateWorkspacesDeletingAtByTemplateID(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateWorkspacesDeletingAtByTemplateID").Observe(time.Since(start).Seconds()) + return r0 +} + func (m metricsStore) UpsertAppSecurityKey(ctx context.Context, value string) error { start := time.Now() r0 := m.s.UpsertAppSecurityKey(ctx, value) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 2a63a4c83f2c4..fd6b5d3f33013 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -3191,18 +3191,18 @@ func (mr *MockStoreMockRecorder) UpdateWorkspaceLastUsedAt(arg0, arg1 interface{ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateWorkspaceLastUsedAt", reflect.TypeOf((*MockStore)(nil).UpdateWorkspaceLastUsedAt), arg0, arg1) } -// UpdateWorkspaceLockedAt mocks base method. -func (m *MockStore) UpdateWorkspaceLockedAt(arg0 context.Context, arg1 database.UpdateWorkspaceLockedAtParams) error { +// UpdateWorkspaceLockedDeletingAt mocks base method. +func (m *MockStore) UpdateWorkspaceLockedDeletingAt(arg0 context.Context, arg1 database.UpdateWorkspaceLockedDeletingAtParams) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateWorkspaceLockedAt", arg0, arg1) + ret := m.ctrl.Call(m, "UpdateWorkspaceLockedDeletingAt", arg0, arg1) ret0, _ := ret[0].(error) return ret0 } -// UpdateWorkspaceLockedAt indicates an expected call of UpdateWorkspaceLockedAt. -func (mr *MockStoreMockRecorder) UpdateWorkspaceLockedAt(arg0, arg1 interface{}) *gomock.Call { +// UpdateWorkspaceLockedDeletingAt indicates an expected call of UpdateWorkspaceLockedDeletingAt. +func (mr *MockStoreMockRecorder) UpdateWorkspaceLockedDeletingAt(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateWorkspaceLockedAt", reflect.TypeOf((*MockStore)(nil).UpdateWorkspaceLockedAt), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateWorkspaceLockedDeletingAt", reflect.TypeOf((*MockStore)(nil).UpdateWorkspaceLockedDeletingAt), arg0, arg1) } // UpdateWorkspaceProxy mocks base method. @@ -3248,6 +3248,20 @@ func (mr *MockStoreMockRecorder) UpdateWorkspaceTTL(arg0, arg1 interface{}) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateWorkspaceTTL", reflect.TypeOf((*MockStore)(nil).UpdateWorkspaceTTL), arg0, arg1) } +// UpdateWorkspacesDeletingAtByTemplateID mocks base method. +func (m *MockStore) UpdateWorkspacesDeletingAtByTemplateID(arg0 context.Context, arg1 database.UpdateWorkspacesDeletingAtByTemplateIDParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateWorkspacesDeletingAtByTemplateID", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateWorkspacesDeletingAtByTemplateID indicates an expected call of UpdateWorkspacesDeletingAtByTemplateID. +func (mr *MockStoreMockRecorder) UpdateWorkspacesDeletingAtByTemplateID(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateWorkspacesDeletingAtByTemplateID", reflect.TypeOf((*MockStore)(nil).UpdateWorkspacesDeletingAtByTemplateID), arg0, arg1) +} + // UpsertAppSecurityKey mocks base method. func (m *MockStore) UpsertAppSecurityKey(arg0 context.Context, arg1 string) error { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index f486d162e15c4..42baf9ea31c57 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -876,7 +876,8 @@ CREATE TABLE workspaces ( autostart_schedule text, ttl bigint, last_used_at timestamp without time zone DEFAULT '0001-01-01 00:00:00'::timestamp without time zone NOT NULL, - locked_at timestamp with time zone + locked_at timestamp with time zone, + deleting_at timestamp with time zone ); ALTER TABLE ONLY licenses ALTER COLUMN id SET DEFAULT nextval('licenses_id_seq'::regclass); diff --git a/coderd/database/migrations/000140_workspace_deleting_at.down.sql b/coderd/database/migrations/000140_workspace_deleting_at.down.sql new file mode 100644 index 0000000000000..13bd9049fc3ba --- /dev/null +++ b/coderd/database/migrations/000140_workspace_deleting_at.down.sql @@ -0,0 +1 @@ +ALTER TABLE workspaces DROP COLUMN deleting_at; diff --git a/coderd/database/migrations/000140_workspace_deleting_at.up.sql b/coderd/database/migrations/000140_workspace_deleting_at.up.sql new file mode 100644 index 0000000000000..a2e540c1b02af --- /dev/null +++ b/coderd/database/migrations/000140_workspace_deleting_at.up.sql @@ -0,0 +1 @@ +ALTER TABLE workspaces ADD COLUMN deleting_at timestamptz NULL; diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index bb7dfdd1bb818..536cc4c1178fb 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -354,6 +354,8 @@ func ConvertWorkspaceRows(rows []GetWorkspacesRow) []Workspace { AutostartSchedule: r.AutostartSchedule, Ttl: r.Ttl, LastUsedAt: r.LastUsedAt, + LockedAt: r.LockedAt, + DeletingAt: r.DeletingAt, } } diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index 0682b930baaf0..3e2cfbc125696 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -240,6 +240,7 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa &i.Ttl, &i.LastUsedAt, &i.LockedAt, + &i.DeletingAt, &i.TemplateName, &i.TemplateVersionID, &i.TemplateVersionName, diff --git a/coderd/database/models.go b/coderd/database/models.go index 2fb9fbb244abd..80787f57b7642 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1747,6 +1747,7 @@ type Workspace struct { Ttl sql.NullInt64 `db:"ttl" json:"ttl"` LastUsedAt time.Time `db:"last_used_at" json:"last_used_at"` LockedAt sql.NullTime `db:"locked_at" json:"locked_at"` + DeletingAt sql.NullTime `db:"deleting_at" json:"deleting_at"` } type WorkspaceAgent struct { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 05b5a07acdf32..acb93080af824 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -255,11 +255,12 @@ type sqlcQuerier interface { UpdateWorkspaceBuildCostByID(ctx context.Context, arg UpdateWorkspaceBuildCostByIDParams) (WorkspaceBuild, error) UpdateWorkspaceDeletedByID(ctx context.Context, arg UpdateWorkspaceDeletedByIDParams) error UpdateWorkspaceLastUsedAt(ctx context.Context, arg UpdateWorkspaceLastUsedAtParams) error - UpdateWorkspaceLockedAt(ctx context.Context, arg UpdateWorkspaceLockedAtParams) error + UpdateWorkspaceLockedDeletingAt(ctx context.Context, arg UpdateWorkspaceLockedDeletingAtParams) error // This allows editing the properties of a workspace proxy. UpdateWorkspaceProxy(ctx context.Context, arg UpdateWorkspaceProxyParams) (WorkspaceProxy, error) UpdateWorkspaceProxyDeleted(ctx context.Context, arg UpdateWorkspaceProxyDeletedParams) error UpdateWorkspaceTTL(ctx context.Context, arg UpdateWorkspaceTTLParams) error + UpdateWorkspacesDeletingAtByTemplateID(ctx context.Context, arg UpdateWorkspacesDeletingAtByTemplateIDParams) error UpsertAppSecurityKey(ctx context.Context, value string) error // The default proxy is implied and not actually stored in the database. // So we need to store it's configuration here for display purposes. diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index e5786aad31a63..b23cdfe646db2 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -8148,7 +8148,7 @@ func (q *sqlQuerier) GetDeploymentWorkspaceStats(ctx context.Context) (GetDeploy const getWorkspaceByAgentID = `-- name: GetWorkspaceByAgentID :one SELECT - id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, locked_at + id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, locked_at, deleting_at FROM workspaces WHERE @@ -8192,13 +8192,14 @@ func (q *sqlQuerier) GetWorkspaceByAgentID(ctx context.Context, agentID uuid.UUI &i.Ttl, &i.LastUsedAt, &i.LockedAt, + &i.DeletingAt, ) return i, err } const getWorkspaceByID = `-- name: GetWorkspaceByID :one SELECT - id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, locked_at + id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, locked_at, deleting_at FROM workspaces WHERE @@ -8223,13 +8224,14 @@ func (q *sqlQuerier) GetWorkspaceByID(ctx context.Context, id uuid.UUID) (Worksp &i.Ttl, &i.LastUsedAt, &i.LockedAt, + &i.DeletingAt, ) return i, err } const getWorkspaceByOwnerIDAndName = `-- name: GetWorkspaceByOwnerIDAndName :one SELECT - id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, locked_at + id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, locked_at, deleting_at FROM workspaces WHERE @@ -8261,13 +8263,14 @@ func (q *sqlQuerier) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg GetWo &i.Ttl, &i.LastUsedAt, &i.LockedAt, + &i.DeletingAt, ) return i, err } const getWorkspaceByWorkspaceAppID = `-- name: GetWorkspaceByWorkspaceAppID :one SELECT - id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, locked_at + id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, locked_at, deleting_at FROM workspaces WHERE @@ -8318,13 +8321,14 @@ func (q *sqlQuerier) GetWorkspaceByWorkspaceAppID(ctx context.Context, workspace &i.Ttl, &i.LastUsedAt, &i.LockedAt, + &i.DeletingAt, ) return i, err } const getWorkspaces = `-- name: GetWorkspaces :many SELECT - workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, workspaces.locked_at, + workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, workspaces.locked_at, workspaces.deleting_at, COALESCE(template_name.template_name, 'unknown') as template_name, latest_build.template_version_id, latest_build.template_version_name, @@ -8553,6 +8557,7 @@ type GetWorkspacesRow struct { Ttl sql.NullInt64 `db:"ttl" json:"ttl"` LastUsedAt time.Time `db:"last_used_at" json:"last_used_at"` LockedAt sql.NullTime `db:"locked_at" json:"locked_at"` + DeletingAt sql.NullTime `db:"deleting_at" json:"deleting_at"` TemplateName string `db:"template_name" json:"template_name"` TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` TemplateVersionName sql.NullString `db:"template_version_name" json:"template_version_name"` @@ -8593,6 +8598,7 @@ func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) &i.Ttl, &i.LastUsedAt, &i.LockedAt, + &i.DeletingAt, &i.TemplateName, &i.TemplateVersionID, &i.TemplateVersionName, @@ -8613,7 +8619,7 @@ func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) const getWorkspacesEligibleForTransition = `-- name: GetWorkspacesEligibleForTransition :many SELECT - workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, workspaces.locked_at + workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, workspaces.locked_at, workspaces.deleting_at FROM workspaces LEFT JOIN @@ -8699,6 +8705,7 @@ func (q *sqlQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, now &i.Ttl, &i.LastUsedAt, &i.LockedAt, + &i.DeletingAt, ); err != nil { return nil, err } @@ -8728,7 +8735,7 @@ INSERT INTO last_used_at ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, locked_at + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, locked_at, deleting_at ` type InsertWorkspaceParams struct { @@ -8771,6 +8778,7 @@ func (q *sqlQuerier) InsertWorkspace(ctx context.Context, arg InsertWorkspacePar &i.Ttl, &i.LastUsedAt, &i.LockedAt, + &i.DeletingAt, ) return i, err } @@ -8783,7 +8791,7 @@ SET WHERE id = $1 AND deleted = false -RETURNING id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, locked_at +RETURNING id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, locked_at, deleting_at ` type UpdateWorkspaceParams struct { @@ -8807,6 +8815,7 @@ func (q *sqlQuerier) UpdateWorkspace(ctx context.Context, arg UpdateWorkspacePar &i.Ttl, &i.LastUsedAt, &i.LockedAt, + &i.DeletingAt, ) return i, err } @@ -8868,23 +8877,32 @@ func (q *sqlQuerier) UpdateWorkspaceLastUsedAt(ctx context.Context, arg UpdateWo return err } -const updateWorkspaceLockedAt = `-- name: UpdateWorkspaceLockedAt :exec +const updateWorkspaceLockedDeletingAt = `-- name: UpdateWorkspaceLockedDeletingAt :exec UPDATE workspaces SET locked_at = $2, - last_used_at = now() at time zone 'utc' + -- When a workspace is unlocked we want to update the last_used_at to avoid the workspace getting re-locked. + -- if we're locking the workspace then we leave it alone. + last_used_at = CASE WHEN $2::timestamptz IS NULL THEN now() at time zone 'utc' ELSE last_used_at END, + -- If locked_at is null (meaning unlocked) or the template-defined locked_ttl is 0 we should set + -- deleting_at to NULL else set it to the locked_at + locked_ttl duration. + deleting_at = CASE WHEN $2::timestamptz IS NULL OR templates.locked_ttl = 0 THEN NULL ELSE $2::timestamptz + INTERVAL '1 milliseconds' * templates.locked_ttl / 1000000 END +FROM + templates WHERE - id = $1 + workspaces.template_id = templates.id +AND + workspaces.id = $1 ` -type UpdateWorkspaceLockedAtParams struct { +type UpdateWorkspaceLockedDeletingAtParams struct { ID uuid.UUID `db:"id" json:"id"` LockedAt sql.NullTime `db:"locked_at" json:"locked_at"` } -func (q *sqlQuerier) UpdateWorkspaceLockedAt(ctx context.Context, arg UpdateWorkspaceLockedAtParams) error { - _, err := q.db.ExecContext(ctx, updateWorkspaceLockedAt, arg.ID, arg.LockedAt) +func (q *sqlQuerier) UpdateWorkspaceLockedDeletingAt(ctx context.Context, arg UpdateWorkspaceLockedDeletingAtParams) error { + _, err := q.db.ExecContext(ctx, updateWorkspaceLockedDeletingAt, arg.ID, arg.LockedAt) return err } @@ -8906,3 +8924,24 @@ func (q *sqlQuerier) UpdateWorkspaceTTL(ctx context.Context, arg UpdateWorkspace _, err := q.db.ExecContext(ctx, updateWorkspaceTTL, arg.ID, arg.Ttl) return err } + +const updateWorkspacesDeletingAtByTemplateID = `-- name: UpdateWorkspacesDeletingAtByTemplateID :exec +UPDATE + workspaces +SET + deleting_at = CASE WHEN $1::bigint = 0 THEN NULL ELSE locked_at + interval '1 milliseconds' * $1::bigint END +WHERE + template_id = $2 +AND + locked_at IS NOT NULL +` + +type UpdateWorkspacesDeletingAtByTemplateIDParams struct { + LockedTtlMs int64 `db:"locked_ttl_ms" json:"locked_ttl_ms"` + TemplateID uuid.UUID `db:"template_id" json:"template_id"` +} + +func (q *sqlQuerier) UpdateWorkspacesDeletingAtByTemplateID(ctx context.Context, arg UpdateWorkspacesDeletingAtByTemplateIDParams) error { + _, err := q.db.ExecContext(ctx, updateWorkspacesDeletingAtByTemplateID, arg.LockedTtlMs, arg.TemplateID) + return err +} diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 2af26fd922402..5e540a0e5c90a 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -474,11 +474,30 @@ WHERE ) ) AND workspaces.deleted = 'false'; --- name: UpdateWorkspaceLockedAt :exec +-- name: UpdateWorkspaceLockedDeletingAt :exec UPDATE workspaces SET locked_at = $2, - last_used_at = now() at time zone 'utc' + -- When a workspace is unlocked we want to update the last_used_at to avoid the workspace getting re-locked. + -- if we're locking the workspace then we leave it alone. + last_used_at = CASE WHEN $2::timestamptz IS NULL THEN now() at time zone 'utc' ELSE last_used_at END, + -- If locked_at is null (meaning unlocked) or the template-defined locked_ttl is 0 we should set + -- deleting_at to NULL else set it to the locked_at + locked_ttl duration. + deleting_at = CASE WHEN $2::timestamptz IS NULL OR templates.locked_ttl = 0 THEN NULL ELSE $2::timestamptz + INTERVAL '1 milliseconds' * templates.locked_ttl / 1000000 END +FROM + templates WHERE - id = $1; + workspaces.template_id = templates.id +AND + workspaces.id = $1; + +-- name: UpdateWorkspacesDeletingAtByTemplateID :exec +UPDATE + workspaces +SET + deleting_at = CASE WHEN @locked_ttl_ms::bigint = 0 THEN NULL ELSE locked_at + interval '1 milliseconds' * @locked_ttl_ms::bigint END +WHERE + template_id = @template_id +AND + locked_at IS NOT NULL; diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 8ac07b5ddd51e..e912ba2fcc40c 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -12,7 +12,6 @@ import ( "github.com/go-chi/chi/v5" "github.com/google/uuid" - "golang.org/x/exp/slices" "golang.org/x/xerrors" "cdr.dev/slog" @@ -797,7 +796,7 @@ func (api *API) putWorkspaceLock(rw http.ResponseWriter, r *http.Request) { lockedAt.Time = database.Now() } - err := api.Database.UpdateWorkspaceLockedAt(ctx, database.UpdateWorkspaceLockedAtParams{ + err := api.Database.UpdateWorkspaceLockedDeletingAt(ctx, database.UpdateWorkspaceLockedDeletingAtParams{ ID: workspace.ID, LockedAt: lockedAt, }) @@ -1119,6 +1118,11 @@ func convertWorkspace( lockedAt = &workspace.LockedAt.Time } + var deletedAt *time.Time + if workspace.DeletingAt.Valid { + deletedAt = &workspace.DeletingAt.Time + } + failingAgents := []uuid.UUID{} for _, resource := range workspaceBuild.Resources { for _, agent := range resource.Agents { @@ -1128,10 +1132,7 @@ func convertWorkspace( } } - var ( - ttlMillis = convertWorkspaceTTLMillis(workspace.Ttl) - deletingAt = calculateDeletingAt(workspace, template, workspaceBuild) - ) + ttlMillis := convertWorkspaceTTLMillis(workspace.Ttl) return codersdk.Workspace{ ID: workspace.ID, @@ -1151,7 +1152,7 @@ func convertWorkspace( AutostartSchedule: autostartSchedule, TTLMillis: ttlMillis, LastUsedAt: workspace.LastUsedAt, - DeletingAt: deletingAt, + DeletingAt: deletedAt, LockedAt: lockedAt, Health: codersdk.WorkspaceHealth{ Healthy: len(failingAgents) == 0, @@ -1169,19 +1170,6 @@ func convertWorkspaceTTLMillis(i sql.NullInt64) *int64 { return &millis } -// Calculate the time of the upcoming workspace deletion, if applicable; otherwise, return nil. -// Workspaces may have impending deletions if InactivityTTL feature is turned on and the workspace is inactive. -func calculateDeletingAt(workspace database.Workspace, template database.Template, build codersdk.WorkspaceBuild) *time.Time { - inactiveStatuses := []codersdk.WorkspaceStatus{codersdk.WorkspaceStatusStopped, codersdk.WorkspaceStatusCanceled, codersdk.WorkspaceStatusFailed, codersdk.WorkspaceStatusDeleted} - isInactive := slices.Contains(inactiveStatuses, build.Status) - // If InactivityTTL is turned off (set to 0) or if the workspace is active, there is no impending deletion - if template.InactivityTTL == 0 || !isInactive { - return nil - } - - return ptr.Ref(workspace.LastUsedAt.Add(time.Duration(template.InactivityTTL) * time.Nanosecond)) -} - func validWorkspaceTTLMillis(millis *int64, templateDefault, templateMax time.Duration) (sql.NullInt64, error) { if templateDefault == 0 && templateMax != 0 || (templateMax > 0 && templateDefault > templateMax) { templateDefault = templateMax diff --git a/coderd/workspaces_internal_test.go b/coderd/workspaces_internal_test.go deleted file mode 100644 index 44c1699309c4c..0000000000000 --- a/coderd/workspaces_internal_test.go +++ /dev/null @@ -1,82 +0,0 @@ -package coderd - -import ( - "testing" - "time" - - "github.com/stretchr/testify/require" - - "github.com/coder/coder/coderd/database" - "github.com/coder/coder/coderd/util/ptr" - "github.com/coder/coder/codersdk" -) - -func Test_calculateDeletingAt(t *testing.T) { - t.Parallel() - - testCases := []struct { - name string - workspace database.Workspace - template database.Template - build codersdk.WorkspaceBuild - expected *time.Time - }{ - { - name: "InactiveWorkspace", - workspace: database.Workspace{ - Deleted: false, - LastUsedAt: time.Now().Add(time.Duration(-10) * time.Hour * 24), // 10 days ago - }, - template: database.Template{ - InactivityTTL: int64(9 * 24 * time.Hour), // 9 days - }, - build: codersdk.WorkspaceBuild{ - Status: codersdk.WorkspaceStatusStopped, - }, - expected: ptr.Ref(time.Now().Add(time.Duration(-1) * time.Hour * 24)), // yesterday - }, - { - name: "InactivityTTLUnset", - workspace: database.Workspace{ - Deleted: false, - LastUsedAt: time.Now().Add(time.Duration(-10) * time.Hour * 24), - }, - template: database.Template{ - InactivityTTL: 0, - }, - build: codersdk.WorkspaceBuild{ - Status: codersdk.WorkspaceStatusStopped, - }, - expected: nil, - }, - { - name: "ActiveWorkspace", - workspace: database.Workspace{ - Deleted: false, - LastUsedAt: time.Now(), - }, - template: database.Template{ - InactivityTTL: int64(1 * 24 * time.Hour), - }, - build: codersdk.WorkspaceBuild{ - Status: codersdk.WorkspaceStatusRunning, - }, - expected: nil, - }, - } - - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - found := calculateDeletingAt(tc.workspace, tc.template, tc.build) - if tc.expected == nil { - require.Nil(t, found, "impending deletion should be nil") - } else { - require.NotNil(t, found) - require.WithinDuration(t, *tc.expected, *found, time.Second, "incorrect impending deletion") - } - }) - } -} diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index d0609efcba355..db5db020488b7 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -2680,14 +2680,19 @@ func TestWorkspaceLock(t *testing.T) { user = coderdtest.CreateFirstUser(t, client) version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - _ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + lockedTTL = time.Minute ) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.LockedTTLMillis = ptr.Ref[int64](lockedTTL.Milliseconds()) + }) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + _ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() + lastUsedAt := workspace.LastUsedAt err := client.UpdateWorkspaceLock(ctx, workspace.ID, codersdk.UpdateWorkspaceLock{ Lock: true, }) @@ -2695,10 +2700,14 @@ func TestWorkspaceLock(t *testing.T) { workspace = coderdtest.MustWorkspace(t, client, workspace.ID) require.NoError(t, err, "fetch provisioned workspace") + // The template doesn't have a locked_ttl set so this should be nil. + require.Nil(t, workspace.DeletingAt) require.NotNil(t, workspace.LockedAt) require.WithinRange(t, *workspace.LockedAt, time.Now().Add(-time.Second*10), time.Now()) + require.Equal(t, lastUsedAt, workspace.LastUsedAt) - lastUsedAt := workspace.LastUsedAt + workspace = coderdtest.MustWorkspace(t, client, workspace.ID) + lastUsedAt = workspace.LastUsedAt err = client.UpdateWorkspaceLock(ctx, workspace.ID, codersdk.UpdateWorkspaceLock{ Lock: false, }) @@ -2707,6 +2716,9 @@ func TestWorkspaceLock(t *testing.T) { workspace, err = client.Workspace(ctx, workspace.ID) require.NoError(t, err, "fetch provisioned workspace") require.Nil(t, workspace.LockedAt) + // The template doesn't have a locked_ttl set so this should be nil. + require.Nil(t, workspace.DeletingAt) + // The last_used_at should get updated when we unlock the workspace. require.True(t, workspace.LastUsedAt.After(lastUsedAt)) }) diff --git a/docs/admin/audit-logs.md b/docs/admin/audit-logs.md index 0f23bed248839..c9a18b3b0362e 100644 --- a/docs/admin/audit-logs.md +++ b/docs/admin/audit-logs.md @@ -19,7 +19,7 @@ We track the following resources: | Template
write, delete |
FieldTracked
active_version_idtrue
allow_user_autostarttrue
allow_user_autostoptrue
allow_user_cancel_workspace_jobstrue
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_usernamefalse
default_ttltrue
deletedfalse
descriptiontrue
display_nametrue
failure_ttltrue
group_acltrue
icontrue
idtrue
inactivity_ttltrue
locked_ttltrue
max_ttltrue
nametrue
organization_idfalse
provisionertrue
restart_requirement_days_of_weektrue
restart_requirement_weekstrue
updated_atfalse
user_acltrue
| | TemplateVersion
create, write |
FieldTracked
created_atfalse
created_bytrue
git_auth_providersfalse
idtrue
job_idfalse
messagefalse
nametrue
organization_idfalse
readmetrue
template_idtrue
updated_atfalse
| | User
create, write, delete |
FieldTracked
avatar_urlfalse
created_atfalse
deletedtrue
emailtrue
hashed_passwordtrue
idtrue
last_seen_atfalse
login_typetrue
quiet_hours_scheduletrue
rbac_rolestrue
statustrue
updated_atfalse
usernametrue
| -| Workspace
create, write, delete |
FieldTracked
autostart_scheduletrue
created_atfalse
deletedfalse
idtrue
last_used_atfalse
locked_attrue
nametrue
organization_idfalse
owner_idtrue
template_idtrue
ttltrue
updated_atfalse
| +| Workspace
create, write, delete |
FieldTracked
autostart_scheduletrue
created_atfalse
deletedfalse
deleting_attrue
idtrue
last_used_atfalse
locked_attrue
nametrue
organization_idfalse
owner_idtrue
template_idtrue
ttltrue
updated_atfalse
| | WorkspaceBuild
start, stop |
FieldTracked
build_numberfalse
created_atfalse
daily_costfalse
deadlinefalse
idfalse
initiator_idfalse
job_idfalse
max_deadlinefalse
provisioner_statefalse
reasonfalse
template_version_idtrue
transitionfalse
updated_atfalse
workspace_idfalse
| | WorkspaceProxy
|
FieldTracked
created_attrue
deletedfalse
display_nametrue
icontrue
idtrue
nametrue
token_hashed_secrettrue
updated_atfalse
urltrue
wildcard_hostnametrue
| diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 1ae7648456eef..a775e28583c8e 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -126,6 +126,7 @@ var auditableResourcesTypes = map[any]map[string]Action{ "ttl": ActionTrack, "last_used_at": ActionIgnore, "locked_at": ActionTrack, + "deleting_at": ActionTrack, }, &database.WorkspaceBuild{}: { "id": ActionIgnore, diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index ec5033d4f690e..9b55d0a7e5a96 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -235,6 +235,78 @@ func TestTemplates(t *testing.T) { require.Equal(t, inactivityTTL, updated.InactivityTTLMillis) require.Equal(t, lockedTTL, updated.LockedTTLMillis) }) + + t.Run("UpdateLockedTTL", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + client, user := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + IncludeProvisionerDaemon: true, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureAdvancedTemplateScheduling: 1, + }, + }, + }) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + + unlockedWorkspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + lockedWorkspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + require.Nil(t, unlockedWorkspace.DeletingAt) + require.Nil(t, lockedWorkspace.DeletingAt) + + _ = coderdtest.AwaitWorkspaceBuildJob(t, client, unlockedWorkspace.LatestBuild.ID) + _ = coderdtest.AwaitWorkspaceBuildJob(t, client, lockedWorkspace.LatestBuild.ID) + + err := client.UpdateWorkspaceLock(ctx, lockedWorkspace.ID, codersdk.UpdateWorkspaceLock{ + Lock: true, + }) + require.NoError(t, err) + + lockedWorkspace = coderdtest.MustWorkspace(t, client, lockedWorkspace.ID) + require.NotNil(t, lockedWorkspace.LockedAt) + // The deleting_at field should be nil since there is no template locked_ttl set. + require.Nil(t, lockedWorkspace.DeletingAt) + + lockedTTL := time.Minute + updated, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ + LockedTTLMillis: lockedTTL.Milliseconds(), + }) + require.NoError(t, err) + require.Equal(t, lockedTTL.Milliseconds(), updated.LockedTTLMillis) + + unlockedWorkspace = coderdtest.MustWorkspace(t, client, unlockedWorkspace.ID) + require.Nil(t, unlockedWorkspace.LockedAt) + require.Nil(t, unlockedWorkspace.DeletingAt) + + lockedWorkspace = coderdtest.MustWorkspace(t, client, lockedWorkspace.ID) + require.NotNil(t, lockedWorkspace.LockedAt) + require.NotNil(t, lockedWorkspace.DeletingAt) + require.Equal(t, lockedWorkspace.LockedAt.Add(lockedTTL), *lockedWorkspace.DeletingAt) + + // Disable the locked_ttl on the template, then we can assert that the workspaces + // no longer have a deleting_at field. + updated, err = client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ + LockedTTLMillis: 0, + }) + require.NoError(t, err) + require.EqualValues(t, 0, updated.LockedTTLMillis) + + // The unlocked workspace should remain unchanged. + unlockedWorkspace = coderdtest.MustWorkspace(t, client, unlockedWorkspace.ID) + require.Nil(t, unlockedWorkspace.LockedAt) + require.Nil(t, unlockedWorkspace.DeletingAt) + + // Fetch the locked workspace. It should still be locked, but it should no + // longer be scheduled for deletion. + lockedWorkspace = coderdtest.MustWorkspace(t, client, lockedWorkspace.ID) + require.NotNil(t, lockedWorkspace.LockedAt) + require.Nil(t, lockedWorkspace.DeletingAt) + }) } func TestTemplateACL(t *testing.T) { diff --git a/enterprise/coderd/templateschedule.go b/enterprise/coderd/templateschedule.go new file mode 100644 index 0000000000000..bbe3735a9d066 --- /dev/null +++ b/enterprise/coderd/templateschedule.go @@ -0,0 +1,107 @@ +package coderd + +import ( + "context" + "time" + + "github.com/google/uuid" + "golang.org/x/xerrors" + + "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/schedule" +) + +type EnterpriseTemplateScheduleStore struct{} + +var _ schedule.TemplateScheduleStore = &EnterpriseTemplateScheduleStore{} + +func (*EnterpriseTemplateScheduleStore) GetTemplateScheduleOptions(ctx context.Context, db database.Store, templateID uuid.UUID) (schedule.TemplateScheduleOptions, error) { + tpl, err := db.GetTemplateByID(ctx, templateID) + if err != nil { + return schedule.TemplateScheduleOptions{}, err + } + + return schedule.TemplateScheduleOptions{ + UserAutostartEnabled: tpl.AllowUserAutostart, + UserAutostopEnabled: tpl.AllowUserAutostop, + DefaultTTL: time.Duration(tpl.DefaultTTL), + MaxTTL: time.Duration(tpl.MaxTTL), + FailureTTL: time.Duration(tpl.FailureTTL), + InactivityTTL: time.Duration(tpl.InactivityTTL), + LockedTTL: time.Duration(tpl.LockedTTL), + }, nil +} + +func (*EnterpriseTemplateScheduleStore) SetTemplateScheduleOptions(ctx context.Context, db database.Store, tpl database.Template, opts schedule.TemplateScheduleOptions) (database.Template, error) { + if int64(opts.DefaultTTL) == tpl.DefaultTTL && + int64(opts.MaxTTL) == tpl.MaxTTL && + int64(opts.FailureTTL) == tpl.FailureTTL && + int64(opts.InactivityTTL) == tpl.InactivityTTL && + int64(opts.LockedTTL) == tpl.LockedTTL && + opts.UserAutostartEnabled == tpl.AllowUserAutostart && + opts.UserAutostopEnabled == tpl.AllowUserAutostop { + // Avoid updating the UpdatedAt timestamp if nothing will be changed. + return tpl, nil + } + + var template database.Template + err := db.InTx(func(db database.Store) error { + err := db.UpdateTemplateScheduleByID(ctx, database.UpdateTemplateScheduleByIDParams{ + ID: tpl.ID, + UpdatedAt: database.Now(), + AllowUserAutostart: opts.UserAutostartEnabled, + AllowUserAutostop: opts.UserAutostopEnabled, + DefaultTTL: int64(opts.DefaultTTL), + MaxTTL: int64(opts.MaxTTL), + FailureTTL: int64(opts.FailureTTL), + InactivityTTL: int64(opts.InactivityTTL), + LockedTTL: int64(opts.LockedTTL), + }) + if err != nil { + return xerrors.Errorf("update template schedule: %w", err) + } + + // Update all workspaces using the template to set the user defined schedule + // to be within the new bounds. This essentially does the following for each + // workspace using the template. + // if (template.ttl != NULL) { + // workspace.ttl = min(workspace.ttl, template.ttl) + // } + // + // NOTE: this does not apply to currently running workspaces as their + // schedule information is committed to the workspace_build during start. + // This limitation is displayed to the user while editing the template. + if opts.MaxTTL > 0 { + err = db.UpdateWorkspaceTTLToBeWithinTemplateMax(ctx, database.UpdateWorkspaceTTLToBeWithinTemplateMaxParams{ + TemplateID: tpl.ID, + TemplateMaxTTL: int64(opts.MaxTTL), + }) + if err != nil { + return xerrors.Errorf("update TTL of all workspaces on template to be within new template max TTL: %w", err) + } + } + + // If we updated the locked_ttl we need to update all the workspaces deleting_at + // to ensure workspaces are being cleaned up correctly. Similarly if we are + // disabling it (by passing 0), then we want to delete nullify the deleting_at + // fields of all the template workspaces. + err = db.UpdateWorkspacesDeletingAtByTemplateID(ctx, database.UpdateWorkspacesDeletingAtByTemplateIDParams{ + TemplateID: tpl.ID, + LockedTtlMs: opts.LockedTTL.Milliseconds(), + }) + if err != nil { + return xerrors.Errorf("update deleting_at of all workspaces for new locked_ttl %q: %w", opts.LockedTTL, err) + } + + template, err = db.GetTemplateByID(ctx, tpl.ID) + if err != nil { + return xerrors.Errorf("get updated template schedule: %w", err) + } + return nil + }, nil) + if err != nil { + return database.Template{}, xerrors.Errorf("in tx: %w", err) + } + + return template, nil +} diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 8220678333d19..24564a654c234 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -7,7 +7,6 @@ import ( "testing" "time" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "cdr.dev/slog/sloggers/slogtest" @@ -625,10 +624,10 @@ func TestWorkspaceAutobuild(t *testing.T) { func TestWorkspacesFiltering(t *testing.T) { t.Parallel() - t.Run("FilterQueryHasDeletingByAndLicensed", func(t *testing.T) { + t.Run("DeletingBy", func(t *testing.T) { t.Parallel() - inactivityTTL := 1 * 24 * time.Hour + lockedTTL := 24 * time.Hour client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ @@ -642,34 +641,103 @@ func TestWorkspacesFiltering(t *testing.T) { }) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - // update template with inactivity ttl ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() template, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ - InactivityTTLMillis: inactivityTTL.Milliseconds(), + LockedTTLMillis: lockedTTL.Milliseconds(), }) - - assert.NoError(t, err) - assert.Equal(t, inactivityTTL.Milliseconds(), template.InactivityTTLMillis) + require.NoError(t, err) + require.Equal(t, lockedTTL.Milliseconds(), template.LockedTTLMillis) workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + _ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) // stop build so workspace is inactive stopBuild := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStop) coderdtest.AwaitWorkspaceBuildJob(t, client, stopBuild.ID) + err = client.UpdateWorkspaceLock(ctx, workspace.ID, codersdk.UpdateWorkspaceLock{ + Lock: true, + }) + require.NoError(t, err) + workspace = coderdtest.MustWorkspace(t, client, workspace.ID) + require.NotNil(t, workspace.DeletingAt) res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ // adding a second to time.Now() to give some buffer in case test runs quickly - FilterQuery: fmt.Sprintf("deleting_by:%s", time.Now().Add(time.Second).Add(inactivityTTL).Format("2006-01-02")), + FilterQuery: fmt.Sprintf("deleting_by:%s", time.Now().Add(time.Second).Add(lockedTTL).Format("2006-01-02")), + }) + require.NoError(t, err) + require.Len(t, res.Workspaces, 1) + require.Equal(t, workspace.ID, res.Workspaces[0].ID) + }) +} + +func TestWorkspaceLock(t *testing.T) { + t.Parallel() + + t.Run("TemplateLockedTTL", func(t *testing.T) { + t.Parallel() + var ( + client, user = coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + IncludeProvisionerDaemon: true, + TemplateScheduleStore: &coderd.EnterpriseTemplateScheduleStore{}, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureAdvancedTemplateScheduling: 1, + }, + }, + }) + + version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + lockedTTL = time.Minute + ) + + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.LockedTTLMillis = ptr.Ref[int64](lockedTTL.Milliseconds()) + }) + + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + _ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + lastUsedAt := workspace.LastUsedAt + err := client.UpdateWorkspaceLock(ctx, workspace.ID, codersdk.UpdateWorkspaceLock{ + Lock: true, + }) + require.NoError(t, err) + + workspace = coderdtest.MustWorkspace(t, client, workspace.ID) + require.NoError(t, err, "fetch provisioned workspace") + require.NotNil(t, workspace.DeletingAt) + require.NotNil(t, workspace.LockedAt) + require.Equal(t, workspace.LockedAt.Add(lockedTTL), *workspace.DeletingAt) + require.WithinRange(t, *workspace.LockedAt, time.Now().Add(-time.Second*10), time.Now()) + // Locking a workspace shouldn't update the last_used_at. + require.Equal(t, lastUsedAt, workspace.LastUsedAt) + + workspace = coderdtest.MustWorkspace(t, client, workspace.ID) + lastUsedAt = workspace.LastUsedAt + err = client.UpdateWorkspaceLock(ctx, workspace.ID, codersdk.UpdateWorkspaceLock{ + Lock: false, }) - assert.NoError(t, err) - assert.Len(t, res.Workspaces, 1) - assert.Equal(t, workspace.ID, res.Workspaces[0].ID) + require.NoError(t, err) + + workspace, err = client.Workspace(ctx, workspace.ID) + require.NoError(t, err, "fetch provisioned workspace") + require.Nil(t, workspace.LockedAt) + // Unlocking a workspace should cause the deleting_at to be unset. + require.Nil(t, workspace.DeletingAt) + // The last_used_at should get updated when we unlock the workspace. + require.True(t, workspace.LastUsedAt.After(lastUsedAt)) }) } From fd49248c0e69701f02c7ce54f5ca7944a455da88 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Fri, 21 Jul 2023 01:45:51 +0000 Subject: [PATCH 2/2] fix some merge woes --- coderd/database/dbfake/dbfake.go | 1 + enterprise/coderd/schedule/template.go | 13 ++- enterprise/coderd/templateschedule.go | 107 ------------------------- enterprise/coderd/workspaces_test.go | 2 +- 4 files changed, 14 insertions(+), 109 deletions(-) delete mode 100644 enterprise/coderd/templateschedule.go diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 99c2c9d10b489..ed147d6488f72 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -4997,6 +4997,7 @@ func (q *FakeQuerier) UpsertLastUpdateCheck(_ context.Context, data string) erro defer q.mutex.Unlock() q.lastUpdateCheck = []byte(data) + return nil } func (q *FakeQuerier) UpsertLogoURL(_ context.Context, data string) error { diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index d65d436020b8f..278e315dda3af 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -103,8 +103,19 @@ func (*EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.Sto return xerrors.Errorf("update template schedule: %w", err) } - // TODO: update all workspace max_deadlines to be within new bounds + // If we updated the locked_ttl we need to update all the workspaces deleting_at + // to ensure workspaces are being cleaned up correctly. Similarly if we are + // disabling it (by passing 0), then we want to delete nullify the deleting_at + // fields of all the template workspaces. + err = db.UpdateWorkspacesDeletingAtByTemplateID(ctx, database.UpdateWorkspacesDeletingAtByTemplateIDParams{ + TemplateID: tpl.ID, + LockedTtlMs: opts.LockedTTL.Milliseconds(), + }) + if err != nil { + return xerrors.Errorf("update deleting_at of all workspaces for new locked_ttl %q: %w", opts.LockedTTL, err) + } + // TODO: update all workspace max_deadlines to be within new bounds template, err = db.GetTemplateByID(ctx, tpl.ID) if err != nil { return xerrors.Errorf("get updated template schedule: %w", err) diff --git a/enterprise/coderd/templateschedule.go b/enterprise/coderd/templateschedule.go deleted file mode 100644 index bbe3735a9d066..0000000000000 --- a/enterprise/coderd/templateschedule.go +++ /dev/null @@ -1,107 +0,0 @@ -package coderd - -import ( - "context" - "time" - - "github.com/google/uuid" - "golang.org/x/xerrors" - - "github.com/coder/coder/coderd/database" - "github.com/coder/coder/coderd/schedule" -) - -type EnterpriseTemplateScheduleStore struct{} - -var _ schedule.TemplateScheduleStore = &EnterpriseTemplateScheduleStore{} - -func (*EnterpriseTemplateScheduleStore) GetTemplateScheduleOptions(ctx context.Context, db database.Store, templateID uuid.UUID) (schedule.TemplateScheduleOptions, error) { - tpl, err := db.GetTemplateByID(ctx, templateID) - if err != nil { - return schedule.TemplateScheduleOptions{}, err - } - - return schedule.TemplateScheduleOptions{ - UserAutostartEnabled: tpl.AllowUserAutostart, - UserAutostopEnabled: tpl.AllowUserAutostop, - DefaultTTL: time.Duration(tpl.DefaultTTL), - MaxTTL: time.Duration(tpl.MaxTTL), - FailureTTL: time.Duration(tpl.FailureTTL), - InactivityTTL: time.Duration(tpl.InactivityTTL), - LockedTTL: time.Duration(tpl.LockedTTL), - }, nil -} - -func (*EnterpriseTemplateScheduleStore) SetTemplateScheduleOptions(ctx context.Context, db database.Store, tpl database.Template, opts schedule.TemplateScheduleOptions) (database.Template, error) { - if int64(opts.DefaultTTL) == tpl.DefaultTTL && - int64(opts.MaxTTL) == tpl.MaxTTL && - int64(opts.FailureTTL) == tpl.FailureTTL && - int64(opts.InactivityTTL) == tpl.InactivityTTL && - int64(opts.LockedTTL) == tpl.LockedTTL && - opts.UserAutostartEnabled == tpl.AllowUserAutostart && - opts.UserAutostopEnabled == tpl.AllowUserAutostop { - // Avoid updating the UpdatedAt timestamp if nothing will be changed. - return tpl, nil - } - - var template database.Template - err := db.InTx(func(db database.Store) error { - err := db.UpdateTemplateScheduleByID(ctx, database.UpdateTemplateScheduleByIDParams{ - ID: tpl.ID, - UpdatedAt: database.Now(), - AllowUserAutostart: opts.UserAutostartEnabled, - AllowUserAutostop: opts.UserAutostopEnabled, - DefaultTTL: int64(opts.DefaultTTL), - MaxTTL: int64(opts.MaxTTL), - FailureTTL: int64(opts.FailureTTL), - InactivityTTL: int64(opts.InactivityTTL), - LockedTTL: int64(opts.LockedTTL), - }) - if err != nil { - return xerrors.Errorf("update template schedule: %w", err) - } - - // Update all workspaces using the template to set the user defined schedule - // to be within the new bounds. This essentially does the following for each - // workspace using the template. - // if (template.ttl != NULL) { - // workspace.ttl = min(workspace.ttl, template.ttl) - // } - // - // NOTE: this does not apply to currently running workspaces as their - // schedule information is committed to the workspace_build during start. - // This limitation is displayed to the user while editing the template. - if opts.MaxTTL > 0 { - err = db.UpdateWorkspaceTTLToBeWithinTemplateMax(ctx, database.UpdateWorkspaceTTLToBeWithinTemplateMaxParams{ - TemplateID: tpl.ID, - TemplateMaxTTL: int64(opts.MaxTTL), - }) - if err != nil { - return xerrors.Errorf("update TTL of all workspaces on template to be within new template max TTL: %w", err) - } - } - - // If we updated the locked_ttl we need to update all the workspaces deleting_at - // to ensure workspaces are being cleaned up correctly. Similarly if we are - // disabling it (by passing 0), then we want to delete nullify the deleting_at - // fields of all the template workspaces. - err = db.UpdateWorkspacesDeletingAtByTemplateID(ctx, database.UpdateWorkspacesDeletingAtByTemplateIDParams{ - TemplateID: tpl.ID, - LockedTtlMs: opts.LockedTTL.Milliseconds(), - }) - if err != nil { - return xerrors.Errorf("update deleting_at of all workspaces for new locked_ttl %q: %w", opts.LockedTTL, err) - } - - template, err = db.GetTemplateByID(ctx, tpl.ID) - if err != nil { - return xerrors.Errorf("get updated template schedule: %w", err) - } - return nil - }, nil) - if err != nil { - return database.Template{}, xerrors.Errorf("in tx: %w", err) - } - - return template, nil -} diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 24564a654c234..07e501032f9e0 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -686,7 +686,7 @@ func TestWorkspaceLock(t *testing.T) { client, user = coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, - TemplateScheduleStore: &coderd.EnterpriseTemplateScheduleStore{}, + TemplateScheduleStore: &schedule.EnterpriseTemplateScheduleStore{}, }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{