Skip to content

Commit 013c509

Browse files
crosbymichaelcrosbymichael
authored
Merge pull request #2654 from estesp/cherrypick-commit-fix
[release/1.1] Backported: check exists on content commit + testcase
2 parents 6f4c738 + 56f9c44 commit 013c509

8 files changed

Lines changed: 61 additions & 23 deletions

File tree

‎content/local/store_test.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636

3737
"github.com/containerd/containerd/content"
3838
"github.com/containerd/containerd/content/testsuite"
39+
"github.com/containerd/containerd/errdefs"
3940
"github.com/containerd/containerd/testutil"
4041
"github.com/gotestyourself/gotestyourself/assert"
4142
"github.com/opencontainers/go-digest"
@@ -173,7 +174,9 @@ func TestContentWriter(t *testing.T) {
173174

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

‎content/local/writer.go‎

Lines changed: 4 additions & 4 deletions
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‎

Lines changed: 36 additions & 1 deletion
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/testutil"
3435
"github.com/gotestyourself/gotestyourself/assert"
3536
digest "github.com/opencontainers/go-digest"
@@ -40,6 +41,7 @@ import (
4041
func ContentSuite(t *testing.T, name string, storeFn func(ctx context.Context, root string) (context.Context, content.Store, func() error, error)) {
4142
t.Run("Writer", makeTest(t, name, storeFn, checkContentStoreWriter))
4243
t.Run("UpdateStatus", makeTest(t, name, storeFn, checkUpdateStatus))
44+
t.Run("CommitExists", makeTest(t, name, storeFn, checkCommitExists))
4345
t.Run("Resume", makeTest(t, name, storeFn, checkResumeWriter))
4446
t.Run("ResumeTruncate", makeTest(t, name, storeFn, checkResume(resumeTruncate)))
4547
t.Run("ResumeDiscard", makeTest(t, name, storeFn, checkResume(resumeDiscard)))
@@ -280,6 +282,39 @@ func checkResumeWriter(ctx context.Context, t *testing.T, cs content.Store) {
280282
}
281283
}
282284

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

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

355-
w1, err := cs.Writer(ctx, "c1", 256, d1)
390+
w1, err := cs.Writer(ctx, "c1-checklabels", 256, d1)
356391
if err != nil {
357392
t.Fatal(err)
358393
}

‎diff/walking/differ.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,9 @@ func (s *walkingDiff) Compare(ctx context.Context, lower, upper []mount.Mount, o
135135

136136
dgst := cw.Digest()
137137
if err := cw.Commit(ctx, 0, dgst, commitopts...); err != nil {
138-
return errors.Wrap(err, "failed to commit")
138+
if !errdefs.IsAlreadyExists(err) {
139+
return errors.Wrap(err, "failed to commit")
140+
}
139141
}
140142

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

‎metadata/buckets.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,11 @@ func getSnapshotterBucket(tx *bolt.Tx, namespace, snapshotter string) *bolt.Buck
167167
}
168168

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

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

‎metadata/content.go‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -552,9 +552,6 @@ func (nw *namespacedWriter) commit(ctx context.Context, tx *bolt.Tx, size int64,
552552
}
553553
size = nw.size
554554
actual = nw.expected
555-
if getBlobBucket(tx, nw.namespace, actual) != nil {
556-
return "", errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", actual)
557-
}
558555
} else {
559556
status, err := nw.w.Status()
560557
if err != nil {
@@ -566,18 +563,16 @@ func (nw *namespacedWriter) commit(ctx context.Context, tx *bolt.Tx, size int64,
566563
size = status.Offset
567564
actual = nw.w.Digest()
568565

569-
if err := nw.w.Commit(ctx, size, expected); err != nil {
570-
if !errdefs.IsAlreadyExists(err) {
571-
return "", err
572-
}
573-
if getBlobBucket(tx, nw.namespace, actual) != nil {
574-
return "", errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", actual)
575-
}
566+
if err := nw.w.Commit(ctx, size, expected); err != nil && !errdefs.IsAlreadyExists(err) {
567+
return "", err
576568
}
577569
}
578570

579571
bkt, err := createBlobBucket(tx, nw.namespace, actual)
580572
if err != nil {
573+
if err == bolt.ErrBucketExists {
574+
return "", errors.Wrapf(errdefs.ErrAlreadyExists, "content %v", actual)
575+
}
581576
return "", err
582577
}
583578

‎services/content/service.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ func (s *service) Read(req *api.ReadContentRequest, session api.Content_ReadServ
212212
_, err = io.CopyBuffer(
213213
&readResponseWriter{session: session},
214214
io.NewSectionReader(ra, offset, size), *p)
215-
return err
215+
return errdefs.ToGRPC(err)
216216
}
217217

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

423423
if n != len(req.Data) {
@@ -435,7 +435,7 @@ func (s *service) Write(session api.Content_WriteServer) (err error) {
435435
opts = append(opts, content.WithLabels(req.Labels))
436436
}
437437
if err := wr.Commit(ctx, total, expected, opts...); err != nil {
438-
return err
438+
return errdefs.ToGRPC(err)
439439
}
440440
}
441441

‎task.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,8 +606,11 @@ func writeContent(ctx context.Context, store content.Ingester, mediaType, ref st
606606
if err != nil {
607607
return d, err
608608
}
609+
609610
if err := writer.Commit(ctx, size, "", opts...); err != nil {
610-
return d, err
611+
if !errdefs.IsAlreadyExists(err) {
612+
return d, err
613+
}
611614
}
612615
return v1.Descriptor{
613616
MediaType: mediaType,

0 commit comments

Comments
 (0)