if run sandbox failed, clean up; deduplicate the default mounts with user defined ones #1468
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…user defined ones Signed-off-by: YaoZengzeng <yaozengzeng@zju.edu.cn>
Codecov Report
@@ Coverage Diff @@
## master #1468 +/- ##
=========================================
- Coverage 40.12% 40.03% -0.1%
=========================================
Files 250 250
Lines 16352 16390 +38
=========================================
Hits 6561 6561
- Misses 8949 8983 +34
- Partials 842 846 +4
|
|
@allencloud PTAL |
allencloud
reviewed
Jun 11, 2018
| defer func() { | ||
| // If running sandbox failed, clean up the sandbox directory. | ||
| if retErr != nil { | ||
| os.RemoveAll(sandboxRootDir) |
There was a problem hiding this comment.
How about declaring the variable sandboxRootDir before the first defer func(), and then I think we could combine the two defers into a single one. WDYT? @YaoZengzeng
There was a problem hiding this comment.
Actually we can't, because sandboxRootDir := path.Join(c.SandboxBaseDir, id) and we should get the container ID first :)
|
the design LGTM, tiny comments there. |
|
LGTM |
pouchrobot
added
the
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: YaoZengzeng yaozengzeng@zju.edu.cn
Ⅰ. Describe what this PR did
When I try to launch a pod of flannel, there are two problems:
flannel pod will mount the
/runin the host to the container's/run, which is duplicate with the default mounts created by pouchd and we should override the default one.When an error occurred in the process of running a sandbox (for example, network not initialized), we should delete the running sandbox container. Otherwise kubelet will list the sandboxes and to wrongly assume the error sandbox is running which is not expected.
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how you did it
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews