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.

Sign up for GitHub

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

Jump to bottom

Scheduledjob api #24970

Merged
merged 2 commits into from
May 12, 2016
Merged

Scheduledjob api #24970

merged 2 commits into from
May 12, 2016

Conversation

soltysh
Copy link
Contributor

@soltysh soltysh commented Apr 29, 2016

@erictune ScheduledJob api types, based on #21675, so only last two commits counts.
@sdminonne fyi

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

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/new-api size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 29, 2016
@soltysh
Copy link
Contributor Author

soltysh commented Apr 29, 2016

The tests are failing apparently due to relying on older master. I'll rebase this PR once #21675 merges.

@soltysh
Copy link
Contributor Author

soltysh commented Apr 29, 2016

Rebase was clean, so I did it.

derekwaynecarr
derekwaynecarr reviewed Apr 29, 2016
View reviewed changes
pkg/apis/batch/types.go

// Suspend flag tells the controller to suspend subsequent executions, it does
// not apply to already started executions. Defaults to false.
Suspend bool `json: "suspend"`
Copy link
Member

Choose a reason for hiding this comment

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

I like this field for other controllers too... is this the same idea as inert objects generally?

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'm not familiar with the idea you've mentioned, can you point me to a doc? The idea of having a scheduledjob suspended came out during fleshing out the details for ScheduledJobs what it should allow. Basically in this world you can suspend further job that might start, until some later time, when eg. your server will be less busy or whatever else.

Copy link
Member

Choose a reason for hiding this comment

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

i was interested in the idea of a replica set w/ size=0 supporting a similar suspend flag which would mean that controllers could effectively ignore it. I will look up the old discussion, but here was a similar concept: #170 (comment)

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 think I remember that concept, so no this is completely different idea. Some discussion wrt suspend you can find in the proposal PR: #11980

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 30, 2016
@ixdy ixdy removed their assignment May 2, 2016
@erictune erictune assigned erictune and unassigned bgrant0607 May 4, 2016
erictune
erictune reviewed May 4, 2016
View reviewed changes
pkg/apis/batch/types.go
// ScheduledJobStatus represents the current state of a Job.
type ScheduledJobStatus struct {
// Active holds pointers to currently running jobs.
Active []api.ObjectReference `json:"active,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Will there be something in the referenced job which identifies which execution it is from (in case of allowed concurrency?)

Copy link
Member

Choose a reason for hiding this comment

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

This could be an annotation or it could be embedded in the generated name as a date code or something. No action needed here.

@erictune
Copy link
Member

erictune commented May 4, 2016 •
edited
Loading

I've written what I think is a valid scheduledJob object:

apiVersion: batch/v2alpha1
kind: ScheduledJob
metadata:
  name: pi
spec:
  schedule: "0 0 0 0 0"
  startingDeadlineSeconds: 30
  concurrencyPolicy: "Allow" 
  suspend: false
  jobTemplate:
    spec: 
      template:
        spec:
          containers:
          - name: pi
            image: perl
            command: ["perl",  "-Mbignum=bpi", "-wle", "print bpi(2000)"]
            restartPolicy: Never

@erictune
Copy link
Member

erictune commented May 5, 2016

TIL: when a controller like replicaSet creates a pod from a template, it (surprisingly) ignores template.metadata.generateName and template.metadata.name. It just uses the name from the replicaSet and adds the random stuff at the end. For the job controller, it could do the same thing. I assumed that was the case, and so the only name in above example is on the SJ object.

@soltysh
Copy link
Contributor Author

soltysh commented May 6, 2016

I haven't thought about it earlier, but that's I guess the only reasonable pattern to follow.

@erictune
Copy link
Member

How is this going?

@erictune
Copy link
Member

2 weeks before code freeze.

@soltysh soltysh force-pushed the scheduledjob_api branch from eafc2e3 to 622f24a Compare May 11, 2016 09:46
@soltysh soltysh force-pushed the scheduledjob_api branch from 622f24a to f1c204b Compare May 11, 2016 10:19
@soltysh soltysh force-pushed the scheduledjob_api branch from f1c204b to 22bada0 Compare May 11, 2016 11:52
@k8s-bot
Copy link

k8s-bot commented May 11, 2016

GCE e2e build/test passed for commit 22bada0.

@soltysh soltysh mentioned this pull request May 11, 2016
Merged
@erictune
Copy link
Member

LGTM

@erictune erictune added lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note-none Denotes a PR that doesn't merit a release note. release-note-experimental and removed release-note-label-needed release-note-none Denotes a PR that doesn't merit a release note. labels May 11, 2016
@k8s-github-robot k8s-github-robot added release-note-label-needed and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 11, 2016
@erictune erictune added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-experimental labels May 11, 2016
@erictune erictune added this to the v1.3 milestone May 11, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented May 12, 2016

GCE e2e build/test passed for commit 22bada0.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 196b1c1 into kubernetes:master May 12, 2016
@soltysh soltysh deleted the scheduledjob_api branch May 12, 2016 06:24
k8s-github-robot pushed a commit that referenced this pull request May 21, 2016
Merge pull request #25475 from soltysh/scheduledjob_client
4d69e2c 
Automatic merge from submit-queue

ScheduledJob client 

@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 :)

```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)]()
@soltysh soltysh mentioned this pull request Jul 4, 2016
Closed
16 tasks
@SEJeff SEJeff mentioned this pull request Jul 27, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@erictune erictune

Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.
10 participants
@soltysh @erictune @k8s-bot @k8s-github-robot @derekwaynecarr @smarterclayton @googlebot @thockin @bgrant0607 @ixdy