From 1d782c38f238ef0d7c3cafa92dc08cbf20865690 Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Fri, 6 May 2016 10:46:37 +0100 Subject: [PATCH 1/3] StorageDriver: Test case for #1698 Signed-off-by: Arthur Baars --- registry/handlers/api_test.go | 18 ++--- registry/handlers/app_test.go | 6 +- registry/storage/blob_test.go | 16 +++-- .../storage/driver/testdriver/testdriver.go | 71 +++++++++++++++++++ 4 files changed, 92 insertions(+), 19 deletions(-) create mode 100644 registry/storage/driver/testdriver/testdriver.go diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index 523ecca28..8f4bff0ed 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -29,7 +29,7 @@ import ( "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" - _ "github.com/docker/distribution/registry/storage/driver/inmemory" + _ "github.com/docker/distribution/registry/storage/driver/testdriver" "github.com/docker/distribution/testutil" "github.com/docker/libtrust" "github.com/gorilla/handlers" @@ -219,7 +219,7 @@ func contains(elems []string, e string) bool { func TestURLPrefix(t *testing.T) { config := configuration.Configuration{ Storage: configuration.Storage{ - "inmemory": configuration.Parameters{}, + "testdriver": configuration.Parameters{}, }, } config.HTTP.Prefix = "/test/" @@ -296,7 +296,7 @@ func TestBlobDelete(t *testing.T) { func TestRelativeURL(t *testing.T) { config := configuration.Configuration{ Storage: configuration.Storage{ - "inmemory": configuration.Parameters{}, + "testdriver": configuration.Parameters{}, }, } config.HTTP.Headers = headerConfig @@ -1884,8 +1884,8 @@ type testEnv struct { func newTestEnvMirror(t *testing.T, deleteEnabled bool) *testEnv { config := configuration.Configuration{ Storage: configuration.Storage{ - "inmemory": configuration.Parameters{}, - "delete": configuration.Parameters{"enabled": deleteEnabled}, + "testdriver": configuration.Parameters{}, + "delete": configuration.Parameters{"enabled": deleteEnabled}, }, Proxy: configuration.Proxy{ RemoteURL: "http://example.com", @@ -1899,8 +1899,8 @@ func newTestEnvMirror(t *testing.T, deleteEnabled bool) *testEnv { func newTestEnv(t *testing.T, deleteEnabled bool) *testEnv { config := configuration.Configuration{ Storage: configuration.Storage{ - "inmemory": configuration.Parameters{}, - "delete": configuration.Parameters{"enabled": deleteEnabled}, + "testdriver": configuration.Parameters{}, + "delete": configuration.Parameters{"enabled": deleteEnabled}, }, } @@ -2413,7 +2413,7 @@ func TestCheckContextNotifier(t *testing.T) { func TestProxyManifestGetByTag(t *testing.T) { truthConfig := configuration.Configuration{ Storage: configuration.Storage{ - "inmemory": configuration.Parameters{}, + "testdriver": configuration.Parameters{}, }, } truthConfig.HTTP.Headers = headerConfig @@ -2427,7 +2427,7 @@ func TestProxyManifestGetByTag(t *testing.T) { proxyConfig := configuration.Configuration{ Storage: configuration.Storage{ - "inmemory": configuration.Parameters{}, + "testdriver": configuration.Parameters{}, }, Proxy: configuration.Proxy{ RemoteURL: truthEnv.server.URL, diff --git a/registry/handlers/app_test.go b/registry/handlers/app_test.go index caa7ab97e..3a8e4e1e4 100644 --- a/registry/handlers/app_test.go +++ b/registry/handlers/app_test.go @@ -16,7 +16,7 @@ import ( _ "github.com/docker/distribution/registry/auth/silly" "github.com/docker/distribution/registry/storage" memorycache "github.com/docker/distribution/registry/storage/cache/memory" - "github.com/docker/distribution/registry/storage/driver/inmemory" + "github.com/docker/distribution/registry/storage/driver/testdriver" ) // TestAppDispatcher builds an application with a test dispatcher and ensures @@ -24,7 +24,7 @@ import ( // This only tests the dispatch mechanism. The underlying dispatchers must be // tested individually. func TestAppDispatcher(t *testing.T) { - driver := inmemory.New() + driver := testdriver.New() ctx := context.Background() registry, err := storage.NewRegistry(ctx, driver, storage.BlobDescriptorCacheProvider(memorycache.NewInMemoryBlobDescriptorCacheProvider()), storage.EnableDelete, storage.EnableRedirect) if err != nil { @@ -142,7 +142,7 @@ func TestNewApp(t *testing.T) { ctx := context.Background() config := configuration.Configuration{ Storage: configuration.Storage{ - "inmemory": nil, + "testdriver": nil, }, Auth: configuration.Auth{ // For now, we simply test that new auth results in a viable diff --git a/registry/storage/blob_test.go b/registry/storage/blob_test.go index 7e1a7cd44..3cec3bff8 100644 --- a/registry/storage/blob_test.go +++ b/registry/storage/blob_test.go @@ -14,7 +14,7 @@ import ( "github.com/docker/distribution/digest" "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/storage/cache/memory" - "github.com/docker/distribution/registry/storage/driver/inmemory" + "github.com/docker/distribution/registry/storage/driver/testdriver" "github.com/docker/distribution/testutil" "path" ) @@ -24,7 +24,7 @@ import ( func TestWriteSeek(t *testing.T) { ctx := context.Background() imageName, _ := reference.ParseNamed("foo/bar") - driver := inmemory.New() + driver := testdriver.New() registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect) if err != nil { t.Fatalf("error creating registry: %v", err) @@ -42,6 +42,7 @@ func TestWriteSeek(t *testing.T) { } contents := []byte{1, 2, 3} blobUpload.Write(contents) + blobUpload.Close() offset := blobUpload.Size() if offset != int64(len(contents)) { t.Fatalf("unexpected value for blobUpload offset: %v != %v", offset, len(contents)) @@ -59,7 +60,7 @@ func TestSimpleBlobUpload(t *testing.T) { ctx := context.Background() imageName, _ := reference.ParseNamed("foo/bar") - driver := inmemory.New() + driver := testdriver.New() registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect) if err != nil { t.Fatalf("error creating registry: %v", err) @@ -120,11 +121,12 @@ func TestSimpleBlobUpload(t *testing.T) { t.Fatalf("layer data write incomplete") } + blobUpload.Close() + offset := blobUpload.Size() if offset != nn { t.Fatalf("blobUpload not updated with correct offset: %v != %v", offset, nn) } - blobUpload.Close() // Do a resume, for good fun blobUpload, err = bs.Resume(ctx, blobUpload.ID()) @@ -253,7 +255,7 @@ func TestSimpleBlobUpload(t *testing.T) { func TestSimpleBlobRead(t *testing.T) { ctx := context.Background() imageName, _ := reference.ParseNamed("foo/bar") - driver := inmemory.New() + driver := testdriver.New() registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect) if err != nil { t.Fatalf("error creating registry: %v", err) @@ -365,7 +367,7 @@ func TestBlobMount(t *testing.T) { ctx := context.Background() imageName, _ := reference.ParseNamed("foo/bar") sourceImageName, _ := reference.ParseNamed("foo/source") - driver := inmemory.New() + driver := testdriver.New() registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect) if err != nil { t.Fatalf("error creating registry: %v", err) @@ -516,7 +518,7 @@ func TestBlobMount(t *testing.T) { func TestLayerUploadZeroLength(t *testing.T) { ctx := context.Background() imageName, _ := reference.ParseNamed("foo/bar") - driver := inmemory.New() + driver := testdriver.New() registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect) if err != nil { t.Fatalf("error creating registry: %v", err) diff --git a/registry/storage/driver/testdriver/testdriver.go b/registry/storage/driver/testdriver/testdriver.go new file mode 100644 index 000000000..988e5d33b --- /dev/null +++ b/registry/storage/driver/testdriver/testdriver.go @@ -0,0 +1,71 @@ +package testdriver + +import ( + "github.com/docker/distribution/context" + storagedriver "github.com/docker/distribution/registry/storage/driver" + "github.com/docker/distribution/registry/storage/driver/factory" + "github.com/docker/distribution/registry/storage/driver/inmemory" +) + +const driverName = "testdriver" + +func init() { + factory.Register(driverName, &testDriverFactory{}) +} + +// testDriverFactory implements the factory.StorageDriverFactory interface. +type testDriverFactory struct{} + +func (factory *testDriverFactory) Create(parameters map[string]interface{}) (storagedriver.StorageDriver, error) { + return New(), nil +} + +// TestDriver is a StorageDriver for testing purposes. The Writer returned by this driver +// simulates the case where Write operations are buffered. This causes the value returned by Size to lag +// behind until Close (or Commit, or Cancel) is called. +type TestDriver struct { + storagedriver.StorageDriver +} + +type testFileWriter struct { + storagedriver.FileWriter + prevchunk []byte +} + +var _ storagedriver.StorageDriver = &TestDriver{} + +// New constructs a new StorageDriver for testing purposes. The Writer returned by this driver +// simulates the case where Write operations are buffered. This causes the value returned by Size to lag +// behind until Close (or Commit, or Cancel) is called. +func New() *TestDriver { + return &TestDriver{StorageDriver: inmemory.New()} +} + +// Writer returns a FileWriter which will store the content written to it +// at the location designated by "path" after the call to Commit. +func (td *TestDriver) Writer(ctx context.Context, path string, append bool) (storagedriver.FileWriter, error) { + fw, err := td.StorageDriver.Writer(ctx, path, append) + return &testFileWriter{FileWriter: fw}, err +} + +func (tfw *testFileWriter) Write(p []byte) (int, error) { + _, err := tfw.FileWriter.Write(tfw.prevchunk) + tfw.prevchunk = make([]byte, len(p)) + copy(tfw.prevchunk, p) + return len(p), err +} + +func (tfw *testFileWriter) Close() error { + tfw.Write(nil) + return tfw.FileWriter.Close() +} + +func (tfw *testFileWriter) Cancel() error { + tfw.Write(nil) + return tfw.FileWriter.Cancel() +} + +func (tfw *testFileWriter) Commit() error { + tfw.Write(nil) + return tfw.FileWriter.Commit() +} From 0490ff450b797e7092d98d848625bd949987d56b Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Thu, 5 May 2016 15:49:14 +0100 Subject: [PATCH 2/3] Blobwriter: call BlobWriter.Size after BlobWriter.Close Signed-off-by: Arthur Baars --- registry/handlers/blobupload.go | 5 +---- registry/storage/blobwriter.go | 1 + 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/registry/handlers/blobupload.go b/registry/handlers/blobupload.go index 673e2c591..b403a1672 100644 --- a/registry/handlers/blobupload.go +++ b/registry/handlers/blobupload.go @@ -134,7 +134,6 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req } buh.Upload = upload - defer buh.Upload.Close() if err := buh.blobUploadResponse(w, r, true); err != nil { buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) @@ -224,11 +223,8 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht return } - size := buh.Upload.Size() - desc, err := buh.Upload.Commit(buh, distribution.Descriptor{ Digest: dgst, - Size: size, // TODO(stevvooe): This isn't wildly important yet, but we should // really set the mediatype. For now, we can let the backend take care @@ -293,6 +289,7 @@ func (buh *blobUploadHandler) blobUploadResponse(w http.ResponseWriter, r *http. // TODO(stevvooe): Need a better way to manage the upload state automatically. buh.State.Name = buh.Repository.Named().Name() buh.State.UUID = buh.Upload.ID() + buh.Upload.Close() buh.State.Offset = buh.Upload.Size() buh.State.StartedAt = buh.Upload.StartedAt() diff --git a/registry/storage/blobwriter.go b/registry/storage/blobwriter.go index 3387bafb1..48ac8a752 100644 --- a/registry/storage/blobwriter.go +++ b/registry/storage/blobwriter.go @@ -58,6 +58,7 @@ func (bw *blobWriter) Commit(ctx context.Context, desc distribution.Descriptor) } bw.Close() + desc.Size = bw.Size() canonical, err := bw.validateBlob(ctx, desc) if err != nil { From eca581cf36d7994b171f8b357f78de1d923474ff Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Thu, 5 May 2016 17:16:48 +0100 Subject: [PATCH 3/3] StorageDriver: GCS: allow Cancel on a closed FileWriter Signed-off-by: Arthur Baars --- registry/storage/driver/gcs/gcs.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/registry/storage/driver/gcs/gcs.go b/registry/storage/driver/gcs/gcs.go index abe0b9f68..1369c280a 100644 --- a/registry/storage/driver/gcs/gcs.go +++ b/registry/storage/driver/gcs/gcs.go @@ -321,12 +321,8 @@ type writer struct { // Cancel removes any written content from this FileWriter. func (w *writer) Cancel() error { - err := w.checkClosed() - if err != nil { - return err - } w.closed = true - err = storageDeleteObject(cloud.NewContext(dummyProjectID, w.client), w.bucket, w.name) + err := storageDeleteObject(cloud.NewContext(dummyProjectID, w.client), w.bucket, w.name) if err != nil { if status, ok := err.(*googleapi.Error); ok { if status.Code == http.StatusNotFound {