engine VirtualizationRemove return ErrWorkloadNotExists

projecteru2 · Jan 27, 2022 · 43b0bb2 · 43b0bb2
1 parent fc584f7
commit 43b0bb2
Show file tree

Hide file tree

Showing 15 changed files with 34 additions and 39 deletions.
diff --git a/cluster/calcium/create.go b/cluster/calcium/create.go
@@ -56,8 +56,8 @@ func (c *Calcium) doCreateWorkloads(ctx context.Context, opts *types.DeployOptio
 			cctx, cancel := context.WithTimeout(utils.InheritTracingInfo(ctx, context.TODO()), c.config.GlobalTimeout)
 			for nodename := range deployMap {
 				processing := opts.GetProcessing(nodename)
-				if e := c.store.DeleteProcessing(cctx, processing); e != nil {
-					logger.Errorf(ctx, "[Calcium.doCreateWorkloads] delete processing failed for %s: %+v", nodename, e)
+				if err := c.store.DeleteProcessing(cctx, processing); err != nil {
+					logger.Errorf(ctx, "[Calcium.doCreateWorkloads] delete processing failed for %s: %+v", nodename, err)
 				}
 			}
 			close(ch)

diff --git a/cluster/calcium/create_test.go b/cluster/calcium/create_test.go
@@ -233,7 +233,7 @@ func TestCreateWorkloadTxn(t *testing.T) {
 	engine.On("ImageLocalDigests", mock.Anything, mock.Anything).Return([]string{""}, nil)
 	engine.On("ImageRemoteDigest", mock.Anything, mock.Anything).Return("", nil)
 	engine.On("VirtualizationCreate", mock.Anything, mock.Anything).Return(nil, errors.Wrap(context.DeadlineExceeded, "VirtualizationCreate")).Twice()
-	engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(1, nil)
+	engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
 	store.On("ListNodeWorkloads", mock.Anything, mock.Anything, mock.Anything).Return(nil, types.ErrNoETCD)
 	walCommitted = false
 	ch, err = c.CreateWorkload(ctx, opts)

diff --git a/cluster/calcium/lambda_test.go b/cluster/calcium/lambda_test.go
@@ -268,7 +268,7 @@ func newLambdaCluster(t *testing.T) (*Calcium, []*types.Node) {
 	// doCreateAndStartWorkload fails: AddWorkload
 	engine.On("VirtualizationCreate", mock.Anything, mock.Anything).Return(&enginetypes.VirtualizationCreated{ID: "workloadfortonictest"}, nil)
 	engine.On("VirtualizationStart", mock.Anything, mock.Anything).Return(nil)
-	engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(1, nil)
+	engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
 	store.On("ListNodeWorkloads", mock.Anything, mock.Anything, mock.Anything).Return(nil, types.ErrNoETCD)
 	engine.On("VirtualizationInspect", mock.Anything, mock.Anything).Return(&enginetypes.VirtualizationInfo{}, nil)
 	store.On("AddWorkload", mock.Anything, mock.Anything, mock.Anything).Return(nil)

diff --git a/cluster/calcium/remove_test.go b/cluster/calcium/remove_test.go
@@ -65,7 +65,7 @@ func TestRemoveWorkload(t *testing.T) {
 	// success
 	engine := &enginemocks.API{}
 	workload.Engine = engine
-	engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(1, nil)
+	engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
 	store.On("GetWorkloads", mock.Anything, mock.Anything).Return([]*types.Workload{workload}, nil)
 	store.On("RemoveWorkload", mock.Anything, mock.Anything).Return(nil)
 	store.On("UpdateNodeResource", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)

diff --git a/cluster/calcium/replace_test.go b/cluster/calcium/replace_test.go
@@ -168,9 +168,6 @@ func TestReplaceWorkload(t *testing.T) {
 	// failed by VirtualizationCreate
 	engine.On("VirtualizationCreate", mock.Anything, mock.Anything).Return(nil, types.ErrCannotGetEngine).Once()
 	engine.On("VirtualizationStart", mock.Anything, mock.Anything).Return(types.ErrCannotGetEngine).Once()
-	//store.On("UpdateNodeResource", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
-	//engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(1, nil).Once()
-	//store.On("RemoveWorkload", mock.Anything, mock.Anything).Return(nil).Once()
 	ch, err = c.ReplaceWorkload(ctx, opts)
 	assert.NoError(t, err)
 	for r := range ch {
@@ -200,7 +197,7 @@ func TestReplaceWorkload(t *testing.T) {
 	store.AssertExpectations(t)
 	engine.AssertExpectations(t)
 
-	engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(1, nil)
+	engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
 	store.On("RemoveWorkload", mock.Anything, mock.Anything).Return(nil)
 	store.On("ListNodeWorkloads", mock.Anything, mock.Anything, mock.Anything).Return(nil, types.ErrNoETCD)
 	// succ

diff --git a/cluster/calcium/wal.go b/cluster/calcium/wal.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"time"
 
+	"github.com/pkg/errors"
 	"github.com/projecteru2/core/log"
 	"github.com/projecteru2/core/types"
 	"github.com/projecteru2/core/utils"
@@ -123,7 +124,7 @@ func (h *CreateWorkloadHandler) Handle(ctx context.Context, raw interface{}) (er
 	if err != nil {
 		return logger.Err(ctx, err)
 	}
-	if _, err = node.Engine.VirtualizationRemove(ctx, wrk.ID, true, true); err != nil {
+	if err = node.Engine.VirtualizationRemove(ctx, wrk.ID, true, true); err != nil && !errors.Is(err, types.ErrWorkloadNotExists) {
 		return logger.Err(ctx, err)
 	}
 

diff --git a/cluster/calcium/wal_test.go b/cluster/calcium/wal_test.go
@@ -72,14 +72,14 @@ func TestHandleCreateWorkloadError(t *testing.T) {
 
 	store.On("GetWorkload", mock.Anything, mock.Anything).Return(wrk, err).Once()
 	store.On("GetNode", mock.Anything, wrk.Nodename).Return(node, nil).Once()
-	engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(0, err).Once()
+	engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(err).Once()
 	c.wal.Recover(context.TODO())
 	store.AssertExpectations(t)
 	engine.AssertExpectations(t)
 
 	store.On("GetWorkload", mock.Anything, wrkid).Return(wrk, fmt.Errorf("err")).Once()
 	store.On("GetNode", mock.Anything, mock.Anything).Return(node, nil).Once()
-	engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(1, nil).Once()
+	engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(types.ErrWorkloadNotExists).Once()
 	c.wal.Recover(context.TODO())
 	store.AssertExpectations(t)
 	engine.AssertExpectations(t)
@@ -117,7 +117,7 @@ func TestHandleCreateWorkloadHandled(t *testing.T) {
 	eng, ok := node.Engine.(*enginemocks.API)
 	require.True(t, ok)
 	eng.On("VirtualizationRemove", mock.Anything, wrk.ID, true, true).
-		Return(1, nil).
+		Return(nil).
 		Once()
 
 	c.wal.Recover(context.TODO())
@@ -163,7 +163,7 @@ func TestHandleCreateLambda(t *testing.T) {
 	eng := wrk.Engine.(*enginemocks.API)
 	eng.On("VirtualizationWait", mock.Anything, wrk.ID, "").Return(&enginetypes.VirtualizationWaitResult{Code: 0}, nil).Once()
 	eng.On("VirtualizationRemove", mock.Anything, wrk.ID, true, true).
-		Return(1, nil).
+		Return(nil).
 		Once()
 	eng.On("VirtualizationResourceRemap", mock.Anything, mock.Anything).Return(nil, nil).Once()
 	store.On("GetWorkloads", mock.Anything, []string{wrk.ID}).

diff --git a/engine/docker/container.go b/engine/docker/container.go
@@ -17,6 +17,7 @@ import (
 	corecluster "github.com/projecteru2/core/cluster"
 	enginetypes "github.com/projecteru2/core/engine/types"
 	"github.com/projecteru2/core/log"
+	"github.com/projecteru2/core/types"
 	coretypes "github.com/projecteru2/core/types"
 	"github.com/projecteru2/core/utils"
 
@@ -346,12 +347,12 @@ func (e *Engine) VirtualizationStop(ctx context.Context, ID string, gracefulTime
 }
 
 // VirtualizationRemove remove virtualization
-func (e *Engine) VirtualizationRemove(ctx context.Context, ID string, removeVolumes, force bool) (removed int, err error) {
+func (e *Engine) VirtualizationRemove(ctx context.Context, ID string, removeVolumes, force bool) (err error) {
 	if err = e.client.ContainerRemove(ctx, ID, dockertypes.ContainerRemoveOptions{RemoveVolumes: removeVolumes, Force: force}); err == nil {
-		return 1, nil
+		return nil
 	}
 	if strings.Contains(err.Error(), "No such container") {
-		return 0, nil
+		return types.ErrWorkloadNotExists
 	}
 	return
 }

diff --git a/engine/engine.go b/engine/engine.go
@@ -41,7 +41,7 @@ type API interface {
 	VirtualizationCopyTo(ctx context.Context, ID, target string, content []byte, uid, gid int, mode int64) error
 	VirtualizationStart(ctx context.Context, ID string) error
 	VirtualizationStop(ctx context.Context, ID string, gracefulTimeout time.Duration) error
-	VirtualizationRemove(ctx context.Context, ID string, volumes, force bool) (int, error)
+	VirtualizationRemove(ctx context.Context, ID string, volumes, force bool) error
 	VirtualizationInspect(ctx context.Context, ID string) (*enginetypes.VirtualizationInfo, error)
 	VirtualizationLogs(ctx context.Context, opts *enginetypes.VirtualizationLogStreamOptions) (stdout, stderr io.ReadCloser, err error)
 	VirtualizationAttach(ctx context.Context, ID string, stream, openStdin bool) (stdout, stderr io.ReadCloser, stdin io.WriteCloser, err error)

diff --git a/engine/fake/fake.go b/engine/fake/fake.go
@@ -139,8 +139,8 @@ func (f *Engine) VirtualizationStop(ctx context.Context, ID string, gracefulTime
 }
 
 // VirtualizationRemove .
-func (f *Engine) VirtualizationRemove(ctx context.Context, ID string, volumes, force bool) (int, error) {
-	return 0, types.ErrNilEngine
+func (f *Engine) VirtualizationRemove(ctx context.Context, ID string, volumes, force bool) error {
+	return types.ErrNilEngine
 }
 
 // VirtualizationInspect .

diff --git a/engine/mocks/API.go b/engine/mocks/API.go
diff --git a/engine/mocks/fakeengine/mock.go b/engine/mocks/fakeengine/mock.go
@@ -97,7 +97,7 @@ func MakeClient(ctx context.Context, config coretypes.Config, nodename, endpoint
 	e.On("VirtualizationCopyTo", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
 	e.On("VirtualizationStart", mock.Anything, mock.Anything).Return(nil)
 	e.On("VirtualizationStop", mock.Anything, mock.Anything, mock.Anything).Return(nil)
-	e.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(1, nil)
+	e.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
 	vcJSON := &enginetypes.VirtualizationInfo{ID: ID, Image: "mock-image", Running: true, Networks: map[string]string{"mock-network": "1.1.1.1"}}
 	e.On("VirtualizationInspect", mock.Anything, mock.Anything).Return(vcJSON, nil)
 	logs := ioutil.NopCloser(bytes.NewBufferString("logs1...\nlogs2...\n"))

diff --git a/engine/virt/virt.go b/engine/virt/virt.go
@@ -19,6 +19,7 @@ import (
 	enginetypes "github.com/projecteru2/core/engine/types"
 	"github.com/projecteru2/core/log"
 	coresource "github.com/projecteru2/core/source"
+	"github.com/projecteru2/core/types"
 	coretypes "github.com/projecteru2/core/types"
 )
 
@@ -228,12 +229,12 @@ func (v *Virt) VirtualizationStop(ctx context.Context, ID string, gracefulTimeou
 }
 
 // VirtualizationRemove removes a guest.
-func (v *Virt) VirtualizationRemove(ctx context.Context, ID string, volumes, force bool) (removed int, err error) {
+func (v *Virt) VirtualizationRemove(ctx context.Context, ID string, volumes, force bool) (err error) {
 	if _, err = v.client.DestroyGuest(ctx, ID, force); err == nil {
-		return 1, nil
+		return nil
 	}
 	if strings.Contains(err.Error(), "key not exists") {
-		return 0, nil
+		return types.ErrWorkloadNotExists
 	}
 	return
 }

diff --git a/types/workload.go b/types/workload.go
@@ -81,11 +81,13 @@ func (c *Workload) Stop(ctx context.Context, force bool) error {
 }
 
 // Remove a workload
-func (c *Workload) Remove(ctx context.Context, force bool) error {
+func (c *Workload) Remove(ctx context.Context, force bool) (err error) {
 	if c.Engine == nil {
 		return errors.WithStack(ErrNilEngine)
 	}
-	_, err := c.Engine.VirtualizationRemove(ctx, c.ID, true, force)
+	if err = c.Engine.VirtualizationRemove(ctx, c.ID, true, force); errors.Is(err, ErrWorkloadNotExists) {
+		err = nil
+	}
 	return errors.WithStack(err)
 }
 

diff --git a/types/workload_test.go b/types/workload_test.go
@@ -27,7 +27,7 @@ func TestWorkloadControl(t *testing.T) {
 	mockEngine := &mocks.API{}
 	mockEngine.On("VirtualizationStart", mock.Anything, mock.Anything).Return(nil)
 	mockEngine.On("VirtualizationStop", mock.Anything, mock.Anything, mock.Anything).Return(nil)
-	mockEngine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(1, nil)
+	mockEngine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
 
 	ctx := context.Background()
 	c := Workload{}