cli: streamline autostart ux (coder#2251)

This commit adds the following changes:

- autostart enable|disable => autostart set|unset
- autostart enable now accepts a more natual schedule format: <time> <days-of-week> <location>
- autostart show now shows configured timezone
- 🎉 automatic timezone detection across mac, windows, linux 🎉

Fixes coder#1647

Author: johnstcn

cli/autostart.go

@@ -2,32 +2,41 @@ package cli
 
 import (
 	"fmt"
-	"os"
+	"strings"
 	"time"
 
 	"github.com/spf13/cobra"
+	"golang.org/x/xerrors"
 
 	"github.com/coder/coder/coderd/autobuild/schedule"
+	"github.com/coder/coder/coderd/util/ptr"
+	"github.com/coder/coder/coderd/util/tz"
 	"github.com/coder/coder/codersdk"
 )
 
 const autostartDescriptionLong = `To have your workspace build automatically at a regular time you can enable autostart.
-When enabling autostart, provide the minute, hour, and day(s) of week.
-The default schedule is at 09:00 in your local timezone (TZ env, UTC by default).
+When enabling autostart, enter a schedule in the format: <start-time> [day-of-week] [location].
+  * Start-time (required) is accepted either in 12-hour (hh:mm{am|pm}) format, or 24-hour format hh:mm.
+  * Day-of-week (optional) allows specifying in the cron format, e.g. 1,3,5 or Mon-Fri.
+    Aliases such as @daily are not supported.
+    Default: * (every day)
+  * Location (optional) must be a valid location in the IANA timezone database.
+    If omitted, we will fall back to either the TZ environment variable or /etc/localtime.
+    You can check your corresponding location by visiting https://ipinfo.io - it shows in the demo widget on the right.
 `
 
 func autostart() *cobra.Command {
 	autostartCmd := &cobra.Command{
 		Annotations: workspaceCommand,
-		Use:         "autostart enable <workspace>",
+		Use:         "autostart set <workspace> <start-time> [day-of-week] [location]",
 		Short:       "schedule a workspace to automatically start at a regular time",
 		Long:        autostartDescriptionLong,
-		Example:     "coder autostart enable my-workspace --minute 30 --hour 9 --days 1-5 --tz Europe/Dublin",
+		Example:     "coder autostart set my-workspace 9:30AM Mon-Fri Europe/Dublin",
 	}
 
 	autostartCmd.AddCommand(autostartShow())
-	autostartCmd.AddCommand(autostartEnable())
-	autostartCmd.AddCommand(autostartDisable())
+	autostartCmd.AddCommand(autostartSet())
+	autostartCmd.AddCommand(autostartUnset())
 
 	return autostartCmd
 }
@@ -60,13 +69,12 @@ func autostartShow() *cobra.Command {
 			}
 
 			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),
+				validSchedule.Location(),
+				next.In(validSchedule.Location()),
 			)
 
 			return nil
@@ -75,23 +83,17 @@ func autostartShow() *cobra.Command {
 	return cmd
 }
 
-func autostartEnable() *cobra.Command {
-	// yes some of these are technically numbers but the cron library will do that work
-	var autostartMinute string
-	var autostartHour string
-	var autostartDayOfWeek string
-	var autostartTimezone string
+func autostartSet() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:  "enable <workspace_name> <schedule>",
-		Args: cobra.ExactArgs(1),
+		Use:  "set <workspace_name> <start-time> [day-of-week] [location]",
+		Args: cobra.RangeArgs(2, 4),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			client, err := createClient(cmd)
 			if err != nil {
 				return err
 			}
 
-			spec := fmt.Sprintf("CRON_TZ=%s %s %s * * %s", autostartTimezone, autostartMinute, autostartHour, autostartDayOfWeek)
-			validSchedule, err := schedule.Weekly(spec)
+			sched, err := parseCLISchedule(args[1:]...)
 			if err != nil {
 				return err
 			}
@@ -102,32 +104,30 @@ func autostartEnable() *cobra.Command {
 			}
 
 			err = client.UpdateWorkspaceAutostart(cmd.Context(), workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
-				Schedule: &spec,
+				Schedule: ptr.Ref(sched.String()),
 			})
 			if err != nil {
 				return err
 			}
 
-			_, _ = fmt.Fprintf(cmd.OutOrStdout(), "\nThe %s workspace will automatically start at %s.\n\n", workspace.Name, validSchedule.Next(time.Now()))
-
+			schedNext := sched.Next(time.Now())
+			_, _ = fmt.Fprintf(cmd.OutOrStdout(),
+				"%s will automatically start at %s %s (%s)\n",
+				workspace.Name,
+				schedNext.In(sched.Location()).Format(time.Kitchen),
+				sched.DaysOfWeek(),
+				sched.Location().String(),
+			)
 			return nil
 		},
 	}
 
