Skip to content

Commit 4249f44

Browse files
estespestesp
authored
Merge pull request #2493 from dmcgowan/sync-lease-removal
Add sync option to lease removal
2 parents d0ab8c8 + b760cee commit 4249f44

10 files changed

Lines changed: 196 additions & 60 deletions

File tree

‎api/next.pb.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2563,6 +2563,13 @@ file {
25632563
type: TYPE_STRING
25642564
json_name: "id"
25652565
}
2566+
field {
2567+
name: "sync"
2568+
number: 2
2569+
label: LABEL_OPTIONAL
2570+
type: TYPE_BOOL
2571+
json_name: "sync"
2572+
}
25662573
}
25672574
message_type {
25682575
name: "ListRequest"

‎api/services/leases/v1/leases.pb.go‎

Lines changed: 72 additions & 32 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/services/leases/v1/leases.proto‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,12 @@ message CreateResponse {
4747

4848
message DeleteRequest {
4949
string id = 1;
50+
51+
// Sync indicates that the delete and cleanup should be done
52+
// synchronously before returning to the caller
53+
//
54+
// Default is false
55+
bool sync = 2;
5056
}
5157

5258
message ListRequest {

‎cmd/ctr/commands/leases/leases.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,12 @@ var deleteCommand = cli.Command{
163163
Usage: "delete a lease",
164164
ArgsUsage: "[flags] <lease id> ...",
165165
Description: "delete a lease",
166+
Flags: []cli.Flag{
167+
cli.BoolFlag{
168+
Name: "sync",
169+
Usage: "Synchronously remove leases and all unreferenced resources",
170+
},
171+
},
166172
Action: func(context *cli.Context) error {
167173
var lids = context.Args()
168174
if len(lids) == 0 {
@@ -175,11 +181,17 @@ var deleteCommand = cli.Command{
175181
defer cancel()
176182

177183
ls := client.LeasesService()
178-
for _, lid := range lids {
184+
sync := context.Bool("sync")
185+
for i, lid := range lids {
186+
var opts []leases.DeleteOpt
187+
if sync && i == len(lids)-1 {
188+
opts = append(opts, leases.SynchronousDelete)
189+
}
190+
179191
lease := leases.Lease{
180192
ID: lid,
181193
}
182-
if err := ls.Delete(ctx, lease); err != nil {
194+
if err := ls.Delete(ctx, lease, opts...); err != nil {
183195
return err
184196
}
185197
fmt.Println(lid)

‎leases/lease.go‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,13 @@ import (
2424
// Opt is used to set options on a lease
2525
type Opt func(*Lease) error
2626

27+
// DeleteOpt allows configuring a delete operation
28+
type DeleteOpt func(context.Context, *DeleteOptions) error
29+
2730
// Manager is used to create, list, and remove leases
2831
type Manager interface {
2932
Create(context.Context, ...Opt) (Lease, error)
30-
Delete(context.Context, Lease) error
33+
Delete(context.Context, Lease, ...DeleteOpt) error
3134
List(context.Context, ...string) ([]Lease, error)
3235
}
3336

@@ -39,6 +42,19 @@ type Lease struct {
3942
Labels map[string]string
4043
}
4144

45+
// DeleteOptions provide options on image delete
46+
type DeleteOptions struct {
47+
Synchronous bool
48+
}
49+
50+
// SynchronousDelete is used to indicate that a lease deletion and removal of
51+
// any unreferenced resources should occur synchronously before returning the
52+
// result.
53+
func SynchronousDelete(ctx context.Context, o *DeleteOptions) error {
54+
o.Synchronous = true
55+
return nil
56+
}
57+
4258
// WithLabels sets labels on a lease
4359
func WithLabels(labels map[string]string) Opt {
4460
return func(l *Lease) error {

‎leases/proxy/manager.go‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121

2222
leasesapi "github.com/containerd/containerd/api/services/leases/v1"
23+
"github.com/containerd/containerd/errdefs"
2324
"github.com/containerd/containerd/leases"
2425
)
2526

@@ -47,7 +48,7 @@ func (pm *proxyManager) Create(ctx context.Context, opts ...leases.Opt) (leases.
4748
Labels: l.Labels,
4849
})
4950
if err != nil {
50-
return leases.Lease{}, err
51+
return leases.Lease{}, errdefs.FromGRPC(err)
5152
}
5253

5354
return leases.Lease{
@@ -57,19 +58,27 @@ func (pm *proxyManager) Create(ctx context.Context, opts ...leases.Opt) (leases.
5758
}, nil
5859
}
5960

60-
func (pm *proxyManager) Delete(ctx context.Context, l leases.Lease) error {
61+
func (pm *proxyManager) Delete(ctx context.Context, l leases.Lease, opts ...leases.DeleteOpt) error {
62+
var do leases.DeleteOptions
63+
for _, opt := range opts {
64+
if err := opt(ctx, &do); err != nil {
65+
return err
66+
}
67+
}
68+
6169
_, err := pm.client.Delete(ctx, &leasesapi.DeleteRequest{
62-
ID: l.ID,
70+
ID: l.ID,
71+
Sync: do.Synchronous,
6372
})
64-
return err
73+
return errdefs.FromGRPC(err)
6574
}
6675

6776
func (pm *proxyManager) List(ctx context.Context, filters ...string) ([]leases.Lease, error) {
6877
resp, err := pm.client.List(ctx, &leasesapi.ListRequest{
6978
Filters: filters,
7079
})
7180
if err != nil {
72-
return nil, err
81+
return nil, errdefs.FromGRPC(err)
7382
}
7483
l := make([]leases.Lease, len(resp.Leases))
7584
for i := range resp.Leases {

‎metadata/leases.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,17 +94,20 @@ func (lm *LeaseManager) Create(ctx context.Context, opts ...leases.Opt) (leases.
9494
}
9595

9696
// Delete delets the lease with the provided lease ID
97-
func (lm *LeaseManager) Delete(ctx context.Context, lease leases.Lease) error {
97+
func (lm *LeaseManager) Delete(ctx context.Context, lease leases.Lease, _ ...leases.DeleteOpt) error {
9898
namespace, err := namespaces.NamespaceRequired(ctx)
9999
if err != nil {
100100
return err
101101
}
102102

103103
topbkt := getBucket(lm.tx, bucketKeyVersion, []byte(namespace), bucketKeyObjectLeases)
104104
if topbkt == nil {
105-
return nil
105+
return errors.Wrapf(errdefs.ErrNotFound, "lease %q", lease.ID)
106106
}
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+
}
108111
return err
109112
}
110113
return nil

‎metadata/leases_test.go‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,17 @@ func TestLeases(t *testing.T) {
3030
defer cancel()
3131

3232
testCases := []struct {
33-
ID string
34-
Cause error
33+
ID string
34+
CreateErr error
35+
DeleteErr error
3536
}{
3637
{
3738
ID: "tx1",
3839
},
3940
{
40-
ID: "tx1",
41-
Cause: errdefs.ErrAlreadyExists,
41+
ID: "tx1",
42+
CreateErr: errdefs.ErrAlreadyExists,
43+
DeleteErr: errdefs.ErrNotFound,
4244
},
4345
{
4446
ID: "tx2",
@@ -51,7 +53,7 @@ func TestLeases(t *testing.T) {
5153
if err := db.Update(func(tx *bolt.Tx) error {
5254
lease, err := NewLeaseManager(tx).Create(ctx, leases.WithID(tc.ID))
5355
if err != nil {
54-
if tc.Cause != nil && errors.Cause(err) == tc.Cause {
56+
if tc.CreateErr != nil && errors.Cause(err) == tc.CreateErr {
5557
return nil
5658
}
5759
return err
@@ -91,7 +93,10 @@ func TestLeases(t *testing.T) {
9193
ID: tc.ID,
9294
})
9395
}); err != nil {
94-
t.Fatal(err)
96+
if tc.DeleteErr == nil && errors.Cause(err) != tc.DeleteErr {
97+
t.Fatal(err)
98+
}
99+
95100
}
96101
}
97102

@@ -248,12 +253,14 @@ func TestLeasesList(t *testing.T) {
248253
t.Fatal(err)
249254
}
250255

251-
// try it again, get nil
256+
// try it again, get not found
252257
if err := db.Update(func(tx *bolt.Tx) error {
253258
lm := NewLeaseManager(tx)
254259
return lm.Delete(ctx, lease)
255-
}); err != nil {
256-
t.Fatalf("unexpected error %v", err)
260+
}); err == nil {
261+
t.Fatalf("expected error deleting non-existent lease")
262+
} else if !errdefs.IsNotFound(err) {
263+
t.Fatalf("unexpected error: %s", err)
257264
}
258265
}
259266
}

0 commit comments

Comments
 (0)