Skip to content

Commit

Permalink
fix(scheduler): mix cron with start date
Browse files Browse the repository at this point in the history
  • Loading branch information
@karol-kokoszka
karol-kokoszka committed Feb 19, 2024
1 parent 1684aad commit b2c37f6
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 22 deletions.
8 changes: 4 additions & 4 deletions pkg/cmd/scylla-manager/tasks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func makeAutoHealthCheckTasks(clusterID uuid.UUID) []*scheduler.Task {
Enabled: true,
Name: "cql",
Sched: scheduler.Schedule{
Cron: scheduler.NewCronEvery(15 * time.Second),
Cron: scheduler.NewCronEvery(15*time.Second, time.Time{}),
Timezone: localTimezone(),
},
Properties: healthCheckModeProperties(healthcheck.CQLMode),
Expand All @@ -46,7 +46,7 @@ func makeAutoHealthCheckTasks(clusterID uuid.UUID) []*scheduler.Task {
Enabled: true,
Name: "rest",
Sched: scheduler.Schedule{
Cron: scheduler.NewCronEvery(1 * time.Minute),
Cron: scheduler.NewCronEvery(1*time.Minute, time.Time{}),
Timezone: localTimezone(),
},
Properties: healthCheckModeProperties(healthcheck.RESTMode),
Expand All @@ -57,7 +57,7 @@ func makeAutoHealthCheckTasks(clusterID uuid.UUID) []*scheduler.Task {
Enabled: true,
Name: "alternator",
Sched: scheduler.Schedule{
Cron: scheduler.NewCronEvery(15 * time.Second),
Cron: scheduler.NewCronEvery(15*time.Second, time.Time{}),
Timezone: localTimezone(),
},
Properties: healthCheckModeProperties(healthcheck.AlternatorMode),
Expand All @@ -74,7 +74,7 @@ func makeAutoRepairTask(clusterID uuid.UUID) *scheduler.Task {
Enabled: true,
Name: "all-weekly",
Sched: scheduler.Schedule{
Cron: scheduler.MustCron("0 23 * * SAT"),
Cron: scheduler.MustCron("0 23 * * SAT", time.Time{}),
Timezone: localTimezone(),
NumRetries: 3,
},
Expand Down
44 changes: 34 additions & 10 deletions pkg/service/scheduler/model.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,30 +70,38 @@ func (t *TaskType) UnmarshalText(text []byte) error {
// Cron implements a trigger based on cron expression.
// It supports the extended syntax including @monthly, @weekly, @daily, @midnight, @hourly, @every <time.Duration>.
type Cron struct {
spec []byte
*cronSpecification
inner scheduler.Trigger
}

func NewCron(spec string) (Cron, error) {
type cronSpecification struct {
Spec string
StartDate time.Time
}

func NewCron(spec string, startDate time.Time) (Cron, error) {
t, err := trigger.NewCron(spec)
if err != nil {
return Cron{}, err
}

return Cron{
spec: []byte(spec),
cronSpecification: &cronSpecification{
Spec: spec,
StartDate: startDate,
},
inner: t,
}, nil
}

func NewCronEvery(d time.Duration) Cron {
c, _ := NewCron("@every " + d.String()) // nolint: errcheck
func NewCronEvery(d time.Duration, startDate time.Time) Cron {
c, _ := NewCron("@every "+d.String(), startDate) // nolint: errcheck
return c
}

// MustCron calls NewCron and panics on error.
func MustCron(spec string) Cron {
c, err := NewCron(spec)
func MustCron(spec string, startDate time.Time) Cron {
c, err := NewCron(spec, startDate)
if err != nil {
panic(err)
}
Expand All @@ -105,21 +113,37 @@ func (c Cron) Next(now time.Time) time.Time {
if c.inner == nil {
return time.Time{}
}
if c.StartDate.After(now) {
return c.inner.Next(c.StartDate)
}
return c.inner.Next(now)
}

func (c Cron) MarshalText() (text []byte, err error) {
return c.spec, nil
bytes, err := json.Marshal(c.cronSpecification)
if err != nil {
return nil, errors.Wrapf(err, "cannot json marshal {%v}", c.cronSpecification)
}
return bytes, nil
}

func (c *Cron) UnmarshalText(text []byte) error {
if len(text) == 0 {
return nil
}

v, err := NewCron(string(text))
var cronSpec cronSpecification
err := json.Unmarshal(text, &cronSpec)
if err != nil {
return errors.Wrap(err, "cron")
// fallback to the < 3.2.6 approach where cron was not coupled with start date
cronSpec = cronSpecification{
Spec: string(text),
}
}

v, err2 := NewCron(cronSpec.Spec, cronSpec.StartDate)
if err2 != nil {
return errors.Wrap(multierr.Combine(err, err2), "cron")
}

*c = v
Expand Down
110 changes: 102 additions & 8 deletions pkg/service/scheduler/model_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,25 +56,119 @@ func TestStatus(t *testing.T) {
}

func TestCronMarshalUnmarshal(t *testing.T) {
spec := "@every 15s"

var cron Cron
if err := cron.UnmarshalText([]byte(spec)); err != nil {
nonZeroTimeString := "2024-02-23T01:12:00Z"
nonZeroTime, err := time.Parse(time.RFC3339, nonZeroTimeString)
if err != nil {
t.Fatal(err)
}
b, _ := cron.MarshalText()
if string(b) != spec {
t.Fatalf("MarshalText() = %s, expected %s", string(b), spec)

for _, tc := range []struct {
name string
data []byte
expectedSpec cronSpecification
}{
{
name: "(3.2.6 backward compatibility) unmarshal spec",
data: []byte("@every 15s"),
expectedSpec: cronSpecification{
Spec: "@every 15s",
StartDate: time.Time{},
},
},
{
name: "unmarshal spec full struct zero time",
data: []byte(`{"Spec": "@every 15s", "StartDate": "0001-01-01T00:00:00Z"}`),
expectedSpec: cronSpecification{
Spec: "@every 15s",
StartDate: time.Time{},
},
},
{
name: "unmarshal spec full struct non-zero time",
data: []byte(`{"Spec": "@every 15s", "StartDate": "` + nonZeroTimeString + `"}`),
expectedSpec: cronSpecification{
Spec: "@every 15s",
StartDate: nonZeroTime,
},
},
} {
t.Run(tc.name, func(t *testing.T) {
var cron, finalCron Cron
if err := cron.UnmarshalText(tc.data); err != nil {
t.Fatal(err)
}
b, err := cron.MarshalText()
if err != nil {
t.Fatal(err)
}
if err := finalCron.UnmarshalText(b); err != nil {
t.Fatal(err)
}

if finalCron.Spec != tc.expectedSpec.Spec {
t.Fatalf("MarshalText() = %s, expected spec %s", finalCron.Spec, tc.expectedSpec.Spec)
}
if finalCron.StartDate != tc.expectedSpec.StartDate {
t.Fatalf("MarshalText() = %s, expected startDate %s", finalCron.StartDate, tc.expectedSpec.StartDate)
}

})
}
}

func TestNewCronEvery(t *testing.T) {
c := NewCronEvery(15 * time.Second)
c := NewCronEvery(15*time.Second, time.Time{})
if c.IsZero() {
t.Fatal()
}
}

func TestNewCronWithNonZeroStartDate(t *testing.T) {
for _, tc := range []struct {
name string
nowRFC3339 string
startDateRFC3339 string
cronExpression string
expectedNextRFC3339 string
}{
{
name: "current time couple of rounds before start date",
nowRFC3339: "2024-01-01T03:00:00Z",
startDateRFC3339: "2024-02-23T03:00:00Z",
cronExpression: "0 2 * * *",
expectedNextRFC3339: "2024-02-24T02:00:00Z",
},
{
name: "current time couple of rounds before start date",
nowRFC3339: "2024-01-01T03:00:00Z",
startDateRFC3339: "0000-01-01T00:00:00Z",
cronExpression: "0 2 * * *",
expectedNextRFC3339: "2024-01-02T02:00:00Z",
},
} {
parsedStart, err := time.Parse(time.RFC3339, tc.startDateRFC3339)
if err != nil {
t.Fatal(err)
}
parsedExpected, err := time.Parse(time.RFC3339, tc.expectedNextRFC3339)
if err != nil {
t.Fatal(err)
}
parsedNow, err := time.Parse(time.RFC3339, tc.nowRFC3339)
if err != nil {
t.Fatal(err)
}
c, err := NewCron(tc.cronExpression, parsedStart)
if err != nil {
t.Fatal(err)
}
next := c.Next(parsedNow)
if !next.Equal(parsedExpected) {
t.Fatalf("expected next schedule %v, but got %v", parsedExpected, next)
}
}
}

func TestEmptyCron(t *testing.T) {
var cron Cron
if err := cron.UnmarshalText(nil); err != nil {
Expand Down

0 comments on commit b2c37f6

Please sign in to comment.