daemon: cleanupContainer: don't fail if container is already stopped

Saw this failure in a flaky test, and I wondered why we consider this an
error condition;

    === RUN   TestKillWithStopSignalAndRestartPolicies
        main_test.go:32: assertion failed: error is not nil: Error response from daemon: Could not kill running container 668f62511f4aa62357269cd405cff1fbe295b7f6d5011e7cfed434e3072330b7, cannot remove - Container 668f62511f4aa62357269cd405cff1fbe295b7f6d5011e7cfed434e3072330b7 is not running: failed to remove 668f62511f4aa62357269cd405cff1fbe295b7f6d5011e7cfed434e3072330b7
    --- FAIL: TestKillWithStopSignalAndRestartPolicies (0.84s)
    === RUN   TestKillWithStopSignalAndRestartPolicies/same-signal-disables-restart-policy
        --- PASS: TestKillWithStopSignalAndRestartPolicies/same-signal-disables-restart-policy (0.42s)
    === RUN   TestKillWithStopSignalAndRestartPolicies/different-signal-keep-restart-policy
        --- PASS: TestKillWithStopSignalAndRestartPolicies/different-signal-keep-restart-policy (0.23s)

In the above;

1. `Error response from daemon: Could not kill running container 668f62511f4aa62357269cd405cff1fbe295b7f6d5011e7cfed434e3072330b7`
2. `cannot remove - Container 668f62511f4aa62357269cd405cff1fbe295b7f6d5011e7cfed434e3072330b7 is not running`
3. `failed to remove 668f62511f4aa62357269cd405cff1fbe295b7f6d5011e7cfed434e3072330b7`

So it looks like the removal fails because we couldn't kill the container
because it was already stopped, which may be a race condition where the first
check shows the container to be running (but may already be in process to be
removed or killed. In either case, we probably shouldn't fail the removal if
the container is already stopped.

This patch adds a `isNotRunning()` utility, so that we can ignore this case,
and proceed with the removal.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

daemon/delete.go

@@ -96,8 +96,8 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, config ty
 			err := fmt.Errorf("You cannot remove a %s container %s. %s", state, container.ID, procedure)
 			return errdefs.Conflict(err)
 		}
-		if err := daemon.Kill(container); err != nil {
-			return fmt.Errorf("Could not kill running container %s, cannot remove - %v", container.ID, err)
+		if err := daemon.Kill(container); err != nil && !isNotRunning(err) {
+			return fmt.Errorf("cannot remove container %q: could not kill: %w", container.Name, err)
 		}
 	}
 

daemon/errors.go

@@ -10,10 +10,21 @@ import (
 	"google.golang.org/grpc/status"
 )
 
+func isNotRunning(err error) bool {
+	var nre *containerNotRunningError
+	return errors.As(err, &nre)
+}
+
 func errNotRunning(id string) error {
-	return errdefs.Conflict(errors.Errorf("Container %s is not running", id))
+	return &containerNotRunningError{errors.Errorf("container %s is not running", id)}
 }
 
+type containerNotRunningError struct {
+	error
+}
+
+func (e containerNotRunningError) Conflict() {}
+
 func containerNotFound(id string) error {
 	return objNotFoundError{"container", id}
 }

daemon/errors_test.go

@@ -0,0 +1,12 @@
+package daemon
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func TestContainerNotRunningError(t *testing.T) {
+	err := errNotRunning("12345")
+	assert.Check(t, isNotRunning(err))
+}