UPSTREAM: 62874: dockershim/sandbox: clean up pod network even if SetUpPod() failed #19576
Conversation
…UpPod() failed If the CNI network plugin completes successfully, but something fails between that success and dockerhsim's sandbox setup code, plugin resources may not be cleaned up. A non-trivial amount of code runs after the plugin itself exits and the CNI driver's SetUpPod() returns, and any error condition recognized by that code would cause this leakage. The Kubernetes CRI RunPodSandbox() request does not attempt to clean up on errors, since it cannot know how much (if any) networking was actually set up. It depends on the CRI implementation to do that cleanup for it. In the dockershim case, a SetUpPod() failure means networkReady is FALSE for the sandbox, and TearDownPod() will not be called later by garbage collection even though networking was configured, because dockershim can't know how far SetUpPod() got. Concrete examples include if the sandbox's container is somehow removed during during that time, or another OS error is encountered, or the plugin returns a malformed result to the CNI driver. Upstream: kubernetes/kubernetes#62874 Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1532965
openshift-ci-robot
added
the
size/S
|
/test gcp |
|
/test extended_conformance_install |
|
/lgtm |
|
@liggitt @smarterclayton review/approve on this one if you could please, thanks! |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dcbw, smarterclayton 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 |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
8 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
If the CNI network plugin completes successfully, but something fails
between that success and dockerhsim's sandbox setup code, plugin resources
may not be cleaned up. A non-trivial amount of code runs after the
plugin itself exits and the CNI driver's SetUpPod() returns, and any error
condition recognized by that code would cause this leakage.
The Kubernetes CRI RunPodSandbox() request does not attempt to clean
up on errors, since it cannot know how much (if any) networking
was actually set up. It depends on the CRI implementation to do
that cleanup for it.
In the dockershim case, a SetUpPod() failure means networkReady is
FALSE for the sandbox, and TearDownPod() will not be called later by
garbage collection even though networking was configured, because
dockershim can't know how far SetUpPod() got.
Concrete examples include if the sandbox's container is somehow
removed during during that time, or another OS error is encountered,
or the plugin returns a malformed result to the CNI driver.
Upstream: kubernetes/kubernetes#62874
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1532965
@openshift/networking