diff --git a/registry/storage/blob_test.go b/registry/storage/blob_test.go index 0cea43893..9273b7668 100644 --- a/registry/storage/blob_test.go +++ b/registry/storage/blob_test.go @@ -50,6 +50,44 @@ func TestWriteSeek(t *testing.T) { } } +// TestBlobUploadCloseAfterCancel checks that closing a cancelled upload, as +// the HTTP handler does with a deferred Close, does not write the hash state +// back into the upload directory that Cancel removed. +func TestBlobUploadCloseAfterCancel(t *testing.T) { + ctx := context.Background() + imageName, _ := reference.WithName("foo/bar") + driver := inmemory.New() + registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider(memory.UnlimitedSize)), EnableDelete, EnableRedirect) + if err != nil { + t.Fatalf("error creating registry: %v", err) + } + repository, err := registry.Repository(ctx, imageName) + if err != nil { + t.Fatalf("unexpected error getting repo: %v", err) + } + bs := repository.Blobs(ctx) + + blobUpload, err := bs.Create(ctx) + if err != nil { + t.Fatalf("unexpected error starting layer upload: %s", err) + } + if _, err := blobUpload.Write([]byte{1, 2, 3}); err != nil { + t.Fatalf("unexpected error writing contents: %v", err) + } + uploadPath := path.Dir(blobUpload.(*blobWriter).path) + + if err := blobUpload.Cancel(ctx); err != nil { + t.Fatalf("unexpected error during upload cancellation: %v", err) + } + if err := blobUpload.Close(); err != nil { + t.Fatalf("unexpected error closing cancelled upload: %v", err) + } + + if paths, err := driver.List(ctx, uploadPath); err == nil { + t.Fatalf("upload path recreated after cancel and close: %v", paths) + } +} + // TestSimpleBlobUpload covers the blob upload process, exercising common // error paths that might be seen during an upload. func TestSimpleBlobUpload(t *testing.T) { diff --git a/registry/storage/blobwriter.go b/registry/storage/blobwriter.go index 7fe916fc3..026d73d4c 100644 --- a/registry/storage/blobwriter.go +++ b/registry/storage/blobwriter.go @@ -40,6 +40,7 @@ type blobWriter struct { resumableDigestEnabled bool committed bool + cancelled bool } var _ distribution.BlobWriter = &blobWriter{} @@ -98,6 +99,7 @@ func (bw *blobWriter) Cancel(ctx context.Context) error { if err := bw.fileWriter.Cancel(ctx); err != nil { return err } + bw.cancelled = true if err := bw.Close(); err != nil { dcontext.GetLogger(ctx).Errorf("error closing blobwriter: %s", err) @@ -152,6 +154,16 @@ func (bw *blobWriter) Close() error { return errors.New("blobwriter close after commit") } + if bw.cancelled { + // The upload is being discarded, so there is no hash state worth + // saving. Writing it would recreate the upload directory after + // removeResources deleted it, without a startedat file, and + // PurgeUploads never removes such a directory. The file writer has + // already been cancelled, so an error from closing it is expected. + _ = bw.fileWriter.Close() + return nil + } + if err := bw.storeHashState(bw.blobStore.ctx); err != nil && err != errResumableDigestNotAvailable { return err }