Log shim panic logs before shim cleanup #16
Merged
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
|
@ambarve Bundle cleanup always gets called when cleaning up a container but shim delete doesn't, is that correct? So in most cases we'd get the shim panic logs logged twice? Once on shim delete and once on bundle cleanup when reading the file itself? |
msscotb
reviewed
May 5, 2021
msscotb
reviewed
May 5, 2021
msscotb
reviewed
May 5, 2021
|
@dcantah You are right, I was planning on removing that code from the shim delete command, I will push a PR for that. |
|
@ambarve Cool thanks for clarification! |
|
@dcantah Actually we shouldn't remove the logging changes added to hcsshim. This is because in some cases containerd calls shim delete first before calling delete on the bundle. In that case the shim delete command will remove the bundle directory and bundle cleanup will never find panic.log file. |
|
@ambarve Cool thanks for checking on this |
ambarve
force-pushed
the
shim_panic_log
branch
2 times, most recently
from
aec59f7 to
3b7de65
Compare
ambarve
commented
May 14, 2021
katiewasnothere
approved these changes
May 18, 2021
dcantah
approved these changes
May 18, 2021
We already have code in hcsshim that logs the contents of the panic.log file during shim delete command. However, that doesn't handle all of the cases. This change should fix that. Signed-off-by: Amit Barve <ambarve@microsoft.com>
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.
Changes the bundle cleanup function to look for a file named panic.log and log the contents of that file if it exists.
We already added a change to hcsshim to log the contents of panic.log file during
shim deletecommand. However, that doesn't handle all of the cases. This change should fix that.Signed-off-by: Amit Barve ambarve@microsoft.com