diff --git a/nomad/fsm.go b/nomad/fsm.go index 9ab5368d8..4ab6dd108 100644 --- a/nomad/fsm.go +++ b/nomad/fsm.go @@ -425,7 +425,7 @@ func (n *nomadFSM) applyUpsertJob(buf []byte, index uint64) interface{} { // tracking it. if err := n.periodicDispatcher.Add(req.Job); err != nil { n.logger.Printf("[ERR] nomad.fsm: periodicDispatcher.Add failed: %v", err) - return err + return fmt.Errorf("failed adding job to periodic dispatcher: %v", err) } // Create a watch set diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 9923b995f..4acc885ff 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -1219,7 +1219,7 @@ func (j *Job) Plan(args *structs.JobPlanRequest, reply *structs.JobPlanResponse) if args.Job.IsPeriodic() && args.Job.Periodic.Enabled { reply.NextPeriodicLaunch, err = args.Job.Periodic.Next(time.Now().In(args.Job.Periodic.GetLocation())) if err != nil { - return fmt.Errorf("Failed to parse cron time %v", err) + return fmt.Errorf("Failed to parse cron expression: %v", err) } } diff --git a/nomad/leader.go b/nomad/leader.go index ead72778e..a30ea9f2d 100644 --- a/nomad/leader.go +++ b/nomad/leader.go @@ -402,7 +402,8 @@ func (s *Server) restorePeriodicDispatcher() error { // nextLaunch is the next launch that should occur. nextLaunch, err := job.Periodic.Next(launch.Launch.In(job.Periodic.GetLocation())) if err != nil { - return fmt.Errorf("failed to parse periodic time: %v", err) + s.logger.Printf("[ERR] nomad.periodic: failed to determine next periodic launch for job %s: %v", job.NamespacedID(), err) + continue } // We skip force launching the job if there should be no next launch diff --git a/nomad/periodic.go b/nomad/periodic.go index c140cd8e0..05f953189 100644 --- a/nomad/periodic.go +++ b/nomad/periodic.go @@ -223,7 +223,7 @@ func (p *PeriodicDispatch) Add(job *structs.Job) error { p.tracked[tuple] = job next, err := job.Periodic.Next(time.Now().In(job.Periodic.GetLocation())) if err != nil { - return fmt.Errorf("[ERR] nomad.periodic unable to parse cron expression: %v", err) + return fmt.Errorf("failed adding job %s: %v", job.NamespacedID(), err) } if tracked { if err := p.heap.Update(job, next); err != nil { @@ -349,10 +349,9 @@ func (p *PeriodicDispatch) dispatch(job *structs.Job, launchTime time.Time) { nextLaunch, err := job.Periodic.Next(launchTime) if err != nil { - p.logger.Printf("[ERR] nomad.periodic: failed to parse periodic job %v", err) - } - if err := p.heap.Update(job, nextLaunch); err != nil { - p.logger.Printf("[ERR] nomad.periodic: failed to update next launch of periodic job %q (%s): %v", job.ID, job.Namespace, err) + p.logger.Printf("[ERR] nomad.periodic: failed to parse next periodic launch for job %s: %v", job.NamespacedID(), err) + } else if err := p.heap.Update(job, nextLaunch); err != nil { + p.logger.Printf("[ERR] nomad.periodic: failed to update next launch of periodic job %s: %v", job.NamespacedID(), err) } // If the job prohibits overlapping and there are running children, we skip diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 91830e914..a9df935ce 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1997,6 +1997,14 @@ type Job struct { JobModifyIndex uint64 } +// NamespacedID returns the namespaced id useful for logging +func (j *Job) NamespacedID() *NamespacedID { + return &NamespacedID{ + ID: j.ID, + Namespace: j.Namespace, + } +} + // Canonicalize is used to canonicalize fields in the Job. This should be called // when registering a Job. A set of warnings are returned if the job was changed // in anyway that the user should be made aware of. @@ -2649,11 +2657,13 @@ func (p *PeriodicConfig) Canonicalize() { p.location = l } -func cronParseNext(e *cronexpr.Expression, fromTime time.Time) (t time.Time, err error) { +// 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) { defer func() { if recover() != nil { t = time.Time{} - err = fmt.Errorf("Unable to parse cron expression from time") + err = fmt.Errorf("failed parsing cron expression: %q", spec) } }() @@ -2668,7 +2678,7 @@ func (p *PeriodicConfig) Next(fromTime time.Time) (time.Time, error) { switch p.SpecType { case PeriodicSpecCron: if e, err := cronexpr.Parse(p.Spec); err == nil { - return cronParseNext(e, fromTime) + return cronParseNext(e, fromTime, p.Spec) } case PeriodicSpecTest: split := strings.Split(p.Spec, ",") diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 0720b0dfb..49f1ad199 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -2015,7 +2015,10 @@ func TestPeriodicConfig_NextCron(t *testing.T) { for i, spec := range specs { p := &PeriodicConfig{Enabled: true, SpecType: PeriodicSpecCron, Spec: spec} p.Canonicalize() - n := p.Next(from) + n, err := p.Next(from) + if err != nil { + t.Fatalf("Next returned error: %v", err) + } if expected[i] != n { t.Fatalf("Next(%v) returned %v; want %v", from, n, expected[i]) } @@ -2050,8 +2053,14 @@ func TestPeriodicConfig_DST(t *testing.T) { 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() + n1, err1 := p.Next(t1) + n2, err2 := p.Next(t2) + if err1 != nil || err2 != nil { + t.Fatalf("bad: %v %v", err1, err2) + } + + n1 = n1.UTC() + n2 = n2.UTC() if !reflect.DeepEqual(e1, n1) { t.Fatalf("Got %v; want %v", n1, e1)