test: next start at is only a valid value

coder · DanielleMaywood · Dec 2, 2024 · Nov 18, 2024 · Nov 18, 2024 · Nov 19, 2024
commit dd05558179e6d1cb3ed6195f21c945fbd65300d0
diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -6954,6 +6954,10 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no
 			job.JobStatus != database.ProvisionerJobStatusFailed &&
 			build.Transition == database.WorkspaceTransitionStop &&
 			workspace.AutostartSchedule.Valid &&
+			// We do not know if workspace with a zero next start is eligible
+			// for autostart, so we accept this false-positive. This can occur
+			// when a coder version is upgraded and next_start_at has yet to
+			// be set.
 			(workspace.NextStartAt.Time.IsZero() ||
 				!now.Before(workspace.NextStartAt.Time)) {
 			workspaces = append(workspaces, database.GetWorkspacesEligibleForTransitionRow{

diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql
@@ -624,7 +624,7 @@ WHERE
 				-- this happens we leave it up to the Coder server to figure out if
 				-- the workspace is ready to autostart.
 				workspaces.next_start_at IS NULL OR
-				workspaces.next_start_at <= @now :: timestamp
+				workspaces.next_start_at <= @now :: timestamptz
 			)
 		) OR
 

diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go
@@ -1453,14 +1453,15 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob)
 					return xerrors.Errorf("get template schedule options: %w", err)
 				}
 
-				nextStartAt, _ := schedule.NextAutostart(now, workspace.AutostartSchedule.String, templateScheduleOptions)
-
-				err = db.UpdateWorkspaceNextStartAt(ctx, database.UpdateWorkspaceNextStartAtParams{
-					ID:          workspace.ID,
-					NextStartAt: sql.NullTime{Valid: true, Time: nextStartAt.UTC()},
-				})
-				if err != nil {
-					return xerrors.Errorf("update workspace next start at: %w", err)
+				nextStartAt, err := schedule.NextAllowedAutostart(now, workspace.AutostartSchedule.String, templateScheduleOptions)
+				if err == nil {
+					err = db.UpdateWorkspaceNextStartAt(ctx, database.UpdateWorkspaceNextStartAtParams{
+						ID:          workspace.ID,
+						NextStartAt: sql.NullTime{Valid: true, Time: nextStartAt.UTC()},
+					})
+					if err != nil {
+						return xerrors.Errorf("update workspace next start at: %w", err)
+					}
 				}
 			}
 

diff --git a/coderd/schedule/autostart.go b/coderd/schedule/autostart.go
@@ -3,9 +3,13 @@ package schedule
 import (
 	"time"
 
+	"golang.org/x/xerrors"
+
 	"github.com/coder/coder/v2/coderd/schedule/cron"
 )
 
+var ErrNoAllowedAutostart = xerrors.New("no allowed autostart")
+
 // NextAutostart 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.
@@ -28,3 +32,17 @@ func NextAutostart(at time.Time, wsSchedule string, templateSchedule TemplateSch
 
 	return zonedTransition, allowed
 }
+
+func NextAllowedAutostart(at time.Time, wsSchedule string, templateSchedule TemplateScheduleOptions) (time.Time, error) {
+	const daysInWeek = 7
+
+	for range daysInWeek {
+		next, valid := NextAutostart(at, wsSchedule, templateSchedule)
+		if valid {
+			return next, nil
+		}
+		at = next
+	}
+
+	return time.Time{}, ErrNoAllowedAutostart
+}
diff --git a/coderd/schedule/autostart_test.go b/coderd/schedule/autostart_test.go
@@ -0,0 +1,38 @@
+package schedule_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/coder/coder/v2/coderd/schedule"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNextAllowedAutostart(t *testing.T) {
+	t.Parallel()
+
+	t.Run("WhenScheduleOutOfSync", func(t *testing.T) {
+		// 1st January 2024 is a Monday
+		at := time.Date(2024, time.January, 1, 10, 0, 0, 0, time.UTC)
+		//  Monday-Friday 9:00AM UTC
+		sched := "CRON_TZ=UTC 00 09 * * 1-5"
+		// Only allow an autostart on mondays
+		opts := schedule.TemplateScheduleOptions{
+			AutostartRequirement: schedule.TemplateAutostartRequirement{
+				DaysOfWeek: 0b00000001,
+			},
+		}
+
+		// NextAutostart wil, return a non-allowed autostart time as
+		// our AutostartRequirement only allows Mondays but we expect
+		// this to return a Tuesday.
+		next, allowed := schedule.NextAutostart(at, sched, opts)
+		require.False(t, allowed)
+		require.Equal(t, time.Date(2024, time.January, 2, 9, 0, 0, 0, time.UTC), next)
+
+		// NextAllowedAutostart should return the next allowed autostart time.
+		next, err := schedule.NextAllowedAutostart(at, sched, opts)
+		require.NoError(t, err)
+		require.Equal(t, time.Date(2024, time.January, 8, 9, 0, 0, 0, time.UTC), next)
+	})
+}
diff --git a/coderd/workspaces.go b/coderd/workspaces.go
@@ -557,8 +557,10 @@ func createWorkspace(
 
 	nextStartAt := sql.NullTime{}
 	if dbAutostartSchedule.Valid {
-		next, _ := schedule.NextAutostart(dbtime.Now(), dbAutostartSchedule.String, templateSchedule)
-		nextStartAt = sql.NullTime{Valid: true, Time: next.UTC()}
+		next, err := schedule.NextAllowedAutostart(dbtime.Now(), dbAutostartSchedule.String, templateSchedule)
+		if err == nil {
+			nextStartAt = sql.NullTime{Valid: true, Time: next.UTC()}
+		}
 	}
 
 	dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis, templateSchedule.DefaultTTL)
