chore: cover deadline crossing autostart border

coder · Emyrk · May 1, 2024 · Apr 30, 2024 · Apr 30, 2024 · Apr 30, 2024
commit 7523edf12aac0a2fe9235d6b6358f59aa9cc9d6f
diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go
@@ -20,7 +20,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"
 )
 
@@ -368,7 +367,7 @@ func isEligibleForAutostart(user database.User, ws database.Workspace, build dat
 		return false
 	}
 
-	nextTransition, allowed := NextAutostartSchedule(build.CreatedAt, ws.AutostartSchedule.String, templateSchedule)
+	nextTransition, allowed := schedule.NextAutostartSchedule(build.CreatedAt, ws.AutostartSchedule.String, templateSchedule)
 	if !allowed {
 		return false
 	}
@@ -377,29 +376,6 @@ func isEligibleForAutostart(user database.User, ws database.Workspace, build dat
 	return !currentTick.Before(nextTransition)
 }
 
-// NextAutostartSchedule takes the workspace and template schedule and returns the next autostart schedule
-// after "at". The boolean returned is if the autostart should be allowed to start based on the template
-// schedule.
-func NextAutostartSchedule(at time.Time, wsSchedule string, templateSchedule schedule.TemplateScheduleOptions) (time.Time, bool) {
-	sched, err := cron.Weekly(wsSchedule)
-	if err != nil {
-		return time.Time{}, 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(at).Truncate(time.Minute)
-
-	// The nextTransition is when the auto start should kick off. If it lands on a
-	// forbidden day, do not allow the auto start. We use the time location of the
-	// schedule to determine the weekday. So if "Saturday" is disallowed, the
-	// definition of "Saturday" depends on the location of the schedule.
-	zonedTransition := nextTransition.In(sched.Location())
-	allowed := templateSchedule.AutostartRequirement.DaysMap()[zonedTransition.Weekday()]
-
-	return zonedTransition, allowed
-}
-
 // isEligibleForAutostart returns true if the workspace should be autostopped.
 func isEligibleForAutostop(ws database.Workspace, build database.WorkspaceBuild, job database.ProvisionerJob, currentTick time.Time) bool {
 	if job.JobStatus == database.ProvisionerJobStatusFailed {

diff --git a/coderd/schedule/autostart.go b/coderd/schedule/autostart.go
@@ -0,0 +1,30 @@
+package schedule
+
+import (
+	"time"
+
+	"github.com/coder/coder/v2/coderd/schedule/cron"
+)
+
+// NextAutostartSchedule takes the workspace and template schedule and returns the next autostart schedule
+// after "at". The boolean returned is if the autostart should be allowed to start based on the template
+// schedule.
+func NextAutostartSchedule(at time.Time, wsSchedule string, templateSchedule TemplateScheduleOptions) (time.Time, bool) {
+	sched, err := cron.Weekly(wsSchedule)
+	if err != nil {
+		return time.Time{}, 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(at).Truncate(time.Minute)
+
+	// The nextTransition is when the auto start should kick off. If it lands on a
+	// forbidden day, do not allow the auto start. We use the time location of the
+	// schedule to determine the weekday. So if "Saturday" is disallowed, the
+	// definition of "Saturday" depends on the location of the schedule.
+	zonedTransition := nextTransition.In(sched.Location())
+	allowed := templateSchedule.AutostartRequirement.DaysMap()[zonedTransition.Weekday()]
+
+	return zonedTransition, allowed
+}
@@ -44,6 +44,11 @@ type CalculateAutostopParams struct {
 	Database                    database.Store
 	TemplateScheduleStore       TemplateScheduleStore
 	UserQuietHoursScheduleStore UserQuietHoursScheduleStore
+	// WorkspaceAutostart can be the empty string if no workspace autostart
+	// is configured.
+	// If configured, this is expected to be a cron weekly event parsable
+	// by autobuild.NextAutostartSchedule
+	WorkspaceAutostart string
 
 	Now       time.Time
 	Workspace database.Workspace
@@ -90,6 +95,14 @@ func CalculateAutostop(ctx context.Context, params CalculateAutostopParams) (Aut
 		autostop AutostopTime
 	)
 
+	var ttl time.Duration
+	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).
+		ttl = time.Duration(workspace.Ttl.Int64)
+	}
+
 	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
@@ -104,9 +117,30 @@ func CalculateAutostop(ctx context.Context, params CalculateAutostopParams) (Aut
 	if !templateSchedule.UserAutostopEnabled {
 		// The user is not permitted to set their own TTL, so use the template
 		// default.
-		autostop.Deadline = time.Time{}
+		ttl = 0
 		if templateSchedule.DefaultTTL > 0 {
-			autostop.Deadline = now.Add(templateSchedule.DefaultTTL)
+			ttl = templateSchedule.DefaultTTL
+		}
+	}
+
+	if ttl > 0 {
+		// Only apply non-zero TTLs.
+		autostop.Deadline = now.Add(ttl)
+		if params.WorkspaceAutostart != "" {
+			// If the deadline passes the next autostart, we need to extend the deadline to
+			// autostart + deadline. ActivityBumpWorkspace already covers this case
+			// when extending the deadline.
+			//
+			// Situation this is solving.
+			// 1. User has workspace with auto-start at 9:00am, 12 hour auto-stop.
+			// 2. Coder stops workspace at 9pm
+			// 3. User starts workspace at 9:45pm.
+			//	- The initial deadline is calculated to be 9:45am
+			//	- This crosses the autostart deadline, so the deadline is extended to 9pm
+			nextAutostart, ok := NextAutostartSchedule(params.Now, params.WorkspaceAutostart, templateSchedule)
+			if ok && autostop.Deadline.After(nextAutostart) {
+				autostop.Deadline = nextAutostart.Add(ttl)
 autostop.Deadline = autostop.MaxDeadline 
 autostop.Deadline = autostop.MaxDeadline 
+			}
 		}
 	}
 

diff --git a/coderd/schedule/autostop_test.go b/coderd/schedule/autostop_test.go
@@ -25,6 +25,12 @@ func TestCalculateAutoStop(t *testing.T) {
 
 	now := time.Now()
 
+	chicago, err := time.LoadLocation("America/Chicago")
+	require.NoError(t, err, "loading chicago time location")
+
+	// pastDateNight is 9:45pm on a wednesday
+	pastDateNight := time.Date(2024, 2, 14, 21, 45, 0, 0, chicago)
+
 	// Wednesday the 8th of February 2023 at midnight. This date was
 	// specifically chosen as it doesn't fall on a applicable week for both
 	// fortnightly and triweekly autostop requirements.
@@ -70,8 +76,12 @@ func TestCalculateAutoStop(t *testing.T) {
 	t.Log("saturdayMidnightAfterDstOut", saturdayMidnightAfterDstOut)
 
 	cases := []struct {
-		name                        string
-		now                         time.Time
+		name string
+		now  time.Time
+
+		wsAutostart       string
+		templateAutoStart schedule.TemplateAutostartRequirement
+
 		templateAllowAutostop       bool
 		templateDefaultTTL          time.Duration
 		templateAutostopRequirement schedule.TemplateAutostopRequirement
@@ -364,6 +374,33 @@ func TestCalculateAutoStop(t *testing.T) {
 			// expectedDeadline is copied from expectedMaxDeadline.
 			expectedMaxDeadline: dstOutQuietHoursExpectedTime,
 		},
+		{
+			// A user expects this workspace to be online from 9am -> 9pm.
+			// So if a deadline is going to land in the middle of this range,
+			// we should bump it to the end.
+			// This is already done on `ActivityBumpWorkspace`, but that requires
+			// activity on the workspace.
+			name: "AutostopCrossAutostartBorder",
+			// Starting at 9:45pm, with the autostart at 9am.
+			now:                   pastDateNight,
+			templateAllowAutostop: false,
+			templateDefaultTTL:    time.Hour * 12,
+			workspaceTTL:          time.Hour * 12,
+			// At 9am every morning
+			wsAutostart: "CRON_TZ=America/Chicago 0 9 * * *",
+
+			// No quiet hours
+			templateAutoStart: schedule.TemplateAutostartRequirement{
+				// Just allow all days of the week
+				DaysOfWeek: 0b01111111,
+			},
+			templateAutostopRequirement: schedule.TemplateAutostopRequirement{},
+			userQuietHoursSchedule:      "",
+
+			expectedDeadline:    time.Date(pastDateNight.Year(), pastDateNight.Month(), pastDateNight.Day()+1, 21, 0, 0, 0, chicago),
+			expectedMaxDeadline: time.Time{},
+			errContains:         "",
+		},
 	}
 
 	for _, c := range cases {
@@ -382,6 +419,7 @@ func TestCalculateAutoStop(t *testing.T) {
 						UserAutostopEnabled:  c.templateAllowAutostop,
 						DefaultTTL:           c.templateDefaultTTL,
 						AutostopRequirement:  c.templateAutostopRequirement,
+						AutostartRequirement: c.templateAutoStart,
 					}, nil
 				},
 			}
@@ -433,11 +471,20 @@ func TestCalculateAutoStop(t *testing.T) {
 					Valid: true,
 				}
 			}
+
+			autostart := sql.NullString{}
+			if c.wsAutostart != "" {
+				autostart = sql.NullString{
+					String: c.wsAutostart,
+					Valid:  true,
+				}
+			}
 			workspace := dbgen.Workspace(t, db, database.Workspace{
-				TemplateID:     template.ID,
-				OrganizationID: org.ID,
-				OwnerID:        user.ID,
-				Ttl:            workspaceTTL,
+				TemplateID:        template.ID,
+				OrganizationID:    org.ID,
+				OwnerID:           user.ID,
+				Ttl:               workspaceTTL,
+				AutostartSchedule: autostart,
 			})
 
 			autostop, err := schedule.CalculateAutostop(ctx, schedule.CalculateAutostopParams{
@@ -446,6 +493,7 @@ func TestCalculateAutoStop(t *testing.T) {
 				UserQuietHoursScheduleStore: userQuietHoursScheduleStore,
 				Now:                         c.now,
 				Workspace:                   workspace,
+				WorkspaceAutostart:          c.wsAutostart,
 			})
 			if c.errContains != "" {
 				require.Error(t, err)

diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go
@@ -27,7 +27,6 @@ import (
 	"cdr.dev/slog"
 	agentproto "github.com/coder/coder/v2/agent/proto"
 	"github.com/coder/coder/v2/coderd/agentapi"
-	"github.com/coder/coder/v2/coderd/autobuild"
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/database/db2sdk"
 	"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -37,6 +36,7 @@ import (
 	"github.com/coder/coder/v2/coderd/httpmw"
 	"github.com/coder/coder/v2/coderd/prometheusmetrics"
 	"github.com/coder/coder/v2/coderd/rbac"
+	"github.com/coder/coder/v2/coderd/schedule"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/codersdk/agentsdk"
 	"github.com/coder/coder/v2/codersdk/workspacesdk"
@@ -1186,7 +1186,7 @@ func (api *API) workspaceAgentReportStats(rw http.ResponseWriter, r *http.Reques
 					slog.Error(err),
 				)
 			} else {
-				next, allowed := autobuild.NextAutostartSchedule(time.Now(), workspace.AutostartSchedule.String, templateSchedule)
+				next, allowed := schedule.NextAutostartSchedule(time.Now(), workspace.AutostartSchedule.String, templateSchedule)
 				if allowed {
 					nextAutostart = next
 				}