From cf9075163848e9a7c8ee4dbcf2f61743283671e5 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Fri, 12 Apr 2019 02:01:02 -0700 Subject: [PATCH 1/3] vc: export vc error types So that shimv2 can convert it into grpc errors. Signed-off-by: Peng Tao --- virtcontainers/api.go | 62 +++++++++++++++--------------- virtcontainers/container.go | 3 +- virtcontainers/errors.go | 19 --------- virtcontainers/pkg/types/errors.go | 19 +++++++++ virtcontainers/sandbox.go | 22 +++++------ 5 files changed, 63 insertions(+), 62 deletions(-) delete mode 100644 virtcontainers/errors.go create mode 100644 virtcontainers/pkg/types/errors.go diff --git a/virtcontainers/api.go b/virtcontainers/api.go index 5466d5f6d4..8925f458f1 100644 --- a/virtcontainers/api.go +++ b/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) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 083186f037..49e4772781 100644 --- a/virtcontainers/container.go +++ b/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) diff --git a/virtcontainers/errors.go b/virtcontainers/errors.go deleted file mode 100644 index 774ce2e31e..0000000000 --- a/virtcontainers/errors.go +++ /dev/null @@ -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") -) diff --git a/virtcontainers/pkg/types/errors.go b/virtcontainers/pkg/types/errors.go new file mode 100644 index 0000000000..2140227080 --- /dev/null +++ b/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") +) diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 7f88973adc..b398565db1 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -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 { @@ -686,11 +686,11 @@ 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 { @@ -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) From 8215a3ce9a6869546862314d10c50d4ee50098ef Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Fri, 12 Apr 2019 02:07:58 -0700 Subject: [PATCH 2/3] shimv2: convert vc errors to grpc errors containerd checks for the grpc error code to determine correct recover action upon grpc errors. We need to provide them properly. Unfortunately ttrpc doesn't support grpc interceptor so we have to modify every service function for it. Fixes: #1527 Signed-off-by: Peng Tao --- containerd-shim-v2/errors.go | 62 +++++++++++++++++ containerd-shim-v2/errors_test.go | 29 ++++++++ containerd-shim-v2/service.go | 106 +++++++++++++++++++++++++----- 3 files changed, 180 insertions(+), 17 deletions(-) create mode 100644 containerd-shim-v2/errors.go create mode 100644 containerd-shim-v2/errors_test.go diff --git a/containerd-shim-v2/errors.go b/containerd-shim-v2/errors.go new file mode 100644 index 0000000000..adfe5c052f --- /dev/null +++ b/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") +} diff --git a/containerd-shim-v2/errors_test.go b/containerd-shim-v2/errors_test.go new file mode 100644 index 0000000000..99168f1547 --- /dev/null +++ b/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)) + } +} diff --git a/containerd-shim-v2/service.go b/containerd-shim-v2/service.go index 8e7ede431b..f927e5ac64 100644 --- a/containerd-shim-v2/service.go +++ b/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() From f5125421d085c92e816ee3a7aa7a9465c54d7ca7 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Fri, 12 Apr 2019 03:15:47 -0700 Subject: [PATCH 3/3] sandbox: return ErrNoSuchContainer when failing to find a container So that caller can determine that it is ENOENT-alike error. Signed-off-by: Peng Tao --- virtcontainers/sandbox.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index b398565db1..1faf9f4441 100644 --- a/virtcontainers/sandbox.go +++ b/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 ( @@ -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) } @@ -694,7 +694,7 @@ func (s *Sandbox) removeContainer(containerID string) error { } 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) }