diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 393d37537bb..c2fc6af51a7 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -2083,8 +2083,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/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 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<