Skip to content

Commit 845407f

Browse files
EmyrkEmyrk
authored
chore: cover deadline crossing autostart border on start (coder#13115)
When starting a workspace, if the deadline crosses an autostart boundary, the deadline is set to autostart + TTL. This copies the behavior in `ActivityBumpWorkspace`, but does not require activity.
1 parent 71a03a8 commit 845407f

File tree

8 files changed

+211
-39
lines changed

8 files changed

+211
-39
lines changed

‎coderd/agentapi/stats.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313

1414
"cdr.dev/slog"
1515
agentproto "github.com/coder/coder/v2/agent/proto"
16-
"github.com/coder/coder/v2/coderd/autobuild"
1716
"github.com/coder/coder/v2/coderd/database"
1817
"github.com/coder/coder/v2/coderd/database/dbtime"
1918
"github.com/coder/coder/v2/coderd/database/pubsub"
@@ -84,7 +83,7 @@ func (a *StatsAPI) UpdateStats(ctx context.Context, req *agentproto.UpdateStatsR
8483
slog.Error(err),
8584
)
8685
} else {
87-
next, allowed := autobuild.NextAutostartSchedule(now, workspace.AutostartSchedule.String, templateSchedule)
86+
next, allowed := schedule.NextAutostart(now, workspace.AutostartSchedule.String, templateSchedule)
8887
if allowed {
8988
nextAutostart = next
9089
}

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"github.com/coder/coder/v2/coderd/database/provisionerjobs"
2121
"github.com/coder/coder/v2/coderd/database/pubsub"
2222
"github.com/coder/coder/v2/coderd/schedule"
23-
"github.com/coder/coder/v2/coderd/schedule/cron"
2423
"github.com/coder/coder/v2/coderd/wsbuilder"
2524
)
2625

@@ -368,7 +367,7 @@ func isEligibleForAutostart(user database.User, ws database.Workspace, build dat
368367
return false
369368
}
370369

371-
nextTransition, allowed := NextAutostartSchedule(build.CreatedAt, ws.AutostartSchedule.String, templateSchedule)
370+
nextTransition, allowed := schedule.NextAutostart(build.CreatedAt, ws.AutostartSchedule.String, templateSchedule)
372371
if !allowed {
373372
return false
374373
}
@@ -377,29 +376,6 @@ func isEligibleForAutostart(user database.User, ws database.Workspace, build dat
377376
return !currentTick.Before(nextTransition)
378377
}
379378

