Skip to content

KEP 4818: Promote zero value for Pod Lifecycle Sleep action to beta #5139

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

Merged
merged 2 commits into from
Feb 11, 2025
Merged

KEP 4818: Promote zero value for Pod Lifecycle Sleep action to beta #5139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
21f3345
KEP 4818: Promote zero value for Pod Lifecycle Sleep action to beta
sreeram-venkitesh Feb 6, 2025
914315b
Added PRR approver and fixed implementation history
sreeram-venkitesh Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions keps/prod-readiness/sig-node/4818.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
kep-number: 4818
alpha:
approver: "@wojtek-t"
beta:
approver: "@wojtek-t"
147 changes: 21 additions & 126 deletions keps/sig-node/4818-allow-zero-value-for-sleep-action-of-prestop-hook/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,75 +1,5 @@
<!--
**Note:** When your KEP is complete, all of these comment blocks should be removed.

To get started with this template:

- [x] **Pick a hosting SIG.**
Make sure that the problem space is something the SIG is interested in taking
up. KEPs should not be checked in without a sponsoring SIG.
- [x] **Create an issue in kubernetes/enhancements**
When filing an enhancement tracking issue, please make sure to complete all
fields in that template. One of the fields asks for a link to the KEP. You
can leave that blank until this KEP is filed, and then go back to the
enhancement and add the link.
- [x] **Make a copy of this template directory.**
Copy this template into the owning SIG's directory and name it
`NNNN-short-descriptive-title`, where `NNNN` is the issue number (with no
leading-zero padding) assigned to your enhancement above.
- [x] **Fill out as much of the kep.yaml file as you can.**
At minimum, you should fill in the "Title", "Authors", "Owning-sig",
"Status", and date-related fields.
- [x] **Fill out this file as best you can.**
At minimum, you should fill in the "Summary" and "Motivation" sections.
These should be easy if you've preflighted the idea of the KEP with the
appropriate SIG(s).
- [x] **Create a PR for this KEP.**
Assign it to people in the SIG who are sponsoring this process.
- [ ] **Merge early and iterate.**
Avoid getting hung up on specific details and instead aim to get the goals of
the KEP clarified and merged quickly. The best way to do this is to just
start with the high-level sections and fill out details incrementally in
subsequent PRs.

Just because a KEP is merged does not mean it is complete or approved. Any KEP
marked as `provisional` is a working document and subject to change. You can
denote sections that are under active debate as follows:

```
<<[UNRESOLVED optional short context or usernames ]>>
Stuff that is being argued.
<<[/UNRESOLVED]>>
```

When editing KEPS, aim for tightly-scoped, single-topic PRs to keep discussions
focused. If you disagree with what is already in a document, open a new PR
with suggested changes.

One KEP corresponds to one "feature" or "enhancement" for its whole lifecycle.
You do not need a new KEP to move from beta to GA, for example. If
new details emerge that belong in the KEP, edit the KEP. Once a feature has become
"implemented", major changes should get new KEPs.

The canonical place for the latest set of instructions (and the likely source
of this file) is [here](/keps/NNNN-kep-template/README.md).

**Note:** Any PRs to move a KEP to `implementable`, or significant changes once
it is marked `implementable`, must be approved by each of the KEP approvers.
If none of those approvers are still appropriate, then changes to that list
should be approved by the remaining approvers and/or the owning SIG (or
SIG Architecture for cross-cutting KEPs).
-->
# KEP-4818: Allow zero value for Sleep Action of PreStop Hook

<!--
A table of contents is helpful for quickly jumping to sections of a KEP and for
highlighting any additional information provided beyond the standard KEP
template.

Ensure the TOC is wrapped with
<code>&lt;!-- toc --&rt;&lt;!-- /toc --&rt;</code>
tags, and then generate with `hack/update-toc.sh`.
-->

