janetkuo commented 6 years ago

Feature Description

One-line feature description (can be used as a release note): Introduce a TTL controller to clean up Jobs and Pods after they finish.
Primary contact (assignee): @janetkuo
Responsible SIGs: sig-apps
Design proposal link (community repo): https://github.com/kubernetes/enhancements/blob/master/keps/sig-apps/0026-ttl-after-finish.md (https://goo.gl/YxtxTi, https://github.com/kubernetes/community/pull/2552)
Link to e2e and/or unit tests:
Reviewer(s) - (for LGTM) recommend having 2+ reviewers (at least one from code-area OWNERS file) agreed to review. Reviewers from multiple companies preferred: @kow3ns @enisoc @tnozicka
Approver (likely from SIG/area to which feature belongs): @kow3ns
Feature target (which target equals to which milestone):
- Alpha release target (1.12)
- Beta release target (1.21)
- Stable release target (1.23)

wojtek-t commented 3 years ago

+1

ahg-g commented 3 years ago

Discussed in today's sig meeting, there is agreement on moving forward with graduating it to beta for jobs without blocking on pods. We will make a decision on pods when graduating from beta to GA.

boluisa commented 3 years ago

excited to see this feature progressing to Beta. Thank you all. ❤️ cc: @matthewdkaiser

JamesLaverack commented 3 years ago

Hey @ahg-g, Kubernetes 1.21 enhancements shadow here. 👋

We're reviewing this KEP For inclusion in Kubernetes 1.21, and I've got a few questions about the KEP:

Does SIG API Machinery need to do anything? They're listed in your kep.yaml as a participating SIG.
Are the metrics in your kep.yaml file the ones referenced by the graduation criteria. Have they been agreed on?
What is your test plan? I can't see one in your KEP.

ahg-g commented 3 years ago

Does SIG API Machinery need to do anything? They're listed in your kep.yaml as a participating SIG.

No, not for beta.

Are the metrics in your kep.yaml file the ones referenced by the graduation criteria. Have they been agreed on?

PRR and sig-apps approved the updated KEP, so I am guessing we have agreement on that.

What is your test plan? I can't see one in your KEP.

There are unit and e2e tests already that were part of alpha graduation.

ahg-g commented 3 years ago

@JamesLaverack does that answer your questions?

JamesLaverack commented 3 years ago

Thanks for the response @ahg-g.

If SIG API Machinery doesn't need to do anything at this stage then that's fine, and if the metrics are approved then that's fine too.

However I belive we need a testing section in the KEP to accept it for the release, as it's part of our checklist. The modern KEP Template has this as a section next to graduation criteria. (I'm guessing from the age of this enhancement that it might not have been part of the KEP template at the time, which is why it might not have been in here.)

It should be a relatively small change though, here's the guidence from the KEP template:


### Test Plan

<!--
**Note:** *Not required until targeted at a release.*

Consider the following in developing a test plan for this enhancement:
- Will there be e2e and integration tests, in addition to unit tests?
- How will it be tested in isolation vs with other components?

No need to outline all of the test cases, just the general strategy. Anything
that would count as tricky in the implementation, and anything particularly
challenging to test, should be called out.

All code is expected to have adequate tests (eventually with coverage
expectations). Please adhere to the [Kubernetes testing guidelines][testing-guidelines]
when drafting this test plan.

[testing-guidelines]: https://git.k8s.io/community/contributors/devel/sig-testing/testing.md
-->

Other than that I think this enhancement is fine, so once that's added we can mark it as tracked for 1.21. :)

ahg-g commented 3 years ago

@JamesLaverack I opened a PR for the change, it doesn't have any material impact though...

JamesLaverack commented 3 years ago

Thanks @ahg-g. That change looks good to me. I'll follow the PR and when it merges I can set this enhancment to be tracked for 1.21.

sftim commented 3 years ago

Does https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/ look OK for a beta feature? Do these docs merit extending?

JamesLaverack commented 3 years ago

Hi @ahg-g,

Since your Enhancement is scheduled to be in 1.21, please keep in mind the important upcoming dates:

Tuesday, March 9th: Week 9 — Code Freeze
Tuesday, March 16th: Week 10 — Docs Placeholder PR deadline
- If this enhancement requires new docs or modification to existing docs, please follow the steps in the Open a placeholder PR doc to open a PR against k/website repo.

As a reminder, please link all of your k/k PR(s) and k/website PR(s) to this issue so we can track them.

