Skip terminal Pods with a deletion timestamp from the Daemonset sync #118716
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
94c88c9
to
be0885e
Compare
Change-Id: I64a347a87c02ee2bd48be10e6fff380c8c81f742
be0885e
to
0a1a0a7
Compare
|
/assign @bobbypage |
mimowo
reviewed
Jun 20, 2023
| @@ -2757,18 +2757,25 @@ func TestGetNodesToDaemonPods(t *testing.T) { | |||
| if err != nil { | |||
| t.Fatal(err) | |||
| } | |||
| addNodes(manager.nodeStore, 0, 2, nil) | |||
| addNodes(manager.nodeStore, 0, 4, nil) | |||
There was a problem hiding this comment.
This does not seem needed since all pods in the test reference node-0 or node-1 only.
There was a problem hiding this comment.
Leftover from a previous approach. Reverted.
|
/assign @atiratree |
|
/lgtm |
Change-Id: I8b921157e6be1c809dd59f8035ec259ea4d96301
|
/lgtm |
|
LGTM label has been added. Git tree hash: 9ffda0efded5aa9801bcc7a05c8f9a333d033eec
|
|
/label tide/merge-method-squash |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alculquicondor, soltysh 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:
Approvers can indicate their approval by writing |
This was referenced Jun 27, 2023
k8s-ci-robot
pushed a commit
that referenced
this pull request
Jul 7, 2023
* Skip terminal Pods with a deletion timestamp from the Daemonset sync Change-Id: I64a347a87c02ee2bd48be10e6fff380c8c81f742 * Review comments and fix integration test Change-Id: I3eb5ec62bce8b4b150726a1e9b2b517c4e993713 * Include deleted terminal pods in history Change-Id: I8b921157e6be1c809dd59f8035ec259ea4d96301 * Exclude terminal pods from Daemonset e2e tests Change-Id: Ic29ca1739ebdc54822d1751fcd56a99c628021c4
k8s-ci-robot
pushed a commit
that referenced
this pull request
Jul 7, 2023
* Skip terminal Pods with a deletion timestamp from the Daemonset sync Change-Id: I64a347a87c02ee2bd48be10e6fff380c8c81f742 * Review comments and fix integration test Change-Id: I3eb5ec62bce8b4b150726a1e9b2b517c4e993713 * Include deleted terminal pods in history Change-Id: I8b921157e6be1c809dd59f8035ec259ea4d96301 * Exclude terminal pods from Daemonset e2e tests Change-Id: Ic29ca1739ebdc54822d1751fcd56a99c628021c4
rayowang
pushed a commit
to rayowang/kubernetes
that referenced
this pull request
Feb 9, 2024
…ubernetes#118716) * Skip terminal Pods with a deletion timestamp from the Daemonset sync Change-Id: I64a347a87c02ee2bd48be10e6fff380c8c81f742 * Review comments and fix integration test Change-Id: I3eb5ec62bce8b4b150726a1e9b2b517c4e993713 * Include deleted terminal pods in history Change-Id: I8b921157e6be1c809dd59f8035ec259ea4d96301
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
What this PR does / why we need it:
/sig apps
If surge is disabled, the Daemonset currently waits for Pods to fully disappear from the apiserver before creating replacements. However, since k8s 1.22, Pods with a terminal phase (Failed, Succeeded) are guaranteed to not be holding any resources from a Node, and are safe to replace.
By skipping terminal Pods with a deletionTimestamp, the DS controller is able to create replacement Pods in the following scenarios:
Which issue(s) this PR fixes:
Fixes #118587
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: