diff --git a/container_linux_test.go b/container_linux_test.go index f70fb5bb3..093225884 100644 --- a/container_linux_test.go +++ b/container_linux_test.go @@ -896,3 +896,79 @@ func TestContainerRuntimeOptions(t *testing.T) { t.Errorf("task creation should have failed because of lack of executable. Instead failed with: %v", err.Error()) } } + +func TestContainerKillInitPidHost(t *testing.T) { + client, err := newClient(t, address) + if err != nil { + t.Fatal(err) + } + defer client.Close() + + var ( + image Image + ctx, cancel = testContext() + id = t.Name() + ) + defer cancel() + + image, err = client.GetImage(ctx, testImage) + if err != nil { + t.Error(err) + return + } + + container, err := client.NewContainer(ctx, id, + withNewSnapshot(id, image), + WithNewSpec(withImageConfig(image), + withProcessArgs("sh", "-c", "sleep 42; echo hi"), + WithHostNamespace(specs.PIDNamespace), + ), + ) + if err != nil { + t.Error(err) + return + } + defer container.Delete(ctx, WithSnapshotCleanup) + + stdout := bytes.NewBuffer(nil) + task, err := container.NewTask(ctx, NewIO(bytes.NewBuffer(nil), stdout, bytes.NewBuffer(nil))) + if err != nil { + t.Error(err) + return + } + defer task.Delete(ctx) + + statusC, err := task.Wait(ctx) + if err != nil { + t.Error(err) + return + } + + if err := task.Start(ctx); err != nil { + t.Error(err) + return + } + + if err := task.Kill(ctx, syscall.SIGKILL); err != nil { + t.Error(err) + } + + // Give the shim time to reap the init process and kill the orphans + select { + case <-statusC: + case <-time.After(100 * time.Millisecond): + } + + b, err := exec.Command("ps", "ax").CombinedOutput() + if err != nil { + t.Fatal(err) + } + + if strings.Contains(string(b), "sleep 42") { + t.Fatalf("killing init didn't kill all its children:\n%v", string(b)) + } + + if _, err := task.Delete(ctx, WithProcessKill); err != nil { + t.Error(err) + } +} diff --git a/linux/runtime.go b/linux/runtime.go index f1764ca71..3e3d540a3 100644 --- a/linux/runtime.go +++ b/linux/runtime.go @@ -367,7 +367,10 @@ func (r *Runtime) loadTasks(ctx context.Context, ns string) ([]*Task, error) { func (r *Runtime) cleanupAfterDeadShim(ctx context.Context, bundle *bundle, ns, id string, pid int, ec chan runc.Exit) error { ctx = namespaces.WithNamespace(ctx, ns) if err := r.terminate(ctx, bundle, ns, id); err != nil { - return errors.New("failed to terminate task, leaving bundle for debugging") + if r.config.ShimDebug { + return errors.Wrap(err, "failed to terminate task, leaving bundle for debugging") + } + log.G(ctx).WithError(err).Warn("failed to terminate task") } if ec != nil { diff --git a/linux/shim/service.go b/linux/shim/service.go index 874babb9e..fc4d0993e 100644 --- a/linux/shim/service.go +++ b/linux/shim/service.go @@ -393,6 +393,14 @@ func (s *Service) checkProcesses(e runc.Exit) { defer s.mu.Unlock() for _, p := range s.processes { if p.Pid() == e.Pid { + if ip, ok := p.(*initProcess); ok { + // Ensure all children are killed + if err := ip.killAll(s.context); err != nil { + log.G(s.context).WithError(err).WithField("id", ip.ID()). + Error("failed to kill init's children") + } + } + p.SetExited(e.Status) s.events <- &eventsapi.TaskExit{ ContainerID: s.id, diff --git a/reaper/reaper.go b/reaper/reaper.go index 97df901dc..76efc6199 100644 --- a/reaper/reaper.go +++ b/reaper/reaper.go @@ -3,7 +3,6 @@ package reaper import ( - "bytes" "os/exec" "sync" "time" @@ -47,23 +46,6 @@ type Monitor struct { subscribers map[chan runc.Exit]struct{} } -func (m *Monitor) Output(c *exec.Cmd) ([]byte, error) { - var b bytes.Buffer - c.Stdout = &b - if err := m.Run(c); err != nil { - return nil, err - } - return b.Bytes(), nil -} - -func (m *Monitor) CombinedOutput(c *exec.Cmd) ([]byte, error) { - var b bytes.Buffer - c.Stdout = &b - c.Stderr = &b - err := m.Run(c) - return b.Bytes(), err -} - // Start starts the command a registers the process with the reaper func (m *Monitor) Start(c *exec.Cmd) (chan runc.Exit, error) { ec := m.Subscribe() @@ -74,16 +56,9 @@ func (m *Monitor) Start(c *exec.Cmd) (chan runc.Exit, error) { return ec, nil } -// Run runs and waits for the command to finish -func (m *Monitor) Run(c *exec.Cmd) error { - ec, err := m.Start(c) - if err != nil { - return err - } - _, err = m.Wait(c, ec) - return err -} - +// Wait blocks until a process is signal as dead. +// User should rely on the value of the exit status to determine if the +// command was successful or not. func (m *Monitor) Wait(c *exec.Cmd, ec chan runc.Exit) (int, error) { for e := range ec { if e.Pid == c.Process.Pid { diff --git a/vendor.conf b/vendor.conf index 7c58b0334..e2c64d0c7 100644 --- a/vendor.conf +++ b/vendor.conf @@ -1,5 +1,5 @@ github.com/coreos/go-systemd 48702e0da86bd25e76cfef347e2adeb434a0d0a6 -github.com/containerd/go-runc e103f453ff3db23ec69d31371cadc1ea0ce87ec0 +github.com/containerd/go-runc ba22f6a82e52be3be4eb4a00000fe816f4b41c2e github.com/containerd/console 76d18fd1d66972718ab2284449591db0b3cdb4de github.com/containerd/cgroups e6d1aa8c71c6103624b2c6e6f4be0863b67027f1 github.com/docker/go-metrics 8fd5772bf1584597834c6f7961a530f06cbfbb87 diff --git a/vendor/github.com/containerd/go-runc/monitor.go b/vendor/github.com/containerd/go-runc/monitor.go index 4ad539927..2d62c5a41 100644 --- a/vendor/github.com/containerd/go-runc/monitor.go +++ b/vendor/github.com/containerd/go-runc/monitor.go @@ -22,9 +22,6 @@ type Exit struct { // These methods should match the methods exposed by exec.Cmd to provide // a consistent experience for the caller type ProcessMonitor interface { - Output(*exec.Cmd) ([]byte, error) - CombinedOutput(*exec.Cmd) ([]byte, error) - Run(*exec.Cmd) error Start(*exec.Cmd) (chan Exit, error) Wait(*exec.Cmd, chan Exit) (int, error) } @@ -32,18 +29,6 @@ type ProcessMonitor interface { type defaultMonitor struct { } -func (m *defaultMonitor) Output(c *exec.Cmd) ([]byte, error) { - return c.Output() -} - -func (m *defaultMonitor) CombinedOutput(c *exec.Cmd) ([]byte, error) { - return c.CombinedOutput() -} - -func (m *defaultMonitor) Run(c *exec.Cmd) error { - return c.Run() -} - func (m *defaultMonitor) Start(c *exec.Cmd) (chan Exit, error) { if err := c.Start(); err != nil { return nil, err diff --git a/vendor/github.com/containerd/go-runc/runc.go b/vendor/github.com/containerd/go-runc/runc.go index 92218db42..d8ae4eec0 100644 --- a/vendor/github.com/containerd/go-runc/runc.go +++ b/vendor/github.com/containerd/go-runc/runc.go @@ -46,7 +46,7 @@ type Runc struct { // List returns all containers created inside the provided runc root directory func (r *Runc) List(context context.Context) ([]*Container, error) { - data, err := Monitor.Output(r.command(context, "list", "--format=json")) + data, err := cmdOutput(r.command(context, "list", "--format=json"), false) if err != nil { return nil, err } @@ -59,7 +59,7 @@ func (r *Runc) List(context context.Context) ([]*Container, error) { // State returns the state for the container provided by id func (r *Runc) State(context context.Context, id string) (*Container, error) { - data, err := Monitor.CombinedOutput(r.command(context, "state", id)) + data, err := cmdOutput(r.command(context, "state", id), true) if err != nil { return nil, fmt.Errorf("%s: %s", err, data) } @@ -128,7 +128,7 @@ func (r *Runc) Create(context context.Context, id, bundle string, opts *CreateOp cmd.ExtraFiles = opts.ExtraFiles if cmd.Stdout == nil && cmd.Stderr == nil { - data, err := Monitor.CombinedOutput(cmd) + data, err := cmdOutput(cmd, true) if err != nil { return fmt.Errorf("%s: %s", err, data) } @@ -145,7 +145,10 @@ func (r *Runc) Create(context context.Context, id, bundle string, opts *CreateOp } } } - _, err = Monitor.Wait(cmd, ec) + status, err := Monitor.Wait(cmd, ec) + if err == nil && status != 0 { + err = fmt.Errorf("%s did not terminate sucessfully", cmd.Args[0]) + } return err } @@ -204,7 +207,7 @@ func (r *Runc) Exec(context context.Context, id string, spec specs.Process, opts opts.Set(cmd) } if cmd.Stdout == nil && cmd.Stderr == nil { - data, err := Monitor.CombinedOutput(cmd) + data, err := cmdOutput(cmd, true) if err != nil { return fmt.Errorf("%s: %s", err, data) } @@ -363,7 +366,7 @@ func (r *Runc) Resume(context context.Context, id string) error { // Ps lists all the processes inside the container returning their pids func (r *Runc) Ps(context context.Context, id string) ([]int, error) { - data, err := Monitor.CombinedOutput(r.command(context, "ps", "--format", "json", id)) + data, err := cmdOutput(r.command(context, "ps", "--format", "json", id), true) if err != nil { return nil, fmt.Errorf("%s: %s", err, data) } @@ -546,7 +549,7 @@ type Version struct { // Version returns the runc and runtime-spec versions func (r *Runc) Version(context context.Context) (Version, error) { - data, err := Monitor.Output(r.command(context, "--version")) + data, err := cmdOutput(r.command(context, "--version"), false) if err != nil { return Version{}, err } @@ -614,11 +617,39 @@ func (r *Runc) args() (out []string) { // func (r *Runc) runOrError(cmd *exec.Cmd) error { if cmd.Stdout != nil || cmd.Stderr != nil { - return Monitor.Run(cmd) + ec, err := Monitor.Start(cmd) + if err != nil { + return err + } + status, err := Monitor.Wait(cmd, ec) + if err == nil && status != 0 { + err = fmt.Errorf("%s did not terminate sucessfully", cmd.Args[0]) + } + return err } - data, err := Monitor.CombinedOutput(cmd) + data, err := cmdOutput(cmd, true) if err != nil { return fmt.Errorf("%s: %s", err, data) } return nil } + +func cmdOutput(cmd *exec.Cmd, combined bool) ([]byte, error) { + var b bytes.Buffer + + cmd.Stdout = &b + if combined { + cmd.Stderr = &b + } + ec, err := Monitor.Start(cmd) + if err != nil { + return nil, err + } + + status, err := Monitor.Wait(cmd, ec) + if err == nil && status != 0 { + err = fmt.Errorf("%s did not terminate sucessfully", cmd.Args[0]) + } + + return b.Bytes(), err +}