Skip to content

Commit cb15809

Browse files
author
Kazuyoshi Kato
committed
metadata: improve deleting a non-empty namespace's error message
Deleting a non-empty namespace fails with > namespace must be empty: failed precondition This change improves the error message by listing the types of the objects in the namespace that prevent deletion. Signed-off-by: Kazuyoshi Kato <[email protected]>
1 parent dda530a commit cb15809

2 files changed

Lines changed: 115 additions & 19 deletions

File tree

‎metadata/namespaces.go‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package metadata
1818

1919
import (
2020
"context"
21+
"strings"
2122

2223
"github.com/containerd/containerd/errdefs"
2324
"github.com/containerd/containerd/identifiers"
@@ -140,10 +141,17 @@ func (s *namespaceStore) Delete(ctx context.Context, namespace string, opts ...n
140141
}
141142
}
142143
bkt := getBucket(s.tx, bucketKeyVersion)
143-
if empty, err := s.namespaceEmpty(ctx, namespace); err != nil {
144+
types, err := s.listNs(namespace)
145+
if err != nil {
144146
return err
145-
} else if !empty {
146-
return errors.Wrapf(errdefs.ErrFailedPrecondition, "namespace %q must be empty", namespace)
147+
}
148+
149+
if len(types) > 0 {
150+
return errors.Wrapf(
151+
errdefs.ErrFailedPrecondition,
152+
"namespace %q must be empty, but it still has %s",
153+
namespace, strings.Join(types, ", "),
154+
)
147155
}
148156

149157
if err := bkt.DeleteBucket([]byte(namespace)); err != nil {
@@ -157,32 +165,35 @@ func (s *namespaceStore) Delete(ctx context.Context, namespace string, opts ...n
157165
return nil
158166
}
159167

160-
func (s *namespaceStore) namespaceEmpty(ctx context.Context, namespace string) (bool, error) {
161-
// Get all data buckets
162-
buckets := []*bolt.Bucket{
163-
getImagesBucket(s.tx, namespace),
164-
getBlobsBucket(s.tx, namespace),
165-
getContainersBucket(s.tx, namespace),
168+
// listNs returns the types of the remaining objects inside the given namespace.
169+
// It doesn't return exact objects due to performance concerns.
170+
func (s *namespaceStore) listNs(namespace string) ([]string, error) {
171+
var out []string
172+
173+
if !isBucketEmpty(getImagesBucket(s.tx, namespace)) {
174+
out = append(out, "images")
175+
}
176+
if !isBucketEmpty(getBlobsBucket(s.tx, namespace)) {
177+
out = append(out, "blobs")
166178
}
179+
if !isBucketEmpty(getContainersBucket(s.tx, namespace)) {
180+
out = append(out, "containers")
181+
}
182+
167183
if snbkt := getSnapshottersBucket(s.tx, namespace); snbkt != nil {
168184
if err := snbkt.ForEach(func(k, v []byte) error {
169185
if v == nil {
170-
buckets = append(buckets, snbkt.Bucket(k))
186+
if !isBucketEmpty(snbkt.Bucket(k)) {
187+
out = append(out, "snapshot-"+string(k))
188+
}
171189
}
172190
return nil
173191
}); err != nil {
174-
return false, err
175-
}
176-
}
177-
178-
// Ensure data buckets are empty
179-
for _, bkt := range buckets {
180-
if !isBucketEmpty(bkt) {
181-
return false, nil
192+
return nil, err
182193
}
183194
}
184195

185-
return true, nil
196+
return out, nil
186197
}
187198

188199
func isBucketEmpty(bkt *bolt.Bucket) bool {

‎metadata/namespaces_test.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
Copyright The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package metadata
18+
19+
import (
20+
"context"
21+
"testing"
22+
23+
"github.com/containerd/containerd/containers"
24+
"github.com/containerd/containerd/namespaces"
25+
"github.com/gogo/protobuf/types"
26+
"github.com/stretchr/testify/assert"
27+
"github.com/stretchr/testify/require"
28+
"go.etcd.io/bbolt"
29+
)
30+
31+
func TestCreateDelete(t *testing.T) {
32+
ctx, db, cleanup := testDB(t)
33+
defer cleanup()
34+
35+
subtests := []struct {
36+
name string
37+
create func(t *testing.T, ctx context.Context)
38+
validate func(t *testing.T, err error)
39+
}{
40+
{
41+
name: "empty",
42+
create: func(t *testing.T, ctx context.Context) {},
43+
validate: func(t *testing.T, err error) {
44+
require.NoError(t, err)
45+
},
46+
},
47+
{
48+
name: "not-empty",
49+
create: func(t *testing.T, ctx context.Context) {
50+
store := NewContainerStore(db)
51+
_, err := store.Create(ctx, containers.Container{
52+
ID: "c1",
53+
Runtime: containers.RuntimeInfo{Name: "rt"},
54+
Spec: &types.Any{},
55+
})
56+
require.NoError(t, err)
57+
},
58+
validate: func(t *testing.T, err error) {
59+
require.Error(t, err)
60+
assert.Contains(t, err.Error(), "still has containers")
61+
},
62+
},
63+
}
64+
65+
for _, subtest := range subtests {
66+
ns := subtest.name
67+
ctx = namespaces.WithNamespace(ctx, ns)
68+
69+
t.Run(subtest.name, func(t *testing.T) {
70+
err := db.Update(func(tx *bbolt.Tx) error {
71+
store := NewNamespaceStore(tx)
72+
return store.Create(ctx, ns, nil)
73+
})
74+
require.NoError(t, err)
75+
76+
subtest.create(t, ctx)
77+
78+
err = db.Update(func(tx *bbolt.Tx) error {
79+
store := NewNamespaceStore(tx)
80+
return store.Delete(ctx, ns)
81+
})
82+
subtest.validate(t, err)
83+
})
84+
}
85+
}

0 commit comments

Comments
 (0)