ScheduledJob client #25475

soltysh · 2016-05-11T14:31:00Z

@erictune SheduledJob part 2, based on #24970 so first two commits doesn't count.
This is still WIP, but it's here so you know :)

Introducing ScheduledJobs as described in [the proposal](https://github.com/kubernetes/kubernetes/blob/master/docs/proposals/scheduledjob.md) as part of `batch/v2alpha1` version (experimental feature).

erictune · 2016-05-11T16:37:58Z

pkg/apis/batch/validation/validation.go

+
+func validateScheduleFormat(schedule string, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	_, err := cron.Parse(schedule)


@lavalamp @bgrant0607 is there a limit to how complex of parser we want to include in validation? At some point, is it up to the controller to react to errors in complex fields? In this case, a third-party library is used to parse a cronspec, which is pretty complex. Not a performance issue, I don't think, but maybe a dependency issue.

@soltysh I think adding godep for robfig/cron is fine in this PR. the only thing I am unsure about is whether validation should call Parse, or whether that should be left to the controller to parse, and then raise an event or set a condition if there is an error in the format.

Personally, I'd prefer to know the cron format is wrong at validation and not when the controller will actually try to run it. Although a complex, it's one of those things I'd expect validation to take care.

erictune · 2016-05-11T16:47:27Z

LGTM.

soltysh · 2016-05-11T20:10:19Z

@erictune there are a few bit still missing in this PR, but hopefully they will be addressed first thing in the morning. Once that is done I'll work on another PR adding kubectl elements.

soltysh · 2016-05-11T20:10:51Z

For now I'd like to have the API part to land, there's some release-note-* labeling issue.

soltysh · 2016-05-12T15:55:33Z

@erictune this is still not quite finished. The biggest problem with this PR is that until now there was no multi-version client testing framework in place 😢 My approach in the last commit (Enable multi-version tests in client tests) but that needs @caesarxuchao eyes. If this is good enough for now, I'll continue with additional necessary changes to finish it. ptal

I need to step away for a few hours, but I'll be online later and I'll try to reach out to you and @caesarxuchao about it on slack.

soltysh · 2016-05-12T16:09:11Z

@erictune one more option is to split the testing part into a separate PR (including scheduled jobs, because that needs to be tested only in batch/v2alpha1). IMHO, that would be the best option, since with client part in place we can work in parallel, wdyt?

erictune · 2016-05-12T21:37:28Z

FAILED   ./hack/../hack/verify-godep-licenses.sh    17s
Verifying ./hack/../hack/verify-godeps.sh
Checking for 'Godeps/' changes against 'origin/master'

erictune · 2016-05-12T22:41:08Z

Your godep licenses file is out of date. Run hack/update-godep-licenses.sh --create-missing and commit the results.

soltysh · 2016-05-18T13:19:47Z

@erictune yup, I'm just updating.

soltysh · 2016-05-18T13:43:35Z

Rebased and regenerated. @erictune mind re-applying the label?

philips · 2016-05-18T19:53:14Z

Why aren't scheduled jobs a third party resource and external controller?

soltysh · 2016-05-18T19:58:54Z

@philips we got a lot of questions about that functionality and after talking with everyone involved we agreed it's one of those features that should be part of the core of the system.

philips · 2016-05-18T20:06:38Z

@soltysh Do you have a link to that discussion? I don't see it on the proposal #11980 and the proposal itself doesn't mention it afaics https://github.com/soltysh/kubernetes/blob/10de4c2d51d14c1efa9ae1f9914136960c081475/docs/proposals/scheduledjob.md I am just searching for "third" so probably just missing it.

soltysh · 2016-05-18T20:12:08Z

@philips the discussion was held over several issues both k8s and openshift, you'll need to follow the links from initial comments. But generally it all started from a proposal for ScheduledJobs, actually. From that point the current Jobs arose and ScheduledJobs is just a continuation of the topic (see #11746).

philips · 2016-05-18T20:37:38Z

@soltysh OK, could you perhaps summarize in a few sentences what about this wouldn't fit in the third party resource model? We have done TPM support and a number of prototypes on top of third party and I would love to understand how we avoid adding more and more types over time to the core API. Not urgent so I will make a note to ping everyone post v1.3 code freeze.

philips · 2016-05-18T20:44:53Z

@soltysh The main thing in the back of my mind is that expression of time in cron format is potentially a step back. For example systemd timers[1] introduced a number of different ways of expressing time that users may want and that aren't captured in the ScheduledJob

Run $now, and then once every 24 hours after $now
Run every 8 hours. Essentially sleep 8 hours between completions. In cron you need to say run on 0, 8, 16.

[1] https://wiki.archlinux.org/index.php/Systemd/Timers#Timer_units

soltysh · 2016-05-18T20:58:46Z

@philips the schedule format was initially influenced by https://github.com/mesos/chronos, we thought about using ISO8601 but after some investigation I've realized it's too limiting and we went with the cron as being the most popular. I don't see any problems with supporting multiple schedule formats with it. Thankfully ScheduledJob is still alpha feature so we're able to make some significant changes to the API based on users and our self's feedback

erictune · 2016-05-19T14:05:38Z

@philips @brendandburns

Regarding format:

definitely open to adding ISO 8601 and Google cron schedule format, as they appear to be distinguishable from each other. I guess systemd format could be supported too, if it is distinguishable from the others.
I interviewed a number of Chronos users about their cron needs, and none seemed to love ISO 8601.

Regarding ThirdParty status:

If @Solysh an I can get it done, this is going to be Alpha in 1.3 due to momentum.

From there, I had assumed it would go to Beta. But I guess another option could be to decide to move from alpha to third-party.

We are going to discuss criteria for something being third-party at a future community meeting (@sarahnovotny is trying to pick a date). That would be a good time to bring up the idea of ScheduledJob being ThirdParty.

If someone were to produce a third-party version of scheduledJob, well before 1.4, and show that was comparably useful, that would be persuasive argument for the latter path. I have been thinking about this path while working on the controller. My impression is that it is going to be harder than some people think. But definitely worth trying, so we can improve thirdParty. Interested in hearing what your experience has been working with third party.

philips · 2016-05-19T14:17:43Z

@erictune Sounds great. I will make a note to follow-up with you in a few weeks about what makes schedule jobs hard to do in third party.

soltysh · 2016-05-21T14:34:09Z

Reapplying lgtm label after rebase.

soltysh · 2016-05-21T17:42:51Z

@k8s-bot test this github flake: #21484

k8s-bot · 2016-05-21T20:24:44Z

GCE e2e build/test passed for commit 6510eb5.

k8s-github-robot · 2016-05-21T21:10:41Z

Automatic merge from submit-queue

@erictune

Automatic merge from submit-queue Scheduledjob storage This builds on top of #25475 so only last commit is significant. There's still one small problem with conversions, I'm currently working, but the biggest is still multi-version client tests. Read on... @erictune unfortunately the multi-version tests are biting again, this time in `pkg/registry/scheduledjob/etcd`. If I run the tests with just `batch/v2alpha1` then these are working correctly, but we can't have only `batch/v2alpha1` enabled since `batch/v1` is the preferred version for the entire `batch/` group. We either going to skip the tests here like we did with the scheduledjob client (I'm talking about `pkg/registry/scheduledjob/etcd/etcd_test.go`) or fix it. I've talked with @deads2k about how we can plumb the tests so it just passes, but there's no simple solution we can come up with 😞 (other than the one from #25566), unless @caesarxuchao can chime in and propose something. ```release-note Introducing ScheduledJobs as described in [the proposal](https://github.com/kubernetes/kubernetes/blob/master/docs/proposals/scheduledjob.md) as part of `batch/v2alpha1` version (experimental feature). ``` [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]()

googlebot added the cla: yes label May 11, 2016

k8s-github-robot assigned lavalamp May 11, 2016

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. release-note-label-needed labels May 11, 2016

soltysh force-pushed the scheduledjob_client branch from 61fc32d to 806bb6b Compare May 11, 2016 15:47

erictune assigned erictune and unassigned lavalamp May 11, 2016

erictune reviewed May 11, 2016
View reviewed changes

erictune added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels May 11, 2016

erictune added this to the v1.3 milestone May 11, 2016

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2016

soltysh force-pushed the scheduledjob_client branch from 806bb6b to 812df20 Compare May 12, 2016 15:50

soltysh force-pushed the scheduledjob_client branch from 812df20 to e4cfe38 Compare May 12, 2016 21:03

soltysh changed the title ~~[WIP] ScheduledJob client~~ ScheduledJob client May 12, 2016

erictune added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 12, 2016

soltysh force-pushed the scheduledjob_client branch from e4cfe38 to a5615ee Compare May 13, 2016 05:30

soltysh force-pushed the scheduledjob_client branch from 08626bf to 85ecc00 Compare May 18, 2016 13:43

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 18, 2016

erictune added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 19, 2016

soltysh added 5 commits May 21, 2016 15:48

ScheduledJob api leftovers

70e5456

ScheduledJob api leftovers - generated

5c9b4bf

Adding github.com/robfig/cron for cron parsing

aabf86a

ScheduledJob validation & defaults

adc890a

ScheduledJob client

6510eb5

soltysh force-pushed the scheduledjob_client branch from 85ecc00 to 6510eb5 Compare May 21, 2016 14:31

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 21, 2016

soltysh added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 21, 2016

k8s-github-robot merged commit 4d69e2c into kubernetes:master May 21, 2016

soltysh deleted the scheduledjob_client branch May 21, 2016 21:22

soltysh mentioned this pull request Jul 4, 2016

CronJobs (previously ScheduledJobs) kubernetes/enhancements#19

Closed

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ScheduledJob client #25475

ScheduledJob client #25475

soltysh commented May 11, 2016

erictune May 11, 2016

erictune May 11, 2016

soltysh May 11, 2016

erictune commented May 11, 2016

soltysh commented May 11, 2016

soltysh commented May 11, 2016

soltysh commented May 12, 2016

soltysh commented May 12, 2016

erictune commented May 12, 2016

erictune commented May 12, 2016

soltysh commented May 18, 2016

soltysh commented May 18, 2016

philips commented May 18, 2016

soltysh commented May 18, 2016

philips commented May 18, 2016

soltysh commented May 18, 2016

philips commented May 18, 2016

philips commented May 18, 2016

soltysh commented May 18, 2016

erictune commented May 19, 2016

philips commented May 19, 2016

soltysh commented May 21, 2016

soltysh commented May 21, 2016

k8s-bot commented May 21, 2016

k8s-github-robot commented May 21, 2016

ScheduledJob client #25475

ScheduledJob client #25475

Conversation

soltysh commented May 11, 2016

erictune May 11, 2016

Choose a reason for hiding this comment

erictune May 11, 2016

Choose a reason for hiding this comment

soltysh May 11, 2016

Choose a reason for hiding this comment

erictune commented May 11, 2016

soltysh commented May 11, 2016

soltysh commented May 11, 2016

soltysh commented May 12, 2016

soltysh commented May 12, 2016

erictune commented May 12, 2016

erictune commented May 12, 2016

soltysh commented May 18, 2016

soltysh commented May 18, 2016

philips commented May 18, 2016

soltysh commented May 18, 2016

philips commented May 18, 2016

soltysh commented May 18, 2016

philips commented May 18, 2016

philips commented May 18, 2016

soltysh commented May 18, 2016

erictune commented May 19, 2016

philips commented May 19, 2016

soltysh commented May 21, 2016

soltysh commented May 21, 2016

k8s-bot commented May 21, 2016

k8s-github-robot commented May 21, 2016