Add separate console socket#1356
Conversation
crosbymichael
force-pushed
the
console-socket
branch
3 times, most recently
from
5f5325c to
69c82f9
Compare
|
@cyphar if you are checking out prs can you take a look at this one? I still have to fix a weird mount test pr but overall it looks good |
|
Yup, will do. |
crosbymichael
force-pushed
the
console-socket
branch
from
69c82f9 to
89b140c
Compare
| // relates to (to allow for consumers to use a single unix socket to handle | ||
| // multiple containers). This structure will probably move to runtime-spec | ||
| // at some point. But for now it lies in libcontainer. | ||
| type TerminalInfo struct { |
There was a problem hiding this comment.
The reason for this was so that you could use a single console-socket and listen for multiple instances of runc passing file descriptors. Without this, you can't tell which container a particular pty comes from. This was one of the concerns raised about the scalability of console-socket -- how do you handle starting hundreds of containers simultaneously if you keep hammering bind and socket?
There was a problem hiding this comment.
Do we know for a fact that it is a slow point in the execution or just a premature optimization?
There was a problem hiding this comment.
To be honest I didn't benchmark it, but it seemed like a useful optimisation because it allowed us to also pass around information about the container to the caller (rather than just a file descriptor). But if you feel that's not necessary, we can implement it in cri-o with a new socket for every conmon and presumably you can do the same thing in containerd-shim.
There was a problem hiding this comment.
I think we can create a new socket for every container for now, at least before we figure out the double socket issue.
|
I'll review this by tomorrow. |
Signed-off-by: Michael Crosby <[email protected]>
This maybe a nice extra but it adds complication to the usecase. The contract is listen on the socket and you get an fd to the pty master and that is that. Signed-off-by: Michael Crosby <[email protected]>
crosbymichael
force-pushed
the
console-socket
branch
from
c0c2125 to
957ef9c
Compare
|
LGTM |
|
Looks much cleaner. LGTM |
These were added in 244c9fc (*: console rewrite, 2016-06-04, opencontainers#1018) alongside procConsole and the associated handling. procConsole and that handling were removed in 00a0ecf (Add separate console socket, 2017-03-02, opencontainers#1356), but 00a0ecf missed this comment. Signed-off-by: W. Trevor King <[email protected]>
4 participants
Fixes #1302
Signed-off-by: Michael Crosby [email protected]