Skip to content

Commit 7523edf

Browse files
EmyrkEmyrk
committed
chore: cover deadline crossing autostart border
1 parent 53f7e9e commit 7523edf

File tree

5 files changed

+123
-35
lines changed

5 files changed

+123
-35
lines changed

‎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.NextAutostartSchedule(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/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+
// NextAutostartSchedule 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 NextAutostartSchedule(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.NextAutostartSchedule
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 := NextAutostartSchedule(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: 54 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,33 @@ 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+
},
367404
}
368405

369406
for _, c := range cases {
@@ -382,6 +419,7 @@ func TestCalculateAutoStop(t *testing.T) {
382419
UserAutostopEnabled: c.templateAllowAutostop,
383420
DefaultTTL: c.templateDefaultTTL,
384421
AutostopRequirement: c.templateAutostopRequirement,
422+
AutostartRequirement: c.templateAutoStart,
385423
}, nil
386424
},
387425
}
@@ -433,11 +471,20 @@ func TestCalculateAutoStop(t *testing.T) {
433471
Valid: true,
434472
}
435473
}
474+
475+
autostart := sql.NullString{}
476+
if c.wsAutostart != "" {
477+
autostart = sql.NullString{
478+
String: c.wsAutostart,
479+
Valid: true,
480+
}
481+
}
436482
workspace := dbgen.Workspace(t, db, database.Workspace{
437-
TemplateID: template.ID,
438-
OrganizationID: org.ID,
439-
OwnerID: user.ID,
440-
Ttl: workspaceTTL,
483+
TemplateID: template.ID,
484+
OrganizationID: org.ID,
485+
OwnerID: user.ID,
486+
Ttl: workspaceTTL,
487+
AutostartSchedule: autostart,
441488
})
442489

443490
autostop, err := schedule.CalculateAutostop(ctx, schedule.CalculateAutostopParams{
@@ -446,6 +493,7 @@ func TestCalculateAutoStop(t *testing.T) {
446493
UserQuietHoursScheduleStore: userQuietHoursScheduleStore,
447494
Now: c.now,
448495
Workspace: workspace,
496+
WorkspaceAutostart: c.wsAutostart,
449497
})
450498
if c.errContains != "" {
451499
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.NextAutostartSchedule(time.Now(), workspace.AutostartSchedule.String, templateSchedule)
11901190
if allowed {
11911191
nextAutostart = next
11921192
}

0 commit comments

Comments
 (0)