Skip to content

ScheduledJob controller proposal - #11980

Merged
erictune merged 1 commit into
kubernetes:masterfrom
soltysh:scheduledjob_controller_proposal
Jan 25, 2016
Merged

erictune merged 1 commit into
kubernetes:masterfrom
soltysh:scheduledjob_controller_proposal

Conversation

@soltysh

@soltysh soltysh commented Jul 29, 2015

Copy link
Copy Markdown
Contributor

@k8s-bot

k8s-bot commented Jul 29, 2015

Copy link
Copy Markdown

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

@bgrant0607 bgrant0607 added the area/api Indicates an issue on api area. label Jul 30, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment still holds on name

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The problem with "Cron" is that is technically focused (a particular linux
tool) but is opaque to higher level users who come from different
backgrounds.

Even the Chronos GitHub page describes it with "It allows you to schedule
your jobs using ISO8601 repeating interval notation, which enables more
flexibility in job scheduling". RepeatingJob or CalendarJob or
RecurringJob all seem less clear to an outside user (someone who is not an
expert at these systems) than ScheduledJob.

On Fri, Jul 31, 2015 at 12:41 PM, Timothy St. Clair <
notifications@github.com> wrote:

In docs/proposals/scheduledjob.md
#11980 (comment)
:

@@ -0,0 +1,132 @@
+# ScheduledJob Controller

Comment still holds on name


Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/11980/files#r35991090
.

Clayton Coleman | Lead Engineer, OpenShift

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Proposal was around (diurnal, periodic, etc.) There may be a couple more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Periodic suffers some of the same usability concerns. I do think API
objects should be precise in their meaning, but designed for end users, not
systems researchers.

On Fri, Jul 31, 2015 at 1:02 PM, Timothy St. Clair <notifications@github.com

wrote:

In docs/proposals/scheduledjob.md
#11980 (comment)
:

@@ -0,0 +1,132 @@
+# ScheduledJob Controller

Proposal was around (diurnal, periodic, etc.) There may be a couple more.


Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/11980/files#r35992880
.

Clayton Coleman | Lead Engineer, OpenShift

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm fine with ScheduledJob. It's similar to ScheduledExecutorService in Java.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could also call it CalendaredJob

@erictune

erictune commented Aug 5, 2015

Copy link
Copy Markdown
Contributor

