Add shim log pipe for log forwarding to the daemon#2528
Add shim log pipe for log forwarding to the daemon#2528dmcgowan merged 1 commit intocontainerd:masterfrom
Conversation
crosbymichael
force-pushed
the
shim-debug
branch
from
807344f to
84363fd
Compare
There was a problem hiding this comment.
Doesn't this leak the listener?
There was a problem hiding this comment.
Same here. This leaks the listener right?
There was a problem hiding this comment.
It would also be more useful I think to name it like: util_windows.go so it can be predicted?
fmt.Sprintf("\\\\.\\pipe\\containerd-shim-%s-%s-log-pipe", ns, id)
jterry75
left a comment
jterry75
left a comment
There was a problem hiding this comment.
Minor feedback otherwise looks pretty good
Codecov Report
@@ Coverage Diff @@
## master #2528 +/- ##
=======================================
Coverage 45.05% 45.05%
=======================================
Files 94 94
Lines 9796 9796
=======================================
Hits 4414 4414
Misses 4662 4662
Partials 720 720
Continue to review full report at Codecov.
|
crosbymichael
force-pushed
the
shim-debug
branch
from
84363fd to
7932b7c
Compare
|
@jterry75 updated based on your feedback |
|
LGTM |
crosbymichael
force-pushed
the
shim-debug
branch
from
7932b7c to
309881d
Compare
There was a problem hiding this comment.
Oh I didn't understand this correctly the first time. I think you want this side to be the winio.DialPipe side for the client connection. The connection is then an io.Writer and the listener is part of the v2/shim.go
There was a problem hiding this comment.
What happens if the listener and other end of the pipe goes away then comes back, this connection connection is still maintained?
There was a problem hiding this comment.
Its a good question. I think that we will have to create some sort of reconnect logic for Windows if the containerd daemon goes down when it reconnects to shims they will re-open the log connection but this is a good start.
|
Ok 1 last comment and I think this is good |
A fifo on unix or named pipe on Windows will be provided to the shim. It can be located inside the `cwd` of the shim named "log". The shims can use the existing `github.com/containerd/containerd/log` package to log debug messages. Messages will automatically be output in the containerd's daemon logs with the correct fiels and runtime set. Signed-off-by: Michael Crosby <[email protected]>
crosbymichael
force-pushed
the
shim-debug
branch
from
309881d to
6ba4ddf
Compare
|
LGTM |
|
Sounds like there is some follow up to make sure this is working on Windows when containerd restarts, for now though LGTM |
5 participants
A fifo on unix or named pipe on Windows will be provided to the shim.
It can be located inside the
cwdof the shim named "log".The shims can use the existing
github.com/containerd/containerd/logpackage to log debug messages.Messages will automatically be output in the containerd's daemon logs with the correct fiels and runtime set.
Using a pipe allows containerd to reconnect after a reboot and collect logs from shims.
Signed-off-by: Michael Crosby [email protected]