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

Fix incorrect info about when CronJob reached GA #29492

Merged

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Aug 20, 2021

Small fixup for PR #29455
/sig apps

@k8s-ci-robot k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. language/en Issues or PRs related to English language labels Aug 20, 2021
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Aug 20, 2021
@netlify
Copy link

netlify bot commented Aug 20, 2021

✔️ Deploy Preview for kubernetes-io-main-staging ready!

🔨 Explore the source changes: a532758

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/61241a76e81d650008a40b61

😎 Browse the preview: https://deploy-preview-29492--kubernetes-io-main-staging.netlify.app

@kbhawkey
Copy link
Contributor

@kbhawkey
Copy link
Contributor

@tengqm
Copy link
Contributor

tengqm commented Aug 21, 2021

According to the feature gates, CronJob v2 was GA'ed in 1.22.

@sftim
Copy link
Contributor Author

sftim commented Aug 22, 2021

It's true that v1.22 graduated a new CronJob controller to GA, but that's not what this feature state shortcode is about. CronJob graduated to stable in v1.21 with the new controller behind an on-by-default feature flag.

@sftim
Copy link
Contributor Author

sftim commented Aug 22, 2021

@kbhawkey Kubernetes APIs are themselves versioned, so a lot of them don't need a feature gate. Instead the client specifies what API they want to use via the URL they interact with. There's more to it than that, but essentially API versioning and deprecation is one thing and feature flags are another.

@kbhawkey
Copy link
Contributor

kbhawkey commented Aug 22, 2021

@kbhawkey Kubernetes APIs are themselves versioned, so a lot of them don't need a feature gate. Instead the client specifies what API they want to use via the URL they interact with. There's more to it than that, but essentially API versioning and deprecation is one thing and feature flags are another.

Hi @sftim. OK, though I am not sure if the feature state shortcode has been used to document the version of an API? Looking at this change, I assumed that the version change corresponds to a gate change.

Also, I found this section somewhat confusing:
https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/#new-controller

@sftim
Copy link
Contributor Author

sftim commented Aug 23, 2021

Another example of a feature-state shortcode being used for an API: https://kubernetes.io/docs/concepts/services-networking/ingress/

@sftim sftim force-pushed the 20210820_fix_cronjob_graduation_release branch from 5bd532a to a532758 Compare August 23, 2021 22:00
@reylejano
Copy link
Member

CronJobs reached GA in 1.21
https://kubernetes.io/blog/2021/04/09/kubernetes-release-1.21-cronjob-ga/

the CronJobControllerV2 reached GA in 1.22
kubernetes/kubernetes#102529

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 24, 2021
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 1b6ca892d927df56b88ecb252242494335b3f1dc

@reylejano
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: reylejano

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 25, 2021
@k8s-ci-robot k8s-ci-robot merged commit c83e410 into kubernetes:main Aug 25, 2021
@benlangfeld
Copy link

@sftim The note about timezones is misleading, because it does not work with the old cronjob controller in 1.21.

@cndoit18
Copy link
Member

cndoit18 commented Sep 2, 2021

@sftim The note about timezones is misleading, because it does not work with the old cronjob controller in 1.21.

ping @tengqm @sftim @reylejano 👀

@sftim sftim deleted the 20210820_fix_cronjob_graduation_release branch September 7, 2021 12:32
@sftim
Copy link
Contributor Author

sftim commented Sep 7, 2021

@benlangfeld it sounds like you want to report a defect with the Kubernetes v1.21 docs about CronJob and timezones?
If that's right, I recommend you file a new issue. This PR has merged.

@benlangfeld
Copy link

@benlangfeld it sounds like you want to report a defect with the Kubernetes v1.21 docs about CronJob and timezones?
If that's right, I recommend you file a new issue. This PR has merged.

I was reporting that this PR was incorrect and shouldn't have been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants