GA TTLAfterFinished #30031

sahilvv · 2021-10-11T06:11:27Z

This PR updates the TTLAfterFinish controller to stable as part of 1.23 k8s release.

Related PRs:

netlify · 2021-10-11T06:11:32Z

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

🔨 Explore the source changes: 1111731

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/616fa008c2bc9400070e0164

sahilvv · 2021-10-11T06:12:32Z

@sftim @ahg-g please review.

sahilvv · 2021-10-11T06:12:54Z

/milestone 1.23

k8s-ci-robot · 2021-10-11T06:12:55Z

@sahilvv: You must be a member of the kubernetes/website-milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Website milestone maintainers and have them propose you as an additional delegate for this responsibility.

In response to this:

/milestone 1.23

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sftim · 2021-10-11T15:26:56Z

/sig apps

sftim

This is OK to merge.

Ideally, we should improve this document as well for general availability. I'd reword “The TTL controller only supports Jobs for now” given that we now have a stable definition of what the control plane does.

The comment about NTP could change to be generic over which time sync technology you use.

Nice to have: follow the style guide throughout.

/lgtm

k8s-ci-robot · 2021-10-11T16:00:52Z

LGTM label has been added.

Git tree hash: 318aa2e2edfe12b5e7e42d991813358ffb519776

jlbutler · 2021-10-11T19:32:42Z

/assign @chrisnegus

ahg-g · 2021-10-12T13:04:25Z

I'd reword “The TTL controller only supports Jobs for now” given that we now have a stable definition of what the control plane does.

+1, I don't think the docs should be speculating about the future now that this is a GA feature.

sahilvv · 2021-10-15T07:39:00Z

@sftim @ahg-g reworded to make it explicit that the feature is supported only for jobs. Please let me know if any further changes needed.

sftim

/lgtm

I have a suggested rewording for the overview
Same as GA TTLAfterFinished #30031 (review): I'd prefer to change “NTP” to mention time sync (you could use another mechanism).

sftim · 2021-10-15T10:21:19Z

content/en/docs/concepts/workloads/controllers/ttlafterfinished.md

@@ -8,23 +8,17 @@ weight: 70

 <!-- overview -->

-{{< feature-state for_k8s_version="v1.21" state="beta" >}}
+{{< feature-state for_k8s_version="v1.23" state="stable" >}}

 The TTL controller provides a TTL (time to live) mechanism to limit the lifetime of resource


For GA, I would prefer to rename this to “The TTL-after-finished controller”

for example:

Suggested change

The TTL controller provides a TTL (time to live) mechanism to limit the lifetime of resource

The TTL-after-finished {{<glossary_tooltip text="controller" term_id="controller">}} provides

a TTL (time to live) mechanism to limit the lifetime of Kubernetes workload

k8s-ci-robot · 2021-10-15T10:22:52Z

LGTM label has been added.

Git tree hash: bb56aa35a71e4393363cf06e1af068b5dfb073f3

sftim · 2021-10-16T10:54:22Z

I also recommend looking at #26917, which is about making sure the right pages hyperlink to the TTLAfterFinished docs.

sahilvv · 2021-10-17T10:45:49Z

@sftim I have addressed the review feedback given so far. Please take a look. Thanks again!

sftim

Yep, still LGTM. Also happy to see tweaks. But bear in mind we can merge this and do the tweaks in a different PR; that works fine too!

Question: should we also retitle the page?
For example, to “Automatic Clean-up for Finished Jobs”.

/lgtm

sftim · 2021-10-17T19:47:58Z

content/en/docs/concepts/workloads/controllers/ttlafterfinished.md

 determine whether the TTL has expired or not, this feature is sensitive to time
 skew in the cluster, which may cause TTL controller to clean up resource objects
 at the wrong time.