<!-- toc -->
- [Release Signoff Checklist](#release-signoff-checklist)
- [Summary](#summary)
Expand Down Expand Up @@ -110,26 +40,12 @@ tags, and then generate with `hack/update-toc.sh`.

## Release Signoff Checklist

<!--
**ACTION REQUIRED:** In order to merge code into a release, there must be an
issue in [kubernetes/enhancements] referencing this KEP and targeting a release
milestone **before the [Enhancement Freeze](https://git.k8s.io/sig-release/releases)
of the targeted release**.

For enhancements that make changes to code or processes/procedures in core
Kubernetes—i.e., [kubernetes/kubernetes], we require the following Release
Signoff checklist to be completed.

Check these off as they are completed for the Release Team to track. These
checklist items _must_ be updated for the enhancement to be released.
-->

Items marked with (R) are required *prior to targeting to a milestone / release*.

- [ ] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR)
- [ ] (R) KEP approvers have approved the KEP status as `implementable`
- [ ] (R) Design details are appropriately documented
- [ ] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input (including test refactors)
- [x] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR)
- [x] (R) KEP approvers have approved the KEP status as `implementable`
- [x] (R) Design details are appropriately documented
- [x] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input (including test refactors)
- [ ] e2e Tests for all Beta API Operations (endpoints)
- [ ] (R) Ensure GA e2e tests meet requirements for [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md)
- [ ] (R) Minimum Two Week Window for GA e2e tests to prove flake free
Expand Down Expand Up @@ -231,13 +147,6 @@ As a Kubernetes user, I want to to be able to have a PreStop hook defined in my

### Notes/Constraints/Caveats (Optional)

<!--
What are the caveats to the proposal?
What are some important details that didn't come across above?
Go in to as much detail as necessary here.
This might be a good place to talk about core concepts and how they relate.
-->

### Risks and Mitigations

The change is opt-in, since it requires configuring a PreStop hook with sleep action of 0 second duration. So there is no risk beyond the upgrade/downgrade risks which are addressed in the Proposal section.
Expand All @@ -248,17 +157,6 @@ Refer to the Proposal section.

### Test Plan

<!--
**Note:** *Not required until targeted at a release.*
The goal is to ensure that we don't accept enhancements with inadequate testing.

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
-->

[x] I/we understand the owners of the involved components may require updates to
existing tests to make this code solid enough prior to committing the changes necessary
to implement this enhancement.
Expand All @@ -272,32 +170,16 @@ implementing this enhancement to ensure the enhancements have also solid foundat

##### Unit tests

<!--
In principle every added code should have complete unit test coverage, so providing
the exact set of tests will not bring additional value.
However, if complete unit test coverage is not possible, explain the reason of it
together with explanation why this is acceptable.
-->

<!--
Additionally, for Alpha try to enumerate the core package you will be touching
to implement this enhancement and provide the current unit coverage for those
in the form of:
- <package>: <date> - <current test coverage>
The data can be easily read from:
https://testgrid.k8s.io/sig-testing-canaries#ci-kubernetes-coverage-unit

This can inform certain test coverage improvements that we want to do before
extending the production code to implement this enhancement.
-->

Alpha:

- Test that the runSleepHandler function returns immediately when given a duration of zero.
- Test that the validation succeeds when given a zero duration with the feature gate enabled.
- Test that the validation fails when given a zero duration with the feature gate disabled.
- Test that the validation returns the appropriate error messages when given an invalid duration value (e.g., a negative value) with the feature gate disabled and enabled.
- Unit tests for testing the disabling of the feature gate after it was enabled and the feature was used.
- Unit tests for pod with zero grace period duration and zero sleep duration with zero value enabled.
- Unit test for pod with nil grace period with zero value disabled
- Unit test for pod with nil grace period with zero value enabled

Current coverages:

Expand Down Expand Up @@ -342,6 +224,12 @@ Basic functionality
- Delete the pod and observe the time it takes for the container to terminate.
- Verify that the container terminates immediately without sleeping.

Additional e2e tests for beta:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do any tests exist for this feature for alpha? And can we point to any presubmits or an existing job?

- Test that pods with sleep value of 0 in PreStop hook can be created
- Test that pods with sleep value of 0 in PostStart hook can be created
- Test that pods with sleep value of 0 in PreStop hook can be updated
- Test that pods with sleep value of 0 in PostStart hook can be updated

### Graduation Criteria

#### Alpha
Expand All @@ -350,7 +238,7 @@ Basic functionality

#### Beta
- Gather feedback from developers and surveys
- Additional e2e tests are completed (if needed)
- Additional e2e tests are completed
- No trouble reports from alpha release
Copy link
Contributor

Choose a reason for hiding this comment

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

We encountered an issue with this feature during alpha testing, and the PR is still unmerged. Should this prevent us from moving to beta in version 1.33?
ref: kubernetes/kubernetes#129946

Copy link
Member Author

@sreeram-venkitesh sreeram-venkitesh Feb 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, not getting that PR would be a blocker, thanks for bringing it up here! I'm hoping it is merged soon since it has already received an lgtm. What do you all think? I'm okay with defering the beta graduation to 1.34 if needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would be fine with still getting this into beta as long as that PR merges. We sometimes require the KEP first and then you act on that plan before promotion. I think this can be a good example of that case.


#### GA
Expand Down Expand Up @@ -565,6 +453,13 @@ Major milestones might include:
- when the KEP was retired or superseded
-->

- 2024-09-16: Alpha KEP PR opened for v1.32
- 2024-10-03: Summary, Motivation and Proposal sections merged
- 2024-09-03: [Alpha code implementation PR](https://github.com/kubernetes/kubernetes/pull/127094) opened
- 2024-11-01: Alpha code PR merged
- 2024-12-11: Kubernetes v1.32 release with PodLifecycleSleepActionAllowZero in alpha stage
- 2025-02-06: KEP updated targeting to beta in v1.33
Copy link
Contributor

Choose a reason for hiding this comment

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

please make sure to be compliant with the last KEP template

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated, please let me know if it looks good.


## Drawbacks

N/A
Expand Down
7 changes: 4 additions & 3 deletions keps/sig-node/4818-allow-zero-value-for-sleep-action-of-prestop-hook/kep.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,20 @@ creation-date: "2024-09-16"

reviewers:
- "@kannon92"
- "@ffromani"
approvers:
- "@SergeyKanzhelev"

see-also:
- "/keps/sig-node/3960-pod-lifecycle-sleep-action"

stage: alpha
stage: beta

latest-milestone: "v1.32"
latest-milestone: "v1.33"

milestone:
alpha: "v1.32"
beta: ""
beta: "v1.33"
stable: ""
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure is it ok to leave it blank? it should be filed per estimation in the graduation criteria
Anyway, exactly because I'm not sure, not blocking from my side

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm yea, I had updated it since this is a requirement for the release team to track the KEP and was asked to do so in my other KEP. I can defer updating this to until the KEP has been reviewed and lead-opted-in label has been added.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd file it before to let them evaluate the plan, but your call :)

Copy link
Member Author

@sreeram-venkitesh sreeram-venkitesh Feb 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Got it. And when we say "let them evaluate the plan", we're talking about SIG leads right? Also are we talking about leaving the stable release blank?

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant reviewers in general, including of course (but not limited to) leads and Prod-Readiness Reviewers.


feature-gates:
Expand Down