Schedule doesn't seem to work correctly about pod-reaper HOT 7 CLOSED

Well... my mind is kinda blown right now... Apparently the cron library is using the first value as seconds instead of minutes...

package main

import (
	"fmt"

	"github.com/robfig/cron"
)

func printNext(s string) {
	schedule := cron.New()
	schedule.AddFunc(s, func() {})
	schedule.Start()
	entry := schedule.Entries()[0]
	fmt.Println(entry.Next.String())
}

func main() {
	printNext("1 2 3 4 5")
}

Results in:

2020-05-04 03:02:01 -0500 CDT

And given that the (documentation)[https://godoc.org/github.com/robfig/cron#hdr-Alternative_Formats] says...

Alternative Cron expression formats support other fields like seconds. You can implement that by creating a custom Parser as follows.

And given that I definitely didn't do that (here)[https://github.com/target/pod-reaper/blob/master/reaper/reaper.go#L98] I'm going to have to dig deeper on this one.

For an ultra quick fix, based on what's currently out there, adding a zero to this perplexing "seconds" value should get you going 0 0 20 * * *

func main() {
	printNext("0 20 * * *")
	printNext("0 0 20 * * *")
}

Results in:

2020-03-27 07:20:00 -0500 CDT
2020-03-27 20:00:00 -0500 CDT

More to come on this...

from pod-reaper.

Well... turns out the version of the library that I've been using has a surprise (to me anyway)

var defaultParser = NewParser(
	Second | Minute | Hour | Dom | Month | DowOptional | Descriptor,
)

Also, the defaultParser was removed entirely in (this commit)[https://github.com/robfig/cron/commit/1f8ec97c87338ef2c06af340dcf4289c691c6f95#diff-f615844d3497ff38db57e459d6ef657b]

So now that I know what's going on, it seems like it's just a matter of updating the dependency... just with one small note: I'd like to make sure I don't break anyone currently using that seconds field.

I think that means my best bet forward would be to actually use a customer parser using the optionalSeconds flag provided in the library. My hope is that it will function like this:

• got 6 fields? great, we'll assume you're starting with seconds
• got 5 fields? great, we'll assume you're starting with minutes

from pod-reaper.

Sounds like a plan.

I don't know much about go, but I guess you could pin the version of the cron library to avoid future breaking changes.

Thank you very much for your quick response!

from pod-reaper.

The newest version of that cron dependency has this.

var standardParser = NewParser(
	Minute | Hour | Dom | Month | Dow | Descriptor,
)

I don't really see a way to NOT break this without leaving the seconds value... because (of all things) that previous version had an optional Day of Week value. That's extremely ugly...

Honestly, I think that means that I need need to bite this and get it to where I want it.
PR incoming in not too long.

from pod-reaper.

Guess this helps explain #39 huh? 😄
edit: formatting

from pod-reaper.

Guess I does!

from pod-reaper.

docker images with this:

• target/pod-reaper:latest
• target/pod-reaper:2.6.0

from pod-reaper.