-
Notifications
You must be signed in to change notification settings - Fork 949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: return error if it fails to tear down network in stopsandbox #2836
Conversation
Signed-off-by: zhuangqh <zhuangqhc@gmail.com>
treat it as success if the the netns dont exists, otherwise, return error kubelet will retry the StopPodSandbox method, avoiding cni leaking Signed-off-by: zhuangqh <zhuangqhc@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #2836 +/- ##
=========================================
+ Coverage 69.09% 69.1% +<.01%
=========================================
Files 285 285
Lines 17779 17780 +1
=========================================
+ Hits 12285 12287 +2
+ Misses 4097 4095 -2
- Partials 1397 1398 +1
|
@@ -473,7 +473,7 @@ func (c *CriManager) StopPodSandbox(ctx context.Context, r *runtime.StopPodSandb | |||
// Teardown network of the pod, if it is not in host network mode. | |||
if sandboxNetworkMode(sandboxMeta.Config) != runtime.NamespaceMode_NODE { | |||
if err = c.teardownNetwork(podSandboxID, sandboxMeta.NetNS, sandboxMeta.Config); err != nil { | |||
logrus.Warnf("failed to find network namespace file %s of sandbox %s which may have been already stopped", sandboxMeta.NetNS, podSandboxID) | |||
return nil, fmt.Errorf("failed to teardown network of sandbox %s, ns path: %v", podSandboxID, sandboxMeta.NetNS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not output the detailed err?
if _, err = os.Stat(podNetwork.NetNS); err != nil { | ||
if os.IsNotExist(err) { | ||
logrus.Warnf("failed to find network namespace file %s of sandbox %s", podNetwork.NetNS, podNetwork.ID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not output the detailed error?
LGTM |
Ⅰ. Describe what this PR did
treat it as success if the the netns dont exists, otherwise, return error
kubelet will retry the StopPodSandbox method, avoiding cni leaking
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews