autostart/autostop: move to traditional 5-valued cron string for compatibility (#1049)

This PR modfies the original 3-valued cron strings used in package schedule to be traditional 5-valued cron strings.

- schedule.Weekly will validate that the month and dom fields are equal to *
- cli autostart/autostop will attempt to detect local timezone using TZ env var, defaulting to UTC
- cli autostart/autostop no longer accepts a raw schedule -- instead use the --minute, --hour, --dow, and --tz arguments.
- Default schedules are provided that should suffice for most users.

Fixes #993
This commit is contained in:
Cian Johnston
2022-04-18 11:04:48 -05:00
committed by GitHub
parent 3311c2f65d
commit af672803a2
11 changed files with 171 additions and 151 deletions
+27 -3
View File
@@ -3,6 +3,7 @@
package schedule
import (
"strings"
"time"
"github.com/robfig/cron/v3"
@@ -10,16 +11,19 @@ import (
)
// For the purposes of this library, we only need minute, hour, and
// day-of-week.
const parserFormatWeekly = cron.Minute | cron.Hour | cron.Dow
// day-of-week. However to ensure interoperability we will use the standard
// five-valued cron format. Descriptors are not supported.
const parserFormat = cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow
var defaultParser = cron.NewParser(parserFormatWeekly)
var defaultParser = cron.NewParser(parserFormat)
// Weekly parses a Schedule from spec scoped to a recurring weekly event.
// Spec consists of the following space-delimited fields, in the following order:
// - timezone e.g. CRON_TZ=US/Central (optional)
// - minutes of hour e.g. 30 (required)
// - hour of day e.g. 9 (required)
// - day of month (must be *)
// - month (must be *)
// - day of week e.g. 1 (required)
//
// Example Usage:
@@ -31,6 +35,10 @@ var defaultParser = cron.NewParser(parserFormatWeekly)
// 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 {
return nil, xerrors.Errorf("validate weekly schedule: %w", err)
}
specSched, err := defaultParser.Parse(spec)
if err != nil {
return nil, xerrors.Errorf("parse schedule: %w", err)
@@ -65,3 +73,19 @@ func (s Schedule) String() string {
func (s Schedule) Next(t time.Time) time.Time {
return s.sched.Next(t)
}
// validateWeeklySpec ensures that the day-of-month and month options of
// spec are both set to *
func validateWeeklySpec(spec string) error {
parts := strings.Fields(spec)
if len(parts) < 5 {
return xerrors.Errorf("expected schedule to consist of 5 fields with an optional CRON_TZ=<timezone> prefix")
}
if len(parts) == 6 {
parts = parts[1:]
}
if parts[2] != "*" || parts[3] != "*" {
return xerrors.Errorf("expected month and dom to be *")
}
return nil
}
+32 -4
View File
@@ -20,14 +20,14 @@ func Test_Weekly(t *testing.T) {
}{
{
name: "with timezone",
spec: "CRON_TZ=US/Central 30 9 1-5",
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: "",
},
{
name: "without timezone",
spec: "30 9 1-5",
spec: "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: "",
@@ -37,15 +37,43 @@ func Test_Weekly(t *testing.T) {
spec: "asdfasdfasdfsd",
at: time.Time{},
expectedNext: time.Time{},
expectedError: "parse schedule: expected exactly 3 fields, found 1: [asdfasdfasdfsd]",
expectedError: "validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ=<timezone> prefix",
},
{
name: "invalid location",
spec: "CRON_TZ=Fictional/Country 30 9 1-5",
spec: "CRON_TZ=Fictional/Country 30 9 * * 1-5",
at: time.Time{},
expectedNext: time.Time{},
expectedError: "parse schedule: provided bad location Fictional/Country: unknown time zone Fictional/Country",
},
{
name: "invalid schedule with 3 fields",
spec: "CRON_TZ=Fictional/Country 30 9 1-5",
at: time.Time{},
expectedNext: time.Time{},
expectedError: "validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ=<timezone> prefix",
},
{
name: "invalid schedule with 3 fields and no timezone",
spec: "30 9 1-5",
at: time.Time{},
expectedNext: time.Time{},
expectedError: "validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ=<timezone> prefix",
},
{
name: "valid schedule with 5 fields but month and dom not set to *",
spec: "30 9 1 1 1-5",
at: time.Time{},
expectedNext: time.Time{},
expectedError: "validate weekly schedule: expected month and dom to be *",
},
{
name: "valid schedule with 5 fields and timezone but month and dom not set to *",
spec: "CRON_TZ=Europe/Dublin 30 9 1 1 1-5",
at: time.Time{},
expectedNext: time.Time{},
expectedError: "validate weekly schedule: expected month and dom to be *",
},
}
for _, testCase := range testCases {
+22 -12
View File
@@ -205,7 +205,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
},
{
name: "friday to monday",
schedule: "CRON_TZ=Europe/Dublin 30 9 1-5",
schedule: "CRON_TZ=Europe/Dublin 30 9 * * 1-5",
expectedError: "",
at: time.Date(2022, 5, 6, 9, 31, 0, 0, dublinLoc),
expectedNext: time.Date(2022, 5, 9, 9, 30, 0, 0, dublinLoc),
@@ -213,7 +213,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
},
{
name: "monday to tuesday",
schedule: "CRON_TZ=Europe/Dublin 30 9 1-5",
schedule: "CRON_TZ=Europe/Dublin 30 9 * * 1-5",
expectedError: "",
at: time.Date(2022, 5, 9, 9, 31, 0, 0, dublinLoc),
expectedNext: time.Date(2022, 5, 10, 9, 30, 0, 0, dublinLoc),
@@ -222,7 +222,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
{
// DST in Ireland began on Mar 27 in 2022 at 0100. Forward 1 hour.
name: "DST start",
schedule: "CRON_TZ=Europe/Dublin 30 9 *",
schedule: "CRON_TZ=Europe/Dublin 30 9 * * *",
expectedError: "",
at: time.Date(2022, 3, 26, 9, 31, 0, 0, dublinLoc),
expectedNext: time.Date(2022, 3, 27, 9, 30, 0, 0, dublinLoc),
@@ -231,7 +231,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
{
// DST in Ireland ends on Oct 30 in 2022 at 0200. Back 1 hour.
name: "DST end",
schedule: "CRON_TZ=Europe/Dublin 30 9 *",
schedule: "CRON_TZ=Europe/Dublin 30 9 * * *",
expectedError: "",
at: time.Date(2022, 10, 29, 9, 31, 0, 0, dublinLoc),
expectedNext: time.Date(2022, 10, 30, 9, 30, 0, 0, dublinLoc),
@@ -239,13 +239,18 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
},
{
name: "invalid location",
schedule: "CRON_TZ=Imaginary/Place 30 9 1-5",
schedule: "CRON_TZ=Imaginary/Place 30 9 * * 1-5",
expectedError: "status code 500: invalid autostart schedule: parse schedule: provided bad location Imaginary/Place: unknown time zone Imaginary/Place",
},
{
name: "invalid schedule",
schedule: "asdf asdf asdf ",
expectedError: `status code 500: invalid autostart schedule: parse schedule: failed to parse int from asdf: strconv.Atoi: parsing "asdf": invalid syntax`,
expectedError: `status code 500: invalid autostart schedule: validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ=<timezone> prefix`,
},
{
name: "only 3 values",
schedule: "CRON_TZ=Europe/Dublin 30 9 *",
expectedError: `status code 500: invalid autostart schedule: validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ=<timezone> prefix`,
},
}
@@ -334,7 +339,7 @@ func TestWorkspaceUpdateAutostop(t *testing.T) {
},
{
name: "friday to monday",
schedule: "CRON_TZ=Europe/Dublin 30 17 1-5",
schedule: "CRON_TZ=Europe/Dublin 30 17 * * 1-5",
expectedError: "",
at: time.Date(2022, 5, 6, 17, 31, 0, 0, dublinLoc),
expectedNext: time.Date(2022, 5, 9, 17, 30, 0, 0, dublinLoc),
@@ -342,7 +347,7 @@ func TestWorkspaceUpdateAutostop(t *testing.T) {
},
{
name: "monday to tuesday",
schedule: "CRON_TZ=Europe/Dublin 30 17 1-5",
schedule: "CRON_TZ=Europe/Dublin 30 17 * * 1-5",
expectedError: "",
at: time.Date(2022, 5, 9, 17, 31, 0, 0, dublinLoc),
expectedNext: time.Date(2022, 5, 10, 17, 30, 0, 0, dublinLoc),
@@ -351,7 +356,7 @@ func TestWorkspaceUpdateAutostop(t *testing.T) {
{
// DST in Ireland began on Mar 27 in 2022 at 0100. Forward 1 hour.
name: "DST start",
schedule: "CRON_TZ=Europe/Dublin 30 17 *",
schedule: "CRON_TZ=Europe/Dublin 30 17 * * *",
expectedError: "",
at: time.Date(2022, 3, 26, 17, 31, 0, 0, dublinLoc),
expectedNext: time.Date(2022, 3, 27, 17, 30, 0, 0, dublinLoc),
@@ -360,7 +365,7 @@ func TestWorkspaceUpdateAutostop(t *testing.T) {
{
// DST in Ireland ends on Oct 30 in 2022 at 0200. Back 1 hour.
name: "DST end",
schedule: "CRON_TZ=Europe/Dublin 30 17 *",
schedule: "CRON_TZ=Europe/Dublin 30 17 * * *",
expectedError: "",
at: time.Date(2022, 10, 29, 17, 31, 0, 0, dublinLoc),
expectedNext: time.Date(2022, 10, 30, 17, 30, 0, 0, dublinLoc),
@@ -368,13 +373,18 @@ func TestWorkspaceUpdateAutostop(t *testing.T) {
},
{
name: "invalid location",
schedule: "CRON_TZ=Imaginary/Place 30 17 1-5",
schedule: "CRON_TZ=Imaginary/Place 30 17 * * 1-5",
expectedError: "status code 500: invalid autostop schedule: parse schedule: provided bad location Imaginary/Place: unknown time zone Imaginary/Place",
},
{
name: "invalid schedule",
schedule: "asdf asdf asdf ",
expectedError: `status code 500: invalid autostop schedule: parse schedule: failed to parse int from asdf: strconv.Atoi: parsing "asdf": invalid syntax`,
expectedError: `status code 500: invalid autostop schedule: validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ=<timezone> prefix`,
},
{
name: "only 3 values",
schedule: "CRON_TZ=Europe/Dublin 30 9 *",
expectedError: `status code 500: invalid autostop schedule: validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ=<timezone> prefix`,
},
}