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

Rename ScheduledJob to CronJob #32150

Closed
soltysh opened this issue Sep 6, 2016 · 15 comments · Fixed by #36021
Closed

Rename ScheduledJob to CronJob #32150

soltysh opened this issue Sep 6, 2016 · 15 comments · Fixed by #36021
Assignees
Milestone

Comments

@soltysh
Copy link
Contributor

soltysh commented Sep 6, 2016

As proposed by @erictune in #32115 (comment) ScheduledJob should be renamed to CronJob before moving this to beta.

@soltysh
Copy link
Contributor Author

soltysh commented Oct 26, 2016

@janetkuo I've noticed you're taking care of the rename in #35534, I'd like to rename SJ to CronJobs, for 1.5. The only bugging thing for me is whether we just rename everything breaking backwards compatibility. SJ, similarly to PS are just and alpha feature so that's ok, but I want to be sure before coding.
@erictune || @bgrant0607 any opinions on that one?

@justinsb
Copy link
Member

I think if the schema name changes, it should stay in alpha for another release. So if it is renamed to CronJobs in 1.5, it should not go to beta until 1.6 at the earliest.

@soltysh
Copy link
Contributor Author

soltysh commented Oct 26, 2016

Not quite convinced, first off SJ are alpha, so no guarantees about backwards compatibility. Secondly, we're doing exactly similar thing to PetSets (see #35534).

@erictune
Copy link
Member

@justinsb can you elaborate on your reasoning?

@soltysh
Copy link
Contributor Author

soltysh commented Oct 27, 2016

To give one more argument for, the sooner we do it the better for the users to safe the effort needed to switch from SJ to CJ.

@justinsb
Copy link
Member

justinsb commented Oct 27, 2016

Some of my thoughts:

  • When we decided to rename PetSets, we did so after much deliberation, and consideration of the impact it would have on users. I don't want another discussion like that, but we should have it if you want to rename.
  • While I don't understand the advantages for the rename, I do see the costs. We have yet to do a successful rename. I suspect once PetSets -> SS is done, when we have fully weighed the cost, we will say "never again".
  • One of the factors of the PetSet rename is that it does not establish precedent.
  • Alpha doesn't guarantee backwards compatibility, but I don't think that is carte-blanche to make changes without consideration of users. We do want users to try out alpha features.
  • When there is a new name, it feels prudent to let it sit in case there is another PetSet or other objection (e.g. legal objection).
  • What has changed that the name is unacceptable now, but was fine a month ago when it went into the release?
  • If you're making big changes like changing the name, it indicates in my mind that staying in alpha might be a good idea.
  • I don't think staying in alpha for one extra release is a big price to pay.

But what I don't have visibility into, is what is the upside here? It may still be worth it.

Also, and most importantly, it should be "ChronJob". Because it's χρον-, not κρον-

@erictune
Copy link
Member

erictune commented Oct 27, 2016

Justin: does it address your concerns if we do the following.

for at least 2 versions, the apiserver serves both apis/batch/v2alpha1/scheduledJobs and apis/batch/v2beta1/cronJobs, and they are backed by the same storage

This means you could convert your manifests 1 at a time and also you could chose when to do it, not concurrent with an upgrade.

I've talked to @caesarxuchao and he says the APIserver side of this is do-able. It will require some trickery in kubectl. One idea is to use a ThirdParty client in kubectl for scheduledJob/CronJob.

Once we figure out this pattern, we can use it again pretty easily.

@justinsb @smarterclayton

@justinsb
Copy link
Member

I think that would be great @erictune - thank you for considering & suggesting it! It feels very much like the kubernetes way to evolving our schemas. It leaves the human issues (communication of renames, spelling of cron) which feels right, and with a "gradual" migration we could even put warnings into kubectl.

@soltysh
Copy link
Contributor Author

soltysh commented Oct 27, 2016

What has changed that the name is unacceptable now, but was fine a month ago when it went into the release?

A few mentioned that Scheduled might be mistakenly taken with the Scheduler. There were some naming doubts at the proposal time but were dropped at the time.

If you're making big changes like changing the name, it indicates in my mind that staying in alpha might be a good idea.

Actually I didn't plan to move SJ/CJ to beta anytime soon due to the missing parts, see kubernetes/enhancements#19 'Before beta' section and that will definitely won't happen for 1.5. But I wanted to do the rename asap to save users from migration pain.

Also, and most importantly, it should be "ChronJob". Because it's χρον-, not κρον-

Cron it will be, since it has a connotation from plain old linux cron.

Once we figure out this pattern, we can use it again pretty easily.

@erictune we already did, see extensions/v1beta1 and batch/v1 Jobs, that's not that hard ;)

