From 78d9bef493133cf735494cc5594b7847a2818d35 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 18 Nov 2024 10:47:29 +0000 Subject: [PATCH 01/30] fix: begin impl of reducing autostart false-positive rate --- coderd/database/dbauthz/dbauthz.go | 7 ++ coderd/database/dbauthz/dbauthz_test.go | 7 ++ coderd/database/dbmem/dbmem.go | 23 ++++++ coderd/database/dbmetrics/querymetrics.go | 7 ++ coderd/database/dbmock/dbmock.go | 14 ++++ coderd/database/dump.sql | 4 +- ..._cron_type_for_autostart_schedule.down.sql | 37 +++++++++ ...te_cron_type_for_autostart_schedule.up.sql | 38 ++++++++++ coderd/database/modelmethods.go | 2 + coderd/database/modelqueries.go | 1 + coderd/database/models.go | 4 +- coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 75 ++++++++++++++----- coderd/database/queries/workspaces.sql | 24 +++++- .../provisionerdserver/provisionerdserver.go | 21 +++++- coderd/workspaces.go | 15 ++++ docs/admin/security/audit-logs.md | 2 +- enterprise/audit/table.go | 1 + enterprise/coderd/schedule/template.go | 18 +++++ 19 files changed, 276 insertions(+), 25 deletions(-) create mode 100644 coderd/database/migrations/000276_create_cron_type_for_autostart_schedule.down.sql create mode 100644 coderd/database/migrations/000276_create_cron_type_for_autostart_schedule.up.sql diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 4845ff22288fe..976e5da36dc0c 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -4044,6 +4044,13 @@ 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) UpdateWorkspaceNextStartAt(ctx context.Context, arg database.UpdateWorkspaceNextStartAtParams) error { + fetch := func(ctx context.Context, arg database.UpdateWorkspaceNextStartAtParams) (database.Workspace, error) { + return q.db.GetWorkspaceByID(ctx, arg.ID) + } + return update(q.log, q.auth, fetch, q.db.UpdateWorkspaceNextStartAt)(ctx, arg) +} + func (q *querier) UpdateWorkspaceProxy(ctx context.Context, arg database.UpdateWorkspaceProxyParams) (database.WorkspaceProxy, error) { fetch := func(ctx context.Context, arg database.UpdateWorkspaceProxyParams) (database.WorkspaceProxy, error) { return q.db.GetWorkspaceProxyByID(ctx, arg.ID) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 2eb75f8b738c4..0fe4f1e8fc58f 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1887,6 +1887,13 @@ func (s *MethodTestSuite) TestWorkspace() { ID: ws.ID, }).Asserts(ws, policy.ActionUpdate).Returns() })) + s.Run("UpdateWorkspaceNextStartAt", s.Subtest(func(db database.Store, check *expects) { + ws := dbgen.Workspace(s.T(), db, database.WorkspaceTable{}) + check.Args(database.UpdateWorkspaceNextStartAtParams{ + ID: ws.ID, + NextStartAt: sql.NullTime{Valid: true, Time: dbtime.Now()}, + }).Asserts(ws, policy.ActionUpdate) + })) s.Run("BatchUpdateWorkspaceLastUsedAt", s.Subtest(func(db database.Store, check *expects) { ws1 := dbgen.Workspace(s.T(), db, database.WorkspaceTable{}) ws2 := dbgen.Workspace(s.T(), db, database.WorkspaceTable{}) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index aed57e9284b3a..a2e8027697871 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -9950,6 +9950,29 @@ func (q *FakeQuerier) UpdateWorkspaceLastUsedAt(_ context.Context, arg database. return sql.ErrNoRows } +func (q *FakeQuerier) UpdateWorkspaceNextStartAt(ctx context.Context, arg database.UpdateWorkspaceNextStartAtParams) error { + err := validateDatabaseType(arg) + if err != nil { + return err + } + + q.mutex.Lock() + defer q.mutex.Unlock() + + for index, workspace := range q.workspaces { + if workspace.ID != arg.ID { + continue + } + + workspace.NextStartAt = arg.NextStartAt + q.workspaces[index] = workspace + + return nil + } + + return sql.ErrNoRows +} + func (q *FakeQuerier) UpdateWorkspaceProxy(_ context.Context, arg database.UpdateWorkspaceProxyParams) (database.WorkspaceProxy, error) { q.mutex.Lock() defer q.mutex.Unlock() diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 32d3cce658525..27eafa7db4f2f 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -2527,6 +2527,13 @@ func (m queryMetricsStore) UpdateWorkspaceLastUsedAt(ctx context.Context, arg da return err } +func (m queryMetricsStore) UpdateWorkspaceNextStartAt(ctx context.Context, arg database.UpdateWorkspaceNextStartAtParams) error { + start := time.Now() + r0 := m.s.UpdateWorkspaceNextStartAt(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateWorkspaceNextStartAt").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) UpdateWorkspaceProxy(ctx context.Context, arg database.UpdateWorkspaceProxyParams) (database.WorkspaceProxy, error) { start := time.Now() proxy, err := m.s.UpdateWorkspaceProxy(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index d6c34411f8208..875b6e7981a45 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -5356,6 +5356,20 @@ func (mr *MockStoreMockRecorder) UpdateWorkspaceLastUsedAt(arg0, arg1 any) *gomo return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateWorkspaceLastUsedAt", reflect.TypeOf((*MockStore)(nil).UpdateWorkspaceLastUsedAt), arg0, arg1) } +// UpdateWorkspaceNextStartAt mocks base method. +func (m *MockStore) UpdateWorkspaceNextStartAt(arg0 context.Context, arg1 database.UpdateWorkspaceNextStartAtParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateWorkspaceNextStartAt", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateWorkspaceNextStartAt indicates an expected call of UpdateWorkspaceNextStartAt. +func (mr *MockStoreMockRecorder) UpdateWorkspaceNextStartAt(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateWorkspaceNextStartAt", reflect.TypeOf((*MockStore)(nil).UpdateWorkspaceNextStartAt), arg0, arg1) +} + // UpdateWorkspaceProxy mocks base method. func (m *MockStore) UpdateWorkspaceProxy(arg0 context.Context, arg1 database.UpdateWorkspaceProxyParams) (database.WorkspaceProxy, error) { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 9919011579bde..9874e8d38d61e 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1729,7 +1729,8 @@ CREATE TABLE workspaces ( dormant_at timestamp with time zone, deleting_at timestamp with time zone, automatic_updates automatic_updates DEFAULT 'never'::automatic_updates NOT NULL, - favorite boolean DEFAULT false NOT NULL + favorite boolean DEFAULT false NOT NULL, + next_start_at timestamp with time zone ); COMMENT ON COLUMN workspaces.favorite IS 'Favorite is true if the workspace owner has favorited the workspace.'; @@ -1750,6 +1751,7 @@ CREATE VIEW workspaces_expanded AS workspaces.deleting_at, workspaces.automatic_updates, workspaces.favorite, + workspaces.next_start_at, visible_users.avatar_url AS owner_avatar_url, visible_users.username AS owner_username, organizations.name AS organization_name, diff --git a/coderd/database/migrations/000276_create_cron_type_for_autostart_schedule.down.sql b/coderd/database/migrations/000276_create_cron_type_for_autostart_schedule.down.sql new file mode 100644 index 0000000000000..705df03948a1f --- /dev/null +++ b/coderd/database/migrations/000276_create_cron_type_for_autostart_schedule.down.sql @@ -0,0 +1,37 @@ +DROP VIEW workspaces_expanded; + +ALTER TABLE ONLY workspaces DROP COLUMN IF EXISTS next_start_at; + +CREATE VIEW + workspaces_expanded +AS +SELECT + workspaces.*, + -- Owner + visible_users.avatar_url AS owner_avatar_url, + visible_users.username AS owner_username, + -- Organization + organizations.name AS organization_name, + organizations.display_name AS organization_display_name, + organizations.icon AS organization_icon, + organizations.description AS organization_description, + -- Template + templates.name AS template_name, + templates.display_name AS template_display_name, + templates.icon AS template_icon, + templates.description AS template_description +FROM + workspaces + INNER JOIN + visible_users + ON + workspaces.owner_id = visible_users.id + INNER JOIN + organizations + ON workspaces.organization_id = organizations.id + INNER JOIN + templates + ON workspaces.template_id = templates.id +; + +COMMENT ON VIEW workspaces_expanded IS 'Joins in the display name information such as username, avatar, and organization name.'; diff --git a/coderd/database/migrations/000276_create_cron_type_for_autostart_schedule.up.sql b/coderd/database/migrations/000276_create_cron_type_for_autostart_schedule.up.sql new file mode 100644 index 0000000000000..2abab266fc477 --- /dev/null +++ b/coderd/database/migrations/000276_create_cron_type_for_autostart_schedule.up.sql @@ -0,0 +1,38 @@ +ALTER TABLE ONLY workspaces ADD COLUMN IF NOT EXISTS next_start_at TIMESTAMPTZ DEFAULT NULL; + +-- Recreate view +DROP VIEW workspaces_expanded; + +CREATE VIEW + workspaces_expanded +AS +SELECT + workspaces.*, + -- Owner + visible_users.avatar_url AS owner_avatar_url, + visible_users.username AS owner_username, + -- Organization + organizations.name AS organization_name, + organizations.display_name AS organization_display_name, + organizations.icon AS organization_icon, + organizations.description AS organization_description, + -- Template + templates.name AS template_name, + templates.display_name AS template_display_name, + templates.icon AS template_icon, + templates.description AS template_description +FROM + workspaces + INNER JOIN + visible_users + ON + workspaces.owner_id = visible_users.id + INNER JOIN + organizations + ON workspaces.organization_id = organizations.id + INNER JOIN + templates + ON workspaces.template_id = templates.id +; + +COMMENT ON VIEW workspaces_expanded IS 'Joins in the display name information such as username, avatar, and organization name.'; diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index a74ddf29bfcf9..002c48a9b4f81 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -214,6 +214,7 @@ func (w Workspace) WorkspaceTable() WorkspaceTable { DeletingAt: w.DeletingAt, AutomaticUpdates: w.AutomaticUpdates, Favorite: w.Favorite, + NextStartAt: w.NextStartAt, } } @@ -438,6 +439,7 @@ func ConvertWorkspaceRows(rows []GetWorkspacesRow) []Workspace { TemplateDisplayName: r.TemplateDisplayName, TemplateIcon: r.TemplateIcon, TemplateDescription: r.TemplateDescription, + NextStartAt: r.NextStartAt, } } diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index ff77012755fa2..2a61f339398f2 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -290,6 +290,7 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa &i.DeletingAt, &i.AutomaticUpdates, &i.Favorite, + &i.NextStartAt, &i.OwnerAvatarUrl, &i.OwnerUsername, &i.OrganizationName, diff --git a/coderd/database/models.go b/coderd/database/models.go index af0a3122f7964..278b283bf00c2 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -2920,6 +2920,7 @@ type Workspace struct { DeletingAt sql.NullTime `db:"deleting_at" json:"deleting_at"` AutomaticUpdates AutomaticUpdates `db:"automatic_updates" json:"automatic_updates"` Favorite bool `db:"favorite" json:"favorite"` + NextStartAt sql.NullTime `db:"next_start_at" json:"next_start_at"` OwnerAvatarUrl string `db:"owner_avatar_url" json:"owner_avatar_url"` OwnerUsername string `db:"owner_username" json:"owner_username"` OrganizationName string `db:"organization_name" json:"organization_name"` @@ -3223,5 +3224,6 @@ type WorkspaceTable struct { DeletingAt sql.NullTime `db:"deleting_at" json:"deleting_at"` AutomaticUpdates AutomaticUpdates `db:"automatic_updates" json:"automatic_updates"` // Favorite is true if the workspace owner has favorited the workspace. - Favorite bool `db:"favorite" json:"favorite"` + Favorite bool `db:"favorite" json:"favorite"` + NextStartAt sql.NullTime `db:"next_start_at" json:"next_start_at"` } diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 49ba6fbf8496a..c786d73d4e55b 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -491,6 +491,7 @@ type sqlcQuerier interface { UpdateWorkspaceDeletedByID(ctx context.Context, arg UpdateWorkspaceDeletedByIDParams) error UpdateWorkspaceDormantDeletingAt(ctx context.Context, arg UpdateWorkspaceDormantDeletingAtParams) (WorkspaceTable, error) UpdateWorkspaceLastUsedAt(ctx context.Context, arg UpdateWorkspaceLastUsedAtParams) error + UpdateWorkspaceNextStartAt(ctx context.Context, arg UpdateWorkspaceNextStartAtParams) 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 diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 09dd4c1fbc488..3a6ba4554e24a 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -11134,7 +11134,7 @@ func (q *sqlQuerier) DeleteOldWorkspaceAgentLogs(ctx context.Context, threshold const getWorkspaceAgentAndLatestBuildByAuthToken = `-- name: GetWorkspaceAgentAndLatestBuildByAuthToken :one 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.dormant_at, workspaces.deleting_at, workspaces.automatic_updates, workspaces.favorite, + 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.dormant_at, workspaces.deleting_at, workspaces.automatic_updates, workspaces.favorite, workspaces.next_start_at, workspace_agents.id, workspace_agents.created_at, workspace_agents.updated_at, workspace_agents.name, workspace_agents.first_connected_at, workspace_agents.last_connected_at, workspace_agents.disconnected_at, workspace_agents.resource_id, workspace_agents.auth_token, workspace_agents.auth_instance_id, workspace_agents.architecture, workspace_agents.environment_variables, workspace_agents.operating_system, workspace_agents.instance_metadata, workspace_agents.resource_metadata, workspace_agents.directory, workspace_agents.version, workspace_agents.last_connected_replica_id, workspace_agents.connection_timeout_seconds, workspace_agents.troubleshooting_url, workspace_agents.motd_file, workspace_agents.lifecycle_state, workspace_agents.expanded_directory, workspace_agents.logs_length, workspace_agents.logs_overflowed, workspace_agents.started_at, workspace_agents.ready_at, workspace_agents.subsystems, workspace_agents.display_apps, workspace_agents.api_version, workspace_agents.display_order, workspace_build_with_user.id, workspace_build_with_user.created_at, workspace_build_with_user.updated_at, workspace_build_with_user.workspace_id, workspace_build_with_user.template_version_id, workspace_build_with_user.build_number, workspace_build_with_user.transition, workspace_build_with_user.initiator_id, workspace_build_with_user.provisioner_state, workspace_build_with_user.job_id, workspace_build_with_user.deadline, workspace_build_with_user.reason, workspace_build_with_user.daily_cost, workspace_build_with_user.max_deadline, workspace_build_with_user.initiator_by_avatar_url, workspace_build_with_user.initiator_by_username FROM @@ -11193,6 +11193,7 @@ func (q *sqlQuerier) GetWorkspaceAgentAndLatestBuildByAuthToken(ctx context.Cont &i.WorkspaceTable.DeletingAt, &i.WorkspaceTable.AutomaticUpdates, &i.WorkspaceTable.Favorite, + &i.WorkspaceTable.NextStartAt, &i.WorkspaceAgent.ID, &i.WorkspaceAgent.CreatedAt, &i.WorkspaceAgent.UpdatedAt, @@ -14721,7 +14722,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, dormant_at, deleting_at, automatic_updates, favorite, owner_avatar_url, owner_username, organization_name, organization_display_name, organization_icon, organization_description, template_name, template_display_name, template_icon, template_description + id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite, next_start_at, owner_avatar_url, owner_username, organization_name, organization_display_name, organization_icon, organization_description, template_name, template_display_name, template_icon, template_description FROM workspaces_expanded as workspaces WHERE @@ -14768,6 +14769,7 @@ func (q *sqlQuerier) GetWorkspaceByAgentID(ctx context.Context, agentID uuid.UUI &i.DeletingAt, &i.AutomaticUpdates, &i.Favorite, + &i.NextStartAt, &i.OwnerAvatarUrl, &i.OwnerUsername, &i.OrganizationName, @@ -14784,7 +14786,7 @@ func (q *sqlQuerier) GetWorkspaceByAgentID(ctx context.Context, agentID uuid.UUI 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, dormant_at, deleting_at, automatic_updates, favorite, owner_avatar_url, owner_username, organization_name, organization_display_name, organization_icon, organization_description, template_name, template_display_name, template_icon, template_description + id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite, next_start_at, owner_avatar_url, owner_username, organization_name, organization_display_name, organization_icon, organization_description, template_name, template_display_name, template_icon, template_description FROM workspaces_expanded WHERE @@ -14812,6 +14814,7 @@ func (q *sqlQuerier) GetWorkspaceByID(ctx context.Context, id uuid.UUID) (Worksp &i.DeletingAt, &i.AutomaticUpdates, &i.Favorite, + &i.NextStartAt, &i.OwnerAvatarUrl, &i.OwnerUsername, &i.OrganizationName, @@ -14828,7 +14831,7 @@ func (q *sqlQuerier) GetWorkspaceByID(ctx context.Context, id uuid.UUID) (Worksp 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, dormant_at, deleting_at, automatic_updates, favorite, owner_avatar_url, owner_username, organization_name, organization_display_name, organization_icon, organization_description, template_name, template_display_name, template_icon, template_description + id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite, next_start_at, owner_avatar_url, owner_username, organization_name, organization_display_name, organization_icon, organization_description, template_name, template_display_name, template_icon, template_description FROM workspaces_expanded as workspaces WHERE @@ -14863,6 +14866,7 @@ func (q *sqlQuerier) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg GetWo &i.DeletingAt, &i.AutomaticUpdates, &i.Favorite, + &i.NextStartAt, &i.OwnerAvatarUrl, &i.OwnerUsername, &i.OrganizationName, @@ -14879,7 +14883,7 @@ func (q *sqlQuerier) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg GetWo 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, dormant_at, deleting_at, automatic_updates, favorite, owner_avatar_url, owner_username, organization_name, organization_display_name, organization_icon, organization_description, template_name, template_display_name, template_icon, template_description + id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite, next_start_at, owner_avatar_url, owner_username, organization_name, organization_display_name, organization_icon, organization_description, template_name, template_display_name, template_icon, template_description FROM workspaces_expanded as workspaces WHERE @@ -14933,6 +14937,7 @@ func (q *sqlQuerier) GetWorkspaceByWorkspaceAppID(ctx context.Context, workspace &i.DeletingAt, &i.AutomaticUpdates, &i.Favorite, + &i.NextStartAt, &i.OwnerAvatarUrl, &i.OwnerUsername, &i.OrganizationName, @@ -14994,7 +14999,7 @@ SELECT ), filtered_workspaces AS ( 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.dormant_at, workspaces.deleting_at, workspaces.automatic_updates, workspaces.favorite, workspaces.owner_avatar_url, workspaces.owner_username, workspaces.organization_name, workspaces.organization_display_name, workspaces.organization_icon, workspaces.organization_description, workspaces.template_name, workspaces.template_display_name, workspaces.template_icon, workspaces.template_description, + 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.dormant_at, workspaces.deleting_at, workspaces.automatic_updates, workspaces.favorite, workspaces.next_start_at, workspaces.owner_avatar_url, workspaces.owner_username, workspaces.organization_name, workspaces.organization_display_name, workspaces.organization_icon, workspaces.organization_description, workspaces.template_name, workspaces.template_display_name, workspaces.template_icon, workspaces.template_description, latest_build.template_version_id, latest_build.template_version_name, latest_build.completed_at as latest_build_completed_at, @@ -15234,7 +15239,7 @@ WHERE -- @authorize_filter ), filtered_workspaces_order AS ( SELECT - fw.id, fw.created_at, fw.updated_at, fw.owner_id, fw.organization_id, fw.template_id, fw.deleted, fw.name, fw.autostart_schedule, fw.ttl, fw.last_used_at, fw.dormant_at, fw.deleting_at, fw.automatic_updates, fw.favorite, fw.owner_avatar_url, fw.owner_username, fw.organization_name, fw.organization_display_name, fw.organization_icon, fw.organization_description, fw.template_name, fw.template_display_name, fw.template_icon, fw.template_description, fw.template_version_id, fw.template_version_name, fw.latest_build_completed_at, fw.latest_build_canceled_at, fw.latest_build_error, fw.latest_build_transition, fw.latest_build_status + fw.id, fw.created_at, fw.updated_at, fw.owner_id, fw.organization_id, fw.template_id, fw.deleted, fw.name, fw.autostart_schedule, fw.ttl, fw.last_used_at, fw.dormant_at, fw.deleting_at, fw.automatic_updates, fw.favorite, fw.next_start_at, fw.owner_avatar_url, fw.owner_username, fw.organization_name, fw.organization_display_name, fw.organization_icon, fw.organization_description, fw.template_name, fw.template_display_name, fw.template_icon, fw.template_description, fw.template_version_id, fw.template_version_name, fw.latest_build_completed_at, fw.latest_build_canceled_at, fw.latest_build_error, fw.latest_build_transition, fw.latest_build_status FROM filtered_workspaces fw ORDER BY @@ -15255,7 +15260,7 @@ WHERE $20 ), filtered_workspaces_order_with_summary AS ( SELECT - fwo.id, fwo.created_at, fwo.updated_at, fwo.owner_id, fwo.organization_id, fwo.template_id, fwo.deleted, fwo.name, fwo.autostart_schedule, fwo.ttl, fwo.last_used_at, fwo.dormant_at, fwo.deleting_at, fwo.automatic_updates, fwo.favorite, fwo.owner_avatar_url, fwo.owner_username, fwo.organization_name, fwo.organization_display_name, fwo.organization_icon, fwo.organization_description, fwo.template_name, fwo.template_display_name, fwo.template_icon, fwo.template_description, fwo.template_version_id, fwo.template_version_name, fwo.latest_build_completed_at, fwo.latest_build_canceled_at, fwo.latest_build_error, fwo.latest_build_transition, fwo.latest_build_status + fwo.id, fwo.created_at, fwo.updated_at, fwo.owner_id, fwo.organization_id, fwo.template_id, fwo.deleted, fwo.name, fwo.autostart_schedule, fwo.ttl, fwo.last_used_at, fwo.dormant_at, fwo.deleting_at, fwo.automatic_updates, fwo.favorite, fwo.next_start_at, fwo.owner_avatar_url, fwo.owner_username, fwo.organization_name, fwo.organization_display_name, fwo.organization_icon, fwo.organization_description, fwo.template_name, fwo.template_display_name, fwo.template_icon, fwo.template_description, fwo.template_version_id, fwo.template_version_name, fwo.latest_build_completed_at, fwo.latest_build_canceled_at, fwo.latest_build_error, fwo.latest_build_transition, fwo.latest_build_status FROM filtered_workspaces_order fwo -- Return a technical summary row with total count of workspaces. @@ -15277,6 +15282,7 @@ WHERE '0001-01-01 00:00:00+00'::timestamptz, -- deleting_at 'never'::automatic_updates, -- automatic_updates false, -- favorite + '0001-01-01 00:00:00+00'::timestamptz, -- next_start_at '', -- owner_avatar_url '', -- owner_username '', -- organization_name @@ -15304,7 +15310,7 @@ WHERE filtered_workspaces ) SELECT - fwos.id, fwos.created_at, fwos.updated_at, fwos.owner_id, fwos.organization_id, fwos.template_id, fwos.deleted, fwos.name, fwos.autostart_schedule, fwos.ttl, fwos.last_used_at, fwos.dormant_at, fwos.deleting_at, fwos.automatic_updates, fwos.favorite, fwos.owner_avatar_url, fwos.owner_username, fwos.organization_name, fwos.organization_display_name, fwos.organization_icon, fwos.organization_description, fwos.template_name, fwos.template_display_name, fwos.template_icon, fwos.template_description, fwos.template_version_id, fwos.template_version_name, fwos.latest_build_completed_at, fwos.latest_build_canceled_at, fwos.latest_build_error, fwos.latest_build_transition, fwos.latest_build_status, + fwos.id, fwos.created_at, fwos.updated_at, fwos.owner_id, fwos.organization_id, fwos.template_id, fwos.deleted, fwos.name, fwos.autostart_schedule, fwos.ttl, fwos.last_used_at, fwos.dormant_at, fwos.deleting_at, fwos.automatic_updates, fwos.favorite, fwos.next_start_at, fwos.owner_avatar_url, fwos.owner_username, fwos.organization_name, fwos.organization_display_name, fwos.organization_icon, fwos.organization_description, fwos.template_name, fwos.template_display_name, fwos.template_icon, fwos.template_description, fwos.template_version_id, fwos.template_version_name, fwos.latest_build_completed_at, fwos.latest_build_canceled_at, fwos.latest_build_error, fwos.latest_build_transition, fwos.latest_build_status, tc.count FROM filtered_workspaces_order_with_summary fwos @@ -15353,6 +15359,7 @@ type GetWorkspacesRow struct { DeletingAt sql.NullTime `db:"deleting_at" json:"deleting_at"` AutomaticUpdates AutomaticUpdates `db:"automatic_updates" json:"automatic_updates"` Favorite bool `db:"favorite" json:"favorite"` + NextStartAt sql.NullTime `db:"next_start_at" json:"next_start_at"` OwnerAvatarUrl string `db:"owner_avatar_url" json:"owner_avatar_url"` OwnerUsername string `db:"owner_username" json:"owner_username"` OrganizationName string `db:"organization_name" json:"organization_name"` @@ -15424,6 +15431,7 @@ func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) &i.DeletingAt, &i.AutomaticUpdates, &i.Favorite, + &i.NextStartAt, &i.OwnerAvatarUrl, &i.OwnerUsername, &i.OrganizationName, @@ -15577,11 +15585,16 @@ WHERE -- * The provisioner job did not fail. -- * The workspace build was a stop transition. -- * The workspace has an autostart schedule. + -- * It is after the workspace's next start time. ( users.status = 'active'::user_status AND provisioner_jobs.job_status != 'failed'::provisioner_job_status AND workspace_builds.transition = 'stop'::workspace_transition AND - workspaces.autostart_schedule IS NOT NULL + workspaces.autostart_schedule IS NOT NULL AND + ( + workspaces.next_start_at IS NULL OR + workspaces.next_start_at <= $1 :: timestamp + ) ) OR -- A workspace may be eligible for dormant stop if the following are true: @@ -15680,10 +15693,11 @@ INSERT INTO autostart_schedule, ttl, last_used_at, - automatic_updates + automatic_updates, + next_start_at ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) RETURNING id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite, next_start_at ` type InsertWorkspaceParams struct { @@ -15698,6 +15712,7 @@ type InsertWorkspaceParams struct { Ttl sql.NullInt64 `db:"ttl" json:"ttl"` LastUsedAt time.Time `db:"last_used_at" json:"last_used_at"` AutomaticUpdates AutomaticUpdates `db:"automatic_updates" json:"automatic_updates"` + NextStartAt sql.NullTime `db:"next_start_at" json:"next_start_at"` } func (q *sqlQuerier) InsertWorkspace(ctx context.Context, arg InsertWorkspaceParams) (WorkspaceTable, error) { @@ -15713,6 +15728,7 @@ func (q *sqlQuerier) InsertWorkspace(ctx context.Context, arg InsertWorkspacePar arg.Ttl, arg.LastUsedAt, arg.AutomaticUpdates, + arg.NextStartAt, ) var i WorkspaceTable err := row.Scan( @@ -15731,6 +15747,7 @@ func (q *sqlQuerier) InsertWorkspace(ctx context.Context, arg InsertWorkspacePar &i.DeletingAt, &i.AutomaticUpdates, &i.Favorite, + &i.NextStartAt, ) return i, err } @@ -15770,7 +15787,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, dormant_at, deleting_at, automatic_updates, favorite +RETURNING id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite, next_start_at ` type UpdateWorkspaceParams struct { @@ -15797,6 +15814,7 @@ func (q *sqlQuerier) UpdateWorkspace(ctx context.Context, arg UpdateWorkspacePar &i.DeletingAt, &i.AutomaticUpdates, &i.Favorite, + &i.NextStartAt, ) return i, err } @@ -15824,7 +15842,8 @@ const updateWorkspaceAutostart = `-- name: UpdateWorkspaceAutostart :exec UPDATE workspaces SET - autostart_schedule = $2 + autostart_schedule = $2, + next_start_at = $3 WHERE id = $1 ` @@ -15832,10 +15851,11 @@ WHERE type UpdateWorkspaceAutostartParams struct { ID uuid.UUID `db:"id" json:"id"` AutostartSchedule sql.NullString `db:"autostart_schedule" json:"autostart_schedule"` + NextStartAt sql.NullTime `db:"next_start_at" json:"next_start_at"` } func (q *sqlQuerier) UpdateWorkspaceAutostart(ctx context.Context, arg UpdateWorkspaceAutostartParams) error { - _, err := q.db.ExecContext(ctx, updateWorkspaceAutostart, arg.ID, arg.AutostartSchedule) + _, err := q.db.ExecContext(ctx, updateWorkspaceAutostart, arg.ID, arg.AutostartSchedule, arg.NextStartAt) return err } @@ -15883,7 +15903,7 @@ WHERE workspaces.id = $1 AND templates.id = workspaces.template_id RETURNING - 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.dormant_at, workspaces.deleting_at, workspaces.automatic_updates, workspaces.favorite + 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.dormant_at, workspaces.deleting_at, workspaces.automatic_updates, workspaces.favorite, workspaces.next_start_at ` type UpdateWorkspaceDormantDeletingAtParams struct { @@ -15910,6 +15930,7 @@ func (q *sqlQuerier) UpdateWorkspaceDormantDeletingAt(ctx context.Context, arg U &i.DeletingAt, &i.AutomaticUpdates, &i.Favorite, + &i.NextStartAt, ) return i, err } @@ -15933,6 +15954,25 @@ func (q *sqlQuerier) UpdateWorkspaceLastUsedAt(ctx context.Context, arg UpdateWo return err } +const updateWorkspaceNextStartAt = `-- name: UpdateWorkspaceNextStartAt :exec +UPDATE + workspaces +SET + next_start_at = $2 +WHERE + id = $1 +` + +type UpdateWorkspaceNextStartAtParams struct { + ID uuid.UUID `db:"id" json:"id"` + NextStartAt sql.NullTime `db:"next_start_at" json:"next_start_at"` +} + +func (q *sqlQuerier) UpdateWorkspaceNextStartAt(ctx context.Context, arg UpdateWorkspaceNextStartAtParams) error { + _, err := q.db.ExecContext(ctx, updateWorkspaceNextStartAt, arg.ID, arg.NextStartAt) + return err +} + const updateWorkspaceTTL = `-- name: UpdateWorkspaceTTL :exec UPDATE workspaces @@ -15965,7 +16005,7 @@ WHERE template_id = $3 AND dormant_at IS NOT NULL -RETURNING id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite +RETURNING id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite, next_start_at ` type UpdateWorkspacesDormantDeletingAtByTemplateIDParams struct { @@ -15999,6 +16039,7 @@ func (q *sqlQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.C &i.DeletingAt, &i.AutomaticUpdates, &i.Favorite, + &i.NextStartAt, ); err != nil { return nil, err } diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 4d200a33f1620..17b022187dcd6 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -368,6 +368,7 @@ WHERE '0001-01-01 00:00:00+00'::timestamptz, -- deleting_at 'never'::automatic_updates, -- automatic_updates false, -- favorite + '0001-01-01 00:00:00+00'::timestamptz, -- next_start_at '', -- owner_avatar_url '', -- owner_username '', -- organization_name @@ -435,10 +436,11 @@ INSERT INTO autostart_schedule, ttl, last_used_at, - automatic_updates + automatic_updates, + next_start_at ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) RETURNING *; + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING *; -- name: UpdateWorkspaceDeletedByID :exec UPDATE @@ -462,7 +464,16 @@ RETURNING *; UPDATE workspaces SET - autostart_schedule = $2 + autostart_schedule = $2, + next_start_at = $3 +WHERE + id = $1; + +-- name: UpdateWorkspaceNextStartAt :exec +UPDATE + workspaces +SET + next_start_at = $2 WHERE id = $1; @@ -601,11 +612,16 @@ WHERE -- * The provisioner job did not fail. -- * The workspace build was a stop transition. -- * The workspace has an autostart schedule. + -- * It is after the workspace's next start time. ( users.status = 'active'::user_status AND provisioner_jobs.job_status != 'failed'::provisioner_job_status AND workspace_builds.transition = 'stop'::workspace_transition AND - workspaces.autostart_schedule IS NOT NULL + workspaces.autostart_schedule IS NOT NULL AND + ( + workspaces.next_start_at IS NULL OR + workspaces.next_start_at <= @now :: timestamp + ) ) OR -- A workspace may be eligible for dormant stop if the following are true: diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 8387f97ea21cb..1421094c7e8d0 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -1432,9 +1432,11 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob) return getWorkspaceError } + templateScheduleStore := *s.TemplateScheduleStore.Load() + autoStop, err := schedule.CalculateAutostop(ctx, schedule.CalculateAutostopParams{ Database: db, - TemplateScheduleStore: *s.TemplateScheduleStore.Load(), + TemplateScheduleStore: templateScheduleStore, UserQuietHoursScheduleStore: *s.UserQuietHoursScheduleStore.Load(), Now: now, Workspace: workspace.WorkspaceTable(), @@ -1445,6 +1447,23 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob) return xerrors.Errorf("calculate auto stop: %w", err) } + if workspace.AutostartSchedule.Valid { + templateScheduleOptions, err := templateScheduleStore.Get(ctx, db, workspace.TemplateID) + if err != nil { + return xerrors.Errorf("get template schedule options: %w", err) + } + + nextStartAt, _ := schedule.NextAutostart(now, workspace.AutostartSchedule.String, templateScheduleOptions) + + err = db.UpdateWorkspaceNextStartAt(ctx, database.UpdateWorkspaceNextStartAtParams{ + ID: workspace.ID, + NextStartAt: sql.NullTime{Valid: true, Time: nextStartAt}, + }) + if err != nil { + return xerrors.Errorf("update workspace next start at: %w", err) + } + } + err = db.UpdateProvisionerJobWithCompleteByID(ctx, database.UpdateProvisionerJobWithCompleteByIDParams{ ID: jobID, UpdatedAt: now, diff --git a/coderd/workspaces.go b/coderd/workspaces.go index ff8a55ded775a..6a6eee4a0bc41 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -29,6 +29,7 @@ import ( "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/searchquery" "github.com/coder/coder/v2/coderd/telemetry" @@ -554,6 +555,12 @@ func createWorkspace( return } + nextStartAt := sql.NullTime{} + if dbAutostartSchedule.Valid { + time, _ := schedule.NextAutostart(dbtime.Now(), dbAutostartSchedule.String, templateSchedule) + nextStartAt = sql.NullTime{Valid: true, Time: time} + } + dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis, templateSchedule.DefaultTTL) if err != nil { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ @@ -618,6 +625,7 @@ func createWorkspace( TemplateID: template.ID, Name: req.Name, AutostartSchedule: dbAutostartSchedule, + NextStartAt: nextStartAt, Ttl: dbTTL, // The workspaces page will sort by last used at, and it's useful to // have the newly created workspace at the top of the list! @@ -873,9 +881,16 @@ func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) { return } + nextStartAt := sql.NullTime{} + if dbSched.Valid { + time, _ := schedule.NextAutostart(dbtime.Now(), dbSched.String, templateSchedule) + nextStartAt = sql.NullTime{Valid: true, Time: time} + } + err = api.Database.UpdateWorkspaceAutostart(ctx, database.UpdateWorkspaceAutostartParams{ ID: workspace.ID, AutostartSchedule: dbSched, + NextStartAt: nextStartAt, }) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ diff --git a/docs/admin/security/audit-logs.md b/docs/admin/security/audit-logs.md index 3ea4e145d13eb..362b3362d3f3b 100644 --- a/docs/admin/security/audit-logs.md +++ b/docs/admin/security/audit-logs.md @@ -28,7 +28,7 @@ We track the following resources: | User
create, write, delete |
FieldTracked
avatar_urlfalse
created_atfalse
deletedtrue
emailtrue
github_com_user_idfalse
hashed_one_time_passcodefalse
hashed_passwordtrue
idtrue
last_seen_atfalse
login_typetrue
nametrue
one_time_passcode_expires_attrue
quiet_hours_scheduletrue
rbac_rolestrue
statustrue
theme_preferencefalse
updated_atfalse
usernametrue
| | WorkspaceBuild
start, stop |
FieldTracked
build_numberfalse
created_atfalse
daily_costfalse
deadlinefalse
idfalse
initiator_by_avatar_urlfalse
initiator_by_usernamefalse
initiator_idfalse
job_idfalse
max_deadlinefalse
provisioner_statefalse
reasonfalse
template_version_idtrue
transitionfalse
updated_atfalse
workspace_idfalse
| | WorkspaceProxy
|
FieldTracked
created_attrue
deletedfalse
derp_enabledtrue
derp_onlytrue
display_nametrue
icontrue
idtrue
nametrue
region_idtrue
token_hashed_secrettrue
updated_atfalse
urltrue
versiontrue
wildcard_hostnametrue
| -| WorkspaceTable
|
FieldTracked
automatic_updatestrue
autostart_scheduletrue
created_atfalse
deletedfalse
deleting_attrue
dormant_attrue
favoritetrue
idtrue
last_used_atfalse
nametrue
organization_idfalse
owner_idtrue
template_idtrue
ttltrue
updated_atfalse
| +| WorkspaceTable
|
FieldTracked
automatic_updatestrue
autostart_scheduletrue
created_atfalse
deletedfalse
deleting_attrue
dormant_attrue
favoritetrue
idtrue
last_used_atfalse
nametrue
next_start_attrue
organization_idfalse
owner_idtrue
template_idtrue
ttltrue
updated_atfalse
| diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index f9e74959f2a28..47bc984dca234 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -164,6 +164,7 @@ var auditableResourcesTypes = map[any]map[string]Action{ "deleting_at": ActionTrack, "automatic_updates": ActionTrack, "favorite": ActionTrack, + "next_start_at": ActionTrack, }, &database.WorkspaceBuild{}: { "id": ActionIgnore, diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index a3f36e08dd218..f8aa18f0fdbcd 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -18,6 +18,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/schedule" agpl "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/codersdk" @@ -304,6 +305,23 @@ func (s *EnterpriseTemplateScheduleStore) updateWorkspaceBuild(ctx context.Conte return xerrors.Errorf("calculate new autostop for workspace %q: %w", workspace.ID, err) } + if workspace.AutostartSchedule.Valid { + templateScheduleOptions, err := s.Get(ctx, db, workspace.TemplateID) + if err != nil { + return xerrors.Errorf("get template schedule options: %w", err) + } + + nextStartAt, _ := schedule.NextAutostart(dbtime.Now(), workspace.AutostartSchedule.String, templateScheduleOptions) + + err = db.UpdateWorkspaceNextStartAt(ctx, database.UpdateWorkspaceNextStartAtParams{ + ID: workspace.ID, + NextStartAt: sql.NullTime{Valid: true, Time: nextStartAt}, + }) + if err != nil { + return xerrors.Errorf("update workspace next start at: %w", err) + } + } + // If max deadline is before now()+2h, then set it to that. // This is intended to give ample warning to this workspace about an upcoming auto-stop. // If we were to omit this "grace" period, then this workspace could be set to be stopped "now". From dea0c205e2c9fed6f1450ec783f3e69769ae8675 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 18 Nov 2024 12:27:33 +0000 Subject: [PATCH 02/30] chore: update dbmem.go --- cli/testdata/coder_list_--output_json.golden | 3 ++- coderd/apidoc/docs.go | 4 ++++ coderd/apidoc/swagger.json | 4 ++++ coderd/database/dbmem/dbmem.go | 1 + ...ule.down.sql => 000277_workspace_next_start_at.down.sql} | 0 ...chedule.up.sql => 000277_workspace_next_start_at.up.sql} | 0 coderd/database/queries.sql.go | 4 ++++ coderd/database/queries/workspaces.sql | 4 ++++ codersdk/workspaces.go | 1 + docs/reference/api/schemas.md | 3 +++ docs/reference/api/workspaces.md | 6 ++++++ site/src/api/typesGenerated.ts | 1 + 12 files changed, 30 insertions(+), 1 deletion(-) rename coderd/database/migrations/{000276_create_cron_type_for_autostart_schedule.down.sql => 000277_workspace_next_start_at.down.sql} (100%) rename coderd/database/migrations/{000276_create_cron_type_for_autostart_schedule.up.sql => 000277_workspace_next_start_at.up.sql} (100%) diff --git a/cli/testdata/coder_list_--output_json.golden b/cli/testdata/coder_list_--output_json.golden index 8f45fd79cfd5a..87c64ee0fe5be 100644 --- a/cli/testdata/coder_list_--output_json.golden +++ b/cli/testdata/coder_list_--output_json.golden @@ -65,6 +65,7 @@ }, "automatic_updates": "never", "allow_renames": false, - "favorite": false + "favorite": false, + "next_start_at": null } ] diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 7c96af792e13c..9390f99421169 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -14440,6 +14440,10 @@ const docTemplate = `{ "name": { "type": "string" }, + "next_start_at": { + "type": "string", + "format": "date-time" + }, "organization_id": { "type": "string", "format": "uuid" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 56b36a4580a16..dd0bd30f93d70 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -13122,6 +13122,10 @@ "name": { "type": "string" }, + "next_start_at": { + "type": "string", + "format": "date-time" + }, "organization_id": { "type": "string", "format": "uuid" diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index a2e8027697871..91a9eda32e092 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -9801,6 +9801,7 @@ func (q *FakeQuerier) UpdateWorkspaceAutostart(_ context.Context, arg database.U continue } workspace.AutostartSchedule = arg.AutostartSchedule + workspace.NextStartAt = arg.NextStartAt q.workspaces[index] = workspace return nil } diff --git a/coderd/database/migrations/000276_create_cron_type_for_autostart_schedule.down.sql b/coderd/database/migrations/000277_workspace_next_start_at.down.sql similarity index 100% rename from coderd/database/migrations/000276_create_cron_type_for_autostart_schedule.down.sql rename to coderd/database/migrations/000277_workspace_next_start_at.down.sql diff --git a/coderd/database/migrations/000276_create_cron_type_for_autostart_schedule.up.sql b/coderd/database/migrations/000277_workspace_next_start_at.up.sql similarity index 100% rename from coderd/database/migrations/000276_create_cron_type_for_autostart_schedule.up.sql rename to coderd/database/migrations/000277_workspace_next_start_at.up.sql diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 3a6ba4554e24a..260faf8257f81 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -15592,6 +15592,10 @@ WHERE workspace_builds.transition = 'stop'::workspace_transition AND workspaces.autostart_schedule IS NOT NULL AND ( + -- next_start_at might be null when a coder instance has been updated + -- and we haven't yet had an opportunity to set next_start_at. When + -- this happens we leave it up to the Coder server to figure out if + -- the workspace is ready to autostart. workspaces.next_start_at IS NULL OR workspaces.next_start_at <= $1 :: timestamp ) diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 17b022187dcd6..5e18ae5f73477 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -619,6 +619,10 @@ WHERE workspace_builds.transition = 'stop'::workspace_transition AND workspaces.autostart_schedule IS NOT NULL AND ( + -- next_start_at might be null when a coder instance has been updated + -- and we haven't yet had an opportunity to set next_start_at. When + -- this happens we leave it up to the Coder server to figure out if + -- the workspace is ready to autostart. workspaces.next_start_at IS NULL OR workspaces.next_start_at <= @now :: timestamp ) diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index d6f3e30a92979..4614a46b90c54 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -63,6 +63,7 @@ type Workspace struct { AutomaticUpdates AutomaticUpdates `json:"automatic_updates" enums:"always,never"` AllowRenames bool `json:"allow_renames"` Favorite bool `json:"favorite"` + NextStartAt *time.Time `json:"next_start_at" format:"date-time"` } func (w Workspace) FullName() string { diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index d40fe8e240005..000fa35d6f1c2 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -6702,6 +6702,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| "workspace_owner_name": "string" }, "name": "string", + "next_start_at": "2019-08-24T14:15:22Z", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "organization_name": "string", "outdated": true, @@ -6736,6 +6737,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| | `last_used_at` | string | false | | | | `latest_build` | [codersdk.WorkspaceBuild](#codersdkworkspacebuild) | false | | | | `name` | string | false | | | +| `next_start_at` | string | false | | | | `organization_id` | string | false | | | | `organization_name` | string | false | | | | `outdated` | boolean | false | | | @@ -8022,6 +8024,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| "workspace_owner_name": "string" }, "name": "string", + "next_start_at": "2019-08-24T14:15:22Z", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "organization_name": "string", "outdated": true, diff --git a/docs/reference/api/workspaces.md b/docs/reference/api/workspaces.md index 183a59ddd13a3..531e5196233a2 100644 --- a/docs/reference/api/workspaces.md +++ b/docs/reference/api/workspaces.md @@ -217,6 +217,7 @@ of the template will be used. "workspace_owner_name": "string" }, "name": "string", + "next_start_at": "2019-08-24T14:15:22Z", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "organization_name": "string", "outdated": true, @@ -435,6 +436,7 @@ curl -X GET http://coder-server:8080/api/v2/users/{user}/workspace/{workspacenam "workspace_owner_name": "string" }, "name": "string", + "next_start_at": "2019-08-24T14:15:22Z", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "organization_name": "string", "outdated": true, @@ -677,6 +679,7 @@ of the template will be used. "workspace_owner_name": "string" }, "name": "string", + "next_start_at": "2019-08-24T14:15:22Z", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "organization_name": "string", "outdated": true, @@ -894,6 +897,7 @@ curl -X GET http://coder-server:8080/api/v2/workspaces \ "workspace_owner_name": "string" }, "name": "string", + "next_start_at": "2019-08-24T14:15:22Z", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "organization_name": "string", "outdated": true, @@ -1113,6 +1117,7 @@ curl -X GET http://coder-server:8080/api/v2/workspaces/{workspace} \ "workspace_owner_name": "string" }, "name": "string", + "next_start_at": "2019-08-24T14:15:22Z", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "organization_name": "string", "outdated": true, @@ -1447,6 +1452,7 @@ curl -X PUT http://coder-server:8080/api/v2/workspaces/{workspace}/dormant \ "workspace_owner_name": "string" }, "name": "string", + "next_start_at": "2019-08-24T14:15:22Z", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "organization_name": "string", "outdated": true, diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index bd00dbda353c3..dc4bd84c94584 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1838,6 +1838,7 @@ export interface Workspace { readonly automatic_updates: AutomaticUpdates; readonly allow_renames: boolean; readonly favorite: boolean; + readonly next_start_at?: string; } // From codersdk/workspaceagents.go From d0f1851299d6d8cc9129ca5520050c5671094ba9 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 19 Nov 2024 12:48:49 +0000 Subject: [PATCH 03/30] fix: dbmem.go impl --- coderd/autobuild/lifecycle_executor_test.go | 1 + coderd/database/dbmem/dbmem.go | 6 +++++- coderd/workspaces.go | 6 ++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 667b20dd9fd4f..9b58bdd894fd5 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -132,6 +132,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { cwr.AutomaticUpdates = tc.automaticUpdates }) ) + // Given: workspace is stopped workspace = coderdtest.MustTransitionWorkspace( t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 91a9eda32e092..2a86702534ac6 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -6952,7 +6952,10 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no if user.Status == database.UserStatusActive && job.JobStatus != database.ProvisionerJobStatusFailed && build.Transition == database.WorkspaceTransitionStop && - workspace.AutostartSchedule.Valid { + workspace.AutostartSchedule.Valid && + (workspace.NextStartAt.Time.IsZero() || + now.After(workspace.NextStartAt.Time) || + now.Equal(workspace.NextStartAt.Time)) { workspaces = append(workspaces, database.GetWorkspacesEligibleForTransitionRow{ ID: workspace.ID, Name: workspace.Name, @@ -7926,6 +7929,7 @@ func (q *FakeQuerier) InsertWorkspace(_ context.Context, arg database.InsertWork Ttl: arg.Ttl, LastUsedAt: arg.LastUsedAt, AutomaticUpdates: arg.AutomaticUpdates, + NextStartAt: arg.NextStartAt, } q.workspaces = append(q.workspaces, workspace) return workspace, nil diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 6a6eee4a0bc41..93cf417c871b1 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -1910,6 +1910,11 @@ func convertWorkspace( deletingAt = &workspace.DeletingAt.Time } + var nextStartAt *time.Time + if workspace.NextStartAt.Valid { + nextStartAt = &workspace.NextStartAt.Time + } + failingAgents := []uuid.UUID{} for _, resource := range workspaceBuild.Resources { for _, agent := range resource.Agents { @@ -1960,6 +1965,7 @@ func convertWorkspace( AutomaticUpdates: codersdk.AutomaticUpdates(workspace.AutomaticUpdates), AllowRenames: allowRenames, Favorite: requesterFavorite, + NextStartAt: nextStartAt, }, nil } From 077439ce6efc001b6f4b15bfbf409b9bcb284543 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 19 Nov 2024 21:50:28 +0000 Subject: [PATCH 04/30] test: refactor test slightly --- coderd/autobuild/lifecycle_executor_test.go | 9 ++++++++- coderd/database/dbgen/dbgen.go | 1 + coderd/database/dbmem/dbmem.go | 4 ++-- coderd/database/queries.sql.go | 2 +- coderd/database/queries/workspaces.sql | 2 +- coderd/workspaces.go | 8 ++++---- enterprise/coderd/schedule/template.go | 3 +-- 7 files changed, 18 insertions(+), 11 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 9b58bdd894fd5..f73b3bc46a2e3 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -1084,6 +1084,10 @@ func TestNotifications(t *testing.T) { IncludeProvisionerDaemon: true, NotificationsEnqueuer: ¬ifyEnq, TemplateScheduleStore: schedule.MockTemplateScheduleStore{ + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { + template.TimeTilDormant = int64(options.TimeTilDormant) + return schedule.NewAGPLTemplateScheduleStore().Set(ctx, db, template, options) + }, GetFn: func(_ context.Context, _ database.Store, _ uuid.UUID) (schedule.TemplateScheduleOptions, error) { return schedule.TemplateScheduleOptions{ UserAutostartEnabled: false, @@ -1100,7 +1104,10 @@ func TestNotifications(t *testing.T) { ) coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) - template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) + template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + timeTilDormantMillis := timeTilDormant.Milliseconds() + ctr.TimeTilDormantMillis = &timeTilDormantMillis + }) userClient, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID) workspace := coderdtest.CreateWorkspace(t, userClient, template.ID) coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID) diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 34ebc617496e5..5ef1c96d43b03 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -247,6 +247,7 @@ func Workspace(t testing.TB, db database.Store, orig database.WorkspaceTable) da AutostartSchedule: orig.AutostartSchedule, Ttl: orig.Ttl, AutomaticUpdates: takeFirst(orig.AutomaticUpdates, database.AutomaticUpdatesNever), + NextStartAt: orig.NextStartAt, }) require.NoError(t, err, "insert workspace") return workspace diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 2a86702534ac6..9d07bfb70d386 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -6965,7 +6965,7 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no if !workspace.DormantAt.Valid && template.TimeTilDormant > 0 && - now.Sub(workspace.LastUsedAt) > time.Duration(template.TimeTilDormant) { + now.Sub(workspace.LastUsedAt) >= time.Duration(template.TimeTilDormant) { workspaces = append(workspaces, database.GetWorkspacesEligibleForTransitionRow{ ID: workspace.ID, Name: workspace.Name, @@ -9955,7 +9955,7 @@ func (q *FakeQuerier) UpdateWorkspaceLastUsedAt(_ context.Context, arg database. return sql.ErrNoRows } -func (q *FakeQuerier) UpdateWorkspaceNextStartAt(ctx context.Context, arg database.UpdateWorkspaceNextStartAtParams) error { +func (q *FakeQuerier) UpdateWorkspaceNextStartAt(_ context.Context, arg database.UpdateWorkspaceNextStartAtParams) error { err := validateDatabaseType(arg) if err != nil { return err diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 260faf8257f81..0d133521e8af6 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -15608,7 +15608,7 @@ WHERE ( workspaces.dormant_at IS NULL AND templates.time_til_dormant > 0 AND - ($1 :: timestamptz) - workspaces.last_used_at > (INTERVAL '1 millisecond' * (templates.time_til_dormant / 1000000)) + ($1 :: timestamptz) - workspaces.last_used_at >= (INTERVAL '1 millisecond' * (templates.time_til_dormant / 1000000)) ) OR -- A workspace may be eligible for deletion if the following are true: diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 5e18ae5f73477..88b2bd78abebf 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -635,7 +635,7 @@ WHERE ( workspaces.dormant_at IS NULL AND templates.time_til_dormant > 0 AND - (@now :: timestamptz) - workspaces.last_used_at > (INTERVAL '1 millisecond' * (templates.time_til_dormant / 1000000)) + (@now :: timestamptz) - workspaces.last_used_at >= (INTERVAL '1 millisecond' * (templates.time_til_dormant / 1000000)) ) OR -- A workspace may be eligible for deletion if the following are true: diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 93cf417c871b1..8bab5dfc9b00a 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -557,8 +557,8 @@ func createWorkspace( nextStartAt := sql.NullTime{} if dbAutostartSchedule.Valid { - time, _ := schedule.NextAutostart(dbtime.Now(), dbAutostartSchedule.String, templateSchedule) - nextStartAt = sql.NullTime{Valid: true, Time: time} + next, _ := schedule.NextAutostart(dbtime.Now(), dbAutostartSchedule.String, templateSchedule) + nextStartAt = sql.NullTime{Valid: true, Time: next} } dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis, templateSchedule.DefaultTTL) @@ -883,8 +883,8 @@ func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) { nextStartAt := sql.NullTime{} if dbSched.Valid { - time, _ := schedule.NextAutostart(dbtime.Now(), dbSched.String, templateSchedule) - nextStartAt = sql.NullTime{Valid: true, Time: time} + next, _ := schedule.NextAutostart(dbtime.Now(), dbSched.String, templateSchedule) + nextStartAt = sql.NullTime{Valid: true, Time: next} } err = api.Database.UpdateWorkspaceAutostart(ctx, database.UpdateWorkspaceAutostartParams{ diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index f8aa18f0fdbcd..1d27d1a4beade 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -18,7 +18,6 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/notifications" - "github.com/coder/coder/v2/coderd/schedule" agpl "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/codersdk" @@ -311,7 +310,7 @@ func (s *EnterpriseTemplateScheduleStore) updateWorkspaceBuild(ctx context.Conte return xerrors.Errorf("get template schedule options: %w", err) } - nextStartAt, _ := schedule.NextAutostart(dbtime.Now(), workspace.AutostartSchedule.String, templateScheduleOptions) + nextStartAt, _ := agpl.NextAutostart(dbtime.Now(), workspace.AutostartSchedule.String, templateScheduleOptions) err = db.UpdateWorkspaceNextStartAt(ctx, database.UpdateWorkspaceNextStartAtParams{ ID: workspace.ID, From 3b41985f875c39f650823183d7cbb64a87387983 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 19 Nov 2024 22:00:00 +0000 Subject: [PATCH 05/30] fix: update golden files --- cli/testdata/coder_list_--output_json.golden | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/testdata/coder_list_--output_json.golden b/cli/testdata/coder_list_--output_json.golden index 87c64ee0fe5be..6fe0a72311220 100644 --- a/cli/testdata/coder_list_--output_json.golden +++ b/cli/testdata/coder_list_--output_json.golden @@ -66,6 +66,6 @@ "automatic_updates": "never", "allow_renames": false, "favorite": false, - "next_start_at": null + "next_start_at": "[timestamp]" } ] From 1a66a14e7d01ff84148d3eef998d93bd902250ff Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 19 Nov 2024 22:07:53 +0000 Subject: [PATCH 06/30] fix: dbmem.go convertToWorkspaceRowsNoLock --- coderd/database/dbmem/dbmem.go | 1 + 1 file changed, 1 insertion(+) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 9d07bfb70d386..da3da797d25d5 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -475,6 +475,7 @@ func (q *FakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspac DeletingAt: w.DeletingAt, AutomaticUpdates: w.AutomaticUpdates, Favorite: w.Favorite, + NextStartAt: w.NextStartAt, OwnerAvatarUrl: extended.OwnerAvatarUrl, OwnerUsername: extended.OwnerUsername, From 9725ec707f173331fcd011ac9b78722a646f2876 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 20 Nov 2024 15:23:20 +0000 Subject: [PATCH 07/30] fix: pass currentTick to GetWorkspacesEligibleForTransition --- coderd/autobuild/lifecycle_executor.go | 2 +- coderd/database/dbmem/dbmem.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index ac2930c9e32c8..eb88286b794b7 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -142,7 +142,7 @@ func (e *Executor) runOnce(t time.Time) Stats { // NOTE: If a workspace build is created with a given TTL and then the user either // changes or unsets the TTL, the deadline for the workspace build will not // have changed. This behavior is as expected per #2229. - workspaces, err := e.db.GetWorkspacesEligibleForTransition(e.ctx, t) + workspaces, err := e.db.GetWorkspacesEligibleForTransition(e.ctx, currentTick) if err != nil { e.log.Error(e.ctx, "get workspaces for autostart or autostop", slog.Error(err)) return stats diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index da3da797d25d5..6a4549fc8c148 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -6955,8 +6955,7 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no build.Transition == database.WorkspaceTransitionStop && workspace.AutostartSchedule.Valid && (workspace.NextStartAt.Time.IsZero() || - now.After(workspace.NextStartAt.Time) || - now.Equal(workspace.NextStartAt.Time)) { + !now.Before(workspace.NextStartAt.Time)) { workspaces = append(workspaces, database.GetWorkspacesEligibleForTransitionRow{ ID: workspace.ID, Name: workspace.Name, From 5e9b806fb159bdfc80fa0caf7f4b4c237a4e6acf Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 21 Nov 2024 09:34:58 +0000 Subject: [PATCH 08/30] revert: query change --- coderd/database/queries.sql.go | 2 +- coderd/database/queries/workspaces.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 0d133521e8af6..260faf8257f81 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -15608,7 +15608,7 @@ WHERE ( workspaces.dormant_at IS NULL AND templates.time_til_dormant > 0 AND - ($1 :: timestamptz) - workspaces.last_used_at >= (INTERVAL '1 millisecond' * (templates.time_til_dormant / 1000000)) + ($1 :: timestamptz) - workspaces.last_used_at > (INTERVAL '1 millisecond' * (templates.time_til_dormant / 1000000)) ) OR -- A workspace may be eligible for deletion if the following are true: diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 88b2bd78abebf..5e18ae5f73477 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -635,7 +635,7 @@ WHERE ( workspaces.dormant_at IS NULL AND templates.time_til_dormant > 0 AND - (@now :: timestamptz) - workspaces.last_used_at >= (INTERVAL '1 millisecond' * (templates.time_til_dormant / 1000000)) + (@now :: timestamptz) - workspaces.last_used_at > (INTERVAL '1 millisecond' * (templates.time_til_dormant / 1000000)) ) OR -- A workspace may be eligible for deletion if the following are true: From b2a037b2ef287c8ac51019ecd03aeb1ef29922df Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 21 Nov 2024 09:40:06 +0000 Subject: [PATCH 09/30] fix: ensure times are converted back to UTC --- coderd/provisionerdserver/provisionerdserver.go | 2 +- coderd/workspaces.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 1421094c7e8d0..0caa1fdfb2c26 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -1457,7 +1457,7 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob) err = db.UpdateWorkspaceNextStartAt(ctx, database.UpdateWorkspaceNextStartAtParams{ ID: workspace.ID, - NextStartAt: sql.NullTime{Valid: true, Time: nextStartAt}, + NextStartAt: sql.NullTime{Valid: true, Time: nextStartAt.UTC()}, }) if err != nil { return xerrors.Errorf("update workspace next start at: %w", err) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 8bab5dfc9b00a..c7061dd5fdec3 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -558,7 +558,7 @@ func createWorkspace( nextStartAt := sql.NullTime{} if dbAutostartSchedule.Valid { next, _ := schedule.NextAutostart(dbtime.Now(), dbAutostartSchedule.String, templateSchedule) - nextStartAt = sql.NullTime{Valid: true, Time: next} + nextStartAt = sql.NullTime{Valid: true, Time: next.UTC()} } dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis, templateSchedule.DefaultTTL) @@ -884,7 +884,7 @@ func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) { nextStartAt := sql.NullTime{} if dbSched.Valid { next, _ := schedule.NextAutostart(dbtime.Now(), dbSched.String, templateSchedule) - nextStartAt = sql.NullTime{Valid: true, Time: next} + nextStartAt = sql.NullTime{Valid: true, Time: next.UTC()} } err = api.Database.UpdateWorkspaceAutostart(ctx, database.UpdateWorkspaceAutostartParams{ From dd05558179e6d1cb3ed6195f21c945fbd65300d0 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 22 Nov 2024 10:18:30 +0000 Subject: [PATCH 10/30] test: next start at is only a valid value --- coderd/database/dbmem/dbmem.go | 4 + coderd/database/queries.sql.go | 2 +- coderd/database/queries/workspaces.sql | 2 +- .../provisionerdserver/provisionerdserver.go | 17 +++-- coderd/schedule/autostart.go | 18 +++++ coderd/schedule/autostart_test.go | 38 ++++++++++ coderd/workspaces.go | 12 ++- enterprise/coderd/workspaces_test.go | 75 +++++++++++++++++++ 8 files changed, 154 insertions(+), 14 deletions(-) create mode 100644 coderd/schedule/autostart_test.go diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 994feeaaea6c7..ab38b74057fd4 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -6954,6 +6954,10 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no job.JobStatus != database.ProvisionerJobStatusFailed && build.Transition == database.WorkspaceTransitionStop && workspace.AutostartSchedule.Valid && + // We do not know if workspace with a zero next start is eligible + // for autostart, so we accept this false-positive. This can occur + // when a coder version is upgraded and next_start_at has yet to + // be set. (workspace.NextStartAt.Time.IsZero() || !now.Before(workspace.NextStartAt.Time)) { workspaces = append(workspaces, database.GetWorkspacesEligibleForTransitionRow{ diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a6f225cf4c531..f1afc20550ea6 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -15620,7 +15620,7 @@ WHERE -- this happens we leave it up to the Coder server to figure out if -- the workspace is ready to autostart. workspaces.next_start_at IS NULL OR - workspaces.next_start_at <= $1 :: timestamp + workspaces.next_start_at <= $1 :: timestamptz ) ) OR diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 5e18ae5f73477..05a63e9e3a21a 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -624,7 +624,7 @@ WHERE -- this happens we leave it up to the Coder server to figure out if -- the workspace is ready to autostart. workspaces.next_start_at IS NULL OR - workspaces.next_start_at <= @now :: timestamp + workspaces.next_start_at <= @now :: timestamptz ) ) OR diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 0caa1fdfb2c26..7020b1de93763 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -1453,14 +1453,15 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob) return xerrors.Errorf("get template schedule options: %w", err) } - nextStartAt, _ := schedule.NextAutostart(now, workspace.AutostartSchedule.String, templateScheduleOptions) - - err = db.UpdateWorkspaceNextStartAt(ctx, database.UpdateWorkspaceNextStartAtParams{ - ID: workspace.ID, - NextStartAt: sql.NullTime{Valid: true, Time: nextStartAt.UTC()}, - }) - if err != nil { - return xerrors.Errorf("update workspace next start at: %w", err) + nextStartAt, err := schedule.NextAllowedAutostart(now, workspace.AutostartSchedule.String, templateScheduleOptions) + if err == nil { + err = db.UpdateWorkspaceNextStartAt(ctx, database.UpdateWorkspaceNextStartAtParams{ + ID: workspace.ID, + NextStartAt: sql.NullTime{Valid: true, Time: nextStartAt.UTC()}, + }) + if err != nil { + return xerrors.Errorf("update workspace next start at: %w", err) + } } } diff --git a/coderd/schedule/autostart.go b/coderd/schedule/autostart.go index 681bd5cfda718..33c67866bc502 100644 --- a/coderd/schedule/autostart.go +++ b/coderd/schedule/autostart.go @@ -3,9 +3,13 @@ package schedule import ( "time" + "golang.org/x/xerrors" + "github.com/coder/coder/v2/coderd/schedule/cron" ) +var ErrNoAllowedAutostart = xerrors.New("no allowed autostart") + // NextAutostart takes the workspace and template schedule and returns the next autostart schedule // after "at". The boolean returned is if the autostart should be allowed to start based on the template // schedule. @@ -28,3 +32,17 @@ func NextAutostart(at time.Time, wsSchedule string, templateSchedule TemplateSch return zonedTransition, allowed } + +func NextAllowedAutostart(at time.Time, wsSchedule string, templateSchedule TemplateScheduleOptions) (time.Time, error) { + const daysInWeek = 7 + + for range daysInWeek { + next, valid := NextAutostart(at, wsSchedule, templateSchedule) + if valid { + return next, nil + } + at = next + } + + return time.Time{}, ErrNoAllowedAutostart +} diff --git a/coderd/schedule/autostart_test.go b/coderd/schedule/autostart_test.go new file mode 100644 index 0000000000000..94bee530a51c3 --- /dev/null +++ b/coderd/schedule/autostart_test.go @@ -0,0 +1,38 @@ +package schedule_test + +import ( + "testing" + "time" + + "github.com/coder/coder/v2/coderd/schedule" + "github.com/stretchr/testify/require" +) + +func TestNextAllowedAutostart(t *testing.T) { + t.Parallel() + + t.Run("WhenScheduleOutOfSync", func(t *testing.T) { + // 1st January 2024 is a Monday + at := time.Date(2024, time.January, 1, 10, 0, 0, 0, time.UTC) + // Monday-Friday 9:00AM UTC + sched := "CRON_TZ=UTC 00 09 * * 1-5" + // Only allow an autostart on mondays + opts := schedule.TemplateScheduleOptions{ + AutostartRequirement: schedule.TemplateAutostartRequirement{ + DaysOfWeek: 0b00000001, + }, + } + + // NextAutostart wil, return a non-allowed autostart time as + // our AutostartRequirement only allows Mondays but we expect + // this to return a Tuesday. + next, allowed := schedule.NextAutostart(at, sched, opts) + require.False(t, allowed) + require.Equal(t, time.Date(2024, time.January, 2, 9, 0, 0, 0, time.UTC), next) + + // NextAllowedAutostart should return the next allowed autostart time. + next, err := schedule.NextAllowedAutostart(at, sched, opts) + require.NoError(t, err) + require.Equal(t, time.Date(2024, time.January, 8, 9, 0, 0, 0, time.UTC), next) + }) +} diff --git a/coderd/workspaces.go b/coderd/workspaces.go index c7061dd5fdec3..c61ba21a2c73e 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -557,8 +557,10 @@ func createWorkspace( nextStartAt := sql.NullTime{} if dbAutostartSchedule.Valid { - next, _ := schedule.NextAutostart(dbtime.Now(), dbAutostartSchedule.String, templateSchedule) - nextStartAt = sql.NullTime{Valid: true, Time: next.UTC()} + next, err := schedule.NextAllowedAutostart(dbtime.Now(), dbAutostartSchedule.String, templateSchedule) + if err == nil { + nextStartAt = sql.NullTime{Valid: true, Time: next.UTC()} + } } dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis, templateSchedule.DefaultTTL) @@ -883,8 +885,10 @@ func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) { nextStartAt := sql.NullTime{} if dbSched.Valid { - next, _ := schedule.NextAutostart(dbtime.Now(), dbSched.String, templateSchedule) - nextStartAt = sql.NullTime{Valid: true, Time: next.UTC()} + next, err := schedule.NextAllowedAutostart(dbtime.Now(), dbSched.String, templateSchedule) + if err == nil { + nextStartAt = sql.NullTime{Valid: true, Time: next.UTC()} + } } err = api.Database.UpdateWorkspaceAutostart(ctx, database.UpdateWorkspaceAutostartParams{ diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 239c7ae377102..77e5eb6700d0d 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "cdr.dev/slog" @@ -1102,6 +1103,80 @@ func TestWorkspaceAutobuild(t *testing.T) { ws = coderdtest.MustWorkspace(t, client, ws.ID) require.Equal(t, version2.ID, ws.LatestBuild.TemplateVersionID) }) + + t.Run("NextStartAtIsValid", func(t *testing.T) { + t.Parallel() + + var ( + // ctx = testutil.Context(t, testutil.WaitMedium) + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + ) + + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + client, user := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerDaemon: true, + AutobuildStats: statsCh, + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, + }, + }) + + version1 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version1.ID) + + // First create a template that only supports Monday-Friday + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version1.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.AutostartRequirement = &codersdk.TemplateAutostartRequirement{DaysOfWeek: codersdk.BitmapToWeekdays(0b00011111)} + }) + require.Equal(t, version1.ID, template.ActiveVersionID) + + // Then create a workspace with a schedule Sunday-Saturday + sched, err := cron.Weekly("CRON_TZ=UTC 0 9 * * 0-6") + require.NoError(t, err) + ws := coderdtest.CreateWorkspace(t, client, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.AutostartSchedule = ptr.Ref(sched.String()) + }) + + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) + ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + + // For each day of the week + for range 7 { + next := sched.Next(ws.LatestBuild.CreatedAt) + + go func() { + tickCh <- next + }() + + stats := <-statsCh + + // Our cron schedule specifies Sunday-Saturday but the template only allows + // Monday-Friday so we expect there to be no transitions on the weekend. + if next.Weekday() == time.Saturday || next.Weekday() == time.Sunday { + assert.Len(t, stats.Errors, 0) + assert.Len(t, stats.Transitions, 0) + + ws = coderdtest.MustWorkspace(t, client, ws.ID) + } else { + assert.Len(t, stats.Errors, 0) + assert.Len(t, stats.Transitions, 1) + assert.Contains(t, stats.Transitions, ws.ID) + assert.Equal(t, database.WorkspaceTransitionStart, stats.Transitions[ws.ID]) + + ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + } + + // Verify that the workspaces next_start_at is a valid autostart day + require.NotNil(t, ws.NextStartAt) + require.NotEqual(t, time.Saturday, ws.NextStartAt.Weekday()) + require.NotEqual(t, time.Sunday, ws.NextStartAt.Weekday()) + } + }) } func TestTemplateDoesNotAllowUserAutostop(t *testing.T) { From 8bddfdfc81603a5c27b51c356d8ffb9d16b44e1b Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 22 Nov 2024 10:30:58 +0000 Subject: [PATCH 11/30] fix: appease linter --- coderd/schedule/autostart_test.go | 5 ++++- enterprise/coderd/workspaces_test.go | 1 - 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/coderd/schedule/autostart_test.go b/coderd/schedule/autostart_test.go index 94bee530a51c3..d172c5aa52f75 100644 --- a/coderd/schedule/autostart_test.go +++ b/coderd/schedule/autostart_test.go @@ -4,14 +4,17 @@ import ( "testing" "time" - "github.com/coder/coder/v2/coderd/schedule" "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/schedule" ) func TestNextAllowedAutostart(t *testing.T) { t.Parallel() t.Run("WhenScheduleOutOfSync", func(t *testing.T) { + t.Parallel() + // 1st January 2024 is a Monday at := time.Date(2024, time.January, 1, 10, 0, 0, 0, time.UTC) // Monday-Friday 9:00AM UTC diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 77e5eb6700d0d..565e3c70d9ebb 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -1108,7 +1108,6 @@ func TestWorkspaceAutobuild(t *testing.T) { t.Parallel() var ( - // ctx = testutil.Context(t, testutil.WaitMedium) tickCh = make(chan time.Time) statsCh = make(chan autobuild.Stats) ) From 66a01a346cdd4979dc058749a4e1b94b45c02323 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 22 Nov 2024 10:50:46 +0000 Subject: [PATCH 12/30] chore: add an index for next_start_at --- .../database/migrations/000277_workspace_next_start_at.down.sql | 2 ++ .../database/migrations/000277_workspace_next_start_at.up.sql | 2 ++ 2 files changed, 4 insertions(+) diff --git a/coderd/database/migrations/000277_workspace_next_start_at.down.sql b/coderd/database/migrations/000277_workspace_next_start_at.down.sql index 705df03948a1f..25118296a39fc 100644 --- a/coderd/database/migrations/000277_workspace_next_start_at.down.sql +++ b/coderd/database/migrations/000277_workspace_next_start_at.down.sql @@ -1,5 +1,7 @@ DROP VIEW workspaces_expanded; +DROP INDEX workspace_next_start_at_idx; + ALTER TABLE ONLY workspaces DROP COLUMN IF EXISTS next_start_at; CREATE VIEW diff --git a/coderd/database/migrations/000277_workspace_next_start_at.up.sql b/coderd/database/migrations/000277_workspace_next_start_at.up.sql index 2abab266fc477..5296214f5076b 100644 --- a/coderd/database/migrations/000277_workspace_next_start_at.up.sql +++ b/coderd/database/migrations/000277_workspace_next_start_at.up.sql @@ -1,5 +1,7 @@ ALTER TABLE ONLY workspaces ADD COLUMN IF NOT EXISTS next_start_at TIMESTAMPTZ DEFAULT NULL; +CREATE INDEX workspace_next_start_at_idx ON workspaces USING btree (next_start_at) WHERE (deleted=false); + -- Recreate view DROP VIEW workspaces_expanded; From b7434c04ae41482c95a98a4982cf6e4265f7e7f3 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 22 Nov 2024 10:57:54 +0000 Subject: [PATCH 13/30] fix: run 'make gen' --- coderd/database/dump.sql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 9874e8d38d61e..fb700fa54a1b3 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -2110,6 +2110,8 @@ CREATE INDEX workspace_app_stats_workspace_id_idx ON workspace_app_stats USING b CREATE INDEX workspace_modules_created_at_idx ON workspace_modules USING btree (created_at); +CREATE INDEX workspace_next_start_at_idx ON workspaces USING btree (next_start_at) WHERE (deleted = false); + CREATE UNIQUE INDEX workspace_proxies_lower_name_idx ON workspace_proxies USING btree (lower(name)) WHERE (deleted = false); CREATE INDEX workspace_resources_job_id_idx ON workspace_resources USING btree (job_id); From 100f54c1f5b9f6f95af883c5ee434ea1f0cb0b1c Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 25 Nov 2024 16:36:54 +0000 Subject: [PATCH 14/30] chore: begin impl of tests --- coderd/autobuild/lifecycle_executor.go | 4 +- coderd/database/dbauthz/dbauthz.go | 14 ++++ coderd/database/dbauthz/dbauthz_test.go | 9 +++ coderd/database/dbmem/dbmem.go | 43 ++++++++++++ coderd/database/dbmetrics/querymetrics.go | 14 ++++ coderd/database/dbmock/dbmock.go | 29 ++++++++ coderd/database/querier.go | 2 + coderd/database/queries.sql.go | 71 +++++++++++++++++++ coderd/database/queries/workspaces.sql | 19 ++++++ coderd/wsbuilder/wsbuilder.go | 1 + enterprise/coderd/schedule/template.go | 37 ++++++++++ enterprise/coderd/workspaces_test.go | 83 +++++++++++++++++++++-- 12 files changed, 319 insertions(+), 7 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index eb88286b794b7..ad89cc5aac5f9 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -463,8 +463,8 @@ func isEligibleForAutostart(user database.User, ws database.Workspace, build dat return false } - nextTransition, allowed := schedule.NextAutostart(build.CreatedAt, ws.AutostartSchedule.String, templateSchedule) - if !allowed { + nextTransition, err := schedule.NextAllowedAutostart(build.CreatedAt, ws.AutostartSchedule.String, templateSchedule) + if err != nil { return false } diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 6ff75d4c06aab..a4e866e92e6ff 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1030,6 +1030,13 @@ func (q *querier) BatchUpdateWorkspaceLastUsedAt(ctx context.Context, arg databa return q.db.BatchUpdateWorkspaceLastUsedAt(ctx, arg) } +func (q *querier) BatchUpdateWorkspaceNextStartAt(ctx context.Context, arg database.BatchUpdateWorkspaceNextStartAtParams) error { + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceWorkspace.All()); err != nil { + return err + } + return q.db.BatchUpdateWorkspaceNextStartAt(ctx, arg) +} + func (q *querier) BulkMarkNotificationMessagesFailed(ctx context.Context, arg database.BulkMarkNotificationMessagesFailedParams) (int64, error) { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceNotificationMessage); err != nil { return 0, err @@ -2817,6 +2824,13 @@ func (q *querier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID u return q.db.GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx, ownerID, prep) } +func (q *querier) GetWorkspacesByTemplateID(ctx context.Context, templateID uuid.UUID) ([]database.WorkspaceTable, error) { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } + return q.db.GetWorkspacesByTemplateID(ctx, templateID) +} + func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.GetWorkspacesEligibleForTransitionRow, error) { return q.db.GetWorkspacesEligibleForTransition(ctx, now) } diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 5010960627d34..1d37a73eed8eb 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1902,6 +1902,12 @@ func (s *MethodTestSuite) TestWorkspace() { NextStartAt: sql.NullTime{Valid: true, Time: dbtime.Now()}, }).Asserts(ws, policy.ActionUpdate) })) + s.Run("BatchUpdateWorkspaceNextStartAt", s.Subtest(func(db database.Store, check *expects) { + check.Args(database.BatchUpdateWorkspaceNextStartAtParams{ + IDs: []uuid.UUID{uuid.New()}, + NextStartAts: []time.Time{dbtime.Now()}, + }).Asserts(rbac.ResourceWorkspace.All(), policy.ActionUpdate) + })) s.Run("BatchUpdateWorkspaceLastUsedAt", s.Subtest(func(db database.Store, check *expects) { ws1 := dbgen.Workspace(s.T(), db, database.WorkspaceTable{}) ws2 := dbgen.Workspace(s.T(), db, database.WorkspaceTable{}) @@ -2778,6 +2784,9 @@ func (s *MethodTestSuite) TestSystemFunctions() { s.Run("GetTemplateAverageBuildTime", s.Subtest(func(db database.Store, check *expects) { check.Args(database.GetTemplateAverageBuildTimeParams{}).Asserts(rbac.ResourceSystem, policy.ActionRead) })) + s.Run("GetWorkspacesByTemplateID", s.Subtest(func(db database.Store, check *expects) { + check.Args(uuid.Nil).Asserts(rbac.ResourceSystem, policy.ActionRead) + })) s.Run("GetWorkspacesEligibleForTransition", s.Subtest(func(db database.Store, check *expects) { check.Args(time.Time{}).Asserts() })) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index ab38b74057fd4..fbf6b78431e0a 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -1432,6 +1432,35 @@ func (q *FakeQuerier) BatchUpdateWorkspaceLastUsedAt(_ context.Context, arg data return nil } +func (q *FakeQuerier) BatchUpdateWorkspaceNextStartAt(_ context.Context, arg database.BatchUpdateWorkspaceNextStartAtParams) error { + err := validateDatabaseType(arg) + if err != nil { + return err + } + + q.mutex.Lock() + defer q.mutex.Unlock() + + for i, workspace := range q.workspaces { + for j, workspaceID := range arg.IDs { + if workspace.ID != workspaceID { + continue + } + + nextStartAt := arg.NextStartAts[j] + if nextStartAt.IsZero() { + q.workspaces[i].NextStartAt = sql.NullTime{} + } else { + q.workspaces[i].NextStartAt = sql.NullTime{Valid: true, Time: nextStartAt} + } + + break + } + } + + return nil +} + func (*FakeQuerier) BulkMarkNotificationMessagesFailed(_ context.Context, arg database.BulkMarkNotificationMessagesFailedParams) (int64, error) { err := validateDatabaseType(arg) if err != nil { @@ -6909,6 +6938,20 @@ func (q *FakeQuerier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, owner return q.GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx, ownerID, nil) } +func (q *FakeQuerier) GetWorkspacesByTemplateID(_ context.Context, templateID uuid.UUID) ([]database.WorkspaceTable, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + workspaces := []database.WorkspaceTable{} + for _, workspace := range q.workspaces { + if workspace.TemplateID == templateID { + workspaces = append(workspaces, workspace) + } + } + + return workspaces, nil +} + func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.GetWorkspacesEligibleForTransitionRow, error) { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 04e258b62a280..9b13be064afb9 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -126,6 +126,13 @@ func (m queryMetricsStore) BatchUpdateWorkspaceLastUsedAt(ctx context.Context, a return r0 } +func (m queryMetricsStore) BatchUpdateWorkspaceNextStartAt(ctx context.Context, arg database.BatchUpdateWorkspaceNextStartAtParams) error { + start := time.Now() + r0 := m.s.BatchUpdateWorkspaceNextStartAt(ctx, arg) + m.queryLatencies.WithLabelValues("BatchUpdateWorkspaceNextStartAt").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) BulkMarkNotificationMessagesFailed(ctx context.Context, arg database.BulkMarkNotificationMessagesFailedParams) (int64, error) { start := time.Now() r0, r1 := m.s.BulkMarkNotificationMessagesFailed(ctx, arg) @@ -1673,6 +1680,13 @@ func (m queryMetricsStore) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, return r0, r1 } +func (m queryMetricsStore) GetWorkspacesByTemplateID(ctx context.Context, templateID uuid.UUID) ([]database.WorkspaceTable, error) { + start := time.Now() + r0, r1 := m.s.GetWorkspacesByTemplateID(ctx, templateID) + m.queryLatencies.WithLabelValues("GetWorkspacesByTemplateID").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.GetWorkspacesEligibleForTransitionRow, error) { start := time.Now() workspaces, err := m.s.GetWorkspacesEligibleForTransition(ctx, now) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 2714ac2e3ef46..1bd3778a0705a 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -145,6 +145,20 @@ func (mr *MockStoreMockRecorder) BatchUpdateWorkspaceLastUsedAt(arg0, arg1 any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BatchUpdateWorkspaceLastUsedAt", reflect.TypeOf((*MockStore)(nil).BatchUpdateWorkspaceLastUsedAt), arg0, arg1) } +// BatchUpdateWorkspaceNextStartAt mocks base method. +func (m *MockStore) BatchUpdateWorkspaceNextStartAt(arg0 context.Context, arg1 database.BatchUpdateWorkspaceNextStartAtParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BatchUpdateWorkspaceNextStartAt", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// BatchUpdateWorkspaceNextStartAt indicates an expected call of BatchUpdateWorkspaceNextStartAt. +func (mr *MockStoreMockRecorder) BatchUpdateWorkspaceNextStartAt(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BatchUpdateWorkspaceNextStartAt", reflect.TypeOf((*MockStore)(nil).BatchUpdateWorkspaceNextStartAt), arg0, arg1) +} + // BulkMarkNotificationMessagesFailed mocks base method. func (m *MockStore) BulkMarkNotificationMessagesFailed(arg0 context.Context, arg1 database.BulkMarkNotificationMessagesFailedParams) (int64, error) { m.ctrl.T.Helper() @@ -3532,6 +3546,21 @@ func (mr *MockStoreMockRecorder) GetWorkspacesAndAgentsByOwnerID(arg0, arg1 any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWorkspacesAndAgentsByOwnerID", reflect.TypeOf((*MockStore)(nil).GetWorkspacesAndAgentsByOwnerID), arg0, arg1) } +// GetWorkspacesByTemplateID mocks base method. +func (m *MockStore) GetWorkspacesByTemplateID(arg0 context.Context, arg1 uuid.UUID) ([]database.WorkspaceTable, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetWorkspacesByTemplateID", arg0, arg1) + ret0, _ := ret[0].([]database.WorkspaceTable) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetWorkspacesByTemplateID indicates an expected call of GetWorkspacesByTemplateID. +func (mr *MockStoreMockRecorder) GetWorkspacesByTemplateID(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWorkspacesByTemplateID", reflect.TypeOf((*MockStore)(nil).GetWorkspacesByTemplateID), arg0, arg1) +} + // GetWorkspacesEligibleForTransition mocks base method. func (m *MockStore) GetWorkspacesEligibleForTransition(arg0 context.Context, arg1 time.Time) ([]database.GetWorkspacesEligibleForTransitionRow, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index fdcfe69f749fc..1ed1a632e12ac 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -57,6 +57,7 @@ type sqlcQuerier interface { // referenced by the latest build of a workspace. ArchiveUnusedTemplateVersions(ctx context.Context, arg ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error) BatchUpdateWorkspaceLastUsedAt(ctx context.Context, arg BatchUpdateWorkspaceLastUsedAtParams) error + BatchUpdateWorkspaceNextStartAt(ctx context.Context, arg BatchUpdateWorkspaceNextStartAtParams) error BulkMarkNotificationMessagesFailed(ctx context.Context, arg BulkMarkNotificationMessagesFailedParams) (int64, error) BulkMarkNotificationMessagesSent(ctx context.Context, arg BulkMarkNotificationMessagesSentParams) (int64, error) CleanTailnetCoordinators(ctx context.Context) error @@ -348,6 +349,7 @@ type sqlcQuerier interface { // be used in a WHERE clause. GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) ([]GetWorkspacesRow, error) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]GetWorkspacesAndAgentsByOwnerIDRow, error) + GetWorkspacesByTemplateID(ctx context.Context, templateID uuid.UUID) ([]WorkspaceTable, error) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]GetWorkspacesEligibleForTransitionRow, error) InsertAPIKey(ctx context.Context, arg InsertAPIKeyParams) (APIKey, error) // We use the organization_id as the id diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index f1afc20550ea6..1139b0216c6c5 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -14650,6 +14650,33 @@ func (q *sqlQuerier) BatchUpdateWorkspaceLastUsedAt(ctx context.Context, arg Bat return err } +const batchUpdateWorkspaceNextStartAt = `-- name: BatchUpdateWorkspaceNextStartAt :exec +UPDATE + workspaces +SET + next_start_at = CASE + WHEN batch.next_start_at = '0001-01-01 00:00:00+00'::timestamptz THEN NULL + ELSE batch.next_start_at + END +FROM ( + SELECT + unnest($1::uuid[]) AS id, + unnest($2::timestamptz[]) AS next_start_at +) AS batch +WHERE + workspaces.id = batch.id +` + +type BatchUpdateWorkspaceNextStartAtParams struct { + IDs []uuid.UUID `db:"ids" json:"ids"` + NextStartAts []time.Time `db:"next_start_ats" json:"next_start_ats"` +} + +func (q *sqlQuerier) BatchUpdateWorkspaceNextStartAt(ctx context.Context, arg BatchUpdateWorkspaceNextStartAtParams) error { + _, err := q.db.ExecContext(ctx, batchUpdateWorkspaceNextStartAt, pq.Array(arg.IDs), pq.Array(arg.NextStartAts)) + return err +} + const favoriteWorkspace = `-- name: FavoriteWorkspace :exec UPDATE workspaces SET favorite = true WHERE id = $1 ` @@ -15562,6 +15589,50 @@ func (q *sqlQuerier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerI return items, nil } +const getWorkspacesByTemplateID = `-- name: GetWorkspacesByTemplateID :many +SELECT id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite, next_start_at FROM workspaces WHERE template_id = $1 AND deleted = false +` + +func (q *sqlQuerier) GetWorkspacesByTemplateID(ctx context.Context, templateID uuid.UUID) ([]WorkspaceTable, error) { + rows, err := q.db.QueryContext(ctx, getWorkspacesByTemplateID, templateID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceTable + for rows.Next() { + var i WorkspaceTable + if err := rows.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.OwnerID, + &i.OrganizationID, + &i.TemplateID, + &i.Deleted, + &i.Name, + &i.AutostartSchedule, + &i.Ttl, + &i.LastUsedAt, + &i.DormantAt, + &i.DeletingAt, + &i.AutomaticUpdates, + &i.Favorite, + &i.NextStartAt, + ); 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 getWorkspacesEligibleForTransition = `-- name: GetWorkspacesEligibleForTransition :many SELECT workspaces.id, diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 05a63e9e3a21a..a398c6c8dca89 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -477,6 +477,22 @@ SET WHERE id = $1; +-- name: BatchUpdateWorkspaceNextStartAt :exec +UPDATE + workspaces +SET + next_start_at = CASE + WHEN batch.next_start_at = '0001-01-01 00:00:00+00'::timestamptz THEN NULL + ELSE batch.next_start_at + END +FROM ( + SELECT + unnest(sqlc.arg(ids)::uuid[]) AS id, + unnest(sqlc.arg(next_start_ats)::timestamptz[]) AS next_start_at +) AS batch +WHERE + workspaces.id = batch.id; + -- name: UpdateWorkspaceTTL :exec UPDATE workspaces @@ -781,3 +797,6 @@ WHERE -- Authorize Filter clause will be injected below in GetAuthorizedWorkspacesAndAgentsByOwnerID -- @authorize_filter GROUP BY workspaces.id, workspaces.name, latest_build.job_status, latest_build.job_id, latest_build.transition; + +-- name: GetWorkspacesByTemplateID :many +SELECT * FROM workspaces WHERE template_id = $1 AND deleted = false; diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 9e8de1d688768..aed920b97ddf9 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -236,6 +236,7 @@ func (b *Builder) Build( if err != nil { return nil, nil, xerrors.Errorf("build tx: %w", err) } + return workspaceBuild, provisionerJob, nil } diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 1d27d1a4beade..38166d6170f13 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -205,6 +205,43 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S return database.Template{}, err } + if opts.AutostartRequirement.DaysOfWeek != tpl.AutostartAllowedDays() { + templateSchedule, err := s.Get(ctx, db, tpl.ID) + if err != nil { + return database.Template{}, xerrors.Errorf("get template schedule: %w", err) + } + + //nolint:gocritic // We need to be able to read information about all workspaces. + workspaces, err := db.GetWorkspacesByTemplateID(dbauthz.AsSystemRestricted(ctx), tpl.ID) + if err != nil { + return database.Template{}, xerrors.Errorf("get workspaces by template id: %w", err) + } + + workspaceIDs := []uuid.UUID{} + nextStartAts := []time.Time{} + + for _, workspace := range workspaces { + nextStartAt := time.Time{} + if workspace.AutostartSchedule.Valid { + next, err := agpl.NextAllowedAutostart(s.now(), workspace.AutostartSchedule.String, templateSchedule) + if err == nil { + nextStartAt = next.UTC() + } + } + + workspaceIDs = append(workspaceIDs, workspace.ID) + nextStartAts = append(nextStartAts, nextStartAt) + } + + //nolint:gocritic // We need to be able to update information about all workspaces. + if err := db.BatchUpdateWorkspaceNextStartAt(dbauthz.AsSystemRestricted(ctx), database.BatchUpdateWorkspaceNextStartAtParams{ + IDs: workspaceIDs, + NextStartAts: nextStartAts, + }); err != nil { + return database.Template{}, xerrors.Errorf("update workspace next start at: %w", err) + } + } + for _, ws := range markedForDeletion { dormantTime := dbtime.Now().Add(opts.TimeTilDormantAutoDelete) _, err = s.enqueuer.Enqueue( diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 565e3c70d9ebb..fee885db014bc 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -33,6 +33,7 @@ import ( "github.com/coder/coder/v2/enterprise/coderd/schedule" "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/testutil" + "github.com/coder/quartz" ) // agplUserQuietHoursScheduleStore is passed to @@ -1118,6 +1119,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, + Logger: &logger, TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ @@ -1130,6 +1132,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // First create a template that only supports Monday-Friday template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version1.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.AllowUserAutostart = ptr.Ref(true) ctr.AutostartRequirement = &codersdk.TemplateAutostartRequirement{DaysOfWeek: codersdk.BitmapToWeekdays(0b00011111)} }) require.Equal(t, version1.ID, template.ActiveVersionID) @@ -1146,13 +1149,11 @@ func TestWorkspaceAutobuild(t *testing.T) { // For each day of the week for range 7 { - next := sched.Next(ws.LatestBuild.CreatedAt) - - go func() { - tickCh <- next - }() + next = sched.Next(next) + tickCh <- next stats := <-statsCh + ws = coderdtest.MustWorkspace(t, client, ws.ID) // Our cron schedule specifies Sunday-Saturday but the template only allows // Monday-Friday so we expect there to be no transitions on the weekend. @@ -1167,15 +1168,87 @@ func TestWorkspaceAutobuild(t *testing.T) { assert.Contains(t, stats.Transitions, ws.ID) assert.Equal(t, database.WorkspaceTransitionStart, stats.Transitions[ws.ID]) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) } // Verify that the workspaces next_start_at is a valid autostart day require.NotNil(t, ws.NextStartAt) + require.Greater(t, ws.NextStartAt, next) require.NotEqual(t, time.Saturday, ws.NextStartAt.Weekday()) require.NotEqual(t, time.Sunday, ws.NextStartAt.Weekday()) } }) + + t.Run("NextStartAtIsUpdatedWhenTemplateAutostartRequirementsChange", func(t *testing.T) { + t.Parallel() + + var ( + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + clock = quartz.NewMock(t) + ) + + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + client, user := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerDaemon: true, + AutobuildStats: statsCh, + Logger: &logger, + Clock: clock, + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, + }, + }) + + version1 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version1.ID) + + // First create a template that only supports Monday-Friday + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version1.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.AllowUserAutostart = ptr.Ref(true) + ctr.AutostartRequirement = &codersdk.TemplateAutostartRequirement{DaysOfWeek: codersdk.BitmapToWeekdays(0b00011111)} + }) + require.Equal(t, version1.ID, template.ActiveVersionID) + + // Then create a workspace with a schedule Sunday-Saturday + sched, err := cron.Weekly("CRON_TZ=UTC 0 9 * * 0-6") + require.NoError(t, err) + ws := coderdtest.CreateWorkspace(t, client, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.AutostartSchedule = ptr.Ref(sched.String()) + }) + + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) + ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + + // Create a monday schedule + monday, err := cron.Weekly("CRON_TZ=UTC 0 9 * * 1") + require.NoError(t, err) + + next := monday.Next(ws.LatestBuild.CreatedAt) + + tickCh <- next + stats := <-statsCh + ws = coderdtest.MustWorkspace(t, client, ws.ID) + + assert.Len(t, stats.Errors, 0) + assert.Len(t, stats.Transitions, 1) + assert.Contains(t, stats.Transitions, ws.ID) + assert.Equal(t, database.WorkspaceTransitionStart, stats.Transitions[ws.ID]) + + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) + ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + + t.Logf("next = %s", next) + t.Logf("next_start_at = %s", ws.NextStartAt) + + // Verify that the workspaces next_start_at is a valid autostart day + require.NotNil(t, ws.NextStartAt) + require.NotEqual(t, next, ws.NextStartAt) + }) } func TestTemplateDoesNotAllowUserAutostop(t *testing.T) { From eac63b7738f64cfaed26a9bf67ca37a645187890 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 25 Nov 2024 16:59:08 +0000 Subject: [PATCH 15/30] test: fix tests --- enterprise/coderd/workspaces_test.go | 59 ++++++++++++++++------------ 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index fee885db014bc..8018d60ed58da 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -1111,8 +1111,13 @@ func TestWorkspaceAutobuild(t *testing.T) { var ( tickCh = make(chan time.Time) statsCh = make(chan autobuild.Stats) + clock = quartz.NewMock(t) ) + // Set the clock to 8AM Monday, 1st January, 2024 to keep + // this test deterministic. + clock.Set(time.Date(2024, 1, 1, 8, 0, 0, 0, time.UTC)) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ @@ -1120,6 +1125,7 @@ func TestWorkspaceAutobuild(t *testing.T) { IncludeProvisionerDaemon: true, AutobuildStats: statsCh, Logger: &logger, + Clock: clock, TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ @@ -1146,11 +1152,15 @@ func TestWorkspaceAutobuild(t *testing.T) { coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + next := ws.LatestBuild.CreatedAt - // For each day of the week + // For each day of the week (Monday-Sunday) + // We iterate through each day of the week to ensure the behaviour of each + // day of the week is as expected. for range 7 { next = sched.Next(next) + clock.Set(next) tickCh <- next stats := <-statsCh ws = coderdtest.MustWorkspace(t, client, ws.ID) @@ -1172,9 +1182,13 @@ func TestWorkspaceAutobuild(t *testing.T) { ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) } - // Verify that the workspaces next_start_at is a valid autostart day + // Ensure that there is a valid next start at and that is is after + // the preivous start. require.NotNil(t, ws.NextStartAt) - require.Greater(t, ws.NextStartAt, next) + require.Greater(t, *ws.NextStartAt, next) + + // Our autostart requirement disallows sundays and saturdays so + // the next start at should never land on these days. require.NotEqual(t, time.Saturday, ws.NextStartAt.Weekday()) require.NotEqual(t, time.Sunday, ws.NextStartAt.Weekday()) } @@ -1189,6 +1203,10 @@ func TestWorkspaceAutobuild(t *testing.T) { clock = quartz.NewMock(t) ) + // Set the clock to 8AM Monday, 1st January, 2024 to keep + // this test deterministic. + clock.Set(time.Date(2024, 1, 1, 8, 0, 0, 0, time.UTC)) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ @@ -1214,8 +1232,8 @@ func TestWorkspaceAutobuild(t *testing.T) { }) require.Equal(t, version1.ID, template.ActiveVersionID) - // Then create a workspace with a schedule Sunday-Saturday - sched, err := cron.Weekly("CRON_TZ=UTC 0 9 * * 0-6") + // Then create a workspace with a schedule Monday-Friday + sched, err := cron.Weekly("CRON_TZ=UTC 0 9 * * 1-5") require.NoError(t, err) ws := coderdtest.CreateWorkspace(t, client, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { cwr.AutostartSchedule = ptr.Ref(sched.String()) @@ -1224,30 +1242,21 @@ func TestWorkspaceAutobuild(t *testing.T) { coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) - // Create a monday schedule - monday, err := cron.Weekly("CRON_TZ=UTC 0 9 * * 1") - require.NoError(t, err) + // Our next start at should be Monday + require.NotNil(t, ws.NextStartAt) + require.Equal(t, time.Monday, ws.NextStartAt.Weekday()) - next := monday.Next(ws.LatestBuild.CreatedAt) + // Now update the template to only allow Tuesday-Friday + coderdtest.UpdateTemplateMeta(t, client, template.ID, codersdk.UpdateTemplateMeta{ + AutostartRequirement: &codersdk.TemplateAutostartRequirement{ + DaysOfWeek: codersdk.BitmapToWeekdays(0b00011110), + }, + }) - tickCh <- next - stats := <-statsCh + // Verify that our next start at has been updated to Tuesday ws = coderdtest.MustWorkspace(t, client, ws.ID) - - assert.Len(t, stats.Errors, 0) - assert.Len(t, stats.Transitions, 1) - assert.Contains(t, stats.Transitions, ws.ID) - assert.Equal(t, database.WorkspaceTransitionStart, stats.Transitions[ws.ID]) - - coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) - ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) - - t.Logf("next = %s", next) - t.Logf("next_start_at = %s", ws.NextStartAt) - - // Verify that the workspaces next_start_at is a valid autostart day require.NotNil(t, ws.NextStartAt) - require.NotEqual(t, next, ws.NextStartAt) + require.Equal(t, time.Tuesday, ws.NextStartAt.Weekday()) }) } From 1b31cbb7739875d4eba7630f0908e4646714ef32 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 25 Nov 2024 17:08:14 +0000 Subject: [PATCH 16/30] fix: use american english --- enterprise/coderd/workspaces_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 8018d60ed58da..51d530c314553 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -1155,7 +1155,7 @@ func TestWorkspaceAutobuild(t *testing.T) { next := ws.LatestBuild.CreatedAt // For each day of the week (Monday-Sunday) - // We iterate through each day of the week to ensure the behaviour of each + // We iterate through each day of the week to ensure the behavior of each // day of the week is as expected. for range 7 { next = sched.Next(next) From eaf32ee4286fae713fee81492779e2e44a3184c9 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 26 Nov 2024 15:00:52 +0000 Subject: [PATCH 17/30] fix: make NextAllowedAutostart look up to 7 days into the future --- coderd/schedule/autostart.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/coderd/schedule/autostart.go b/coderd/schedule/autostart.go index 33c67866bc502..7160811600b3a 100644 --- a/coderd/schedule/autostart.go +++ b/coderd/schedule/autostart.go @@ -34,10 +34,13 @@ func NextAutostart(at time.Time, wsSchedule string, templateSchedule TemplateSch } func NextAllowedAutostart(at time.Time, wsSchedule string, templateSchedule TemplateScheduleOptions) (time.Time, error) { - const daysInWeek = 7 + next := at - for range daysInWeek { - next, valid := NextAutostart(at, wsSchedule, templateSchedule) + // Our cron schedules work on a weekly basis, so to ensure we've exhausted all + // possible autostart times we need to check up to 7 days worth of autostarts. + for next.Sub(at) < 7*24*time.Hour { + var valid bool + next, valid = NextAutostart(at, wsSchedule, templateSchedule) if valid { return next, nil } From 1599b2ae7b5b2d75c6067e4b439b8898291351e4 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 26 Nov 2024 15:29:39 +0000 Subject: [PATCH 18/30] fix: TestExecutorAutostartBlocked test --- enterprise/coderd/workspaces_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 51d530c314553..85f6997381712 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -1381,9 +1381,9 @@ func TestExecutorAutostartBlocked(t *testing.T) { // Given: workspace is stopped workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) - // When: the autobuild executor ticks way into the future + // When: the autobuild executor ticks into the future go func() { - tickCh <- workspace.LatestBuild.CreatedAt.Add(24 * time.Hour) + tickCh <- workspace.LatestBuild.CreatedAt.Add(2 * time.Hour) close(tickCh) }() From 142e33531f3059a8f7899db818274a204e5a2bfc Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 26 Nov 2024 16:26:42 +0000 Subject: [PATCH 19/30] fix: make enterprise template schedule store use quartz.clock --- enterprise/coderd/schedule/template.go | 16 +++++++++------- enterprise/coderd/schedule/template_test.go | 16 +++++++++------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 38166d6170f13..3c2efa08de408 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -21,6 +21,7 @@ import ( agpl "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/codersdk" + "github.com/coder/quartz" ) // EnterpriseTemplateScheduleStore provides an agpl.TemplateScheduleStore that @@ -31,7 +32,7 @@ type EnterpriseTemplateScheduleStore struct { UserQuietHoursScheduleStore *atomic.Pointer[agpl.UserQuietHoursScheduleStore] // Custom time.Now() function to use in tests. Defaults to dbtime.Now(). - TimeNowFn func() time.Time + Clock quartz.Clock enqueuer notifications.Enqueuer logger slog.Logger @@ -42,16 +43,17 @@ var _ agpl.TemplateScheduleStore = &EnterpriseTemplateScheduleStore{} func NewEnterpriseTemplateScheduleStore(userQuietHoursStore *atomic.Pointer[agpl.UserQuietHoursScheduleStore], enqueuer notifications.Enqueuer, logger slog.Logger) *EnterpriseTemplateScheduleStore { return &EnterpriseTemplateScheduleStore{ UserQuietHoursScheduleStore: userQuietHoursStore, + Clock: quartz.NewReal(), enqueuer: enqueuer, logger: logger, } } func (s *EnterpriseTemplateScheduleStore) now() time.Time { - if s.TimeNowFn != nil { - return s.TimeNowFn() + if s.Clock == nil { + s.Clock = quartz.NewReal() } - return dbtime.Now() + return dbtime.Time(s.Clock.Now()) } // Get implements agpl.TemplateScheduleStore. @@ -164,7 +166,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S var dormantAt time.Time if opts.UpdateWorkspaceDormantAt { - dormantAt = dbtime.Now() + dormantAt = s.now() } // If we updated the time_til_dormant_autodelete we need to update all the workspaces deleting_at @@ -243,7 +245,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S } for _, ws := range markedForDeletion { - dormantTime := dbtime.Now().Add(opts.TimeTilDormantAutoDelete) + dormantTime := s.now().Add(opts.TimeTilDormantAutoDelete) _, err = s.enqueuer.Enqueue( // nolint:gocritic // Need actor to enqueue notification dbauthz.AsNotifier(ctx), @@ -347,7 +349,7 @@ func (s *EnterpriseTemplateScheduleStore) updateWorkspaceBuild(ctx context.Conte return xerrors.Errorf("get template schedule options: %w", err) } - nextStartAt, _ := agpl.NextAutostart(dbtime.Now(), workspace.AutostartSchedule.String, templateScheduleOptions) + nextStartAt, _ := agpl.NextAutostart(s.now(), workspace.AutostartSchedule.String, templateScheduleOptions) err = db.UpdateWorkspaceNextStartAt(ctx, database.UpdateWorkspaceNextStartAtParams{ ID: workspace.ID, diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index ee84dbe90ff78..f5d57e77a2137 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -26,6 +26,7 @@ import ( "github.com/coder/coder/v2/cryptorand" "github.com/coder/coder/v2/enterprise/coderd/schedule" "github.com/coder/coder/v2/testutil" + "github.com/coder/quartz" ) func TestTemplateUpdateBuildDeadlines(t *testing.T) { @@ -283,11 +284,12 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { userQuietHoursStorePtr := &atomic.Pointer[agplschedule.UserQuietHoursScheduleStore]{} userQuietHoursStorePtr.Store(&userQuietHoursStore) + clock := quartz.NewMock(t) + clock.Set(c.now) + // Set the template policy. templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, notifications.NewNoopEnqueuer(), logger) - templateScheduleStore.TimeNowFn = func() time.Time { - return c.now - } + templateScheduleStore.Clock = clock autostopReq := agplschedule.TemplateAutostopRequirement{ // Every day @@ -570,11 +572,12 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { userQuietHoursStorePtr := &atomic.Pointer[agplschedule.UserQuietHoursScheduleStore]{} userQuietHoursStorePtr.Store(&userQuietHoursStore) + clock := quartz.NewMock(t) + clock.Set(now) + // Set the template policy. templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, notifications.NewNoopEnqueuer(), logger) - templateScheduleStore.TimeNowFn = func() time.Time { - return now - } + templateScheduleStore.Clock = clock _, err = templateScheduleStore.Set(ctx, db, template, agplschedule.TemplateScheduleOptions{ UserAutostartEnabled: false, UserAutostopEnabled: false, @@ -683,7 +686,6 @@ func TestNotifications(t *testing.T) { userQuietHoursStorePtr := &atomic.Pointer[agplschedule.UserQuietHoursScheduleStore]{} userQuietHoursStorePtr.Store(&userQuietHoursStore) templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, ¬ifyEnq, logger) - templateScheduleStore.TimeNowFn = time.Now // Lower the dormancy TTL to ensure the schedule recalculates deadlines and // triggers notifications. From 3f17e466128b13a596cf0ceabe5e65c640f17459 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 26 Nov 2024 16:56:58 +0000 Subject: [PATCH 20/30] fix: infinite loop --- coderd/schedule/autostart.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/coderd/schedule/autostart.go b/coderd/schedule/autostart.go index 7160811600b3a..0a7f583e4f9b2 100644 --- a/coderd/schedule/autostart.go +++ b/coderd/schedule/autostart.go @@ -40,11 +40,10 @@ func NextAllowedAutostart(at time.Time, wsSchedule string, templateSchedule Temp // possible autostart times we need to check up to 7 days worth of autostarts. for next.Sub(at) < 7*24*time.Hour { var valid bool - next, valid = NextAutostart(at, wsSchedule, templateSchedule) + next, valid = NextAutostart(next, wsSchedule, templateSchedule) if valid { return next, nil } - at = next } return time.Time{}, ErrNoAllowedAutostart From e9936434c332834a154c2a47c3386da6c795b4d4 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 26 Nov 2024 17:15:41 +0000 Subject: [PATCH 21/30] refactor: give template schedule store a quartz.clock --- enterprise/coderd/coderd.go | 2 +- enterprise/coderd/schedule/template.go | 11 +++--- enterprise/coderd/schedule/template_test.go | 8 ++--- enterprise/coderd/templates_test.go | 4 +-- enterprise/coderd/workspaces_test.go | 40 +++++++++++---------- 5 files changed, 33 insertions(+), 32 deletions(-) diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index b6d60b5e4c20e..f8d6fc98edfe2 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -738,7 +738,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { if initial, changed, enabled := featureChanged(codersdk.FeatureAdvancedTemplateScheduling); shouldUpdate(initial, changed, enabled) { if enabled { - templateStore := schedule.NewEnterpriseTemplateScheduleStore(api.AGPL.UserQuietHoursScheduleStore, api.NotificationsEnqueuer, api.Logger.Named("template.schedule-store")) + templateStore := schedule.NewEnterpriseTemplateScheduleStore(api.AGPL.UserQuietHoursScheduleStore, api.NotificationsEnqueuer, api.Logger.Named("template.schedule-store"), api.Clock) templateStoreInterface := agplschedule.TemplateScheduleStore(templateStore) api.AGPL.TemplateScheduleStore.Store(&templateStoreInterface) diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 3c2efa08de408..3c93bac1286b3 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -40,19 +40,20 @@ type EnterpriseTemplateScheduleStore struct { var _ agpl.TemplateScheduleStore = &EnterpriseTemplateScheduleStore{} -func NewEnterpriseTemplateScheduleStore(userQuietHoursStore *atomic.Pointer[agpl.UserQuietHoursScheduleStore], enqueuer notifications.Enqueuer, logger slog.Logger) *EnterpriseTemplateScheduleStore { +func NewEnterpriseTemplateScheduleStore(userQuietHoursStore *atomic.Pointer[agpl.UserQuietHoursScheduleStore], enqueuer notifications.Enqueuer, logger slog.Logger, clock quartz.Clock) *EnterpriseTemplateScheduleStore { + if clock == nil { + clock = quartz.NewReal() + } + return &EnterpriseTemplateScheduleStore{ UserQuietHoursScheduleStore: userQuietHoursStore, - Clock: quartz.NewReal(), + Clock: clock, enqueuer: enqueuer, logger: logger, } } func (s *EnterpriseTemplateScheduleStore) now() time.Time { - if s.Clock == nil { - s.Clock = quartz.NewReal() - } return dbtime.Time(s.Clock.Now()) } diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index f5d57e77a2137..5e3c9fd658cf3 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -288,8 +288,7 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { clock.Set(c.now) // Set the template policy. - templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, notifications.NewNoopEnqueuer(), logger) - templateScheduleStore.Clock = clock + templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, notifications.NewNoopEnqueuer(), logger, clock) autostopReq := agplschedule.TemplateAutostopRequirement{ // Every day @@ -576,8 +575,7 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { clock.Set(now) // Set the template policy. - templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, notifications.NewNoopEnqueuer(), logger) - templateScheduleStore.Clock = clock + templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, notifications.NewNoopEnqueuer(), logger, clock) _, err = templateScheduleStore.Set(ctx, db, template, agplschedule.TemplateScheduleOptions{ UserAutostartEnabled: false, UserAutostopEnabled: false, @@ -685,7 +683,7 @@ func TestNotifications(t *testing.T) { require.NoError(t, err) userQuietHoursStorePtr := &atomic.Pointer[agplschedule.UserQuietHoursScheduleStore]{} userQuietHoursStorePtr.Store(&userQuietHoursStore) - templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, ¬ifyEnq, logger) + templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, ¬ifyEnq, logger, nil) // Lower the dormancy TTL to ensure the schedule recalculates deadlines and // triggers notifications. diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 2fc6bf9fda087..22314f45bb3c7 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -689,7 +689,7 @@ func TestTemplates(t *testing.T) { client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ @@ -739,7 +739,7 @@ func TestTemplates(t *testing.T) { owner, first := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 85f6997381712..f88adc83542fc 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -297,7 +297,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -344,7 +344,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -390,7 +390,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -434,7 +434,7 @@ func TestWorkspaceAutobuild(t *testing.T) { Options: &coderdtest.Options{ AutobuildTicker: ticker, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), Auditor: auditRecorder, }, LicenseOptions: &coderdenttest.LicenseOptions{ @@ -529,7 +529,7 @@ func TestWorkspaceAutobuild(t *testing.T) { Options: &coderdtest.Options{ AutobuildTicker: ticker, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), Database: db, Pubsub: pubsub, Auditor: auditor, @@ -587,7 +587,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -630,7 +630,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -673,7 +673,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -727,7 +727,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -799,7 +799,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -863,7 +863,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -943,7 +943,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -1029,7 +1029,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAccessControl: 1}, @@ -1126,7 +1126,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildStats: statsCh, Logger: &logger, Clock: clock, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -1208,6 +1208,8 @@ func TestWorkspaceAutobuild(t *testing.T) { clock.Set(time.Date(2024, 1, 1, 8, 0, 0, 0, time.UTC)) logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil) + templateScheduleStore.Clock = clock client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: tickCh, @@ -1215,7 +1217,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildStats: statsCh, Logger: &logger, Clock: clock, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: templateScheduleStore, }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -1268,7 +1270,7 @@ func TestTemplateDoesNotAllowUserAutostop(t *testing.T) { logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client := coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) @@ -1307,7 +1309,7 @@ func TestTemplateDoesNotAllowUserAutostop(t *testing.T) { logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client := coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) @@ -1359,7 +1361,7 @@ func TestExecutorAutostartBlocked(t *testing.T) { AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -1403,7 +1405,7 @@ func TestWorkspacesFiltering(t *testing.T) { logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, db, owner := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, From d349c568917ee1a36df447d1bd2ad5a0d676d8d0 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 26 Nov 2024 17:29:28 +0000 Subject: [PATCH 22/30] fix: ensure tests give template schedule store a clock --- enterprise/coderd/workspaces_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index f88adc83542fc..df345f4c66820 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -1520,7 +1520,7 @@ func TestWorkspaceLock(t *testing.T) { client, user = coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, - TemplateScheduleStore: &schedule.EnterpriseTemplateScheduleStore{}, + TemplateScheduleStore: &schedule.EnterpriseTemplateScheduleStore{Clock: quartz.NewReal()}, }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ @@ -1581,7 +1581,7 @@ func TestResolveAutostart(t *testing.T) { ownerClient, db, owner := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ - TemplateScheduleStore: &schedule.EnterpriseTemplateScheduleStore{}, + TemplateScheduleStore: &schedule.EnterpriseTemplateScheduleStore{Clock: quartz.NewReal()}, }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ From a48fb995f7631af81ea42d15094a2311de7a6419 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 27 Nov 2024 10:49:02 +0000 Subject: [PATCH 23/30] fix: nullify next_start_at on schedule update --- coderd/autobuild/lifecycle_executor.go | 21 +++++ coderd/database/dump.sql | 16 ++++ .../000277_workspace_next_start_at.down.sql | 3 + .../000277_workspace_next_start_at.up.sql | 19 ++++ enterprise/coderd/workspaces_test.go | 89 ++++++++++++++++++- 5 files changed, 147 insertions(+), 1 deletion(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index ad89cc5aac5f9..efd482a4346cf 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -205,6 +205,27 @@ func (e *Executor) runOnce(t time.Time) Stats { return xerrors.Errorf("get template scheduling options: %w", err) } + // If next start at is not valid we need to re-compute it. + if !ws.NextStartAt.Valid && ws.AutostartSchedule.Valid { + next, err := schedule.NextAllowedAutostart(currentTick, ws.AutostartSchedule.String, templateSchedule) + if err != nil { + return xerrors.Errorf("compute next allowed autostart: %w", err) + } + + e.log.Debug(e.ctx, "computed next allowed", slog.F("currentTick", currentTick), slog.F("next", next), slog.F("UTC", next.UTC())) + + nextStartAt := sql.NullTime{Valid: true, Time: next.UTC()} + if err = tx.UpdateWorkspaceNextStartAt(e.ctx, database.UpdateWorkspaceNextStartAtParams{ + ID: wsID, + NextStartAt: nextStartAt, + }); err != nil { + return xerrors.Errorf("update workspace next start at: %w", err) + } + + // Save re-fetching the workspace + ws.NextStartAt = nextStartAt + } + tmpl, err = tx.GetTemplateByID(e.ctx, ws.TemplateID) if err != nil { return xerrors.Errorf("get template by ID: %w", err) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index fb700fa54a1b3..9da7018d972e6 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -380,6 +380,20 @@ BEGIN END; $$; +CREATE FUNCTION nullify_workspace_next_start_at() RETURNS trigger + LANGUAGE plpgsql + AS $$ +DECLARE +BEGIN + IF (NEW.autostart_schedule <> OLD.autostart_schedule) AND (NEW.next_start_at = OLD.next_start_at) THEN + UPDATE workspaces + SET next_start_at = NULL + WHERE id = NEW.id; + END IF; + RETURN NEW; +END; +$$; + CREATE FUNCTION provisioner_tagset_contains(provisioner_tags tagset, job_tags tagset) RETURNS boolean LANGUAGE plpgsql AS $$ @@ -2196,6 +2210,8 @@ CREATE TRIGGER trigger_insert_apikeys BEFORE INSERT ON api_keys FOR EACH ROW EXE CREATE TRIGGER trigger_update_users AFTER INSERT OR UPDATE ON users FOR EACH ROW WHEN ((new.deleted = true)) EXECUTE FUNCTION delete_deleted_user_resources(); +CREATE TRIGGER trigger_update_workspaces_schedule AFTER UPDATE ON workspaces FOR EACH ROW EXECUTE FUNCTION nullify_workspace_next_start_at(); + CREATE TRIGGER trigger_upsert_user_links BEFORE INSERT OR UPDATE ON user_links FOR EACH ROW EXECUTE FUNCTION insert_user_links_fail_if_user_deleted(); CREATE TRIGGER update_notification_message_dedupe_hash BEFORE INSERT OR UPDATE ON notification_messages FOR EACH ROW EXECUTE FUNCTION compute_notification_message_dedupe_hash(); diff --git a/coderd/database/migrations/000277_workspace_next_start_at.down.sql b/coderd/database/migrations/000277_workspace_next_start_at.down.sql index 25118296a39fc..a0eab4835e63b 100644 --- a/coderd/database/migrations/000277_workspace_next_start_at.down.sql +++ b/coderd/database/migrations/000277_workspace_next_start_at.down.sql @@ -1,5 +1,8 @@ DROP VIEW workspaces_expanded; +DROP TRIGGER IF EXISTS trigger_update_workspaces_schedule ON workspaces; +DROP FUNCTION IF EXISTS nullify_workspace_next_start_at; + DROP INDEX workspace_next_start_at_idx; ALTER TABLE ONLY workspaces DROP COLUMN IF EXISTS next_start_at; diff --git a/coderd/database/migrations/000277_workspace_next_start_at.up.sql b/coderd/database/migrations/000277_workspace_next_start_at.up.sql index 5296214f5076b..65be1e92f92b1 100644 --- a/coderd/database/migrations/000277_workspace_next_start_at.up.sql +++ b/coderd/database/migrations/000277_workspace_next_start_at.up.sql @@ -2,6 +2,25 @@ ALTER TABLE ONLY workspaces ADD COLUMN IF NOT EXISTS next_start_at TIMESTAMPTZ D CREATE INDEX workspace_next_start_at_idx ON workspaces USING btree (next_start_at) WHERE (deleted=false); +CREATE FUNCTION nullify_workspace_next_start_at() RETURNS trigger + LANGUAGE plpgsql +AS $$ +DECLARE +BEGIN + IF (NEW.autostart_schedule <> OLD.autostart_schedule) AND (NEW.next_start_at = OLD.next_start_at) THEN + UPDATE workspaces + SET next_start_at = NULL + WHERE id = NEW.id; + END IF; + RETURN NEW; +END; +$$; + +CREATE TRIGGER trigger_update_workspaces_schedule + AFTER UPDATE ON workspaces + FOR EACH ROW +EXECUTE PROCEDURE nullify_workspace_next_start_at(); + -- Recreate view DROP VIEW workspaces_expanded; diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index df345f4c66820..d96d68d0525fc 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -2,6 +2,7 @@ package coderd_test import ( "context" + "database/sql" "net/http" "sync/atomic" "testing" @@ -18,8 +19,10 @@ import ( "github.com/coder/coder/v2/coderd/autobuild" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/rbac" agplschedule "github.com/coder/coder/v2/coderd/schedule" @@ -1138,7 +1141,6 @@ func TestWorkspaceAutobuild(t *testing.T) { // First create a template that only supports Monday-Friday template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version1.ID, func(ctr *codersdk.CreateTemplateRequest) { - ctr.AllowUserAutostart = ptr.Ref(true) ctr.AutostartRequirement = &codersdk.TemplateAutostartRequirement{DaysOfWeek: codersdk.BitmapToWeekdays(0b00011111)} }) require.Equal(t, version1.ID, template.ActiveVersionID) @@ -1680,6 +1682,91 @@ func TestAdminViewAllWorkspaces(t *testing.T) { require.Equal(t, 0, len(memberViewWorkspaces.Workspaces), "member in other org should see 0 workspaces") } +func TestNextStartAtIsNullifiedOnScheduleChange(t *testing.T) { + t.Parallel() + + var ( + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + clock = quartz.NewMock(t) + ) + + // Set the clock to 8AM Monday, 1st January, 2024 to keep + // this test deterministic. + clock.Set(time.Date(2024, 1, 1, 8, 0, 0, 0, time.UTC)) + + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerDaemon: true, + AutobuildStats: statsCh, + Logger: &logger, + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, clock), + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, + }, + }) + + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + + // Create a template that allows autostart Monday-Sunday + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.AutostartRequirement = &codersdk.TemplateAutostartRequirement{DaysOfWeek: codersdk.AllDaysOfWeek} + }) + require.Equal(t, version.ID, template.ActiveVersionID) + + // Create a workspace with a schedule Sunday-Saturday + sched, err := cron.Weekly("CRON_TZ=UTC 0 9 * * 0-6") + require.NoError(t, err) + ws := coderdtest.CreateWorkspace(t, client, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.AutostartSchedule = ptr.Ref(sched.String()) + }) + + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) + ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + + // Check we have a 'NextStartAt' + require.NotNil(t, ws.NextStartAt) + + // Create a new slightly different cron schedule that could + // potentially make NextStartAt invalid. + sched, err = cron.Weekly("CRON_TZ=UTC 0 9 * * 1-6") + require.NoError(t, err) + ctx := testutil.Context(t, testutil.WaitShort) + + // We want to test the database nullifies the NextStartAt so we + // make a raw DB call here. We pass in NextStartAt here so we + // can test the database will nullify it and not us. + //nolint: gocritic // We need system context to modify this. + err = db.UpdateWorkspaceAutostart(dbauthz.AsSystemRestricted(ctx), database.UpdateWorkspaceAutostartParams{ + ID: ws.ID, + AutostartSchedule: sql.NullString{Valid: true, String: sched.String()}, + NextStartAt: sql.NullTime{Valid: true, Time: *ws.NextStartAt}, + }) + require.NoError(t, err) + + ws = coderdtest.MustWorkspace(t, client, ws.ID) + + // Check 'NextStartAt' has been nullified + require.Nil(t, ws.NextStartAt) + + // Now we let the lifecycle executor run. This should spot that the + // NextStartAt is null and update it for us. + next := dbtime.Now() + tickCh <- next + stats := <-statsCh + assert.Len(t, stats.Errors, 0) + assert.Len(t, stats.Transitions, 0) + + // Ensure NextStartAt has been set, and is the expected value + ws = coderdtest.MustWorkspace(t, client, ws.ID) + require.NotNil(t, ws.NextStartAt) + require.Equal(t, sched.Next(next), ws.NextStartAt.UTC()) +} + func must[T any](value T, err error) T { if err != nil { panic(err) From cc93075f1e588dcb7e129061053492bb28cefb61 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 27 Nov 2024 11:05:40 +0000 Subject: [PATCH 24/30] fix: tests --- coderd/autobuild/lifecycle_executor.go | 24 ++++++++++-------------- enterprise/coderd/workspaces_test.go | 4 ++++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index efd482a4346cf..20cb5a130beee 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -208,22 +208,18 @@ func (e *Executor) runOnce(t time.Time) Stats { // If next start at is not valid we need to re-compute it. if !ws.NextStartAt.Valid && ws.AutostartSchedule.Valid { next, err := schedule.NextAllowedAutostart(currentTick, ws.AutostartSchedule.String, templateSchedule) - if err != nil { - return xerrors.Errorf("compute next allowed autostart: %w", err) - } - - e.log.Debug(e.ctx, "computed next allowed", slog.F("currentTick", currentTick), slog.F("next", next), slog.F("UTC", next.UTC())) + if err == nil { + nextStartAt := sql.NullTime{Valid: true, Time: next.UTC()} + if err = tx.UpdateWorkspaceNextStartAt(e.ctx, database.UpdateWorkspaceNextStartAtParams{ + ID: wsID, + NextStartAt: nextStartAt, + }); err != nil { + return xerrors.Errorf("update workspace next start at: %w", err) + } - nextStartAt := sql.NullTime{Valid: true, Time: next.UTC()} - if err = tx.UpdateWorkspaceNextStartAt(e.ctx, database.UpdateWorkspaceNextStartAtParams{ - ID: wsID, - NextStartAt: nextStartAt, - }); err != nil { - return xerrors.Errorf("update workspace next start at: %w", err) + // Save re-fetching the workspace + ws.NextStartAt = nextStartAt } - - // Save re-fetching the workspace - ws.NextStartAt = nextStartAt } tmpl, err = tx.GetTemplateByID(e.ctx, ws.TemplateID) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index d96d68d0525fc..f47b4241c18cc 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -1685,6 +1685,10 @@ func TestAdminViewAllWorkspaces(t *testing.T) { func TestNextStartAtIsNullifiedOnScheduleChange(t *testing.T) { t.Parallel() + if !dbtestutil.WillUsePostgres() { + t.Skip("this test uses triggers so does not work with dbmem.go") + } + var ( tickCh = make(chan time.Time) statsCh = make(chan autobuild.Stats) From 45350d177ee4e0f73d8014b555dfcbd9dc7ac05e Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 27 Nov 2024 11:47:17 +0000 Subject: [PATCH 25/30] chore: remove clock from test --- enterprise/coderd/workspaces_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index f47b4241c18cc..46c4a8627135b 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -1692,13 +1692,8 @@ func TestNextStartAtIsNullifiedOnScheduleChange(t *testing.T) { var ( tickCh = make(chan time.Time) statsCh = make(chan autobuild.Stats) - clock = quartz.NewMock(t) ) - // Set the clock to 8AM Monday, 1st January, 2024 to keep - // this test deterministic. - clock.Set(time.Date(2024, 1, 1, 8, 0, 0, 0, time.UTC)) - logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ @@ -1706,7 +1701,7 @@ func TestNextStartAtIsNullifiedOnScheduleChange(t *testing.T) { IncludeProvisionerDaemon: true, AutobuildStats: statsCh, Logger: &logger, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, clock), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, From 03d0b7ff5c5d25ae8eaf70367fa27826b09aa416 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 27 Nov 2024 16:38:21 +0000 Subject: [PATCH 26/30] chore: relocate test --- coderd/autobuild/lifecycle_executor_test.go | 1 - coderd/wsbuilder/wsbuilder.go | 1 - enterprise/coderd/workspaces_test.go | 168 ++++++++++---------- 3 files changed, 84 insertions(+), 86 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index f73b3bc46a2e3..54064e7fa4b6b 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -132,7 +132,6 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { cwr.AutomaticUpdates = tc.automaticUpdates }) ) - // Given: workspace is stopped workspace = coderdtest.MustTransitionWorkspace( t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index aed920b97ddf9..9e8de1d688768 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -236,7 +236,6 @@ func (b *Builder) Build( if err != nil { return nil, nil, xerrors.Errorf("build tx: %w", err) } - return workspaceBuild, provisionerJob, nil } diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 46c4a8627135b..fcaeb0f62038a 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -1262,6 +1262,90 @@ func TestWorkspaceAutobuild(t *testing.T) { require.NotNil(t, ws.NextStartAt) require.Equal(t, time.Tuesday, ws.NextStartAt.Weekday()) }) + + t.Run("NextStartAtIsNullifiedOnScheduleChange", func(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("this test uses triggers so does not work with dbmem.go") + } + + var ( + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + ) + + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerDaemon: true, + AutobuildStats: statsCh, + Logger: &logger, + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, + }, + }) + + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + + // Create a template that allows autostart Monday-Sunday + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.AutostartRequirement = &codersdk.TemplateAutostartRequirement{DaysOfWeek: codersdk.AllDaysOfWeek} + }) + require.Equal(t, version.ID, template.ActiveVersionID) + + // Create a workspace with a schedule Sunday-Saturday + sched, err := cron.Weekly("CRON_TZ=UTC 0 9 * * 0-6") + require.NoError(t, err) + ws := coderdtest.CreateWorkspace(t, client, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.AutostartSchedule = ptr.Ref(sched.String()) + }) + + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) + ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + + // Check we have a 'NextStartAt' + require.NotNil(t, ws.NextStartAt) + + // Create a new slightly different cron schedule that could + // potentially make NextStartAt invalid. + sched, err = cron.Weekly("CRON_TZ=UTC 0 9 * * 1-6") + require.NoError(t, err) + ctx := testutil.Context(t, testutil.WaitShort) + + // We want to test the database nullifies the NextStartAt so we + // make a raw DB call here. We pass in NextStartAt here so we + // can test the database will nullify it and not us. + //nolint: gocritic // We need system context to modify this. + err = db.UpdateWorkspaceAutostart(dbauthz.AsSystemRestricted(ctx), database.UpdateWorkspaceAutostartParams{ + ID: ws.ID, + AutostartSchedule: sql.NullString{Valid: true, String: sched.String()}, + NextStartAt: sql.NullTime{Valid: true, Time: *ws.NextStartAt}, + }) + require.NoError(t, err) + + ws = coderdtest.MustWorkspace(t, client, ws.ID) + + // Check 'NextStartAt' has been nullified + require.Nil(t, ws.NextStartAt) + + // Now we let the lifecycle executor run. This should spot that the + // NextStartAt is null and update it for us. + next := dbtime.Now() + tickCh <- next + stats := <-statsCh + assert.Len(t, stats.Errors, 0) + assert.Len(t, stats.Transitions, 0) + + // Ensure NextStartAt has been set, and is the expected value + ws = coderdtest.MustWorkspace(t, client, ws.ID) + require.NotNil(t, ws.NextStartAt) + require.Equal(t, sched.Next(next), ws.NextStartAt.UTC()) + }) } func TestTemplateDoesNotAllowUserAutostop(t *testing.T) { @@ -1682,90 +1766,6 @@ func TestAdminViewAllWorkspaces(t *testing.T) { require.Equal(t, 0, len(memberViewWorkspaces.Workspaces), "member in other org should see 0 workspaces") } -func TestNextStartAtIsNullifiedOnScheduleChange(t *testing.T) { - t.Parallel() - - if !dbtestutil.WillUsePostgres() { - t.Skip("this test uses triggers so does not work with dbmem.go") - } - - var ( - tickCh = make(chan time.Time) - statsCh = make(chan autobuild.Stats) - ) - - logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) - client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ - Options: &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerDaemon: true, - AutobuildStats: statsCh, - Logger: &logger, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), - }, - LicenseOptions: &coderdenttest.LicenseOptions{ - Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, - }, - }) - - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) - - // Create a template that allows autostart Monday-Sunday - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { - ctr.AutostartRequirement = &codersdk.TemplateAutostartRequirement{DaysOfWeek: codersdk.AllDaysOfWeek} - }) - require.Equal(t, version.ID, template.ActiveVersionID) - - // Create a workspace with a schedule Sunday-Saturday - sched, err := cron.Weekly("CRON_TZ=UTC 0 9 * * 0-6") - require.NoError(t, err) - ws := coderdtest.CreateWorkspace(t, client, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { - cwr.AutostartSchedule = ptr.Ref(sched.String()) - }) - - coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) - ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) - - // Check we have a 'NextStartAt' - require.NotNil(t, ws.NextStartAt) - - // Create a new slightly different cron schedule that could - // potentially make NextStartAt invalid. - sched, err = cron.Weekly("CRON_TZ=UTC 0 9 * * 1-6") - require.NoError(t, err) - ctx := testutil.Context(t, testutil.WaitShort) - - // We want to test the database nullifies the NextStartAt so we - // make a raw DB call here. We pass in NextStartAt here so we - // can test the database will nullify it and not us. - //nolint: gocritic // We need system context to modify this. - err = db.UpdateWorkspaceAutostart(dbauthz.AsSystemRestricted(ctx), database.UpdateWorkspaceAutostartParams{ - ID: ws.ID, - AutostartSchedule: sql.NullString{Valid: true, String: sched.String()}, - NextStartAt: sql.NullTime{Valid: true, Time: *ws.NextStartAt}, - }) - require.NoError(t, err) - - ws = coderdtest.MustWorkspace(t, client, ws.ID) - - // Check 'NextStartAt' has been nullified - require.Nil(t, ws.NextStartAt) - - // Now we let the lifecycle executor run. This should spot that the - // NextStartAt is null and update it for us. - next := dbtime.Now() - tickCh <- next - stats := <-statsCh - assert.Len(t, stats.Errors, 0) - assert.Len(t, stats.Transitions, 0) - - // Ensure NextStartAt has been set, and is the expected value - ws = coderdtest.MustWorkspace(t, client, ws.ID) - require.NotNil(t, ws.NextStartAt) - require.Equal(t, sched.Next(next), ws.NextStartAt.UTC()) -} - func must[T any](value T, err error) T { if err != nil { panic(err) From 23b2ec909c03ae969b0f3f944035b4da4077645e Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 28 Nov 2024 09:08:19 +0000 Subject: [PATCH 27/30] chore: nits --- coderd/autobuild/lifecycle_executor.go | 2 +- coderd/autobuild/lifecycle_executor_test.go | 3 +-- coderd/schedule/autostart_test.go | 2 +- coderd/workspaces.go | 4 ++-- enterprise/coderd/schedule/template.go | 4 ++-- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 20cb5a130beee..c238e4b0c7939 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -209,7 +209,7 @@ func (e *Executor) runOnce(t time.Time) Stats { if !ws.NextStartAt.Valid && ws.AutostartSchedule.Valid { next, err := schedule.NextAllowedAutostart(currentTick, ws.AutostartSchedule.String, templateSchedule) if err == nil { - nextStartAt := sql.NullTime{Valid: true, Time: next.UTC()} + nextStartAt := sql.NullTime{Valid: true, Time: dbtime.Time(next.UTC())} if err = tx.UpdateWorkspaceNextStartAt(e.ctx, database.UpdateWorkspaceNextStartAtParams{ ID: wsID, NextStartAt: nextStartAt, diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 54064e7fa4b6b..794d99f778446 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -1104,8 +1104,7 @@ func TestNotifications(t *testing.T) { coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { - timeTilDormantMillis := timeTilDormant.Milliseconds() - ctr.TimeTilDormantMillis = &timeTilDormantMillis + ctr.TimeTilDormantMillis = ptr.Ref(timeTilDormant.Milliseconds()) }) userClient, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID) workspace := coderdtest.CreateWorkspace(t, userClient, template.ID) diff --git a/coderd/schedule/autostart_test.go b/coderd/schedule/autostart_test.go index d172c5aa52f75..6dacee14614d7 100644 --- a/coderd/schedule/autostart_test.go +++ b/coderd/schedule/autostart_test.go @@ -26,7 +26,7 @@ func TestNextAllowedAutostart(t *testing.T) { }, } - // NextAutostart wil, return a non-allowed autostart time as + // NextAutostart will return a non-allowed autostart time as // our AutostartRequirement only allows Mondays but we expect // this to return a Tuesday. next, allowed := schedule.NextAutostart(at, sched, opts) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index c61ba21a2c73e..4f1cd31700eca 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -559,7 +559,7 @@ func createWorkspace( if dbAutostartSchedule.Valid { next, err := schedule.NextAllowedAutostart(dbtime.Now(), dbAutostartSchedule.String, templateSchedule) if err == nil { - nextStartAt = sql.NullTime{Valid: true, Time: next.UTC()} + nextStartAt = sql.NullTime{Valid: true, Time: dbtime.Time(next.UTC())} } } @@ -887,7 +887,7 @@ func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) { if dbSched.Valid { next, err := schedule.NextAllowedAutostart(dbtime.Now(), dbSched.String, templateSchedule) if err == nil { - nextStartAt = sql.NullTime{Valid: true, Time: next.UTC()} + nextStartAt = sql.NullTime{Valid: true, Time: dbtime.Time(next.UTC())} } } diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 3c93bac1286b3..82ec97b531a5a 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -31,7 +31,7 @@ type EnterpriseTemplateScheduleStore struct { // update. UserQuietHoursScheduleStore *atomic.Pointer[agpl.UserQuietHoursScheduleStore] - // Custom time.Now() function to use in tests. Defaults to dbtime.Now(). + // Clock for testing Clock quartz.Clock enqueuer notifications.Enqueuer @@ -228,7 +228,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S if workspace.AutostartSchedule.Valid { next, err := agpl.NextAllowedAutostart(s.now(), workspace.AutostartSchedule.String, templateSchedule) if err == nil { - nextStartAt = next.UTC() + nextStartAt = dbtime.Time(next.UTC()) } } From 8b1b6d90d4c66d03f925f57bc1b77672491b76dc Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 28 Nov 2024 09:13:02 +0000 Subject: [PATCH 28/30] chore: bump migration number --- ..._start_at.down.sql => 000278_workspace_next_start_at.down.sql} | 0 ...next_start_at.up.sql => 000278_workspace_next_start_at.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000277_workspace_next_start_at.down.sql => 000278_workspace_next_start_at.down.sql} (100%) rename coderd/database/migrations/{000277_workspace_next_start_at.up.sql => 000278_workspace_next_start_at.up.sql} (100%) diff --git a/coderd/database/migrations/000277_workspace_next_start_at.down.sql b/coderd/database/migrations/000278_workspace_next_start_at.down.sql similarity index 100% rename from coderd/database/migrations/000277_workspace_next_start_at.down.sql rename to coderd/database/migrations/000278_workspace_next_start_at.down.sql diff --git a/coderd/database/migrations/000277_workspace_next_start_at.up.sql b/coderd/database/migrations/000278_workspace_next_start_at.up.sql similarity index 100% rename from coderd/database/migrations/000277_workspace_next_start_at.up.sql rename to coderd/database/migrations/000278_workspace_next_start_at.up.sql From 2d239a6d8dea9b74bd89b34da01a64337ae906d4 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 28 Nov 2024 14:51:02 +0000 Subject: [PATCH 29/30] fix: stop query returning dormant workspaces --- coderd/autobuild/lifecycle_executor.go | 2 +- coderd/database/dump.sql | 13 +++++++++---- .../000278_workspace_next_start_at.down.sql | 7 +++++-- .../000278_workspace_next_start_at.up.sql | 13 +++++++++---- coderd/database/queries.sql.go | 12 ++++++++---- coderd/database/queries/workspaces.sql | 12 ++++++++---- 6 files changed, 40 insertions(+), 19 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index c238e4b0c7939..66e577f3a8603 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -205,7 +205,7 @@ func (e *Executor) runOnce(t time.Time) Stats { return xerrors.Errorf("get template scheduling options: %w", err) } - // If next start at is not valid we need to re-compute it. + // If next start at is not valid we need to re-compute it if !ws.NextStartAt.Valid && ws.AutostartSchedule.Valid { next, err := schedule.NextAllowedAutostart(currentTick, ws.AutostartSchedule.String, templateSchedule) if err == nil { diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 91d74906f1206..e08f01bfac0c0 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -380,12 +380,17 @@ BEGIN END; $$; -CREATE FUNCTION nullify_workspace_next_start_at() RETURNS trigger +CREATE FUNCTION nullify_next_start_at_on_workspace_autostart_modification() RETURNS trigger LANGUAGE plpgsql AS $$ DECLARE BEGIN - IF (NEW.autostart_schedule <> OLD.autostart_schedule) AND (NEW.next_start_at = OLD.next_start_at) THEN + -- A workspace's next_start_at might be invalidated by the following: + -- * The autostart schedule has changed independent to next_start_at + -- * The workspace has been marked as dormant + IF (NEW.autostart_schedule <> OLD.autostart_schedule AND NEW.next_start_at = OLD.next_start_at) + OR (NEW.dormant_at IS NOT NULL AND NEW.next_start_at IS NOT NULL) + THEN UPDATE workspaces SET next_start_at = NULL WHERE id = NEW.id; @@ -2210,9 +2215,9 @@ CREATE TRIGGER trigger_delete_oauth2_provider_app_token AFTER DELETE ON oauth2_p CREATE TRIGGER trigger_insert_apikeys BEFORE INSERT ON api_keys FOR EACH ROW EXECUTE FUNCTION insert_apikey_fail_if_user_deleted(); -CREATE TRIGGER trigger_update_users AFTER INSERT OR UPDATE ON users FOR EACH ROW WHEN ((new.deleted = true)) EXECUTE FUNCTION delete_deleted_user_resources(); +CREATE TRIGGER trigger_nullify_next_start_at_on_workspace_autostart_modificati AFTER UPDATE ON workspaces FOR EACH ROW EXECUTE FUNCTION nullify_next_start_at_on_workspace_autostart_modification(); -CREATE TRIGGER trigger_update_workspaces_schedule AFTER UPDATE ON workspaces FOR EACH ROW EXECUTE FUNCTION nullify_workspace_next_start_at(); +CREATE TRIGGER trigger_update_users AFTER INSERT OR UPDATE ON users FOR EACH ROW WHEN ((new.deleted = true)) EXECUTE FUNCTION delete_deleted_user_resources(); CREATE TRIGGER trigger_upsert_user_links BEFORE INSERT OR UPDATE ON user_links FOR EACH ROW EXECUTE FUNCTION insert_user_links_fail_if_user_deleted(); diff --git a/coderd/database/migrations/000278_workspace_next_start_at.down.sql b/coderd/database/migrations/000278_workspace_next_start_at.down.sql index a0eab4835e63b..0c15cb13adb1e 100644 --- a/coderd/database/migrations/000278_workspace_next_start_at.down.sql +++ b/coderd/database/migrations/000278_workspace_next_start_at.down.sql @@ -1,7 +1,10 @@ DROP VIEW workspaces_expanded; -DROP TRIGGER IF EXISTS trigger_update_workspaces_schedule ON workspaces; -DROP FUNCTION IF EXISTS nullify_workspace_next_start_at; +DROP TRIGGER IF EXISTS trigger_nullify_next_start_at_on_template_autostart_modification ON templates; +DROP FUNCTION IF EXISTS nullify_next_start_at_on_template_autostart_modification; + +DROP TRIGGER IF EXISTS trigger_nullify_next_start_at_on_workspace_autostart_modification ON workspaces; +DROP FUNCTION IF EXISTS nullify_next_start_at_on_workspace_autostart_modification; DROP INDEX workspace_next_start_at_idx; diff --git a/coderd/database/migrations/000278_workspace_next_start_at.up.sql b/coderd/database/migrations/000278_workspace_next_start_at.up.sql index 65be1e92f92b1..93bfb32cb2e2c 100644 --- a/coderd/database/migrations/000278_workspace_next_start_at.up.sql +++ b/coderd/database/migrations/000278_workspace_next_start_at.up.sql @@ -2,12 +2,17 @@ ALTER TABLE ONLY workspaces ADD COLUMN IF NOT EXISTS next_start_at TIMESTAMPTZ D CREATE INDEX workspace_next_start_at_idx ON workspaces USING btree (next_start_at) WHERE (deleted=false); -CREATE FUNCTION nullify_workspace_next_start_at() RETURNS trigger +CREATE FUNCTION nullify_next_start_at_on_workspace_autostart_modification() RETURNS trigger LANGUAGE plpgsql AS $$ DECLARE BEGIN - IF (NEW.autostart_schedule <> OLD.autostart_schedule) AND (NEW.next_start_at = OLD.next_start_at) THEN + -- A workspace's next_start_at might be invalidated by the following: + -- * The autostart schedule has changed independent to next_start_at + -- * The workspace has been marked as dormant + IF (NEW.autostart_schedule <> OLD.autostart_schedule AND NEW.next_start_at = OLD.next_start_at) + OR (NEW.dormant_at IS NOT NULL AND NEW.next_start_at IS NOT NULL) + THEN UPDATE workspaces SET next_start_at = NULL WHERE id = NEW.id; @@ -16,10 +21,10 @@ BEGIN END; $$; -CREATE TRIGGER trigger_update_workspaces_schedule +CREATE TRIGGER trigger_nullify_next_start_at_on_workspace_autostart_modification AFTER UPDATE ON workspaces FOR EACH ROW -EXECUTE PROCEDURE nullify_workspace_next_start_at(); +EXECUTE PROCEDURE nullify_next_start_at_on_workspace_autostart_modification(); -- Recreate view DROP VIEW workspaces_expanded; diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a9e9ae17510f3..4a97519a99f45 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -15749,18 +15749,22 @@ WHERE -- * The workspace's owner is active. -- * The provisioner job did not fail. -- * The workspace build was a stop transition. + -- * The workspace is not dormant -- * The workspace has an autostart schedule. -- * It is after the workspace's next start time. ( users.status = 'active'::user_status AND provisioner_jobs.job_status != 'failed'::provisioner_job_status AND workspace_builds.transition = 'stop'::workspace_transition AND + workspaces.dormant_at IS NULL AND workspaces.autostart_schedule IS NOT NULL AND ( - -- next_start_at might be null when a coder instance has been updated - -- and we haven't yet had an opportunity to set next_start_at. When - -- this happens we leave it up to the Coder server to figure out if - -- the workspace is ready to autostart. + -- next_start_at might be null in these two scenarios: + -- * A coder instance was updated and we haven't updated next_start_at yet. + -- * A database trigger made it null because of an update to a related column. + -- + -- When this occurs, we return the workspace so the Coder server can + -- compute a valid next start at and update it. workspaces.next_start_at IS NULL OR workspaces.next_start_at <= $1 :: timestamptz ) diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index a398c6c8dca89..cdf4dfa5f0e3e 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -627,18 +627,22 @@ WHERE -- * The workspace's owner is active. -- * The provisioner job did not fail. -- * The workspace build was a stop transition. + -- * The workspace is not dormant -- * The workspace has an autostart schedule. -- * It is after the workspace's next start time. ( users.status = 'active'::user_status AND provisioner_jobs.job_status != 'failed'::provisioner_job_status AND workspace_builds.transition = 'stop'::workspace_transition AND + workspaces.dormant_at IS NULL AND workspaces.autostart_schedule IS NOT NULL AND ( - -- next_start_at might be null when a coder instance has been updated - -- and we haven't yet had an opportunity to set next_start_at. When - -- this happens we leave it up to the Coder server to figure out if - -- the workspace is ready to autostart. + -- next_start_at might be null in these two scenarios: + -- * A coder instance was updated and we haven't updated next_start_at yet. + -- * A database trigger made it null because of an update to a related column. + -- + -- When this occurs, we return the workspace so the Coder server can + -- compute a valid next start at and update it. workspaces.next_start_at IS NULL OR workspaces.next_start_at <= @now :: timestamptz ) From dec3d6cf85e0ec06878b9bf668f7316ab3c13b8f Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 2 Dec 2024 12:27:58 +0000 Subject: [PATCH 30/30] chore: add index to template_id on workspaces --- coderd/database/dump.sql | 2 ++ .../database/migrations/000278_workspace_next_start_at.down.sql | 1 + .../database/migrations/000278_workspace_next_start_at.up.sql | 1 + 3 files changed, 4 insertions(+) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index e08f01bfac0c0..782bc4969d799 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -2137,6 +2137,8 @@ CREATE UNIQUE INDEX workspace_proxies_lower_name_idx ON workspace_proxies USING CREATE INDEX workspace_resources_job_id_idx ON workspace_resources USING btree (job_id); +CREATE INDEX workspace_template_id_idx ON workspaces USING btree (template_id) WHERE (deleted = false); + CREATE UNIQUE INDEX workspaces_owner_id_lower_idx ON workspaces USING btree (owner_id, lower((name)::text)) WHERE (deleted = false); CREATE OR REPLACE VIEW provisioner_job_stats AS diff --git a/coderd/database/migrations/000278_workspace_next_start_at.down.sql b/coderd/database/migrations/000278_workspace_next_start_at.down.sql index 0c15cb13adb1e..f47b190b59763 100644 --- a/coderd/database/migrations/000278_workspace_next_start_at.down.sql +++ b/coderd/database/migrations/000278_workspace_next_start_at.down.sql @@ -6,6 +6,7 @@ DROP FUNCTION IF EXISTS nullify_next_start_at_on_template_autostart_modification DROP TRIGGER IF EXISTS trigger_nullify_next_start_at_on_workspace_autostart_modification ON workspaces; DROP FUNCTION IF EXISTS nullify_next_start_at_on_workspace_autostart_modification; +DROP INDEX workspace_template_id_idx; DROP INDEX workspace_next_start_at_idx; ALTER TABLE ONLY workspaces DROP COLUMN IF EXISTS next_start_at; diff --git a/coderd/database/migrations/000278_workspace_next_start_at.up.sql b/coderd/database/migrations/000278_workspace_next_start_at.up.sql index 93bfb32cb2e2c..81240d6e08451 100644 --- a/coderd/database/migrations/000278_workspace_next_start_at.up.sql +++ b/coderd/database/migrations/000278_workspace_next_start_at.up.sql @@ -1,6 +1,7 @@ ALTER TABLE ONLY workspaces ADD COLUMN IF NOT EXISTS next_start_at TIMESTAMPTZ DEFAULT NULL; CREATE INDEX workspace_next_start_at_idx ON workspaces USING btree (next_start_at) WHERE (deleted=false); +CREATE INDEX workspace_template_id_idx ON workspaces USING btree (template_id) WHERE (deleted=false); CREATE FUNCTION nullify_next_start_at_on_workspace_autostart_modification() RETURNS trigger LANGUAGE plpgsql