Skip to content

Commit 8fc59c4

Browse files
deansheatherjohnstcn
deansheather
authored and
johnstcn
committed
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.
1 parent 2a4ac2a commit 8fc59c4

24 files changed

+822
-215
lines changed

‎cli/schedule_test.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -243,9 +243,8 @@ func TestScheduleOverride(t *testing.T) {
243243
require.NoError(t, err)
244244
expectedDeadline := time.Now().Add(10 * time.Hour)
245245

246-
// Assert test invariant: workspace build has a deadline set equal to now plus ttl
247-
initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond)
248-
require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline.Time, time.Minute)
246+
// Assert test invariant: workspace build has a deadline set
247+
require.False(t, workspace.LatestBuild.Deadline.IsZero())
249248

250249
inv, root := clitest.New(t, cmdArgs...)
251250
clitest.SetupConfig(t, client, root)
@@ -283,10 +282,6 @@ func TestScheduleOverride(t *testing.T) {
283282
workspace, err = client.Workspace(ctx, workspace.ID)
284283
require.NoError(t, err)
285284

286-
// Assert test invariant: workspace build has a deadline set equal to now plus ttl
287-
initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond)
288-
require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline.Time, time.Minute)
289-
290285
inv, root := clitest.New(t, cmdArgs...)
291286
clitest.SetupConfig(t, client, root)
292287
inv.Stdout = stdoutBuf

‎coderd/activitybump_internal_test.go

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,15 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
3232
}
3333

