From 03555ab1aee7228f11a341d6bf1075c68dfd9345 Mon Sep 17 00:00:00 2001 From: Maciej Szulik Date: Wed, 14 Sep 2016 15:58:14 +0200 Subject: [PATCH 1/2] bump(github.com/robfig/cron/):783cfcb01fb00c48f740c9de79122bd410294149 --- Godeps/Godeps.json | 4 +- vendor/github.com/robfig/cron/cron.go | 32 +++- vendor/github.com/robfig/cron/parser.go | 191 +++++++++++++++++------- vendor/github.com/robfig/cron/spec.go | 2 +- 4 files changed, 162 insertions(+), 67 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 3b9876b1e68..a892c92e7eb 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -2011,8 +2011,8 @@ }, { "ImportPath": "github.com/robfig/cron", - "Comment": "v1-16-g0f39cf7", - "Rev": "0f39cf7ebc65a602f45692f9894bd6a193faf8fa" + "Comment": "v1-40-g783cfcb", + "Rev": "783cfcb01fb00c48f740c9de79122bd410294149" }, { "ImportPath": "github.com/russross/blackfriday", diff --git a/vendor/github.com/robfig/cron/cron.go b/vendor/github.com/robfig/cron/cron.go index 9c1e6c31a5c..f648e6dee15 100644 --- a/vendor/github.com/robfig/cron/cron.go +++ b/vendor/github.com/robfig/cron/cron.go @@ -19,6 +19,7 @@ type Cron struct { snapshot chan []*Entry running bool ErrorLog *log.Logger + location *time.Location } // Job is an interface for submitted cron jobs. @@ -69,8 +70,13 @@ func (s byTime) Less(i, j int) bool { return s[i].Next.Before(s[j].Next) } -// New returns a new Cron job runner. +// New returns a new Cron job runner, in the Local time zone. func New() *Cron { + return NewWithLocation(time.Now().Location()) +} + +// NewWithLocation returns a new Cron job runner. +func NewWithLocation(location *time.Location) *Cron { return &Cron{ entries: nil, add: make(chan *Entry), @@ -78,6 +84,7 @@ func New() *Cron { snapshot: make(chan []*Entry), running: false, ErrorLog: nil, + location: location, } } @@ -125,8 +132,16 @@ func (c *Cron) Entries() []*Entry { return c.entrySnapshot() } -// Start the cron scheduler in its own go-routine. +// Location gets the time zone location +func (c *Cron) Location() *time.Location { + return c.location +} + +// Start the cron scheduler in its own go-routine, or no-op if already started. func (c *Cron) Start() { + if c.running { + return + } c.running = true go c.run() } @@ -147,7 +162,7 @@ func (c *Cron) runWithRecovery(j Job) { // access to the 'running' state variable. func (c *Cron) run() { // Figure out the next activation times for each entry. - now := time.Now().Local() + now := time.Now().In(c.location) for _, entry := range c.entries { entry.Next = entry.Schedule.Next(now) } @@ -165,8 +180,9 @@ func (c *Cron) run() { effective = c.entries[0].Next } + timer := time.NewTimer(effective.Sub(now)) select { - case now = <-time.After(effective.Sub(now)): + case now = <-timer.C: // Run every entry whose next time was this effective time. for _, e := range c.entries { if e.Next != effective { @@ -174,23 +190,25 @@ func (c *Cron) run() { } go c.runWithRecovery(e.Job) e.Prev = e.Next - e.Next = e.Schedule.Next(effective) + e.Next = e.Schedule.Next(now) } continue case newEntry := <-c.add: c.entries = append(c.entries, newEntry) - newEntry.Next = newEntry.Schedule.Next(time.Now().Local()) + newEntry.Next = newEntry.Schedule.Next(time.Now().In(c.location)) case <-c.snapshot: c.snapshot <- c.entrySnapshot() case <-c.stop: + timer.Stop() return } // 'now' should be updated after newEntry and snapshot cases. - now = time.Now().Local() + now = time.Now().In(c.location) + timer.Stop() } } diff --git a/vendor/github.com/robfig/cron/parser.go b/vendor/github.com/robfig/cron/parser.go index 4224fa93083..d6c73c220b7 100644 --- a/vendor/github.com/robfig/cron/parser.go +++ b/vendor/github.com/robfig/cron/parser.go @@ -2,36 +2,78 @@ package cron import ( "fmt" - "log" "math" "strconv" "strings" "time" ) +// ParseStandard returns a new crontab schedule representing the given standardSpec +// (https://en.wikipedia.org/wiki/Cron). It differs from Parse requiring to always +// pass 5 entries representing: minute, hour, day of month, month and day of week, +// in that order. It returns a descriptive error if the spec is not valid. +// +// It accepts +// - Standard crontab specs, e.g. "* * * * ?" +// - Descriptors, e.g. "@midnight", "@every 1h30m" +func ParseStandard(standardSpec string) (Schedule, error) { + if standardSpec[0] == '@' { + return parseDescriptor(standardSpec) + } + + // Split on whitespace. We require exactly 5 fields. + // (minute) (hour) (day of month) (month) (day of week) + fields := strings.Fields(standardSpec) + if len(fields) != 5 { + return nil, fmt.Errorf("Expected exactly 5 fields, found %d: %s", len(fields), standardSpec) + } + + var err error + field := func(field string, r bounds) uint64 { + if err != nil { + return uint64(0) + } + var bits uint64 + bits, err = getField(field, r) + return bits + } + var ( + minute = field(fields[0], minutes) + hour = field(fields[1], hours) + dayofmonth = field(fields[2], dom) + month = field(fields[3], months) + dayofweek = field(fields[4], dow) + ) + if err != nil { + return nil, err + } + + return &SpecSchedule{ + Second: 1 << seconds.min, + Minute: minute, + Hour: hour, + Dom: dayofmonth, + Month: month, + Dow: dayofweek, + }, nil +} + // Parse returns a new crontab schedule representing the given spec. // It returns a descriptive error if the spec is not valid. // // It accepts // - Full crontab specs, e.g. "* * * * * ?" // - Descriptors, e.g. "@midnight", "@every 1h30m" -func Parse(spec string) (_ Schedule, err error) { - // Convert panics into errors - defer func() { - if recovered := recover(); recovered != nil { - err = fmt.Errorf("%v", recovered) - } - }() - +func Parse(spec string) (Schedule, error) { if spec[0] == '@' { - return parseDescriptor(spec), nil + return parseDescriptor(spec) } // Split on whitespace. We require 5 or 6 fields. // (second) (minute) (hour) (day of month) (month) (day of week, optional) fields := strings.Fields(spec) if len(fields) != 5 && len(fields) != 6 { - log.Panicf("Expected 5 or 6 fields, found %d: %s", len(fields), spec) + return nil, fmt.Errorf("Expected 5 or 6 fields, found %d: %s", len(fields), spec) } // If a sixth field is not provided (DayOfWeek), then it is equivalent to star. @@ -39,39 +81,64 @@ func Parse(spec string) (_ Schedule, err error) { fields = append(fields, "*") } - schedule := &SpecSchedule{ - Second: getField(fields[0], seconds), - Minute: getField(fields[1], minutes), - Hour: getField(fields[2], hours), - Dom: getField(fields[3], dom), - Month: getField(fields[4], months), - Dow: getField(fields[5], dow), + var err error + field := func(field string, r bounds) uint64 { + if err != nil { + return uint64(0) + } + var bits uint64 + bits, err = getField(field, r) + return bits + } + var ( + second = field(fields[0], seconds) + minute = field(fields[1], minutes) + hour = field(fields[2], hours) + dayofmonth = field(fields[3], dom) + month = field(fields[4], months) + dayofweek = field(fields[5], dow) + ) + if err != nil { + return nil, err } - return schedule, nil + return &SpecSchedule{ + Second: second, + Minute: minute, + Hour: hour, + Dom: dayofmonth, + Month: month, + Dow: dayofweek, + }, nil } // getField returns an Int with the bits set representing all of the times that -// the field represents. A "field" is a comma-separated list of "ranges". -func getField(field string, r bounds) uint64 { - // list = range {"," range} +// the field represents or error parsing field value. A "field" is a comma-separated +// list of "ranges". +func getField(field string, r bounds) (uint64, error) { var bits uint64 ranges := strings.FieldsFunc(field, func(r rune) bool { return r == ',' }) for _, expr := range ranges { - bits |= getRange(expr, r) + bit, err := getRange(expr, r) + if err != nil { + return bits, err + } + bits |= bit } - return bits + return bits, nil } // getRange returns the bits indicated by the given expression: // number | number "-" number [ "/" number ] -func getRange(expr string, r bounds) uint64 { - +// or error parsing range. +func getRange(expr string, r bounds) (uint64, error) { var ( start, end, step uint rangeAndStep = strings.Split(expr, "/") lowAndHigh = strings.Split(rangeAndStep[0], "-") singleDigit = len(lowAndHigh) == 1 + err error + zero = uint64(0) ) var extra_star uint64 @@ -80,14 +147,20 @@ func getRange(expr string, r bounds) uint64 { end = r.max extra_star = starBit } else { - start = parseIntOrName(lowAndHigh[0], r.names) + start, err = parseIntOrName(lowAndHigh[0], r.names) + if err != nil { + return zero, err + } switch len(lowAndHigh) { case 1: end = start case 2: - end = parseIntOrName(lowAndHigh[1], r.names) + end, err = parseIntOrName(lowAndHigh[1], r.names) + if err != nil { + return zero, err + } default: - log.Panicf("Too many hyphens: %s", expr) + return zero, fmt.Errorf("Too many hyphens: %s", expr) } } @@ -95,50 +168,56 @@ func getRange(expr string, r bounds) uint64 { case 1: step = 1 case 2: - step = mustParseInt(rangeAndStep[1]) + step, err = mustParseInt(rangeAndStep[1]) + if err != nil { + return zero, err + } // Special handling: "N/step" means "N-max/step". if singleDigit { end = r.max } default: - log.Panicf("Too many slashes: %s", expr) + return zero, fmt.Errorf("Too many slashes: %s", expr) } if start < r.min { - log.Panicf("Beginning of range (%d) below minimum (%d): %s", start, r.min, expr) + return zero, fmt.Errorf("Beginning of range (%d) below minimum (%d): %s", start, r.min, expr) } if end > r.max { - log.Panicf("End of range (%d) above maximum (%d): %s", end, r.max, expr) + return zero, fmt.Errorf("End of range (%d) above maximum (%d): %s", end, r.max, expr) } if start > end { - log.Panicf("Beginning of range (%d) beyond end of range (%d): %s", start, end, expr) + return zero, fmt.Errorf("Beginning of range (%d) beyond end of range (%d): %s", start, end, expr) + } + if step == 0 { + return zero, fmt.Errorf("Step of range should be a positive number: %s", expr) } - return getBits(start, end, step) | extra_star + return getBits(start, end, step) | extra_star, nil } // parseIntOrName returns the (possibly-named) integer contained in expr. -func parseIntOrName(expr string, names map[string]uint) uint { +func parseIntOrName(expr string, names map[string]uint) (uint, error) { if names != nil { if namedInt, ok := names[strings.ToLower(expr)]; ok { - return namedInt + return namedInt, nil } } return mustParseInt(expr) } -// mustParseInt parses the given expression as an int or panics. -func mustParseInt(expr string) uint { +// mustParseInt parses the given expression as an int or returns an error. +func mustParseInt(expr string) (uint, error) { num, err := strconv.Atoi(expr) if err != nil { - log.Panicf("Failed to parse int from %s: %s", expr, err) + return 0, fmt.Errorf("Failed to parse int from %s: %s", expr, err) } if num < 0 { - log.Panicf("Negative number (%d) not allowed: %s", num, expr) + return 0, fmt.Errorf("Negative number (%d) not allowed: %s", num, expr) } - return uint(num) + return uint(num), nil } // getBits sets all bits in the range [min, max], modulo the given step size. @@ -162,10 +241,9 @@ func all(r bounds) uint64 { return getBits(r.min, r.max, 1) | starBit } -// parseDescriptor returns a pre-defined schedule for the expression, or panics -// if none matches. -func parseDescriptor(spec string) Schedule { - switch spec { +// parseDescriptor returns a predefined schedule for the expression, or error if none matches. +func parseDescriptor(descriptor string) (Schedule, error) { + switch descriptor { case "@yearly", "@annually": return &SpecSchedule{ Second: 1 << seconds.min, @@ -174,7 +252,7 @@ func parseDescriptor(spec string) Schedule { Dom: 1 << dom.min, Month: 1 << months.min, Dow: all(dow), - } + }, nil case "@monthly": return &SpecSchedule{ @@ -184,7 +262,7 @@ func parseDescriptor(spec string) Schedule { Dom: 1 << dom.min, Month: all(months), Dow: all(dow), - } + }, nil case "@weekly": return &SpecSchedule{ @@ -194,7 +272,7 @@ func parseDescriptor(spec string) Schedule { Dom: all(dom), Month: all(months), Dow: 1 << dow.min, - } + }, nil case "@daily", "@midnight": return &SpecSchedule{ @@ -204,7 +282,7 @@ func parseDescriptor(spec string) Schedule { Dom: all(dom), Month: all(months), Dow: all(dow), - } + }, nil case "@hourly": return &SpecSchedule{ @@ -214,18 +292,17 @@ func parseDescriptor(spec string) Schedule { Dom: all(dom), Month: all(months), Dow: all(dow), - } + }, nil } const every = "@every " - if strings.HasPrefix(spec, every) { - duration, err := time.ParseDuration(spec[len(every):]) + if strings.HasPrefix(descriptor, every) { + duration, err := time.ParseDuration(descriptor[len(every):]) if err != nil { - log.Panicf("Failed to parse duration %s: %s", spec, err) + return nil, fmt.Errorf("Failed to parse duration %s: %s", descriptor, err) } - return Every(duration) + return Every(duration), nil } - log.Panicf("Unrecognized descriptor: %s", spec) - return nil + return nil, fmt.Errorf("Unrecognized descriptor: %s", descriptor) } diff --git a/vendor/github.com/robfig/cron/spec.go b/vendor/github.com/robfig/cron/spec.go index afa5ac86cc3..2b541269522 100644 --- a/vendor/github.com/robfig/cron/spec.go +++ b/vendor/github.com/robfig/cron/spec.go @@ -108,7 +108,7 @@ WRAP: for 1< Date: Wed, 14 Sep 2016 16:06:21 +0200 Subject: [PATCH 2/2] Remove hacks from ScheduledJobs cron spec parsing Previusly github.com/robfig/cron library did not allow passing cron spec without seconds. Previous commit updates the library, which has additional method ParseStandard which follows the standard cron spec, iow. minute, hour, day of month, month, day of week. --- pkg/apis/batch/validation/validation.go | 7 +------ pkg/controller/scheduledjob/utils.go | 15 ++------------- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/pkg/apis/batch/validation/validation.go b/pkg/apis/batch/validation/validation.go index 24d97a53134..b840f892ae5 100644 --- a/pkg/apis/batch/validation/validation.go +++ b/pkg/apis/batch/validation/validation.go @@ -199,12 +199,7 @@ func validateConcurrencyPolicy(concurrencyPolicy *batch.ConcurrencyPolicy, fldPa func validateScheduleFormat(schedule string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - // TODO soltysh: this should be removed when https://github.com/robfig/cron/issues/58 is fixed - tmpSchedule := schedule - if len(schedule) > 0 && schedule[0] != '@' { - tmpSchedule = "0 " + schedule - } - if _, err := cron.Parse(tmpSchedule); err != nil { + if _, err := cron.ParseStandard(schedule); err != nil { allErrs = append(allErrs, field.Invalid(fldPath, schedule, err.Error())) } diff --git a/pkg/controller/scheduledjob/utils.go b/pkg/controller/scheduledjob/utils.go index 72e10b58a96..185624b38fe 100644 --- a/pkg/controller/scheduledjob/utils.go +++ b/pkg/controller/scheduledjob/utils.go @@ -109,8 +109,7 @@ func getNextStartTimeAfter(schedule string, now time.Time) (time.Time, error) { // How to handle concurrency control. // How to detect changes to schedules or deleted schedules and then // update the jobs? - tmpSched := addSeconds(schedule) - sched, err := cron.Parse(tmpSched) + sched, err := cron.Parse(schedule) if err != nil { return time.Unix(0, 0), fmt.Errorf("Unparseable schedule: %s : %s", schedule, err) } @@ -123,8 +122,7 @@ func getNextStartTimeAfter(schedule string, now time.Time) (time.Time, error) { // If there were missed times prior to the last known start time, then those are not returned. func getRecentUnmetScheduleTimes(sj batch.ScheduledJob, now time.Time) ([]time.Time, error) { starts := []time.Time{} - tmpSched := addSeconds(sj.Spec.Schedule) - sched, err := cron.Parse(tmpSched) + sched, err := cron.ParseStandard(sj.Spec.Schedule) if err != nil { return starts, fmt.Errorf("Unparseable schedule: %s : %s", sj.Spec.Schedule, err) } @@ -172,15 +170,6 @@ func getRecentUnmetScheduleTimes(sj batch.ScheduledJob, now time.Time) ([]time.T return starts, nil } -// TODO soltysh: this should be removed when https://github.com/robfig/cron/issues/58 is fixed -func addSeconds(schedule string) string { - tmpSched := schedule - if len(schedule) > 0 && schedule[0] != '@' { - tmpSched = "0 " + schedule - } - return tmpSched -} - // XXX unit test this // getJobFromTemplate makes a Job from a ScheduledJob