Merge pull request #1528 from bergwolf/grpc

shimv2 should return grpc error codes

containerd-shim-v2/errors.go

@@ -0,0 +1,62 @@
+// Copyright (c) 2019 hyper.sh
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package containerdshim
+
+import (
+	"strings"
+	"syscall"
+
+	"github.com/pkg/errors"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+
+	vc "github.com/kata-containers/runtime/virtcontainers/pkg/types"
+)
+
+// toGRPC maps the virtcontainers error into a grpc error,
+// using the original error message as a description.
+func toGRPC(err error) error {
+	if err == nil {
+		return nil
+	}
+
+	if isGRPCError(err) {
+		// error has already been mapped to grpc
+		return err
+	}
+
+	err = errors.Cause(err)
+	switch {
+	case isInvalidArgument(err):
+		return status.Errorf(codes.InvalidArgument, err.Error())
+	case isNotFound(err):
+		return status.Errorf(codes.NotFound, err.Error())
+	}
+
+	return err
+}
+
+// toGRPCf maps the error to grpc error codes, assembling the formatting string
+// and combining it with the target error string.
+func toGRPCf(err error, format string, args ...interface{}) error {
+	return toGRPC(errors.Wrapf(err, format, args...))
+}
+
+func isGRPCError(err error) bool {
+	_, ok := status.FromError(err)
+	return ok
+}
+
+func isInvalidArgument(err error) bool {
+	return err == vc.ErrNeedSandbox || err == vc.ErrNeedSandboxID ||
+		err == vc.ErrNeedContainerID || err == vc.ErrNeedState ||
+		err == syscall.EINVAL
+}
+
+func isNotFound(err error) bool {
+	return err == vc.ErrNoSuchContainer || err == syscall.ENOENT ||
+		strings.Contains(err.Error(), "not found") || strings.Contains(err.Error(), "not exist")
+}

containerd-shim-v2/errors_test.go

