Kubelet may reject static pods when the node is out of disk space, and will not retry them

[justinsb]

If a pod fails admission (admission controller, disk space, or network not ready as of #33347), it is not retried.

I think this is the cause of #35409

Working up a PR now... I am thinking of keeping a list of pods that were rejected, and reattempting them every housekeeping interval.

[yujuhong]

@justinsb pod phases progress monotonically. Once a pod has been rejected and transition to the "Failed" state, it is not supposed to go back to pending or running. I don't think kubelet should retry them

[justinsb]

But - for example - we check disk space in canAdmitPod. That is a temporary condition.

How do you propose to handle this instead?

[yujuhong]

kubelet doesn't care about whether the condition is temporary or not (for regular pods). It assumes the control plane (e.g., ReplicaSet) can react and determine what to do.

[yujuhong]

/cc @kubernetes/sig-node

[justinsb]

Hmmm.. that is tricky because of static pod manifests. I put the network-readiness check in the same place as the disk space check, as I figured that was the closest analog.

Looking for a place to move it such that the pod can be admitted, but where we can prevent it actually starting:

1. Any suggestion?
2. Should I move the disk free-space check there also?

[justinsb]

How about in Kubelet::syncPod?

[yujuhong]

Hmmm.. that is tricky because of static pod manifests. I put the network-readiness check in the same place as the disk space check, as I figured that was the closest analog.

@justinsb hmm....even for regular pods, this behavior seems undesirable. If kubelet restarts, and the network plugin is not ready (due to podCIDIR) yet, it would reject all pods assigned to it. That seems to be what caused the serial suite to fail.

How about in Kubelet::syncPod?

It might work. That just means kubelet will not sync the pod if network is not ready. I don't think disk space should be moved to the same space because kubelet actually wants to reject them.
/cc @dchen1107

Can we revert your original PR first? Thanks.

[justinsb]

Can we revert your original PR first? Thanks.

I feel like I've seen bad behaviour with full disks, so it feels like this is actually exposing a pre-existing problem. I don't understand why we would permanently reject a static pod if the disk was full?

It feels like we should move both to syncPod... I have a WIP PR which I'm testing now - can we give me a few minutes to see if that just fixes things, and then if not we can revert?

[justinsb]

Actually, on second thoughts, if we're blocking the submit queue we can just revert. It sounds like this is a non-trivial issue, so it's worth finding the right solution.

[yujuhong]

Actually, on second thoughts, if we're blocking the submit queue we can just revert. It sounds like this is a non-trivial issue, so it's worth finding the right solution.

I don't think it's blocking the submit queue. Since you no longer need to change the admission logic, there are parts in your original PR that's not needed anymore. I think reverting makes sense here, and will make cherrypicking easier too.

[yujuhong]

I feel like I've seen bad behaviour with full disks, so it feels like this is actually exposing a pre-existing problem. I don't understand why we would permanently reject a static pod if the disk was full?

This is a bug, and it should be fixed. I changed the title to reflect that (thanks). The immediate solution if anyone encounters such a situation is to restart kubelet to let the pods go through the admission process again.

@justinsb I don't think your new PR needs to change the admission logic at all, so it shouldn't be affected by this bug.

[timstclair]

Related PR: #35342

Conditions which the scheduler is not aware of, or that should be retried on the same node should be "soft rejected", i.e. the pod should not be marked as failed, instead it should be blocked from starting.

[dchen1107]

Related to: #22212

[fejta-bot]

Issues go stale after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle rotten
/remove-lifecycle stale

[wgahnagl]

/kind bug

[gjkim42]

/assign

[gjkim42]

/unassign

I don't want to make another exception for static pods...

/priority backlog

[ehashman]

/milestone clear

[SergeyKanzhelev]

/triage needs-information

need repro on the latest versions of confirmation from somebody experiencing it

[matthyx]

Please reopen if you experience this issue.
/close

[k8s-ci-robot]

@matthyx: Closing this issue.

In response to this:

Please reopen if you experience this issue.
/close

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.