-	cmd.Flags().StringVar(&autostartMinute, "minute", "0", "autostart minute")
-	cmd.Flags().StringVar(&autostartHour, "hour", "9", "autostart hour")
-	cmd.Flags().StringVar(&autostartDayOfWeek, "days", "1-5", "autostart day(s) of week")
-	tzEnv := os.Getenv("TZ")
-	if tzEnv == "" {
-		tzEnv = "UTC"
-	}
-	cmd.Flags().StringVar(&autostartTimezone, "tz", tzEnv, "autostart timezone")
 	return cmd
 }
 
-func autostartDisable() *cobra.Command {
+func autostartUnset() *cobra.Command {
 	return &cobra.Command{
-		Use:  "disable <workspace_name>",
+		Use:  "unset <workspace_name>",
 		Args: cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			client, err := createClient(cmd)
@@ -147,9 +147,98 @@ func autostartDisable() *cobra.Command {
 				return err
 			}
 
-			_, _ = fmt.Fprintf(cmd.OutOrStdout(), "\nThe %s workspace will no longer automatically start.\n\n", workspace.Name)
+			_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s will no longer automatically start.\n", workspace.Name)
 
 			return nil
 		},
 	}
 }
+
+var errInvalidScheduleFormat = xerrors.New("Schedule must be in the format Mon-Fri 09:00AM America/Chicago")
+var errInvalidTimeFormat = xerrors.New("Start time must be in the format hh:mm[am|pm] or HH:MM")
+var errUnsupportedTimezone = xerrors.New("The location you provided looks like a timezone. Check https://ipinfo.io for your location.")
+
+// parseCLISchedule parses a schedule in the format HH:MM{AM|PM} [DOW] [LOCATION]
+func parseCLISchedule(parts ...string) (*schedule.Schedule, error) {
+	// If the user was careful and quoted the schedule, un-quote it.
+	// In the case that only time was specified, this will be a no-op.
+	if len(parts) == 1 {
+		parts = strings.Fields(parts[0])
+	}
+	var loc *time.Location
+	dayOfWeek := "*"
+	t, err := parseTime(parts[0])
+	if err != nil {
+		return nil, err
+	}
+	hour, minute := t.Hour(), t.Minute()
+
+	// Any additional parts get ignored.
+	switch len(parts) {
+	case 3:
+		dayOfWeek = parts[1]
+		loc, err = time.LoadLocation(parts[2])
+		if err != nil {
+			_, err = time.Parse("MST", parts[2])
+			if err == nil {
+				return nil, errUnsupportedTimezone
+			}
+			return nil, xerrors.Errorf("Invalid timezone %q specified: a valid IANA timezone is required", parts[2])
+		}
+	case 2:
+		// Did they provide day-of-week or location?
+		if maybeLoc, err := time.LoadLocation(parts[1]); err != nil {
+			// Assume day-of-week.
+			dayOfWeek = parts[1]
+		} else {
+			loc = maybeLoc
+		}
+	case 1: // already handled
+	default:
+		return nil, errInvalidScheduleFormat
+	}
+
+	// If location was not specified, attempt to automatically determine it as a last resort.
+	if loc == nil {
+		loc, err = tz.TimezoneIANA()
+		if err != nil {
+			return nil, xerrors.Errorf("Could not automatically determine your timezone")
+		}
+	}
+
+	sched, err := schedule.Weekly(fmt.Sprintf(
+		"CRON_TZ=%s %d %d * * %s",
+		loc.String(),
+		minute,
+		hour,
+		dayOfWeek,
+	))
+	if err != nil {
+		// This will either be an invalid dayOfWeek or an invalid timezone.
+		return nil, xerrors.Errorf("Invalid schedule: %w", err)
+	}
+
+	return sched, nil
+}
+
+func parseTime(s string) (time.Time, error) {
+	// Try a number of possible layouts.
+	for _, layout := range []string{
+		time.Kitchen, // 03:04PM
+		"03:04pm",
+		"3:04PM",
+		"3:04pm",
+		"15:04",
+		"1504",
+		"03PM",
+		"03pm",
+		"3PM",
+		"3pm",
+	} {
+		t, err := time.Parse(layout, s)
+		if err == nil {
+			return t, nil
+		}
+	}
+	return time.Time{}, errInvalidTimeFormat
+}