3434
for _, tt := range []struct {
35-
name string
36-
transition database.WorkspaceTransition
37-
jobCompletedAt sql.NullTime
38-
buildDeadlineOffset *time.Duration
39-
maxDeadlineOffset *time.Duration
40-
workspaceTTL time.Duration
41-
expectedBump time.Duration
35+
name string
36+
transition database.WorkspaceTransition
37+
jobCompletedAt sql.NullTime
38+
buildDeadlineOffset *time.Duration
39+
maxDeadlineOffset *time.Duration
40+
workspaceTTL time.Duration
41+
templateDisallowUserAutostop bool // inverted
42+
templateTTL time.Duration
43+
expectedBump time.Duration
4244
}{
4345
{
4446
name: "NotFinishedYet",
@@ -69,6 +71,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
6971
jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-24 * time.Minute)},
7072
buildDeadlineOffset: ptr.Ref(8*time.Hour - 24*time.Minute),
7173
workspaceTTL: 8 * time.Hour,
74+
templateTTL: 6 * time.Hour, // unused
7275
expectedBump: 8 * time.Hour,
7376
},
7477
{
@@ -99,6 +102,18 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
99102
workspaceTTL: 8 * time.Hour,
100103
expectedBump: 0,
101104
},
105+
{
106+
// If the template disallows user autostop, then the TTL of the
107+
// template should be used.
108+
name: "TemplateDisallowsUserAutostop",
109+
transition: database.WorkspaceTransitionStart,
110+
jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-24 * time.Minute)},
111+
buildDeadlineOffset: ptr.Ref(8*time.Hour - 24*time.Minute),
112+
workspaceTTL: 6 * time.Hour,
113+
templateDisallowUserAutostop: true,
114+
templateTTL: 8 * time.Hour,
115+
expectedBump: 8 * time.Hour,
116+
},
102117
} {
103118
tt := tt
104119
for _, tz := range timezones {
@@ -144,6 +159,15 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
144159
buildID = uuid.New()
145160
)
146161

162+
err := db.UpdateTemplateScheduleByID(ctx, database.UpdateTemplateScheduleByIDParams{
163+
ID: template.ID,
164+
UpdatedAt: dbtime.Now(),
165+
AllowUserAutostop: !tt.templateDisallowUserAutostop,
166+
DefaultTTL: int64(tt.templateTTL),
167+
// The other fields don't matter.
168+
})
169+
require.NoError(t, err)
170+
147171
var buildNumber int32 = 1
148172
// Insert a number of previous workspace builds.
149173
for i := 0; i < 5; i++ {
@@ -162,7 +186,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
162186
if tt.maxDeadlineOffset != nil {
163187
maxDeadline = now.Add(*tt.maxDeadlineOffset)
164188
}
165-
err := db.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{
189+
err = db.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{
166190
ID: buildID,
167191
CreatedAt: dbtime.Now(),
168192
UpdatedAt: dbtime.Now(),

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ import (
2222
"github.com/coder/coder/v2/coderd/database/provisionerjobs"
2323
"github.com/coder/coder/v2/coderd/database/pubsub"
2424
"github.com/coder/coder/v2/coderd/schedule"
25-
"github.com/coder/coder/v2/coderd/schedule/cron"
2625
"github.com/coder/coder/v2/coderd/wsbuilder"
26+
"github.com/coder/coder/v2/codersdk"
2727
)
2828

2929
// Executor automatically starts or stops workspaces.
@@ -120,7 +120,7 @@ func (e *Executor) runOnce(t time.Time) Stats {
120120
// NOTE: If a workspace build is created with a given TTL and then the user either
121121
// changes or unsets the TTL, the deadline for the workspace build will not
122122
// have changed. This behavior is as expected per #2229.
123-
workspaces, err := e.db.GetWorkspacesEligibleForTransition(e.ctx, t)
123+
workspaces, err := e.db.GetWorkspacesEligibleForTransition(e.ctx)
124124
if err != nil {
125125
e.log.Error(e.ctx, "get workspaces for autostart or autostop", slog.Error(err))
126126
return stats
@@ -194,7 +194,26 @@ func (e *Executor) runOnce(t time.Time) Stats {
194194
}
195195
}
196196

197-
// Transition the workspace to dormant if it has breached the template's
197+
if reason == database.BuildReasonBump {
198+
newDeadline := shouldBump(ws, latestBuild, latestJob, templateSchedule, currentTick)
199+
if !newDeadline.IsZero() {
200+
err := tx.UpdateWorkspaceBuildDeadlineByID(e.ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{
201+
ID: latestBuild.ID,
202+
UpdatedAt: dbtime.Now(),
203+
Deadline: newDeadline,
204+
MaxDeadline: latestBuild.MaxDeadline,
205+
})
206+
if err != nil {
207+
log.Error(e.ctx, "unable to bump workspace deadline",
208+
slog.F("new_deadline", newDeadline),
209+
slog.Error(err),
210+
)
211+
return nil
212+
}
213+
}
214+
}
215+
216+
// Transition the workspace to dormant if it has breached the template's
198217
// threshold for inactivity.
199218
if reason == database.BuildReasonAutolock {
200219
wsOld := ws
@@ -317,6 +336,8 @@ func getNextTransition(
317336

318337
case isEligibleForDelete(ws, templateSchedule, latestBuild, latestJob, currentTick):
319338
return database.WorkspaceTransitionDelete, database.BuildReasonAutodelete, nil
339+
case !shouldBump(ws, latestBuild, latestJob, templateSchedule, currentTick).IsZero():
340+
return "", database.BuildReasonBump, nil
320341
default:
321342
return "", "", xerrors.Errorf("last transition not valid for autostart or autostop")
322343
}
@@ -342,14 +363,11 @@ func isEligibleForAutostart(ws database.Workspace, build database.WorkspaceBuild
342363

343364
// If autostart isn't enabled, or the schedule isn't valid/populated we can't
344365
// autostart the workspace.
345-
if !templateSchedule.UserAutostartEnabled || !ws.AutostartSchedule.Valid || ws.AutostartSchedule.String == "" {
366+
sched, err := schedule.GetWorkspaceAutostartSchedule(templateSchedule, ws)
367+
if err != nil || sched == nil {
346368
return false
347369
}
348370

349-
sched, err := cron.Weekly(ws.AutostartSchedule.String)
350-
if err != nil {
351-
return false
352-
}
353371
// Round down to the nearest minute, as this is the finest granularity cron supports.
354372
// Truncate is probably not necessary here, but doing it anyway to be sure.
355373
nextTransition := sched.Next(build.CreatedAt).Truncate(time.Minute)
@@ -458,3 +476,49 @@ func auditBuild(ctx context.Context, log slog.Logger, auditor audit.Auditor, par
458476
AdditionalFields: raw,
459477
})
460478
}
479+
480+
// shouldBump returns a non-zero time if the workspace deadline should
481+
// should be bumped.
482+
func shouldBump(ws database.Workspace, build database.WorkspaceBuild, job database.ProvisionerJob, templateSchedule schedule.TemplateScheduleOptions, currentTick time.Time) time.Time {
483+
if build.Transition != database.WorkspaceTransitionStart {
484+
return time.Time{}
485+
}
486+
if codersdk.ProvisionerJobStatus(job.JobStatus) != codersdk.ProvisionerJobSucceeded {
487+
return time.Time{}
488+
}
489+
if build.Deadline.IsZero() {
490+
return time.Time{}
491+
}
492+
if currentTick.After(build.Deadline.Add(time.Hour)) {
493+
// If the workspace has already breached its deadline + 1h, we should
494+
// not bump it, because we don't want to race autostop code.
495+
return time.Time{}
496+
}
497+
498+
ttl := schedule.WorkspaceTTL(templateSchedule, ws)
499+
if ttl <= 0 {
500+
return time.Time{}
501+
}
502+
503+
// If autostart isn't enabled, or the schedule isn't valid/populated we
504+
// don't care about bumping it.
505+
sched, err := schedule.GetWorkspaceAutostartSchedule(templateSchedule, ws)
506+
if err != nil || sched == nil {
507+
return time.Time{}
508+
}
509+
510+
newDeadline := schedule.MaybeBumpDeadline(sched, build.Deadline, ttl)
511+
if newDeadline.IsZero() {
512+
return time.Time{}
513+
}
514+
if newDeadline.Before(build.Deadline) {
515+
// Don't reduce the deadline.
516+
return time.Time{}
517+
}
518+
if !build.MaxDeadline.IsZero() && newDeadline.After(build.MaxDeadline) {
519+
// Don't exceed the max deadline.
520+
return time.Time{}
521+
}
522+
523+
return newDeadline
524+
}

‎coderd/autobuild/lifecycle_executor_test.go

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,15 @@ import (
1717
"github.com/coder/coder/v2/coderd/autobuild"
1818
"github.com/coder/coder/v2/coderd/coderdtest"
1919
"github.com/coder/coder/v2/coderd/database"
20+
"github.com/coder/coder/v2/coderd/database/dbtestutil"
21+
"github.com/coder/coder/v2/coderd/database/dbtime"
2022
"github.com/coder/coder/v2/coderd/schedule"
2123
"github.com/coder/coder/v2/coderd/schedule/cron"
2224
"github.com/coder/coder/v2/coderd/util/ptr"
2325
"github.com/coder/coder/v2/codersdk"
2426
"github.com/coder/coder/v2/provisioner/echo"
2527
"github.com/coder/coder/v2/provisionersdk/proto"
28+
"github.com/coder/coder/v2/testutil"
2629
)
2730

2831
func TestExecutorAutostartOK(t *testing.T) {
@@ -781,6 +784,131 @@ func TestExecutorFailedWorkspace(t *testing.T) {
781784
})
782785
}
783786

787+
func TestExecutorBumpWorkspace(t *testing.T) {
788+
t.Parallel()
789+
790+
midnight := time.Date(2023, 10, 4, 0, 0, 0, 0, time.UTC)
791+
792+
cases := []struct {
793+
name string
794+
autostartSchedule string
795+
now time.Time // defaults to 1h before the deadline
796+
deadline time.Time
797+
maxDeadline time.Time
798+
ttl time.Duration
799+
expected time.Time
800+
}{
801+
{
802+
name: "not eligible",
803+
autostartSchedule: "CRON_TZ=UTC 0 0 * * *",
804+
deadline: midnight.Add(time.Hour * -2),
805+
ttl: time.Hour * 10,
806+
// not eligible because the deadline is over an hour before the next
807+
// autostart time
808+
expected: time.Time{},
809+
},
810+
{
811+
name: "autostart before deadline by 1h",
812+
autostartSchedule: "CRON_TZ=UTC 0 0 * * *",
813+
deadline: midnight.Add(time.Hour),
814+
ttl: time.Hour * 10,
815+
expected: midnight.Add(time.Hour * 10),
816+
},
817+
{
818+
name: "autostart before deadline by 9h",
819+
autostartSchedule: "CRON_TZ=UTC 0 0 * * *",
820+
deadline: midnight.Add(time.Hour * 9),
821+
ttl: time.Hour * 10,
822+
// should still be bumped
823+
expected: midnight.Add(time.Hour * 10),
824+
},
825+
{
826+
name: "eligible but exceeds next next autostart",
827+
autostartSchedule: "CRON_TZ=UTC 0 0 * * *",
828+
deadline: midnight.Add(time.Hour * 1),
829+
// ttl causes next autostart + 25h to exceed the next next autostart
830+
ttl: time.Hour * 25,
831+
// should not be bumped to avoid infinite bumping every day
832+
expected: time.Time{},
833+
},
834+
{
835+
name: "deadline is 1h before autostart",
836+
autostartSchedule: "CRON_TZ=UTC 0 0 * * *",
837+
deadline: midnight.Add(time.Hour * -1).Add(time.Minute),
838+
ttl: time.Hour * 10,
839+
// should still be bumped
840+
expected: midnight.Add(time.Hour * 10),
841+
},
842+
}
843+
844+
for _, c := range cases {
845+
c := c
846+
t.Run(c.name, func(t *testing.T) {
847+
t.Parallel()
848+
849+
var (
850+
tickCh = make(chan time.Time)
851+
statsCh = make(chan autobuild.Stats)
852+
db, pubsub = dbtestutil.NewDB(t)
853+
854+
client = coderdtest.New(t, &coderdtest.Options{
855+
AutobuildTicker: tickCh,
856+
AutobuildStats: statsCh,
857+
Database: db,
858+
Pubsub: pubsub,
859+
IncludeProvisionerDaemon: true,
860+
TemplateScheduleStore: schedule.MockTemplateScheduleStore{
861+
GetFn: func(_ context.Context, _ database.Store, _ uuid.UUID) (schedule.TemplateScheduleOptions, error) {
862+
return schedule.TemplateScheduleOptions{
863+
UserAutostartEnabled: true,
864+
UserAutostopEnabled: true,
865+
DefaultTTL: 0,
866+
AutostopRequirement: schedule.TemplateAutostopRequirement{},
867+
}, nil
868+
},
869+
},
870+
})
871+
workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) {
872+
cwr.AutostartSchedule = ptr.Ref(c.autostartSchedule)
873+
cwr.TTLMillis = ptr.Ref(c.ttl.Milliseconds())
874+
})
875+
)
876+
877+
// Set the deadline and max deadline to what the test expects.
878+
ctx := testutil.Context(t, testutil.WaitLong)
879+
err := db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{
880+
ID: workspace.LatestBuild.ID,
881+
UpdatedAt: dbtime.Now(),
882+
Deadline: c.deadline,
883+
MaxDeadline: c.maxDeadline,
884+
})
885+
require.NoError(t, err)
886+
887+
if c.now.IsZero() {
888+
c.now = c.deadline.Add(-time.Hour)
889+
}
890+
891+
go func() {
892+
tickCh <- c.now
893+
close(tickCh)
894+
}()
895+
stats := <-statsCh
896+
require.NoError(t, stats.Error)
897+
require.Len(t, stats.Transitions, 0)
898+
899+
// Get the latest workspace build.
900+
build, err := client.WorkspaceBuild(ctx, workspace.LatestBuild.ID)
901+
require.NoError(t, err)
902+
903+
// Should be bumped to the expected time.
904+
if c.expected.IsZero() {
905+
c.expected = c.deadline
906+
}
907+
require.WithinDuration(t, c.expected, build.Deadline.Time, time.Minute)
908+
})
909+
}
910+
}
911+
784912
// TestExecutorInactiveWorkspace test AGPL functionality which mainly
785913
// ensures that autostop actions as a result of an inactive workspace
786914
// do not trigger.

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1926,8 +1926,8 @@ func (q *querier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesP
19261926
return q.db.GetAuthorizedWorkspaces(ctx, arg, prep)
19271927
}
19281928

1929-
func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.Workspace, error) {
1930-
return q.db.GetWorkspacesEligibleForTransition(ctx, now)
1929+
func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context) ([]database.Workspace, error) {
1930+
return q.db.GetWorkspacesEligibleForTransition(ctx)
19311931
}
19321932

19331933
func (q *querier) InsertAPIKey(ctx context.Context, arg database.InsertAPIKeyParams) (database.APIKey, error) {

0 commit comments

Comments
 (0)