From b7049032a0e05b552015848a897ec752871831b0 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 16 May 2022 17:02:44 +0100 Subject: [PATCH] fix: cli: prettify schedule when printing output (#1440) * 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 --- cli/autostart.go | 10 ++++- cli/autostart_test.go | 3 +- cli/autostop.go | 10 ++++- cli/autostop_test.go | 3 +- cli/list.go | 13 +++++-- coderd/autobuild/schedule/schedule.go | 44 ++++++++++++++++++---- coderd/autobuild/schedule/schedule_test.go | 10 ++++- 7 files changed, 77 insertions(+), 16 deletions(-) diff --git a/cli/autostart.go b/cli/autostart.go index 1e29b63982..870eafa94c 100644 --- 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 index 2f65bbc15d..78f11a5380 100644 --- 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 index e9520e9df0..5805b4516e 100644 --- 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 index dad4f8d4ee..2d5205ad9c 100644 --- 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 index d32c66ee87..6fb2a369d6 100644 --- 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 index e98d513bc0..7fbef2f21f 100644 --- 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 index 1109ced96c..0dd9884fb3 100644 --- a/coderd/autobuild/schedule/schedule_test.go +++ b/coderd/autobuild/schedule/schedule_test.go @@ -17,6 +17,8 @@ func Test_Weekly(t *testing.T) { at time.Time expectedNext time.Time expectedError string + expectedCron string + expectedTz string }{ { name: "with timezone", @@ -24,13 +26,17 @@ func Test_Weekly(t *testing.T) { 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)