From 163a08177627818bcee631d4a37851cc74468f47 Mon Sep 17 00:00:00 2001
From: Sebastien Boeuf <sebastien.boeuf@intel.com>
Date: Wed, 25 Apr 2018 09:01:53 -0700
Subject: [PATCH] cli: Check sandbox state before to issue a StopSandbox

The way a delete works, it was always trying to stop the sandbox, even
when the force flag was not enabled. Because we want to be able to stop
the sandbox from a kill command, this means a sandbox stop might be
called twice, and we don't want the second stop to fail, leading to the
failure of the delete command.

That's why this commit checks for the sandbox status before to try
stopping the sandbox.

Fixes #246

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---
 cli/delete.go      |  9 ++++++++-
 cli/delete_test.go | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/cli/delete.go b/cli/delete.go
index b775b6a11a..11db6fc5b0 100644
--- a/cli/delete.go
+++ b/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
 	}
diff --git a/cli/delete_test.go b/cli/delete_test.go
index 89c5715466..66debda43e 100644
--- a/cli/delete_test.go
+++ b/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
 	}