380-
// NextAutostartSchedule takes the workspace and template schedule and returns the next autostart schedule
381-
// after "at". The boolean returned is if the autostart should be allowed to start based on the template
382-
// schedule.
383-
func NextAutostartSchedule(at time.Time, wsSchedule string, templateSchedule schedule.TemplateScheduleOptions) (time.Time, bool) {
384-
sched, err := cron.Weekly(wsSchedule)
385-
if err != nil {
386-
return time.Time{}, false
387-
}
388-
389-
// Round down to the nearest minute, as this is the finest granularity cron supports.
390-
// Truncate is probably not necessary here, but doing it anyway to be sure.
391-
nextTransition := sched.Next(at).Truncate(time.Minute)
392-
393-
// The nextTransition is when the auto start should kick off. If it lands on a
394-
// forbidden day, do not allow the auto start. We use the time location of the
395-
// schedule to determine the weekday. So if "Saturday" is disallowed, the
396-
// definition of "Saturday" depends on the location of the schedule.
397-
zonedTransition := nextTransition.In(sched.Location())
398-
allowed := templateSchedule.AutostartRequirement.DaysMap()[zonedTransition.Weekday()]
399-
400-
return zonedTransition, allowed
401-
}
402-
403379
// isEligibleForAutostart returns true if the workspace should be autostopped.
404380
func isEligibleForAutostop(ws database.Workspace, build database.WorkspaceBuild, job database.ProvisionerJob, currentTick time.Time) bool {
405381
if job.JobStatus == database.ProvisionerJobStatusFailed {

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,6 +1257,8 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob)
12571257
UserQuietHoursScheduleStore: *s.UserQuietHoursScheduleStore.Load(),
12581258
Now: now,
12591259
Workspace: workspace,
1260+
// Allowed to be the empty string.
1261+
WorkspaceAutostart: workspace.AutostartSchedule.String,
12601262
})
12611263
if err != nil {
12621264
return xerrors.Errorf("calculate auto stop: %w", err)

‎coderd/schedule/autostart.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package schedule
2+
3+
import (
4+
"time"
5+
6+
"github.com/coder/coder/v2/coderd/schedule/cron"
7+
)
8+
9+
// NextAutostart takes the workspace and template schedule and returns the next autostart schedule
10+
// after "at". The boolean returned is if the autostart should be allowed to start based on the template
11+
// schedule.
12+
func NextAutostart(at time.Time, wsSchedule string, templateSchedule TemplateScheduleOptions) (time.Time, bool) {
13+
sched, err := cron.Weekly(wsSchedule)
14+
if err != nil {
15+
return time.Time{}, false
16+
}
17+
18+
// Round down to the nearest minute, as this is the finest granularity cron supports.
19+
// Truncate is probably not necessary here, but doing it anyway to be sure.
20+
nextTransition := sched.Next(at).Truncate(time.Minute)
21+
22+
// The nextTransition is when the auto start should kick off. If it lands on a
23+
// forbidden day, do not allow the auto start. We use the time location of the
24+
// schedule to determine the weekday. So if "Saturday" is disallowed, the
25+
// definition of "Saturday" depends on the location of the schedule.
26+
zonedTransition := nextTransition.In(sched.Location())
27+
allowed := templateSchedule.AutostartRequirement.DaysMap()[zonedTransition.Weekday()]
28+
29+
return zonedTransition, allowed
30+
}

‎coderd/schedule/autostop.go

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ type CalculateAutostopParams struct {
4444
Database database.Store
4545
TemplateScheduleStore TemplateScheduleStore
4646
UserQuietHoursScheduleStore UserQuietHoursScheduleStore
47+
// WorkspaceAutostart can be the empty string if no workspace autostart
48+
// is configured.
49+
// If configured, this is expected to be a cron weekly event parsable
50+
// by autobuild.NextAutostart
51+
WorkspaceAutostart string
4752

4853
Now time.Time
4954
Workspace database.Workspace
@@ -90,6 +95,14 @@ func CalculateAutostop(ctx context.Context, params CalculateAutostopParams) (Aut
9095
autostop AutostopTime
9196
)
9297

98+
var ttl time.Duration
99+
if workspace.Ttl.Valid {
100+
// When the workspace is made it copies the template's TTL, and the user
101+
// can unset it to disable it (unless the template has
102+
// UserAutoStopEnabled set to false, see below).
103+
ttl = time.Duration(workspace.Ttl.Int64)
104+
}
105+
93106
if workspace.Ttl.Valid {
94107
// When the workspace is made it copies the template's TTL, and the user
95108
// can unset it to disable it (unless the template has
@@ -104,9 +117,30 @@ func CalculateAutostop(ctx context.Context, params CalculateAutostopParams) (Aut
104117
if !templateSchedule.UserAutostopEnabled {
105118
// The user is not permitted to set their own TTL, so use the template
106119
// default.
107-
autostop.Deadline = time.Time{}
120+
ttl = 0
108121
if templateSchedule.DefaultTTL > 0 {
109-
autostop.Deadline = now.Add(templateSchedule.DefaultTTL)
122+
ttl = templateSchedule.DefaultTTL
123+
}
124+
}
125+
126+
if ttl > 0 {
127+
// Only apply non-zero TTLs.
128+
autostop.Deadline = now.Add(ttl)
129+
if params.WorkspaceAutostart != "" {
130+
// If the deadline passes the next autostart, we need to extend the deadline to
131+
// autostart + deadline. ActivityBumpWorkspace already covers this case
132+
// when extending the deadline.
133+
//
134+
// Situation this is solving.
135+
// 1. User has workspace with auto-start at 9:00am, 12 hour auto-stop.
136+
// 2. Coder stops workspace at 9pm
137+
// 3. User starts workspace at 9:45pm.
138+
// - The initial deadline is calculated to be 9:45am
139+
// - This crosses the autostart deadline, so the deadline is extended to 9pm
140+
nextAutostart, ok := NextAutostart(params.Now, params.WorkspaceAutostart, templateSchedule)
141+
if ok && autostop.Deadline.After(nextAutostart) {
142+
autostop.Deadline = nextAutostart.Add(ttl)
143+
}
110144
}
111145
}
112146

‎coderd/schedule/autostop_test.go

Lines changed: 136 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ func TestCalculateAutoStop(t *testing.T) {
2525

2626
now := time.Now()
2727

28+
chicago, err := time.LoadLocation("America/Chicago")
29+
require.NoError(t, err, "loading chicago time location")
30+
31+
// pastDateNight is 9:45pm on a wednesday
32+
pastDateNight := time.Date(2024, 2, 14, 21, 45, 0, 0, chicago)
33+
2834
// Wednesday the 8th of February 2023 at midnight. This date was
2935
// specifically chosen as it doesn't fall on a applicable week for both
3036
// fortnightly and triweekly autostop requirements.
@@ -70,8 +76,12 @@ func TestCalculateAutoStop(t *testing.T) {
7076
t.Log("saturdayMidnightAfterDstOut", saturdayMidnightAfterDstOut)
7177

7278
cases := []struct {
73-
name string
74-
now time.Time
79+
name string
80+
now time.Time
81+
82+
wsAutostart string
83+
templateAutoStart schedule.TemplateAutostartRequirement
84+
7585
templateAllowAutostop bool
7686
templateDefaultTTL time.Duration
7787
templateAutostopRequirement schedule.TemplateAutostopRequirement
@@ -364,6 +374,115 @@ func TestCalculateAutoStop(t *testing.T) {
364374
// expectedDeadline is copied from expectedMaxDeadline.
365375
expectedMaxDeadline: dstOutQuietHoursExpectedTime,
366376
},
377+
{
378+
// A user expects this workspace to be online from 9am -> 9pm.
379+
// So if a deadline is going to land in the middle of this range,
380+
// we should bump it to the end.
381+
// This is already done on `ActivityBumpWorkspace`, but that requires
382+
// activity on the workspace.
383+
name: "AutostopCrossAutostartBorder",
384+
// Starting at 9:45pm, with the autostart at 9am.
385+
now: pastDateNight,
386+
templateAllowAutostop: false,
387+
templateDefaultTTL: time.Hour * 12,
388+
workspaceTTL: time.Hour * 12,
389+
// At 9am every morning
390+
wsAutostart: "CRON_TZ=America/Chicago 0 9 * * *",
391+
392+
// No quiet hours
393+
templateAutoStart: schedule.TemplateAutostartRequirement{
394+
// Just allow all days of the week
395+
DaysOfWeek: 0b01111111,
396+
},
397+
templateAutostopRequirement: schedule.TemplateAutostopRequirement{},
398+
userQuietHoursSchedule: "",
399+
400+
expectedDeadline: time.Date(pastDateNight.Year(), pastDateNight.Month(), pastDateNight.Day()+1, 21, 0, 0, 0, chicago),
401+
expectedMaxDeadline: time.Time{},
402+
errContains: "",
403+
},
404+
{
405+
// Same as AutostopCrossAutostartBorder, but just misses the autostart.
406+
name: "AutostopCrossMissAutostartBorder",
407+
// Starting at 8:45pm, with the autostart at 9am.
408+
now: time.Date(pastDateNight.Year(), pastDateNight.Month(), pastDateNight.Day(), 20, 30, 0, 0, chicago),
409+
templateAllowAutostop: false,
410+
templateDefaultTTL: time.Hour * 12,
411+
workspaceTTL: time.Hour * 12,
412+
// At 9am every morning
413+
wsAutostart: "CRON_TZ=America/Chicago 0 9 * * *",
414+
415+
// No quiet hours
416+
templateAutoStart: schedule.TemplateAutostartRequirement{
417+
// Just allow all days of the week
418+
DaysOfWeek: 0b01111111,
419+
},
420+
templateAutostopRequirement: schedule.TemplateAutostopRequirement{},
421+
userQuietHoursSchedule: "",
422+
423+
expectedDeadline: time.Date(pastDateNight.Year(), pastDateNight.Month(), pastDateNight.Day()+1, 8, 30, 0, 0, chicago),
424+
expectedMaxDeadline: time.Time{},
425+
errContains: "",
426+
},
427+
{
428+
// Same as AutostopCrossAutostartBorderMaxEarlyDeadline with max deadline to limit it.
429+
// The autostop deadline is before the autostart threshold.
430+
name: "AutostopCrossAutostartBorderMaxEarlyDeadline",
431+
// Starting at 9:45pm, with the autostart at 9am.
432+
now: pastDateNight,
433+
templateAllowAutostop: false,
434+
templateDefaultTTL: time.Hour * 12,
435+
workspaceTTL: time.Hour * 12,
436+
// At 9am every morning
437+
wsAutostart: "CRON_TZ=America/Chicago 0 9 * * *",
438+
439+
// No quiet hours
440+
templateAutoStart: schedule.TemplateAutostartRequirement{
441+
// Just allow all days of the week
442+
DaysOfWeek: 0b01111111,
443+
},
444+
templateAutostopRequirement: schedule.TemplateAutostopRequirement{
445+
// Autostop every day
446+
DaysOfWeek: 0b01111111,
447+
Weeks: 0,
448+
},
449+
// 6am quiet hours
450+
userQuietHoursSchedule: "CRON_TZ=America/Chicago 0 6 * * *",
451+
452+
expectedDeadline: time.Date(pastDateNight.Year(), pastDateNight.Month(), pastDateNight.Day()+1, 6, 0, 0, 0, chicago),
453+
expectedMaxDeadline: time.Date(pastDateNight.Year(), pastDateNight.Month(), pastDateNight.Day()+1, 6, 0, 0, 0, chicago),
454+
errContains: "",
455+
},
456+
{
457+
// Same as AutostopCrossAutostartBorder with max deadline to limit it.
458+
// The autostop deadline is after autostart threshold.
459+
// So the deadline is > 12 hours, but stops at the max deadline.
460+
name: "AutostopCrossAutostartBorderMaxDeadline",
461+
// Starting at 9:45pm, with the autostart at 9am.
462+
now: pastDateNight,
463+
templateAllowAutostop: false,
464+
templateDefaultTTL: time.Hour * 12,
465+
workspaceTTL: time.Hour * 12,
466+
// At 9am every morning
467+
wsAutostart: "CRON_TZ=America/Chicago 0 9 * * *",
468+
469+
// No quiet hours
470+
templateAutoStart: schedule.TemplateAutostartRequirement{
471+
// Just allow all days of the week
472+
DaysOfWeek: 0b01111111,
473+
},
474+
templateAutostopRequirement: schedule.TemplateAutostopRequirement{
475+
// Autostop every day
476+
DaysOfWeek: 0b01111111,
477+
Weeks: 0,
478+
},
479+
// 11am quiet hours, yea this is werid case.
480+
userQuietHoursSchedule: "CRON_TZ=America/Chicago 0 11 * * *",
481+
482+
expectedDeadline: time.Date(pastDateNight.Year(), pastDateNight.Month(), pastDateNight.Day()+1, 11, 0, 0, 0, chicago),
483+
expectedMaxDeadline: time.Date(pastDateNight.Year(), pastDateNight.Month(), pastDateNight.Day()+1, 11, 0, 0, 0, chicago),
484+
errContains: "",
485+
},
367486
}
368487

369488
for _, c := range cases {
@@ -382,6 +501,7 @@ func TestCalculateAutoStop(t *testing.T) {
382501
UserAutostopEnabled: c.templateAllowAutostop,
383502
DefaultTTL: c.templateDefaultTTL,
384503
AutostopRequirement: c.templateAutostopRequirement,
504+
AutostartRequirement: c.templateAutoStart,
385505
}, nil
386506
},
387507
}
@@ -433,11 +553,20 @@ func TestCalculateAutoStop(t *testing.T) {
433553
Valid: true,
434554
}
435555
}
556+
557+
autostart := sql.NullString{}
558+
if c.wsAutostart != "" {
559+
autostart = sql.NullString{
560+
String: c.wsAutostart,
561+
Valid: true,
562+
}
563+
}
436564
workspace := dbgen.Workspace(t, db, database.Workspace{
437-
TemplateID: template.ID,
438-
OrganizationID: org.ID,
439-
OwnerID: user.ID,
440-
Ttl: workspaceTTL,
565+
TemplateID: template.ID,
566+
OrganizationID: org.ID,
567+
OwnerID: user.ID,
568+
Ttl: workspaceTTL,
569+
AutostartSchedule: autostart,
441570
})
442571

443572
autostop, err := schedule.CalculateAutostop(ctx, schedule.CalculateAutostopParams{
@@ -446,6 +575,7 @@ func TestCalculateAutoStop(t *testing.T) {
446575
UserQuietHoursScheduleStore: userQuietHoursScheduleStore,
447576
Now: c.now,
448577
Workspace: workspace,
578+
WorkspaceAutostart: c.wsAutostart,
449579
})
450580
if c.errContains != "" {
451581
require.Error(t, err)

‎coderd/workspaceagents.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
"cdr.dev/slog"
2828
agentproto "github.com/coder/coder/v2/agent/proto"
2929
"github.com/coder/coder/v2/coderd/agentapi"
30-
"github.com/coder/coder/v2/coderd/autobuild"
3130
"github.com/coder/coder/v2/coderd/database"
3231
"github.com/coder/coder/v2/coderd/database/db2sdk"
3332
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -37,6 +36,7 @@ import (
3736
"github.com/coder/coder/v2/coderd/httpmw"
3837
"github.com/coder/coder/v2/coderd/prometheusmetrics"
3938
"github.com/coder/coder/v2/coderd/rbac"
39+
"github.com/coder/coder/v2/coderd/schedule"
4040
"github.com/coder/coder/v2/codersdk"
4141
"github.com/coder/coder/v2/codersdk/agentsdk"
4242
"github.com/coder/coder/v2/codersdk/workspacesdk"
@@ -1186,7 +1186,7 @@ func (api *API) workspaceAgentReportStats(rw http.ResponseWriter, r *http.Reques
11861186
slog.Error(err),
11871187
)
11881188
} else {
1189-
next, allowed := autobuild.NextAutostartSchedule(time.Now(), workspace.AutostartSchedule.String, templateSchedule)
1189+
next, allowed := schedule.NextAutostart(time.Now(), workspace.AutostartSchedule.String, templateSchedule)
11901190
if allowed {
11911191
nextAutostart = next
11921192
}

‎enterprise/coderd/schedule/template.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,9 @@ func (s *EnterpriseTemplateScheduleStore) updateWorkspaceBuild(ctx context.Conte
263263
TemplateScheduleStore: s,
264264
UserQuietHoursScheduleStore: *s.UserQuietHoursScheduleStore.Load(),
265265
// Use the job completion time as the time we calculate autostop from.
266-
Now: job.CompletedAt.Time,
267-
Workspace: workspace,
266+
Now: job.CompletedAt.Time,
267+
Workspace: workspace,
268+
WorkspaceAutostart: workspace.AutostartSchedule.String,
268269
})
269270
if err != nil {
270271
return xerrors.Errorf("calculate new autostop for workspace %q: %w", workspace.ID, err)

0 commit comments

Comments
 (0)