From c7dff0e4519084bd509c477f693fb99f5096daea Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 24 May 2022 16:09:43 +0000 Subject: [PATCH 01/11] add initial deadline field to workspace_build --- coderd/coderd.go | 1 + coderd/workspaces.go | 29 +++++++++++++++++++++++++++++ coderd/workspaces_test.go | 25 +++++++++++++++++++++++++ codersdk/workspaces.go | 20 ++++++++++++++++++++ site/src/api/typesGenerated.ts | 7 ++++++- 5 files changed, 81 insertions(+), 1 deletion(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 2987b67364088..694518852317e 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -315,6 +315,7 @@ func New(options *Options) *API { r.Put("/", api.putWorkspaceTTL) }) r.Get("/watch", api.watchWorkspace) + r.Put("/deadline", api.putWorkspaceDeadline) }) }) r.Route("/workspacebuilds/{workspacebuild}", func(r chi.Router) { diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 340697a436baa..97dad77c972c3 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -570,6 +570,35 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) { } } +func (api *API) putWorkspaceDeadline(rw http.ResponseWriter, r *http.Request) { + workspace := httpmw.WorkspaceParam(r) + + if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceWorkspace. + InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + return + } + + var req codersdk.PutExtendWorkspaceRequest + if !httpapi.Read(rw, r, &req) { + return + } + + build, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: fmt.Sprintf("get latest workspace build: %s", err), + }) + return + } + + if build.Transition != database.WorkspaceTransitionStart { + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: fmt.Sprintf("workspace must be started, current status: %s", build.Transition), + }) + return + } +} + func (api *API) watchWorkspace(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 97191ec95f0ac..15a6b545cb73a 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -615,6 +615,31 @@ func TestWorkspaceUpdateAutostop(t *testing.T) { }) } +func TestWorkspaceExtendAutostop(t *testing.T) { + t.Parallel() + var ( + ctx = context.Background() + client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user = coderdtest.CreateFirstUser(t, client) + version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID) + extend = 90 * time.Minute + ) + + initTTL := time.Now() + req := codersdk.PutExtendWorkspaceRequest{ + Deadline: initTTL.Add(extend), + } + err := client.PutExtendWorkspace(ctx, workspace.ID, req) + require.NoError(t, err, "failed to update workspace ttl") + + updated, err := client.Workspace(ctx, workspace.ID) + require.NoError(t, err, "failed to fetch updated workspace") + require.Equal(t, workspace.LatestBuild.Deadline.Add(extend), updated.LatestBuild.Deadline) +} + func TestWorkspaceWatcher(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index a8b39d4dc1057..6fcb72cd10260 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -177,6 +177,26 @@ func (c *Client) UpdateWorkspaceTTL(ctx context.Context, id uuid.UUID, req Updat return nil } +// PutExtendWorkspaceRequest is a request to extend the deadline of +// the active workspace build. +type PutExtendWorkspaceRequest struct { + Deadline time.Time `json:"deadline" validate:"required, datetime=RFC3339"` +} + +// PutExtendWorkspace updates the deadline for resources of the latest workspace build. +func (c *Client) PutExtendWorkspace(ctx context.Context, id uuid.UUID, req PutExtendWorkspaceRequest) error { + path := fmt.Sprintf("/api/v2/workspaces/%s/extend", id.String()) + res, err := c.Request(ctx, http.MethodPut, path, req) + if err != nil { + return xerrors.Errorf("extend workspace ttl: %w", err) + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return readBodyAsError(res) + } + return nil +} + type WorkspaceFilter struct { OrganizationID uuid.UUID // Owner can be a user_id (uuid), "me", or a username diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 9248dc179d4eb..40b74ec7a06d2 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -216,6 +216,11 @@ export interface ProvisionerJobLog { readonly output: string } +// From codersdk/workspaces.go:182:6 +export interface PutWorkspaceDeadlineRequest { + readonly stop_at: string +} + // From codersdk/roles.go:12:6 export interface Role { readonly name: string @@ -430,7 +435,7 @@ export interface WorkspaceBuildsRequest extends Pagination { readonly WorkspaceID: string } -// From codersdk/workspaces.go:180:6 +// From codersdk/workspaces.go:200:6 export interface WorkspaceFilter { readonly OrganizationID: string readonly Owner: string From 8987e9ec38e4aa00d224d0d3bb786367e92ae5b4 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 25 May 2022 20:01:11 +0000 Subject: [PATCH 02/11] RED: add deadline to workspace builds --- .../autobuild/executor/lifecycle_executor.go | 10 +++---- coderd/database/databasefake/databasefake.go | 1 + coderd/database/dump.sql | 3 ++- .../000014_workspacebuild_deadline.down.sql | 1 + .../000014_workspacebuild_deadline.up.sql | 1 + coderd/database/models.go | 1 + coderd/database/queries.sql.go | 26 +++++++++++++------ coderd/database/queries/workspacebuilds.sql | 5 ++-- codersdk/workspacebuilds.go | 1 + codersdk/workspaces.go | 1 + site/src/api/typesGenerated.ts | 18 +++++++------ 11 files changed, 43 insertions(+), 25 deletions(-) create mode 100644 coderd/database/migrations/000014_workspacebuild_deadline.down.sql create mode 100644 coderd/database/migrations/000014_workspacebuild_deadline.up.sql diff --git a/coderd/autobuild/executor/lifecycle_executor.go b/coderd/autobuild/executor/lifecycle_executor.go index a6c004fa2de85..46a411a7babf7 100644 --- a/coderd/autobuild/executor/lifecycle_executor.go +++ b/coderd/autobuild/executor/lifecycle_executor.go @@ -88,18 +88,16 @@ func (e *Executor) runOnce(t time.Time) error { switch priorHistory.Transition { case database.WorkspaceTransitionStart: validTransition = database.WorkspaceTransitionStop - if !ws.Ttl.Valid || ws.Ttl.Int64 == 0 { - e.log.Debug(e.ctx, "invalid or zero ws ttl, skipping", + if priorHistory.Deadline.IsZero() { + e.log.Debug(e.ctx, "latest workspace build has zero deadline, skipping", slog.F("workspace_id", ws.ID), - slog.F("ttl", time.Duration(ws.Ttl.Int64)), + slog.F("workspace_build_id", priorHistory.ID), ) continue } - ttl := time.Duration(ws.Ttl.Int64) - // Measure TTL from the time the workspace finished building. // Truncate to nearest minute for consistency with autostart // behavior, and add one minute for padding. - nextTransition = priorHistory.UpdatedAt.Truncate(time.Minute).Add(ttl + time.Minute) + nextTransition = priorHistory.Deadline.Truncate(time.Minute) case database.WorkspaceTransitionStop: validTransition = database.WorkspaceTransitionStart sched, err := schedule.Weekly(ws.AutostartSchedule.String) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 4fdbcbc275960..b01e691d2d5b6 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1485,6 +1485,7 @@ func (q *fakeQuerier) InsertWorkspaceBuild(_ context.Context, arg database.Inser InitiatorID: arg.InitiatorID, JobID: arg.JobID, ProvisionerState: arg.ProvisionerState, + Deadline: arg.Deadline, } q.workspaceBuilds = append(q.workspaceBuilds, workspaceBuild) return workspaceBuild, nil diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 6cbae1af44b9d..a36f04d003474 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -291,7 +291,8 @@ CREATE TABLE workspace_builds ( transition workspace_transition NOT NULL, initiator_id uuid NOT NULL, provisioner_state bytea, - job_id uuid NOT NULL + job_id uuid NOT NULL, + deadline timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL ); CREATE TABLE workspace_resources ( diff --git a/coderd/database/migrations/000014_workspacebuild_deadline.down.sql b/coderd/database/migrations/000014_workspacebuild_deadline.down.sql new file mode 100644 index 0000000000000..e7366a3a6f3a6 --- /dev/null +++ b/coderd/database/migrations/000014_workspacebuild_deadline.down.sql @@ -0,0 +1 @@ +ALTER TABLE ONLY workspace_builds DROP COLUMN deadline; diff --git a/coderd/database/migrations/000014_workspacebuild_deadline.up.sql b/coderd/database/migrations/000014_workspacebuild_deadline.up.sql new file mode 100644 index 0000000000000..529ffd5db8f69 --- /dev/null +++ b/coderd/database/migrations/000014_workspacebuild_deadline.up.sql @@ -0,0 +1 @@ +ALTER TABLE ONLY workspace_builds ADD COLUMN deadline TIMESTAMPTZ NOT NULL DEFAULT TIMESTAMPTZ '0001-01-01 00:00:00+00:00'; diff --git a/coderd/database/models.go b/coderd/database/models.go index 581e05af75147..dae316781354e 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -505,6 +505,7 @@ type WorkspaceBuild struct { InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` ProvisionerState []byte `db:"provisioner_state" json:"provisioner_state"` JobID uuid.UUID `db:"job_id" json:"job_id"` + Deadline time.Time `db:"deadline" json:"deadline"` } type WorkspaceResource struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index e951047b0e599..bcf2dcbe31611 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2745,7 +2745,7 @@ func (q *sqlQuerier) UpdateWorkspaceAgentConnectionByID(ctx context.Context, arg const getLatestWorkspaceBuildByWorkspaceID = `-- name: GetLatestWorkspaceBuildByWorkspaceID :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id + id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id, deadline FROM workspace_builds WHERE @@ -2771,12 +2771,13 @@ func (q *sqlQuerier) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, w &i.InitiatorID, &i.ProvisionerState, &i.JobID, + &i.Deadline, ) return i, err } const getLatestWorkspaceBuildsByWorkspaceIDs = `-- name: GetLatestWorkspaceBuildsByWorkspaceIDs :many -SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.name, wb.build_number, wb.transition, wb.initiator_id, wb.provisioner_state, wb.job_id +SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.name, wb.build_number, wb.transition, wb.initiator_id, wb.provisioner_state, wb.job_id, wb.deadline FROM ( SELECT workspace_id, MAX(build_number) as max_build_number @@ -2813,6 +2814,7 @@ func (q *sqlQuerier) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, &i.InitiatorID, &i.ProvisionerState, &i.JobID, + &i.Deadline, ); err != nil { return nil, err } @@ -2829,7 +2831,7 @@ func (q *sqlQuerier) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, const getWorkspaceBuildByID = `-- name: GetWorkspaceBuildByID :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id + id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id, deadline FROM workspace_builds WHERE @@ -2853,13 +2855,14 @@ func (q *sqlQuerier) GetWorkspaceBuildByID(ctx context.Context, id uuid.UUID) (W &i.InitiatorID, &i.ProvisionerState, &i.JobID, + &i.Deadline, ) return i, err } const getWorkspaceBuildByJobID = `-- name: GetWorkspaceBuildByJobID :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id + id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id, deadline FROM workspace_builds WHERE @@ -2883,13 +2886,14 @@ func (q *sqlQuerier) GetWorkspaceBuildByJobID(ctx context.Context, jobID uuid.UU &i.InitiatorID, &i.ProvisionerState, &i.JobID, + &i.Deadline, ) return i, err } const getWorkspaceBuildByWorkspaceID = `-- name: GetWorkspaceBuildByWorkspaceID :many SELECT - id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id + id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id, deadline FROM workspace_builds WHERE @@ -2953,6 +2957,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceID(ctx context.Context, arg Get &i.InitiatorID, &i.ProvisionerState, &i.JobID, + &i.Deadline, ); err != nil { return nil, err } @@ -2969,7 +2974,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceID(ctx context.Context, arg Get const getWorkspaceBuildByWorkspaceIDAndName = `-- name: GetWorkspaceBuildByWorkspaceIDAndName :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id + id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id, deadline FROM workspace_builds WHERE @@ -2997,6 +3002,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceIDAndName(ctx context.Context, &i.InitiatorID, &i.ProvisionerState, &i.JobID, + &i.Deadline, ) return i, err } @@ -3014,10 +3020,11 @@ INSERT INTO transition, initiator_id, job_id, - provisioner_state + provisioner_state, + deadline ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) RETURNING id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING id, created_at, updated_at, workspace_id, template_version_id, name, build_number, transition, initiator_id, provisioner_state, job_id, deadline ` type InsertWorkspaceBuildParams struct { @@ -3032,6 +3039,7 @@ type InsertWorkspaceBuildParams struct { InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` JobID uuid.UUID `db:"job_id" json:"job_id"` ProvisionerState []byte `db:"provisioner_state" json:"provisioner_state"` + Deadline time.Time `db:"deadline" json:"deadline"` } func (q *sqlQuerier) InsertWorkspaceBuild(ctx context.Context, arg InsertWorkspaceBuildParams) (WorkspaceBuild, error) { @@ -3047,6 +3055,7 @@ func (q *sqlQuerier) InsertWorkspaceBuild(ctx context.Context, arg InsertWorkspa arg.InitiatorID, arg.JobID, arg.ProvisionerState, + arg.Deadline, ) var i WorkspaceBuild err := row.Scan( @@ -3061,6 +3070,7 @@ func (q *sqlQuerier) InsertWorkspaceBuild(ctx context.Context, arg InsertWorkspa &i.InitiatorID, &i.ProvisionerState, &i.JobID, + &i.Deadline, ) return i, err } diff --git a/coderd/database/queries/workspacebuilds.sql b/coderd/database/queries/workspacebuilds.sql index 733725131eb9f..900415c0c1a42 100644 --- a/coderd/database/queries/workspacebuilds.sql +++ b/coderd/database/queries/workspacebuilds.sql @@ -101,10 +101,11 @@ INSERT INTO transition, initiator_id, job_id, - provisioner_state + provisioner_state, + deadline ) 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: UpdateWorkspaceBuildByID :exec UPDATE diff --git a/codersdk/workspacebuilds.go b/codersdk/workspacebuilds.go index 073fae6e88bf3..2a4b04c787a7a 100644 --- a/codersdk/workspacebuilds.go +++ b/codersdk/workspacebuilds.go @@ -32,6 +32,7 @@ type WorkspaceBuild struct { Transition WorkspaceTransition `json:"transition"` InitiatorID uuid.UUID `json:"initiator_id"` Job ProvisionerJob `json:"job"` + Deadline time.Time `json:"deadline"` } // WorkspaceBuild returns a single workspace build for a workspace. diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index 6fcb72cd10260..7b54690a34a42 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -28,6 +28,7 @@ type Workspace struct { Name string `json:"name"` AutostartSchedule string `json:"autostart_schedule"` TTL *time.Duration `json:"ttl"` + Deadline time.Time `json:"deadline"` } // CreateWorkspaceBuildRequest provides options to update the latest workspace build. diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 40b74ec7a06d2..3fd0130b195b1 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -82,7 +82,7 @@ export interface CreateUserRequest { readonly organization_id: string } -// From codersdk/workspaces.go:34:6 +// From codersdk/workspaces.go:35:6 export interface CreateWorkspaceBuildRequest { readonly template_version_id?: string readonly transition: WorkspaceTransition @@ -216,9 +216,9 @@ export interface ProvisionerJobLog { readonly output: string } -// From codersdk/workspaces.go:182:6 -export interface PutWorkspaceDeadlineRequest { - readonly stop_at: string +// From codersdk/workspaces.go:183:6 +export interface PutExtendWorkspaceRequest { + readonly deadline: string } // From codersdk/roles.go:12:6 @@ -292,12 +292,12 @@ export interface UpdateUserProfileRequest { readonly username: string } -// From codersdk/workspaces.go:141:6 +// From codersdk/workspaces.go:142:6 export interface UpdateWorkspaceAutostartRequest { readonly schedule: string } -// From codersdk/workspaces.go:161:6 +// From codersdk/workspaces.go:162:6 export interface UpdateWorkspaceTTLRequest { // This is likely an enum in an external package ("time.Duration") readonly ttl?: number @@ -368,6 +368,7 @@ export interface Workspace { readonly autostart_schedule: string // This is likely an enum in an external package ("time.Duration") readonly ttl?: number + readonly deadline: string } // From codersdk/workspaceresources.go:31:6 @@ -428,14 +429,15 @@ export interface WorkspaceBuild { readonly transition: WorkspaceTransition readonly initiator_id: string readonly job: ProvisionerJob + readonly deadline: string } -// From codersdk/workspaces.go:64:6 +// From codersdk/workspaces.go:65:6 export interface WorkspaceBuildsRequest extends Pagination { readonly WorkspaceID: string } -// From codersdk/workspaces.go:200:6 +// From codersdk/workspaces.go:201:6 export interface WorkspaceFilter { readonly OrganizationID: string readonly Owner: string From f6cb70520c6a3f2ee0c39e19d1d21064d7759005 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 25 May 2022 22:00:50 +0000 Subject: [PATCH 03/11] plumb deadline field through database --- coderd/coderd.go | 2 +- coderd/database/queries.sql.go | 11 +++- coderd/database/queries/workspacebuilds.sql | 3 +- coderd/provisionerdaemons.go | 16 ++++- coderd/workspacebuilds.go | 1 + coderd/workspaces.go | 40 +++++++++++- coderd/workspaces_test.go | 69 +++++++++++++++------ codersdk/workspaces.go | 5 +- site/src/api/typesGenerated.ts | 13 ++-- 9 files changed, 123 insertions(+), 37 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 694518852317e..ed8cba833ccf1 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -315,7 +315,7 @@ func New(options *Options) *API { r.Put("/", api.putWorkspaceTTL) }) r.Get("/watch", api.watchWorkspace) - r.Put("/deadline", api.putWorkspaceDeadline) + r.Put("/extend", api.putExtendWorkspace) }) }) r.Route("/workspacebuilds/{workspacebuild}", func(r chi.Router) { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index bcf2dcbe31611..137fe36b4456b 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3080,7 +3080,8 @@ UPDATE workspace_builds SET updated_at = $2, - provisioner_state = $3 + provisioner_state = $3, + deadline = $4 WHERE id = $1 ` @@ -3089,10 +3090,16 @@ type UpdateWorkspaceBuildByIDParams struct { ID uuid.UUID `db:"id" json:"id"` UpdatedAt time.Time `db:"updated_at" json:"updated_at"` ProvisionerState []byte `db:"provisioner_state" json:"provisioner_state"` + Deadline time.Time `db:"deadline" json:"deadline"` } func (q *sqlQuerier) UpdateWorkspaceBuildByID(ctx context.Context, arg UpdateWorkspaceBuildByIDParams) error { - _, err := q.db.ExecContext(ctx, updateWorkspaceBuildByID, arg.ID, arg.UpdatedAt, arg.ProvisionerState) + _, err := q.db.ExecContext(ctx, updateWorkspaceBuildByID, + arg.ID, + arg.UpdatedAt, + arg.ProvisionerState, + arg.Deadline, + ) return err } diff --git a/coderd/database/queries/workspacebuilds.sql b/coderd/database/queries/workspacebuilds.sql index 900415c0c1a42..5b53a874060e8 100644 --- a/coderd/database/queries/workspacebuilds.sql +++ b/coderd/database/queries/workspacebuilds.sql @@ -112,6 +112,7 @@ UPDATE workspace_builds SET updated_at = $2, - provisioner_state = $3 + provisioner_state = $3, + deadline = $4 WHERE id = $1; diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go index 13cfcd4dc8de2..518fc5205cfcc 100644 --- a/coderd/provisionerdaemons.go +++ b/coderd/provisionerdaemons.go @@ -473,6 +473,7 @@ func (server *provisionerdServer) FailJob(ctx context.Context, failJob *proto.Fa ID: input.WorkspaceBuildID, UpdatedAt: database.Now(), ProvisionerState: jobType.WorkspaceBuild.State, + // We are explicitly not updating deadline here. }) if err != nil { return nil, xerrors.Errorf("update workspace build state: %w", err) @@ -544,6 +545,18 @@ func (server *provisionerdServer) CompleteJob(ctx context.Context, completed *pr } err = server.Database.InTx(func(db database.Store) error { + now := database.Now() + var workspaceDeadline time.Time + workspace, err := db.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID) + if err == nil { + if workspace.Ttl.Valid { + workspaceDeadline = now.Add(time.Duration(workspace.Ttl.Int64)) + } + } else { + // Huh? Did the workspace get deleted? + // In any case, since this is just for the TTL, try and continue anyway. + server.Logger.Error(ctx, "fetch workspace for build", slog.F("workspace_build_id", workspaceBuild.ID), slog.F("workspace_id", workspaceBuild.WorkspaceID)) + } err = db.UpdateProvisionerJobWithCompleteByID(ctx, database.UpdateProvisionerJobWithCompleteByIDParams{ ID: jobID, UpdatedAt: database.Now(), @@ -556,9 +569,10 @@ func (server *provisionerdServer) CompleteJob(ctx context.Context, completed *pr return xerrors.Errorf("update provisioner job: %w", err) } err = db.UpdateWorkspaceBuildByID(ctx, database.UpdateWorkspaceBuildByIDParams{ + Deadline: now, ID: workspaceBuild.ID, - UpdatedAt: database.Now(), ProvisionerState: jobType.WorkspaceBuild.State, + UpdatedAt: workspaceDeadline, }) if err != nil { return xerrors.Errorf("update workspace build: %w", err) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index b07f38b7f068e..c19c900987873 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -445,6 +445,7 @@ func convertWorkspaceBuild(workspaceBuild database.WorkspaceBuild, job codersdk. Transition: codersdk.WorkspaceTransition(workspaceBuild.Transition), InitiatorID: workspaceBuild.InitiatorID, Job: job, + Deadline: workspaceBuild.Deadline, } } diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 97dad77c972c3..9c608fd626688 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -477,7 +477,8 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req InitiatorID: apiKey.UserID, Transition: database.WorkspaceTransitionStart, JobID: provisionerJob.ID, - BuildNumber: 1, // First build! + BuildNumber: 1, // First build! + Deadline: time.Time{}, // provisionerd will set this upon success }) if err != nil { return xerrors.Errorf("insert workspace build: %w", err) @@ -570,7 +571,7 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) { } } -func (api *API) putWorkspaceDeadline(rw http.ResponseWriter, r *http.Request) { +func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceWorkspace. @@ -597,6 +598,41 @@ func (api *API) putWorkspaceDeadline(rw http.ResponseWriter, r *http.Request) { }) return } + + newDeadline := req.Deadline.UTC() + if newDeadline.IsZero() { + // This should not be possible because the validation requires a non-zero value. + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: fmt.Sprintf("new deadline %q cannot be zero", newDeadline), + }) + return + } + + if newDeadline.Before(build.Deadline) { + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: fmt.Sprintf("new deadline %q must be after existing deadline %q", newDeadline, build.Deadline), + }) + return + } + + if newDeadline == build.Deadline { + httpapi.Write(rw, http.StatusNotModified, httpapi.Response{}) + return + } + + if err := api.Database.UpdateWorkspaceBuildByID(r.Context(), database.UpdateWorkspaceBuildByIDParams{ + ID: build.ID, + UpdatedAt: build.UpdatedAt, + ProvisionerState: build.ProvisionerState, + Deadline: newDeadline, + }); err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: err.Error(), + }) + return + } + + httpapi.Write(rw, http.StatusOK, httpapi.Response{}) } func (api *API) watchWorkspace(rw http.ResponseWriter, r *http.Request) { diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 15a6b545cb73a..ec385149aa078 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -617,27 +617,56 @@ func TestWorkspaceUpdateAutostop(t *testing.T) { func TestWorkspaceExtendAutostop(t *testing.T) { t.Parallel() - var ( - ctx = context.Background() - client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) - user = coderdtest.CreateFirstUser(t, client) - version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID) - extend = 90 * time.Minute - ) - - initTTL := time.Now() - req := codersdk.PutExtendWorkspaceRequest{ - Deadline: initTTL.Add(extend), - } - err := client.PutExtendWorkspace(ctx, workspace.ID, req) - require.NoError(t, err, "failed to update workspace ttl") + t.Run("OK", func(t *testing.T) { + t.Parallel() + var ( + ctx = context.Background() + client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user = coderdtest.CreateFirstUser(t, client) + version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID) + extend = 90 * time.Minute + _ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + ) + + workspace, err := client.Workspace(ctx, workspace.ID) + require.NoError(t, err, "fetch provisioned workspace") + req := codersdk.PutExtendWorkspaceRequest{ + Deadline: workspace.LatestBuild.UpdatedAt.Add(extend), + } + err = client.PutExtendWorkspace(ctx, workspace.ID, req) + require.NoError(t, err, "failed to extend workspace") + + updated, err := client.Workspace(ctx, workspace.ID) + require.NoError(t, err, "failed to fetch updated workspace") + require.Equal(t, workspace.LatestBuild.Deadline.Add(extend), updated.LatestBuild.Deadline) + }) - updated, err := client.Workspace(ctx, workspace.ID) - require.NoError(t, err, "failed to fetch updated workspace") - require.Equal(t, workspace.LatestBuild.Deadline.Add(extend), updated.LatestBuild.Deadline) + t.Run("DeadlineZero", func(t *testing.T) { + t.Parallel() + var ( + ctx = context.Background() + client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user = coderdtest.CreateFirstUser(t, client) + version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID) + _ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + ) + + req := codersdk.PutExtendWorkspaceRequest{ + Deadline: time.Time{}, + } + err := client.PutExtendWorkspace(ctx, workspace.ID, req) + require.ErrorContains(t, err, "deadline: required", "failed to update workspace ttl") + + updated, err := client.Workspace(ctx, workspace.ID) + require.NoError(t, err, "failed to fetch updated workspace") + require.Equal(t, workspace.LatestBuild.Deadline, updated.LatestBuild.Deadline) + }) } func TestWorkspaceWatcher(t *testing.T) { diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index 7b54690a34a42..3ad1ac42fb830 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -28,7 +28,6 @@ type Workspace struct { Name string `json:"name"` AutostartSchedule string `json:"autostart_schedule"` TTL *time.Duration `json:"ttl"` - Deadline time.Time `json:"deadline"` } // CreateWorkspaceBuildRequest provides options to update the latest workspace build. @@ -181,7 +180,7 @@ func (c *Client) UpdateWorkspaceTTL(ctx context.Context, id uuid.UUID, req Updat // PutExtendWorkspaceRequest is a request to extend the deadline of // the active workspace build. type PutExtendWorkspaceRequest struct { - Deadline time.Time `json:"deadline" validate:"required, datetime=RFC3339"` + Deadline time.Time `json:"deadline" validate:"required"` } // PutExtendWorkspace updates the deadline for resources of the latest workspace build. @@ -192,7 +191,7 @@ func (c *Client) PutExtendWorkspace(ctx context.Context, id uuid.UUID, req PutEx return xerrors.Errorf("extend workspace ttl: %w", err) } defer res.Body.Close() - if res.StatusCode != http.StatusOK { + if res.StatusCode != http.StatusOK && res.StatusCode != http.StatusNotModified { return readBodyAsError(res) } return nil diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 3fd0130b195b1..57bc83ebc6dc3 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -82,7 +82,7 @@ export interface CreateUserRequest { readonly organization_id: string } -// From codersdk/workspaces.go:35:6 +// From codersdk/workspaces.go:34:6 export interface CreateWorkspaceBuildRequest { readonly template_version_id?: string readonly transition: WorkspaceTransition @@ -216,7 +216,7 @@ export interface ProvisionerJobLog { readonly output: string } -// From codersdk/workspaces.go:183:6 +// From codersdk/workspaces.go:182:6 export interface PutExtendWorkspaceRequest { readonly deadline: string } @@ -292,12 +292,12 @@ export interface UpdateUserProfileRequest { readonly username: string } -// From codersdk/workspaces.go:142:6 +// From codersdk/workspaces.go:141:6 export interface UpdateWorkspaceAutostartRequest { readonly schedule: string } -// From codersdk/workspaces.go:162:6 +// From codersdk/workspaces.go:161:6 export interface UpdateWorkspaceTTLRequest { // This is likely an enum in an external package ("time.Duration") readonly ttl?: number @@ -368,7 +368,6 @@ export interface Workspace { readonly autostart_schedule: string // This is likely an enum in an external package ("time.Duration") readonly ttl?: number - readonly deadline: string } // From codersdk/workspaceresources.go:31:6 @@ -432,12 +431,12 @@ export interface WorkspaceBuild { readonly deadline: string } -// From codersdk/workspaces.go:65:6 +// From codersdk/workspaces.go:64:6 export interface WorkspaceBuildsRequest extends Pagination { readonly WorkspaceID: string } -// From codersdk/workspaces.go:201:6 +// From codersdk/workspaces.go:200:6 export interface WorkspaceFilter { readonly OrganizationID: string readonly Owner: string From 759fb8a86684a271d5ef5799ff92d1c1a56c5284 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 26 May 2022 10:41:03 +0000 Subject: [PATCH 04/11] rename test --- coderd/workspaces_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index ec385149aa078..49f0e901b9ba4 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -538,7 +538,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) { }) } -func TestWorkspaceUpdateAutostop(t *testing.T) { +func TestWorkspaceUpdateTTL(t *testing.T) { t.Parallel() testCases := []struct { From b92c86e0bb7a42d519e253aad79a31ac8a5c07bf Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 26 May 2022 12:22:17 +0000 Subject: [PATCH 05/11] GREEN: api complete --- .../executor/lifecycle_executor_test.go | 6 +- coderd/database/databasefake/databasefake.go | 1 + coderd/provisionerdaemons.go | 4 +- coderd/workspaces.go | 85 +++++++++--------- coderd/workspaces_test.go | 90 +++++++++---------- 5 files changed, 91 insertions(+), 95 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index 203355d53efff..23ea13e42e62e 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -190,14 +190,14 @@ func TestExecutorAutostopOK(t *testing.T) { }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) - ttl = *workspace.TTL ) // Given: workspace is running require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition) + require.NotZero(t, workspace.LatestBuild.Deadline) - // When: the autobuild executor ticks *after* the TTL: + // When: the autobuild executor ticks *after* the deadline: go func() { - tickCh <- time.Now().UTC().Add(ttl + time.Minute) + tickCh <- workspace.LatestBuild.Deadline.Add(time.Minute) close(tickCh) }() diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index b01e691d2d5b6..6acc5d156bdac 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1694,6 +1694,7 @@ func (q *fakeQuerier) UpdateWorkspaceBuildByID(_ context.Context, arg database.U } workspaceBuild.UpdatedAt = arg.UpdatedAt workspaceBuild.ProvisionerState = arg.ProvisionerState + workspaceBuild.Deadline = arg.Deadline q.workspaceBuilds[index] = workspaceBuild return nil } diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go index 518fc5205cfcc..719cc28c69a9d 100644 --- a/coderd/provisionerdaemons.go +++ b/coderd/provisionerdaemons.go @@ -569,10 +569,10 @@ func (server *provisionerdServer) CompleteJob(ctx context.Context, completed *pr return xerrors.Errorf("update provisioner job: %w", err) } err = db.UpdateWorkspaceBuildByID(ctx, database.UpdateWorkspaceBuildByIDParams{ - Deadline: now, + Deadline: workspaceDeadline, ID: workspaceBuild.ID, ProvisionerState: jobType.WorkspaceBuild.State, - UpdatedAt: workspaceDeadline, + UpdatedAt: now, }) if err != nil { return xerrors.Errorf("update workspace build: %w", err) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 9c608fd626688..543fae6185dc7 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -584,55 +584,54 @@ func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) { return } - build, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) - if err != nil { - httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ - Message: fmt.Sprintf("get latest workspace build: %s", err), - }) - return - } + var code = http.StatusOK - if build.Transition != database.WorkspaceTransitionStart { - httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: fmt.Sprintf("workspace must be started, current status: %s", build.Transition), - }) - return - } + err := api.Database.InTx(func(s database.Store) error { + build, err := s.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) + if err != nil { + code = http.StatusInternalServerError + return xerrors.Errorf("get latest workspace build: %w", err) + } - newDeadline := req.Deadline.UTC() - if newDeadline.IsZero() { - // This should not be possible because the validation requires a non-zero value. - httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: fmt.Sprintf("new deadline %q cannot be zero", newDeadline), - }) - return - } + if build.Transition != database.WorkspaceTransitionStart { + code = http.StatusConflict + return xerrors.Errorf("workspace must be started, current status: %s", build.Transition) + } - if newDeadline.Before(build.Deadline) { - httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: fmt.Sprintf("new deadline %q must be after existing deadline %q", newDeadline, build.Deadline), - }) - return - } + newDeadline := req.Deadline.UTC() + if newDeadline.IsZero() { + // This should not be possible because the struct validation field enforces a non-zero value. + code = http.StatusBadRequest + return xerrors.New("new deadline cannot be zero") + } - if newDeadline == build.Deadline { - httpapi.Write(rw, http.StatusNotModified, httpapi.Response{}) - return - } + if newDeadline.Before(build.Deadline) { + code = http.StatusBadRequest + return xerrors.Errorf("new deadline %q must be after existing deadline %q", newDeadline.Format(time.RFC3339), build.Deadline.Format(time.RFC3339)) + } - if err := api.Database.UpdateWorkspaceBuildByID(r.Context(), database.UpdateWorkspaceBuildByIDParams{ - ID: build.ID, - UpdatedAt: build.UpdatedAt, - ProvisionerState: build.ProvisionerState, - Deadline: newDeadline, - }); err != nil { - httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ - Message: err.Error(), - }) - return - } + if newDeadline == build.Deadline { + code = http.StatusNotModified + return nil + } - httpapi.Write(rw, http.StatusOK, httpapi.Response{}) + if err := s.UpdateWorkspaceBuildByID(r.Context(), database.UpdateWorkspaceBuildByIDParams{ + ID: build.ID, + UpdatedAt: build.UpdatedAt, + ProvisionerState: build.ProvisionerState, + Deadline: newDeadline, + }); err != nil { + return xerrors.Errorf("update workspace build: %w", err) + } + + return nil + }) + + var resp = httpapi.Response{} + if err != nil { + resp.Message = err.Error() + } + httpapi.Write(rw, code, resp) } func (api *API) watchWorkspace(rw http.ResponseWriter, r *http.Request) { diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 49f0e901b9ba4..899abb5834e38 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -615,58 +615,54 @@ func TestWorkspaceUpdateTTL(t *testing.T) { }) } -func TestWorkspaceExtendAutostop(t *testing.T) { +func TestWorkspaceExtend(t *testing.T) { t.Parallel() - t.Run("OK", func(t *testing.T) { - t.Parallel() - var ( - ctx = context.Background() - client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) - user = coderdtest.CreateFirstUser(t, client) - version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID) - extend = 90 * time.Minute - _ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - ) - - workspace, err := client.Workspace(ctx, workspace.ID) - require.NoError(t, err, "fetch provisioned workspace") - req := codersdk.PutExtendWorkspaceRequest{ - Deadline: workspace.LatestBuild.UpdatedAt.Add(extend), - } - err = client.PutExtendWorkspace(ctx, workspace.ID, req) - require.NoError(t, err, "failed to extend workspace") - - updated, err := client.Workspace(ctx, workspace.ID) - require.NoError(t, err, "failed to fetch updated workspace") - require.Equal(t, workspace.LatestBuild.Deadline.Add(extend), updated.LatestBuild.Deadline) - }) + var ( + ctx = context.Background() + client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user = coderdtest.CreateFirstUser(t, client) + version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID) + extend = 90 * time.Minute + _ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + oldDeadline = time.Now().Add(*workspace.TTL).UTC() + newDeadline = time.Now().Add(*workspace.TTL + extend).UTC() + ) + + workspace, err := client.Workspace(ctx, workspace.ID) + require.NoError(t, err, "fetch provisioned workspace") + require.InDelta(t, oldDeadline.Unix(), workspace.LatestBuild.Deadline.Unix(), 1) + + // Updating the deadline should succeed + req := codersdk.PutExtendWorkspaceRequest{ + Deadline: newDeadline, + } + err = client.PutExtendWorkspace(ctx, workspace.ID, req) + require.NoError(t, err, "failed to extend workspace") - t.Run("DeadlineZero", func(t *testing.T) { - t.Parallel() - var ( - ctx = context.Background() - client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) - user = coderdtest.CreateFirstUser(t, client) - version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID) - _ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - ) + // Ensure deadline set correctly + updated, err := client.Workspace(ctx, workspace.ID) + require.NoError(t, err, "failed to fetch updated workspace") + require.InDelta(t, newDeadline.Unix(), updated.LatestBuild.Deadline.Unix(), 1) - req := codersdk.PutExtendWorkspaceRequest{ - Deadline: time.Time{}, - } - err := client.PutExtendWorkspace(ctx, workspace.ID, req) - require.ErrorContains(t, err, "deadline: required", "failed to update workspace ttl") + // Zero time should fail + err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{ + Deadline: time.Time{}, + }) + require.ErrorContains(t, err, "deadline: required", "setting an empty deadline on a workspace should fail") - updated, err := client.Workspace(ctx, workspace.ID) - require.NoError(t, err, "failed to fetch updated workspace") - require.Equal(t, workspace.LatestBuild.Deadline, updated.LatestBuild.Deadline) + // Updating with an earlier time should also fail + err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{ + Deadline: oldDeadline, }) + require.ErrorContains(t, err, "must be after existing deadline", "setting an earlier deadline should fail") + + // Ensure deadline still set correctly + updated, err = client.Workspace(ctx, workspace.ID) + require.NoError(t, err, "failed to fetch updated workspace") + require.InDelta(t, newDeadline.Unix(), updated.LatestBuild.Deadline.Unix(), 1) } func TestWorkspaceWatcher(t *testing.T) { From 597fbd35fd31083ee6b21363677fdb772cd7e776 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 26 May 2022 12:55:16 +0000 Subject: [PATCH 06/11] test: executor: add test case for extension --- .../executor/lifecycle_executor_test.go | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index 23ea13e42e62e..1a9a4545b6506 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -209,6 +209,55 @@ func TestExecutorAutostopOK(t *testing.T) { require.Equal(t, codersdk.WorkspaceTransitionStop, ws.LatestBuild.Transition, "expected workspace not to be running") } +func TestExecutorAutostopExtend(t *testing.T) { + t.Parallel() + + var ( + ctx = context.Background() + tickCh = make(chan time.Time) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + }) + // Given: we have a user with a workspace + workspace = mustProvisionWorkspace(t, client) + originalDeadline = workspace.LatestBuild.Deadline + ) + // Given: workspace is running + require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition) + require.NotZero(t, originalDeadline) + + // Given: we extend the workspace deadline + err := client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{ + Deadline: originalDeadline.Add(30 * time.Minute), + }) + require.NoError(t, err, "extend workspace deadline") + + // When: the autobuild executor ticks *after* the original deadline: + go func() { + tickCh <- originalDeadline.Add(time.Minute) + }() + + // Then: nothing should happen + <-time.After(5 * time.Second) + ws := mustWorkspace(t, client, workspace.ID) + require.Equal(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected no further workspace builds to occur") + require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected workspace to be running") + + // When: the autobuild executor ticks after the *new* deadline: + go func() { + tickCh <- ws.LatestBuild.Deadline.Add(time.Minute) + close(tickCh) + }() + + // Then: the workspace should be stopped + <-time.After(5 * time.Second) + ws = mustWorkspace(t, client, workspace.ID) + require.NotEqual(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected a workspace build to occur") + require.Equal(t, codersdk.ProvisionerJobSucceeded, ws.LatestBuild.Job.Status, "expected provisioner job to have succeeded") + require.Equal(t, codersdk.WorkspaceTransitionStop, ws.LatestBuild.Transition, "expected workspace not to be running") +} + func TestExecutorAutostopAlreadyStopped(t *testing.T) { t.Parallel() From b6fcb20a90acfccb7db264591c1b5034896cba9d Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 26 May 2022 13:25:12 +0000 Subject: [PATCH 07/11] autobuild: fixup unit tests, clarify behaviour --- .../autobuild/executor/lifecycle_executor.go | 3 ++ .../executor/lifecycle_executor_test.go | 41 ++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor.go b/coderd/autobuild/executor/lifecycle_executor.go index 46a411a7babf7..50fe73a33fc30 100644 --- a/coderd/autobuild/executor/lifecycle_executor.go +++ b/coderd/autobuild/executor/lifecycle_executor.go @@ -50,6 +50,9 @@ func (e *Executor) Run() { func (e *Executor) runOnce(t time.Time) error { currentTick := t.Truncate(time.Minute) return e.db.InTx(func(db database.Store) error { + // XXX: if a workspace build is created when ttl != null and then ttl is unset, + // autostop will still happen for this workspace build. Whether this is + // expected behavior from a user's perspective is not yet known. eligibleWorkspaces, err := db.GetWorkspacesAutostart(e.ctx) if err != nil { return xerrors.Errorf("get eligible workspaces for autostart or autostop: %w", err) diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index 1a9a4545b6506..94f8152208486 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -271,7 +271,6 @@ func TestExecutorAutostopAlreadyStopped(t *testing.T) { workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { cwr.AutostartSchedule = nil }) - ttl = *workspace.TTL ) // Given: workspace is stopped @@ -279,7 +278,7 @@ func TestExecutorAutostopAlreadyStopped(t *testing.T) { // When: the autobuild executor ticks past the TTL go func() { - tickCh <- time.Now().UTC().Add(ttl + time.Minute) + tickCh <- workspace.LatestBuild.Deadline.Add(time.Minute) close(tickCh) }() @@ -313,7 +312,7 @@ func TestExecutorAutostopNotEnabled(t *testing.T) { // When: the autobuild executor ticks past the TTL go func() { - tickCh <- time.Now().UTC().Add(time.Minute) + tickCh <- workspace.LatestBuild.Deadline.Add(time.Minute) close(tickCh) }() @@ -401,7 +400,7 @@ func TestExecutorWorkspaceAutostartTooEarly(t *testing.T) { require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected workspace to be running") } -func TestExecutorWorkspaceTTLTooEarly(t *testing.T) { +func TestExecutorWorkspaceAutostopBeforeDeadline(t *testing.T) { t.Parallel() var ( @@ -416,7 +415,7 @@ func TestExecutorWorkspaceTTLTooEarly(t *testing.T) { // When: the autobuild executor ticks before the TTL go func() { - tickCh <- time.Now().UTC() + tickCh <- workspace.LatestBuild.Deadline.Add(-1 * time.Minute) close(tickCh) }() @@ -427,6 +426,38 @@ func TestExecutorWorkspaceTTLTooEarly(t *testing.T) { require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected workspace to be running") } +func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) { + t.Parallel() + + var ( + ctx = context.Background() + tickCh = make(chan time.Time) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + }) + // Given: we have a user with a workspace + workspace = mustProvisionWorkspace(t, client) + ) + + // Given: the user changes their mind and decides their workspace should not auto-stop + err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTL: nil}) + require.NoError(t, err) + + // When: the autobuild executor ticks after the deadline + go func() { + tickCh <- workspace.LatestBuild.Deadline.Add(time.Minute) + close(tickCh) + }() + + // Then: the workspace should still stop - sorry! + <-time.After(5 * time.Second) + ws := mustWorkspace(t, client, workspace.ID) + require.NotEqual(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected a workspace build to occur") + require.Equal(t, codersdk.ProvisionerJobSucceeded, ws.LatestBuild.Job.Status, "expected provisioner job to have succeeded") + require.Equal(t, codersdk.WorkspaceTransitionStop, ws.LatestBuild.Transition, "expected workspace not to be running") +} + func TestExecutorAutostartMultipleOK(t *testing.T) { if os.Getenv("DB") == "" { t.Skip(`This test only really works when using a "real" database, similar to a HA setup`) From e3df43a6f1222948dbf84852d83441e0a1303eb9 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 26 May 2022 13:25:25 +0000 Subject: [PATCH 08/11] site: add missing field to mock entity --- site/src/testHelpers/entities.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 8c444cba34887..dd937e7b0a2bd 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -124,6 +124,7 @@ export const MockWorkspaceBuild: TypesGen.WorkspaceBuild = { transition: "start", updated_at: "2022-05-17T17:39:01.382927298Z", workspace_id: "test-workspace", + deadline: "2022-05-17T23:39:00.00Z", } export const MockWorkspaceBuildStop: TypesGen.WorkspaceBuild = { From b12e19eff8b36ce1e3eeaa84240e3cbd55e026d3 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 26 May 2022 14:32:57 +0000 Subject: [PATCH 09/11] address PR comments --- coderd/autobuild/executor/lifecycle_executor.go | 3 +-- coderd/provisionerdaemons.go | 4 ++-- coderd/workspaces.go | 8 ++++---- coderd/workspaces_test.go | 6 +++--- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor.go b/coderd/autobuild/executor/lifecycle_executor.go index 50fe73a33fc30..a26219619c4e6 100644 --- a/coderd/autobuild/executor/lifecycle_executor.go +++ b/coderd/autobuild/executor/lifecycle_executor.go @@ -98,8 +98,7 @@ func (e *Executor) runOnce(t time.Time) error { ) continue } - // Truncate to nearest minute for consistency with autostart - // behavior, and add one minute for padding. + // Truncate to nearest minute for consistency with autostart behavior nextTransition = priorHistory.Deadline.Truncate(time.Minute) case database.WorkspaceTransitionStop: validTransition = database.WorkspaceTransitionStart diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go index 719cc28c69a9d..023c238335a2b 100644 --- a/coderd/provisionerdaemons.go +++ b/coderd/provisionerdaemons.go @@ -550,7 +550,7 @@ func (server *provisionerdServer) CompleteJob(ctx context.Context, completed *pr workspace, err := db.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID) if err == nil { if workspace.Ttl.Valid { - workspaceDeadline = now.Add(time.Duration(workspace.Ttl.Int64)) + workspaceDeadline = now.Add(time.Duration(workspace.Ttl.Int64)).Truncate(time.Minute) } } else { // Huh? Did the workspace get deleted? @@ -569,8 +569,8 @@ func (server *provisionerdServer) CompleteJob(ctx context.Context, completed *pr return xerrors.Errorf("update provisioner job: %w", err) } err = db.UpdateWorkspaceBuildByID(ctx, database.UpdateWorkspaceBuildByIDParams{ - Deadline: workspaceDeadline, ID: workspaceBuild.ID, + Deadline: workspaceDeadline, ProvisionerState: jobType.WorkspaceBuild.State, UpdatedAt: now, }) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 543fae6185dc7..1b841414cf531 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -574,8 +574,7 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) { func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) - if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceWorkspace. - InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) { + if !api.Authorize(rw, r, rbac.ActionUpdate, workspace) { return } @@ -598,18 +597,19 @@ func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) { return xerrors.Errorf("workspace must be started, current status: %s", build.Transition) } - newDeadline := req.Deadline.UTC() + newDeadline := req.Deadline.Truncate(time.Minute).UTC() if newDeadline.IsZero() { // This should not be possible because the struct validation field enforces a non-zero value. code = http.StatusBadRequest return xerrors.New("new deadline cannot be zero") } - if newDeadline.Before(build.Deadline) { + if newDeadline.Before(build.Deadline) || newDeadline.Before(time.Now()) { code = http.StatusBadRequest return xerrors.Errorf("new deadline %q must be after existing deadline %q", newDeadline.Format(time.RFC3339), build.Deadline.Format(time.RFC3339)) } + // both newDeadline and build.Deadline are truncated to time.Minute if newDeadline == build.Deadline { code = http.StatusNotModified return nil diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 899abb5834e38..aa57b528e77b2 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -633,7 +633,7 @@ func TestWorkspaceExtend(t *testing.T) { workspace, err := client.Workspace(ctx, workspace.ID) require.NoError(t, err, "fetch provisioned workspace") - require.InDelta(t, oldDeadline.Unix(), workspace.LatestBuild.Deadline.Unix(), 1) + require.InDelta(t, oldDeadline.Unix(), workspace.LatestBuild.Deadline.Unix(), 60) // Updating the deadline should succeed req := codersdk.PutExtendWorkspaceRequest{ @@ -645,7 +645,7 @@ func TestWorkspaceExtend(t *testing.T) { // Ensure deadline set correctly updated, err := client.Workspace(ctx, workspace.ID) require.NoError(t, err, "failed to fetch updated workspace") - require.InDelta(t, newDeadline.Unix(), updated.LatestBuild.Deadline.Unix(), 1) + require.InDelta(t, newDeadline.Unix(), updated.LatestBuild.Deadline.Unix(), 60) // Zero time should fail err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{ @@ -662,7 +662,7 @@ func TestWorkspaceExtend(t *testing.T) { // Ensure deadline still set correctly updated, err = client.Workspace(ctx, workspace.ID) require.NoError(t, err, "failed to fetch updated workspace") - require.InDelta(t, newDeadline.Unix(), updated.LatestBuild.Deadline.Unix(), 1) + require.InDelta(t, newDeadline.Unix(), updated.LatestBuild.Deadline.Unix(), 60) } func TestWorkspaceWatcher(t *testing.T) { From 03ec987a3ab50c76919112d90d58328f52d4883b Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 26 May 2022 15:42:17 +0000 Subject: [PATCH 10/11] update comment with more info --- coderd/autobuild/executor/lifecycle_executor.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor.go b/coderd/autobuild/executor/lifecycle_executor.go index a26219619c4e6..b031d322116ba 100644 --- a/coderd/autobuild/executor/lifecycle_executor.go +++ b/coderd/autobuild/executor/lifecycle_executor.go @@ -50,9 +50,18 @@ func (e *Executor) Run() { func (e *Executor) runOnce(t time.Time) error { currentTick := t.Truncate(time.Minute) return e.db.InTx(func(db database.Store) error { - // XXX: if a workspace build is created when ttl != null and then ttl is unset, - // autostop will still happen for this workspace build. Whether this is - // expected behavior from a user's perspective is not yet known. + // TTL is set at the workspace level, and deadline at the workspace build level. + // When a workspace build is created, its deadline initially starts at zero. + // When provisionerd successfully completes a provision job, the deadline is + // set to now + TTL if the associated workspace has a TTL set. This deadline + // is what we compare against when performing autostop operations, rounded down + // to the minute. + // + // NOTE: Currently, 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. So, autostop will still happen at the + // original TTL value from when the workspace build was created. + // Whether this is expected behavior from a user's perspective is not yet known. eligibleWorkspaces, err := db.GetWorkspacesAutostart(e.ctx) if err != nil { return xerrors.Errorf("get eligible workspaces for autostart or autostop: %w", err) From 6e63fc3da4fe320ad1842cad06f2bc75732f495a Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 26 May 2022 16:55:43 +0000 Subject: [PATCH 11/11] rename migration --- ..._deadline.down.sql => 000015_workspacebuild_deadline.down.sql} | 0 ...uild_deadline.up.sql => 000015_workspacebuild_deadline.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000014_workspacebuild_deadline.down.sql => 000015_workspacebuild_deadline.down.sql} (100%) rename coderd/database/migrations/{000014_workspacebuild_deadline.up.sql => 000015_workspacebuild_deadline.up.sql} (100%) diff --git a/coderd/database/migrations/000014_workspacebuild_deadline.down.sql b/coderd/database/migrations/000015_workspacebuild_deadline.down.sql similarity index 100% rename from coderd/database/migrations/000014_workspacebuild_deadline.down.sql rename to coderd/database/migrations/000015_workspacebuild_deadline.down.sql diff --git a/coderd/database/migrations/000014_workspacebuild_deadline.up.sql b/coderd/database/migrations/000015_workspacebuild_deadline.up.sql similarity index 100% rename from coderd/database/migrations/000014_workspacebuild_deadline.up.sql rename to coderd/database/migrations/000015_workspacebuild_deadline.up.sql