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

[sreeram-venkitesh]

• One-line PR description: KEP update for KEP Allow zero value for Sleep Action of PreStop Hook #4818 for beta graduation

• Issue link: Allow zero value for Sleep Action of PreStop Hook #4818

[sreeram-venkitesh]

@ffromani Please take a look! Thank you!

[sreeram-venkitesh]

PRR approver has not been assigned yet 🤔 Should I reach out to someone from the PRR team to take a look at this or wait until the KEP is opted in and someone from the PRR team assigns it to themself?

[ffromani]

The suggestion I got when having the same problem was to pick a random PRR team member. The team will rebalance per their needs and the assigned reviewer will ask to put their name here.

[ffromani]

In this case just use the same alpha reviewer

[sreeram-venkitesh]

Done, I've added wojtek as the PRR approver for beta as well.

[ffromani]

initial review, need to do a proper pass

[ffromani]

In this case just use the same alpha reviewer

[ffromani]

please make sure to be compliant with the last KEP template

[sreeram-venkitesh]

Updated, please let me know if it looks good.

[ffromani]

@sreeram-venkitesh thanks for the updates. Didn't check yet if we're good with latest template, will do that ASAP.
AFAIU from sig-node perspective the work needed is about adding/extending e2e tests (and perhaps some internal cleanups along the way), correct?
If so it would make a pretty straightforward graduation

[sreeram-venkitesh]

Yes that is correct. I'm referring to how the beta graduation was done in the parent KEP #3960 with respect to the changes needed with the feature gate etc. The code implementation itself is a simple change to the validation allowing sleep to have a zero value. This has already been done, although we'd need to clean up the code during the graduation.

[ffromani]

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

[sreeram-venkitesh]

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.

[ffromani]

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

[sreeram-venkitesh]

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?

[ffromani]

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

[ffromani]

Yes that is correct. I'm referring to how the beta graduation was done in the parent KEP #3960 with respect to the changes needed with the feature gate etc. The code implementation itself is a simple change to the validation allowing sleep to have a zero value. This has already been done, although we'd need to clean up the code during the graduation.

got it: we tried to promote to GA during the 1.32 cycle but had to revert. So we retry in 1.33. From bureaucracy perspective it seems as nice (edit: meant "easy") as it can get. It would be nice to be proactive and add the missing bits that let the flakes/issues gone unnoticed till it was too late in the previous cycle

Will have a deeper and proper look early next week but provisional LGTM from sig-node perspective.

[kannon92]

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

[wojtek-t]

/approve PRR

[ffromani]

/lgtm

from sig-node: this work is about fixing/adding tests and completing the work which was almost done during the 1.32 cycle.
Deferring comments (if any) about the PRR section to the PRR reviewers

[sreeram-venkitesh]

@ffromani The original KEP which almost went to GA in 1.32 is #3960. This KEP is for allowing zero value for the sleep duration and this was in alpha in 1.32. Putting this out there since I felt like this can be confusing at times.

The work here is still small since the changes for supporting zero value were just a change to the validation and this was completed in v1.32 as alpha. We still needed to add the feature gate etc since #3960 was aiming for GA in 1.32 (which had to be reverted after a test flake). So now #3960 is aiming for GA now and this KEP is aiming for beta. Looking at the #3960, I can't see it being tracked for 1.33 though.

[HirazawaUi]

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

[sreeram-venkitesh]

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.

[kannon92]

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.

[ffromani]

/lgtm cancel

@ffromani The original KEP which almost went to GA in 1.32 is #3960. This KEP is for allowing zero value for the sleep duration and this was in alpha in 1.32. Putting this out there since I felt like this can be confusing at times.

The work here is still small since the changes for supporting zero value were just a change to the validation and this was completed in v1.32 as alpha. We still needed to add the feature gate etc since #3960 was aiming for GA in 1.32 (which had to be reverted after a test flake). So now #3960 is aiming for GA now and this KEP is aiming for beta. Looking at the #3960, I can't see it being tracked for 1.33 though.

Thanks, let me re-evaluate then. I don't think that change much, but I fell into confusion here so let me do my due diligence

[ffromani]

/lgtm

/lgtm cancel

@ffromani The original KEP which almost went to GA in 1.32 is #3960. This KEP is for allowing zero value for the sleep duration and this was in alpha in 1.32. Putting this out there since I felt like this can be confusing at times.
The work here is still small since the changes for supporting zero value were just a change to the validation and this was completed in v1.32 as alpha. We still needed to add the feature gate etc since #3960 was aiming for GA in 1.32 (which had to be reverted after a test flake). So now #3960 is aiming for GA now and this KEP is aiming for beta. Looking at the #3960, I can't see it being tracked for 1.33 though.

Thanks, let me re-evaluate then. I don't think that change much, but I fell into confusion here so let me do my due diligence

I reviewed again considering the above and from sig-node perspective still seems straightforward :) the plan makes sense and what we need to be done is clear.

I think we may want to GA this KEP a cycle or two after the parent KEP 3960 is GA, but this is my take (I may very much be too cautious here). Other than that this seems straightfotward to me.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SergeyKanzhelev, sreeram-venkitesh, wojtek-t

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:

• keps/prod-readiness/OWNERS [wojtek-t]
• keps/sig-node/4818-allow-zero-value-for-sleep-action-of-prestop-hook/OWNERS [SergeyKanzhelev]

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