diff --git a/.changelog/17858.txt b/.changelog/17858.txt new file mode 100644 index 000000000..9af1a2a93 --- /dev/null +++ b/.changelog/17858.txt @@ -0,0 +1,3 @@ +```release-note:feature +jobspec: Add 'crons' fileld for multiple `cron` expressions +``` diff --git a/api/jobs.go b/api/jobs.go index 3b60f695b..702680cc9 100644 --- a/api/jobs.go +++ b/api/jobs.go @@ -821,8 +821,9 @@ type MultiregionRegion struct { // PeriodicConfig is for serializing periodic config for a job. type PeriodicConfig struct { - Enabled *bool `hcl:"enabled,optional"` - Spec *string `hcl:"cron,optional"` + Enabled *bool `hcl:"enabled,optional"` + Spec *string `hcl:"cron,optional"` + Specs []string `hcl:"crons,optional"` SpecType *string ProhibitOverlap *bool `mapstructure:"prohibit_overlap" hcl:"prohibit_overlap,optional"` TimeZone *string `mapstructure:"time_zone" hcl:"time_zone,optional"` @@ -835,6 +836,9 @@ func (p *PeriodicConfig) Canonicalize() { if p.Spec == nil { p.Spec = pointerOf("") } + if p.Specs == nil { + p.Specs = []string{} + } if p.SpecType == nil { p.SpecType = pointerOf(PeriodicSpecCron) } @@ -851,30 +855,43 @@ func (p *PeriodicConfig) Canonicalize() { // returned. The `time.Location` of the returned value matches that of the // passed time. func (p *PeriodicConfig) Next(fromTime time.Time) (time.Time, error) { + // Single spec parsing if p != nil && *p.SpecType == PeriodicSpecCron { - e, err := cronexpr.Parse(*p.Spec) - if err != nil { - return time.Time{}, fmt.Errorf("failed parsing cron expression %q: %v", *p.Spec, err) + if p.Spec != nil && *p.Spec != "" { + return cronParseNext(fromTime, *p.Spec) } - return cronParseNext(e, fromTime, *p.Spec) } - return time.Time{}, nil + // multiple specs parsing + var nextTime time.Time + for _, spec := range p.Specs { + t, err := cronParseNext(fromTime, spec) + if err != nil { + return time.Time{}, fmt.Errorf("failed parsing cron expression %s: %v", spec, err) + } + if nextTime.IsZero() || t.Before(nextTime) { + nextTime = t + } + } + return nextTime, nil } // cronParseNext is a helper that parses the next time for the given expression // but captures any panic that may occur in the underlying library. // --- THIS FUNCTION IS REPLICATED IN nomad/structs/structs.go // and should be kept in sync. -func cronParseNext(e *cronexpr.Expression, fromTime time.Time, spec string) (t time.Time, err error) { +func cronParseNext(fromTime time.Time, spec string) (t time.Time, err error) { defer func() { if recover() != nil { t = time.Time{} err = fmt.Errorf("failed parsing cron expression: %q", spec) } }() - - return e.Next(fromTime), nil + exp, err := cronexpr.Parse(spec) + if err != nil { + return time.Time{}, fmt.Errorf("failed parsing cron expression: %s: %v", spec, err) + } + return exp.Next(fromTime), nil } func (p *PeriodicConfig) GetLocation() (*time.Location, error) { diff --git a/api/jobs_test.go b/api/jobs_test.go index 83c20c971..ee3c10612 100644 --- a/api/jobs_test.go +++ b/api/jobs_test.go @@ -834,6 +834,7 @@ func TestJobs_Canonicalize(t *testing.T) { Periodic: &PeriodicConfig{ Enabled: pointerOf(true), Spec: pointerOf(""), + Specs: []string{}, SpecType: pointerOf(PeriodicSpecCron), ProhibitOverlap: pointerOf(false), TimeZone: pointerOf("UTC"), diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 12a404215..da4700276 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1007,6 +1007,10 @@ func ApiJobToStructJob(job *api.Job) *structs.Job { if job.Periodic.Spec != nil { j.Periodic.Spec = *job.Periodic.Spec } + + if job.Periodic.Specs != nil { + j.Periodic.Specs = job.Periodic.Specs + } } if job.ParameterizedJob != nil { diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index df981735b..852f01cdc 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -2471,6 +2471,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Periodic: &api.PeriodicConfig{ Enabled: pointer.Of(true), Spec: pointer.Of("spec"), + Specs: []string{"spec"}, SpecType: pointer.Of("cron"), ProhibitOverlap: pointer.Of(true), TimeZone: pointer.Of("test zone"), @@ -2882,6 +2883,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Periodic: &structs.PeriodicConfig{ Enabled: true, Spec: "spec", + Specs: []string{"spec"}, SpecType: "cron", ProhibitOverlap: true, TimeZone: "test zone", diff --git a/jobspec/parse_job.go b/jobspec/parse_job.go index 46c3dd757..3ce283087 100644 --- a/jobspec/parse_job.go +++ b/jobspec/parse_job.go @@ -234,6 +234,7 @@ func parsePeriodic(result **api.PeriodicConfig, list *ast.ObjectList) error { valid := []string{ "enabled", "cron", + "crons", "prohibit_overlap", "time_zone", } @@ -255,6 +256,12 @@ func parsePeriodic(result **api.PeriodicConfig, list *ast.ObjectList) error { m["Spec"] = cron } + // If "crons" is provided, set the type to "cron" and store the spec. + if cron, ok := m["crons"]; ok { + m["SpecType"] = api.PeriodicSpecCron + m["Specs"] = cron + } + // Build the constraint var p api.PeriodicConfig if err := mapstructure.WeakDecode(m, &p); err != nil { diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 8a7342e41..36c305106 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -568,6 +568,21 @@ func TestParse(t *testing.T) { false, }, + { + "periodic-crons.hcl", + &api.Job{ + ID: stringToPtr("foo"), + Name: stringToPtr("foo"), + Periodic: &api.PeriodicConfig{ + SpecType: stringToPtr(api.PeriodicSpecCron), + Specs: []string{"*/5 * * *", "*/7 * * *"}, + ProhibitOverlap: boolToPtr(true), + TimeZone: stringToPtr("Europe/Minsk"), + }, + }, + false, + }, + { "specify-job.hcl", &api.Job{ diff --git a/jobspec/test-fixtures/periodic-crons.hcl b/jobspec/test-fixtures/periodic-crons.hcl new file mode 100644 index 000000000..3baa1b4a1 --- /dev/null +++ b/jobspec/test-fixtures/periodic-crons.hcl @@ -0,0 +1,13 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: MPL-2.0 + +job "foo" { + periodic { + crons = [ + "*/5 * * *", + "*/7 * * *" + ] + prohibit_overlap = true + time_zone = "Europe/Minsk" + } +} diff --git a/jobspec2/parse_job.go b/jobspec2/parse_job.go index 001d23b0c..029b1fcee 100644 --- a/jobspec2/parse_job.go +++ b/jobspec2/parse_job.go @@ -19,7 +19,7 @@ func normalizeJob(jc *jobConfig) { j.ID = &jc.JobID } - if j.Periodic != nil && j.Periodic.Spec != nil { + if j.Periodic != nil && (j.Periodic.Spec != nil || j.Periodic.Specs != nil) { v := "cron" j.Periodic.SpecType = &v } diff --git a/nomad/structs/diff.go b/nomad/structs/diff.go index 0312b2b4a..1d9aa320e 100644 --- a/nomad/structs/diff.go +++ b/nomad/structs/diff.go @@ -136,7 +136,7 @@ func (j *Job) Diff(other *Job, contextual bool) (*JobDiff, error) { diff.TaskGroups = tgs // Periodic diff - if pDiff := primitiveObjectDiff(j.Periodic, other.Periodic, nil, "Periodic", contextual); pDiff != nil { + if pDiff := periodicDiff(j.Periodic, other.Periodic, contextual); pDiff != nil { diff.Objects = append(diff.Objects, pDiff) } @@ -2628,6 +2628,37 @@ func stringSetDiff(old, new []string, name string, contextual bool) *ObjectDiff return diff } +func periodicDiff(old, new *PeriodicConfig, contextual bool) *ObjectDiff { + diff := &ObjectDiff{Type: DiffTypeNone, Name: "Periodic"} + var oldPeriodicFlat, newPeriodicFlat map[string]string + + if reflect.DeepEqual(old, new) { + return nil + } else if old == nil { + old = &PeriodicConfig{} + diff.Type = DiffTypeAdded + newPeriodicFlat = flatmap.Flatten(new, nil, true) + } else if new == nil { + new = &PeriodicConfig{} + diff.Type = DiffTypeDeleted + oldPeriodicFlat = flatmap.Flatten(old, nil, true) + } else { + diff.Type = DiffTypeEdited + oldPeriodicFlat = flatmap.Flatten(old, nil, true) + newPeriodicFlat = flatmap.Flatten(new, nil, true) + } + + // Diff the primitive fields. + diff.Fields = fieldDiffs(oldPeriodicFlat, newPeriodicFlat, contextual) + + if setDiff := stringSetDiff(old.Specs, new.Specs, "Specs", contextual); setDiff != nil && setDiff.Type != DiffTypeNone { + diff.Objects = append(diff.Objects, setDiff) + } + + sort.Sort(FieldDiffs(diff.Fields)) + return diff +} + // primitiveObjectDiff returns a diff of the passed objects' primitive fields. // The filter field can be used to exclude fields from the diff. The name is the // name of the objects. If contextual is set, non-changed fields will also be diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index f7286d69f..713955202 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -565,6 +565,74 @@ func TestJobDiff(t *testing.T) { }, }, }, + { + // Periodic multiple times added + Old: &Job{}, + New: &Job{ + Periodic: &PeriodicConfig{ + Enabled: false, + Specs: []string{"*/15 * * * * *", "*/16 * * * * *"}, + SpecType: "foo", + ProhibitOverlap: false, + TimeZone: "Europe/Minsk", + }, + }, + Expected: &JobDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "Periodic", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Enabled", + Old: "", + New: "false", + }, + { + Type: DiffTypeAdded, + Name: "ProhibitOverlap", + Old: "", + New: "false", + }, + { + Type: DiffTypeAdded, + Name: "SpecType", + Old: "", + New: "foo", + }, + { + Type: DiffTypeAdded, + Name: "TimeZone", + Old: "", + New: "Europe/Minsk", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "Specs", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Specs", + Old: "", + New: "*/15 * * * * *", + }, + { + Type: DiffTypeAdded, + Name: "Specs", + Old: "", + New: "*/16 * * * * *", + }, + }, + }, + }, + }, + }, + }, + }, { // Periodic deleted Old: &Job{ @@ -681,6 +749,258 @@ func TestJobDiff(t *testing.T) { }, }, }, + { + // Periodic single to multiple times + Old: &Job{ + Periodic: &PeriodicConfig{ + Enabled: false, + Spec: "*/15 * * * * *", + SpecType: "foo", + ProhibitOverlap: false, + TimeZone: "Europe/Minsk", + }, + }, + New: &Job{ + Periodic: &PeriodicConfig{ + Enabled: true, + Specs: []string{"* * * * * *", "*/5 * * * * *"}, + SpecType: "cron", + ProhibitOverlap: true, + TimeZone: "America/Los_Angeles", + }, + }, + Expected: &JobDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Periodic", + Fields: []*FieldDiff{ + { + Type: DiffTypeEdited, + Name: "Enabled", + Old: "false", + New: "true", + }, + { + Type: DiffTypeEdited, + Name: "ProhibitOverlap", + Old: "false", + New: "true", + }, + { + Type: DiffTypeDeleted, + Name: "Spec", + Old: "*/15 * * * * *", + New: "", + }, + { + Type: DiffTypeEdited, + Name: "SpecType", + Old: "foo", + New: "cron", + }, + { + Type: DiffTypeEdited, + Name: "TimeZone", + Old: "Europe/Minsk", + New: "America/Los_Angeles", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "Specs", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Specs", + Old: "", + New: "* * * * * *", + }, + { + Type: DiffTypeAdded, + Name: "Specs", + Old: "", + New: "*/5 * * * * *", + }, + }, + }, + }, + }, + }, + }, + }, + { + // Periodic multiple times to single + Old: &Job{ + Periodic: &PeriodicConfig{ + Enabled: false, + Specs: []string{"* * * * * *", "*/5 * * * * *"}, + SpecType: "foo", + ProhibitOverlap: false, + TimeZone: "Europe/Minsk", + }, + }, + New: &Job{ + Periodic: &PeriodicConfig{ + Enabled: true, + Spec: "*/15 * * * * *", + SpecType: "cron", + ProhibitOverlap: true, + TimeZone: "America/Los_Angeles", + }, + }, + Expected: &JobDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Periodic", + Fields: []*FieldDiff{ + { + Type: DiffTypeEdited, + Name: "Enabled", + Old: "false", + New: "true", + }, + { + Type: DiffTypeEdited, + Name: "ProhibitOverlap", + Old: "false", + New: "true", + }, + { + Type: DiffTypeAdded, + Name: "Spec", + Old: "", + New: "*/15 * * * * *", + }, + { + Type: DiffTypeEdited, + Name: "SpecType", + Old: "foo", + New: "cron", + }, + { + Type: DiffTypeEdited, + Name: "TimeZone", + Old: "Europe/Minsk", + New: "America/Los_Angeles", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeDeleted, + Name: "Specs", + Fields: []*FieldDiff{ + { + Type: DiffTypeDeleted, + Name: "Specs", + Old: "* * * * * *", + New: "", + }, + { + Type: DiffTypeDeleted, + Name: "Specs", + Old: "*/5 * * * * *", + New: "", + }, + }, + }, + }, + }, + }, + }, + }, + { + // Periodic edit multiple times + Old: &Job{ + Periodic: &PeriodicConfig{ + Enabled: false, + Specs: []string{"*/4 * * * * *", "*/6 * * * * *"}, + SpecType: "foo", + ProhibitOverlap: false, + TimeZone: "Europe/Minsk", + }, + }, + New: &Job{ + Periodic: &PeriodicConfig{ + Enabled: true, + Specs: []string{"*/5 * * * * *", "*/7 * * * * *"}, + SpecType: "cron", + ProhibitOverlap: true, + TimeZone: "America/Los_Angeles", + }, + }, + Expected: &JobDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Periodic", + Fields: []*FieldDiff{ + { + Type: DiffTypeEdited, + Name: "Enabled", + Old: "false", + New: "true", + }, + { + Type: DiffTypeEdited, + Name: "ProhibitOverlap", + Old: "false", + New: "true", + }, + { + Type: DiffTypeEdited, + Name: "SpecType", + Old: "foo", + New: "cron", + }, + { + Type: DiffTypeEdited, + Name: "TimeZone", + Old: "Europe/Minsk", + New: "America/Los_Angeles", + }, + }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Specs", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Specs", + Old: "", + New: "*/5 * * * * *", + }, + { + Type: DiffTypeAdded, + Name: "Specs", + Old: "", + New: "*/7 * * * * *", + }, + { + Type: DiffTypeDeleted, + Name: "Specs", + Old: "*/4 * * * * *", + New: "", + }, + { + Type: DiffTypeDeleted, + Name: "Specs", + Old: "*/6 * * * * *", + New: "", + }, + }, + }, + }, + }, + }, + }, + }, { // Periodic edited with context Contextual: true, diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index e50378e36..71bc102ca 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -4797,6 +4797,12 @@ func (j *Job) Warnings() error { mErr.Errors = append(mErr.Errors, err) } + // cron -> crons + if j.Periodic != nil && j.Periodic.Spec != "" { + err := fmt.Errorf("cron is deprecated and may be removed in a future release. Use crons instead") + mErr.Errors = append(mErr.Errors, err) + } + return mErr.ErrorOrNil() } @@ -5578,6 +5584,10 @@ type PeriodicConfig struct { // on the SpecType. Spec string + // Specs specifies the intervals the job should be run as. It is parsed based + // on the SpecType. + Specs []string + // SpecType defines the format of the spec. SpecType string @@ -5610,7 +5620,10 @@ func (p *PeriodicConfig) Validate() error { } var mErr multierror.Error - if p.Spec == "" { + if p.Spec != "" && len(p.Specs) != 0 { + _ = multierror.Append(&mErr, fmt.Errorf("Only cron or crons may be used")) + } + if p.Spec == "" && len(p.Specs) == 0 { _ = multierror.Append(&mErr, fmt.Errorf("Must specify a spec")) } @@ -5624,9 +5637,18 @@ func (p *PeriodicConfig) Validate() error { switch p.SpecType { case PeriodicSpecCron: // Validate the cron spec - if _, err := cronexpr.Parse(p.Spec); err != nil { - _ = multierror.Append(&mErr, fmt.Errorf("Invalid cron spec %q: %v", p.Spec, err)) + if p.Spec != "" { + if _, err := cronexpr.Parse(p.Spec); err != nil { + _ = multierror.Append(&mErr, fmt.Errorf("Invalid cron spec %q: %v", p.Spec, err)) + } } + // Validate the cron specs + for _, spec := range p.Specs { + if _, err := cronexpr.Parse(spec); err != nil { + _ = multierror.Append(&mErr, fmt.Errorf("Invalid cron spec %q: %v", spec, err)) + } + } + case PeriodicSpecTest: // No-op default: @@ -5648,15 +5670,18 @@ func (p *PeriodicConfig) Canonicalize() { // CronParseNext is a helper that parses the next time for the given expression // but captures any panic that may occur in the underlying library. -func CronParseNext(e *cronexpr.Expression, fromTime time.Time, spec string) (t time.Time, err error) { +func CronParseNext(fromTime time.Time, spec string) (t time.Time, err error) { defer func() { if recover() != nil { t = time.Time{} err = fmt.Errorf("failed parsing cron expression: %q", spec) } }() - - return e.Next(fromTime), nil + exp, err := cronexpr.Parse(spec) + if err != nil { + return time.Time{}, fmt.Errorf("failed parsing cron expression: %s: %v", spec, err) + } + return exp.Next(fromTime), nil } // Next returns the closest time instant matching the spec that is after the @@ -5666,11 +5691,24 @@ func CronParseNext(e *cronexpr.Expression, fromTime time.Time, spec string) (t t func (p *PeriodicConfig) Next(fromTime time.Time) (time.Time, error) { switch p.SpecType { case PeriodicSpecCron: - e, err := cronexpr.Parse(p.Spec) - if err != nil { - return time.Time{}, fmt.Errorf("failed parsing cron expression: %q: %v", p.Spec, err) + // Single spec parsing + if p.Spec != "" { + return CronParseNext(fromTime, p.Spec) } - return CronParseNext(e, fromTime, p.Spec) + + // multiple specs parsing + var nextTime time.Time + for _, spec := range p.Specs { + t, err := CronParseNext(fromTime, spec) + if err != nil { + return time.Time{}, fmt.Errorf("failed parsing cron expression %s: %v", spec, err) + } + if nextTime.IsZero() || t.Before(nextTime) { + nextTime = t + } + } + return nextTime, nil + case PeriodicSpecTest: split := strings.Split(p.Spec, ",") if len(split) == 1 && split[0] == "" { diff --git a/ui/app/templates/components/job-page/periodic.hbs b/ui/app/templates/components/job-page/periodic.hbs index cb0a66f06..2c41b8971 100644 --- a/ui/app/templates/components/job-page/periodic.hbs +++ b/ui/app/templates/components/job-page/periodic.hbs @@ -23,9 +23,13 @@ <:after-namespace> - Cron + {{pluralize "Cron" (or @job.periodicDetails.Specs.length 1)}} - {{@job.periodicDetails.Spec}} + {{#each @job.periodicDetails.Specs as |spec|}} + {{spec}} + {{else}} + {{@job.periodicDetails.Spec}} + {{/each}} diff --git a/website/content/api-docs/json-jobs.mdx b/website/content/api-docs/json-jobs.mdx index 59521cc52..f35168cb0 100644 --- a/website/content/api-docs/json-jobs.mdx +++ b/website/content/api-docs/json-jobs.mdx @@ -336,6 +336,14 @@ The `Job` object supports the following keys: [here](https://github.com/gorhill/cronexpr#implementation) for full documentation of supported cron specs and the predefined expressions. + - `Specs` - A list of cron expressions configuring the intervals the job is + launched at. The job runs at the next earliest time that matches any of the + expressions. Supports predefined expressions such as `@daily` and + `@weekly`. Refer to [the + documentation](https://github.com/gorhill/cronexpr#implementation) for full + details about the supported cron specs and the predefined expressions. + Conflicts with `Spec`. + - `ProhibitOverlap` - `ProhibitOverlap` can be set to true to enforce that the periodic job doesn't spawn a new instance of the job if any of the previous jobs are still running. It is defaulted to false. diff --git a/website/content/docs/job-specification/periodic.mdx b/website/content/docs/job-specification/periodic.mdx index e91f04b91..67482d2c4 100644 --- a/website/content/docs/job-specification/periodic.mdx +++ b/website/content/docs/job-specification/periodic.mdx @@ -38,6 +38,14 @@ consistent evaluation when Nomad spans multiple time zones. interval to launch the job. In addition to [cron-specific formats][cron], this option also includes predefined expressions such as `@daily` or `@weekly`. +- `crons` - A list of cron expressions configuring the intervals the job is + launched at. The job runs at the next earliest time that matches any of the + expressions. Supports predefined expressions such as `@daily` and + `@weekly`. Refer to [the + documentation](https://github.com/gorhill/cronexpr#implementation) for full + details about the supported cron specs and the predefined expressions. + Conflicts with `cron`. + - `prohibit_overlap` `(bool: false)` - Specifies if this job should wait until previous instances of this job have completed. This only applies to this job; it does not prevent other periodic jobs from running at the same time.