Skip to content

Commit 05984a9

Browse files
estespestesp
authored
Merge pull request #2642 from dmcgowan/fix-commit-already-exists
Fix content store bug when already exists
2 parents c95bb88 + 6875d3d commit 05984a9

File tree

8 files changed

+61
-23
lines changed

8 files changed

+61
-23
lines changed

‎content/local/store_test.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535

3636
"github.com/containerd/containerd/content"
3737
"github.com/containerd/containerd/content/testsuite"
38+
"github.com/containerd/containerd/errdefs"
3839
"github.com/containerd/containerd/pkg/testutil"
3940

4041
"github.com/opencontainers/go-digest"
@@ -174,7 +175,9 @@ func TestContentWriter(t *testing.T) {
174175

175176
// now, attempt to write the same data again
176177
checkCopy(t, int64(len(p)), cw, bufio.NewReader(ioutil.NopCloser(bytes.NewReader(p))))
177-
if err := cw.Commit(ctx, int64(len(p)), expected); err != nil {
178+
if err := cw.Commit(ctx, int64(len(p)), expected); err == nil {
179+
t.Fatal("expected already exists error")
180+
} else if !errdefs.IsAlreadyExists(err) {
178181
t.Fatal(err)
179182
}
180183

‎content/local/writer.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,11 @@ func (w *writer) Commit(ctx context.Context, size int64, expected digest.Digest,
132132
// clean up!!
133133
defer os.RemoveAll(w.path)
134134

135+
if _, err := os.Stat(target); err == nil {
136+
// collision with the target file!
137+
return errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", dgst)
138+
}
135139
if err := os.Rename(ingest, target); err != nil {
136-
if os.IsExist(err) {
137-
// collision with the target file!
138-
return errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", dgst)
139-
}
140140
return err
141141
}
142142
commitTime := time.Now()

‎content/testsuite/testsuite.go

+36-1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"time"
3131

3232
"github.com/containerd/containerd/content"
33+
"github.com/containerd/containerd/errdefs"
3334
"github.com/containerd/containerd/pkg/testutil"
3435
digest "github.com/opencontainers/go-digest"
3536
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -41,6 +42,7 @@ import (
4142
func ContentSuite(t *testing.T, name string, storeFn func(ctx context.Context, root string) (context.Context, content.Store, func() error, error)) {
4243
t.Run("Writer", makeTest(t, name, storeFn, checkContentStoreWriter))
4344
t.Run("UpdateStatus", makeTest(t, name, storeFn, checkUpdateStatus))
45+
t.Run("CommitExists", makeTest(t, name, storeFn, checkCommitExists))
4446
t.Run("Resume", makeTest(t, name, storeFn, checkResumeWriter))
4547
t.Run("ResumeTruncate", makeTest(t, name, storeFn, checkResume(resumeTruncate)))
4648
t.Run("ResumeDiscard", makeTest(t, name, storeFn, checkResume(resumeDiscard)))
@@ -281,6 +283,39 @@ func checkResumeWriter(ctx context.Context, t *testing.T, cs content.Store) {
281283
}
282284
}
283285

286+
func checkCommitExists(ctx context.Context, t *testing.T, cs content.Store) {
287+
c1, d1 := createContent(256)
288+
if err := content.WriteBlob(ctx, cs, "c1", bytes.NewReader(c1), ocispec.Descriptor{Digest: d1}); err != nil {
289+
t.Fatal(err)
290+
}
291+
292+
for i, tc := range []struct {
293+
expected digest.Digest
294+
}{
295+
{
296+
expected: d1,
297+
},
298+
{},
299+
} {
300+
w, err := cs.Writer(ctx, content.WithRef(fmt.Sprintf("c1-commitexists-%d", i)))
301+
if err != nil {
302+
t.Fatal(err)
303+
}
304+
if _, err := w.Write(c1); err != nil {
305+
w.Close()
306+
t.Fatal(err)
307+
}
308+
err = w.Commit(ctx, int64(len(c1)), tc.expected)
309+
w.Close()
310+
if err == nil {
311+
t.Errorf("(%d) Expected already exists error", i)
312+
} else if !errdefs.IsAlreadyExists(err) {
313+
t.Fatalf("(%d) Unexpected error: %+v", i, err)
314+
}
315+
316+
}
317+
}
318+
284319
func checkUpdateStatus(ctx context.Context, t *testing.T, cs content.Store) {
285320
c1, d1 := createContent(256)
286321

@@ -353,7 +388,7 @@ func checkUpdateStatus(ctx context.Context, t *testing.T, cs content.Store) {
353388
func checkLabels(ctx context.Context, t *testing.T, cs content.Store) {
354389
c1, d1 := createContent(256)
355390

356-
w1, err := cs.Writer(ctx, content.WithRef("c1"), content.WithDescriptor(ocispec.Descriptor{Size: 256, Digest: d1}))
391+
w1, err := cs.Writer(ctx, content.WithRef("c1-checklabels"), content.WithDescriptor(ocispec.Descriptor{Size: 256, Digest: d1}))
357392
if err != nil {
358393
t.Fatal(err)
359394
}

‎diff/walking/differ.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,9 @@ func (s *walkingDiff) Compare(ctx context.Context, lower, upper []mount.Mount, o
139139

140140
dgst := cw.Digest()
141141
if err := cw.Commit(ctx, 0, dgst, commitopts...); err != nil {
142-
return errors.Wrap(err, "failed to commit")
142+
if !errdefs.IsAlreadyExists(err) {
143+
return errors.Wrap(err, "failed to commit")
144+
}
143145
}
144146

145147
info, err := s.store.Info(ctx, dgst)

‎metadata/buckets.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -164,11 +164,11 @@ func getSnapshotterBucket(tx *bolt.Tx, namespace, snapshotter string) *bolt.Buck
164164
}
165165

166166
func createBlobBucket(tx *bolt.Tx, namespace string, dgst digest.Digest) (*bolt.Bucket, error) {
167-
bkt, err := createBucketIfNotExists(tx, bucketKeyVersion, []byte(namespace), bucketKeyObjectContent, bucketKeyObjectBlob, []byte(dgst.String()))
167+
bkt, err := createBucketIfNotExists(tx, bucketKeyVersion, []byte(namespace), bucketKeyObjectContent, bucketKeyObjectBlob)
168168
if err != nil {
169169
return nil, err
170170
}
171-
return bkt, nil
171+
return bkt.CreateBucket([]byte(dgst.String()))
172172
}
173173

174174
func getBlobsBucket(tx *bolt.Tx, namespace string) *bolt.Bucket {

‎metadata/content.go

+5-10
Original file line numberDiff line numberDiff line change
@@ -592,9 +592,6 @@ func (nw *namespacedWriter) commit(ctx context.Context, tx *bolt.Tx, size int64,
592592
}
593593
size = nw.desc.Size
594594
actual = nw.desc.Digest
595-
if getBlobBucket(tx, nw.namespace, actual) != nil {
596-
return "", errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", actual)
597-
}
598595
} else {
599596
status, err := nw.w.Status()
600597
if err != nil {
@@ -606,18 +603,16 @@ func (nw *namespacedWriter) commit(ctx context.Context, tx *bolt.Tx, size int64,
606603
size = status.Offset
607604
actual = nw.w.Digest()
608605

609-
if err := nw.w.Commit(ctx, size, expected); err != nil {
610-
if !errdefs.IsAlreadyExists(err) {
611-
return "", err
612-
}
613-
if getBlobBucket(tx, nw.namespace, actual) != nil {
614-
return "", errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", actual)
615-
}
606+
if err := nw.w.Commit(ctx, size, expected); err != nil && !errdefs.IsAlreadyExists(err) {
607+
return "", err
616608
}
617609
}
618610

619611
bkt, err := createBlobBucket(tx, nw.namespace, actual)
620612
if err != nil {
613+
if err == bolt.ErrBucketExists {
614+
return "", errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", actual)
615+
}
621616
return "", err
622617
}
623618

‎services/content/service.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ func (s *service) Read(req *api.ReadContentRequest, session api.Content_ReadServ
213213
_, err = io.CopyBuffer(
214214
&readResponseWriter{session: session},
215215
io.NewSectionReader(ra, offset, size), *p)
216-
return err
216+
return errdefs.ToGRPC(err)
217217
}
218218

219219
// readResponseWriter is a writer that places the output into ReadContentRequest messages.
@@ -420,7 +420,7 @@ func (s *service) Write(session api.Content_WriteServer) (err error) {
420420
// maintain the offset as append only, we just issue the write.
421421
n, err := wr.Write(req.Data)
422422
if err != nil {
423-
return err
423+
return errdefs.ToGRPC(err)
424424
}
425425

426426
if n != len(req.Data) {
@@ -438,7 +438,7 @@ func (s *service) Write(session api.Content_WriteServer) (err error) {
438438
opts = append(opts, content.WithLabels(req.Labels))
439439
}
440440
if err := wr.Commit(ctx, total, expected, opts...); err != nil {
441-
return err
441+
return errdefs.ToGRPC(err)
442442
}
443443
}
444444

‎task.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -607,8 +607,11 @@ func writeContent(ctx context.Context, store content.Ingester, mediaType, ref st
607607
if err != nil {
608608
return d, err
609609
}
610+
610611
if err := writer.Commit(ctx, size, "", opts...); err != nil {
611-
return d, err
612+
if !errdefs.IsAlreadyExists(err) {
613+
return d, err
614+
}
612615
}
613616
return v1.Descriptor{
614617
MediaType: mediaType,

0 commit comments

Comments
 (0)