From 47d1e0841c541290e5ac061dbcff99da8c473727 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Wed, 4 Oct 2023 12:21:04 +0000 Subject: [PATCH] feat: increase autostop deadline if it passes autostart If the autostop deadline passes the next autostart deadline, increase the autostop deadline to be `next_autostart + ttl` instead of `now + ttl`. This applies both on workspace build and periodically via the lifecycle executor system. It cannot happen immediately on activity bump as there's no way to execute cron statements in the database. --- cli/schedule_test.go | 9 +- coderd/activitybump_internal_test.go | 40 ++- coderd/autobuild/lifecycle_executor.go | 79 +++++- coderd/autobuild/lifecycle_executor_test.go | 128 +++++++++ coderd/database/dbauthz/dbauthz.go | 4 +- coderd/database/dbfake/dbfake.go | 45 ++- coderd/database/dbmetrics/dbmetrics.go | 4 +- coderd/database/dbmock/dbmock.go | 8 +- coderd/database/dump.sql | 3 +- ...00160_workspace_build_reason_bump.down.sql | 1 + .../000160_workspace_build_reason_bump.up.sql | 1 + coderd/database/models.go | 5 +- coderd/database/querier.go | 3 +- coderd/database/queries.sql.go | 257 +++++++++--------- coderd/database/queries/activitybump.sql | 13 +- coderd/database/queries/workspaces.sql | 14 +- .../provisionerdserver/provisionerdserver.go | 3 +- .../provisionerdserver_test.go | 4 +- coderd/schedule/{ => autostop}/autostop.go | 44 +-- .../schedule/{ => autostop}/autostop_test.go | 67 ++++- coderd/schedule/template.go | 12 +- coderd/schedule/workspace.go | 77 ++++++ coderd/schedule/workspace_test.go | 212 +++++++++++++++ enterprise/coderd/schedule/template.go | 3 +- 24 files changed, 821 insertions(+), 215 deletions(-) create mode 100644 coderd/database/migrations/000160_workspace_build_reason_bump.down.sql create mode 100644 coderd/database/migrations/000160_workspace_build_reason_bump.up.sql rename coderd/schedule/{ => autostop}/autostop.go (92%) rename coderd/schedule/{ => autostop}/autostop_test.go (89%) create mode 100644 coderd/schedule/workspace.go create mode 100644 coderd/schedule/workspace_test.go 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(),