Update lease service errors
Ensure delete returns a typed error on not found Signed-off-by: Derek McGowan <derek@mcgstyle.net>
This commit is contained in:
		| @@ -20,6 +20,7 @@ import ( | |||||||
| 	"context" | 	"context" | ||||||
|  |  | ||||||
| 	leasesapi "github.com/containerd/containerd/api/services/leases/v1" | 	leasesapi "github.com/containerd/containerd/api/services/leases/v1" | ||||||
|  | 	"github.com/containerd/containerd/errdefs" | ||||||
| 	"github.com/containerd/containerd/leases" | 	"github.com/containerd/containerd/leases" | ||||||
| ) | ) | ||||||
|  |  | ||||||
| @@ -47,7 +48,7 @@ func (pm *proxyManager) Create(ctx context.Context, opts ...leases.Opt) (leases. | |||||||
| 		Labels: l.Labels, | 		Labels: l.Labels, | ||||||
| 	}) | 	}) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return leases.Lease{}, err | 		return leases.Lease{}, errdefs.FromGRPC(err) | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	return leases.Lease{ | 	return leases.Lease{ | ||||||
| @@ -69,7 +70,7 @@ func (pm *proxyManager) Delete(ctx context.Context, l leases.Lease, opts ...leas | |||||||
| 		ID:   l.ID, | 		ID:   l.ID, | ||||||
| 		Sync: do.Synchronous, | 		Sync: do.Synchronous, | ||||||
| 	}) | 	}) | ||||||
| 	return err | 	return errdefs.FromGRPC(err) | ||||||
| } | } | ||||||
|  |  | ||||||
| func (pm *proxyManager) List(ctx context.Context, filters ...string) ([]leases.Lease, error) { | func (pm *proxyManager) List(ctx context.Context, filters ...string) ([]leases.Lease, error) { | ||||||
| @@ -77,7 +78,7 @@ func (pm *proxyManager) List(ctx context.Context, filters ...string) ([]leases.L | |||||||
| 		Filters: filters, | 		Filters: filters, | ||||||
| 	}) | 	}) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return nil, err | 		return nil, errdefs.FromGRPC(err) | ||||||
| 	} | 	} | ||||||
| 	l := make([]leases.Lease, len(resp.Leases)) | 	l := make([]leases.Lease, len(resp.Leases)) | ||||||
| 	for i := range resp.Leases { | 	for i := range resp.Leases { | ||||||
|   | |||||||
| @@ -102,9 +102,12 @@ func (lm *LeaseManager) Delete(ctx context.Context, lease leases.Lease, _ ...lea | |||||||
|  |  | ||||||
| 	topbkt := getBucket(lm.tx, bucketKeyVersion, []byte(namespace), bucketKeyObjectLeases) | 	topbkt := getBucket(lm.tx, bucketKeyVersion, []byte(namespace), bucketKeyObjectLeases) | ||||||
| 	if topbkt == nil { | 	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 err | ||||||
| 	} | 	} | ||||||
| 	return nil | 	return nil | ||||||
|   | |||||||
| @@ -30,15 +30,17 @@ func TestLeases(t *testing.T) { | |||||||
| 	defer cancel() | 	defer cancel() | ||||||
|  |  | ||||||
| 	testCases := []struct { | 	testCases := []struct { | ||||||
| 		ID    string | 		ID        string | ||||||
| 		Cause error | 		CreateErr error | ||||||
|  | 		DeleteErr error | ||||||
| 	}{ | 	}{ | ||||||
| 		{ | 		{ | ||||||
| 			ID: "tx1", | 			ID: "tx1", | ||||||
| 		}, | 		}, | ||||||
| 		{ | 		{ | ||||||
| 			ID:    "tx1", | 			ID:        "tx1", | ||||||
| 			Cause: errdefs.ErrAlreadyExists, | 			CreateErr: errdefs.ErrAlreadyExists, | ||||||
|  | 			DeleteErr: errdefs.ErrNotFound, | ||||||
| 		}, | 		}, | ||||||
| 		{ | 		{ | ||||||
| 			ID: "tx2", | 			ID: "tx2", | ||||||
| @@ -51,7 +53,7 @@ func TestLeases(t *testing.T) { | |||||||
| 		if err := db.Update(func(tx *bolt.Tx) error { | 		if err := db.Update(func(tx *bolt.Tx) error { | ||||||
| 			lease, err := NewLeaseManager(tx).Create(ctx, leases.WithID(tc.ID)) | 			lease, err := NewLeaseManager(tx).Create(ctx, leases.WithID(tc.ID)) | ||||||
| 			if err != nil { | 			if err != nil { | ||||||
| 				if tc.Cause != nil && errors.Cause(err) == tc.Cause { | 				if tc.CreateErr != nil && errors.Cause(err) == tc.CreateErr { | ||||||
| 					return nil | 					return nil | ||||||
| 				} | 				} | ||||||
| 				return err | 				return err | ||||||
| @@ -91,7 +93,10 @@ func TestLeases(t *testing.T) { | |||||||
| 				ID: tc.ID, | 				ID: tc.ID, | ||||||
| 			}) | 			}) | ||||||
| 		}); err != nil { | 		}); 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) | 			t.Fatal(err) | ||||||
| 		} | 		} | ||||||
|  |  | ||||||
| 		// try it again, get nil | 		// try it again, get not found | ||||||
| 		if err := db.Update(func(tx *bolt.Tx) error { | 		if err := db.Update(func(tx *bolt.Tx) error { | ||||||
| 			lm := NewLeaseManager(tx) | 			lm := NewLeaseManager(tx) | ||||||
| 			return lm.Delete(ctx, lease) | 			return lm.Delete(ctx, lease) | ||||||
| 		}); err != nil { | 		}); err == nil { | ||||||
| 			t.Fatalf("unexpected error %v", err) | 			t.Fatalf("expected error deleting non-existent lease") | ||||||
|  | 		} else if !errdefs.IsNotFound(err) { | ||||||
|  | 			t.Fatalf("unexpected error: %s", err) | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
|   | |||||||
| @@ -22,6 +22,7 @@ import ( | |||||||
| 	"google.golang.org/grpc" | 	"google.golang.org/grpc" | ||||||
|  |  | ||||||
| 	api "github.com/containerd/containerd/api/services/leases/v1" | 	api "github.com/containerd/containerd/api/services/leases/v1" | ||||||
|  | 	"github.com/containerd/containerd/errdefs" | ||||||
| 	"github.com/containerd/containerd/leases" | 	"github.com/containerd/containerd/leases" | ||||||
| 	"github.com/containerd/containerd/plugin" | 	"github.com/containerd/containerd/plugin" | ||||||
| 	"github.com/containerd/containerd/services" | 	"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...) | 	l, err := s.lm.Create(ctx, opts...) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return nil, err | 		return nil, errdefs.ToGRPC(err) | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	return &api.CreateResponse{ | 	return &api.CreateResponse{ | ||||||
| @@ -91,7 +92,7 @@ func (s *service) Delete(ctx context.Context, r *api.DeleteRequest) (*ptypes.Emp | |||||||
| 	if err := s.lm.Delete(ctx, leases.Lease{ | 	if err := s.lm.Delete(ctx, leases.Lease{ | ||||||
| 		ID: r.ID, | 		ID: r.ID, | ||||||
| 	}, opts...); err != nil { | 	}, opts...); err != nil { | ||||||
| 		return nil, err | 		return nil, errdefs.ToGRPC(err) | ||||||
| 	} | 	} | ||||||
| 	return &ptypes.Empty{}, nil | 	return &ptypes.Empty{}, nil | ||||||
| } | } | ||||||
| @@ -99,7 +100,7 @@ func (s *service) Delete(ctx context.Context, r *api.DeleteRequest) (*ptypes.Emp | |||||||
| func (s *service) List(ctx context.Context, r *api.ListRequest) (*api.ListResponse, error) { | func (s *service) List(ctx context.Context, r *api.ListRequest) (*api.ListResponse, error) { | ||||||
| 	l, err := s.lm.List(ctx, r.Filters...) | 	l, err := s.lm.List(ctx, r.Filters...) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return nil, err | 		return nil, errdefs.ToGRPC(err) | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	apileases := make([]*api.Lease, len(l)) | 	apileases := make([]*api.Lease, len(l)) | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Derek McGowan
					Derek McGowan