Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cron definition to prow. #1895

Merged
merged 3 commits into from Feb 17, 2017
Merged

Add cron definition to prow. #1895

merged 3 commits into from Feb 17, 2017

Conversation

spxtr
Copy link
Contributor

@spxtr spxtr commented Feb 14, 2017

First commit is from #1891, please only review the second. There is still more cleanup to do in the next PR before I actually write the code that runs the jobs.

@spxtr spxtr added the area/prow Issues or PRs related to prow label Feb 14, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 14, 2017
@@ -46,6 +46,7 @@ var (

presubmit = flag.String("presubmits", "/etc/jobs/presubmit", "Path to presubmit.yaml.")
postsubmit = flag.String("postsubmits", "/etc/jobs/postsubmit", "Path to postsubmit.yaml.")
cron = flag.String("crons", "/etc/jobs/cron", "Path to cron.yaml.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Traditionally, this would be called crontab

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose, but it's not the same format at all. It might be more confusing to call it that...

}
for _, js := range nj {
for j := range js {
d, err := time.ParseDuration(js[j].Duration)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this time between runs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, duration is a lousy name. I'll call it interval instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

This does *not* add any code to actually run the jobs. It's only the
spec and associated plumbing. The plumbing should be cleaned up soon,
since this is a pretty gross commit.
@spxtr
Copy link
Contributor Author

spxtr commented Feb 15, 2017

@rmmh rebased, PTAL

}
for _, js := range nj {
for j := range js {
d, err := time.ParseDuration(js[j].Interval)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious here, seems ParseDuration does not handle cron string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that's why it's "interval". Rather than saying a cron string you say a golang interval, such as 5m or 1h30m.

@rmmh
Copy link
Contributor

rmmh commented Feb 16, 2017

It might be worth not even invoking the word "cron", since it's clearly pretty confusing :-)

If there are "presubmit" and "postsubmit", maybe a "periodic" section of the unified config.yaml would be appropriate?

@spxtr
Copy link
Contributor Author

spxtr commented Feb 16, 2017

It might be worth not even invoking the word "cron", since it's clearly pretty confusing :-)

Done.

@krzyzacy
Copy link
Member

@k8s-bot bazel test this

@rmmh
Copy link
Contributor

rmmh commented Feb 16, 2017

/lgtm

feel free to squash

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 16, 2017
@spxtr spxtr merged commit 85fcd03 into kubernetes:master Feb 17, 2017
@spxtr spxtr deleted the crons branch February 17, 2017 04:38
gmarek pushed a commit to gmarek/test-infra that referenced this pull request Feb 21, 2017
Automatic merge from submit-queue

Don't make subdirs when publishing client-go

Part of the client-go restructuring effort. This PR make publisher not to create subdirectories in client-go.

The latest commit in https://github.com/caesarxuchao/client-go/commits/master is a sample commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/prow Issues or PRs related to prow cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants