From e55bead518e4c72cdecf7de2e49db6c477cb58eb Mon Sep 17 00:00:00 2001
From: Brian Goff <cpuguy83@gmail.com>
Date: Fri, 15 Dec 2017 10:00:15 -0500
Subject: [PATCH] Fix error handling for kill/process not found

With the contianerd 1.0 migration we now have strongly typed errors that
we can check for process not found.
We also had some bad error checks looking for `ESRCH` which would only
be returned from `unix.Kill` and never from containerd even though we
were checking containerd responses for it.

Fixes some race conditions around process handling and our error checks
that could lead to errors that propagate up to the user that should not.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
---
 daemon/kill.go                 | 14 +++++---------
 libcontainerd/client_daemon.go | 22 +++++++++++++++-------
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/daemon/kill.go b/daemon/kill.go
index 1292f86b0c..5cde0d776d 100644
--- a/daemon/kill.go
+++ b/daemon/kill.go
@@ -4,10 +4,10 @@ import (
 	"context"
 	"fmt"
 	"runtime"
-	"strings"
 	"syscall"
 	"time"
 
+	"github.com/docker/docker/api/errdefs"
 	containerpkg "github.com/docker/docker/container"
 	"github.com/docker/docker/libcontainerd"
 	"github.com/docker/docker/pkg/signal"
@@ -97,15 +97,11 @@ func (daemon *Daemon) killWithSignal(container *containerpkg.Container, sig int)
 	}
 
 	if err := daemon.kill(container, sig); err != nil {
-		err = errors.Wrapf(err, "Cannot kill container %s", container.ID)
-		// if container or process not exists, ignore the error
-		// TODO: we shouldn't have to parse error strings from containerd
-		if strings.Contains(err.Error(), "container not found") ||
-			strings.Contains(err.Error(), "no such process") {
-			logrus.Warnf("container kill failed because of 'container not found' or 'no such process': %s", err.Error())
+		if errdefs.IsNotFound(err) {
 			unpause = false
+			logrus.WithError(err).WithField("container", container.ID).WithField("action", "kill").Debug("container kill failed because of 'container not found' or 'no such process'")
 		} else {
-			return err
+			return errors.Wrapf(err, "Cannot kill container %s", container.ID)
 		}
 	}
 
@@ -171,7 +167,7 @@ func (daemon *Daemon) Kill(container *containerpkg.Container) error {
 // killPossibleDeadProcess is a wrapper around killSig() suppressing "no such process" error.
 func (daemon *Daemon) killPossiblyDeadProcess(container *containerpkg.Container, sig int) error {
 	err := daemon.killWithSignal(container, sig)
-	if err == syscall.ESRCH {
+	if errdefs.IsNotFound(err) {
 		e := errNoSuchProcess{container.GetPID(), sig}
 		logrus.Debug(e)
 		return e
diff --git a/libcontainerd/client_daemon.go b/libcontainerd/client_daemon.go
index 0a3502c347..78b1412068 100644
--- a/libcontainerd/client_daemon.go
+++ b/libcontainerd/client_daemon.go
@@ -27,6 +27,7 @@ import (
 	"github.com/containerd/containerd/archive"
 	"github.com/containerd/containerd/cio"
 	"github.com/containerd/containerd/content"
+	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/linux/runctypes"
 	"github.com/containerd/typeurl"
@@ -317,7 +318,7 @@ func (c *client) SignalProcess(ctx context.Context, containerID, processID strin
 	if err != nil {
 		return err
 	}
-	return p.Kill(ctx, syscall.Signal(signal))
+	return wrapError(p.Kill(ctx, syscall.Signal(signal)))
 }
 
 func (c *client) ResizeTerminal(ctx context.Context, containerID, processID string, width, height int) error {
@@ -816,12 +817,19 @@ func (c *client) writeContent(ctx context.Context, mediaType, ref string, r io.R
 }
 
 func wrapError(err error) error {
-	if err != nil {
-		msg := err.Error()
-		for _, s := range []string{"container does not exist", "not found", "no such container"} {
-			if strings.Contains(msg, s) {
-				return wrapNotFoundError(err)
-			}
+	if err == nil {
+		return nil
+	}
+
+	switch {
+	case errdefs.IsNotFound(err):
+		return wrapNotFoundError(err)
+	}
+
+	msg := err.Error()
+	for _, s := range []string{"container does not exist", "not found", "no such container"} {
+		if strings.Contains(msg, s) {
+			return wrapNotFoundError(err)
 		}
 	}
 	return err