Skip to content

Commit 47d1e08

Browse files
deansheatherdeansheather
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 4ab5276 commit 47d1e08

24 files changed

+821
-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: 71 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"github.com/coder/coder/v2/coderd/database/provisionerjobs"
2020
"github.com/coder/coder/v2/coderd/database/pubsub"
2121
"github.com/coder/coder/v2/coderd/schedule"
22-
"github.com/coder/coder/v2/coderd/schedule/cron"
2322
"github.com/coder/coder/v2/coderd/wsbuilder"
2423
"github.com/coder/coder/v2/codersdk"
2524
)
@@ -116,7 +115,7 @@ func (e *Executor) runOnce(t time.Time) Stats {
116115
// NOTE: If a workspace build is created with a given TTL and then the user either
117116
// changes or unsets the TTL, the deadline for the workspace build will not
118117
// have changed. This behavior is as expected per #2229.
119-
workspaces, err := e.db.GetWorkspacesEligibleForTransition(e.ctx, t)
118+
workspaces, err := e.db.GetWorkspacesEligibleForTransition(e.ctx)
120119
if err != nil {
121120
e.log.Error(e.ctx, "get workspaces for autostart or autostop", slog.Error(err))
122121
return stats
@@ -183,7 +182,26 @@ func (e *Executor) runOnce(t time.Time) Stats {
183182
}
184183
}
185184

186-
// Transition the workspace to dormant if it has breached the template's
185+
if reason == database.BuildReasonBump {
186+
newDeadline := shouldBump(ws, latestBuild, latestJob, templateSchedule, currentTick)
187+
if !newDeadline.IsZero() {
188+
err := tx.UpdateWorkspaceBuildDeadlineByID(e.ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{
189+
ID: latestBuild.ID,
190+
UpdatedAt: dbtime.Now(),
191+
Deadline: newDeadline,
192+
MaxDeadline: latestBuild.MaxDeadline,
193+
})
194+
if err != nil {
195+
log.Error(e.ctx, "unable to bump workspace deadline",
196+
slog.F("new_deadline", newDeadline),
197+
slog.Error(err),
198+
)
199+
return nil
200+
}
201+
}
202+
}
203+
204+
// Transition the workspace to dormant if it has breached the template's
187205
// threshold for inactivity.
188206
if reason == database.BuildReasonAutolock {
189207
ws, err = tx.UpdateWorkspaceDormantDeletingAt(e.ctx, database.UpdateWorkspaceDormantDeletingAtParams{
@@ -295,6 +313,8 @@ func getNextTransition(
295313

296314
case isEligibleForDelete(ws, templateSchedule, currentTick):
297315
return database.WorkspaceTransitionDelete, database.BuildReasonAutodelete, nil
316+
case !shouldBump(ws, latestBuild, latestJob, templateSchedule, currentTick).IsZero():
317+
return "", database.BuildReasonBump, nil
298318
default:
299319
return "", "", xerrors.Errorf("last transition not valid for autostart or autostop")
300320
}
@@ -320,14 +340,11 @@ func isEligibleForAutostart(ws database.Workspace, build database.WorkspaceBuild
320340

321341
// If autostart isn't enabled, or the schedule isn't valid/populated we can't
322342
// autostart the workspace.
323-
if !templateSchedule.UserAutostartEnabled || !ws.AutostartSchedule.Valid || ws.AutostartSchedule.String == "" {
343+
sched, err := schedule.GetWorkspaceAutostartSchedule(templateSchedule, ws)
344+
if err != nil || sched == nil {
324345
return false
325346
}
326347

327-
sched, err := cron.Weekly(ws.AutostartSchedule.String)
328-
if err != nil {
329-
return false
330-
}
331348
// Round down to the nearest minute, as this is the finest granularity cron supports.
332349
// Truncate is probably not necessary here, but doing it anyway to be sure.
333350
nextTransition := sched.Next(build.CreatedAt).Truncate(time.Minute)
@@ -385,3 +402,49 @@ func isEligibleForFailedStop(build database.WorkspaceBuild, job database.Provisi
385402
job.CompletedAt.Valid &&
386403
currentTick.Sub(job.CompletedAt.Time) > templateSchedule.FailureTTL
387404
}
405+
406+
// shouldBump returns a non-zero time if the workspace deadline should
407+
// should be bumped.
408+
func shouldBump(ws database.Workspace, build database.WorkspaceBuild, job database.ProvisionerJob, templateSchedule schedule.TemplateScheduleOptions, currentTick time.Time) time.Time {
409+
if build.Transition != database.WorkspaceTransitionStart {
410+
return time.Time{}
411+
}
412+
if db2sdk.ProvisionerJobStatus(job) != codersdk.ProvisionerJobSucceeded {
413+
return time.Time{}
414+
}
415+
if build.Deadline.IsZero() {
416+
return time.Time{}
417+
}
418+
if currentTick.After(build.Deadline.Add(time.Hour)) {
419+
// If the workspace has already breached its deadline + 1h, we should
420+
// not bump it, because we don't want to race autostop code.
421+
return time.Time{}
422+
}
423+
424+
ttl := schedule.WorkspaceTTL(templateSchedule, ws)
425+
if ttl <= 0 {
426+
return time.Time{}
427+
}
428+
429+
// If autostart isn't enabled, or the schedule isn't valid/populated we
430+
// don't care about bumping it.
431+
sched, err := schedule.GetWorkspaceAutostartSchedule(templateSchedule, ws)
432+
if err != nil || sched == nil {
433+
return time.Time{}
434+
}
435+
436+
newDeadline := schedule.MaybeBumpDeadline(sched, build.Deadline, ttl)
437+
if newDeadline.IsZero() {
438+
return time.Time{}
439+
}
440+
if newDeadline.Before(build.Deadline) {
441+
// Don't reduce the deadline.
442+
return time.Time{}
443+
}
444+
if !build.MaxDeadline.IsZero() && newDeadline.After(build.MaxDeadline) {
445+
// Don't exceed the max deadline.
446+
return time.Time{}
447+
}
448+
449+
return newDeadline
450+
}

‎coderd/autobuild/lifecycle_executor_test.go

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

2730
func TestExecutorAutostartOK(t *testing.T) {
@@ -701,6 +704,131 @@ func TestExecutorFailedWorkspace(t *testing.T) {
701704
})
702705
}
703706

707+
func TestExecutorBumpWorkspace(t *testing.T) {
708+
t.Parallel()
709+
710+
midnight := time.Date(2023, 10, 4, 0, 0, 0, 0, time.UTC)
711+
712+
cases := []struct {
713+
name string
714+
autostartSchedule string
715+
now time.Time // defaults to 1h before the deadline
716+
deadline time.Time
717+
maxDeadline time.Time
718+
ttl time.Duration
719+
expected time.Time
720+
}{
721+
{
722+
name: "not eligible",
723+
autostartSchedule: "CRON_TZ=UTC 0 0 * * *",
724+
deadline: midnight.Add(time.Hour * -2),
725+
ttl: time.Hour * 10,
726+
// not eligible because the deadline is over an hour before the next
727+
// autostart time
728+
expected: time.Time{},
729+
},
730+
{
731+
name: "autostart before deadline by 1h",
732+
autostartSchedule: "CRON_TZ=UTC 0 0 * * *",
733+
deadline: midnight.Add(time.Hour),
734+
ttl: time.Hour * 10,
735+
expected: midnight.Add(time.Hour * 10),
736+
},
737+
{
738+
name: "autostart before deadline by 9h",
739+
autostartSchedule: "CRON_TZ=UTC 0 0 * * *",
740+
deadline: midnight.Add(time.Hour * 9),
741+
ttl: time.Hour * 10,
742+
// should still be bumped
743+
expected: midnight.Add(time.Hour * 10),
744+
},
745+
{
746+
name: "eligible but exceeds next next autostart",
747+
autostartSchedule: "CRON_TZ=UTC 0 0 * * *",
748+
deadline: midnight.Add(time.Hour * 1),
749+
// ttl causes next autostart + 25h to exceed the next next autostart
750+
ttl: time.Hour * 25,
751+
// should not be bumped to avoid infinite bumping every day
752+
expected: time.Time{},
753+
},
754+
{
755+
name: "deadline is 1h before autostart",
756+
autostartSchedule: "CRON_TZ=UTC 0 0 * * *",
757+
deadline: midnight.Add(time.Hour * -1).Add(time.Minute),
758+
ttl: time.Hour * 10,
759+
// should still be bumped
760+
expected: midnight.Add(time.Hour * 10),
761+
},
762+
}
763+
764+
for _, c := range cases {
765+
c := c
766+
t.Run(c.name, func(t *testing.T) {
767+
t.Parallel()
768+
769+
var (
770+
tickCh = make(chan time.Time)
771+
statsCh = make(chan autobuild.Stats)
772+
db, pubsub = dbtestutil.NewDB(t)
773+
774+
client = coderdtest.New(t, &coderdtest.Options{
775+
AutobuildTicker: tickCh,
776+
AutobuildStats: statsCh,
777+
Database: db,
778+
Pubsub: pubsub,
779+
IncludeProvisionerDaemon: true,
780+
TemplateScheduleStore: schedule.MockTemplateScheduleStore{
781+
GetFn: func(_ context.Context, _ database.Store, _ uuid.UUID) (schedule.TemplateScheduleOptions, error) {
782+
return schedule.TemplateScheduleOptions{
783+
UserAutostartEnabled: true,
784+
UserAutostopEnabled: true,
785+
DefaultTTL: 0,
786+
AutostopRequirement: schedule.TemplateAutostopRequirement{},
787+
}, nil
788+
},
789+
},
790+
})
791+
workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) {
792+
cwr.AutostartSchedule = ptr.Ref(c.autostartSchedule)
793+
cwr.TTLMillis = ptr.Ref(c.ttl.Milliseconds())
794+
})
795+
)
796+
797+
// Set the deadline and max deadline to what the test expects.
798+
ctx := testutil.Context(t, testutil.WaitLong)
799+
err := db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{
800+
ID: workspace.LatestBuild.ID,
801+
UpdatedAt: dbtime.Now(),
802+
Deadline: c.deadline,
803+
MaxDeadline: c.maxDeadline,
804+
})
805+
require.NoError(t, err)
806+
807+
if c.now.IsZero() {
808+
c.now = c.deadline.Add(-time.Hour)
809+
}
810+
811+
go func() {
812+
tickCh <- c.now
813+
close(tickCh)
814+
}()
815+
stats := <-statsCh
816+
require.NoError(t, stats.Error)
817+
require.Len(t, stats.Transitions, 0)
818+
819+
// Get the latest workspace build.
820+
build, err := client.WorkspaceBuild(ctx, workspace.LatestBuild.ID)
821+
require.NoError(t, err)
822+
823+
// Should be bumped to the expected time.
824+
if c.expected.IsZero() {
825+
c.expected = c.deadline
826+
}
827+
require.WithinDuration(t, c.expected, build.Deadline.Time, time.Minute)
828+
})
829+
}
830+
}
831+
704832
// TestExecutorInactiveWorkspace test AGPL functionality which mainly
705833
// ensures that autostop actions as a result of an inactive workspace
706834
// do not trigger.

‎coderd/database/dbauthz/dbauthz.go

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

1918-
func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.Workspace, error) {
1919-
return q.db.GetWorkspacesEligibleForTransition(ctx, now)
1918+
func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context) ([]database.Workspace, error) {
1919+
return q.db.GetWorkspacesEligibleForTransition(ctx)
19201920
}
19211921

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

0 commit comments

Comments
 (0)