From de2671b7f5c5ad8c06b6d993927bba8fe91546d7 Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Fri, 28 Jul 2017 12:08:58 +0100 Subject: [PATCH 1/2] bump vendor for containerd/console update Signed-off-by: Daniel Dao --- vendor.conf | 2 +- vendor/github.com/containerd/console/console.go | 2 ++ .../github.com/containerd/console/console_linux.go | 1 + .../github.com/containerd/console/console_unix.go | 13 +++++++++++++ .../containerd/console/console_windows.go | 7 +++++++ 5 files changed, 24 insertions(+), 1 deletion(-) diff --git a/vendor.conf b/vendor.conf index 8676d22ddab7..49662b308ef4 100644 --- a/vendor.conf +++ b/vendor.conf @@ -1,6 +1,6 @@ github.com/coreos/go-systemd 48702e0da86bd25e76cfef347e2adeb434a0d0a6 github.com/containerd/go-runc 2774a2ea124a5c2d0aba13b5c2dd8a5a9a48775d -github.com/containerd/console 7fed77e673ca4abcd0cbd6d4d0e0e22137cbd778 +github.com/containerd/console 2ce1c681f3c3c0dfa7d0af289428d36567c9a6bc github.com/containerd/cgroups 4fd64a776f25b5540cddcb72eea6e35e58baca6e github.com/docker/go-metrics 8fd5772bf1584597834c6f7961a530f06cbfbb87 github.com/docker/go-events 9461782956ad83b30282bf90e31fa6a70c255ba9 diff --git a/vendor/github.com/containerd/console/console.go b/vendor/github.com/containerd/console/console.go index c702a608f514..6f1cca966aa7 100644 --- a/vendor/github.com/containerd/console/console.go +++ b/vendor/github.com/containerd/console/console.go @@ -28,6 +28,8 @@ type Console interface { Size() (WinSize, error) // Fd returns the console's file descriptor Fd() uintptr + // Name returns the console's file name + Name() string } // WinSize specifies the window size of the console diff --git a/vendor/github.com/containerd/console/console_linux.go b/vendor/github.com/containerd/console/console_linux.go index b2000d7d8625..487ed44c04ea 100644 --- a/vendor/github.com/containerd/console/console_linux.go +++ b/vendor/github.com/containerd/console/console_linux.go @@ -1,4 +1,5 @@ // +build linux + package console import ( diff --git a/vendor/github.com/containerd/console/console_unix.go b/vendor/github.com/containerd/console/console_unix.go index baf390be49ab..0bd6b005ab79 100644 --- a/vendor/github.com/containerd/console/console_unix.go +++ b/vendor/github.com/containerd/console/console_unix.go @@ -118,6 +118,10 @@ func (m *master) Fd() uintptr { return m.f.Fd() } +func (m *master) Name() string { + return m.f.Name() +} + // checkConsole checks if the provided file is a console func checkConsole(f *os.File) error { var termios unix.Termios @@ -132,3 +136,12 @@ func newMaster(f *os.File) Console { f: f, } } + +// SaneTerminal sets the necessary tty_ioctl(4)s to ensure that a pty pair +// created by us acts normally. In particular, a not-very-well-known default of +// Linux unix98 ptys is that they have +onlcr by default. While this isn't a +// problem for terminal emulators, because we relay data from the terminal we +// also relay that funky line discipline. +func SaneTerminal(f *os.File) error { + return saneTerminal(f) +} diff --git a/vendor/github.com/containerd/console/console_windows.go b/vendor/github.com/containerd/console/console_windows.go index af3ea9e0e7c9..2e644cba9d4a 100644 --- a/vendor/github.com/containerd/console/console_windows.go +++ b/vendor/github.com/containerd/console/console_windows.go @@ -154,6 +154,13 @@ func (m *master) Fd() uintptr { return m.in } +// on windows, console can only be made from os.Std{in,out,err}, hence there +// isnt a single name here we can use. Return a dummy "console" value in this +// case should be sufficient. +func (m *master) Name() string { + return "console" +} + // makeInputRaw puts the terminal (Windows Console) connected to the given // file descriptor into raw mode func makeInputRaw(fd uintptr, mode uint32) error { From 8e53465842146664900507612fd22423e23c91ed Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Fri, 28 Jul 2017 12:09:13 +0100 Subject: [PATCH 2/2] use epoll to manage console i/o in linux this adds a `platform` interface for shim service to manage platform-specific behaviors such as I/O (which uses epoll in linux to work around bugs with applications that closes all consoles i.e. https://github.com/opencontainers/runc/pull/1434 and https://github.com/moby/moby/issues/27202) Its expected that we only have 1 epollfd per containerd_shim to manage all processes. Since all the work are done outside of the container runtime, upgrading of runc is not required and should be done separately. Signed-off-by: Daniel Dao --- linux/shim/exec.go | 6 ++- linux/shim/init.go | 42 ++++++++++-------- linux/shim/service.go | 14 +++++- linux/shim/service_linux.go | 87 +++++++++++++++++++++++++++++++++++++ linux/shim/service_unix.go | 58 +++++++++++++++++++++++++ 5 files changed, 186 insertions(+), 21 deletions(-) create mode 100644 linux/shim/service_linux.go create mode 100644 linux/shim/service_unix.go diff --git a/linux/shim/exec.go b/linux/shim/exec.go index 41e4924ececf..c031298f9b79 100644 --- a/linux/shim/exec.go +++ b/linux/shim/exec.go @@ -105,10 +105,11 @@ func newExecProcess(context context.Context, path string, r *shimapi.ExecProcess if err != nil { return nil, errors.Wrap(err, "failed to retrieve console master") } - e.console = console - if err := copyConsole(context, console, r.Stdin, r.Stdout, r.Stderr, &e.WaitGroup, ©WaitGroup); err != nil { + console, err = e.parent.platform.copyConsole(context, console, r.Stdin, r.Stdout, r.Stderr, &e.WaitGroup, ©WaitGroup) + if err != nil { return nil, errors.Wrap(err, "failed to start console copy") } + e.console = console } else { if err := copyPipes(context, io, r.Stdin, r.Stdout, r.Stderr, &e.WaitGroup, ©WaitGroup); err != nil { return nil, errors.Wrap(err, "failed to start io pipe copy") @@ -142,6 +143,7 @@ func (e *execProcess) ExitedAt() time.Time { func (e *execProcess) Exited(status int) { e.status = status e.exited = time.Now() + e.parent.platform.shutdownConsole(context.Background(), e.console) e.Wait() if e.io != nil { for _, c := range e.closers { diff --git a/linux/shim/init.go b/linux/shim/init.go index d1eb0a86e5e4..452c1ff53aa8 100644 --- a/linux/shim/init.go +++ b/linux/shim/init.go @@ -39,21 +39,22 @@ type initProcess struct { // the reaper interface. mu sync.Mutex - id string - bundle string - console console.Console - io runc.IO - runtime *runc.Runc - status int - exited time.Time - pid int - closers []io.Closer - stdin io.Closer - stdio stdio - rootfs string + id string + bundle string + console console.Console + platform platform + io runc.IO + runtime *runc.Runc + status int + exited time.Time + pid int + closers []io.Closer + stdin io.Closer + stdio stdio + rootfs string } -func newInitProcess(context context.Context, path, namespace string, r *shimapi.CreateTaskRequest) (*initProcess, error) { +func newInitProcess(context context.Context, plat platform, path, namespace string, r *shimapi.CreateTaskRequest) (*initProcess, error) { var success bool if err := identifiers.Validate(r.ID); err != nil { @@ -98,9 +99,10 @@ func newInitProcess(context context.Context, path, namespace string, r *shimapi. Root: filepath.Join(RuncRoot, namespace), } p := &initProcess{ - id: r.ID, - bundle: r.Bundle, - runtime: runtime, + id: r.ID, + bundle: r.Bundle, + runtime: runtime, + platform: plat, stdio: stdio{ stdin: r.Stdin, stdout: r.Stdout, @@ -170,10 +172,11 @@ func newInitProcess(context context.Context, path, namespace string, r *shimapi. if err != nil { return nil, errors.Wrap(err, "failed to retrieve console master") } - p.console = console - if err := copyConsole(context, console, r.Stdin, r.Stdout, r.Stderr, &p.WaitGroup, ©WaitGroup); err != nil { + console, err = plat.copyConsole(context, console, r.Stdin, r.Stdout, r.Stderr, &p.WaitGroup, ©WaitGroup) + if err != nil { return nil, errors.Wrap(err, "failed to start console copy") } + p.console = console } else { if err := copyPipes(context, io, r.Stdin, r.Stdout, r.Stderr, &p.WaitGroup, ©WaitGroup); err != nil { return nil, errors.Wrap(err, "failed to start io pipe copy") @@ -238,6 +241,9 @@ func (p *initProcess) Delete(context context.Context) error { return fmt.Errorf("cannot delete a running container") } p.killAll(context) + if err := p.platform.shutdownConsole(context, p.console); err != nil { + log.G(context).WithError(err).Warn("Failed to shutdown container console") + } p.Wait() err = p.runtime.Delete(context, p.id, nil) if p.io != nil { diff --git a/linux/shim/service.go b/linux/shim/service.go index 5555394a881d..21b0488b7228 100644 --- a/linux/shim/service.go +++ b/linux/shim/service.go @@ -56,10 +56,20 @@ func NewService(path, namespace, address string) (*Service, error) { namespace: namespace, context: context, } + if err := s.initPlatform(); err != nil { + return nil, errors.Wrap(err, "failed to initialized platform behavior") + } go s.forward(client) return s, nil } +// platform handles platform-specific behavior that may differs across +// platform implementations +type platform interface { + copyConsole(ctx context.Context, console console.Console, stdin, stdout, stderr string, wg, cwg *sync.WaitGroup) (console.Console, error) + shutdownConsole(ctx context.Context, console console.Console) error +} + type Service struct { initProcess *initProcess path string @@ -72,10 +82,12 @@ type Service struct { deferredEvent interface{} namespace string context context.Context + + platform platform } func (s *Service) Create(ctx context.Context, r *shimapi.CreateTaskRequest) (*shimapi.CreateTaskResponse, error) { - process, err := newInitProcess(ctx, s.path, s.namespace, r) + process, err := newInitProcess(ctx, s.platform, s.path, s.namespace, r) if err != nil { return nil, errdefs.ToGRPC(err) } diff --git a/linux/shim/service_linux.go b/linux/shim/service_linux.go new file mode 100644 index 000000000000..2c46481e0cc8 --- /dev/null +++ b/linux/shim/service_linux.go @@ -0,0 +1,87 @@ +package shim + +import ( + "io" + "sync" + "syscall" + + "github.com/containerd/console" + "github.com/containerd/fifo" + "github.com/pkg/errors" + "golang.org/x/net/context" +) + +type linuxPlatform struct { + epoller *console.Epoller +} + +func (p *linuxPlatform) copyConsole(ctx context.Context, console console.Console, stdin, stdout, stderr string, wg, cwg *sync.WaitGroup) (console.Console, error) { + if p.epoller == nil { + return nil, errors.New("uninitialized epoller") + } + + epollConsole, err := p.epoller.Add(console) + if err != nil { + return nil, err + } + + if stdin != "" { + in, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY, 0) + if err != nil { + return nil, err + } + cwg.Add(1) + go func() { + cwg.Done() + io.Copy(epollConsole, in) + }() + } + + outw, err := fifo.OpenFifo(ctx, stdout, syscall.O_WRONLY, 0) + if err != nil { + return nil, err + } + outr, err := fifo.OpenFifo(ctx, stdout, syscall.O_RDONLY, 0) + if err != nil { + return nil, err + } + wg.Add(1) + cwg.Add(1) + go func() { + cwg.Done() + io.Copy(outw, epollConsole) + epollConsole.Close() + outr.Close() + outw.Close() + wg.Done() + }() + return epollConsole, nil +} + +func (p *linuxPlatform) shutdownConsole(ctx context.Context, cons console.Console) error { + if p.epoller == nil { + return errors.New("uninitialized epoller") + } + epollConsole, ok := cons.(*console.EpollConsole) + if !ok { + return errors.Errorf("expected EpollConsole, got %#v", cons) + } + return epollConsole.Shutdown(p.epoller.CloseConsole) +} + +// initialize a single epoll fd to manage our consoles. `initPlatform` should +// only be called once. +func (s *Service) initPlatform() error { + if s.platform != nil { + return nil + } + epoller, err := console.NewEpoller() + if err != nil { + return errors.Wrap(err, "failed to initialize epoller") + } + s.platform = &linuxPlatform{ + epoller: epoller, + } + go epoller.Wait() + return nil +} diff --git a/linux/shim/service_unix.go b/linux/shim/service_unix.go new file mode 100644 index 000000000000..a73786a9df48 --- /dev/null +++ b/linux/shim/service_unix.go @@ -0,0 +1,58 @@ +// +build !windows,!linux + +package shim + +import ( + "io" + "sync" + "syscall" + + "github.com/containerd/console" + "github.com/containerd/fifo" + "golang.org/x/net/context" +) + +type unixPlatform struct { +} + +func (p *unixPlatform) copyConsole(ctx context.Context, console console.Console, stdin, stdout, stderr string, wg, cwg *sync.WaitGroup) (console.Console, error) { + if stdin != "" { + in, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY, 0) + if err != nil { + return nil, err + } + cwg.Add(1) + go func() { + cwg.Done() + io.Copy(console, in) + }() + } + outw, err := fifo.OpenFifo(ctx, stdout, syscall.O_WRONLY, 0) + if err != nil { + return nil, err + } + outr, err := fifo.OpenFifo(ctx, stdout, syscall.O_RDONLY, 0) + if err != nil { + return nil, err + } + wg.Add(1) + cwg.Add(1) + go func() { + cwg.Done() + io.Copy(outw, console) + console.Close() + outr.Close() + outw.Close() + wg.Done() + }() + return console, nil +} + +func (p *unixPlatform) shutdownConsole(ctx context.Context, cons console.Console) error { + return nil +} + +func (s *Service) initPlatform() error { + s.platform = &unixPlatform{} + return nil +}