diff --git a/cli/schedule_test.go b/cli/schedule_test.go index dfb992976bc62..e07b7d65b0637 100644 --- a/cli/schedule_test.go +++ b/cli/schedule_test.go @@ -243,9 +243,8 @@ func TestScheduleOverride(t *testing.T) { require.NoError(t, err) expectedDeadline := time.Now().Add(10 * time.Hour) - // Assert test invariant: workspace build has a deadline set equal to now plus ttl - initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond) - require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline.Time, time.Minute) + // Assert test invariant: workspace build has a deadline set + require.False(t, workspace.LatestBuild.Deadline.IsZero()) inv, root := clitest.New(t, cmdArgs...) clitest.SetupConfig(t, client, root) @@ -283,10 +282,6 @@ func TestScheduleOverride(t *testing.T) { workspace, err = client.Workspace(ctx, workspace.ID) require.NoError(t, err) - // Assert test invariant: workspace build has a deadline set equal to now plus ttl - initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond) - require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline.Time, time.Minute) - inv, root := clitest.New(t, cmdArgs...) clitest.SetupConfig(t, client, root) inv.Stdout = stdoutBuf diff --git a/coderd/activitybump_internal_test.go b/coderd/activitybump_internal_test.go index c561c7664f0ce..be4609d5b0744 100644 --- a/coderd/activitybump_internal_test.go +++ b/coderd/activitybump_internal_test.go @@ -32,13 +32,15 @@ func Test_ActivityBumpWorkspace(t *testing.T) { } for _, tt := range []struct { - name string - transition database.WorkspaceTransition - jobCompletedAt sql.NullTime - buildDeadlineOffset *time.Duration - maxDeadlineOffset *time.Duration - workspaceTTL time.Duration - expectedBump time.Duration + name string + transition database.WorkspaceTransition + jobCompletedAt sql.NullTime + buildDeadlineOffset *time.Duration + maxDeadlineOffset *time.Duration + workspaceTTL time.Duration + templateDisallowUserAutostop bool // inverted + templateTTL time.Duration + expectedBump time.Duration }{ { name: "NotFinishedYet", @@ -69,6 +71,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) { jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-24 * time.Minute)}, buildDeadlineOffset: ptr.Ref(8*time.Hour - 24*time.Minute), workspaceTTL: 8 * time.Hour, + templateTTL: 6 * time.Hour, // unused expectedBump: 8 * time.Hour, }, { @@ -99,6 +102,18 @@ func Test_ActivityBumpWorkspace(t *testing.T) { workspaceTTL: 8 * time.Hour, expectedBump: 0, }, + { + // If the template disallows user autostop, then the TTL of the + // template should be used. + name: "TemplateDisallowsUserAutostop", + transition: database.WorkspaceTransitionStart, + jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-24 * time.Minute)}, + buildDeadlineOffset: ptr.Ref(8*time.Hour - 24*time.Minute), + workspaceTTL: 6 * time.Hour, + templateDisallowUserAutostop: true, + templateTTL: 8 * time.Hour, + expectedBump: 8 * time.Hour, + }, } { tt := tt for _, tz := range timezones { @@ -144,6 +159,15 @@ func Test_ActivityBumpWorkspace(t *testing.T) { buildID = uuid.New() ) + err := db.UpdateTemplateScheduleByID(ctx, database.UpdateTemplateScheduleByIDParams{ + ID: template.ID, + UpdatedAt: dbtime.Now(), + AllowUserAutostop: !tt.templateDisallowUserAutostop, + DefaultTTL: int64(tt.templateTTL), + // The other fields don't matter. + }) + require.NoError(t, err) + var buildNumber int32 = 1 // Insert a number of previous workspace builds. for i := 0; i < 5; i++ { @@ -162,7 +186,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) { if tt.maxDeadlineOffset != nil { maxDeadline = now.Add(*tt.maxDeadlineOffset) } - err := db.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{ + err = db.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{ ID: buildID, CreatedAt: dbtime.Now(), UpdatedAt: dbtime.Now(), diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 294db0034447b..ef84d6c70b4f9 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -19,7 +19,6 @@ import ( "github.com/coder/coder/v2/coderd/database/provisionerjobs" "github.com/coder/coder/v2/coderd/database/pubsub" "github.com/coder/coder/v2/coderd/schedule" - "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/coder/v2/codersdk" ) @@ -116,7 +115,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) if err != nil { e.log.Error(e.ctx, "get workspaces for autostart or autostop", slog.Error(err)) return stats @@ -183,7 +182,26 @@ func (e *Executor) runOnce(t time.Time) Stats { } } - // Transition the workspace to dormant if it has breached the template's + if reason == database.BuildReasonBump { + newDeadline := shouldBump(ws, latestBuild, latestJob, templateSchedule, currentTick) + if !newDeadline.IsZero() { + err := tx.UpdateWorkspaceBuildDeadlineByID(e.ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ + ID: latestBuild.ID, + UpdatedAt: dbtime.Now(), + Deadline: newDeadline, + MaxDeadline: latestBuild.MaxDeadline, + }) + if err != nil { + log.Error(e.ctx, "unable to bump workspace deadline", + slog.F("new_deadline", newDeadline), + slog.Error(err), + ) + return nil + } + } + } + + // Transition the workspace to dormant if it has breached the template's // threshold for inactivity. if reason == database.BuildReasonAutolock { ws, err = tx.UpdateWorkspaceDormantDeletingAt(e.ctx, database.UpdateWorkspaceDormantDeletingAtParams{ @@ -295,6 +313,8 @@ func getNextTransition( case isEligibleForDelete(ws, templateSchedule, currentTick): return database.WorkspaceTransitionDelete, database.BuildReasonAutodelete, nil + case !shouldBump(ws, latestBuild, latestJob, templateSchedule, currentTick).IsZero(): + return "", database.BuildReasonBump, nil default: return "", "", xerrors.Errorf("last transition not valid for autostart or autostop") } @@ -320,14 +340,11 @@ func isEligibleForAutostart(ws database.Workspace, build database.WorkspaceBuild // If autostart isn't enabled, or the schedule isn't valid/populated we can't // autostart the workspace. - if !templateSchedule.UserAutostartEnabled || !ws.AutostartSchedule.Valid || ws.AutostartSchedule.String == "" { + sched, err := schedule.GetWorkspaceAutostartSchedule(templateSchedule, ws) + if err != nil || sched == nil { return false } - sched, err := cron.Weekly(ws.AutostartSchedule.String) - if err != nil { - return false - } // Round down to the nearest minute, as this is the finest granularity cron supports. // Truncate is probably not necessary here, but doing it anyway to be sure. nextTransition := sched.Next(build.CreatedAt).Truncate(time.Minute) @@ -385,3 +402,49 @@ func isEligibleForFailedStop(build database.WorkspaceBuild, job database.Provisi job.CompletedAt.Valid && currentTick.Sub(job.CompletedAt.Time) > templateSchedule.FailureTTL } + +// shouldBump returns a non-zero time if the workspace deadline should +// should be bumped. +func shouldBump(ws database.Workspace, build database.WorkspaceBuild, job database.ProvisionerJob, templateSchedule schedule.TemplateScheduleOptions, currentTick time.Time) time.Time { + if build.Transition != database.WorkspaceTransitionStart { + return time.Time{} + } + if db2sdk.ProvisionerJobStatus(job) != codersdk.ProvisionerJobSucceeded { + return time.Time{} + } + if build.Deadline.IsZero() { + return time.Time{} + } + if currentTick.After(build.Deadline.Add(time.Hour)) { + // If the workspace has already breached its deadline + 1h, we should + // not bump it, because we don't want to race autostop code. + return time.Time{} + } + + ttl := schedule.WorkspaceTTL(templateSchedule, ws) + if ttl <= 0 { + return time.Time{} + } + + // If autostart isn't enabled, or the schedule isn't valid/populated we + // don't care about bumping it. + sched, err := schedule.GetWorkspaceAutostartSchedule(templateSchedule, ws) + if err != nil || sched == nil { + return time.Time{} + } + + newDeadline := schedule.MaybeBumpDeadline(sched, build.Deadline, ttl) + if newDeadline.IsZero() { + return time.Time{} + } + if newDeadline.Before(build.Deadline) { + // Don't reduce the deadline. + return time.Time{} + } + if !build.MaxDeadline.IsZero() && newDeadline.After(build.MaxDeadline) { + // Don't exceed the max deadline. + return time.Time{} + } + + return newDeadline +} diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index a613e03b7bcf3..515e685f45695 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -16,12 +16,15 @@ 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/dbtestutil" + "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/provisionersdk/proto" + "github.com/coder/coder/v2/testutil" ) func TestExecutorAutostartOK(t *testing.T) { @@ -701,6 +704,131 @@ func TestExecutorFailedWorkspace(t *testing.T) { }) } +func TestExecutorBumpWorkspace(t *testing.T) { + t.Parallel() + + midnight := time.Date(2023, 10, 4, 0, 0, 0, 0, time.UTC) + + cases := []struct { + name string + autostartSchedule string + now time.Time // defaults to 1h before the deadline + deadline time.Time + maxDeadline time.Time + ttl time.Duration + expected time.Time + }{ + { + name: "not eligible", + autostartSchedule: "CRON_TZ=UTC 0 0 * * *", + deadline: midnight.Add(time.Hour * -2), + ttl: time.Hour * 10, + // not eligible because the deadline is over an hour before the next + // autostart time + expected: time.Time{}, + }, + { + name: "autostart before deadline by 1h", + autostartSchedule: "CRON_TZ=UTC 0 0 * * *", + deadline: midnight.Add(time.Hour), + ttl: time.Hour * 10, + expected: midnight.Add(time.Hour * 10), + }, + { + name: "autostart before deadline by 9h", + autostartSchedule: "CRON_TZ=UTC 0 0 * * *", + deadline: midnight.Add(time.Hour * 9), + ttl: time.Hour * 10, + // should still be bumped + expected: midnight.Add(time.Hour * 10), + }, + { + name: "eligible but exceeds next next autostart", + autostartSchedule: "CRON_TZ=UTC 0 0 * * *", + deadline: midnight.Add(time.Hour * 1), + // ttl causes next autostart + 25h to exceed the next next autostart + ttl: time.Hour * 25, + // should not be bumped to avoid infinite bumping every day + expected: time.Time{}, + }, + { + name: "deadline is 1h before autostart", + autostartSchedule: "CRON_TZ=UTC 0 0 * * *", + deadline: midnight.Add(time.Hour * -1).Add(time.Minute), + ttl: time.Hour * 10, + // should still be bumped + expected: midnight.Add(time.Hour * 10), + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + var ( + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + db, pubsub = dbtestutil.NewDB(t) + + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + AutobuildStats: statsCh, + Database: db, + Pubsub: pubsub, + IncludeProvisionerDaemon: true, + TemplateScheduleStore: schedule.MockTemplateScheduleStore{ + GetFn: func(_ context.Context, _ database.Store, _ uuid.UUID) (schedule.TemplateScheduleOptions, error) { + return schedule.TemplateScheduleOptions{ + UserAutostartEnabled: true, + UserAutostopEnabled: true, + DefaultTTL: 0, + AutostopRequirement: schedule.TemplateAutostopRequirement{}, + }, nil + }, + }, + }) + workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.AutostartSchedule = ptr.Ref(c.autostartSchedule) + cwr.TTLMillis = ptr.Ref(c.ttl.Milliseconds()) + }) + ) + + // Set the deadline and max deadline to what the test expects. + ctx := testutil.Context(t, testutil.WaitLong) + err := db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ + ID: workspace.LatestBuild.ID, + UpdatedAt: dbtime.Now(), + Deadline: c.deadline, + MaxDeadline: c.maxDeadline, + }) + require.NoError(t, err) + + if c.now.IsZero() { + c.now = c.deadline.Add(-time.Hour) + } + + go func() { + tickCh <- c.now + close(tickCh) + }() + stats := <-statsCh + require.NoError(t, stats.Error) + require.Len(t, stats.Transitions, 0) + + // Get the latest workspace build. + build, err := client.WorkspaceBuild(ctx, workspace.LatestBuild.ID) + require.NoError(t, err) + + // Should be bumped to the expected time. + if c.expected.IsZero() { + c.expected = c.deadline + } + require.WithinDuration(t, c.expected, build.Deadline.Time, time.Minute) + }) + } +} + // TestExecutorInactiveWorkspace test AGPL functionality which mainly // ensures that autostop actions as a result of an inactive workspace // do not trigger. diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index c0a5cfcecf3cf..79b1b5ea7641b 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1915,8 +1915,8 @@ func (q *querier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesP return q.db.GetAuthorizedWorkspaces(ctx, arg, prep) } -func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.Workspace, error) { - return q.db.GetWorkspacesEligibleForTransition(ctx, now) +func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context) ([]database.Workspace, error) { + return q.db.GetWorkspacesEligibleForTransition(ctx) } func (q *querier) InsertAPIKey(ctx context.Context, arg database.InsertAPIKeyParams) (database.APIKey, error) { diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 1e6e4d9d410e7..2f98936691220 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -793,8 +793,25 @@ func (q *FakeQuerier) ActivityBumpWorkspace(ctx context.Context, workspaceID uui if q.workspaceBuilds[i].Deadline.IsZero() { return nil } + + template, err := q.getTemplateByIDNoLock(ctx, workspace.TemplateID) + if err != nil { + return err + } + + var ttlDur time.Duration + if workspace.Ttl.Valid { + ttlDur = time.Duration(workspace.Ttl.Int64) + } + if !template.AllowUserAutostop { + ttlDur = time.Duration(template.DefaultTTL) + } + if ttlDur <= 0 { + // There's no TTL set anymore, so we don't know the bump duration. + return nil + } + // Only bump if 5% of the deadline has passed. - ttlDur := time.Duration(workspace.Ttl.Int64) ttlDur95 := ttlDur - (ttlDur / 20) minBumpDeadline := q.workspaceBuilds[i].Deadline.Add(-ttlDur95) if now.Before(minBumpDeadline) { @@ -4047,7 +4064,7 @@ func (q *FakeQuerier) GetWorkspaces(ctx context.Context, arg database.GetWorkspa return workspaceRows, err } -func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.Workspace, error) { +func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context) ([]database.Workspace, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -4058,26 +4075,26 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no return nil, err } - if build.Transition == database.WorkspaceTransitionStart && - !build.Deadline.IsZero() && - build.Deadline.Before(now) && - !workspace.DormantAt.Valid { + // Refer to the SQL version of this query for comments. + + // Case 1: + if build.Transition == database.WorkspaceTransitionStart && !build.Deadline.IsZero() { workspaces = append(workspaces, workspace) continue } - if build.Transition == database.WorkspaceTransitionStop && - workspace.AutostartSchedule.Valid && - !workspace.DormantAt.Valid { + // Case 2: + if build.Transition == database.WorkspaceTransitionStop && workspace.AutostartSchedule.Valid { workspaces = append(workspaces, workspace) continue } + // Case 3: job, err := q.getProvisionerJobByIDNoLock(ctx, build.JobID) if err != nil { return nil, xerrors.Errorf("get provisioner job by ID: %w", err) } - if db2sdk.ProvisionerJobStatus(job) == codersdk.ProvisionerJobFailed { + if db2sdk.ProvisionerJobStatus(job) == codersdk.ProvisionerJobFailed && build.Transition == database.WorkspaceTransitionStart { workspaces = append(workspaces, workspace) continue } @@ -4086,11 +4103,15 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no if err != nil { return nil, xerrors.Errorf("get template by ID: %w", err) } - if !workspace.DormantAt.Valid && template.TimeTilDormant > 0 { + + // Case 4: + if template.TimeTilDormant > 0 && !workspace.DormantAt.Valid { workspaces = append(workspaces, workspace) continue } - if workspace.DormantAt.Valid && template.TimeTilDormantAutoDelete > 0 { + + // Case 5: + if template.TimeTilDormantAutoDelete > 0 && workspace.DormantAt.Valid { workspaces = append(workspaces, workspace) continue } diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 3811cb9561801..522208a618270 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -1117,9 +1117,9 @@ func (m metricsStore) GetWorkspaces(ctx context.Context, arg database.GetWorkspa return workspaces, err } -func (m metricsStore) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.Workspace, error) { +func (m metricsStore) GetWorkspacesEligibleForTransition(ctx context.Context) ([]database.Workspace, error) { start := time.Now() - workspaces, err := m.s.GetWorkspacesEligibleForTransition(ctx, now) + workspaces, err := m.s.GetWorkspacesEligibleForTransition(ctx) m.queryLatencies.WithLabelValues("GetWorkspacesEligibleForAutoStartStop").Observe(time.Since(start).Seconds()) return workspaces, err } diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index b2001b8b19640..58631eaa8975f 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2334,18 +2334,18 @@ func (mr *MockStoreMockRecorder) GetWorkspaces(arg0, arg1 interface{}) *gomock.C } // GetWorkspacesEligibleForTransition mocks base method. -func (m *MockStore) GetWorkspacesEligibleForTransition(arg0 context.Context, arg1 time.Time) ([]database.Workspace, error) { +func (m *MockStore) GetWorkspacesEligibleForTransition(arg0 context.Context) ([]database.Workspace, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetWorkspacesEligibleForTransition", arg0, arg1) + ret := m.ctrl.Call(m, "GetWorkspacesEligibleForTransition", arg0) ret0, _ := ret[0].([]database.Workspace) ret1, _ := ret[1].(error) return ret0, ret1 } // GetWorkspacesEligibleForTransition indicates an expected call of GetWorkspacesEligibleForTransition. -func (mr *MockStoreMockRecorder) GetWorkspacesEligibleForTransition(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockStoreMockRecorder) GetWorkspacesEligibleForTransition(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWorkspacesEligibleForTransition", reflect.TypeOf((*MockStore)(nil).GetWorkspacesEligibleForTransition), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWorkspacesEligibleForTransition", reflect.TypeOf((*MockStore)(nil).GetWorkspacesEligibleForTransition), arg0) } // InTx mocks base method. diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 17bb9c539e741..d8c1725b468da 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -28,7 +28,8 @@ CREATE TYPE build_reason AS ENUM ( 'autostop', 'autolock', 'failedstop', - 'autodelete' + 'autodelete', + 'bump' ); CREATE TYPE display_app AS ENUM ( diff --git a/coderd/database/migrations/000160_workspace_build_reason_bump.down.sql b/coderd/database/migrations/000160_workspace_build_reason_bump.down.sql new file mode 100644 index 0000000000000..aa79c28f2554d --- /dev/null +++ b/coderd/database/migrations/000160_workspace_build_reason_bump.down.sql @@ -0,0 +1 @@ +-- Cannot drop values from enum type. diff --git a/coderd/database/migrations/000160_workspace_build_reason_bump.up.sql b/coderd/database/migrations/000160_workspace_build_reason_bump.up.sql new file mode 100644 index 0000000000000..a4ccdf52e6b12 --- /dev/null +++ b/coderd/database/migrations/000160_workspace_build_reason_bump.up.sql @@ -0,0 +1 @@ +ALTER TYPE build_reason ADD VALUE IF NOT EXISTS 'bump'; diff --git a/coderd/database/models.go b/coderd/database/models.go index ab6d8861ee434..477f346053b69 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -220,6 +220,7 @@ const ( BuildReasonAutolock BuildReason = "autolock" BuildReasonFailedstop BuildReason = "failedstop" BuildReasonAutodelete BuildReason = "autodelete" + BuildReasonBump BuildReason = "bump" ) func (e *BuildReason) Scan(src interface{}) error { @@ -264,7 +265,8 @@ func (e BuildReason) Valid() bool { BuildReasonAutostop, BuildReasonAutolock, BuildReasonFailedstop, - BuildReasonAutodelete: + BuildReasonAutodelete, + BuildReasonBump: return true } return false @@ -278,6 +280,7 @@ func AllBuildReasonValues() []BuildReason { BuildReasonAutolock, BuildReasonFailedstop, BuildReasonAutodelete, + BuildReasonBump, } } diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 04b7a4a4eedad..93bf012107268 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -28,6 +28,7 @@ type sqlcQuerier interface { // as the TTL wraps. For example, if I set the TTL to 12 hours, sign off // work at midnight, come back at 10am, I would want another full day // of uptime. + // We only bump if the raw interval is positive and non-zero. // We only bump if workspace shutdown is manual. // We only bump when 5% of the deadline has elapsed. ActivityBumpWorkspace(ctx context.Context, workspaceID uuid.UUID) error @@ -224,7 +225,7 @@ type sqlcQuerier interface { GetWorkspaceResourcesByJobIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceResource, error) GetWorkspaceResourcesCreatedAfter(ctx context.Context, createdAt time.Time) ([]WorkspaceResource, error) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) ([]GetWorkspacesRow, error) - GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]Workspace, error) + GetWorkspacesEligibleForTransition(ctx context.Context) ([]Workspace, error) InsertAPIKey(ctx context.Context, arg InsertAPIKeyParams) (APIKey, error) // We use the organization_id as the id // for simplicity since all users is diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 40b7ee7e72715..4833be4c8ec0c 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -23,12 +23,21 @@ WITH latest AS ( workspace_builds.max_deadline::timestamp with time zone AS build_max_deadline, workspace_builds.transition AS build_transition, provisioner_jobs.completed_at::timestamp with time zone AS job_completed_at, - (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval AS ttl_interval + ( + CASE + WHEN templates.allow_user_autostop + THEN workspaces.ttl + ELSE templates.default_ttl + END + ) AS raw_ttl_interval, + (raw_ttl_interval / 1000 / 1000 / 1000 || ' seconds')::interval AS ttl_interval FROM workspace_builds JOIN provisioner_jobs ON provisioner_jobs.id = workspace_builds.job_id JOIN workspaces ON workspaces.id = workspace_builds.workspace_id + JOIN templates + ON templates.id = workspaces.template_id WHERE workspace_builds.workspace_id = $1::uuid ORDER BY workspace_builds.build_number DESC LIMIT 1 @@ -46,6 +55,7 @@ FROM latest l WHERE wb.id = l.build_id AND l.job_completed_at IS NOT NULL AND l.build_transition = 'start' +AND l.raw_ttl_interval > 0 AND l.build_deadline != '0001-01-01 00:00:00+00' AND l.build_deadline - (l.ttl_interval * 0.95) < NOW() ` @@ -54,6 +64,7 @@ AND l.build_deadline - (l.ttl_interval * 0.95) < NOW() // as the TTL wraps. For example, if I set the TTL to 12 hours, sign off // work at midnight, come back at 10am, I would want another full day // of uptime. +// We only bump if the raw interval is positive and non-zero. // We only bump if workspace shutdown is manual. // We only bump when 5% of the deadline has elapsed. func (q *sqlQuerier) ActivityBumpWorkspace(ctx context.Context, workspaceID uuid.UUID) error { @@ -9554,6 +9565,119 @@ func (q *sqlQuerier) InsertWorkspaceResourceMetadata(ctx context.Context, arg In return items, nil } +const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many +SELECT workspace_agent_id, log_source_id, log_path, created_at, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds FROM workspace_agent_scripts WHERE workspace_agent_id = ANY($1 :: uuid [ ]) +` + +func (q *sqlQuerier) GetWorkspaceAgentScriptsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgentScript, error) { + rows, err := q.db.QueryContext(ctx, getWorkspaceAgentScriptsByAgentIDs, pq.Array(ids)) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceAgentScript + for rows.Next() { + var i WorkspaceAgentScript + if err := rows.Scan( + &i.WorkspaceAgentID, + &i.LogSourceID, + &i.LogPath, + &i.CreatedAt, + &i.Script, + &i.Cron, + &i.StartBlocksLogin, + &i.RunOnStart, + &i.RunOnStop, + &i.TimeoutSeconds, + ); 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 insertWorkspaceAgentScripts = `-- name: InsertWorkspaceAgentScripts :many +INSERT INTO + workspace_agent_scripts (workspace_agent_id, created_at, log_source_id, log_path, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds) +SELECT + $1 :: uuid AS workspace_agent_id, + $2 :: timestamptz AS created_at, + unnest($3 :: uuid [ ]) AS log_source_id, + unnest($4 :: text [ ]) AS log_path, + unnest($5 :: text [ ]) AS script, + unnest($6 :: text [ ]) AS cron, + unnest($7 :: boolean [ ]) AS start_blocks_login, + unnest($8 :: boolean [ ]) AS run_on_start, + unnest($9 :: boolean [ ]) AS run_on_stop, + unnest($10 :: integer [ ]) AS timeout_seconds +RETURNING workspace_agent_scripts.workspace_agent_id, workspace_agent_scripts.log_source_id, workspace_agent_scripts.log_path, workspace_agent_scripts.created_at, workspace_agent_scripts.script, workspace_agent_scripts.cron, workspace_agent_scripts.start_blocks_login, workspace_agent_scripts.run_on_start, workspace_agent_scripts.run_on_stop, workspace_agent_scripts.timeout_seconds +` + +type InsertWorkspaceAgentScriptsParams struct { + WorkspaceAgentID uuid.UUID `db:"workspace_agent_id" json:"workspace_agent_id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + LogSourceID []uuid.UUID `db:"log_source_id" json:"log_source_id"` + LogPath []string `db:"log_path" json:"log_path"` + Script []string `db:"script" json:"script"` + Cron []string `db:"cron" json:"cron"` + StartBlocksLogin []bool `db:"start_blocks_login" json:"start_blocks_login"` + RunOnStart []bool `db:"run_on_start" json:"run_on_start"` + RunOnStop []bool `db:"run_on_stop" json:"run_on_stop"` + TimeoutSeconds []int32 `db:"timeout_seconds" json:"timeout_seconds"` +} + +func (q *sqlQuerier) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) { + rows, err := q.db.QueryContext(ctx, insertWorkspaceAgentScripts, + arg.WorkspaceAgentID, + arg.CreatedAt, + pq.Array(arg.LogSourceID), + pq.Array(arg.LogPath), + pq.Array(arg.Script), + pq.Array(arg.Cron), + pq.Array(arg.StartBlocksLogin), + pq.Array(arg.RunOnStart), + pq.Array(arg.RunOnStop), + pq.Array(arg.TimeoutSeconds), + ) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceAgentScript + for rows.Next() { + var i WorkspaceAgentScript + if err := rows.Scan( + &i.WorkspaceAgentID, + &i.LogSourceID, + &i.LogPath, + &i.CreatedAt, + &i.Script, + &i.Cron, + &i.StartBlocksLogin, + &i.RunOnStart, + &i.RunOnStop, + &i.TimeoutSeconds, + ); 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 getDeploymentWorkspaceStats = `-- name: GetDeploymentWorkspaceStats :one WITH workspaces_with_jobs AS ( SELECT @@ -10157,14 +10281,16 @@ WHERE ( -- If the workspace build was a start transition, the workspace is - -- potentially eligible for autostop if it's past the deadline. The - -- deadline is computed at build time upon success and is bumped based - -- on activity (up the max deadline if set). We don't need to check - -- license here since that's done when the values are written to the build. + -- potentially eligible for: + -- - an autostop if it's past the deadline + -- - a deadline bump if it's currently running and has a deadline + -- The deadline is computed at build time upon success and is bumped + -- based on activity (up the max deadline if set). We don't need to + -- check license here since that's done when the values are written to + -- the build. ( workspace_builds.transition = 'start'::workspace_transition AND - workspace_builds.deadline IS NOT NULL AND - workspace_builds.deadline < $1 :: timestamptz + workspace_builds.deadline IS NOT NULL ) OR -- If the workspace build was a stop transition, the workspace is @@ -10200,8 +10326,8 @@ WHERE ) AND workspaces.deleted = 'false' ` -func (q *sqlQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]Workspace, error) { - rows, err := q.db.QueryContext(ctx, getWorkspacesEligibleForTransition, now) +func (q *sqlQuerier) GetWorkspacesEligibleForTransition(ctx context.Context) ([]Workspace, error) { + rows, err := q.db.QueryContext(ctx, getWorkspacesEligibleForTransition) if err != nil { return nil, err } @@ -10502,116 +10628,3 @@ func (q *sqlQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.C _, err := q.db.ExecContext(ctx, updateWorkspacesDormantDeletingAtByTemplateID, arg.TimeTilDormantAutodeleteMs, arg.DormantAt, arg.TemplateID) return err } - -const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many -SELECT workspace_agent_id, log_source_id, log_path, created_at, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds FROM workspace_agent_scripts WHERE workspace_agent_id = ANY($1 :: uuid [ ]) -` - -func (q *sqlQuerier) GetWorkspaceAgentScriptsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgentScript, error) { - rows, err := q.db.QueryContext(ctx, getWorkspaceAgentScriptsByAgentIDs, pq.Array(ids)) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAgentScript - for rows.Next() { - var i WorkspaceAgentScript - if err := rows.Scan( - &i.WorkspaceAgentID, - &i.LogSourceID, - &i.LogPath, - &i.CreatedAt, - &i.Script, - &i.Cron, - &i.StartBlocksLogin, - &i.RunOnStart, - &i.RunOnStop, - &i.TimeoutSeconds, - ); 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 insertWorkspaceAgentScripts = `-- name: InsertWorkspaceAgentScripts :many -INSERT INTO - workspace_agent_scripts (workspace_agent_id, created_at, log_source_id, log_path, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds) -SELECT - $1 :: uuid AS workspace_agent_id, - $2 :: timestamptz AS created_at, - unnest($3 :: uuid [ ]) AS log_source_id, - unnest($4 :: text [ ]) AS log_path, - unnest($5 :: text [ ]) AS script, - unnest($6 :: text [ ]) AS cron, - unnest($7 :: boolean [ ]) AS start_blocks_login, - unnest($8 :: boolean [ ]) AS run_on_start, - unnest($9 :: boolean [ ]) AS run_on_stop, - unnest($10 :: integer [ ]) AS timeout_seconds -RETURNING workspace_agent_scripts.workspace_agent_id, workspace_agent_scripts.log_source_id, workspace_agent_scripts.log_path, workspace_agent_scripts.created_at, workspace_agent_scripts.script, workspace_agent_scripts.cron, workspace_agent_scripts.start_blocks_login, workspace_agent_scripts.run_on_start, workspace_agent_scripts.run_on_stop, workspace_agent_scripts.timeout_seconds -` - -type InsertWorkspaceAgentScriptsParams struct { - WorkspaceAgentID uuid.UUID `db:"workspace_agent_id" json:"workspace_agent_id"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - LogSourceID []uuid.UUID `db:"log_source_id" json:"log_source_id"` - LogPath []string `db:"log_path" json:"log_path"` - Script []string `db:"script" json:"script"` - Cron []string `db:"cron" json:"cron"` - StartBlocksLogin []bool `db:"start_blocks_login" json:"start_blocks_login"` - RunOnStart []bool `db:"run_on_start" json:"run_on_start"` - RunOnStop []bool `db:"run_on_stop" json:"run_on_stop"` - TimeoutSeconds []int32 `db:"timeout_seconds" json:"timeout_seconds"` -} - -func (q *sqlQuerier) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) { - rows, err := q.db.QueryContext(ctx, insertWorkspaceAgentScripts, - arg.WorkspaceAgentID, - arg.CreatedAt, - pq.Array(arg.LogSourceID), - pq.Array(arg.LogPath), - pq.Array(arg.Script), - pq.Array(arg.Cron), - pq.Array(arg.StartBlocksLogin), - pq.Array(arg.RunOnStart), - pq.Array(arg.RunOnStop), - pq.Array(arg.TimeoutSeconds), - ) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAgentScript - for rows.Next() { - var i WorkspaceAgentScript - if err := rows.Scan( - &i.WorkspaceAgentID, - &i.LogSourceID, - &i.LogPath, - &i.CreatedAt, - &i.Script, - &i.Cron, - &i.StartBlocksLogin, - &i.RunOnStart, - &i.RunOnStop, - &i.TimeoutSeconds, - ); 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 -} diff --git a/coderd/database/queries/activitybump.sql b/coderd/database/queries/activitybump.sql index 9b8e358e19000..feb9fb7cb217e 100644 --- a/coderd/database/queries/activitybump.sql +++ b/coderd/database/queries/activitybump.sql @@ -10,12 +10,21 @@ WITH latest AS ( workspace_builds.max_deadline::timestamp with time zone AS build_max_deadline, workspace_builds.transition AS build_transition, provisioner_jobs.completed_at::timestamp with time zone AS job_completed_at, - (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval AS ttl_interval + ( + CASE + WHEN templates.allow_user_autostop + THEN workspaces.ttl + ELSE templates.default_ttl + END + ) AS raw_ttl_interval, + (raw_ttl_interval / 1000 / 1000 / 1000 || ' seconds')::interval AS ttl_interval FROM workspace_builds JOIN provisioner_jobs ON provisioner_jobs.id = workspace_builds.job_id JOIN workspaces ON workspaces.id = workspace_builds.workspace_id + JOIN templates + ON templates.id = workspaces.template_id WHERE workspace_builds.workspace_id = @workspace_id::uuid ORDER BY workspace_builds.build_number DESC LIMIT 1 @@ -33,6 +42,8 @@ FROM latest l WHERE wb.id = l.build_id AND l.job_completed_at IS NOT NULL AND l.build_transition = 'start' +-- We only bump if the raw interval is positive and non-zero. +AND l.raw_ttl_interval > 0 -- We only bump if workspace shutdown is manual. AND l.build_deadline != '0001-01-01 00:00:00+00' -- We only bump when 5% of the deadline has elapsed. diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 0aa073301eb8f..5ffbd664f41f0 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -451,14 +451,16 @@ WHERE ( -- If the workspace build was a start transition, the workspace is - -- potentially eligible for autostop if it's past the deadline. The - -- deadline is computed at build time upon success and is bumped based - -- on activity (up the max deadline if set). We don't need to check - -- license here since that's done when the values are written to the build. + -- potentially eligible for: + -- - an autostop if it's past the deadline + -- - a deadline bump if it's currently running and has a deadline + -- The deadline is computed at build time upon success and is bumped + -- based on activity (up the max deadline if set). We don't need to + -- check license here since that's done when the values are written to + -- the build. ( workspace_builds.transition = 'start'::workspace_transition AND - workspace_builds.deadline IS NOT NULL AND - workspace_builds.deadline < @now :: timestamptz + workspace_builds.deadline IS NOT NULL ) OR -- If the workspace build was a stop transition, the workspace is diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index dd8bed7fef1b5..330c0dbc22aa5 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -34,6 +34,7 @@ import ( "github.com/coder/coder/v2/coderd/externalauth" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/autostop" "github.com/coder/coder/v2/coderd/telemetry" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/codersdk" @@ -1106,7 +1107,7 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob) return getWorkspaceError } - autoStop, err := schedule.CalculateAutostop(ctx, schedule.CalculateAutostopParams{ + autoStop, err := autostop.CalculateAutostop(ctx, autostop.CalculateAutostopParams{ Database: db, TemplateScheduleStore: *s.TemplateScheduleStore.Load(), UserQuietHoursScheduleStore: *s.UserQuietHoursScheduleStore.Load(), diff --git a/coderd/provisionerdserver/provisionerdserver_test.go b/coderd/provisionerdserver/provisionerdserver_test.go index 4b9ccd4e9abcd..a5fa78406a8c9 100644 --- a/coderd/provisionerdserver/provisionerdserver_test.go +++ b/coderd/provisionerdserver/provisionerdserver_test.go @@ -1234,8 +1234,8 @@ func TestCompleteJob(t *testing.T) { now := time.Now() // NOTE: if you're looking for more in-depth deadline/max_deadline - // calculation testing, see the schedule package. The provsiionerdserver - // package calls `schedule.CalculateAutostop()` to generate the deadline + // calculation testing, see the schedule package. The provisionerdserver + // package calls `autostop.CalculateAutostop()` to generate the deadline // and max_deadline. // Wednesday the 8th of February 2023 at midnight. This date was diff --git a/coderd/schedule/autostop.go b/coderd/schedule/autostop/autostop.go similarity index 92% rename from coderd/schedule/autostop.go rename to coderd/schedule/autostop/autostop.go index 8cf645b52f5de..2ffb1b73e86e8 100644 --- a/coderd/schedule/autostop.go +++ b/coderd/schedule/autostop/autostop.go @@ -1,4 +1,4 @@ -package schedule +package autostop import ( "context" @@ -9,6 +9,7 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/tracing" ) @@ -42,13 +43,14 @@ const ( type CalculateAutostopParams struct { Database database.Store - TemplateScheduleStore TemplateScheduleStore - UserQuietHoursScheduleStore UserQuietHoursScheduleStore + TemplateScheduleStore schedule.TemplateScheduleStore + UserQuietHoursScheduleStore schedule.UserQuietHoursScheduleStore Now time.Time Workspace database.Workspace } +//nolint:revive type AutostopTime struct { // Deadline is the time when the workspace will be stopped. The value can be // bumped by user activity or manually by the user via the UI. @@ -90,23 +92,25 @@ func CalculateAutostop(ctx context.Context, params CalculateAutostopParams) (Aut autostop AutostopTime ) - if workspace.Ttl.Valid { - // When the workspace is made it copies the template's TTL, and the user - // can unset it to disable it (unless the template has - // UserAutoStopEnabled set to false, see below). - autostop.Deadline = now.Add(time.Duration(workspace.Ttl.Int64)) - } - templateSchedule, err := params.TemplateScheduleStore.Get(ctx, db, workspace.TemplateID) if err != nil { return autostop, xerrors.Errorf("get template schedule options: %w", err) } - if !templateSchedule.UserAutostopEnabled { - // The user is not permitted to set their own TTL, so use the template - // default. - autostop.Deadline = time.Time{} - if templateSchedule.DefaultTTL > 0 { - autostop.Deadline = now.Add(templateSchedule.DefaultTTL) + + // Apply the TTL to calculate the deadline. + ttl := schedule.WorkspaceTTL(templateSchedule, workspace) + if ttl > 0 { + autostop.Deadline = now.Add(ttl) + + // If the workspace has a deadline, and a configured autostart time, and + // the deadline is past the next autostart time, then bump the deadline + // to be the next autostart time + the TTL. + autostartSched, err := schedule.GetWorkspaceAutostartSchedule(templateSchedule, workspace) + if err == nil && autostartSched != nil { + newDeadline := schedule.MaybeBumpDeadline(autostartSched, autostop.Deadline, ttl) + if !newDeadline.IsZero() { + autostop.Deadline = newDeadline + } } } @@ -165,8 +169,8 @@ func CalculateAutostop(ctx context.Context, params CalculateAutostopParams) (Aut // Iterate from 0 to 7, check if the current startOfDay is in the // autostop requirement. If it isn't then add a day and try again. requirementDays := templateSchedule.AutostopRequirement.DaysMap() - for i := 0; i < len(DaysOfWeek)+1; i++ { - if i == len(DaysOfWeek) { + for i := 0; i < len(schedule.DaysOfWeek)+1; i++ { + if i == len(schedule.DaysOfWeek) { // We've wrapped, so somehow we couldn't find a day in the // autostop requirement in the next week. // @@ -244,7 +248,7 @@ func nextDayMidnight(t time.Time) time.Time { // // The timezone embedded in the time object is used to determine the epoch. func WeeksSinceEpoch(now time.Time) (int64, error) { - epoch := TemplateAutostopRequirementEpoch(now.Location()) + epoch := schedule.TemplateAutostopRequirementEpoch(now.Location()) if now.Before(epoch) { return 0, xerrors.New("coder server system clock is incorrect, cannot calculate template autostop requirement") } @@ -290,7 +294,7 @@ func GetMondayOfWeek(loc *time.Location, n int64) (time.Time, error) { if n < 0 { return time.Time{}, xerrors.New("weeks since epoch must be positive") } - epoch := TemplateAutostopRequirementEpoch(loc) + epoch := schedule.TemplateAutostopRequirementEpoch(loc) monday := epoch.AddDate(0, 0, int(n*7)) y, m, d := monday.Date() diff --git a/coderd/schedule/autostop_test.go b/coderd/schedule/autostop/autostop_test.go similarity index 89% rename from coderd/schedule/autostop_test.go rename to coderd/schedule/autostop/autostop_test.go index cafe2b413eaed..ddd5d034c40c4 100644 --- a/coderd/schedule/autostop_test.go +++ b/coderd/schedule/autostop/autostop_test.go @@ -1,4 +1,4 @@ -package schedule_test +package autostop_test import ( "context" @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/autostop" "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/testutil" ) @@ -70,15 +71,17 @@ func TestCalculateAutoStop(t *testing.T) { t.Log("saturdayMidnightAfterDstOut", saturdayMidnightAfterDstOut) cases := []struct { - name string - now time.Time - templateAllowAutostop bool - templateDefaultTTL time.Duration + name string + now time.Time + templateAllowAutostart bool + templateAllowAutostop bool + templateDefaultTTL time.Duration // TODO(@dean): remove max_ttl tests useMaxTTL bool templateMaxTTL time.Duration templateAutostopRequirement schedule.TemplateAutostopRequirement userQuietHoursSchedule string + workspaceAutostartSchedule string // workspaceTTL is usually copied from the template's TTL when the // workspace is made, so it takes precedence unless // templateAllowAutostop is false. @@ -401,6 +404,42 @@ func TestCalculateAutoStop(t *testing.T) { // expectedDeadline is copied from expectedMaxDeadline. expectedMaxDeadline: fridayEveningSydney.Add(time.Hour).In(time.UTC), }, + { + // Avoid changing the deadline to `next_autostart + ttl` if the + // next autostart is after the current deadline. + name: "AutostartNoWrap", + now: saturdayMidnightSydney, + // The next autostart is 2 hour after the current deadline of 10am. + workspaceAutostartSchedule: "CRON_TZ=Australia/Sydney 0 12 * * *", + templateAllowAutostart: true, + templateAllowAutostop: true, + workspaceTTL: 10 * time.Hour, + expectedDeadline: saturdayMidnightSydney.Add(10 * time.Hour), + }, + { + // The calculated deadline is past an autostart time, so the + // deadline should be changed to `next_autostart + ttl`. + name: "AutostartWrap", + now: saturdayMidnightSydney, + // The next autostart is 59 minutes after the current deadline of 10am. + workspaceAutostartSchedule: "CRON_TZ=Australia/Sydney 59 10 * * *", + templateAllowAutostart: true, + templateAllowAutostop: true, + workspaceTTL: 10 * time.Hour, + expectedDeadline: saturdayMidnightSydney.Add(20*time.Hour + 59*time.Minute), + }, + { + // The calculated deadline is past an autostart time AND the + // following autostart time, so we should not change the deadline. + name: "AutostartAvoidDoubleWrap", + now: saturdayMidnightSydney, + workspaceAutostartSchedule: "CRON_TZ=Australia/Sydney 0 10 * * *", + templateAllowAutostart: true, + templateAllowAutostop: true, + workspaceTTL: 35 * time.Hour, + // Not 45 hours! + expectedDeadline: saturdayMidnightSydney.Add(35 * time.Hour), + }, } for _, c := range cases { @@ -415,7 +454,7 @@ func TestCalculateAutoStop(t *testing.T) { templateScheduleStore := schedule.MockTemplateScheduleStore{ GetFn: func(_ context.Context, _ database.Store, _ uuid.UUID) (schedule.TemplateScheduleOptions, error) { return schedule.TemplateScheduleOptions{ - UserAutostartEnabled: false, + UserAutostartEnabled: c.templateAllowAutostart, UserAutostopEnabled: c.templateAllowAutostop, DefaultTTL: c.templateDefaultTTL, MaxTTL: c.templateMaxTTL, @@ -477,9 +516,13 @@ func TestCalculateAutoStop(t *testing.T) { OrganizationID: org.ID, OwnerID: user.ID, Ttl: workspaceTTL, + AutostartSchedule: sql.NullString{ + String: c.workspaceAutostartSchedule, + Valid: c.workspaceAutostartSchedule != "", + }, }) - autostop, err := schedule.CalculateAutostop(ctx, schedule.CalculateAutostopParams{ + autostop, err := autostop.CalculateAutostop(ctx, autostop.CalculateAutostopParams{ Database: db, TemplateScheduleStore: templateScheduleStore, UserQuietHoursScheduleStore: userQuietHoursScheduleStore, @@ -539,7 +582,7 @@ func TestFindWeek(t *testing.T) { require.NoError(t, err) now := time.Now().In(loc) - currentWeek, err := schedule.WeeksSinceEpoch(now) + currentWeek, err := autostop.WeeksSinceEpoch(now) require.NoError(t, err) diffMonday := now.Weekday() - time.Monday @@ -554,7 +597,7 @@ func TestFindWeek(t *testing.T) { // Change to midnight. currentWeekMondayExpected = time.Date(y, m, d, 0, 0, 0, 0, loc) - currentWeekMonday, err := schedule.GetMondayOfWeek(now.Location(), currentWeek) + currentWeekMonday, err := autostop.GetMondayOfWeek(now.Location(), currentWeek) require.NoError(t, err) require.Equal(t, time.Monday, currentWeekMonday.Weekday()) require.Equal(t, currentWeekMondayExpected, currentWeekMonday) @@ -574,11 +617,11 @@ func TestFindWeek(t *testing.T) { require.Equal(t, monday.Weekday(), time.Monday, msg) t.Log(msg, "monday", monday) - week, err := schedule.WeeksSinceEpoch(monday) + week, err := autostop.WeeksSinceEpoch(monday) require.NoError(t, err, msg) require.Equal(t, currentWeek+i, week, msg) - gotMonday, err := schedule.GetMondayOfWeek(monday.Location(), week) + gotMonday, err := autostop.GetMondayOfWeek(monday.Location(), week) require.NoError(t, err, msg) require.Equal(t, monday, gotMonday, msg) @@ -587,7 +630,7 @@ func TestFindWeek(t *testing.T) { require.Equal(t, sunday.Weekday(), time.Sunday, msg) t.Log(msg, "sunday", sunday) - week, err = schedule.WeeksSinceEpoch(sunday) + week, err = autostop.WeeksSinceEpoch(sunday) require.NoError(t, err, msg) require.Equal(t, currentWeek+i, week, msg) } diff --git a/coderd/schedule/template.go b/coderd/schedule/template.go index 2c916183e1f00..0ed946b1e26d3 100644 --- a/coderd/schedule/template.go +++ b/coderd/schedule/template.go @@ -187,10 +187,14 @@ func (*agplTemplateScheduleStore) Set(ctx context.Context, db database.Store, tp AutostopRequirementDaysOfWeek: tpl.AutostopRequirementDaysOfWeek, AutostopRequirementWeeks: tpl.AutostopRequirementWeeks, AllowUserAutostart: tpl.AllowUserAutostart, - AllowUserAutostop: tpl.AllowUserAutostop, - FailureTTL: tpl.FailureTTL, - TimeTilDormant: tpl.TimeTilDormant, - TimeTilDormantAutoDelete: tpl.TimeTilDormantAutoDelete, + // This value always gets set back to true because it's used in the + // ActivityBumpWorkspace query. If we didn't set this back to true + // then the template could be permanently stuck in the false state + // and activity bumps would evaluate as such. + AllowUserAutostop: true, + FailureTTL: tpl.FailureTTL, + TimeTilDormant: tpl.TimeTilDormant, + TimeTilDormantAutoDelete: tpl.TimeTilDormantAutoDelete, }) if err != nil { return xerrors.Errorf("update template schedule: %w", err) diff --git a/coderd/schedule/workspace.go b/coderd/schedule/workspace.go new file mode 100644 index 0000000000000..632653b587073 --- /dev/null +++ b/coderd/schedule/workspace.go @@ -0,0 +1,77 @@ +package schedule + +import ( + "time" + + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/schedule/cron" +) + +// GetWorkspaceAutostartSchedule will return the workspace's autostart schedule +// if it is eligible for autostart. If the workspace is not eligible for +// autostart it will return nil. +func GetWorkspaceAutostartSchedule(templateSchedule TemplateScheduleOptions, workspace database.Workspace) (*cron.Schedule, error) { + if !templateSchedule.UserAutostartEnabled || !workspace.AutostartSchedule.Valid || workspace.AutostartSchedule.String == "" { + return nil, nil + } + + sched, err := cron.Weekly(workspace.AutostartSchedule.String) + if err != nil { + return nil, xerrors.Errorf("parse workspace autostart schedule %q: %w", workspace.AutostartSchedule.String, err) + } + + return sched, nil +} + +// WorkspaceTTL returns the workspace's TTL or the template's default TTL if the +// template forbids the user from setting their own TTL. +func WorkspaceTTL(templateSchedule TemplateScheduleOptions, ws database.Workspace) time.Duration { + var ttl time.Duration + if ws.Ttl.Valid && ws.Ttl.Int64 > 0 { + ttl = time.Duration(ws.Ttl.Int64) + } + if !templateSchedule.UserAutostopEnabled { + ttl = 0 + if templateSchedule.DefaultTTL > 0 { + ttl = templateSchedule.DefaultTTL + } + } + return ttl +} + +// MaybeBumpDeadline may bump the deadline of the workspace if the deadline +// exceeds the next autostart time. The returned time if non-zero if it should +// be used as the new deadline. +// +// The MaxDeadline is not taken into account. +func MaybeBumpDeadline(autostartSchedule *cron.Schedule, deadline time.Time, ttl time.Duration) time.Time { + // Calculate the next autostart after (build.deadline - ttl). If this value + // is before the current deadline + 1h, we should bump the deadline. + autostartTime := autostartSchedule.Next(deadline.Add(-ttl)) + if !autostartTime.Before(deadline.Add(time.Hour)) { + return time.Time{} + } + + // Calculate the expected new deadline. + newDeadline := autostartTime.Add(ttl) + + // Calculate the next autostart time after the one we just calculated. If + // it's before the new deadline, we should not bump the deadline. + // + // If we didn't have this check, we could end up in a situation where we + // would be bumping the deadline every day and keeping it alive permanently. + // E.g. ttl is 1 week but the autostart is daily. We would bump the deadline + // by 1 week every day, keeping the workspace alive forever. + // + // This essentially means we do nothing* if the TTL is longer than a day. + // *Depends on what days the autostart schedule is set to and the TTL + // duration + nextAutostartTime := autostartSchedule.Next(autostartTime) + if nextAutostartTime.Before(newDeadline) { + return time.Time{} + } + + return newDeadline +} diff --git a/coderd/schedule/workspace_test.go b/coderd/schedule/workspace_test.go new file mode 100644 index 0000000000000..f114823088977 --- /dev/null +++ b/coderd/schedule/workspace_test.go @@ -0,0 +1,212 @@ +package schedule_test + +import ( + "database/sql" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" +) + +func TestGetWorkspaceAutostartSchedule(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + autostartEnabled bool + autostartSchedule string + expectedNotNil bool + expectedErrContains string + }{ + { + name: "autostart disabled", + autostartEnabled: false, + autostartSchedule: "CRON_TZ=UTC 0 0 * * *", + expectedNotNil: false, + expectedErrContains: "", + }, + { + name: "autostart enabled, no schedule", + autostartEnabled: true, + autostartSchedule: "", + expectedNotNil: false, + expectedErrContains: "", + }, + { + name: "autostart enabled, invalid schedule", + autostartEnabled: true, + autostartSchedule: "dean was here", + expectedNotNil: false, + expectedErrContains: "parse workspace autostart schedule", + }, + { + name: "autostart enabled, OK schedule", + autostartEnabled: true, + autostartSchedule: "CRON_TZ=UTC 0 0 * * *", + expectedNotNil: true, + expectedErrContains: "", + }, + } + + for _, c := range cases { + c := c + + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + sched, err := schedule.GetWorkspaceAutostartSchedule(schedule.TemplateScheduleOptions{ + UserAutostartEnabled: c.autostartEnabled, + }, database.Workspace{ + AutostartSchedule: sql.NullString{ + String: c.autostartSchedule, + Valid: true, + }, + }) + + if c.expectedErrContains != "" { + require.Error(t, err) + require.Contains(t, err.Error(), c.expectedErrContains) + return + } + require.NoError(t, err) + if c.expectedNotNil { + require.NotNil(t, sched) + } else { + require.Nil(t, sched) + } + }) + } +} + +func TestWorkspaceTTL(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + wsTTL time.Duration + userAutostopEnabled bool + templateDefaultTTL time.Duration + expected time.Duration + }{ + { + name: "Workspace", + wsTTL: time.Hour, + userAutostopEnabled: true, + templateDefaultTTL: time.Hour * 2, + expected: time.Hour, + }, + { + name: "WorkspaceZero", + wsTTL: 0, + userAutostopEnabled: true, + templateDefaultTTL: time.Hour * 2, + expected: 0, + }, + { + name: "Template", + wsTTL: time.Hour, + userAutostopEnabled: false, + templateDefaultTTL: time.Hour * 2, + expected: time.Hour * 2, + }, + { + name: "TemplateZero", + wsTTL: time.Hour, + userAutostopEnabled: false, + templateDefaultTTL: 0, + expected: 0, + }, + } + + for _, c := range cases { + c := c + + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + got := schedule.WorkspaceTTL(schedule.TemplateScheduleOptions{ + UserAutostopEnabled: c.userAutostopEnabled, + DefaultTTL: c.templateDefaultTTL, + }, database.Workspace{ + Ttl: sql.NullInt64{ + Int64: int64(c.wsTTL), + Valid: true, + }, + }) + require.Equal(t, c.expected, got) + }) + } +} + +func TestMaybeBumpDeadline(t *testing.T) { + t.Parallel() + + midnight := time.Date(2023, 10, 4, 0, 0, 0, 0, time.UTC) + + cases := []struct { + name string + autostartSchedule string + deadline time.Time + ttl time.Duration + expected time.Time + }{ + { + name: "not eligible", + autostartSchedule: "CRON_TZ=UTC 0 0 * * *", + deadline: midnight.Add(time.Hour * -2), + ttl: time.Hour * 10, + // not eligible because the deadline is over an hour before the next + // autostart time + expected: time.Time{}, + }, + { + name: "autostart before deadline by 1h", + autostartSchedule: "CRON_TZ=UTC 0 0 * * *", + deadline: midnight.Add(time.Hour), + ttl: time.Hour * 10, + expected: midnight.Add(time.Hour * 10), + }, + { + name: "autostart before deadline by 9h", + autostartSchedule: "CRON_TZ=UTC 0 0 * * *", + deadline: midnight.Add(time.Hour * 9), + ttl: time.Hour * 10, + // should still be bumped + expected: midnight.Add(time.Hour * 10), + }, + { + name: "eligible but exceeds next next autostart", + autostartSchedule: "CRON_TZ=UTC 0 0 * * *", + deadline: midnight.Add(time.Hour * 1), + // ttl causes next autostart + 25h to exceed the next next autostart + ttl: time.Hour * 25, + // should not be bumped to avoid infinite bumping every day + expected: time.Time{}, + }, + { + name: "deadline is 1h before autostart", + autostartSchedule: "CRON_TZ=UTC 0 0 * * *", + deadline: midnight.Add(time.Hour * -1).Add(time.Minute), + ttl: time.Hour * 10, + // should still be bumped + expected: midnight.Add(time.Hour * 10), + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + sched, err := cron.Weekly(c.autostartSchedule) + require.NoError(t, err) + + got := schedule.MaybeBumpDeadline(sched, c.deadline, c.ttl) + require.WithinDuration(t, c.expected, got, time.Minute) + }) + } +} diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index b686bd7f9fb60..65aabdfac8106 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -15,6 +15,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" agpl "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/autostop" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/codersdk" ) @@ -253,7 +254,7 @@ func (s *EnterpriseTemplateScheduleStore) updateWorkspaceBuild(ctx context.Conte return nil } - autostop, err := agpl.CalculateAutostop(ctx, agpl.CalculateAutostopParams{ + autostop, err := autostop.CalculateAutostop(ctx, autostop.CalculateAutostopParams{ Database: db, TemplateScheduleStore: s, UserQuietHoursScheduleStore: *s.UserQuietHoursScheduleStore.Load(),