prevent pods/status from touching ownerreferences #45826
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-github-robot
added
approved
|
|
||
| // don't allow the pods/status endpoint to touch owner references since old kubelets corrupt them in a way | ||
| // that breaks garbage collection | ||
| newPod.OwnerReferences = oldPod.OwnerReferences |
There was a problem hiding this comment.
Probably should guard this by "if terminating".
There was a problem hiding this comment.
Probably should guard this by "if terminating".
I think this is a general statement. pods/status probably shouldn't be updating ownerrefs under any circumstances
There was a problem hiding this comment.
i worry the terminating guard may cause other side effects. i prefer to keep it as-is.
There was a problem hiding this comment.
we need to keep this as-is. guarding on termination is insufficient. the older kubelets hit status update failures independent of the pod deletion problem when paired with this plug-in.
|
i thought strategies are run after admission control checks unless it was changed. |
this is the spot to fix the data regardless of what admission chain is running. |
derekwaynecarr
added
release-note
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, derekwaynecarr
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
Automatic merge from submit-queue (batch tested with PRs 45826, 45747, 45548, 45606, 41766) |
|
Commit found in the "release-1.6" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked. |
pods/status updates touching ownerreferences causes new fields to be dropped.
I think we really want to protect our metatdata by default with something like #45552 . This fixes the immediate problem.
@derekwaynecarr @eparis