Update lease service errors

dmcgowan · dmcgowan · commit b760cee65a21 · 2018-07-25T14:56:35.000-07:00
Ensure delete returns a typed error on not found

Signed-off-by: Derek McGowan &lt;derek@mcgstyle.net&gt;
diff --git a/leases/proxy/manager.go b/leases/proxy/manager.go
@@ -20,6 +20,7 @@ import (
 	"context"
 
 	leasesapi "github.com/containerd/containerd/api/services/leases/v1"
+	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/leases"
 )
 
@@ -47,7 +48,7 @@ func (pm *proxyManager) Create(ctx context.Context, opts ...leases.Opt) (leases.
 		Labels: l.Labels,
 	})
 	if err != nil {
-		return leases.Lease{}, err
+		return leases.Lease{}, errdefs.FromGRPC(err)
 	}
 
 	return leases.Lease{
@@ -69,15 +70,15 @@ func (pm *proxyManager) Delete(ctx context.Context, l leases.Lease, opts ...leas
 		ID:   l.ID,
 		Sync: do.Synchronous,
 	})
-	return err
+	return errdefs.FromGRPC(err)
 }
 
 func (pm *proxyManager) List(ctx context.Context, filters ...string) ([]leases.Lease, error) {
 	resp, err := pm.client.List(ctx, &leasesapi.ListRequest{
 		Filters: filters,
 	})
 	if err != nil {
-		return nil, err
+		return nil, errdefs.FromGRPC(err)
 	}
 	l := make([]leases.Lease, len(resp.Leases))
 	for i := range resp.Leases {
diff --git a/metadata/leases.go b/metadata/leases.go
@@ -102,9 +102,12 @@ func (lm *LeaseManager) Delete(ctx context.Context, lease leases.Lease, _ ...lea
 
 	topbkt := getBucket(lm.tx, bucketKeyVersion, []byte(namespace), bucketKeyObjectLeases)
 	if topbkt == nil {
-		return nil
+		return errors.Wrapf(errdefs.ErrNotFound, "lease %q", lease.ID)
 	}
-	if err := topbkt.DeleteBucket([]byte(lease.ID)); err != nil && err != bolt.ErrBucketNotFound {
+	if err := topbkt.DeleteBucket([]byte(lease.ID)); err != nil {
+		if err == bolt.ErrBucketNotFound {
+			err = errors.Wrapf(errdefs.ErrNotFound, "lease %q", lease.ID)
+		}
 		return err
 	}
 	return nil
diff --git a/metadata/leases_test.go b/metadata/leases_test.go
@@ -30,15 +30,17 @@ func TestLeases(t *testing.T) {
 	defer cancel()
 
 	testCases := []struct {
-		ID    string
-		Cause error
+		ID        string
+		CreateErr error
+		DeleteErr error
 	}{
 		{
 			ID: "tx1",
 		},
 		{
-			ID:    "tx1",
-			Cause: errdefs.ErrAlreadyExists,
+			ID:        "tx1",
+			CreateErr: errdefs.ErrAlreadyExists,
+			DeleteErr: errdefs.ErrNotFound,
 		},
 		{
 			ID: "tx2",
@@ -51,7 +53,7 @@ func TestLeases(t *testing.T) {
 		if err := db.Update(func(tx *bolt.Tx) error {
 			lease, err := NewLeaseManager(tx).Create(ctx, leases.WithID(tc.ID))
 			if err != nil {
-				if tc.Cause != nil && errors.Cause(err) == tc.Cause {
+				if tc.CreateErr != nil && errors.Cause(err) == tc.CreateErr {
 					return nil
 				}
 				return err
@@ -91,7 +93,10 @@ func TestLeases(t *testing.T) {
 				ID: tc.ID,
 			})
 		}); err != nil {
-			t.Fatal(err)
+			if tc.DeleteErr == nil && errors.Cause(err) != tc.DeleteErr {
+				t.Fatal(err)
+			}
+
 		}
 	}
 
@@ -248,12 +253,14 @@ func TestLeasesList(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		// try it again, get nil
+		// try it again, get not found
 		if err := db.Update(func(tx *bolt.Tx) error {
 			lm := NewLeaseManager(tx)
 			return lm.Delete(ctx, lease)
-		}); err != nil {
-			t.Fatalf("unexpected error %v", err)
+		}); err == nil {
+			t.Fatalf("expected error deleting non-existent lease")
+		} else if !errdefs.IsNotFound(err) {
+			t.Fatalf("unexpected error: %s", err)
 		}
 	}
 }
diff --git a/services/leases/service.go b/services/leases/service.go
@@ -22,6 +22,7 @@ import (
 	"google.golang.org/grpc"
 
 	api "github.com/containerd/containerd/api/services/leases/v1"
+	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/leases"
 	"github.com/containerd/containerd/plugin"
 	"github.com/containerd/containerd/services"
@@ -75,7 +76,7 @@ func (s *service) Create(ctx context.Context, r *api.CreateRequest) (*api.Create
 
 	l, err := s.lm.Create(ctx, opts...)
 	if err != nil {
-		return nil, err
+		return nil, errdefs.ToGRPC(err)
 	}
 
 	return &api.CreateResponse{
@@ -91,15 +92,15 @@ func (s *service) Delete(ctx context.Context, r *api.DeleteRequest) (*ptypes.Emp
 	if err := s.lm.Delete(ctx, leases.Lease{
 		ID: r.ID,
 	}, opts...); err != nil {
-		return nil, err
+		return nil, errdefs.ToGRPC(err)
 	}
 	return &ptypes.Empty{}, nil
 }
 
 func (s *service) List(ctx context.Context, r *api.ListRequest) (*api.ListResponse, error) {
 	l, err := s.lm.List(ctx, r.Filters...)
 	if err != nil {
-		return nil, err
+		return nil, errdefs.ToGRPC(err)
 	}
 
 	apileases := make([]*api.Lease, len(l))

Original file line number	Diff line number	Diff line change
`@@ -102,9 +102,12 @@ func (lm *LeaseManager) Delete(ctx context.Context, lease leases.Lease, _ ...lea`
`102`	`102`
`103`	`103`	`topbkt := getBucket(lm.tx, bucketKeyVersion, []byte(namespace), bucketKeyObjectLeases)`
`104`	`104`	`if topbkt == nil {`
`105`		`- return nil`
	`105`	`+ return errors.Wrapf(errdefs.ErrNotFound, "lease %q", lease.ID)`
`106`	`106`	`}`
`107`		`- if err := topbkt.DeleteBucket([]byte(lease.ID)); err != nil && err != bolt.ErrBucketNotFound {`
	`107`	`+ if err := topbkt.DeleteBucket([]byte(lease.ID)); err != nil {`
	`108`	`+ if err == bolt.ErrBucketNotFound {`
	`109`	`+ err = errors.Wrapf(errdefs.ErrNotFound, "lease %q", lease.ID)`
	`110`	`+ }`
`108`	`111`	`return err`
`109`	`112`	`}`
`110`	`113`	`return nil`