fix: cli: prettify schedule when printing output (#1440)

johnstcn · web-flow · commit b7049032a0e0 · 2022-05-16T17:02:44.000+01:00
* Adds methods to schedule.Schedule to show the raw cron string and timezone
* Uses these methods to clean up output of auto(start|stop) show or ls
* Defaults CRON_TZ=UTC if not provided
diff --git a/cli/autostart.go b/cli/autostart.go
@@ -63,7 +63,15 @@ func autostartShow() *cobra.Command {
 				return nil
 			}
 
-			_, _ = fmt.Fprintf(cmd.OutOrStdout(), "schedule: %s\nnext: %s\n", workspace.AutostartSchedule, validSchedule.Next(time.Now()))
+			next := validSchedule.Next(time.Now())
+			loc, _ := time.LoadLocation(validSchedule.Timezone())
+
+			_, _ = fmt.Fprintf(cmd.OutOrStdout(),
+				"schedule: %s\ntimezone: %s\nnext:     %s\n",
+				validSchedule.Cron(),
+				validSchedule.Timezone(),
+				next.In(loc),
+			)
 
 			return nil
 		},
diff --git a/cli/autostart_test.go b/cli/autostart_test.go
@@ -45,7 +45,8 @@ func TestAutostart(t *testing.T) {
 
 		err = cmd.Execute()
 		require.NoError(t, err, "unexpected error")
-		require.Contains(t, stdoutBuf.String(), "schedule: "+sched)
+		// CRON_TZ gets stripped
+		require.Contains(t, stdoutBuf.String(), "schedule: 30 17 * * 1-5")
 	})
 
 	t.Run("EnableDisableOK", func(t *testing.T) {
diff --git a/cli/autostop.go b/cli/autostop.go
@@ -63,7 +63,15 @@ func autostopShow() *cobra.Command {
 				return nil
 			}
 
-			_, _ = fmt.Fprintf(cmd.OutOrStdout(), "schedule: %s\nnext: %s\n", workspace.AutostopSchedule, validSchedule.Next(time.Now()))
+			next := validSchedule.Next(time.Now())
+			loc, _ := time.LoadLocation(validSchedule.Timezone())
+
+			_, _ = fmt.Fprintf(cmd.OutOrStdout(),
+				"schedule: %s\ntimezone: %s\nnext:     %s\n",
+				validSchedule.Cron(),
+				validSchedule.Timezone(),
+				next.In(loc),
+			)
 
 			return nil
 		},
diff --git a/cli/autostop_test.go b/cli/autostop_test.go
@@ -45,7 +45,8 @@ func TestAutostop(t *testing.T) {
 
 		err = cmd.Execute()
 		require.NoError(t, err, "unexpected error")
-		require.Contains(t, stdoutBuf.String(), "schedule: "+sched)
+		// CRON_TZ gets stripped
+		require.Contains(t, stdoutBuf.String(), "schedule: 30 17 * * 1-5")
 	})
 
 	t.Run("EnableDisableOK", func(t *testing.T) {
diff --git a/cli/list.go b/cli/list.go
@@ -10,6 +10,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/coder/coder/cli/cliui"
+	"github.com/coder/coder/coderd/autobuild/schedule"
 	"github.com/coder/coder/coderd/database"
 	"github.com/coder/coder/codersdk"
 )
@@ -108,14 +109,18 @@ func list() *cobra.Command {
 					durationDisplay = durationDisplay[:len(durationDisplay)-2]
 				}
 
-				autostartDisplay := "not enabled"
+				autostartDisplay := "-"
 				if workspace.AutostartSchedule != "" {
-					autostartDisplay = workspace.AutostartSchedule
+					if sched, err := schedule.Weekly(workspace.AutostartSchedule); err == nil {
+						autostartDisplay = sched.Cron()
+					}
 				}
 
-				autostopDisplay := "not enabled"
+				autostopDisplay := "-"
 				if workspace.AutostopSchedule != "" {
-					autostopDisplay = workspace.AutostopSchedule
+					if sched, err := schedule.Weekly(workspace.AutostopSchedule); err == nil {
+						autostopDisplay = sched.Cron()
+					}
 				}
 
 				user := usersByID[workspace.OwnerID]
diff --git a/coderd/autobuild/schedule/schedule.go b/coderd/autobuild/schedule/schedule.go
@@ -34,12 +34,18 @@ var defaultParser = cron.NewParser(parserFormat)
 //  us_sched, _ := schedule.Weekly("CRON_TZ=US/Central 30 9 1-5")
 //  fmt.Println(sched.Next(time.Now()).Format(time.RFC3339))
 //  // Output: 2022-04-04T14:30:00Z
-func Weekly(spec string) (*Schedule, error) {
-	if err := validateWeeklySpec(spec); err != nil {
+func Weekly(raw string) (*Schedule, error) {
+	if err := validateWeeklySpec(raw); err != nil {
 		return nil, xerrors.Errorf("validate weekly schedule: %w", err)
 	}
 
-	specSched, err := defaultParser.Parse(spec)
+	// If schedule does not specify a timezone, default to UTC. Otherwise,
+	// the library will default to time.Local which we want to avoid.
+	if !strings.HasPrefix(raw, "CRON_TZ=") {
+		raw = "CRON_TZ=UTC " + raw
+	}
+
+	specSched, err := defaultParser.Parse(raw)
 	if err != nil {
 		return nil, xerrors.Errorf("parse schedule: %w", err)
 	}
@@ -49,9 +55,16 @@ func Weekly(spec string) (*Schedule, error) {
 		return nil, xerrors.Errorf("expected *cron.SpecSchedule but got %T", specSched)
 	}
 
+	// Strip the leading CRON_TZ prefix so we just store the cron string.
+	// The timezone info is available in SpecSchedule.
+	cronStr := raw
+	if strings.HasPrefix(raw, "CRON_TZ=") {
+		cronStr = strings.Join(strings.Fields(raw)[1:], " ")
+	}
+
 	cronSched := &Schedule{
-		sched: schedule,
-		spec:  spec,
+		sched:   schedule,
+		cronStr: cronStr,
 	}
 	return cronSched, nil
 }
@@ -61,12 +74,29 @@ func Weekly(spec string) (*Schedule, error) {
 type Schedule struct {
 	sched *cron.SpecSchedule
 	// XXX: there isn't any nice way for robfig/cron to serialize
-	spec string
+	cronStr string
 }
 
 // String serializes the schedule to its original human-friendly format.
+// The leading CRON_TZ is maintained.
 func (s Schedule) String() string {
-	return s.spec
+	var sb strings.Builder
+	_, _ = sb.WriteString("CRON_TZ=")
+	_, _ = sb.WriteString(s.sched.Location.String())
+	_, _ = sb.WriteString(" ")
+	_, _ = sb.WriteString(s.cronStr)
+	return sb.String()
+}
+
+// Timezone returns the timezone for the schedule.
+func (s Schedule) Timezone() string {
+	return s.sched.Location.String()
+}
+
+// Cron returns the cron spec for the schedule with the leading CRON_TZ
+// stripped, if present.
+func (s Schedule) Cron() string {
+	return s.cronStr
 }
 
 // Next returns the next time in the schedule relative to t.
diff --git a/coderd/autobuild/schedule/schedule_test.go b/coderd/autobuild/schedule/schedule_test.go
@@ -17,20 +17,26 @@ func Test_Weekly(t *testing.T) {
 		at            time.Time
 		expectedNext  time.Time
 		expectedError string
+		expectedCron  string
+		expectedTz    string
 	}{
 		{
 			name:          "with timezone",
 			spec:          "CRON_TZ=US/Central 30 9 * * 1-5",
 			at:            time.Date(2022, 4, 1, 14, 29, 0, 0, time.UTC),
 			expectedNext:  time.Date(2022, 4, 1, 14, 30, 0, 0, time.UTC),
 			expectedError: "",
+			expectedCron:  "30 9 * * 1-5",
+			expectedTz:    "US/Central",
 		},
 		{
 			name:          "without timezone",
-			spec:          "30 9 * * 1-5",
+			spec:          "CRON_TZ=UTC 30 9 * * 1-5",
 			at:            time.Date(2022, 4, 1, 9, 29, 0, 0, time.Local),
 			expectedNext:  time.Date(2022, 4, 1, 9, 30, 0, 0, time.Local),
 			expectedError: "",
+			expectedCron:  "30 9 * * 1-5",
+			expectedTz:    "UTC",
 		},
 		{
 			name:          "invalid schedule",
@@ -86,6 +92,8 @@ func Test_Weekly(t *testing.T) {
 				require.NoError(t, err)
 				require.Equal(t, testCase.expectedNext, nextTime)
 				require.Equal(t, testCase.spec, actual.String())
+				require.Equal(t, testCase.expectedCron, actual.Cron())
+				require.Equal(t, testCase.expectedTz, actual.Timezone())
 			} else {
 				require.EqualError(t, err, testCase.expectedError)
 				require.Nil(t, actual)
