-
Notifications
You must be signed in to change notification settings - Fork 40.1k
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.
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
[WIP] Implement deferContainers - Pod Termination Semantics #47422
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dhilipkumars Assign the PR to them by writing Associated issue: 256 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
e38b6f6
to
b6e8f48
Compare
I believe we need to agree on the proposal first... @smarterclayton |
b6e8f48
to
671a56b
Compare
671a56b
to
f729cd8
Compare
f729cd8
to
69aec6c
Compare
/assign @erictune |
Adding do-not-merge/release-note-label-needed because the release note process has not been followed. |
/label keep-open |
Use cases, and comments about each use case:
|
Some more usecases that was presented during AppSig. |
/label keep-open |
…mbing of deferContainers in kubelet
69aec6c
to
2e7f91c
Compare
@dhilipkumars: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
@dhilipkumars PR needs rebase |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/close @dhilipkumars please reopen with a fresh rebase when you need this |
What this PR does / why we need it:
Introduces a new Termination Semantic for pod.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): fixes #Implements this feature. The implementation has diverged reasonably from its orignal proposal.
We will update the proposal to reflect what has been implemented.
Special notes for your reviewer:
NOTE: This is an unpolished implementation, has a lot of rough edges. This PR is raised just to get some consensus on the approach we have taken.
Here are some of the highlights of the approach
Here is a sample pod spec configured with deferContainers and with a gracePeriod of 60 secs
Here is the events for the pod. As you can see the deferContainer named
last
has a different image which is getting pre-pulledAlso the pod does not wait to run-out of all of the GracePeriod, as soon as all the deferContainers are completed the pod is deleted. Even though the grace period is configured for 60seconds since all the deferContainers completed successfully in 7 seconds, so the pod is deleted without waiting further.
CC: @dchen1107 @smarterclayton @erictune @brendandburns @Kargakis @quinton-hoole @irfanurrehman @kow3ns @sig-node-reviewers
TODO:
Release note: none