@@ -0,0 +1,29 @@
+// Copyright (c) 2019 hyper.sh
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package containerdshim
+
+import (
+	"syscall"
+	"testing"
+
+	vc "github.com/kata-containers/runtime/virtcontainers/pkg/types"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestToGRPC(t *testing.T) {
+	assert := assert.New(t)
+
+	for _, err := range []error{vc.ErrNeedSandbox, vc.ErrNeedSandboxID,
+		vc.ErrNeedContainerID, vc.ErrNeedState, syscall.EINVAL, vc.ErrNoSuchContainer, syscall.ENOENT} {
+		assert.False(isGRPCError(err))
+		err = toGRPC(err)
+		assert.True(isGRPCError(err))
+		err = toGRPC(err)
+		assert.True(isGRPCError(err))
+		err = toGRPCf(err, "appending")
+		assert.True(isGRPCError(err))
+	}
+}

containerd-shim-v2/service.go

@@ -258,13 +258,17 @@ func getTopic(ctx context.Context, e interface{}) string {
 	return cdruntime.TaskUnknownTopic
 }
 
-func (s *service) Cleanup(ctx context.Context) (*taskAPI.DeleteResponse, error) {
+func (s *service) Cleanup(ctx context.Context) (_ *taskAPI.DeleteResponse, err error) {
 	//Since the binary cleanup will return the DeleteResponse from stdout to
 	//containerd, thus we must make sure there is no any outputs in stdout except
 	//the returned response, thus here redirect the log to stderr in case there's
 	//any log output to stdout.
 	logrus.SetOutput(os.Stderr)
 
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	if s.id == "" {
 		return nil, errdefs.ToGRPCf(errdefs.ErrInvalidArgument, "the container id is empty, please specify the container id")
 	}
@@ -310,6 +314,10 @@ func (s *service) Cleanup(ctx context.Context) (*taskAPI.DeleteResponse, error)
 
 // Create a new sandbox or container with the underlying OCI runtime
 func (s *service) Create(ctx context.Context, r *taskAPI.CreateTaskRequest) (_ *taskAPI.CreateTaskResponse, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -351,7 +359,11 @@ func (s *service) Create(ctx context.Context, r *taskAPI.CreateTaskRequest) (_ *
 }
 
 // Start a process
-func (s *service) Start(ctx context.Context, r *taskAPI.StartRequest) (*taskAPI.StartResponse, error) {
+func (s *service) Start(ctx context.Context, r *taskAPI.StartRequest) (_ *taskAPI.StartResponse, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -393,7 +405,11 @@ func (s *service) Start(ctx context.Context, r *taskAPI.StartRequest) (*taskAPI.
 }
 
 // Delete the initial process and container
-func (s *service) Delete(ctx context.Context, r *taskAPI.DeleteRequest) (*taskAPI.DeleteResponse, error) {
+func (s *service) Delete(ctx context.Context, r *taskAPI.DeleteRequest) (_ *taskAPI.DeleteResponse, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -453,7 +469,11 @@ func (s *service) Delete(ctx context.Context, r *taskAPI.DeleteRequest) (*taskAP
 }
 
 // Exec an additional process inside the container
-func (s *service) Exec(ctx context.Context, r *taskAPI.ExecProcessRequest) (*ptypes.Empty, error) {
+func (s *service) Exec(ctx context.Context, r *taskAPI.ExecProcessRequest) (_ *ptypes.Empty, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -482,7 +502,11 @@ func (s *service) Exec(ctx context.Context, r *taskAPI.ExecProcessRequest) (*pty
 }
 
 // ResizePty of a process
-func (s *service) ResizePty(ctx context.Context, r *taskAPI.ResizePtyRequest) (*ptypes.Empty, error) {
+func (s *service) ResizePty(ctx context.Context, r *taskAPI.ResizePtyRequest) (_ *ptypes.Empty, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -512,7 +536,11 @@ func (s *service) ResizePty(ctx context.Context, r *taskAPI.ResizePtyRequest) (*
 }
 
 // State returns runtime state information for a process
-func (s *service) State(ctx context.Context, r *taskAPI.StateRequest) (*taskAPI.StateResponse, error) {
+func (s *service) State(ctx context.Context, r *taskAPI.StateRequest) (_ *taskAPI.StateResponse, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -556,7 +584,11 @@ func (s *service) State(ctx context.Context, r *taskAPI.StateRequest) (*taskAPI.
 }
 
 // Pause the container
-func (s *service) Pause(ctx context.Context, r *taskAPI.PauseRequest) (*ptypes.Empty, error) {
+func (s *service) Pause(ctx context.Context, r *taskAPI.PauseRequest) (_ *ptypes.Empty, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -586,7 +618,11 @@ func (s *service) Pause(ctx context.Context, r *taskAPI.PauseRequest) (*ptypes.E
 }
 
 // Resume the container
-func (s *service) Resume(ctx context.Context, r *taskAPI.ResumeRequest) (*ptypes.Empty, error) {
+func (s *service) Resume(ctx context.Context, r *taskAPI.ResumeRequest) (_ *ptypes.Empty, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -614,7 +650,11 @@ func (s *service) Resume(ctx context.Context, r *taskAPI.ResumeRequest) (*ptypes
 }
 
 // Kill a process with the provided signal
-func (s *service) Kill(ctx context.Context, r *taskAPI.KillRequest) (*ptypes.Empty, error) {
+func (s *service) Kill(ctx context.Context, r *taskAPI.KillRequest) (_ *ptypes.Empty, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -640,9 +680,13 @@ func (s *service) Kill(ctx context.Context, r *taskAPI.KillRequest) (*ptypes.Emp
 // Pids returns all pids inside the container
 // Since for kata, it cannot get the process's pid from VM,
 // thus only return the Shim's pid directly.
-func (s *service) Pids(ctx context.Context, r *taskAPI.PidsRequest) (*taskAPI.PidsResponse, error) {
+func (s *service) Pids(ctx context.Context, r *taskAPI.PidsRequest) (_ *taskAPI.PidsResponse, err error) {
 	var processes []*task.ProcessInfo
 
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	pInfo := task.ProcessInfo{
 		Pid: s.pid,
 	}
@@ -654,7 +698,11 @@ func (s *service) Pids(ctx context.Context, r *taskAPI.PidsRequest) (*taskAPI.Pi
 }
 
 // CloseIO of a process
-func (s *service) CloseIO(ctx context.Context, r *taskAPI.CloseIORequest) (*ptypes.Empty, error) {
+func (s *service) CloseIO(ctx context.Context, r *taskAPI.CloseIORequest) (_ *ptypes.Empty, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -682,12 +730,20 @@ func (s *service) CloseIO(ctx context.Context, r *taskAPI.CloseIORequest) (*ptyp
 }
 
 // Checkpoint the container
-func (s *service) Checkpoint(ctx context.Context, r *taskAPI.CheckpointTaskRequest) (*ptypes.Empty, error) {
+func (s *service) Checkpoint(ctx context.Context, r *taskAPI.CheckpointTaskRequest) (_ *ptypes.Empty, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	return nil, errdefs.ToGRPCf(errdefs.ErrNotImplemented, "service Checkpoint")
 }
 
 // Connect returns shim information such as the shim's pid
-func (s *service) Connect(ctx context.Context, r *taskAPI.ConnectRequest) (*taskAPI.ConnectResponse, error) {
+func (s *service) Connect(ctx context.Context, r *taskAPI.ConnectRequest) (_ *taskAPI.ConnectResponse, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -698,7 +754,11 @@ func (s *service) Connect(ctx context.Context, r *taskAPI.ConnectRequest) (*task
 	}, nil
 }
 
-func (s *service) Shutdown(ctx context.Context, r *taskAPI.ShutdownRequest) (*ptypes.Empty, error) {
+func (s *service) Shutdown(ctx context.Context, r *taskAPI.ShutdownRequest) (_ *ptypes.Empty, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	if len(s.containers) != 0 {
 		s.mu.Unlock()
@@ -713,7 +773,11 @@ func (s *service) Shutdown(ctx context.Context, r *taskAPI.ShutdownRequest) (*pt
 	return empty, nil
 }
 
-func (s *service) Stats(ctx context.Context, r *taskAPI.StatsRequest) (*taskAPI.StatsResponse, error) {
+func (s *service) Stats(ctx context.Context, r *taskAPI.StatsRequest) (_ *taskAPI.StatsResponse, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -733,7 +797,11 @@ func (s *service) Stats(ctx context.Context, r *taskAPI.StatsRequest) (*taskAPI.
 }
 
 // Update a running container
-func (s *service) Update(ctx context.Context, r *taskAPI.UpdateTaskRequest) (*ptypes.Empty, error) {
+func (s *service) Update(ctx context.Context, r *taskAPI.UpdateTaskRequest) (_ *ptypes.Empty, err error) {
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
@@ -756,9 +824,13 @@ func (s *service) Update(ctx context.Context, r *taskAPI.UpdateTaskRequest) (*pt
 }
 
 // Wait for a process to exit
-func (s *service) Wait(ctx context.Context, r *taskAPI.WaitRequest) (*taskAPI.WaitResponse, error) {
+func (s *service) Wait(ctx context.Context, r *taskAPI.WaitRequest) (_ *taskAPI.WaitResponse, err error) {
 	var ret uint32
 
+	defer func() {
+		err = toGRPC(err)
+	}()
+
 	s.mu.Lock()
 	c, err := s.getContainer(r.ID)
 	s.mu.Unlock()

virtcontainers/api.go

@@ -132,7 +132,7 @@ func DeleteSandbox(ctx context.Context, sandboxID string) (VCSandbox, error) {
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -165,7 +165,7 @@ func FetchSandbox(ctx context.Context, sandboxID string) (VCSandbox, error) {
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -202,7 +202,7 @@ func StartSandbox(ctx context.Context, sandboxID string) (VCSandbox, error) {
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -234,7 +234,7 @@ func StopSandbox(ctx context.Context, sandboxID string) (VCSandbox, error) {
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandbox
+		return nil, vcTypes.ErrNeedSandbox
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -328,7 +328,7 @@ func StatusSandbox(ctx context.Context, sandboxID string) (SandboxStatus, error)
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return SandboxStatus{}, errNeedSandboxID
+		return SandboxStatus{}, vcTypes.ErrNeedSandboxID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -374,7 +374,7 @@ func CreateContainer(ctx context.Context, sandboxID string, containerConfig Cont
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, nil, errNeedSandboxID
+		return nil, nil, vcTypes.ErrNeedSandboxID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -405,11 +405,11 @@ func DeleteContainer(ctx context.Context, sandboxID, containerID string) (VCCont
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	if containerID == "" {
-		return nil, errNeedContainerID
+		return nil, vcTypes.ErrNeedContainerID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -434,11 +434,11 @@ func StartContainer(ctx context.Context, sandboxID, containerID string) (VCConta
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	if containerID == "" {
-		return nil, errNeedContainerID
+		return nil, vcTypes.ErrNeedContainerID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -463,11 +463,11 @@ func StopContainer(ctx context.Context, sandboxID, containerID string) (VCContai
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	if containerID == "" {
-		return nil, errNeedContainerID
+		return nil, vcTypes.ErrNeedContainerID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -492,11 +492,11 @@ func EnterContainer(ctx context.Context, sandboxID, containerID string, cmd type
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, nil, nil, errNeedSandboxID
+		return nil, nil, nil, vcTypes.ErrNeedSandboxID
 	}
 
 	if containerID == "" {
-		return nil, nil, nil, errNeedContainerID
+		return nil, nil, nil, vcTypes.ErrNeedContainerID
 	}
 
 	lockFile, err := rLockSandbox(ctx, sandboxID)
@@ -526,11 +526,11 @@ func StatusContainer(ctx context.Context, sandboxID, containerID string) (Contai
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return ContainerStatus{}, errNeedSandboxID
+		return ContainerStatus{}, vcTypes.ErrNeedSandboxID
 	}
 
 	if containerID == "" {
-		return ContainerStatus{}, errNeedContainerID
+		return ContainerStatus{}, vcTypes.ErrNeedContainerID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -607,11 +607,11 @@ func KillContainer(ctx context.Context, sandboxID, containerID string, signal sy
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return errNeedSandboxID
+		return vcTypes.ErrNeedSandboxID
 	}
 
 	if containerID == "" {
-		return errNeedContainerID
+		return vcTypes.ErrNeedContainerID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -654,11 +654,11 @@ func ProcessListContainer(ctx context.Context, sandboxID, containerID string, op
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	if containerID == "" {
-		return nil, errNeedContainerID
+		return nil, vcTypes.ErrNeedContainerID
 	}
 
 	lockFile, err := rLockSandbox(ctx, sandboxID)
@@ -683,11 +683,11 @@ func UpdateContainer(ctx context.Context, sandboxID, containerID string, resourc
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return errNeedSandboxID
+		return vcTypes.ErrNeedSandboxID
 	}
 
 	if containerID == "" {
-		return errNeedContainerID
+		return vcTypes.ErrNeedContainerID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -712,11 +712,11 @@ func StatsContainer(ctx context.Context, sandboxID, containerID string) (Contain
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return ContainerStats{}, errNeedSandboxID
+		return ContainerStats{}, vcTypes.ErrNeedSandboxID
 	}
 
 	if containerID == "" {
-		return ContainerStats{}, errNeedContainerID
+		return ContainerStats{}, vcTypes.ErrNeedContainerID
 	}
 	lockFile, err := rLockSandbox(ctx, sandboxID)
 	if err != nil {
@@ -736,11 +736,11 @@ func StatsContainer(ctx context.Context, sandboxID, containerID string) (Contain
 
 func togglePauseContainer(ctx context.Context, sandboxID, containerID string, pause bool) error {
 	if sandboxID == "" {
-		return errNeedSandboxID
+		return vcTypes.ErrNeedSandboxID
 	}
 
 	if containerID == "" {
-		return errNeedContainerID
+		return vcTypes.ErrNeedContainerID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -784,7 +784,7 @@ func AddDevice(ctx context.Context, sandboxID string, info deviceConfig.DeviceIn
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -804,7 +804,7 @@ func AddDevice(ctx context.Context, sandboxID string, info deviceConfig.DeviceIn
 
 func toggleInterface(ctx context.Context, sandboxID string, inf *vcTypes.Interface, add bool) (*vcTypes.Interface, error) {
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -848,7 +848,7 @@ func ListInterfaces(ctx context.Context, sandboxID string) ([]*vcTypes.Interface
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	lockFile, err := rLockSandbox(ctx, sandboxID)
@@ -872,7 +872,7 @@ func UpdateRoutes(ctx context.Context, sandboxID string, routes []*vcTypes.Route
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)
@@ -896,7 +896,7 @@ func ListRoutes(ctx context.Context, sandboxID string) ([]*vcTypes.Route, error)
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	lockFile, err := rLockSandbox(ctx, sandboxID)

virtcontainers/container.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/containerd/cgroups"
 	"github.com/kata-containers/runtime/virtcontainers/pkg/annotations"
+	vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types"
 	"github.com/kata-containers/runtime/virtcontainers/types"
 	"github.com/kata-containers/runtime/virtcontainers/utils"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
@@ -421,7 +422,7 @@ func (c *Container) loadDevices() ([]ContainerDevice, error) {
 // container.
 func (c *Container) setContainerState(state types.StateString) error {
 	if state == "" {
-		return errNeedState
+		return vcTypes.ErrNeedState
 	}
 
 	c.Logger().Debugf("Setting container state from %v to %v", c.state.State, state)

virtcontainers/errors.go

@@ -1,19 +0,0 @@
-// Copyright (c) 2017 Intel Corporation
-//
-// SPDX-License-Identifier: Apache-2.0
-//
-
-package virtcontainers
-
-import (
-	"errors"
-)
-
-// common error objects used for argument checking
-var (
-	errNeedSandbox     = errors.New("Sandbox must be specified")
-	errNeedSandboxID   = errors.New("Sandbox ID cannot be empty")
-	errNeedContainerID = errors.New("Container ID cannot be empty")
-	errNeedState       = errors.New("State cannot be empty")
-	errNoSuchContainer = errors.New("Container does not exist")
-)

virtcontainers/pkg/types/errors.go

@@ -0,0 +1,19 @@
+// Copyright (c) 2017 Intel Corporation
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package types
+
+import (
+	"errors"
+)
+
+// common error objects used for argument checking
+var (
+	ErrNeedSandbox     = errors.New("Sandbox must be specified")
+	ErrNeedSandboxID   = errors.New("Sandbox ID cannot be empty")
+	ErrNeedContainerID = errors.New("Container ID cannot be empty")
+	ErrNeedState       = errors.New("State cannot be empty")
+	ErrNoSuchContainer = errors.New("Container does not exist")
+)

virtcontainers/sandbox.go

@@ -7,7 +7,6 @@ package virtcontainers
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -18,7 +17,9 @@ import (
 	"github.com/containernetworking/plugins/pkg/ns"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	opentracing "github.com/opentracing/opentracing-go"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+	"github.com/vishvananda/netlink"
 
 	"github.com/kata-containers/agent/protocols/grpc"
 	"github.com/kata-containers/runtime/virtcontainers/device/api"
@@ -31,7 +32,6 @@ import (
 	"github.com/kata-containers/runtime/virtcontainers/store"
 	"github.com/kata-containers/runtime/virtcontainers/types"
 	"github.com/kata-containers/runtime/virtcontainers/utils"
-	"github.com/vishvananda/netlink"
 )
 
 const (
@@ -627,7 +627,7 @@ func unlockSandbox(ctx context.Context, sandboxID, token string) error {
 func fetchSandbox(ctx context.Context, sandboxID string) (sandbox *Sandbox, err error) {
 	virtLog.Info("fetch sandbox")
 	if sandboxID == "" {
-		return nil, errNeedSandboxID
+		return nil, vcTypes.ErrNeedSandboxID
 	}
 
 	sandbox, err = globalSandboxList.lookupSandbox(sandboxID)
@@ -665,11 +665,11 @@ func fetchSandbox(ctx context.Context, sandboxID string) (sandbox *Sandbox, err
 // sandbox structure, based on a container ID.
 func (s *Sandbox) findContainer(containerID string) (*Container, error) {
 	if s == nil {
-		return nil, errNeedSandbox
+		return nil, vcTypes.ErrNeedSandbox
 	}
 
 	if containerID == "" {
-		return nil, errNeedContainerID
+		return nil, vcTypes.ErrNeedContainerID
 	}
 
 	for id, c := range s.containers {
@@ -678,7 +678,7 @@ func (s *Sandbox) findContainer(containerID string) (*Container, error) {
 		}
 	}
 
-	return nil, fmt.Errorf("Could not find the container %q from the sandbox %q containers list",
+	return nil, errors.Wrapf(vcTypes.ErrNoSuchContainer, "Could not find the container %q from the sandbox %q containers list",
 		containerID, s.id)
 }
 
@@ -686,15 +686,15 @@ func (s *Sandbox) findContainer(containerID string) (*Container, error) {
 // sandbox structure, based on a container ID.
 func (s *Sandbox) removeContainer(containerID string) error {
 	if s == nil {
-		return errNeedSandbox
+		return vcTypes.ErrNeedSandbox
 	}
 
 	if containerID == "" {
-		return errNeedContainerID
+		return vcTypes.ErrNeedContainerID
 	}
 
 	if _, ok := s.containers[containerID]; !ok {
-		return fmt.Errorf("Could not remove the container %q from the sandbox %q containers list",
+		return errors.Wrapf(vcTypes.ErrNoSuchContainer, "Could not remove the container %q from the sandbox %q containers list",
 			containerID, s.id)
 	}
 
@@ -1130,7 +1130,7 @@ func (s *Sandbox) KillContainer(containerID string, signal syscall.Signal, all b
 // DeleteContainer deletes a container from the sandbox
 func (s *Sandbox) DeleteContainer(containerID string) (VCContainer, error) {
 	if containerID == "" {
-		return nil, errNeedContainerID
+		return nil, vcTypes.ErrNeedContainerID
 	}
 
 	// Fetch the container.
@@ -1178,7 +1178,7 @@ func (s *Sandbox) ProcessListContainer(containerID string, options ProcessListOp
 // TODO: update container status properly, see kata-containers/runtime#253
 func (s *Sandbox) StatusContainer(containerID string) (ContainerStatus, error) {
 	if containerID == "" {
-		return ContainerStatus{}, errNeedContainerID
+		return ContainerStatus{}, vcTypes.ErrNeedContainerID
 	}
 
 	for id, c := range s.containers {
@@ -1198,7 +1198,7 @@ func (s *Sandbox) StatusContainer(containerID string) (ContainerStatus, error) {
 		}
 	}
 
-	return ContainerStatus{}, errNoSuchContainer
+	return ContainerStatus{}, vcTypes.ErrNoSuchContainer
 }
 
 // EnterContainer is the virtcontainers container command execution entry point.
@@ -1405,7 +1405,7 @@ func (s *Sandbox) enter(args []string) error {
 // sandbox.
 func (s *Sandbox) setSandboxState(state types.StateString) error {
 	if state == "" {
-		return errNeedState
+		return vcTypes.ErrNeedState
 	}
 
 	// update in-memory state
@@ -1467,7 +1467,7 @@ func (s *Sandbox) decrementSandboxBlockIndex() error {
 
 func (s *Sandbox) setContainersState(state types.StateString) error {
 	if state == "" {
-		return errNeedState
+		return vcTypes.ErrNeedState
 	}
 
 	for _, c := range s.containers {
@@ -1485,7 +1485,7 @@ func togglePauseSandbox(ctx context.Context, sandboxID string, pause bool) (*San
 	defer span.Finish()
 
 	if sandboxID == "" {
-		return nil, errNeedSandbox
+		return nil, vcTypes.ErrNeedSandbox
 	}
 
 	lockFile, err := rwLockSandbox(ctx, sandboxID)