prevent mutation of deletion options during delete collection#100101
Conversation
|
/retest |
1 similar comment
|
/retest |
|
/test pull-kubernetes-integration |
|
/retest |
|
/test pull-kubernetes-integration |
|
/hold this would be one of the stranger bug triggers I've seen, but let's see /test pull-kubernetes-integration |
|
yep, found it /hold cancel |
8a7ef6d to
b3c50c0
Compare
There was a problem hiding this comment.
this helper only has one callsite and it was already relying on termination with no grace.
|
/triage accepted |
|
Is there an explicit unit test we can add that guarantees the DELETECOLLECTION call doesn't do this? |
|
/assign @mrunalp |
|
Why is the mutation of |
|
/hold @deads2k can you add a unit test per @smarterclayton ? |
CheckGracefulDelete is "per pod". A copy has to be used anytime you invoke "per pod" from a "per list". Delete connection calling it "per list" is wrong. Note there are several other PRs in flight dealing with the problems around what CHeckGracefulDelete is doing (triggered by this thread, originally, since we started looking and saw that things were burning) #102025 #102344 and #98866 |
|
@deads2k do we want this in 1.22? given code freeze today and this is marked |
|
/hold cancel |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, smarterclayton The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/milestone v1.22 |
|
/retest |
|
/retest Review the full test history for this PR. Silence the bot with an |
2 similar comments
|
/retest Review the full test history for this PR. Silence the bot with an |
|
/retest Review the full test history for this PR. Silence the bot with an |
DeepCopy the deletion options because individual graceful deleters communicate changes via a mutating function in the delete strategy called in the delete method. While that is always ugly, it works when making a single call. When making multiple calls via delete collection, the mutation applied to pod/A can change the option ultimately used for pod/B. This can result in pods be non-gracefully terminated
The mutation happens here https://github.com/kubernetes/kubernetes/blob/master/pkg/registry/core/pod/strategy.go#L159 and is documented as the proper way for gracefuldeleters to indicate new values.
/kind bug
/priority important-soon
@kubernetes/sig-node-bugs