-In Kubernetes, it's required to run NTP on all nodes
+In Kubernetes, it's required to run a time sync solution on all nodes
 (see [#6159](https://github.com/kubernetes/kubernetes/issues/6159#issuecomment-93844058))


nit: I'd omit this line.

removed @sftim.

k8s-ci-robot · 2021-10-17T19:50:12Z

LGTM label has been added.

Git tree hash: d699732a280fe69dc12b7e727de016ec23b16c0d

sahilvv · 2021-10-18T19:13:57Z

@chrisnegus @ahg-g @janetkuo please review.

janetkuo · 2021-10-19T21:27:05Z

content/en/docs/concepts/workloads/controllers/ttlafterfinished.md


-The TTL controller only supports Jobs for now. A cluster operator can use this feature to clean
+The TTL-after-finished controller is only supported for Jobs. A cluster operator can use this feature to clean


It we change this, we should also change other references of "resources" to "Jobs".

Great comment @janetkuo. I have updated the references of resources to jobs.

sftim · 2021-10-20T07:54:27Z

Still LGTM. Also happy to see tweaks. But bear in mind we can merge this and do the tweaks in a different PR; that works fine too!

/lgtm

k8s-ci-robot · 2021-10-20T07:54:43Z

LGTM label has been added.

Git tree hash: 6208ac93ca2cf582c8e9e68bcfb2366d943da74f

sahilvv · 2021-10-22T06:19:32Z

@janetkuo could you please re-review and approve if changes look good.

sahilvv · 2021-10-22T07:58:42Z

@sftim can you grant me /approve on this.

sftim · 2021-10-22T09:50:31Z

I'll leave approval to the SIG Release Docs folks.

sahilvv · 2021-10-22T16:37:44Z

@mehabhalodiya @chrisnegus Please review.

reylejano · 2021-10-26T02:06:28Z

/approve

k8s-ci-robot · 2021-10-26T02:06:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: janetkuo, 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:

~~content/en/OWNERS~~ [reylejano]

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

chrisnegus · 2021-10-26T12:08:18Z

@sftim @reylejano Thanks for taking care of this.
/lgtm

k8s-ci-robot added this to the 1.23 milestone Oct 11, 2021

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 11, 2021

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. language/en Issues or PRs related to English language labels Oct 11, 2021

k8s-ci-robot requested review from bradtopol and celestehorgan October 11, 2021 06:11

k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Oct 11, 2021

sahilvv mentioned this pull request Oct 11, 2021

GA TTLAfterFinished #29987

Closed

k8s-ci-robot added the sig/apps Categorizes an issue or PR as relevant to SIG Apps. label Oct 11, 2021

sftim reviewed Oct 11, 2021

View reviewed changes

k8s-ci-robot assigned sftim Oct 11, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 11, 2021

chrisnegus mentioned this pull request Oct 11, 2021

TTL after finish kubernetes/enhancements#592

Closed

k8s-ci-robot assigned chrisnegus Oct 11, 2021

sahilvv force-pushed the ga_ttl branch from a41f144 to 8fdd4b1 Compare October 15, 2021 07:37

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 15, 2021

sftim reviewed Oct 15, 2021

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 15, 2021

sahilvv force-pushed the ga_ttl branch from 8fdd4b1 to 42976a6 Compare October 17, 2021 10:37

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 17, 2021

sahilvv force-pushed the ga_ttl branch from 42976a6 to 81ecbb6 Compare October 17, 2021 10:44

sftim reviewed Oct 17, 2021

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 17, 2021

sahilvv force-pushed the ga_ttl branch from 81ecbb6 to b869cb1 Compare October 18, 2021 19:12

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 18, 2021

janetkuo reviewed Oct 19, 2021

View reviewed changes

GA TTLAfterFinish

1111731

sahilvv force-pushed the ga_ttl branch from b869cb1 to 1111731 Compare October 20, 2021 04:50

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 20, 2021

janetkuo approved these changes Oct 22, 2021

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 26, 2021

k8s-ci-robot merged commit 0af4dd9 into kubernetes:dev-1.23 Oct 26, 2021

sahilvv deleted the ga_ttl branch October 26, 2021 02:19

sahilvv mentioned this pull request Oct 26, 2021

REQUEST: New membership for sahilvv kubernetes/org#3057

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GA TTLAfterFinished #30031

GA TTLAfterFinished #30031

sahilvv commented Oct 11, 2021

netlify bot commented Oct 11, 2021 •

edited

Loading

sahilvv commented Oct 11, 2021

sahilvv commented Oct 11, 2021

k8s-ci-robot commented Oct 11, 2021

sftim commented Oct 11, 2021

sftim left a comment

k8s-ci-robot commented Oct 11, 2021

jlbutler commented Oct 11, 2021

ahg-g commented Oct 12, 2021

sahilvv commented Oct 15, 2021

sftim left a comment

sftim Oct 15, 2021

k8s-ci-robot commented Oct 15, 2021

sftim commented Oct 16, 2021

sahilvv commented Oct 17, 2021

sftim left a comment

sftim Oct 17, 2021

sahilvv Oct 18, 2021

k8s-ci-robot commented Oct 17, 2021

sahilvv commented Oct 18, 2021

janetkuo Oct 19, 2021

sahilvv Oct 20, 2021

sftim commented Oct 20, 2021

k8s-ci-robot commented Oct 20, 2021

sahilvv commented Oct 22, 2021

sahilvv commented Oct 22, 2021

sftim commented Oct 22, 2021

sahilvv commented Oct 22, 2021

reylejano commented Oct 26, 2021

k8s-ci-robot commented Oct 26, 2021

chrisnegus commented Oct 26, 2021

	The TTL controller provides a TTL (time to live) mechanism to limit the lifetime of resource
	The TTL-after-finished {{<glossary_tooltip text="controller" term_id="controller">}} provides
	a TTL (time to live) mechanism to limit the lifetime of Kubernetes workload


		The TTL controller only supports Jobs for now. A cluster operator can use this feature to clean
		The TTL-after-finished controller is only supported for Jobs. A cluster operator can use this feature to clean

GA TTLAfterFinished #30031

GA TTLAfterFinished #30031

Conversation

sahilvv commented Oct 11, 2021

netlify bot commented Oct 11, 2021 • edited Loading

sahilvv commented Oct 11, 2021

sahilvv commented Oct 11, 2021

k8s-ci-robot commented Oct 11, 2021

sftim commented Oct 11, 2021

sftim left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 11, 2021

jlbutler commented Oct 11, 2021

ahg-g commented Oct 12, 2021

sahilvv commented Oct 15, 2021

sftim left a comment

Choose a reason for hiding this comment

sftim Oct 15, 2021

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 15, 2021

sftim commented Oct 16, 2021

sahilvv commented Oct 17, 2021

sftim left a comment

Choose a reason for hiding this comment

sftim Oct 17, 2021

Choose a reason for hiding this comment

sahilvv Oct 18, 2021

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 17, 2021

sahilvv commented Oct 18, 2021

janetkuo Oct 19, 2021

Choose a reason for hiding this comment

sahilvv Oct 20, 2021

Choose a reason for hiding this comment

sftim commented Oct 20, 2021

k8s-ci-robot commented Oct 20, 2021

sahilvv commented Oct 22, 2021

sahilvv commented Oct 22, 2021

sftim commented Oct 22, 2021

sahilvv commented Oct 22, 2021

reylejano commented Oct 26, 2021

k8s-ci-robot commented Oct 26, 2021

chrisnegus commented Oct 26, 2021

netlify bot commented Oct 11, 2021 •

edited

Loading