Thanks!

ahg-g commented 3 years ago

All k/k PRs merged.

Docs PR: https://github.com/kubernetes/website/pull/26738

JamesLaverack commented 3 years ago

Hi @ahg-g

Enhancements team is currently tracking the following PRs

https://github.com/kubernetes/kubernetes/pull/98678

As this PR is merged, can we mark this enhancement complete for code freeze or do you have other PR(s) that are being worked on as part of the release?

ahg-g commented 3 years ago

Hi @JamesLaverack, it is complete for this release. We will keep this issue open to track graduating the feature to GA in future releases.

JamesLaverack commented 3 years ago

Thank you @ahg-g. :)

salaxander commented 3 years ago

/milestone v1.23

salaxander commented 3 years ago

Hi @ahg-g! 1.23 Enhancements team here. Just checking in as we approach enhancements freeze on Thursday 09/09. Here's where this enhancement currently stands:

[X] ~~KEP file using the latest template has been merged into the k/enhancements repo.~~
[X] ~~KEP status is marked as implementable~~
[X] ~~KEP has a test plan section filled out.~~
[X] ~~KEP has up to date graduation criteria.~~
[X] ~~KEP has a production readiness review that has been completed and merged into k/enhancements.~~

Looks like we're all set for enhancements freeze :)

Thanks!

jlbutler commented 3 years ago

Hi @ahg-g :wave: 1.23 Docs lead here.

This enhancement is marked as 'Needs Docs' for the 1.23 release.

Please follow the steps detailed in the documentation to open a PR against the dev-1.23 branch in the k/website repo. This PR can be just a placeholder at this time and must be created before Thu November 18, 11:59 PM PDT.

Also, if needed take a look at Documenting for a release to familiarize yourself with the docs requirement for the release.

Thanks!

chrisnegus commented 3 years ago

Hi @sahilvv 👋 1.23 Docs shadow here.

Just want to verify that there are two docs PRs to track for this KEP in 1.23:

https://github.com/kubernetes/website/pull/29987 https://github.com/kubernetes/website/pull/30031

sahilvv commented 3 years ago

@chrisnegus you can ignore kubernetes/website#29987. kubernetes/website#30031 contains all the changes needed for graduating this feature to stable in 1.23. Let me know if any further questions.

salaxander commented 2 years ago

Hi @ahg-g. Checking in once more as we approach 1.23 code freeze at 6:00 pm PST on Tuesday, November 16.

Please ensure the following items are completed:

All PRs to the Kubernetes repo that are related to your enhancement are linked in the above issue description (for tracking purposes).
All PRs are fully merged by the code freeze deadline.
Have a documentation placeholder PR open by Thursday, November 18.

As always, we are here to help should questions come up.

Thanks!!

sahilvv commented 2 years ago

@salaxander I am the owner of this KEP and feature to take to GA state in 1.23. To answer your questions: 1) All PRs are merged to the k8s repo at this time 2) Doc update PRs are merged to dev-1.23 branch at this time.

No pending tasks left related to this KEP to the best of my knowledge.

Thanks.

salaxander commented 2 years ago

Hi @sahilvv - thanks for the update!

I've updated our tracking to show this as complete. When you get a chance, could you link the k/k PRs in the issue description?

Thanks!

sahilvv commented 2 years ago

@salaxander Following are the PRs to move this feature from Beta to Stable/GA state:

https://github.com/kubernetes/kubernetes/pull/105219 https://github.com/kubernetes/website/pull/30031

gracenng commented 2 years ago

1.24 Enhancements Lead here,

@sahilvv, could you please update the KEP status to implemented and close this issue then?

Thanks :)

sahilvv commented 2 years ago

@gracenng pardon my dumb question, I did mark the KEP as stable in PR https://github.com/kubernetes/enhancements/pull/2841/files. Let me know where I need to flip the status to implemented.

gracenng commented 2 years ago

hi @sahilvv , I could have been more clear. This line should be implemented instead of implementable

https://github.com/sahilvv/enhancements/blob/ee3a716a89c3aa0a16891447a46123bf9bd564f4/keps/sig-apps/592-ttl-after-finish/kep.yaml#L10

sahilvv commented 2 years ago

@gracenng above PR#3147 is merged. I am not sure how to close the issue as I don't see that as an option.

gracenng commented 2 years ago

Done, thanks

kubernetes / enhancements

TTL after finish #592

Feature Description