From 77417b7365ae544e26ff1d6c403711852ed2cd02 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 4 Sep 2023 10:22:36 +0000 Subject: [PATCH 1/2] refactor(coderd/schedule): move cron schedule to cron package This removes an indirect import of `coderd/database` from the CLI and results in a logical separation between server related and generalized schedule. No size change (yet). Ref: #9380 --- cli/list.go | 4 ++-- cli/schedule.go | 4 ++-- cli/util.go | 6 +++--- coderd/autobuild/lifecycle_executor.go | 3 ++- coderd/autobuild/lifecycle_executor_test.go | 5 +++-- coderd/provisionerdserver/provisionerdserver_test.go | 3 ++- coderd/schedule/autostop_test.go | 3 ++- coderd/schedule/{ => cron}/cron.go | 10 +++++----- coderd/schedule/{ => cron}/cron_test.go | 6 +++--- coderd/schedule/user.go | 3 ++- coderd/workspaces.go | 4 ++-- coderd/workspaces_test.go | 3 ++- enterprise/coderd/schedule/user.go | 3 ++- enterprise/coderd/users_test.go | 10 +++++----- enterprise/coderd/workspaces_test.go | 3 ++- 15 files changed, 39 insertions(+), 31 deletions(-) rename coderd/schedule/{ => cron}/cron.go (96%) rename coderd/schedule/{ => cron}/cron_test.go (98%) diff --git a/cli/list.go b/cli/list.go index 12d0a48149ef7..e6a1f6fee6d7f 100644 --- a/cli/list.go +++ b/cli/list.go @@ -9,7 +9,7 @@ import ( "github.com/coder/coder/v2/cli/clibase" "github.com/coder/coder/v2/cli/cliui" - "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" ) @@ -39,7 +39,7 @@ func workspaceListRowFromWorkspace(now time.Time, usersByID map[uuid.UUID]coders lastBuilt := now.UTC().Sub(workspace.LatestBuild.Job.CreatedAt).Truncate(time.Second) autostartDisplay := "-" if !ptr.NilOrEmpty(workspace.AutostartSchedule) { - if sched, err := schedule.Weekly(*workspace.AutostartSchedule); err == nil { + if sched, err := cron.Weekly(*workspace.AutostartSchedule); err == nil { autostartDisplay = fmt.Sprintf("%s %s (%s)", sched.Time(), sched.DaysOfWeek(), sched.Location()) } } diff --git a/cli/schedule.go b/cli/schedule.go index 629d6160fa3ee..6b0f105875c80 100644 --- a/cli/schedule.go +++ b/cli/schedule.go @@ -10,7 +10,7 @@ import ( "github.com/coder/coder/v2/cli/clibase" "github.com/coder/coder/v2/cli/cliui" - "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/coderd/util/tz" "github.com/coder/coder/v2/codersdk" @@ -255,7 +255,7 @@ func displaySchedule(workspace codersdk.Workspace, out io.Writer) error { schedNextStop = "-" ) if !ptr.NilOrEmpty(workspace.AutostartSchedule) { - sched, err := schedule.Weekly(ptr.NilToEmpty(workspace.AutostartSchedule)) + sched, err := cron.Weekly(ptr.NilToEmpty(workspace.AutostartSchedule)) if err != nil { // This should never happen. _, _ = fmt.Fprintf(out, "Invalid autostart schedule %q for workspace %s: %s\n", *workspace.AutostartSchedule, workspace.Name, err.Error()) diff --git a/cli/util.go b/cli/util.go index e0fe340e45ca2..0b86c10a2cb0d 100644 --- a/cli/util.go +++ b/cli/util.go @@ -8,7 +8,7 @@ import ( "golang.org/x/xerrors" - "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/util/tz" ) @@ -74,7 +74,7 @@ func relative(d time.Duration) string { } // parseCLISchedule parses a schedule in the format HH:MM{AM|PM} [DOW] [LOCATION] -func parseCLISchedule(parts ...string) (*schedule.Schedule, error) { +func parseCLISchedule(parts ...string) (*cron.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 { @@ -121,7 +121,7 @@ func parseCLISchedule(parts ...string) (*schedule.Schedule, error) { } } - sched, err := schedule.Weekly(fmt.Sprintf( + sched, err := cron.Weekly(fmt.Sprintf( "CRON_TZ=%s %d %d * * %s", loc.String(), minute, diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 55adb31733647..2b3b3195f8387 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -17,6 +17,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/coder/v2/codersdk" ) @@ -306,7 +307,7 @@ func isEligibleForAutostart(ws database.Workspace, build database.WorkspaceBuild return false } - sched, err := schedule.Weekly(ws.AutostartSchedule.String) + sched, err := cron.Weekly(ws.AutostartSchedule.String) if err != nil { return false } diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index fab7510933f5f..d6d60257a1f9c 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -17,6 +17,7 @@ import ( "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisioner/echo" @@ -784,9 +785,9 @@ func mustProvisionWorkspaceWithParameters(t *testing.T, client *codersdk.Client, return coderdtest.MustWorkspace(t, client, ws.ID) } -func mustSchedule(t *testing.T, s string) *schedule.Schedule { +func mustSchedule(t *testing.T, s string) *cron.Schedule { t.Helper() - sched, err := schedule.Weekly(s) + sched, err := cron.Weekly(s) require.NoError(t, err) return sched } diff --git a/coderd/provisionerdserver/provisionerdserver_test.go b/coderd/provisionerdserver/provisionerdserver_test.go index 3ac5ac4396ca7..6502a2d79675f 100644 --- a/coderd/provisionerdserver/provisionerdserver_test.go +++ b/coderd/provisionerdserver/provisionerdserver_test.go @@ -27,6 +27,7 @@ import ( "github.com/coder/coder/v2/coderd/gitauth" "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/telemetry" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisionerd/proto" @@ -1330,7 +1331,7 @@ func TestCompleteJob(t *testing.T) { }, nil } - sched, err := schedule.Daily(c.userQuietHoursSchedule) + sched, err := cron.Daily(c.userQuietHoursSchedule) if !assert.NoError(t, err) { return schedule.UserQuietHoursScheduleOptions{}, err } diff --git a/coderd/schedule/autostop_test.go b/coderd/schedule/autostop_test.go index 18758d8ff2338..cafe2b413eaed 100644 --- a/coderd/schedule/autostop_test.go +++ b/coderd/schedule/autostop_test.go @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/testutil" ) @@ -432,7 +433,7 @@ func TestCalculateAutoStop(t *testing.T) { }, nil } - sched, err := schedule.Daily(c.userQuietHoursSchedule) + sched, err := cron.Daily(c.userQuietHoursSchedule) if !assert.NoError(t, err) { return schedule.UserQuietHoursScheduleOptions{}, err } diff --git a/coderd/schedule/cron.go b/coderd/schedule/cron/cron.go similarity index 96% rename from coderd/schedule/cron.go rename to coderd/schedule/cron/cron.go index 7a86b130d24bc..7662212e22f48 100644 --- a/coderd/schedule/cron.go +++ b/coderd/schedule/cron/cron.go @@ -1,7 +1,7 @@ // package schedule provides utilities for managing template and workspace // autostart and autostop schedules. This includes utilities for parsing and // deserializing cron-style expressions. -package schedule +package cron import ( "fmt" @@ -30,11 +30,11 @@ var defaultParser = cron.NewParser(parserFormat) // // Example Usage: // -// local_sched, _ := schedule.Weekly("59 23 *") +// local_sched, _ := cron.Weekly("59 23 *") // fmt.Println(sched.Next(time.Now().Format(time.RFC3339))) // // Output: 2022-04-04T23:59:00Z // -// us_sched, _ := schedule.Weekly("CRON_TZ=US/Central 30 9 1-5") +// us_sched, _ := cron.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(raw string) (*Schedule, error) { @@ -56,11 +56,11 @@ func Weekly(raw string) (*Schedule, error) { // // Example Usage: // -// local_sched, _ := schedule.Weekly("59 23 * * *") +// local_sched, _ := cron.Weekly("59 23 * * *") // fmt.Println(sched.Next(time.Now().Format(time.RFC3339))) // // Output: 2022-04-04T23:59:00Z // -// us_sched, _ := schedule.Weekly("CRON_TZ=US/Central 30 9 * * *") +// us_sched, _ := cron.Weekly("CRON_TZ=US/Central 30 9 * * *") // fmt.Println(sched.Next(time.Now()).Format(time.RFC3339)) // // Output: 2022-04-04T14:30:00Z func Daily(raw string) (*Schedule, error) { diff --git a/coderd/schedule/cron_test.go b/coderd/schedule/cron/cron_test.go similarity index 98% rename from coderd/schedule/cron_test.go rename to coderd/schedule/cron/cron_test.go index b8ab1600ef2c5..7cf146767fab3 100644 --- a/coderd/schedule/cron_test.go +++ b/coderd/schedule/cron/cron_test.go @@ -1,4 +1,4 @@ -package schedule_test +package cron_test import ( "testing" @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/require" - "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" ) func Test_Weekly(t *testing.T) { @@ -144,7 +144,7 @@ func Test_Weekly(t *testing.T) { testCase := testCase t.Run(testCase.name, func(t *testing.T) { t.Parallel() - actual, err := schedule.Weekly(testCase.spec) + actual, err := cron.Weekly(testCase.spec) if testCase.expectedError == "" { nextTime := actual.Next(testCase.at) require.NoError(t, err) diff --git a/coderd/schedule/user.go b/coderd/schedule/user.go index 18df0b40e7a37..2ba9ce1621e37 100644 --- a/coderd/schedule/user.go +++ b/coderd/schedule/user.go @@ -6,6 +6,7 @@ import ( "github.com/google/uuid" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/schedule/cron" ) type UserQuietHoursScheduleOptions struct { @@ -17,7 +18,7 @@ type UserQuietHoursScheduleOptions struct { // schedule (and UserSet will be false). If quiet hours schedules are not // entitled or disabled instance-wide, this value will be nil to denote that // quiet hours windows should not be used. - Schedule *Schedule + Schedule *cron.Schedule UserSet bool } diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 3c486ed6bb079..7b6ad9f0b8c5d 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -22,7 +22,7 @@ import ( "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/rbac" - "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/searchquery" "github.com/coder/coder/v2/coderd/telemetry" "github.com/coder/coder/v2/coderd/util/ptr" @@ -1306,7 +1306,7 @@ func validWorkspaceSchedule(s *string) (sql.NullString, error) { return sql.NullString{}, nil } - _, err := schedule.Weekly(*s) + _, err := cron.Weekly(*s) if err != nil { return sql.NullString{}, err } diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 5f8dcf084ac70..9d4082d4e4e39 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -30,6 +30,7 @@ import ( "github.com/coder/coder/v2/coderd/parameter" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" @@ -1869,7 +1870,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) { require.EqualValues(t, *testCase.schedule, *updated.AutostartSchedule, "expected autostart schedule to equal requested") - sched, err := schedule.Weekly(*updated.AutostartSchedule) + sched, err := cron.Weekly(*updated.AutostartSchedule) require.NoError(t, err, "parse returned schedule") next := sched.Next(testCase.at) diff --git a/enterprise/coderd/schedule/user.go b/enterprise/coderd/schedule/user.go index bb054305a6db2..49c2b61b30e99 100644 --- a/enterprise/coderd/schedule/user.go +++ b/enterprise/coderd/schedule/user.go @@ -9,6 +9,7 @@ import ( "github.com/coder/coder/v2/coderd/database" agpl "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/tracing" ) @@ -49,7 +50,7 @@ func (s *enterpriseUserQuietHoursScheduleStore) parseSchedule(ctx context.Contex rawSchedule = s.defaultSchedule } - sched, err := agpl.Daily(rawSchedule) + sched, err := cron.Daily(rawSchedule) if err != nil { // This shouldn't get hit during Gets, only Sets. return agpl.UserQuietHoursScheduleOptions{}, xerrors.Errorf("parse daily schedule %q: %w", rawSchedule, err) diff --git a/enterprise/coderd/users_test.go b/enterprise/coderd/users_test.go index 6ad93b2d92e06..f91b3b348ca08 100644 --- a/enterprise/coderd/users_test.go +++ b/enterprise/coderd/users_test.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/coderdtest" - "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" "github.com/coder/coder/v2/enterprise/coderd/license" @@ -22,14 +22,14 @@ func TestUserQuietHours(t *testing.T) { t.Parallel() defaultQuietHoursSchedule := "CRON_TZ=America/Chicago 0 0 * * *" - defaultScheduleParsed, err := schedule.Daily(defaultQuietHoursSchedule) + defaultScheduleParsed, err := cron.Daily(defaultQuietHoursSchedule) require.NoError(t, err) nextTime := defaultScheduleParsed.Next(time.Now().In(defaultScheduleParsed.Location())) if time.Until(nextTime) < time.Hour { // Use a different default schedule instead, because we want to avoid // the schedule "ticking over" during this test run. defaultQuietHoursSchedule = "CRON_TZ=America/Chicago 0 12 * * *" - defaultScheduleParsed, err = schedule.Daily(defaultQuietHoursSchedule) + defaultScheduleParsed, err = cron.Daily(defaultQuietHoursSchedule) require.NoError(t, err) } @@ -61,14 +61,14 @@ func TestUserQuietHours(t *testing.T) { // Set their quiet hours. customQuietHoursSchedule := "CRON_TZ=Australia/Sydney 0 0 * * *" - customScheduleParsed, err := schedule.Daily(customQuietHoursSchedule) + customScheduleParsed, err := cron.Daily(customQuietHoursSchedule) require.NoError(t, err) nextTime = customScheduleParsed.Next(time.Now().In(customScheduleParsed.Location())) if time.Until(nextTime) < time.Hour { // Use a different default schedule instead, because we want to avoid // the schedule "ticking over" during this test run. customQuietHoursSchedule = "CRON_TZ=Australia/Sydney 0 12 * * *" - customScheduleParsed, err = schedule.Daily(customQuietHoursSchedule) + customScheduleParsed, err = cron.Daily(customQuietHoursSchedule) require.NoError(t, err) } diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index db14ae96f1100..2424b9ee8cc06 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" agplschedule "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" @@ -585,7 +586,7 @@ func TestWorkspaceAutobuild(t *testing.T) { template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - sched, err := agplschedule.Weekly("CRON_TZ=UTC 0 * * * *") + sched, err := cron.Weekly("CRON_TZ=UTC 0 * * * *") require.NoError(t, err) ws := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { From 990da42a0818595cc264dcb14d45f37a721413d0 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 4 Sep 2023 12:15:45 +0000 Subject: [PATCH 2/2] rename robfig/cron import to rbcron to avoid conflict --- coderd/schedule/cron/cron.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/coderd/schedule/cron/cron.go b/coderd/schedule/cron/cron.go index 7662212e22f48..1399215843999 100644 --- a/coderd/schedule/cron/cron.go +++ b/coderd/schedule/cron/cron.go @@ -8,16 +8,16 @@ import ( "strings" "time" - "github.com/robfig/cron/v3" + rbcron "github.com/robfig/cron/v3" "golang.org/x/xerrors" ) // For the purposes of this library, we only need minute, hour, and // 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 +const parserFormat = rbcron.Minute | rbcron.Hour | rbcron.Dom | rbcron.Month | rbcron.Dow -var defaultParser = cron.NewParser(parserFormat) +var defaultParser = rbcron.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: @@ -83,7 +83,7 @@ func parse(raw string) (*Schedule, error) { return nil, xerrors.Errorf("parse schedule: %w", err) } - schedule, ok := specSched.(*cron.SpecSchedule) + schedule, ok := specSched.(*rbcron.SpecSchedule) if !ok { return nil, xerrors.Errorf("expected *cron.SpecSchedule but got %T", specSched) } @@ -110,7 +110,7 @@ func parse(raw string) (*Schedule, error) { // It's essentially a wrapper for robfig/cron/v3 that has additional // convenience methods. type Schedule struct { - sched *cron.SpecSchedule + sched *rbcron.SpecSchedule // XXX: there isn't any nice way for robfig/cron to serialize cronStr string }