@soltysh
Copy link
Contributor Author

soltysh commented Oct 27, 2016

Upon a discussion with @erictune and @justinsb we've had on Slack we're going to leave SJ as is and we'll introduce yet another object in batch/v2alpha1 called CronJob. Both of them should use cohabitating resource mechanism and we'll drop SJ support when we move to beta. All user facing docs should use the new name.

@smarterclayton
Copy link
Contributor

SGTM

On Thu, Oct 27, 2016 at 5:56 PM, Maciej Szulik notifications@github.com
wrote:

Upon a discussion with @erictune https://github.com/erictune and
@justinsb https://github.com/justinsb we've had on Slack we're going to
leave SJ as is and we'll introduce yet another object in batch/v2alpha1
called CronJob. Both of them should use cohabitating resource mechanism and
we'll drop SJ support when we move to beta. All user facing docs should use
the new name.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#32150 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_p53WT6-eaKwdjgfPj3x34gOaOMOZks5q4R36gaJpZM4J2NBV
.

@chrislovecnm
Copy link
Contributor

Renaming stuff creates a mountain of work. For example back porting StatefulSet bug fixes back to PetSet... So I am against this.

@smarterclayton
Copy link
Contributor

The reason things are alpha is for getting experience and feedback. The
rename argument does not really apply to core API types in that having the
wrong name has substantial costs over time.

It may mean it won't get completed before a particular release freeze, but
that's not a reason not to have things named.

On Oct 29, 2016, at 5:02 PM, Chris Love notifications@github.com wrote:

Renaming stuff creates a mountain of work. For example back porting
StatefulSet bug fixes back to PetSet... So I am against this.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#32150 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_p1PvqEJlKpx8prRVscaasOw5lTxXks5q47RpgaJpZM4J2NBV
.

@chrislovecnm
Copy link
Contributor

chrislovecnm commented Oct 30, 2016

@smarterclayton I understand what you are saying, and maybe we need to figure out how to rename stuff better. For instance, there is a PetSet issue that will not get backported to 1.4 because of the rename and the way we are using alpha -> beta -> don't know the next name. Renaming is probably inevitable, but to me, we have better use of our time. Totally personal opinion. Where is that issue about renaming stuff :)

I am still against this, because of the work generated, the fact it actually is NOT a cron job, in the manner that I learned about cron 15 years ago, and the business risk and cost that a rename creates.

Cron daemon has a specific Linux admin meaning, and having a name like cron means that we have a cron like daemon. ScheduledJob make sense to me. Cron muddies the waters a bit because shares the name with another well-known feature that admins use a ton.

@smarterclayton just read your comment after @erictune comment, after I wrote this comment. Pretty much wrote the exact same reasoning that you used, without reading your comment.

@smarterclayton
Copy link
Contributor

The intent of the resource is for cluster wide cron, and has been designed
to map to that use case. Scheduled overlaps too heavily with extent
terminology and creates undue confusion. It's ambiguous in use.

Business risk isn't enough of a justification, simply because the process
we follow is about alpha being a safe space for experimentation. Treating
alpha resources like final resources hurts the project more than it helps.
There are plenty of reasons to be cautious about naming, but alpha is alpha
so that we can change it.

On Oct 30, 2016, at 12:57 PM, Chris Love notifications@github.com wrote:

@smarterclayton https://github.com/smarterclayton I understand what you
are saying, and maybe we need to figure out how to rename stuff better. For
instance, there is a PetSet issue that will not get backported to 1.4
because of the rename and the way we are using alpha -> beta -> don't know
the next name. Renaming is probably inevitable, but to me, we have better
use of our time. Totally personal opinion. Where is that issue about
renaming stuff :)

I am still against this, because of the work generated, the fact it
actually is NOT a cron job, in the manner that I learned about cron 15
years ago, and the business risk and cost that a rename creates.

Cron daemon has a specific Linux admin meaning, and having a name like cron
means that we have a cron like daemon. ScheduledJob make sense to me. Cron
muddies the waters a bit because shares the name with another well-known
feature that admins use a ton.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#32150 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_p8jNS9AnZGz43hAU1dk0BYYX6Yehks5q5MxcgaJpZM4J2NBV
.

k8s-github-robot pushed a commit that referenced this issue Nov 7, 2016
Automatic merge from submit-queue

Rename ScheduledJobs to CronJobs

I went with @smarterclayton idea of registering named types in schema. This way we can support both the new (CronJobs) and old (ScheduledJobs) resource name. Fixes #32150.

fyi @erictune @caesarxuchao @janetkuo 

Not ready yet, but getting close there...

**Release note**:
```release-note
Rename ScheduledJobs to CronJobs.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants