kubelet: Pods created and rapidly terminated get stuck

[rphillips]

What type of PR is this?
/kind bug
/sig node

What this PR does / why we need it:
This PR fixes the race when a pod gets terminated but crio has not had the chance to clean up the cgroup yet. The podKiller will register all pods into a map and bypass the cgroup cleanup in cleanupOrphanedPodCgroups if the kill is still pending.

In Openshift, we are seeing an issue where pods can be deleted from the API and the cgroup is torn down before the pod is completely killed. This is due to the Kubelet seeing the terminated pod from the API and the housekeeping routine (runs every 2 seconds) starts cleaning up the cgroup (cleanupOrphanedPodCgroups) if cgroupPerQOS is enabled.

The fix skips over the cgroup cleanup if the pod is being killed by podKiller. Once the pod exits the podKiller then pcm.Destroy is called within cleanupOrphanedPodCgroups.

• Adds mock for FakeContainerManager
• Adds mock for PodContainerManager
• Unit test

Which issue(s) this PR fixes:
Fixes #98142

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[k8s-ci-robot]

@rphillips: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

[rphillips]

/retest

[ehashman]

/cc @SergeyKanzhelev

[ehashman]

This generally looks like an improvement to me (yay thread safety!!)

I'd love to see what this looks like against some of the e2es that are frequently failing. @rphillips I think some of them are OpenShift-only, do you want me to try porting them back to k8s? Maybe I can run them against this patch in another DNM PR?

[rphillips]

/test pull-kubernetes-e2e-gce-ubuntu-containerd

[rphillips]

/retest

[rphillips]

Added a lock for the final validation step.
https://github.com/kubernetes/kubernetes/pull/98424/files#diff-ab8f7cf0865da30dc5a3c29c539591dff7fd7d8d9202df58206b7d70b37fae2aR459

[sjenning]

/lgtm

[ehashman]

/lgtm

[rphillips]

/test pull-kubernetes-e2e-kind

[k8s-ci-robot]

@rphillips: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubernetes-bazel-test	f918e11	link	/test pull-kubernetes-bazel-test

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.

[jingxu97]

I think this PR could consider cherrypick?

[jingxu97]

@rphillips Just want to confirm, without this fix, the issue is pod is sometimes stuck in terminating phase due to cgroup clean up issue? Thanks!

[ehashman]

@jingxu97 it's a pretty big change, I'm not sure if we want to cherry-pick this. There are some other improvements like #98933 going in that would be good to land together with it.