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

@@ -129,13 +129,15 @@ func kill(ctx context.Context, containerID, signal string, all bool) error {
 		return err
 	}
 
-	// container MUST be created, running or paused
+	kataLog.WithField("signal", signal).WithField("container state", status.State.State).Info("kill")
-	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)
-	}
 
-	if err := vci.KillContainer(ctx, sandboxID, containerID, signum, all); err != nil {
+	// container MUST be created, running or paused
-		return err
+	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 {

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)
+}

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
 

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
 	}

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)
+}