When you implement this please put in expapi. (#12001).

@soltysh

soltysh commented Aug 11, 2015

Copy link
Copy Markdown
Contributor Author

@erictune gotcha.

Comment thread docs/proposals/scheduledjob.md Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Much of the same feedback applies to this API as to Job.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'll address them as soon as we have consensus on Job API, which I'm hoping will happen withing next couple of days :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My feedback re: Job API and overall status applies here too.

@soltysh
soltysh force-pushed the scheduledjob_controller_proposal branch from 463be2b to 15a69e4 Compare August 18, 2015 12:51
@soltysh

soltysh commented Aug 18, 2015

Copy link
Copy Markdown
Contributor Author

@bgrant0607 @erictune I've updated the proposal, cutting the status structure significantly. I've only left there a list of currently executing jobs, which is needed to track these executions and fulfill the defined ConcurrentPolicy.

@soltysh

soltysh commented Aug 18, 2015

Copy link
Copy Markdown
Contributor Author

@derekwaynecarr @smarterclayton ptal as well

Comment thread docs/proposals/scheduledjob.md Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not a selector query? (you'd have to add a selector to ScheduledJobSpec)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How would you differentiate concurrent runs? We'd have to generate some random uuid for that selector in that case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The selector should select all Jobs created by the ScheduledJob. We can sort them by CreationTimestamp. What do you mean differentiate? How does having an array help you differentiate?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I haven't thought about sorting them by CreationTimestamp, which generally speaking allows me to distinguish (that's the word I've meant to use, sorry :/) different ScheduledJob executions/runs needed to properly handle ConcurrentPolicy.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorting by CreationTimestamp seems exactly what the user would want, since ScheduledJobs are launched based on time.

Please no array.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Array removed, added selector to ScheduledJobSpec.

Comment thread docs/proposals/scheduledjob.md Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If these accumulate indefinitely, they should probably be int64.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@soltysh
soltysh force-pushed the scheduledjob_controller_proposal branch from 15a69e4 to e234d1f Compare August 25, 2015 12:43
@soltysh

soltysh commented Aug 25, 2015

Copy link
Copy Markdown
Contributor Author

@mikedanese @bgrant0607 addressed your comments, appreciate another round of reviews.

@k8s-bot

k8s-bot commented Aug 25, 2015

Copy link
Copy Markdown

GCE e2e build/test passed for commit e234d1f91b25a3bb16d4412459cd761416a2b248.

@k8s-github-robot k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 27, 2015
@k8s-github-robot

Copy link
Copy Markdown

Labelling this PR as size/L

@mikedanese

Copy link
Copy Markdown
Member

Let's continue this review targeting v1.2

@soltysh
soltysh force-pushed the scheduledjob_controller_proposal branch from b00be17 to 10de4c2 Compare January 14, 2016 11:10
@soltysh

soltysh commented Jan 14, 2016

Copy link
Copy Markdown
Contributor Author

@erictune as requested:

  • removed pointer from Active
  • made JobTemplate pointer
  • made JobTemplate be JobTemplateSpec type
  • moved label generation to job (new section about JobSpec modifications)

I've also:

  • fixed all the ISO8601 leftovers
  • extended future evolution with proper links to workflow and templating

It's all squashed and waiting your sing-off.

@k8s-bot

k8s-bot commented Jan 14, 2016

Copy link
Copy Markdown

GCE e2e test build/test passed for commit 10de4c2.

@erictune erictune added e2e-not-required lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jan 16, 2016
@erictune

Copy link
Copy Markdown
Contributor

This looks great. LGTM.

@soltysh

soltysh commented Jan 16, 2016

Copy link
Copy Markdown
Contributor Author

Thanks for your help!

@alex-mohr

Copy link
Copy Markdown
Contributor

@k8s-bot unit test this

@soltysh

soltysh commented Jan 25, 2016

Copy link
Copy Markdown
Contributor Author

@erictune any chance on having this one merged?

@erictune

Copy link
Copy Markdown
Contributor

Merging because this is just a new doc. Build failure is known flaky test.

erictune added a commit that referenced this pull request Jan 25, 2016
@erictune
erictune merged commit 99f301d into kubernetes:master Jan 25, 2016
@erictune

Copy link
Copy Markdown
Contributor

@soltysh merged.

@soltysh

soltysh commented Jan 25, 2016

Copy link
Copy Markdown
Contributor Author

@erictune thank you!

@soltysh
soltysh deleted the scheduledjob_controller_proposal branch January 26, 2016 08:37
@yuvipanda

Copy link
Copy Markdown
Contributor

Is this being implemented anywhere?

@soltysh

soltysh commented Feb 27, 2016

Copy link
Copy Markdown
Contributor Author

Not yet. I'm planning to start working on it after 1.2 is released.
On Feb 27, 2016 3:25 AM, "Yuvi Panda" notifications@github.com wrote:

Is this being implemented anywhere?


Reply to this email directly or view it on GitHub
#11980 (comment)
.

@haizaar

haizaar commented Mar 25, 2016

Copy link
Copy Markdown

Jobs guide says that "cron is expected in the next minor release". Do you plan this for 1.3 or 1.2.x?

@erictune

Copy link
Copy Markdown
Contributor

1.3

@zacharynevin

Copy link
Copy Markdown

What is the projected release date for v1.3?

@sdminonne

Copy link
Copy Markdown
Contributor

@soltysh soltysh mentioned this pull request Apr 29, 2016
@philips philips mentioned this pull request May 18, 2016
@wernight

wernight commented Oct 11, 2016

Copy link
Copy Markdown

Looks like documentation on http://kubernetes.io/docs/user-guide/jobs/#future-work didn't get updated (still pointing here). Should probably contain some of http://kubernetes.io/docs/user-guide/scheduled-jobs/

Also the reference documentation seems outdated http://kubernetes.io/docs/api-reference/batch/v1/definitions/

Last, but not least, the example on http://kubernetes.io/docs/user-guide/scheduled-jobs/#writing-a-scheduled-job-spec uses two non-standard notations: ? and 0/1 (not */1) which are not explained.

xingzhou pushed a commit to xingzhou/kubernetes that referenced this pull request Dec 15, 2016
…ler_proposal

ScheduledJob controller proposal
mhan8796 pushed a commit to mhan8796/kubernetes that referenced this pull request Jun 26, 2026
…ler_proposal

ScheduledJob controller proposal
sanyamk23 pushed a commit to sanyamk23/kubernetes that referenced this pull request Sep 1, 2026
…netes#11980)

Adds `metricSystemErrorsForOperations` and `metricThrottledRequests` to the `ITable` interface. Previously they were just in the `Table` class.

Thx @SomayaB and @skinny85 for highlighting the fix.

Fixes kubernetes#11541


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api Indicates an issue on api area. kind/design Categorizes issue or PR as related to design. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.