@@ -883,8 +885,10 @@ func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) {
 
 	nextStartAt := sql.NullTime{}
 	if dbSched.Valid {
-		next, _ := schedule.NextAutostart(dbtime.Now(), dbSched.String, templateSchedule)
-		nextStartAt = sql.NullTime{Valid: true, Time: next.UTC()}
+		next, err := schedule.NextAllowedAutostart(dbtime.Now(), dbSched.String, templateSchedule)
+		if err == nil {
+			nextStartAt = sql.NullTime{Valid: true, Time: next.UTC()}
+		}
 	}
 
 	err = api.Database.UpdateWorkspaceAutostart(ctx, database.UpdateWorkspaceAutostartParams{

diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"cdr.dev/slog"
@@ -1102,6 +1103,80 @@ func TestWorkspaceAutobuild(t *testing.T) {
 		ws = coderdtest.MustWorkspace(t, client, ws.ID)
 		require.Equal(t, version2.ID, ws.LatestBuild.TemplateVersionID)
 	})
+
+	t.Run("NextStartAtIsValid", func(t *testing.T) {
+		t.Parallel()
+
+		var (
+			// ctx = testutil.Context(t, testutil.WaitMedium)
+			tickCh  = make(chan time.Time)
+			statsCh = make(chan autobuild.Stats)
+		)
+
+		logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
+		client, user := coderdenttest.New(t, &coderdenttest.Options{
+			Options: &coderdtest.Options{
+				AutobuildTicker:          tickCh,
+				IncludeProvisionerDaemon: true,
+				AutobuildStats:           statsCh,
+				TemplateScheduleStore:    schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger),
+			},
+			LicenseOptions: &coderdenttest.LicenseOptions{
+				Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1},
+			},
+		})
+
+		version1 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		coderdtest.AwaitTemplateVersionJobCompleted(t, client, version1.ID)
+
+		// First create a template that only supports Monday-Friday
+		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version1.ID, func(ctr *codersdk.CreateTemplateRequest) {
+			ctr.AutostartRequirement = &codersdk.TemplateAutostartRequirement{DaysOfWeek: codersdk.BitmapToWeekdays(0b00011111)}
+		})
+		require.Equal(t, version1.ID, template.ActiveVersionID)
+
+		// Then create a workspace with a schedule Sunday-Saturday
+		sched, err := cron.Weekly("CRON_TZ=UTC 0 9 * * 0-6")
+		require.NoError(t, err)
+		ws := coderdtest.CreateWorkspace(t, client, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
+			cwr.AutostartSchedule = ptr.Ref(sched.String())
+		})
+
+		coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID)
+		ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
+
+		// For each day of the week
+		for range 7 {
+			next := sched.Next(ws.LatestBuild.CreatedAt)
+
+			go func() {
+				tickCh <- next
+			}()
+
+			stats := <-statsCh
+
+			// Our cron schedule specifies Sunday-Saturday but the template only allows
+			// Monday-Friday so we expect there to be no transitions on the weekend.
+			if next.Weekday() == time.Saturday || next.Weekday() == time.Sunday {
+				assert.Len(t, stats.Errors, 0)
+				assert.Len(t, stats.Transitions, 0)
+
+				ws = coderdtest.MustWorkspace(t, client, ws.ID)
+			} else {
+				assert.Len(t, stats.Errors, 0)
+				assert.Len(t, stats.Transitions, 1)
+				assert.Contains(t, stats.Transitions, ws.ID)
+				assert.Equal(t, database.WorkspaceTransitionStart, stats.Transitions[ws.ID])
+
+				ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
+			}
+
+			// Verify that the workspaces next_start_at is a valid autostart day
+			require.NotNil(t, ws.NextStartAt)
+			require.NotEqual(t, time.Saturday, ws.NextStartAt.Weekday())
+			require.NotEqual(t, time.Sunday, ws.NextStartAt.Weekday())
+		}
+	})
 }
 
 func TestTemplateDoesNotAllowUserAutostop(t *testing.T) {