steven comments and test fix

coder · deansheather · Jul 20, 2023 · Jun 20, 2023 · Jun 20, 2023 · Jun 21, 2023
commit cd7713871b98fd8e0b9b4898181b355c8fd425c9
diff --git a/cli/templateedit.go b/cli/templateedit.go
@@ -3,6 +3,7 @@ package cli
 import (
 	"fmt"
 	"net/http"
+	"strings"
 	"time"
 
 	"golang.org/x/xerrors"
@@ -90,12 +91,6 @@ func (r *RootCmd) templateEdit() *clibase.Cmd {
 				restartRequirementDaysOfWeek = []string{}
 			}
 
-			// Check that the user didn't specify a value that is not allowed.
-			_, err = codersdk.WeekdaysToBitmap(restartRequirementDaysOfWeek)
-			if err != nil {
-				return xerrors.Errorf("invalid restart requirement days of week: %w", err)
-			}
-
 			// NOTE: coderd will ignore empty fields.
 			req := codersdk.UpdateTemplateMeta{
 				Name:             name,
@@ -160,7 +155,17 @@ func (r *RootCmd) templateEdit() *clibase.Cmd {
 			Description: "Edit the template restart requirement weekdays - workspaces created from this template must be restarted on the given weekdays. To unset this value for the template (and disable the restart requirement for the template), pass 'none'.",
 			// TODO(@dean): unhide when we delete max_ttl
 			Hidden: true,
-			Value:  clibase.StringArrayOf(&restartRequirementDaysOfWeek),
+			Value: clibase.Validate(clibase.StringArrayOf(&restartRequirementDaysOfWeek), func(value *clibase.StringArray) error {
+				v := value.GetSlice()
+				if len(v) == 1 && v[0] == "none" {
+					return nil
+				}
+				_, err := codersdk.WeekdaysToBitmap(v)
+				if err != nil {
+					return xerrors.Errorf("invalid restart requirement days of week %q: %w", strings.Join(v, ","), err)
+				}
+				return nil
+			}),
 		},
 		{
 			Flag:        "restart-requirement-weeks",

@@ -13,12 +13,27 @@ const (
 	// restartRequirementLeeway is the duration of time before a restart
 	// requirement where we skip the requirement and fall back to the next
 	// scheduled restart. This avoids workspaces being restarted too soon.
+	//
+	// E.g. If the workspace is started within an hour of the quiet hours, we
+	//      will skip the restart requirement and use the next scheduled restart
+	//      requirement.
 	restartRequirementLeeway = 1 * time.Hour
 
 	// restartRequirementBuffer is the duration of time we subtract from the
 	// time when calculating the next scheduled restart time. This avoids issues
 	// where autostart happens on the hour and the scheduled quiet hours are
 	// also on the hour.
+	//
+	// E.g. If the workspace is started at 12am (perhaps due to scheduled
+	//      autostart) and the quiet hours is also 12am, the workspace will skip
+	//      the day it's supposed to stop and use the next day instead. This is
+	//      because getting the next cron schedule time will never include the
+	//      time fed to the calculation (i.e. it's not inclusive). This happens
+	//      because we always check for the next cron time by rounding down to
+	//      midnight.
+	//
+	//      This resolves that problem by subtracting 15 minutes from midnight
+	//      when we check the next cron time.
 	restartRequirementBuffer = -15 * time.Minute
 )
 

diff --git a/coderd/schedule/autostop_test.go b/coderd/schedule/autostop_test.go
@@ -539,11 +539,21 @@ func TestFindWeek(t *testing.T) {
 			currentWeek, err := schedule.WeeksSinceEpoch(now)
 			require.NoError(t, err)
 
-			currentWeekMondayExpected := now.AddDate(0, 0, -int(now.Weekday())+1)
+			diffMonday := now.Weekday() - time.Monday
+			if now.Weekday() == time.Sunday {
+				// Sunday is 0, but Monday is the first day of the week in the
+				// code.
+				diffMonday = 6
+			}
+			currentWeekMondayExpected := now.AddDate(0, 0, -int(diffMonday))
+			require.Equal(t, time.Monday, currentWeekMondayExpected.Weekday())
 			y, m, d := currentWeekMondayExpected.Date()
+			// Change to midnight.
 			currentWeekMondayExpected = time.Date(y, m, d, 0, 0, 0, 0, loc)
+
 			currentWeekMonday, err := schedule.GetMondayOfWeek(now.Location(), currentWeek)
 			require.NoError(t, err)
+			require.Equal(t, time.Monday, currentWeekMonday.Weekday())
 			require.Equal(t, currentWeekMondayExpected, currentWeekMonday)
 
 			t.Log("now", now)