Fix container cleanup on daemon restart#36249
Conversation
There was a problem hiding this comment.
Is this check necessary? won't it fail on the next line, and the pid will be in the pathError message?
There was a problem hiding this comment.
I think this (line 62-79) should be extracted into a helper so it doesn't distract from the test logic:
ppid := getContainerdShimPID(t, inspect.State.Pid)There was a problem hiding this comment.
I'm not really sure where this test should go. It seems like it has more to do with daemon state than the container API/.
s/daemon_linux_test/daemon_test.go/ and add a skip.If() because the relevant OS is where the daemon is running, not where the test client is running, so this would be skipped incorrectly if the client was windows, and the daemon was linux.
There was a problem hiding this comment.
This test will not compile on Windows and requires being on the same host since it's starts up a new daemon, kills processes, etc.
There was a problem hiding this comment.
It seems like it has more to do with daemon state than the container API
It's kind of everything, sadly.
16a80cc to
d3c4444
Compare
When the daemon restores containers on daemon restart, it syncs up with containerd to determine the existing state. For stopped containers it then removes the container metadata from containerd. In some cases this is not handled properly and causes an error when someone attempts to start that container again. In particular, this case is just a bad error check. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
d3c4444 to
c0d56ab
Compare
|
Updated |
When the daemon restores containers on daemon restart, it syncs up with
containerd to determine the existing state. For stopped containers it
then removes the container metadata from containerd.
In some cases this is not handled properly and causes an error when
someone attempts to start that container again.
In particular, this case is just a bad error check.
Fixes #36145