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>
diff --git a/daemon/kill.go b/daemon/kill.go
index 1292f86..5cde0d7 100644
--- a/daemon/kill.go
+++ b/daemon/kill.go
@@ -4,10 +4,10 @@
"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 @@
}
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 @@
// 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 0a3502c..78b1412 100644
--- a/libcontainerd/client_daemon.go
+++ b/libcontainerd/client_daemon.go
@@ -27,6 +27,7 @@
"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 @@
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 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
