X Tutup
Skip to content

Commit 6d0bcd5

Browse files
stevvooestevvooe
committed
linux, linux/shim: remove error definitions
Since we now have a common set of error definitions, mapped to existing error codes, we no longer need the specialized error codes used for interaction with linux processes. The main issue was that string matching was being used to map these to useful error codes. With this change, we use errors defined in the `errdefs` package, which map cleanly to GRPC error codes and are recoverable on either side of the request. The main focus of this PR was in removin these from the shim. We may need follow ups to ensure error codes are preserved by the `Tasks` service. Signed-off-by: Stephen J Day <stephen.day@docker.com>
1 parent 6d305c7 commit 6d0bcd5

File tree

12 files changed

+82
-97
lines changed

12 files changed

+82
-97
lines changed

‎client_test.go‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111
"testing"
1212
"time"
1313

14+
"github.com/Sirupsen/logrus"
15+
"github.com/containerd/containerd/log"
1416
"github.com/containerd/containerd/namespaces"
1517
"github.com/containerd/containerd/testutil"
1618
)
@@ -78,13 +80,25 @@ func TestMain(m *testing.M) {
7880
os.Exit(1)
7981
}
8082

83+
// print out the version in information
84+
version, err := client.Version(ctx)
85+
if err != nil {
86+
fmt.Fprintf(os.Stderr, "error getting version: %s", err)
87+
os.Exit(1)
88+
}
89+
90+
// allow comparison with containerd under test
91+
log.G(ctx).WithFields(logrus.Fields{
92+
"version": version.Version,
93+
"revision": version.Revision,
94+
}).Info("running tests against containerd")
95+
8196
// pull a seed image
8297
if _, err = client.Pull(ctx, testImage, WithPullUnpack); err != nil {
8398
cmd.Process.Signal(syscall.SIGTERM)
8499
cmd.Wait()
85100
fmt.Fprintf(os.Stderr, "%s: %s", err, buf.String())
86101
os.Exit(1)
87-
88102
}
89103
if err := client.Close(); err != nil {
90104
fmt.Fprintln(os.Stderr, err)

‎container.go‎

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,25 +7,15 @@ import (
77
"strings"
88
"sync"
99

10-
"google.golang.org/grpc"
11-
"google.golang.org/grpc/codes"
12-
1310
"github.com/containerd/containerd/api/services/tasks/v1"
1411
"github.com/containerd/containerd/api/types"
1512
"github.com/containerd/containerd/containers"
13+
"github.com/containerd/containerd/errdefs"
1614
"github.com/containerd/containerd/typeurl"
1715
specs "github.com/opencontainers/runtime-spec/specs-go"
1816
"github.com/pkg/errors"
1917
)
2018

21-
var (
22-
ErrNoImage = errors.New("container does not have an image")
23-
ErrNoRunningTask = errors.New("no running task")
24-
ErrDeleteRunningTask = errors.New("cannot delete container with running task")
25-
ErrProcessExited = errors.New("process already exited")
26-
ErrNoExecID = errors.New("exec id must be provided")
27-
)
28-
2919
type DeleteOpts func(context.Context, *Client, containers.Container) error
3020

3121
type Container interface {
@@ -129,7 +119,7 @@ func WithRootFSDeletion(ctx context.Context, client *Client, c containers.Contai
129119
// an error is returned if the container has running tasks
130120
func (c *container) Delete(ctx context.Context, opts ...DeleteOpts) (err error) {
131121
if _, err := c.Task(ctx, nil); err == nil {
132-
return ErrDeleteRunningTask
122+
return errors.Wrapf(errdefs.ErrFailedPrecondition, "cannot delete running task %v", c.ID())
133123
}
134124
for _, o := range opts {
135125
if err := o(ctx, c.client, c.c); err != nil {
@@ -150,11 +140,11 @@ func (c *container) Task(ctx context.Context, attach IOAttach) (Task, error) {
150140
// Image returns the image that the container is based on
151141
func (c *container) Image(ctx context.Context) (Image, error) {
152142
if c.c.Image == "" {
153-
return nil, ErrNoImage
143+
return nil, errors.Wrapf(errdefs.ErrNotFound, "container not created from an image")
154144
}
155145
i, err := c.client.ImageService().Get(ctx, c.c.Image)
156146
if err != nil {
157-
return nil, err
147+
return nil, errors.Wrapf(err, "failed to get image for container")
158148
}
159149
return &image{
160150
client: c.client,
@@ -229,8 +219,9 @@ func (c *container) loadTask(ctx context.Context, ioAttach IOAttach) (Task, erro
229219
ContainerID: c.c.ID,
230220
})
231221
if err != nil {
232-
if grpc.Code(errors.Cause(err)) == codes.NotFound {
233-
return nil, ErrNoRunningTask
222+
err = errdefs.FromGRPC(err)
223+
if errdefs.IsNotFound(err) {
224+
return nil, errors.Wrapf(err, "no running task found")
234225
}
235226
return nil, err
236227
}

‎container_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"testing"
1212

1313
"github.com/containerd/cgroups"
14+
"github.com/containerd/containerd/errdefs"
1415
specs "github.com/opencontainers/runtime-spec/specs-go"
1516
)
1617

@@ -632,8 +633,8 @@ func TestDeleteRunningContainer(t *testing.T) {
632633
if err == nil {
633634
t.Error("delete did not error with running task")
634635
}
635-
if err != ErrDeleteRunningTask {
636-
t.Errorf("expected error %q but received %q", ErrDeleteRunningTask, err)
636+
if !errdefs.IsFailedPrecondition(err) {
637+
t.Errorf("expected error %q but received %q", errdefs.ErrFailedPrecondition, err)
637638
}
638639
if err := task.Kill(ctx, syscall.SIGKILL); err != nil {
639640
t.Error(err)
@@ -703,8 +704,8 @@ func TestContainerKill(t *testing.T) {
703704
t.Error("second call to kill should return an error")
704705
return
705706
}
706-
if err != ErrProcessExited {
707-
t.Errorf("expected error %q but received %q", ErrProcessExited, err)
707+
if !errdefs.IsNotFound(err) {
708+
t.Errorf("expected error %q but received %q", errdefs.ErrNotFound, err)
708709
}
709710
}
710711

‎linux/process.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,11 @@ package linux
44

55
import (
66
"context"
7-
"errors"
87

98
"github.com/containerd/containerd/api/types/task"
9+
"github.com/containerd/containerd/errdefs"
1010
shim "github.com/containerd/containerd/linux/shim/v1"
1111
"github.com/containerd/containerd/runtime"
12-
"google.golang.org/grpc"
1312
)
1413

1514
type Process struct {
@@ -27,7 +26,7 @@ func (p *Process) Kill(ctx context.Context, signal uint32, _ bool) error {
2726
ID: p.id,
2827
})
2928
if err != nil {
30-
err = errors.New(grpc.ErrorDesc(err))
29+
return errdefs.FromGRPC(err)
3130
}
3231
return err
3332
}
@@ -38,7 +37,7 @@ func (p *Process) State(ctx context.Context) (runtime.State, error) {
3837
ID: p.id,
3938
})
4039
if err != nil {
41-
return runtime.State{}, errors.New(grpc.ErrorDesc(err))
40+
return runtime.State{}, errdefs.FromGRPC(err)
4241
}
4342
var status runtime.Status
4443
switch response.Status {
@@ -69,7 +68,7 @@ func (p *Process) ResizePty(ctx context.Context, size runtime.ConsoleSize) error
6968
Height: size.Height,
7069
})
7170
if err != nil {
72-
err = errors.New(grpc.ErrorDesc(err))
71+
err = errdefs.FromGRPC(err)
7372
}
7473
return err
7574
}
@@ -80,7 +79,7 @@ func (p *Process) CloseIO(ctx context.Context) error {
8079
Stdin: true,
8180
})
8281
if err != nil {
83-
err = errors.New(grpc.ErrorDesc(err))
82+
err = errdefs.FromGRPC(err)
8483
}
8584
return err
8685
}

‎linux/runtime.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,9 @@ import (
99
"os"
1010
"path/filepath"
1111

12-
"google.golang.org/grpc"
13-
1412
"github.com/boltdb/bolt"
1513
"github.com/containerd/containerd/api/types"
14+
"github.com/containerd/containerd/errdefs"
1615
"github.com/containerd/containerd/identifiers"
1716
client "github.com/containerd/containerd/linux/shim"
1817
shim "github.com/containerd/containerd/linux/shim/v1"
@@ -170,7 +169,7 @@ func (r *Runtime) Create(ctx context.Context, id string, opts runtime.CreateOpts
170169
})
171170
}
172171
if _, err = s.Create(ctx, sopts); err != nil {
173-
return nil, errors.New(grpc.ErrorDesc(err))
172+
return nil, errdefs.FromGRPC(err)
174173
}
175174
t := newTask(id, namespace, s)
176175
if err := r.tasks.Add(ctx, t); err != nil {
@@ -197,7 +196,7 @@ func (r *Runtime) Delete(ctx context.Context, c runtime.Task) (*runtime.Exit, er
197196
}
198197
rsp, err := lc.shim.Delete(ctx, empty)
199198
if err != nil {
200-
return nil, errors.New(grpc.ErrorDesc(err))
199+
return nil, errdefs.FromGRPC(err)
201200
}
202201
if err := lc.shim.KillShim(ctx); err != nil {
203202
log.G(ctx).WithError(err).Error("failed to kill shim")

‎linux/shim/exec.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ func (e *execProcess) Resize(ws console.WinSize) error {
164164

165165
func (e *execProcess) Kill(ctx context.Context, sig uint32, _ bool) error {
166166
if err := unix.Kill(e.pid, syscall.Signal(sig)); err != nil {
167-
return checkKillError(err)
167+
return errors.Wrapf(checkKillError(err), "exec kill error")
168168
}
169169
return nil
170170
}

‎linux/shim/init.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ import (
1717
"golang.org/x/sys/unix"
1818

1919
"github.com/containerd/console"
20+
"github.com/containerd/containerd/errdefs"
2021
"github.com/containerd/containerd/identifiers"
2122
"github.com/containerd/containerd/linux/runcopts"
2223
shimapi "github.com/containerd/containerd/linux/shim/v1"
2324
"github.com/containerd/containerd/log"
2425
"github.com/containerd/containerd/mount"
25-
"github.com/containerd/containerd/runtime"
2626
"github.com/containerd/containerd/typeurl"
2727
"github.com/containerd/fifo"
2828
runc "github.com/containerd/go-runc"
@@ -388,7 +388,7 @@ func checkKillError(err error) error {
388388
return nil
389389
}
390390
if strings.Contains(err.Error(), "os: process already finished") || err == unix.ESRCH {
391-
return runtime.ErrProcessExited
391+
return errors.Wrapf(errdefs.ErrNotFound, "process already finished")
392392
}
393-
return err
393+
return errors.Wrapf(err, "unknown error after kill")
394394
}

0 commit comments

Comments
 (0)
X Tutup