Merge pull request #232 from sboeuf/fix_openshift_k8s

cli: Don't wait for OCI delete to stop the sandbox

cli/delete.go

@@ -105,10 +105,17 @@ func delete(containerID string, force bool) error {
 }
 
 func deleteSandbox(sandboxID string) error {
-	if _, err := vci.StopSandbox(sandboxID); err != nil {
+	status, err := vci.StatusSandbox(sandboxID)
+	if err != nil {
 		return err
 	}
 
+	if oci.StateToOCIState(status.State) != oci.StateStopped {
+		if _, err := vci.StopSandbox(sandboxID); err != nil {
+			return err
+		}
+	}
+
 	if _, err := vci.DeleteSandbox(sandboxID); err != nil {
 		return err
 	}

cli/delete_test.go

@@ -192,6 +192,23 @@ func TestDeleteSandbox(t *testing.T) {
 	assert.Error(err)
 	assert.True(vcmock.IsMockError(err))
 
+	testingImpl.StatusSandboxFunc = func(sandboxID string) (vc.SandboxStatus, error) {
+		return vc.SandboxStatus{
+			ID: sandbox.ID(),
+			State: vc.State{
+				State: vc.StateReady,
+			},
+		}, nil
+	}
+
+	defer func() {
+		testingImpl.StatusSandboxFunc = nil
+	}()
+
+	err = delete(sandbox.ID(), false)
+	assert.Error(err)
+	assert.True(vcmock.IsMockError(err))
+
 	testingImpl.StopSandboxFunc = func(sandboxID string) (vc.VCSandbox, error) {
 		return sandbox, nil
 	}
@@ -297,11 +314,21 @@ func TestDeleteSandboxRunning(t *testing.T) {
 	assert.Error(err)
 	assert.False(vcmock.IsMockError(err))
 
+	testingImpl.StatusSandboxFunc = func(sandboxID string) (vc.SandboxStatus, error) {
+		return vc.SandboxStatus{
+			ID: sandbox.ID(),
+			State: vc.State{
+				State: vc.StateRunning,
+			},
+		}, nil
+	}
+
 	testingImpl.StopSandboxFunc = func(sandboxID string) (vc.VCSandbox, error) {
 		return sandbox, nil
 	}
 
 	defer func() {
+		testingImpl.StatusSandboxFunc = nil
 		testingImpl.StopSandboxFunc = nil
 	}()
 
@@ -525,6 +552,15 @@ func TestDeleteCLIFunctionSuccess(t *testing.T) {
 		}, nil
 	}
 
+	testingImpl.StatusSandboxFunc = func(sandboxID string) (vc.SandboxStatus, error) {
+		return vc.SandboxStatus{
+			ID: sandbox.ID(),
+			State: vc.State{
+				State: vc.StateReady,
+			},
+		}, nil
+	}
+
 	testingImpl.StopSandboxFunc = func(sandboxID string) (vc.VCSandbox, error) {
 		return sandbox, nil
 	}

cli/kill.go

@@ -12,6 +12,7 @@ import (
 	"syscall"
 
 	vc "github.com/kata-containers/runtime/virtcontainers"
+	"github.com/kata-containers/runtime/virtcontainers/pkg/oci"
 	"github.com/urfave/cli"
 )
 
@@ -115,7 +116,20 @@ func kill(containerID, signal string, all bool) error {
 		return nil
 	}
 
-	_, err = vci.StopContainer(sandboxID, containerID)
+	containerType, err := oci.GetContainerType(status.Annotations)
+	if err != nil {
+		return err
+	}
+
+	switch containerType {
+	case vc.PodSandbox:
+		_, err = vci.StopSandbox(sandboxID)
+	case vc.PodContainer:
+		_, err = vci.StopContainer(sandboxID, containerID)
+	default:
+		return fmt.Errorf("Invalid container type found")
+	}
+
 	return err
 }
 

cli/kill_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 
 	vc "github.com/kata-containers/runtime/virtcontainers"
+	vcAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations"
 	"github.com/kata-containers/runtime/virtcontainers/pkg/vcmock"
 	"github.com/stretchr/testify/assert"
 )
@@ -24,6 +25,10 @@ var (
 	testStopContainerFuncReturnNil = func(sandboxID, containerID string) (vc.VCContainer, error) {
 		return &vcmock.Container{}, nil
 	}
+
+	testStopSandboxFuncReturnNil = func(sandboxID string) (vc.VCSandbox, error) {
+		return &vcmock.Sandbox{}, nil
+	}
 )
 
 func TestProcessSignal(t *testing.T) {
@@ -61,13 +66,18 @@ func testKillCLIFunctionTerminationSignalSuccessful(t *testing.T, sig string) {
 		State: vc.StateRunning,
 	}
 
+	annotations := map[string]string{
+		vcAnnotations.ContainerTypeKey: string(vc.PodContainer),
+	}
+
 	testingImpl.KillContainerFunc = testKillContainerFuncReturnNil
 	testingImpl.StopContainerFunc = testStopContainerFuncReturnNil
 	testingImpl.ListSandboxFunc = func() ([]vc.SandboxStatus, error) {
-		return newSingleContainerSandboxStatusList(testSandboxID, testContainerID, state, state, map[string]string{}), nil
+		return newSingleContainerSandboxStatusList(testSandboxID, testContainerID, state, state, annotations), nil
 	}
 	defer func() {
 		testingImpl.KillContainerFunc = nil
+		testingImpl.StopContainerFunc = nil
 		testingImpl.ListSandboxFunc = nil
 	}()
 
@@ -75,6 +85,21 @@ func testKillCLIFunctionTerminationSignalSuccessful(t *testing.T, sig string) {
 	set.Parse([]string{testContainerID, sig})
 
 	execCLICommandFunc(assert, killCLICommand, set, false)
+
+	annotations = map[string]string{
+		vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
+	}
+
+	testingImpl.StopContainerFunc = nil
+	testingImpl.StopSandboxFunc = testStopSandboxFuncReturnNil
+	testingImpl.ListSandboxFunc = func() ([]vc.SandboxStatus, error) {
+		return newSingleContainerSandboxStatusList(testSandboxID, testContainerID, state, state, annotations), nil
+	}
+	defer func() {
+		testingImpl.StopSandboxFunc = nil
+	}()
+
+	execCLICommandFunc(assert, killCLICommand, set, false)
 }
 
 func TestKillCLIFunctionSigkillSuccessful(t *testing.T) {
@@ -114,12 +139,18 @@ func TestKillCLIFunctionNoSignalSuccessful(t *testing.T) {
 		State: vc.StateRunning,
 	}
 
+	annotations := map[string]string{
+		vcAnnotations.ContainerTypeKey: string(vc.PodContainer),
+	}
+
 	testingImpl.KillContainerFunc = testKillContainerFuncReturnNil
+	testingImpl.StopContainerFunc = testStopContainerFuncReturnNil
 	testingImpl.ListSandboxFunc = func() ([]vc.SandboxStatus, error) {
-		return newSingleContainerSandboxStatusList(testSandboxID, testContainerID, state, state, map[string]string{}), nil
+		return newSingleContainerSandboxStatusList(testSandboxID, testContainerID, state, state, annotations), nil
 	}
 	defer func() {
 		testingImpl.KillContainerFunc = nil
+		testingImpl.StopContainerFunc = nil
 		testingImpl.ListSandboxFunc = nil
 	}()
 
@@ -127,6 +158,21 @@ func TestKillCLIFunctionNoSignalSuccessful(t *testing.T) {
 	set.Parse([]string{testContainerID})
 
 	execCLICommandFunc(assert, killCLICommand, set, false)
+
+	annotations = map[string]string{
+		vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
+	}
+
+	testingImpl.StopContainerFunc = nil
+	testingImpl.StopSandboxFunc = testStopSandboxFuncReturnNil
+	testingImpl.ListSandboxFunc = func() ([]vc.SandboxStatus, error) {
+		return newSingleContainerSandboxStatusList(testSandboxID, testContainerID, state, state, annotations), nil
+	}
+	defer func() {
+		testingImpl.StopSandboxFunc = nil
+	}()
+
+	execCLICommandFunc(assert, killCLICommand, set, false)
 }
 
 func TestKillCLIFunctionEnableAllSuccessful(t *testing.T) {
@@ -136,6 +182,10 @@ func TestKillCLIFunctionEnableAllSuccessful(t *testing.T) {
 		State: vc.StateRunning,
 	}
 
+	annotations := map[string]string{
+		vcAnnotations.ContainerTypeKey: string(vc.PodContainer),
+	}
+
 	testingImpl.KillContainerFunc = func(sandboxID, containerID string, signal syscall.Signal, all bool) error {
 		if !all {
 			return fmt.Errorf("Expecting -all flag = true, Got false")
@@ -143,11 +193,13 @@ func TestKillCLIFunctionEnableAllSuccessful(t *testing.T) {
 
 		return nil
 	}
+	testingImpl.StopContainerFunc = testStopContainerFuncReturnNil
 	testingImpl.ListSandboxFunc = func() ([]vc.SandboxStatus, error) {
-		return newSingleContainerSandboxStatusList(testSandboxID, testContainerID, state, state, map[string]string{}), nil
+		return newSingleContainerSandboxStatusList(testSandboxID, testContainerID, state, state, annotations), nil
 	}
 	defer func() {
 		testingImpl.KillContainerFunc = nil
+		testingImpl.StopContainerFunc = nil
 		testingImpl.ListSandboxFunc = nil
 	}()
 
@@ -156,6 +208,21 @@ func TestKillCLIFunctionEnableAllSuccessful(t *testing.T) {
 	set.Parse([]string{testContainerID})
 
 	execCLICommandFunc(assert, killCLICommand, set, false)
+
+	annotations = map[string]string{
+		vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
+	}
+
+	testingImpl.StopContainerFunc = nil
+	testingImpl.StopSandboxFunc = testStopSandboxFuncReturnNil
+	testingImpl.ListSandboxFunc = func() ([]vc.SandboxStatus, error) {
+		return newSingleContainerSandboxStatusList(testSandboxID, testContainerID, state, state, annotations), nil
+	}
+	defer func() {
+		testingImpl.StopSandboxFunc = nil
+	}()
+
+	execCLICommandFunc(assert, killCLICommand, set, false)
 }
 
 func TestKillCLIFunctionNoContainerIDFailure(t *testing.T) {