From a4fadc596b219ad75837d97a1be3bc40e06d97ce Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Wed, 28 Jun 2017 21:32:15 -0700 Subject: [PATCH] errdefs: centralize error handling Now that we have most of the services required for use with containerd, it was found that common patterns were used throughout services. By defining a central `errdefs` package, we ensure that services will map errors to and from grpc consistently and cleanly. One can decorate an error with as much context as necessary, using `pkg/errors` and still have the error mapped correctly via grpc. We make a few sacrifices. At this point, the common errors we use across the repository all map directly to grpc error codes. While this seems positively crazy, it actually works out quite well. The error conditions that were specific weren't super necessary and the ones that were necessary now simply have better context information. We lose the ability to add new codes, but this constraint may not be a bad thing. Effectively, as long as one uses the errors defined in `errdefs`, the error class will be mapped correctly across the grpc boundary and everything will be good. If you don't use those definitions, the error maps to "unknown" and the error message is preserved. Signed-off-by: Stephen J Day --- cmd/ctr/namespaces.go | 4 +- cmd/dist/delete.go | 4 +- cmd/dist/fetch.go | 3 +- cmd/dist/images.go | 4 +- container_unix.go | 4 +- content/errors.go | 98 ------------------------- content/helpers.go | 5 +- content/locks.go | 6 +- content/store.go | 11 +-- content/writer.go | 3 +- errdefs/errors.go | 54 ++++++++++++++ errdefs/grpc.go | 106 ++++++++++++++++++++++++++++ identifiers/validate.go | 12 +--- identifiers/validate_test.go | 4 +- metadata/containers.go | 19 +++-- metadata/errors.go | 95 ------------------------- metadata/images.go | 8 ++- metadata/namespaces.go | 8 ++- namespaces/context.go | 12 +--- remotes/docker/pusher.go | 7 +- remotes/docker/schema1/converter.go | 3 +- remotes/docker/status.go | 4 +- remotes/handlers.go | 7 +- rootfs/apply.go | 3 +- services/containers/helpers.go | 20 ------ services/containers/service.go | 28 ++++---- services/content/helpers.go | 33 --------- services/content/service.go | 17 ++--- services/content/store.go | 15 ++-- services/content/writer.go | 3 +- services/images/client.go | 9 +-- services/images/helpers.go | 36 ---------- services/images/service.go | 15 ++-- services/namespaces/client.go | 11 +-- services/namespaces/helpers.go | 36 ---------- services/namespaces/service.go | 11 +-- services/snapshot/client.go | 43 +++-------- services/snapshot/service.go | 30 +++----- services/tasks/service.go | 21 +++--- snapshot/errors.go | 44 ------------ snapshot/storage/bolt.go | 21 +++--- snapshot/storage/metastore_test.go | 9 +-- 42 files changed, 331 insertions(+), 555 deletions(-) delete mode 100644 content/errors.go create mode 100644 errdefs/errors.go create mode 100644 errdefs/grpc.go delete mode 100644 metadata/errors.go delete mode 100644 services/content/helpers.go delete mode 100644 services/namespaces/helpers.go delete mode 100644 snapshot/errors.go diff --git a/cmd/ctr/namespaces.go b/cmd/ctr/namespaces.go index 784a61856..c86255391 100644 --- a/cmd/ctr/namespaces.go +++ b/cmd/ctr/namespaces.go @@ -8,8 +8,8 @@ import ( "strings" "text/tabwriter" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/log" - "github.com/containerd/containerd/metadata" "github.com/pkg/errors" "github.com/urfave/cli" ) @@ -183,7 +183,7 @@ var namespacesRemoveCommand = cli.Command{ for _, target := range clicontext.Args() { if err := namespaces.Delete(ctx, target); err != nil { - if !metadata.IsNotFound(err) { + if !errdefs.IsNotFound(err) { if exitErr == nil { exitErr = errors.Wrapf(err, "unable to delete %v", target) } diff --git a/cmd/dist/delete.go b/cmd/dist/delete.go index 4a1be57e2..924912409 100644 --- a/cmd/dist/delete.go +++ b/cmd/dist/delete.go @@ -3,7 +3,7 @@ package main import ( "fmt" - "github.com/containerd/containerd/content" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/log" digest "github.com/opencontainers/go-digest" "github.com/urfave/cli" @@ -43,7 +43,7 @@ var deleteCommand = cli.Command{ } if err := cs.Delete(ctx, dgst); err != nil { - if !content.IsNotFound(err) { + if !errdefs.IsNotFound(err) { if exitError == nil { exitError = err } diff --git a/cmd/dist/fetch.go b/cmd/dist/fetch.go index e541e575c..d8f2f6848 100644 --- a/cmd/dist/fetch.go +++ b/cmd/dist/fetch.go @@ -11,6 +11,7 @@ import ( "github.com/containerd/containerd" "github.com/containerd/containerd/content" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" "github.com/containerd/containerd/log" "github.com/containerd/containerd/progress" @@ -153,7 +154,7 @@ outer: if !done && (!ok || status.Status == "downloading") { info, err := cs.Info(ctx, j.Digest) if err != nil { - if !content.IsNotFound(err) { + if !errdefs.IsNotFound(err) { log.G(ctx).WithError(err).Errorf("failed to get content info") continue outer } else { diff --git a/cmd/dist/images.go b/cmd/dist/images.go index 03d05ee19..8fa2b58b0 100644 --- a/cmd/dist/images.go +++ b/cmd/dist/images.go @@ -5,8 +5,8 @@ import ( "os" "text/tabwriter" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/log" - "github.com/containerd/containerd/metadata" "github.com/containerd/containerd/progress" "github.com/pkg/errors" "github.com/urfave/cli" @@ -83,7 +83,7 @@ var imageRemoveCommand = cli.Command{ for _, target := range clicontext.Args() { if err := imageStore.Delete(ctx, target); err != nil { - if !metadata.IsNotFound(err) { + if !errdefs.IsNotFound(err) { if exitErr == nil { exitErr = errors.Wrapf(err, "unable to delete %v", target) } diff --git a/container_unix.go b/container_unix.go index 8cc263857..7e41bc1b4 100644 --- a/container_unix.go +++ b/container_unix.go @@ -12,8 +12,8 @@ import ( "github.com/containerd/containerd/api/services/tasks/v1" "github.com/containerd/containerd/api/types" "github.com/containerd/containerd/content" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" - "github.com/containerd/containerd/snapshot" protobuf "github.com/gogo/protobuf/types" digest "github.com/opencontainers/go-digest" "github.com/opencontainers/image-spec/identity" @@ -46,7 +46,7 @@ func WithCheckpoint(desc v1.Descriptor, rootfsID string) NewContainerOpts { return err } if _, err := client.SnapshotService().Prepare(ctx, rootfsID, identity.ChainID(diffIDs).String()); err != nil { - if !snapshot.IsExist(err) { + if !errdefs.IsAlreadyExists(err) { return err } } diff --git a/content/errors.go b/content/errors.go deleted file mode 100644 index cd33e3ae9..000000000 --- a/content/errors.go +++ /dev/null @@ -1,98 +0,0 @@ -package content - -import "github.com/pkg/errors" - -type contentExistsErr struct { - desc string -} - -type contentNotFoundErr struct { - desc string -} - -type contentLockedErr struct { - desc string -} - -// ErrExists is returned when something exists when it may not be expected. -func ErrExists(msg string) error { - if msg == "" { - msg = "content: exists" - } - return errors.WithStack(contentExistsErr{ - desc: msg, - }) -} - -// ErrNotFound is returned when an item is not found. -func ErrNotFound(msg string) error { - if msg == "" { - msg = "content: not found" - } - return errors.WithStack(contentNotFoundErr{ - desc: msg, - }) -} - -// ErrLocked is returned when content is actively being uploaded, this -// indicates that another process is attempting to upload the same content. -func ErrLocked(msg string) error { - if msg == "" { - msg = "content: locked" - } - return errors.WithStack(contentLockedErr{ - desc: msg, - }) -} - -func (c contentExistsErr) Error() string { - return c.desc -} -func (c contentNotFoundErr) Error() string { - return c.desc -} -func (c contentLockedErr) Error() string { - return c.desc -} - -func (c contentExistsErr) Exists() bool { - return true -} - -func (c contentNotFoundErr) NotFound() bool { - return true -} - -func (c contentLockedErr) Locked() bool { - return true -} - -// IsNotFound returns true if the error is due to a not found content item -func IsNotFound(err error) bool { - if err, ok := errors.Cause(err).(interface { - NotFound() bool - }); ok { - return err.NotFound() - } - return false -} - -// IsExists returns true if the error is due to an already existing content item -func IsExists(err error) bool { - if err, ok := errors.Cause(err).(interface { - Exists() bool - }); ok { - return err.Exists() - } - return false -} - -// IsLocked returns true if the error is due to a currently locked content item -func IsLocked(err error) bool { - if err, ok := errors.Cause(err).(interface { - Locked() bool - }); ok { - return err.Locked() - } - return false -} diff --git a/content/helpers.go b/content/helpers.go index 3610c844f..3a241c7f2 100644 --- a/content/helpers.go +++ b/content/helpers.go @@ -6,6 +6,7 @@ import ( "io" "io/ioutil" + "github.com/containerd/containerd/errdefs" "github.com/opencontainers/go-digest" "github.com/pkg/errors" ) @@ -33,7 +34,7 @@ func ReadBlob(ctx context.Context, provider Provider, dgst digest.Digest) ([]byt func WriteBlob(ctx context.Context, cs Ingester, ref string, r io.Reader, size int64, expected digest.Digest) error { cw, err := cs.Writer(ctx, ref, size, expected) if err != nil { - if !IsExists(err) { + if !errdefs.IsAlreadyExists(err) { return err } @@ -79,7 +80,7 @@ func Copy(cw Writer, r io.Reader, size int64, expected digest.Digest) error { } if err := cw.Commit(size, expected); err != nil { - if !IsExists(err) { + if !errdefs.IsAlreadyExists(err) { return errors.Wrapf(err, "failed commit on ref %q", ws.Ref) } } diff --git a/content/locks.go b/content/locks.go index 4d3aa110a..c7b1e437a 100644 --- a/content/locks.go +++ b/content/locks.go @@ -1,8 +1,10 @@ package content import ( - "fmt" "sync" + + "github.com/containerd/containerd/errdefs" + "github.com/pkg/errors" ) // Handles locking references @@ -19,7 +21,7 @@ func tryLock(ref string) error { defer locksMu.Unlock() if _, ok := locks[ref]; ok { - return ErrLocked(fmt.Sprintf("key %s is locked", ref)) + return errors.Wrapf(errdefs.ErrUnavailable, "ref %s locked", ref) } locks[ref] = struct{}{} diff --git a/content/store.go b/content/store.go index f9c068751..a1092f791 100644 --- a/content/store.go +++ b/content/store.go @@ -11,6 +11,7 @@ import ( "strconv" "time" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/log" digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" @@ -40,7 +41,7 @@ func (s *store) Info(ctx context.Context, dgst digest.Digest) (Info, error) { fi, err := os.Stat(p) if err != nil { if os.IsNotExist(err) { - err = ErrNotFound("") + err = errors.Wrapf(errdefs.ErrNotFound, "content %v", dgst) } return Info{}, err @@ -62,7 +63,7 @@ func (s *store) Reader(ctx context.Context, dgst digest.Digest) (io.ReadCloser, fp, err := os.Open(s.blobPath(dgst)) if err != nil { if os.IsNotExist(err) { - err = ErrNotFound("") + err = errors.Wrapf(errdefs.ErrNotFound, "content %v", dgst) } return nil, err } @@ -85,7 +86,7 @@ func (cs *store) Delete(ctx context.Context, dgst digest.Digest) error { return err } - return ErrNotFound("") + return errors.Wrapf(errdefs.ErrNotFound, "content %v", dgst) } return nil @@ -232,7 +233,7 @@ func (s *store) Writer(ctx context.Context, ref string, total int64, expected di if expected != "" { p := s.blobPath(expected) if _, err := os.Stat(p); err == nil { - return nil, ErrExists("") + return nil, errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", expected) } } @@ -329,7 +330,7 @@ func (s *store) Abort(ctx context.Context, ref string) error { root := s.ingestRoot(ref) if err := os.RemoveAll(root); err != nil { if os.IsNotExist(err) { - return ErrNotFound("") + return errors.Wrapf(errdefs.ErrNotFound, "ingest ref %q", ref) } return err diff --git a/content/writer.go b/content/writer.go index 5ac3b7e40..5fbaac2f5 100644 --- a/content/writer.go +++ b/content/writer.go @@ -5,6 +5,7 @@ import ( "path/filepath" "time" + "github.com/containerd/containerd/errdefs" "github.com/opencontainers/go-digest" "github.com/pkg/errors" ) @@ -99,7 +100,7 @@ func (w *writer) Commit(size int64, expected digest.Digest) error { if err := os.Rename(ingest, target); err != nil { if os.IsExist(err) { // collision with the target file! - return ErrExists("") + return errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", dgst) } return err } diff --git a/errdefs/errors.go b/errdefs/errors.go new file mode 100644 index 000000000..f84e0dae9 --- /dev/null +++ b/errdefs/errors.go @@ -0,0 +1,54 @@ +// Package errdefs defines the common errors used throughout containerd +// packages. +// +// Use with errors.Wrap and error.Wrapf to add context to an error. +// +// To detect an error class, use the IsXXX functions to tell whether an error +// is of a certain type. +// +// The functions ToGRPC and FromGRPC can be used to map server-side and +// client-side errors to the correct types. +package errdefs + +import "github.com/pkg/errors" + +// Definitions of common error types used throughout containerd. All containerd +// errors returned by most packages will map into one of these errors classes. +// Packages should return errors of these types when they want to instruct a +// client to take a particular action. +// +// For the most part, we just try to provide local grpc errors. Most conditions +// map very well to those defined by grpc. +var ( + ErrUnknown = errors.New("unknown") // used internally to represent a missed mapping. + ErrInvalidArgument = errors.New("invalid") + ErrNotFound = errors.New("not found") + ErrAlreadyExists = errors.New("already exists") + ErrFailedPrecondition = errors.New("failed precondition") + ErrUnavailable = errors.New("unavailable") +) + +func IsInvalidArgument(err error) bool { + return errors.Cause(err) == ErrInvalidArgument +} + +// IsNotFound returns true if the error is due to a missing object +func IsNotFound(err error) bool { + return errors.Cause(err) == ErrNotFound +} + +// IsAlreadyExists returns true if the error is due to an already existing +// metadata item +func IsAlreadyExists(err error) bool { + return errors.Cause(err) == ErrAlreadyExists +} + +// IsFailedPrecondition returns true if an operation could not proceed to the +// lack of a particular condition. +func IsFailedPrecondition(err error) bool { + return errors.Cause(err) == ErrFailedPrecondition +} + +func IsUnavailable(err error) bool { + return errors.Cause(err) == ErrUnavailable +} diff --git a/errdefs/grpc.go b/errdefs/grpc.go new file mode 100644 index 000000000..eb690e87d --- /dev/null +++ b/errdefs/grpc.go @@ -0,0 +1,106 @@ +package errdefs + +import ( + "strings" + + "github.com/pkg/errors" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// ToGRPC will attempt to map the backend containerd error into a grpc error, +// using the original error message as a description. +// +// Further information may be extracted from certain errors depending on their +// type. +// +// If the error is unmapped, the original error will be returned to be handled +// by the regular grpc error handling stack. +func ToGRPC(err error) error { + if err == nil { + return nil + } + + if isGRPCError(err) { + // error has already been mapped to grpc + return err + } + + switch { + case IsInvalidArgument(err): + return grpc.Errorf(codes.InvalidArgument, err.Error()) + case IsNotFound(err): + return grpc.Errorf(codes.NotFound, err.Error()) + case IsAlreadyExists(err): + return grpc.Errorf(codes.AlreadyExists, err.Error()) + case IsFailedPrecondition(err): + return grpc.Errorf(codes.FailedPrecondition, err.Error()) + case IsUnavailable(err): + return grpc.Errorf(codes.Unavailable, err.Error()) + } + + return err +} + +// ToGRPCf maps the error to grpc error codes, assembling the formatting string +// and combining it with the target error string. +// +// This is equivalent to errors.ToGRPC(errors.Wrapf(err, format, args...)) +func ToGRPCf(err error, format string, args ...interface{}) error { + return ToGRPC(errors.Wrapf(err, format, args...)) +} + +func FromGRPC(err error) error { + if err == nil { + return nil + } + + var cls error // divide these into error classes, becomes the cause + + switch grpc.Code(err) { + case codes.InvalidArgument: + cls = ErrInvalidArgument + case codes.AlreadyExists: + cls = ErrAlreadyExists + case codes.NotFound: + cls = ErrNotFound + case codes.Unavailable: + cls = ErrUnavailable + case codes.FailedPrecondition: + cls = ErrFailedPrecondition + default: + cls = ErrUnknown + } + + if cls != nil { + msg := rebaseMessage(cls, err) + if msg != "" { + err = errors.Wrapf(cls, msg) + } else { + err = cls + } + } + + return err +} + +// rebaseMessage removes the repeats for an error at the end of an error +// string. This will happen when taking an error over grpc then remapping it. +// +// Effectively, we just remove the string of cls from the end of err if it +// appears there. +func rebaseMessage(cls error, err error) string { + desc := grpc.ErrorDesc(err) + clss := cls.Error() + if desc == clss { + return "" + } + + return strings.TrimSuffix(desc, ": "+clss) +} + +func isGRPCError(err error) bool { + _, ok := status.FromError(err) + return ok +} diff --git a/identifiers/validate.go b/identifiers/validate.go index 9e477d55a..b99881ea8 100644 --- a/identifiers/validate.go +++ b/identifiers/validate.go @@ -17,6 +17,7 @@ package identifiers import ( "regexp" + "github.com/containerd/containerd/errdefs" "github.com/pkg/errors" ) @@ -32,15 +33,8 @@ var ( // Rules for domains, defined in RFC 1035, section 2.3.1, are used for // identifiers. identifierRe = regexp.MustCompile(reAnchor(label + reGroup("[.]"+reGroup(label)) + "*")) - - errIdentifierInvalid = errors.New("invalid identifier") ) -// IsInvalid return true if the error was due to an invalid identifer. -func IsInvalid(err error) bool { - return errors.Cause(err) == errIdentifierInvalid -} - // Validate return nil if the string s is a valid identifier. // // identifiers must be valid domain identifiers according to RFC 1035, section 2.3.1. To @@ -50,11 +44,11 @@ func IsInvalid(err error) bool { // a domain identifier or filesystem path component. func Validate(s string) error { if len(s) > maxLength { - return errors.Wrapf(errIdentifierInvalid, "identifier %q greater than maximum length (%d characters)", s, maxLength) + return errors.Wrapf(errdefs.ErrInvalidArgument, "identifier %q greater than maximum length (%d characters)", s, maxLength) } if !identifierRe.MatchString(s) { - return errors.Wrapf(errIdentifierInvalid, "identifier %q must match %v", s, identifierRe) + return errors.Wrapf(errdefs.ErrInvalidArgument, "identifier %q must match %v", s, identifierRe) } return nil } diff --git a/identifiers/validate_test.go b/identifiers/validate_test.go index 12a5ac93b..4f8f9267f 100644 --- a/identifiers/validate_test.go +++ b/identifiers/validate_test.go @@ -3,6 +3,8 @@ package identifiers import ( "strings" "testing" + + "github.com/containerd/containerd/errdefs" ) func TestValidIdentifiers(t *testing.T) { @@ -44,7 +46,7 @@ func TestInvalidIdentifiers(t *testing.T) { t.Run(input, func(t *testing.T) { if err := Validate(input); err == nil { t.Fatal("expected invalid error") - } else if !IsInvalid(err) { + } else if !errdefs.IsInvalidArgument(err) { t.Fatal("error should be an invalid identifier error") } }) diff --git a/metadata/containers.go b/metadata/containers.go index 836309865..6989c3c83 100644 --- a/metadata/containers.go +++ b/metadata/containers.go @@ -6,6 +6,7 @@ import ( "github.com/boltdb/bolt" "github.com/containerd/containerd/containers" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/identifiers" "github.com/containerd/containerd/namespaces" "github.com/pkg/errors" @@ -29,12 +30,12 @@ func (s *containerStore) Get(ctx context.Context, id string) (containers.Contain bkt := getContainerBucket(s.tx, namespace, id) if bkt == nil { - return containers.Container{}, ErrNotFound("bucket does not exist") + return containers.Container{}, errors.Wrapf(errdefs.ErrNotFound, "bucket name %q") } container := containers.Container{ID: id} if err := readContainer(&container, bkt); err != nil { - return containers.Container{}, errors.Wrap(err, "failed to read container") + return containers.Container{}, errors.Wrapf(err, "failed to read container %v", id) } return container, nil @@ -90,7 +91,7 @@ func (s *containerStore) Create(ctx context.Context, container containers.Contai cbkt, err := bkt.CreateBucket([]byte(container.ID)) if err != nil { if err == bolt.ErrBucketExists { - err = ErrExists("content for id already exists") + err = errors.Wrapf(errdefs.ErrAlreadyExists, "content %q") } return containers.Container{}, err } @@ -110,14 +111,18 @@ func (s *containerStore) Update(ctx context.Context, container containers.Contai return containers.Container{}, err } + if container.ID == "" { + return containers.Container{}, errors.Wrapf(errdefs.ErrInvalidArgument, "must specify a container id") + } + bkt := getContainersBucket(s.tx, namespace) if bkt == nil { - return containers.Container{}, ErrNotFound("no containers") + return containers.Container{}, errors.Wrapf(errdefs.ErrNotFound, "container %q", container.ID) } cbkt := bkt.Bucket([]byte(container.ID)) if cbkt == nil { - return containers.Container{}, ErrNotFound("no content for id") + return containers.Container{}, errors.Wrapf(errdefs.ErrNotFound, "container %q", container.ID) } container.UpdatedAt = time.Now() @@ -136,11 +141,11 @@ func (s *containerStore) Delete(ctx context.Context, id string) error { bkt := getContainersBucket(s.tx, namespace) if bkt == nil { - return ErrNotFound("no containers") + return errors.Wrapf(errdefs.ErrNotFound, "cannot delete container %v, bucket not present", id) } if err := bkt.DeleteBucket([]byte(id)); err == bolt.ErrBucketNotFound { - return ErrNotFound("no content for id") + return errors.Wrapf(errdefs.ErrNotFound, "container %v", id) } return err } diff --git a/metadata/errors.go b/metadata/errors.go deleted file mode 100644 index f9955c0b7..000000000 --- a/metadata/errors.go +++ /dev/null @@ -1,95 +0,0 @@ -package metadata - -import "github.com/pkg/errors" - -type metadataExistsErr struct { - desc string -} -type metadataNotFoundErr struct { - desc string -} -type metadataNotEmptyErr struct { - desc string -} - -// ErrExists is returned when an item already exists in metadata -func ErrExists(msg string) error { - if msg == "" { - msg = "metadata: exists" - } - return errors.WithStack(metadataExistsErr{ - desc: msg, - }) -} - -// ErrNotFound is returned when an item cannot be found in metadata -func ErrNotFound(msg string) error { - if msg == "" { - msg = "metadata: not found" - } - return errors.WithStack(metadataNotFoundErr{ - desc: msg, - }) -} - -// ErrNotEmpty is returned when a metadata item can't be deleted because it is not empty -func ErrNotEmpty(msg string) error { - if msg == "" { - msg = "metadata: namespace not empty" - } - return errors.WithStack(metadataNotEmptyErr{ - desc: msg, - }) -} - -func (m metadataExistsErr) Error() string { - return m.desc -} -func (m metadataNotFoundErr) Error() string { - return m.desc -} -func (m metadataNotEmptyErr) Error() string { - return m.desc -} - -func (m metadataExistsErr) Exists() bool { - return true -} - -func (m metadataNotFoundErr) NotFound() bool { - return true -} - -func (m metadataNotEmptyErr) NotEmpty() bool { - return true -} - -// IsNotFound returns true if the error is due to a missing metadata item -func IsNotFound(err error) bool { - if err, ok := errors.Cause(err).(interface { - NotFound() bool - }); ok { - return err.NotFound() - } - return false -} - -// IsExists returns true if the error is due to an already existing metadata item -func IsExists(err error) bool { - if err, ok := errors.Cause(err).(interface { - Exists() bool - }); ok { - return err.Exists() - } - return false -} - -// IsNotEmpty returns true if the error is due to delete request of a non-empty metadata item -func IsNotEmpty(err error) bool { - if err, ok := errors.Cause(err).(interface { - NotEmpty() bool - }); ok { - return err.NotEmpty() - } - return false -} diff --git a/metadata/images.go b/metadata/images.go index 1d0a6ba81..2771a6751 100644 --- a/metadata/images.go +++ b/metadata/images.go @@ -6,10 +6,12 @@ import ( "fmt" "github.com/boltdb/bolt" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" "github.com/containerd/containerd/namespaces" digest "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/pkg/errors" ) type imageStore struct { @@ -30,12 +32,12 @@ func (s *imageStore) Get(ctx context.Context, name string) (images.Image, error) bkt := getImagesBucket(s.tx, namespace) if bkt == nil { - return images.Image{}, ErrNotFound("") + return images.Image{}, errors.Wrapf(errdefs.ErrNotFound, "image %q", name) } ibkt := bkt.Bucket([]byte(name)) if ibkt == nil { - return images.Image{}, ErrNotFound("") + return images.Image{}, errors.Wrapf(errdefs.ErrNotFound, "image %q", name) } image.Name = name @@ -124,7 +126,7 @@ func (s *imageStore) Delete(ctx context.Context, name string) error { return withImagesBucket(s.tx, namespace, func(bkt *bolt.Bucket) error { err := bkt.DeleteBucket([]byte(name)) if err == bolt.ErrBucketNotFound { - return ErrNotFound("") + return errors.Wrapf(errdefs.ErrNotFound, "image %q", name) } return err }) diff --git a/metadata/namespaces.go b/metadata/namespaces.go index 89a5af1bc..1b3c2ef1b 100644 --- a/metadata/namespaces.go +++ b/metadata/namespaces.go @@ -4,8 +4,10 @@ import ( "context" "github.com/boltdb/bolt" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/identifiers" "github.com/containerd/containerd/namespaces" + "github.com/pkg/errors" ) type namespaceStore struct { @@ -30,7 +32,7 @@ func (s *namespaceStore) Create(ctx context.Context, namespace string, labels ma bkt, err := topbkt.CreateBucket([]byte(namespace)) if err != nil { if err == bolt.ErrBucketExists { - return ErrExists("") + return errors.Wrapf(errdefs.ErrAlreadyExists, "namespace %q") } return err @@ -105,12 +107,12 @@ func (s *namespaceStore) Delete(ctx context.Context, namespace string) error { if empty, err := s.namespaceEmpty(ctx, namespace); err != nil { return err } else if !empty { - return ErrNotEmpty("") + return errors.Wrapf(errdefs.ErrFailedPrecondition, "namespace %q must be empty", namespace) } if err := bkt.DeleteBucket([]byte(namespace)); err != nil { if err == bolt.ErrBucketNotFound { - return ErrNotFound("") + return errors.Wrapf(errdefs.ErrNotFound, "namespace %q", namespace) } return err diff --git a/namespaces/context.go b/namespaces/context.go index d7515b54d..e38bda721 100644 --- a/namespaces/context.go +++ b/namespaces/context.go @@ -3,6 +3,7 @@ package namespaces import ( "os" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/identifiers" "github.com/pkg/errors" "golang.org/x/net/context" @@ -13,10 +14,6 @@ const ( Default = "default" ) -var ( - errNamespaceRequired = errors.New("namespace is required") -) - type namespaceKey struct{} // WithNamespace sets a given namespace on the context @@ -50,16 +47,11 @@ func Namespace(ctx context.Context) (string, bool) { return namespace, ok } -// IsNamespaceRequired returns whether an error is caused by a missing namespace -func IsNamespaceRequired(err error) bool { - return errors.Cause(err) == errNamespaceRequired -} - // NamespaceRequired returns the valid namepace from the context or an error. func NamespaceRequired(ctx context.Context) (string, error) { namespace, ok := Namespace(ctx) if !ok || namespace == "" { - return "", errNamespaceRequired + return "", errors.Wrapf(errdefs.ErrFailedPrecondition, "namespace is required") } if err := identifiers.Validate(namespace); err != nil { diff --git a/remotes/docker/pusher.go b/remotes/docker/pusher.go index e2c08e3cb..bc9c42e2a 100644 --- a/remotes/docker/pusher.go +++ b/remotes/docker/pusher.go @@ -10,6 +10,7 @@ import ( "time" "github.com/containerd/containerd/content" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" "github.com/containerd/containerd/log" "github.com/containerd/containerd/remotes" @@ -31,10 +32,10 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten status, err := p.tracker.GetStatus(ref) if err == nil { if status.Offset == status.Total { - return nil, content.ErrExists("") + return nil, errors.Wrapf(errdefs.ErrAlreadyExists, "ref %v already exists", ref) } // TODO: Handle incomplete status - } else if !content.IsNotFound(err) { + } else if !errdefs.IsNotFound(err) { return nil, errors.Wrap(err, "failed to get status") } @@ -72,7 +73,7 @@ func (p dockerPusher) Push(ctx context.Context, desc ocispec.Descriptor) (conten // TODO: Set updated time? }, }) - return nil, content.ErrExists("") + return nil, errors.Wrapf(errdefs.ErrAlreadyExists, "content %v on remote", desc.Digest) } if resp.StatusCode != http.StatusNotFound { // TODO: log error diff --git a/remotes/docker/schema1/converter.go b/remotes/docker/schema1/converter.go index cc36c647c..2cf882f7b 100644 --- a/remotes/docker/schema1/converter.go +++ b/remotes/docker/schema1/converter.go @@ -16,6 +16,7 @@ import ( "golang.org/x/sync/errgroup" "github.com/containerd/containerd/content" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" "github.com/containerd/containerd/log" "github.com/containerd/containerd/remotes" @@ -202,7 +203,7 @@ func (c *Converter) fetchBlob(ctx context.Context, desc ocispec.Descriptor) erro cw, err := c.contentStore.Writer(ctx, ref, desc.Size, desc.Digest) if err != nil { - if !content.IsExists(err) { + if !errdefs.IsAlreadyExists(err) { return err } diff --git a/remotes/docker/status.go b/remotes/docker/status.go index dc564b12c..442f9ae0d 100644 --- a/remotes/docker/status.go +++ b/remotes/docker/status.go @@ -4,6 +4,8 @@ import ( "sync" "github.com/containerd/containerd/content" + "github.com/containerd/containerd/errdefs" + "github.com/pkg/errors" ) type Status struct { @@ -34,7 +36,7 @@ func (t *memoryStatusTracker) GetStatus(ref string) (Status, error) { defer t.m.Unlock() status, ok := t.statuses[ref] if !ok { - return Status{}, content.ErrNotFound("") + return Status{}, errors.Wrapf(errdefs.ErrNotFound, "status for ref %v", ref) } return status, nil } diff --git a/remotes/handlers.go b/remotes/handlers.go index a81177326..cdcf71467 100644 --- a/remotes/handlers.go +++ b/remotes/handlers.go @@ -7,6 +7,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/containerd/containerd/content" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" "github.com/containerd/containerd/log" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -69,9 +70,9 @@ func fetch(ctx context.Context, ingester content.Ingester, fetcher Fetcher, desc for { cw, err = ingester.Writer(ctx, ref, desc.Size, desc.Digest) if err != nil { - if content.IsExists(err) { + if errdefs.IsAlreadyExists(err) { return nil - } else if !content.IsLocked(err) { + } else if !errdefs.IsUnavailable(err) { return err } @@ -122,7 +123,7 @@ func push(ctx context.Context, provider content.Provider, pusher Pusher, desc oc cw, err := pusher.Push(ctx, desc) if err != nil { - if !content.IsExists(err) { + if !errdefs.IsAlreadyExists(err) { return err } diff --git a/rootfs/apply.go b/rootfs/apply.go index 59b27442c..ac2139c27 100644 --- a/rootfs/apply.go +++ b/rootfs/apply.go @@ -3,6 +3,7 @@ package rootfs import ( "fmt" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/log" "github.com/containerd/containerd/mount" "github.com/containerd/containerd/snapshot" @@ -46,7 +47,7 @@ func applyLayer(ctx context.Context, layer Layer, chain []digest.Digest, sn snap if err == nil { log.G(ctx).Debugf("Extraction not needed, layer snapshot exists") return nil - } else if !snapshot.IsNotExist(err) { + } else if !errdefs.IsNotFound(err) { return errors.Wrap(err, "failed to stat snapshot") } diff --git a/services/containers/helpers.go b/services/containers/helpers.go index 99960c567..7d84fc4e7 100644 --- a/services/containers/helpers.go +++ b/services/containers/helpers.go @@ -3,13 +3,8 @@ package containers import ( api "github.com/containerd/containerd/api/services/containers/v1" "github.com/containerd/containerd/containers" - "github.com/containerd/containerd/identifiers" - "github.com/containerd/containerd/metadata" - "github.com/containerd/containerd/namespaces" "github.com/gogo/protobuf/types" specs "github.com/opencontainers/runtime-spec/specs-go" - "google.golang.org/grpc" - "google.golang.org/grpc/codes" ) func containersToProto(containers []containers.Container) []api.Container { @@ -52,18 +47,3 @@ func containerFromProto(containerpb *api.Container) containers.Container { RootFS: containerpb.RootFS, } } - -func mapGRPCError(err error, id string) error { - switch { - case metadata.IsNotFound(err): - return grpc.Errorf(codes.NotFound, "container %v not found", id) - case metadata.IsExists(err): - return grpc.Errorf(codes.AlreadyExists, "container %v already exists", id) - case namespaces.IsNamespaceRequired(err): - return grpc.Errorf(codes.InvalidArgument, "namespace required, please set %q header", namespaces.GRPCHeader) - case identifiers.IsInvalid(err): - return grpc.Errorf(codes.InvalidArgument, err.Error()) - } - - return err -} diff --git a/services/containers/service.go b/services/containers/service.go index 185228c2a..567d52eac 100644 --- a/services/containers/service.go +++ b/services/containers/service.go @@ -5,6 +5,7 @@ import ( api "github.com/containerd/containerd/api/services/containers/v1" eventsapi "github.com/containerd/containerd/api/services/events/v1" "github.com/containerd/containerd/containers" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/events" "github.com/containerd/containerd/metadata" "github.com/containerd/containerd/plugin" @@ -49,31 +50,30 @@ func (s *Service) Register(server *grpc.Server) error { func (s *Service) Get(ctx context.Context, req *api.GetContainerRequest) (*api.GetContainerResponse, error) { var resp api.GetContainerResponse - return &resp, s.withStoreView(ctx, func(ctx context.Context, store containers.Store) error { + return &resp, errdefs.ToGRPC(s.withStoreView(ctx, func(ctx context.Context, store containers.Store) error { container, err := store.Get(ctx, req.ID) if err != nil { - return mapGRPCError(err, req.ID) + return err } containerpb := containerToProto(&container) resp.Container = containerpb return nil - }) + })) } func (s *Service) List(ctx context.Context, req *api.ListContainersRequest) (*api.ListContainersResponse, error) { var resp api.ListContainersResponse - return &resp, s.withStoreView(ctx, func(ctx context.Context, store containers.Store) error { + return &resp, errdefs.ToGRPC(s.withStoreView(ctx, func(ctx context.Context, store containers.Store) error { containers, err := store.List(ctx, req.Filter) if err != nil { - return mapGRPCError(err, "") + return err } resp.Containers = containersToProto(containers) return nil - }) - + })) } func (s *Service) Create(ctx context.Context, req *api.CreateContainerRequest) (*api.CreateContainerResponse, error) { @@ -84,14 +84,14 @@ func (s *Service) Create(ctx context.Context, req *api.CreateContainerRequest) ( created, err := store.Create(ctx, container) if err != nil { - return mapGRPCError(err, req.Container.ID) + return err } resp.Container = containerToProto(&created) return nil }); err != nil { - return &resp, err + return &resp, errdefs.ToGRPC(err) } if err := s.emit(ctx, "/containers/create", &eventsapi.ContainerCreate{ ContainerID: resp.Container.ID, @@ -115,7 +115,7 @@ func (s *Service) Update(ctx context.Context, req *api.UpdateContainerRequest) ( current, err := store.Get(ctx, container.ID) if err != nil { - return mapGRPCError(err, container.ID) + return err } if current.ID != container.ID { @@ -150,14 +150,14 @@ func (s *Service) Update(ctx context.Context, req *api.UpdateContainerRequest) ( created, err := store.Update(ctx, container) if err != nil { - return mapGRPCError(err, req.Container.ID) + return err } resp.Container = containerToProto(&created) return nil }); err != nil { - return &resp, err + return &resp, errdefs.ToGRPC(err) } if err := s.emit(ctx, "/containers/update", &eventsapi.ContainerUpdate{ @@ -174,9 +174,9 @@ func (s *Service) Update(ctx context.Context, req *api.UpdateContainerRequest) ( func (s *Service) Delete(ctx context.Context, req *api.DeleteContainerRequest) (*empty.Empty, error) { if err := s.withStoreUpdate(ctx, func(ctx context.Context, store containers.Store) error { - return mapGRPCError(store.Delete(ctx, req.ID), req.ID) + return store.Delete(ctx, req.ID) }); err != nil { - return &empty.Empty{}, mapGRPCError(err, req.ID) + return &empty.Empty{}, errdefs.ToGRPC(err) } if err := s.emit(ctx, "/containers/delete", &eventsapi.ContainerDelete{ diff --git a/services/content/helpers.go b/services/content/helpers.go deleted file mode 100644 index a0a12898e..000000000 --- a/services/content/helpers.go +++ /dev/null @@ -1,33 +0,0 @@ -package content - -import ( - "github.com/containerd/containerd/content" - "github.com/pkg/errors" - "google.golang.org/grpc" - "google.golang.org/grpc/codes" -) - -func rewriteGRPCError(err error) error { - switch grpc.Code(errors.Cause(err)) { - case codes.AlreadyExists: - return content.ErrExists(grpc.ErrorDesc(err)) - case codes.NotFound: - return content.ErrNotFound(grpc.ErrorDesc(err)) - case codes.Unavailable: - return content.ErrLocked(grpc.ErrorDesc(err)) - } - return err -} - -func serverErrorToGRPC(err error, id string) error { - switch { - case content.IsNotFound(err): - return grpc.Errorf(codes.NotFound, "%v: not found", id) - case content.IsExists(err): - return grpc.Errorf(codes.AlreadyExists, "%v: exists", id) - case content.IsLocked(err): - return grpc.Errorf(codes.Unavailable, "%v: locked", id) - } - - return err -} diff --git a/services/content/service.go b/services/content/service.go index ca9cd8148..322d956da 100644 --- a/services/content/service.go +++ b/services/content/service.go @@ -8,6 +8,7 @@ import ( api "github.com/containerd/containerd/api/services/content/v1" eventsapi "github.com/containerd/containerd/api/services/events/v1" "github.com/containerd/containerd/content" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/events" "github.com/containerd/containerd/log" "github.com/containerd/containerd/plugin" @@ -66,7 +67,7 @@ func (s *Service) Info(ctx context.Context, req *api.InfoRequest) (*api.InfoResp bi, err := s.store.Info(ctx, req.Digest) if err != nil { - return nil, serverErrorToGRPC(err, req.Digest.String()) + return nil, errdefs.ToGRPC(err) } return &api.InfoResponse{ @@ -125,7 +126,7 @@ func (s *Service) Delete(ctx context.Context, req *api.DeleteContentRequest) (*e } if err := s.store.Delete(ctx, req.Digest); err != nil { - return nil, serverErrorToGRPC(err, req.Digest.String()) + return nil, errdefs.ToGRPC(err) } if err := s.emit(ctx, "/content/delete", &eventsapi.ContentDelete{ @@ -144,12 +145,12 @@ func (s *Service) Read(req *api.ReadContentRequest, session api.Content_ReadServ oi, err := s.store.Info(session.Context(), req.Digest) if err != nil { - return serverErrorToGRPC(err, req.Digest.String()) + return errdefs.ToGRPC(err) } rc, err := s.store.Reader(session.Context(), req.Digest) if err != nil { - return serverErrorToGRPC(err, req.Digest.String()) + return errdefs.ToGRPC(err) } defer rc.Close() // TODO(stevvooe): Cache these file descriptors for performance. @@ -217,7 +218,7 @@ func (rw *readResponseWriter) Write(p []byte) (n int, err error) { func (s *Service) Status(ctx context.Context, req *api.StatusRequest) (*api.StatusResponse, error) { statuses, err := s.store.Status(ctx, req.Filter) if err != nil { - return nil, serverErrorToGRPC(err, req.Filter) + return nil, errdefs.ToGRPCf(err, "could not get status for filter %q", req.Filter) } var resp api.StatusResponse @@ -295,7 +296,7 @@ func (s *Service) Write(session api.Content_WriteServer) (err error) { // this action locks the writer for the session. wr, err := s.store.Writer(ctx, ref, total, expected) if err != nil { - return serverErrorToGRPC(err, ref) + return errdefs.ToGRPC(err) } defer wr.Close() @@ -303,7 +304,7 @@ func (s *Service) Write(session api.Content_WriteServer) (err error) { msg.Action = req.Action ws, err := wr.Status() if err != nil { - return serverErrorToGRPC(err, ref) + return errdefs.ToGRPC(err) } msg.Offset = ws.Offset // always set the offset. @@ -414,7 +415,7 @@ func (s *Service) Write(session api.Content_WriteServer) (err error) { func (s *Service) Abort(ctx context.Context, req *api.AbortRequest) (*empty.Empty, error) { if err := s.store.Abort(ctx, req.Ref); err != nil { - return nil, serverErrorToGRPC(err, req.Ref) + return nil, errdefs.ToGRPC(err) } return &empty.Empty{}, nil diff --git a/services/content/store.go b/services/content/store.go index 4c0fbe5c4..2153e3d6e 100644 --- a/services/content/store.go +++ b/services/content/store.go @@ -6,6 +6,7 @@ import ( contentapi "github.com/containerd/containerd/api/services/content/v1" "github.com/containerd/containerd/content" + "github.com/containerd/containerd/errdefs" digest "github.com/opencontainers/go-digest" ) @@ -24,7 +25,7 @@ func (rs *remoteStore) Info(ctx context.Context, dgst digest.Digest) (content.In Digest: dgst, }) if err != nil { - return content.Info{}, rewriteGRPCError(err) + return content.Info{}, errdefs.FromGRPC(err) } return content.Info{ @@ -37,14 +38,14 @@ func (rs *remoteStore) Info(ctx context.Context, dgst digest.Digest) (content.In func (rs *remoteStore) Walk(ctx context.Context, fn content.WalkFunc) error { session, err := rs.client.List(ctx, &contentapi.ListContentRequest{}) if err != nil { - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } for { msg, err := session.Recv() if err != nil { if err != io.EOF { - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } break @@ -68,7 +69,7 @@ func (rs *remoteStore) Delete(ctx context.Context, dgst digest.Digest) error { if _, err := rs.client.Delete(ctx, &contentapi.DeleteContentRequest{ Digest: dgst, }); err != nil { - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } return nil @@ -98,7 +99,7 @@ func (rs *remoteStore) Status(ctx context.Context, filter string) ([]content.Sta Filter: filter, }) if err != nil { - return nil, rewriteGRPCError(err) + return nil, errdefs.FromGRPC(err) } var statuses []content.Status @@ -119,7 +120,7 @@ func (rs *remoteStore) Status(ctx context.Context, filter string) ([]content.Sta func (rs *remoteStore) Writer(ctx context.Context, ref string, size int64, expected digest.Digest) (content.Writer, error) { wrclient, offset, err := rs.negotiate(ctx, ref, size, expected) if err != nil { - return nil, rewriteGRPCError(err) + return nil, errdefs.FromGRPC(err) } return &remoteWriter{ @@ -134,7 +135,7 @@ func (rs *remoteStore) Abort(ctx context.Context, ref string) error { if _, err := rs.client.Abort(ctx, &contentapi.AbortRequest{ Ref: ref, }); err != nil { - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } return nil diff --git a/services/content/writer.go b/services/content/writer.go index dfa42690b..eeb1fca3d 100644 --- a/services/content/writer.go +++ b/services/content/writer.go @@ -5,6 +5,7 @@ import ( contentapi "github.com/containerd/containerd/api/services/content/v1" "github.com/containerd/containerd/content" + "github.com/containerd/containerd/errdefs" digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" ) @@ -86,7 +87,7 @@ func (rw *remoteWriter) Commit(size int64, expected digest.Digest) error { Expected: expected, }) if err != nil { - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } if size != 0 && resp.Offset != size { diff --git a/services/images/client.go b/services/images/client.go index 75e20b6bc..8452832e8 100644 --- a/services/images/client.go +++ b/services/images/client.go @@ -4,6 +4,7 @@ import ( "context" imagesapi "github.com/containerd/containerd/api/services/images/v1" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -28,7 +29,7 @@ func (s *remoteStore) Update(ctx context.Context, name string, desc ocispec.Desc }, }) - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } func (s *remoteStore) Get(ctx context.Context, name string) (images.Image, error) { @@ -36,7 +37,7 @@ func (s *remoteStore) Get(ctx context.Context, name string) (images.Image, error Name: name, }) if err != nil { - return images.Image{}, rewriteGRPCError(err) + return images.Image{}, errdefs.FromGRPC(err) } return imageFromProto(resp.Image), nil @@ -45,7 +46,7 @@ func (s *remoteStore) Get(ctx context.Context, name string) (images.Image, error func (s *remoteStore) List(ctx context.Context) ([]images.Image, error) { resp, err := s.client.List(ctx, &imagesapi.ListImagesRequest{}) if err != nil { - return nil, rewriteGRPCError(err) + return nil, errdefs.FromGRPC(err) } return imagesFromProto(resp.Images), nil @@ -56,5 +57,5 @@ func (s *remoteStore) Delete(ctx context.Context, name string) error { Name: name, }) - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } diff --git a/services/images/helpers.go b/services/images/helpers.go index 575ef0468..2693bad1c 100644 --- a/services/images/helpers.go +++ b/services/images/helpers.go @@ -3,14 +3,8 @@ package images import ( imagesapi "github.com/containerd/containerd/api/services/images/v1" "github.com/containerd/containerd/api/types" - "github.com/containerd/containerd/identifiers" "github.com/containerd/containerd/images" - "github.com/containerd/containerd/metadata" - "github.com/containerd/containerd/namespaces" ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "github.com/pkg/errors" - "google.golang.org/grpc" - "google.golang.org/grpc/codes" ) func imagesToProto(images []images.Image) []imagesapi.Image { @@ -62,33 +56,3 @@ func descToProto(desc *ocispec.Descriptor) types.Descriptor { Digest: desc.Digest, } } - -func rewriteGRPCError(err error) error { - if err == nil { - return err - } - - switch grpc.Code(errors.Cause(err)) { - case codes.AlreadyExists: - return metadata.ErrExists(grpc.ErrorDesc(err)) - case codes.NotFound: - return metadata.ErrNotFound(grpc.ErrorDesc(err)) - } - - return err -} - -func mapGRPCError(err error, id string) error { - switch { - case metadata.IsNotFound(err): - return grpc.Errorf(codes.NotFound, "image %v not found", id) - case metadata.IsExists(err): - return grpc.Errorf(codes.AlreadyExists, "image %v already exists", id) - case namespaces.IsNamespaceRequired(err): - return grpc.Errorf(codes.InvalidArgument, "namespace required, please set %q header", namespaces.GRPCHeader) - case identifiers.IsInvalid(err): - return grpc.Errorf(codes.InvalidArgument, err.Error()) - } - - return err -} diff --git a/services/images/service.go b/services/images/service.go index 9456794b1..edfb56c04 100644 --- a/services/images/service.go +++ b/services/images/service.go @@ -4,6 +4,7 @@ import ( "github.com/boltdb/bolt" eventsapi "github.com/containerd/containerd/api/services/events/v1" imagesapi "github.com/containerd/containerd/api/services/images/v1" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/events" "github.com/containerd/containerd/images" "github.com/containerd/containerd/metadata" @@ -51,22 +52,22 @@ func (s *Service) Register(server *grpc.Server) error { func (s *Service) Get(ctx context.Context, req *imagesapi.GetImageRequest) (*imagesapi.GetImageResponse, error) { var resp imagesapi.GetImageResponse - return &resp, s.withStoreView(ctx, func(ctx context.Context, store images.Store) error { + return &resp, errdefs.ToGRPC(s.withStoreView(ctx, func(ctx context.Context, store images.Store) error { image, err := store.Get(ctx, req.Name) if err != nil { - return mapGRPCError(err, req.Name) + return err } imagepb := imageToProto(&image) resp.Image = &imagepb return nil - }) + })) } func (s *Service) Update(ctx context.Context, req *imagesapi.UpdateImageRequest) (*imagesapi.UpdateImageResponse, error) { if err := s.withStoreUpdate(ctx, func(ctx context.Context, store images.Store) error { - return mapGRPCError(store.Update(ctx, req.Image.Name, descFromProto(&req.Image.Target)), req.Image.Name) + return store.Update(ctx, req.Image.Name, descFromProto(&req.Image.Target)) }); err != nil { - return nil, err + return nil, errdefs.ToGRPC(err) } if err := s.emit(ctx, "/images/update", &eventsapi.ImageUpdate{ @@ -88,7 +89,7 @@ func (s *Service) List(ctx context.Context, _ *imagesapi.ListImagesRequest) (*im return &resp, s.withStoreView(ctx, func(ctx context.Context, store images.Store) error { images, err := store.List(ctx) if err != nil { - return mapGRPCError(err, "") + return errdefs.ToGRPC(err) } resp.Images = imagesToProto(images) @@ -98,7 +99,7 @@ func (s *Service) List(ctx context.Context, _ *imagesapi.ListImagesRequest) (*im func (s *Service) Delete(ctx context.Context, req *imagesapi.DeleteImageRequest) (*empty.Empty, error) { if err := s.withStoreUpdate(ctx, func(ctx context.Context, store images.Store) error { - return mapGRPCError(store.Delete(ctx, req.Name), req.Name) + return errdefs.ToGRPC(store.Delete(ctx, req.Name)) }); err != nil { return nil, err } diff --git a/services/namespaces/client.go b/services/namespaces/client.go index c3a148f5e..80709f905 100644 --- a/services/namespaces/client.go +++ b/services/namespaces/client.go @@ -5,6 +5,7 @@ import ( "strings" api "github.com/containerd/containerd/api/services/namespaces/v1" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/namespaces" "github.com/gogo/protobuf/types" ) @@ -27,7 +28,7 @@ func (r *remote) Create(ctx context.Context, namespace string, labels map[string _, err := r.client.Create(ctx, &req) if err != nil { - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } return nil @@ -39,7 +40,7 @@ func (r *remote) Labels(ctx context.Context, namespace string) (map[string]strin resp, err := r.client.Get(ctx, &req) if err != nil { - return nil, rewriteGRPCError(err) + return nil, errdefs.FromGRPC(err) } return resp.Namespace.Labels, nil @@ -59,7 +60,7 @@ func (r *remote) SetLabel(ctx context.Context, namespace, key, value string) err _, err := r.client.Update(ctx, &req) if err != nil { - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } return nil @@ -70,7 +71,7 @@ func (r *remote) List(ctx context.Context) ([]string, error) { resp, err := r.client.List(ctx, &req) if err != nil { - return nil, rewriteGRPCError(err) + return nil, errdefs.FromGRPC(err) } var namespaces []string @@ -88,7 +89,7 @@ func (r *remote) Delete(ctx context.Context, namespace string) error { req.Name = namespace _, err := r.client.Delete(ctx, &req) if err != nil { - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } return nil diff --git a/services/namespaces/helpers.go b/services/namespaces/helpers.go deleted file mode 100644 index 4092f2d4f..000000000 --- a/services/namespaces/helpers.go +++ /dev/null @@ -1,36 +0,0 @@ -package namespaces - -import ( - "github.com/containerd/containerd/metadata" - "github.com/pkg/errors" - "google.golang.org/grpc" - "google.golang.org/grpc/codes" -) - -func mapGRPCError(err error, id string) error { - switch { - case metadata.IsNotFound(err): - return grpc.Errorf(codes.NotFound, "namespace %v not found", id) - case metadata.IsExists(err): - return grpc.Errorf(codes.AlreadyExists, "namespace %v already exists", id) - case metadata.IsNotEmpty(err): - return grpc.Errorf(codes.FailedPrecondition, "namespace %v must be empty", id) - } - - return err -} - -func rewriteGRPCError(err error) error { - if err == nil { - return err - } - - switch grpc.Code(errors.Cause(err)) { - case codes.AlreadyExists: - return metadata.ErrExists(grpc.ErrorDesc(err)) - case codes.NotFound: - return metadata.ErrNotFound(grpc.ErrorDesc(err)) - } - - return err -} diff --git a/services/namespaces/service.go b/services/namespaces/service.go index 7ac29e79a..d0de39295 100644 --- a/services/namespaces/service.go +++ b/services/namespaces/service.go @@ -6,6 +6,7 @@ import ( "github.com/boltdb/bolt" eventsapi "github.com/containerd/containerd/api/services/events/v1" api "github.com/containerd/containerd/api/services/namespaces/v1" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/events" "github.com/containerd/containerd/metadata" "github.com/containerd/containerd/namespaces" @@ -59,7 +60,7 @@ func (s *Service) Get(ctx context.Context, req *api.GetNamespaceRequest) (*api.G return &resp, s.withStoreView(ctx, func(ctx context.Context, store namespaces.Store) error { labels, err := store.Labels(ctx, req.Name) if err != nil { - return mapGRPCError(err, req.Name) + return errdefs.ToGRPC(err) } resp.Namespace = api.Namespace{ @@ -85,7 +86,7 @@ func (s *Service) List(ctx context.Context, req *api.ListNamespacesRequest) (*ap if err != nil { // In general, this should be unlikely, since we are holding a // transaction to service this request. - return mapGRPCError(err, namespace) + return errdefs.ToGRPC(err) } resp.Namespaces = append(resp.Namespaces, api.Namespace{ @@ -103,7 +104,7 @@ func (s *Service) Create(ctx context.Context, req *api.CreateNamespaceRequest) ( if err := s.withStoreUpdate(ctx, func(ctx context.Context, store namespaces.Store) error { if err := store.Create(ctx, req.Namespace.Name, req.Namespace.Labels); err != nil { - return mapGRPCError(err, req.Namespace.Name) + return errdefs.ToGRPC(err) } for k, v := range req.Namespace.Labels { @@ -149,7 +150,7 @@ func (s *Service) Update(ctx context.Context, req *api.UpdateNamespaceRequest) ( // get current set of labels labels, err := store.Labels(ctx, req.Namespace.Name) if err != nil { - return mapGRPCError(err, req.Namespace.Name) + return errdefs.ToGRPC(err) } for k := range labels { @@ -183,7 +184,7 @@ func (s *Service) Update(ctx context.Context, req *api.UpdateNamespaceRequest) ( func (s *Service) Delete(ctx context.Context, req *api.DeleteNamespaceRequest) (*empty.Empty, error) { if err := s.withStoreUpdate(ctx, func(ctx context.Context, store namespaces.Store) error { - return mapGRPCError(store.Delete(ctx, req.Name), req.Name) + return errdefs.ToGRPC(store.Delete(ctx, req.Name)) }); err != nil { return &empty.Empty{}, err } diff --git a/services/snapshot/client.go b/services/snapshot/client.go index 458f0d592..35ca10181 100644 --- a/services/snapshot/client.go +++ b/services/snapshot/client.go @@ -3,16 +3,12 @@ package snapshot import ( "context" "io" - "strings" - - "google.golang.org/grpc" - "google.golang.org/grpc/codes" snapshotapi "github.com/containerd/containerd/api/services/snapshot/v1" "github.com/containerd/containerd/api/types" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/mount" "github.com/containerd/containerd/snapshot" - "github.com/pkg/errors" ) // NewSnapshotterFromClient returns a new Snapshotter which communicates @@ -30,7 +26,7 @@ type remoteSnapshotter struct { func (r *remoteSnapshotter) Stat(ctx context.Context, key string) (snapshot.Info, error) { resp, err := r.client.Stat(ctx, &snapshotapi.StatSnapshotRequest{Key: key}) if err != nil { - return snapshot.Info{}, rewriteGRPCError(err) + return snapshot.Info{}, errdefs.FromGRPC(err) } return toInfo(resp.Info), nil } @@ -38,7 +34,7 @@ func (r *remoteSnapshotter) Stat(ctx context.Context, key string) (snapshot.Info func (r *remoteSnapshotter) Usage(ctx context.Context, key string) (snapshot.Usage, error) { resp, err := r.client.Usage(ctx, &snapshotapi.UsageRequest{Key: key}) if err != nil { - return snapshot.Usage{}, rewriteGRPCError(err) + return snapshot.Usage{}, errdefs.FromGRPC(err) } return toUsage(resp), nil } @@ -46,7 +42,7 @@ func (r *remoteSnapshotter) Usage(ctx context.Context, key string) (snapshot.Usa func (r *remoteSnapshotter) Mounts(ctx context.Context, key string) ([]mount.Mount, error) { resp, err := r.client.Mounts(ctx, &snapshotapi.MountsRequest{Key: key}) if err != nil { - return nil, rewriteGRPCError(err) + return nil, errdefs.FromGRPC(err) } return toMounts(resp.Mounts), nil } @@ -54,7 +50,7 @@ func (r *remoteSnapshotter) Mounts(ctx context.Context, key string) ([]mount.Mou func (r *remoteSnapshotter) Prepare(ctx context.Context, key, parent string) ([]mount.Mount, error) { resp, err := r.client.Prepare(ctx, &snapshotapi.PrepareSnapshotRequest{Key: key, Parent: parent}) if err != nil { - return nil, rewriteGRPCError(err) + return nil, errdefs.FromGRPC(err) } return toMounts(resp.Mounts), nil } @@ -62,7 +58,7 @@ func (r *remoteSnapshotter) Prepare(ctx context.Context, key, parent string) ([] func (r *remoteSnapshotter) View(ctx context.Context, key, parent string) ([]mount.Mount, error) { resp, err := r.client.View(ctx, &snapshotapi.ViewSnapshotRequest{Key: key, Parent: parent}) if err != nil { - return nil, rewriteGRPCError(err) + return nil, errdefs.FromGRPC(err) } return toMounts(resp.Mounts), nil } @@ -72,18 +68,18 @@ func (r *remoteSnapshotter) Commit(ctx context.Context, name, key string) error Name: name, Key: key, }) - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } func (r *remoteSnapshotter) Remove(ctx context.Context, key string) error { _, err := r.client.Remove(ctx, &snapshotapi.RemoveSnapshotRequest{Key: key}) - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } func (r *remoteSnapshotter) Walk(ctx context.Context, fn func(context.Context, snapshot.Info) error) error { sc, err := r.client.List(ctx, &snapshotapi.ListSnapshotsRequest{}) if err != nil { - rewriteGRPCError(err) + errdefs.FromGRPC(err) } for { resp, err := sc.Recv() @@ -91,7 +87,7 @@ func (r *remoteSnapshotter) Walk(ctx context.Context, fn func(context.Context, s if err == io.EOF { return nil } - return rewriteGRPCError(err) + return errdefs.FromGRPC(err) } if resp == nil { return nil @@ -104,25 +100,6 @@ func (r *remoteSnapshotter) Walk(ctx context.Context, fn func(context.Context, s } } -func rewriteGRPCError(err error) error { - switch grpc.Code(errors.Cause(err)) { - case codes.AlreadyExists: - return snapshot.ErrSnapshotExist - case codes.NotFound: - return snapshot.ErrSnapshotNotExist - case codes.FailedPrecondition: - desc := grpc.ErrorDesc(errors.Cause(err)) - if strings.Contains(desc, snapshot.ErrSnapshotNotActive.Error()) { - return snapshot.ErrSnapshotNotActive - } - if strings.Contains(desc, snapshot.ErrSnapshotNotCommitted.Error()) { - return snapshot.ErrSnapshotNotCommitted - } - } - - return err -} - func toKind(kind snapshotapi.Kind) snapshot.Kind { if kind == snapshotapi.KindActive { return snapshot.KindActive diff --git a/services/snapshot/service.go b/services/snapshot/service.go index dcf044544..8a0b8f836 100644 --- a/services/snapshot/service.go +++ b/services/snapshot/service.go @@ -6,6 +6,7 @@ import ( eventsapi "github.com/containerd/containerd/api/services/events/v1" snapshotapi "github.com/containerd/containerd/api/services/snapshot/v1" "github.com/containerd/containerd/api/types" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/events" "github.com/containerd/containerd/log" "github.com/containerd/containerd/mount" @@ -14,7 +15,6 @@ import ( protoempty "github.com/golang/protobuf/ptypes/empty" "golang.org/x/net/context" "google.golang.org/grpc" - "google.golang.org/grpc/codes" ) func init() { @@ -60,7 +60,7 @@ func (s *service) Prepare(ctx context.Context, pr *snapshotapi.PrepareSnapshotRe // TODO: Lookup snapshot id from metadata store mounts, err := s.snapshotter.Prepare(ctx, pr.Key, pr.Parent) if err != nil { - return nil, grpcError(err) + return nil, errdefs.ToGRPC(err) } if err := s.emit(ctx, "/snapshot/prepare", &eventsapi.SnapshotPrepare{ @@ -80,7 +80,7 @@ func (s *service) View(ctx context.Context, pr *snapshotapi.ViewSnapshotRequest) // TODO: Lookup snapshot id from metadata store mounts, err := s.snapshotter.View(ctx, pr.Key, pr.Parent) if err != nil { - return nil, grpcError(err) + return nil, errdefs.ToGRPC(err) } return &snapshotapi.ViewSnapshotResponse{ Mounts: fromMounts(mounts), @@ -93,7 +93,7 @@ func (s *service) Mounts(ctx context.Context, mr *snapshotapi.MountsRequest) (*s // TODO: Lookup snapshot id from metadata store mounts, err := s.snapshotter.Mounts(ctx, mr.Key) if err != nil { - return nil, grpcError(err) + return nil, errdefs.ToGRPC(err) } return &snapshotapi.MountsResponse{ Mounts: fromMounts(mounts), @@ -105,7 +105,7 @@ func (s *service) Commit(ctx context.Context, cr *snapshotapi.CommitSnapshotRequ // TODO: Apply namespace // TODO: Lookup snapshot id from metadata store if err := s.snapshotter.Commit(ctx, cr.Name, cr.Key); err != nil { - return nil, grpcError(err) + return nil, errdefs.ToGRPC(err) } if err := s.emit(ctx, "/snapshot/commit", &eventsapi.SnapshotCommit{ @@ -122,7 +122,7 @@ func (s *service) Remove(ctx context.Context, rr *snapshotapi.RemoveSnapshotRequ // TODO: Apply namespace // TODO: Lookup snapshot id from metadata store if err := s.snapshotter.Remove(ctx, rr.Key); err != nil { - return nil, grpcError(err) + return nil, errdefs.ToGRPC(err) } if err := s.emit(ctx, "/snapshot/remove", &eventsapi.SnapshotRemove{ @@ -138,7 +138,7 @@ func (s *service) Stat(ctx context.Context, sr *snapshotapi.StatSnapshotRequest) // TODO: Apply namespace info, err := s.snapshotter.Stat(ctx, sr.Key) if err != nil { - return nil, grpcError(err) + return nil, errdefs.ToGRPC(err) } return &snapshotapi.StatSnapshotResponse{Info: fromInfo(info)}, nil @@ -184,26 +184,12 @@ func (s *service) Usage(ctx context.Context, ur *snapshotapi.UsageRequest) (*sna // TODO: Apply namespace usage, err := s.snapshotter.Usage(ctx, ur.Key) if err != nil { - return nil, grpcError(err) + return nil, errdefs.ToGRPC(err) } return fromUsage(usage), nil } -func grpcError(err error) error { - if snapshot.IsNotExist(err) { - return grpc.Errorf(codes.NotFound, err.Error()) - } - if snapshot.IsExist(err) { - return grpc.Errorf(codes.AlreadyExists, err.Error()) - } - if snapshot.IsNotActive(err) || snapshot.IsNotCommitted(err) { - return grpc.Errorf(codes.FailedPrecondition, err.Error()) - } - - return err -} - func fromKind(kind snapshot.Kind) snapshotapi.Kind { if kind == snapshot.KindActive { return snapshotapi.KindActive diff --git a/services/tasks/service.go b/services/tasks/service.go index c9bd387f1..ffc31efdf 100644 --- a/services/tasks/service.go +++ b/services/tasks/service.go @@ -16,6 +16,7 @@ import ( "github.com/containerd/containerd/archive" "github.com/containerd/containerd/containers" "github.com/containerd/containerd/content" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/events" "github.com/containerd/containerd/images" "github.com/containerd/containerd/log" @@ -36,6 +37,9 @@ var ( empty = &google_protobuf.Empty{} ) +// TODO(stevvooe): Clean up error mapping to avoid double mapping certain +// errors within helper methods. + func init() { plugin.Register(&plugin.Registration{ Type: plugin.GRPCPlugin, @@ -114,14 +118,7 @@ func (s *Service) Create(ctx context.Context, r *api.CreateTaskRequest) (*api.Cr container, err := s.getContainer(ctx, r.ContainerID) if err != nil { - switch { - case metadata.IsNotFound(err): - return nil, grpc.Errorf(codes.NotFound, "container %v not found", r.ContainerID) - case metadata.IsExists(err): - return nil, grpc.Errorf(codes.AlreadyExists, "container %v already exists", r.ContainerID) - } - - return nil, err + return nil, errdefs.ToGRPC(err) } opts := plugin.CreateOpts{ @@ -491,7 +488,7 @@ func (s *Service) getContainer(ctx context.Context, id string) (containers.Conta container, err = store.Get(ctx, id) return err }); err != nil { - return containers.Container{}, err + return containers.Container{}, errdefs.ToGRPC(err) } return container, nil @@ -500,12 +497,12 @@ func (s *Service) getContainer(ctx context.Context, id string) (containers.Conta func (s *Service) getTask(ctx context.Context, id string) (plugin.Task, error) { container, err := s.getContainer(ctx, id) if err != nil { - return nil, grpc.Errorf(codes.InvalidArgument, "task %v not found: %s", id, err.Error()) + return nil, err } runtime, err := s.getRuntime(container.Runtime.Name) if err != nil { - return nil, grpc.Errorf(codes.NotFound, "task %v not found: %s", id, err.Error()) + return nil, errdefs.ToGRPCf(err, "runtime for task %v", id) } t, err := runtime.Get(ctx, id) @@ -519,7 +516,7 @@ func (s *Service) getTask(ctx context.Context, id string) (plugin.Task, error) { func (s *Service) getRuntime(name string) (plugin.Runtime, error) { runtime, ok := s.runtimes[name] if !ok { - return nil, fmt.Errorf("unknown runtime %q", name) + return nil, grpc.Errorf(codes.NotFound, "unknown runtime %q", name) } return runtime, nil } diff --git a/snapshot/errors.go b/snapshot/errors.go deleted file mode 100644 index 0b6e5174b..000000000 --- a/snapshot/errors.go +++ /dev/null @@ -1,44 +0,0 @@ -package snapshot - -import "github.com/pkg/errors" - -var ( - // ErrSnapshotNotExist is returned when a snapshot cannot be found - ErrSnapshotNotExist = errors.New("snapshot does not exist") - - // ErrSnapshotExist is returned when an operation to create a snapshot - // encounters a snapshot with the same key - ErrSnapshotExist = errors.New("snapshot already exists") - - // ErrSnapshotNotActive is returned when a request which requires an - // active snapshot encounters a non-active snapshot. - ErrSnapshotNotActive = errors.New("snapshot is not active") - - // ErrSnapshotNotCommitted is returned when a request which requires a - // committed snapshot encounters a non-committed snapshot. - ErrSnapshotNotCommitted = errors.New("snapshot is not committed") -) - -// IsNotExist returns whether the error represents that a snapshot -// was not found. -func IsNotExist(err error) bool { - return errors.Cause(err) == ErrSnapshotNotExist -} - -// IsExist returns whether the error represents whether a snapshot -// already exists using a provided key. -func IsExist(err error) bool { - return errors.Cause(err) == ErrSnapshotExist -} - -// IsNotActive returns whether the error represents a request -// for a non active snapshot when an active snapshot is expected. -func IsNotActive(err error) bool { - return errors.Cause(err) == ErrSnapshotNotActive -} - -// IsNotCommitted returns whether the error represents a request -// for a non committed snapshot when a committed snapshot is expected. -func IsNotCommitted(err error) bool { - return errors.Cause(err) == ErrSnapshotNotCommitted -} diff --git a/snapshot/storage/bolt.go b/snapshot/storage/bolt.go index 8be5245da..37952fd47 100644 --- a/snapshot/storage/bolt.go +++ b/snapshot/storage/bolt.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/boltdb/bolt" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/namespaces" "github.com/containerd/containerd/snapshot" db "github.com/containerd/containerd/snapshot/storage/proto" @@ -129,12 +130,12 @@ func CreateActive(ctx context.Context, key, parent string, readonly bool) (a Act } if parentS.Kind != db.KindCommitted { - return errors.Wrap(snapshot.ErrSnapshotNotCommitted, "parent is not committed snapshot") + return errors.Wrap(errdefs.ErrInvalidArgument, "parent is not committed snapshot") } } b := bkt.Get([]byte(key)) if len(b) != 0 { - return snapshot.ErrSnapshotExist + return errors.Wrapf(errdefs.ErrAlreadyExists, "snapshot %v", key) } id, err := nextSequence(ctx) @@ -183,7 +184,7 @@ func GetActive(ctx context.Context, key string) (a Active, err error) { err = withBucket(ctx, func(ctx context.Context, bkt, pbkt *bolt.Bucket) error { b := bkt.Get([]byte(key)) if len(b) == 0 { - return snapshot.ErrSnapshotNotExist + return errors.Wrapf(errdefs.ErrNotFound, "snapshot %v", key) } var ss db.Snapshot @@ -191,7 +192,7 @@ func GetActive(ctx context.Context, key string) (a Active, err error) { return errors.Wrap(err, "failed to unmarshal snapshot") } if ss.Kind != db.KindActive { - return snapshot.ErrSnapshotNotActive + return errors.Wrapf(errdefs.ErrFailedPrecondition, "requested snapshot %v not active", key) } a.ID = fmt.Sprintf("%d", ss.ID) @@ -225,7 +226,7 @@ func Remove(ctx context.Context, key string) (id string, k snapshot.Kind, err er var ss db.Snapshot b := bkt.Get([]byte(key)) if len(b) == 0 { - return snapshot.ErrSnapshotNotExist + return errors.Wrapf(errdefs.ErrNotFound, "snapshot %v", key) } if err := proto.Unmarshal(b, &ss); err != nil { @@ -273,7 +274,7 @@ func CommitActive(ctx context.Context, key, name string, usage snapshot.Usage) ( err = withBucket(ctx, func(ctx context.Context, bkt, pbkt *bolt.Bucket) error { b := bkt.Get([]byte(name)) if len(b) != 0 { - return errors.Wrap(snapshot.ErrSnapshotExist, "committed name already exists") + return errors.Wrapf(errdefs.ErrAlreadyExists, "committed snapshot %v", name) } var ss db.Snapshot @@ -281,7 +282,7 @@ func CommitActive(ctx context.Context, key, name string, usage snapshot.Usage) ( return errors.Wrap(err, "failed to get active snapshot") } if ss.Kind != db.KindActive { - return snapshot.ErrSnapshotNotActive + return errors.Wrapf(errdefs.ErrFailedPrecondition, "snapshot %v is not active", name) } if ss.Readonly { return errors.Errorf("active snapshot is readonly") @@ -340,12 +341,12 @@ func withBucket(ctx context.Context, fn func(context.Context, *bolt.Bucket, *bol } nbkt := t.tx.Bucket(bucketKeyStorageVersion) if nbkt == nil { - return errors.Wrap(snapshot.ErrSnapshotNotExist, "bucket does not exist") + return errors.Wrapf(errdefs.ErrNotFound, "bucket does not exist") } bkt := nbkt.Bucket([]byte(namespace)) if bkt == nil { - return errors.Wrap(snapshot.ErrSnapshotNotExist, "namespace not available in snapshotter") + return errors.Wrapf(errdefs.ErrNotFound, "namespace not available in snapshotter") } return fn(ctx, bkt.Bucket(bucketKeySnapshot), bkt.Bucket(bucketKeyParents)) @@ -409,7 +410,7 @@ func parents(bkt *bolt.Bucket, parent *db.Snapshot) (parents []string, err error func getSnapshot(bkt *bolt.Bucket, key string, ss *db.Snapshot) error { b := bkt.Get([]byte(key)) if len(b) == 0 { - return snapshot.ErrSnapshotNotExist + return errors.Wrapf(errdefs.ErrNotFound, "snapshot %v", key) } if err := proto.Unmarshal(b, ss); err != nil { return errors.Wrap(err, "failed to unmarshal snapshot") diff --git a/snapshot/storage/metastore_test.go b/snapshot/storage/metastore_test.go index ca800db45..6b74bb206 100644 --- a/snapshot/storage/metastore_test.go +++ b/snapshot/storage/metastore_test.go @@ -6,6 +6,7 @@ import ( "os" "testing" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/namespaces" "github.com/containerd/containerd/snapshot" "github.com/pkg/errors" @@ -205,7 +206,7 @@ func assertNotExist(t *testing.T, err error) { if err == nil { t.Fatal("Expected not exist error") } - if !snapshot.IsNotExist(err) { + if !errdefs.IsNotFound(err) { t.Fatalf("Expected not exist error, got %+v", err) } } @@ -214,7 +215,7 @@ func assertNotActive(t *testing.T, err error) { if err == nil { t.Fatal("Expected not active error") } - if !snapshot.IsNotActive(err) { + if !errdefs.IsFailedPrecondition(err) { t.Fatalf("Expected not active error, got %+v", err) } } @@ -223,7 +224,7 @@ func assertNotCommitted(t *testing.T, err error) { if err == nil { t.Fatal("Expected active error") } - if !snapshot.IsNotCommitted(err) { + if !errdefs.IsInvalidArgument(err) { t.Fatalf("Expected active error, got %+v", err) } } @@ -232,7 +233,7 @@ func assertExist(t *testing.T, err error) { if err == nil { t.Fatal("Expected exist error") } - if !snapshot.IsExist(err) { + if !errdefs.IsAlreadyExists(err) { t.Fatalf("Expected exist error, got %+v", err) } }