From bf2813fee89f42c15eeb5526ae390383e95ced77 Mon Sep 17 00:00:00 2001
From: Peng Tao <bergwolf@gmail.com>
Date: Wed, 2 Jan 2019 16:52:34 +0800
Subject: [PATCH] cli: allow to kill a stopped container and sandbox

cri containerd calls kill on stopped sandbox and if we
fail the call, it can cause `cri stopp` command to fail
too.

Fixes: #1084

Signed-off-by: Peng Tao <bergwolf@gmail.com>
---
 cli/kill.go                    | 14 ++++++++------
 cli/kill_test.go               | 30 ++++++++++++++++++++++++++++++
 virtcontainers/container.go    |  1 +
 virtcontainers/sandbox.go      |  5 +++++
 virtcontainers/sandbox_test.go |  7 +++++++
 5 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/cli/kill.go b/cli/kill.go
index bb5e7a795b..645e231c60 100644
--- a/cli/kill.go
+++ b/cli/kill.go
@@ -129,13 +129,15 @@ func kill(ctx context.Context, containerID, signal string, all bool) error {
 		return err
 	}
 
-	// container MUST be created, running or paused
-	if status.State.State != vc.StateReady && status.State.State != vc.StateRunning && status.State.State != vc.StatePaused {
-		return fmt.Errorf("Container %s not ready, running or paused, cannot send a signal", containerID)
-	}
+	kataLog.WithField("signal", signal).WithField("container state", status.State.State).Info("kill")
 
-	if err := vci.KillContainer(ctx, sandboxID, containerID, signum, all); err != nil {
-		return err
+	// container MUST be created, running or paused
+	if status.State.State == vc.StateReady || status.State.State == vc.StateRunning || status.State.State == vc.StatePaused {
+		if err := vci.KillContainer(ctx, sandboxID, containerID, signum, all); err != nil {
+			return err
+		}
+	} else if !all {
+		return fmt.Errorf("container not running")
 	}
 
 	if signum != syscall.SIGKILL && signum != syscall.SIGTERM {
diff --git a/cli/kill_test.go b/cli/kill_test.go
index 470020fa12..41094fce3f 100644
--- a/cli/kill_test.go
+++ b/cli/kill_test.go
@@ -396,3 +396,33 @@ func TestKillCLIFunctionKillContainerFailure(t *testing.T) {
 
 	execCLICommandFunc(assert, killCLICommand, set, true)
 }
+
+func TestKillCLIFunctionInvalidStateStoppedAllSuccess(t *testing.T) {
+	assert := assert.New(t)
+
+	state := vc.State{
+		State: vc.StateStopped,
+	}
+
+	testingImpl.KillContainerFunc = testKillContainerFuncReturnNil
+
+	path, err := createTempContainerIDMapping(testContainerID, testSandboxID)
+	assert.NoError(err)
+	defer os.RemoveAll(path)
+
+	testingImpl.StatusContainerFunc = func(ctx context.Context, sandboxID, containerID string) (vc.ContainerStatus, error) {
+		return newSingleContainerStatus(testContainerID, state, map[string]string{}), nil
+	}
+
+	defer func() {
+		testingImpl.KillContainerFunc = nil
+		testingImpl.StatusContainerFunc = nil
+	}()
+
+	set := flag.NewFlagSet("", 0)
+	var all bool
+	set.BoolVar(&all, "all", false, "")
+	set.Parse([]string{"-all", testContainerID, "10"})
+
+	execCLICommandFunc(assert, killCLICommand, set, false)
+}
diff --git a/virtcontainers/container.go b/virtcontainers/container.go
index fa645169bb..de593134c0 100644
--- a/virtcontainers/container.go
+++ b/virtcontainers/container.go
@@ -404,6 +404,7 @@ func (c *Container) setContainerState(state stateString) error {
 		return errNeedState
 	}
 
+	c.Logger().Debugf("Setting container state from %v to %v", c.state.State, state)
 	// update in-memory state
 	c.state.State = state
 
diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go
index 589ef578e1..025ab8b5f4 100644
--- a/virtcontainers/sandbox.go
+++ b/virtcontainers/sandbox.go
@@ -1569,6 +1569,11 @@ func (s *Sandbox) Stop() error {
 	span, _ := s.trace("stop")
 	defer span.Finish()
 
+	if s.state.State == StateStopped {
+		s.Logger().Info("sandbox already stopped")
+		return nil
+	}
+
 	if err := s.state.validTransition(s.state.State, StateStopped); err != nil {
 		return err
 	}
diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go
index 08604efb79..8503a9a15b 100644
--- a/virtcontainers/sandbox_test.go
+++ b/virtcontainers/sandbox_test.go
@@ -1754,3 +1754,10 @@ func TestStartNetworkMonitor(t *testing.T) {
 	err = s.startNetworkMonitor()
 	assert.Nil(t, err)
 }
+
+func TestSandboxStopStopped(t *testing.T) {
+	s := &Sandbox{state: State{State: StateStopped}}
+	err := s.Stop()
+
+	assert.Nil(t, err)
+}