cli/autostart_internal_test.go

@@ -0,0 +1,119 @@
+package cli
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+//nolint:paralleltest // t.Setenv
+func TestParseCLISchedule(t *testing.T) {
+	for _, testCase := range []struct {
+		name             string
+		input            []string
+		expectedSchedule string
+		expectedError    string
+		tzEnv            string
+	}{
+		{
+			name:             "TimeAndDayOfWeekAndLocation",
+			input:            []string{"09:00AM", "Sun-Sat", "America/Chicago"},
+			expectedSchedule: "CRON_TZ=America/Chicago 0 9 * * Sun-Sat",
+			tzEnv:            "UTC",
+		},
+		{
+			name:             "TimeOfDay24HourAndDayOfWeekAndLocation",
+			input:            []string{"09:00", "Sun-Sat", "America/Chicago"},
+			expectedSchedule: "CRON_TZ=America/Chicago 0 9 * * Sun-Sat",
+			tzEnv:            "UTC",
+		},
+		{
+			name:             "TimeOfDay24HourAndDayOfWeekAndLocationButItsAllQuoted",
+			input:            []string{"09:00 Sun-Sat America/Chicago"},
+			expectedSchedule: "CRON_TZ=America/Chicago 0 9 * * Sun-Sat",
+			tzEnv:            "UTC",
+		},
+		{
+			name:             "TimeOfDayOnly",
+			input:            []string{"09:00AM"},
+			expectedSchedule: "CRON_TZ=America/Chicago 0 9 * * *",
+			tzEnv:            "America/Chicago",
+		},
+		{
+			name:             "Time24Military",
+			input:            []string{"0900"},
+			expectedSchedule: "CRON_TZ=America/Chicago 0 9 * * *",
+			tzEnv:            "America/Chicago",
+		},
+		{
+			name:             "DayOfWeekAndTime",
+			input:            []string{"09:00AM", "Sun-Sat"},
+			expectedSchedule: "CRON_TZ=America/Chicago 0 9 * * Sun-Sat",
+			tzEnv:            "America/Chicago",
+		},
+		{
+			name:             "TimeAndLocation",
+			input:            []string{"09:00AM", "America/Chicago"},
+			expectedSchedule: "CRON_TZ=America/Chicago 0 9 * * *",
+			tzEnv:            "UTC",
+		},
+		{
+			name:             "LazyTime",
+			input:            []string{"9am", "America/Chicago"},
+			expectedSchedule: "CRON_TZ=America/Chicago 0 9 * * *",
+			tzEnv:            "UTC",
+		},
+		{
+			name:             "ZeroPrefixedLazyTime",
+			input:            []string{"09am", "America/Chicago"},
+			expectedSchedule: "CRON_TZ=America/Chicago 0 9 * * *",
+			tzEnv:            "UTC",
+		},
+		{
+			name:          "InvalidTime",
+			input:         []string{"nine"},
+			expectedError: errInvalidTimeFormat.Error(),
+		},
+		{
+			name:          "DayOfWeekAndInvalidTime",
+			input:         []string{"nine", "Sun-Sat"},
+			expectedError: errInvalidTimeFormat.Error(),
+		},
+		{
+			name:          "InvalidTimeAndLocation",
+			input:         []string{"nine", "America/Chicago"},
+			expectedError: errInvalidTimeFormat.Error(),
+		},
+		{
+			name:          "DayOfWeekAndInvalidTimeAndLocation",
+			input:         []string{"nine", "Sun-Sat", "America/Chicago"},
+			expectedError: errInvalidTimeFormat.Error(),
+		},
+		{
+			name:          "TimezoneProvidedInsteadOfLocation",
+			input:         []string{"09:00AM", "Sun-Sat", "CST"},
+			expectedError: errUnsupportedTimezone.Error(),
+		},
+		{
+			name:          "WhoKnows",
+			input:         []string{"Time", "is", "a", "human", "construct"},
+			expectedError: errInvalidTimeFormat.Error(),
+		},
+	} {
+		testCase := testCase
+		//nolint:paralleltest // t.Setenv
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Setenv("TZ", testCase.tzEnv)
+			actualSchedule, actualError := parseCLISchedule(testCase.input...)
+			if testCase.expectedError != "" {
+				assert.Nil(t, actualSchedule)
+				assert.ErrorContains(t, actualError, testCase.expectedError)
+				return
+			}
+			assert.NoError(t, actualError)
+			if assert.NotEmpty(t, actualSchedule) {
+				assert.Equal(t, testCase.expectedSchedule, actualSchedule.String())
+			}
+		})
+	}
+}