From 7e918003bad2c73d1635ddae9698d1c1433430af Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 15 Feb 2017 14:37:06 -0800 Subject: [PATCH 1/3] Allow specification of timezones --- api/jobs.go | 9 ++++ command/plan.go | 3 +- command/run.go | 2 +- command/status.go | 3 +- jobspec/parse.go | 1 + jobspec/parse_test.go | 1 + jobspec/test-fixtures/periodic-cron.hcl | 1 + nomad/job_endpoint.go | 2 +- nomad/periodic.go | 6 +-- nomad/structs/structs.go | 51 +++++++++++++++++-- nomad/structs/structs_test.go | 48 +++++++++++++++++ website/source/docs/http/json-jobs.html.md | 6 +++ .../docs/job-specification/periodic.html.md | 5 ++ 13 files changed, 127 insertions(+), 11 deletions(-) diff --git a/api/jobs.go b/api/jobs.go index edbce83e1..86df9ee95 100644 --- a/api/jobs.go +++ b/api/jobs.go @@ -211,6 +211,15 @@ type PeriodicConfig struct { Spec string SpecType string ProhibitOverlap bool + TimeZone *string +} + +func (p *PeriodicConfig) GetLocation() (*time.Location, error) { + if p.TimeZone == nil || *p.TimeZone == "" { + return time.UTC, nil + } + + return time.LoadLocation(*p.TimeZone) } // ParameterizedJobConfig is used to configure the parameterized job. diff --git a/command/plan.go b/command/plan.go index 99dd11e4f..06d490530 100644 --- a/command/plan.go +++ b/command/plan.go @@ -218,8 +218,9 @@ func formatDryRun(resp *api.JobPlanResponse, job *structs.Job) string { } if next := resp.NextPeriodicLaunch; !next.IsZero() { + now := time.Now().In(job.Periodic.GetLocation()) out += fmt.Sprintf("[green]- If submitted now, next periodic launch would be at %s (%s from now).\n", - formatTime(next), formatTimeDifference(time.Now().UTC(), next, time.Second)) + formatTime(next), formatTimeDifference(now, next, time.Second)) } out = strings.TrimSuffix(out, "\n") diff --git a/command/run.go b/command/run.go index 0efcb0e72..68c1df340 100644 --- a/command/run.go +++ b/command/run.go @@ -243,7 +243,7 @@ func (c *RunCommand) Run(args []string) int { if detach || periodic || paramjob { c.Ui.Output("Job registration successful") if periodic { - now := time.Now().UTC() + now := time.Now().In(job.Periodic.GetLocation()) next := job.Periodic.Next(now) c.Ui.Output(fmt.Sprintf("Approximate next launch time: %s (%s from now)", formatTime(next), formatTimeDifference(now, next, time.Second))) diff --git a/command/status.go b/command/status.go index 0bfc7427d..5d64bc4e1 100644 --- a/command/status.go +++ b/command/status.go @@ -139,6 +139,7 @@ func (c *StatusCommand) Run(args []string) int { c.Ui.Error(fmt.Sprintf("Error converting job: %s", err)) return 1 } + sJob.Canonicalize() periodic := sJob.IsPeriodic() parameterized := sJob.IsParameterized() @@ -155,7 +156,7 @@ func (c *StatusCommand) Run(args []string) int { } if periodic { - now := time.Now().UTC() + now := time.Now().In(sJob.Periodic.GetLocation()) next := sJob.Periodic.Next(now) basic = append(basic, fmt.Sprintf("Next Periodic Launch|%s", fmt.Sprintf("%s (%s from now)", diff --git a/jobspec/parse.go b/jobspec/parse.go index 628e17787..1f6388103 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -1172,6 +1172,7 @@ func parsePeriodic(result **structs.PeriodicConfig, list *ast.ObjectList) error "enabled", "cron", "prohibit_overlap", + "time_zone", } if err := checkHCLKeys(o.Val, valid); err != nil { return err diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 068618d96..862e68f91 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -334,6 +334,7 @@ func TestParse(t *testing.T) { SpecType: structs.PeriodicSpecCron, Spec: "*/5 * * *", ProhibitOverlap: true, + TimeZone: "Europe/Minsk", }, }, false, diff --git a/jobspec/test-fixtures/periodic-cron.hcl b/jobspec/test-fixtures/periodic-cron.hcl index f2116459f..e97711546 100644 --- a/jobspec/test-fixtures/periodic-cron.hcl +++ b/jobspec/test-fixtures/periodic-cron.hcl @@ -2,5 +2,6 @@ job "foo" { periodic { cron = "*/5 * * *" prohibit_overlap = true + time_zone = "Europe/Minsk" } } diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index ce25d1ab2..29f196d57 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -687,7 +687,7 @@ func (j *Job) Plan(args *structs.JobPlanRequest, reply *structs.JobPlanResponse) // If it is a periodic job calculate the next launch if args.Job.IsPeriodic() && args.Job.Periodic.Enabled { - reply.NextPeriodicLaunch = args.Job.Periodic.Next(time.Now().UTC()) + reply.NextPeriodicLaunch = args.Job.Periodic.Next(time.Now().In(args.Job.Periodic.GetLocation())) } reply.FailedTGAllocs = updatedEval.FailedTGAllocs diff --git a/nomad/periodic.go b/nomad/periodic.go index b06267585..a629427a7 100644 --- a/nomad/periodic.go +++ b/nomad/periodic.go @@ -209,7 +209,7 @@ func (p *PeriodicDispatch) Add(job *structs.Job) error { // Add or update the job. p.tracked[job.ID] = job - next := job.Periodic.Next(time.Now().UTC()) + next := job.Periodic.Next(time.Now().In(job.Periodic.GetLocation())) if tracked { if err := p.heap.Update(job, next); err != nil { return fmt.Errorf("failed to update job %v launch time: %v", job.ID, err) @@ -289,7 +289,7 @@ func (p *PeriodicDispatch) ForceRun(jobID string) (*structs.Evaluation, error) { } p.l.Unlock() - return p.createEval(job, time.Now().UTC()) + return p.createEval(job, time.Now().In(job.Periodic.GetLocation())) } // shouldRun returns whether the long lived run function should run. @@ -309,7 +309,7 @@ func (p *PeriodicDispatch) run() { if launch.IsZero() { launchCh = nil } else { - launchDur := launch.Sub(time.Now().UTC()) + launchDur := launch.Sub(time.Now().In(job.Periodic.GetLocation())) launchCh = time.After(launchDur) p.logger.Printf("[DEBUG] nomad.periodic: launching job %q in %s", job.ID, launchDur) } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index c75246a09..aef6b6772 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1175,6 +1175,10 @@ func (j *Job) Canonicalize() { if j.ParameterizedJob != nil { j.ParameterizedJob.Canonicalize() } + + if j.Periodic != nil { + j.Periodic.Canonicalize() + } } // Copy returns a deep copy of the Job. It is expected that callers use recover. @@ -1542,6 +1546,16 @@ type PeriodicConfig struct { // ProhibitOverlap enforces that spawned jobs do not run in parallel. ProhibitOverlap bool `mapstructure:"prohibit_overlap"` + + // TimeZone is the user specified string that determines the time zone to + // launch against. The time zones must be specified from IANA Time Zone + // database, such as "America/New_York". + // Reference: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones + // Reference: https://www.iana.org/time-zones + TimeZone string `mapstructure:"time_zone"` + + // location is the time zone to evaluate the launch time against + location *time.Location } func (p *PeriodicConfig) Copy() *PeriodicConfig { @@ -1558,23 +1572,41 @@ func (p *PeriodicConfig) Validate() error { return nil } + var mErr multierror.Error if p.Spec == "" { - return fmt.Errorf("Must specify a spec") + multierror.Append(&mErr, fmt.Errorf("Must specify a spec")) + } + + // Check if we got a valid time zone + if p.TimeZone != "" { + if _, err := time.LoadLocation(p.TimeZone); err != nil { + multierror.Append(&mErr, fmt.Errorf("Invalid time zone %q: %v", p.TimeZone, err)) + } } switch p.SpecType { case PeriodicSpecCron: // Validate the cron spec if _, err := cronexpr.Parse(p.Spec); err != nil { - return fmt.Errorf("Invalid cron spec %q: %v", p.Spec, err) + multierror.Append(&mErr, fmt.Errorf("Invalid cron spec %q: %v", p.Spec, err)) } case PeriodicSpecTest: // No-op default: - return fmt.Errorf("Unknown periodic specification type %q", p.SpecType) + multierror.Append(&mErr, fmt.Errorf("Unknown periodic specification type %q", p.SpecType)) } - return nil + return mErr.ErrorOrNil() +} + +func (p *PeriodicConfig) Canonicalize() { + // Load the location + l, err := time.LoadLocation(p.TimeZone) + if err != nil { + panic(fmt.Sprintf("Bad location %q: %v", p.TimeZone, err)) + } + + p.location = l } // Next returns the closest time instant matching the spec that is after the @@ -1615,6 +1647,17 @@ func (p *PeriodicConfig) Next(fromTime time.Time) time.Time { return time.Time{} } +// GetLocation returns the location to use for determining the time zone to run +// the periodic job against. +func (p *PeriodicConfig) GetLocation() *time.Location { + // Jobs pre 0.5.5 will not have this + if p.location != nil { + return p.location + } + + return time.UTC +} + const ( // PeriodicLaunchSuffix is the string appended to the periodic jobs ID // when launching derived instances of it. diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 168171791..55baf9270 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -1217,12 +1217,19 @@ func TestPeriodicConfig_EnabledInvalid(t *testing.T) { if err := p.Validate(); err == nil { t.Fatal("Enabled PeriodicConfig with no spec shouldn't be valid") } + + // Create a config that is enabled, with a bad time zone. + p = &PeriodicConfig{Enabled: true, TimeZone: "FOO"} + if err := p.Validate(); err == nil || !strings.Contains(err.Error(), "time zone") { + t.Fatal("Enabled PeriodicConfig with bad time zone shouldn't be valid: %v", err) + } } func TestPeriodicConfig_InvalidCron(t *testing.T) { specs := []string{"foo", "* *", "@foo"} for _, spec := range specs { p := &PeriodicConfig{Enabled: true, SpecType: PeriodicSpecCron, Spec: spec} + p.Canonicalize() if err := p.Validate(); err == nil { t.Fatal("Invalid cron spec") } @@ -1233,6 +1240,7 @@ func TestPeriodicConfig_ValidCron(t *testing.T) { specs := []string{"0 0 29 2 *", "@hourly", "0 0-15 * * *"} for _, spec := range specs { p := &PeriodicConfig{Enabled: true, SpecType: PeriodicSpecCron, Spec: spec} + p.Canonicalize() if err := p.Validate(); err != nil { t.Fatal("Passed valid cron") } @@ -1245,6 +1253,7 @@ func TestPeriodicConfig_NextCron(t *testing.T) { expected := []time.Time{time.Time{}, time.Date(2009, time.November, 10, 23, 25, 0, 0, time.UTC)} for i, spec := range specs { p := &PeriodicConfig{Enabled: true, SpecType: PeriodicSpecCron, Spec: spec} + p.Canonicalize() n := p.Next(from) if expected[i] != n { t.Fatalf("Next(%v) returned %v; want %v", from, n, expected[i]) @@ -1252,6 +1261,45 @@ func TestPeriodicConfig_NextCron(t *testing.T) { } } +func TestPeriodicConfig_ValidTimeZone(t *testing.T) { + zones := []string{"Africa/Abidjan", "America/Chicago", "Europe/Minsk", "UTC"} + for _, zone := range zones { + p := &PeriodicConfig{Enabled: true, SpecType: PeriodicSpecCron, Spec: "0 0 29 2 * 1980", TimeZone: zone} + p.Canonicalize() + if err := p.Validate(); err != nil { + t.Fatal("Valid tz errored: %v", err) + } + } +} + +func TestPeriodicConfig_DST(t *testing.T) { + // On Sun, Mar 12, 2:00 am 2017: +1 hour UTC + p := &PeriodicConfig{ + Enabled: true, + SpecType: PeriodicSpecCron, + Spec: "0 2 11-12 3 * 2017", + TimeZone: "America/Los_Angeles", + } + p.Canonicalize() + + t1 := time.Date(2017, time.March, 11, 1, 0, 0, 0, p.location) + t2 := time.Date(2017, time.March, 12, 1, 0, 0, 0, p.location) + + // E1 is an 8 hour adjustment, E2 is a 7 hour adjustment + e1 := time.Date(2017, time.March, 11, 10, 0, 0, 0, time.UTC) + e2 := time.Date(2017, time.March, 12, 9, 0, 0, 0, time.UTC) + + n1 := p.Next(t1).UTC() + n2 := p.Next(t2).UTC() + + if !reflect.DeepEqual(e1, n1) { + t.Fatalf("Got %v; want %v", n1, e1) + } + if !reflect.DeepEqual(e2, n2) { + t.Fatalf("Got %v; want %v", n1, e1) + } +} + func TestRestartPolicy_Validate(t *testing.T) { // Policy with acceptable restart options passes p := &RestartPolicy{ diff --git a/website/source/docs/http/json-jobs.html.md b/website/source/docs/http/json-jobs.html.md index 48f600925..79f2eda44 100644 --- a/website/source/docs/http/json-jobs.html.md +++ b/website/source/docs/http/json-jobs.html.md @@ -265,6 +265,12 @@ The `Job` object supports the following keys: * `Enabled` - `Enabled` determines whether the periodic job will spawn child jobs. + * `time_zone` - Specifies the time zone to evaluate the next launch interval + against. This is useful when wanting to account for day light savings in + various time zones. The time zone must be parsable by Golang's + [LoadLocation](https://golang.org/pkg/time/#LoadLocation). The default is + UTC. + * `SpecType` - `SpecType` determines how Nomad is going to interpret the periodic expression. `cron` is the only supported `SpecType` currently. diff --git a/website/source/docs/job-specification/periodic.html.md b/website/source/docs/job-specification/periodic.html.md index 4f480448f..414402f19 100644 --- a/website/source/docs/job-specification/periodic.html.md +++ b/website/source/docs/job-specification/periodic.html.md @@ -49,6 +49,11 @@ consistent evaluation when Nomad spans multiple time zones. 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. +- `time_zone` `(string: "UTC")` - Specifies the time zone to evaluate the next + launch interval against. This is useful when wanting to account for day light + savings in various time zones. The time zone must be parsable by Golang's + [LoadLocation](https://golang.org/pkg/time/#LoadLocation). + ## `periodic` Examples The following examples only show the `periodic` stanzas. Remember that the From 471d63d5ff2106cc5ba6079857c5e03abbe0d00d Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 15 Feb 2017 15:23:29 -0800 Subject: [PATCH 2/3] Fix diff --- nomad/structs/diff_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 5bf1c0a24..dd1a335c9 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -515,6 +515,7 @@ func TestJobDiff(t *testing.T) { Spec: "*/15 * * * * *", SpecType: "foo", ProhibitOverlap: false, + TimeZone: "Europe/Minsk", }, }, Expected: &JobDiff{ @@ -548,6 +549,12 @@ func TestJobDiff(t *testing.T) { Old: "", New: "foo", }, + { + Type: DiffTypeAdded, + Name: "TimeZone", + Old: "", + New: "Europe/Minsk", + }, }, }, }, @@ -561,6 +568,7 @@ func TestJobDiff(t *testing.T) { Spec: "*/15 * * * * *", SpecType: "foo", ProhibitOverlap: false, + TimeZone: "Europe/Minsk", }, }, New: &Job{}, @@ -595,6 +603,12 @@ func TestJobDiff(t *testing.T) { Old: "foo", New: "", }, + { + Type: DiffTypeDeleted, + Name: "TimeZone", + Old: "Europe/Minsk", + New: "", + }, }, }, }, @@ -608,6 +622,7 @@ func TestJobDiff(t *testing.T) { Spec: "*/15 * * * * *", SpecType: "foo", ProhibitOverlap: false, + TimeZone: "Europe/Minsk", }, }, New: &Job{ @@ -616,6 +631,7 @@ func TestJobDiff(t *testing.T) { Spec: "* * * * * *", SpecType: "cron", ProhibitOverlap: true, + TimeZone: "America/Los_Angeles", }, }, Expected: &JobDiff{ @@ -649,6 +665,12 @@ func TestJobDiff(t *testing.T) { Old: "foo", New: "cron", }, + { + Type: DiffTypeEdited, + Name: "TimeZone", + Old: "Europe/Minsk", + New: "America/Los_Angeles", + }, }, }, }, @@ -663,6 +685,7 @@ func TestJobDiff(t *testing.T) { Spec: "*/15 * * * * *", SpecType: "foo", ProhibitOverlap: false, + TimeZone: "Europe/Minsk", }, }, New: &Job{ @@ -671,6 +694,7 @@ func TestJobDiff(t *testing.T) { Spec: "* * * * * *", SpecType: "foo", ProhibitOverlap: false, + TimeZone: "Europe/Minsk", }, }, Expected: &JobDiff{ @@ -704,6 +728,12 @@ func TestJobDiff(t *testing.T) { Old: "foo", New: "foo", }, + { + Type: DiffTypeNone, + Name: "TimeZone", + Old: "Europe/Minsk", + New: "Europe/Minsk", + }, }, }, }, From 733038bc6c3cff46fe7b71554fbc21a53eb26fc7 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 17 Feb 2017 11:21:49 -0800 Subject: [PATCH 3/3] Remove panic --- nomad/structs/structs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index aef6b6772..1ba453a3f 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1603,7 +1603,7 @@ func (p *PeriodicConfig) Canonicalize() { // Load the location l, err := time.LoadLocation(p.TimeZone) if err != nil { - panic(fmt.Sprintf("Bad location %q: %v", p.TimeZone, err)) + p.location = time.UTC } p.location = l