Skip to content

Commit f631103

Browse files
Updates to readme, clarified feature gate disablement, added tests
1 parent 9963bf5 commit f631103

File tree

1 file changed

+21
-7
lines changed
  • keps/sig-node/4818-allow-zero-value-for-sleep-action-of-prestop-hook

1 file changed

+21
-7
lines changed

keps/sig-node/4818-allow-zero-value-for-sleep-action-of-prestop-hook/README.md

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -175,20 +175,19 @@ A potential use case for this behaviour is when you need a PreStop hook to be de
175175

176176
### Non-Goals
177177

178-
N/A
178+
- This KEP does not support adding negative values for the sleep duration.
179+
- This KEP does not aim to provide a way to pause or delay pod termination indefinitely.
179180

180181
## Proposal
181182

182183
Introduce a `PodLifecycleSleepActionAllowZero` feature gate which is disabled by default. When the feature gate is enabled, the `validateSleepAction` method would allow values greater than or equal to zero as a valid sleep duration.
183184

184-
185185
Since this update to the validation allows previously invalid values, care must be taken to support cluster downgrades safely. To accomplish this, the validation will distinguish between new resources and updates to existing resources:
186186

187-
188187
- When the feature gate is disabled:
189188
- (a) New resources will no longer allow setting zero as the sleep duration second for the PreStop hook. (no change to current validation)
190189
- (b) Existing resources cannot be updated to have a sleep duration of zero seconds
191-
- (c) Existing resources with a PreStop sleep duration set to zero will continue to run and use a sleep duration of zero seconds. These resources however cannot be updated without also updating the sleep duration to a non-zero value, since the validation would now fail.
190+
- (c) Existing resources with a PreStop sleep duration set to zero will continue to run and use a sleep duration of zero seconds. These can be updated and the zero sleep duration would continue to work.
192191
- When the feature gate is enabled:
193192
- (c) New resources allow zero as a valid sleep duration.
194193
- (d) Updates to existing resources will allow zero as a valid sleep duration.
@@ -220,7 +219,7 @@ The proposed change adds another layer to the `validateSleepAction` function to
220219
}
221220
```
222221

223-
If the feature gate must be turned off after it was turned on initially, users should take care to update all their resources with a PreStop sleep duration of zero seconds to a non-zero value. This can be done after the feature gate is disabled too.
222+
Currently, the kubelet accepts `0` as a valid duration. There is no validation done at the kubelet level. All the validation for the duration itself is done at the kube-apiserver. The [runSleepHandler](https://github.com/AxeZhan/kubernetes/blob/3a96afdfefdf329c637623ae31a61d20dbdb0393/pkg/kubelet/lifecycle/handlers.go#L129-L141) in the kubelet uses the `time.After()` function from the [time](https://pkg.go.dev/time) package, which supports a `0` duration input. `time.After` also accepts negative values which are also returned immediately similar to zero. We don't support negative values however.
224223

225224
See the entire code changes in the WIP PR: [https://github.com/kubernetes/kubernetes/pull/127094](https://github.com/kubernetes/kubernetes/pull/127094)
226225

@@ -294,6 +293,16 @@ extending the production code to implement this enhancement.
294293

295294
- `<package>`: `<date>` - `<test coverage>`
296295

296+
Alpha:
297+
298+
- Test that the runSleepHandler function returns immediately when given a duration of zero.
299+
- Test that the validation returns an error when given an invalid duration value (e.g., a negative value).
300+
301+
Current coverages:
302+
303+
- `k8s.io/kubernetes/pkg/apis/core/validation` : 2024-09-20 - 84.3
304+
- `k8s.io/kubernetes/pkg/kubelet/lifecycle/handlers` :2024-09-20 - 86.4
305+
297306
##### Integration tests
298307

299308
<!--
@@ -311,7 +320,7 @@ For Beta and GA, add links to added tests together with links to k8s-triage for
311320
https://storage.googleapis.com/k8s-triage/index.html
312321
-->
313322

314-
- <test>: <link to test coverage>
323+
N/A
315324

316325
##### e2e tests
317326

@@ -325,7 +334,12 @@ https://storage.googleapis.com/k8s-triage/index.html
325334
We expect no non-infra related flakes in the last month as a GA graduation criteria.
326335
-->
327336

328-
- <test>: <link to test coverage>
337+
Basic functionality
338+
339+
- Create a simple pod with a container that runs a long-running process.
340+
- Add a preStop hook to the container configuration, using the new sleepAction with a sleep duration of `0`.
341+
- Delete the pod and observe the time it takes for the container to terminate.
342+
- Verify that the container terminates immediately without sleeping.
329343

330344
### Graduation Criteria
331345

0 commit comments

Comments
 (0)