From 90ece48d7750663d962adc8d12e80f60420ca7fc Mon Sep 17 00:00:00 2001 From: Flavian Missi Date: Tue, 30 May 2023 09:20:55 +0200 Subject: [PATCH 1/4] registry/storage/driver: add test call to Stat on "/" Stat(ctx, "/") is called by the registry healthcheck. Also fixes blob name building in the Azure driver so it no longer returns empty blob names. This was causing errors in the healthcheck call to Stat for Azure. Signed-off-by: Flavian Missi --- registry/storage/driver/azure/azure.go | 10 +++++++++- registry/storage/driver/testsuites/testsuites.go | 9 +++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/registry/storage/driver/azure/azure.go b/registry/storage/driver/azure/azure.go index 4e6d9a20..028239fd 100644 --- a/registry/storage/driver/azure/azure.go +++ b/registry/storage/driver/azure/azure.go @@ -63,7 +63,8 @@ func New(params *Parameters) (*Driver, error) { d := &driver{ azClient: azClient, client: client, - rootDirectory: params.RootDirectory} + rootDirectory: params.RootDirectory, + } return &Driver{baseEmbed: baseEmbed{Base: base.Base{StorageDriver: d}}}, nil } @@ -420,6 +421,13 @@ func (d *driver) listBlobs(ctx context.Context, virtPath string) ([]string, erro } func (d *driver) blobName(path string) string { + // avoid returning an empty blob name. + // this will happen when rootDirectory is unset, and path == "/", + // which is what we get from the storage driver health check Stat call. + if d.rootDirectory == "" && path == "/" { + return path + } + return strings.TrimLeft(strings.TrimRight(d.rootDirectory, "/")+path, "/") } diff --git a/registry/storage/driver/testsuites/testsuites.go b/registry/storage/driver/testsuites/testsuites.go index 58f6b6d6..27923aa3 100644 --- a/registry/storage/driver/testsuites/testsuites.go +++ b/registry/storage/driver/testsuites/testsuites.go @@ -824,6 +824,15 @@ func (suite *DriverSuite) TestStatCall(c *check.C) { c.Assert(fi.Path(), check.Equals, dirPath) c.Assert(fi.Size(), check.Equals, int64(0)) c.Assert(fi.IsDir(), check.Equals, true) + + // The storage healthcheck performs this exact call to Stat. + // PathNotFoundErrors are not considered health check failures. + _, err = suite.StorageDriver.Stat(suite.ctx, "/") + // Some drivers will return a not found here, while others will not + // return an error at all. If we get an error, ensure it's a not found. + if err != nil { + c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + } } // TestPutContentMultipleTimes checks that if storage driver can overwrite the content From d2e16fc74add3e1514444c3f140dc3df4797f647 Mon Sep 17 00:00:00 2001 From: Flavian Missi Date: Tue, 30 May 2023 09:23:17 +0200 Subject: [PATCH 2/4] registry/storage/driver/azure: fix driver parameters on tests fixes "azure: no Host in request URL" tests failure, allowing azure driver tests to run. Signed-off-by: Flavian Missi --- registry/storage/driver/azure/azure_test.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/registry/storage/driver/azure/azure_test.go b/registry/storage/driver/azure/azure_test.go index 5b0b9533..42a8a9b4 100644 --- a/registry/storage/driver/azure/azure_test.go +++ b/registry/storage/driver/azure/azure_test.go @@ -52,14 +52,18 @@ func init() { } azureDriverConstructor := func() (storagedriver.StorageDriver, error) { - params := Parameters{ - Container: container, - AccountName: accountName, - AccountKey: accountKey, - Realm: realm, - RootDirectory: rootDirectory, + parameters := map[string]interface{}{ + "container": container, + "accountname": accountName, + "accountkey": accountKey, + "realm": realm, + "rootdirectory": rootDirectory, } - return New(¶ms) + params, err := NewParameters(parameters) + if err != nil { + return nil, err + } + return New(params) } // Skip Azure storage driver tests if environment variable parameters are not provided From 0c33bb1092195f50aa652bdb6c7d31129b93a77a Mon Sep 17 00:00:00 2001 From: Flavian Missi Date: Tue, 30 May 2023 14:45:33 +0200 Subject: [PATCH 3/4] registry/storage/driver/azure: consider CannotVerifyCopySource as 404 Azure will return CannotVerifyCopySource with a 404 status code from a call to Move when the source blob does not exist. Details: https://learn.microsoft.com/en-us/rest/api/storageservices/blob-service-error-codes. This fixes the TestMoveNonexistent test case for the Azure driver. Signed-off-by: Flavian Missi --- registry/storage/driver/azure/azure.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/registry/storage/driver/azure/azure.go b/registry/storage/driver/azure/azure.go index 028239fd..88aea274 100644 --- a/registry/storage/driver/azure/azure.go +++ b/registry/storage/driver/azure/azure.go @@ -432,7 +432,13 @@ func (d *driver) blobName(path string) string { } func is404(err error) bool { - return bloberror.HasCode(err, bloberror.BlobNotFound, bloberror.ContainerNotFound, bloberror.ResourceNotFound) + return bloberror.HasCode( + err, + bloberror.BlobNotFound, + bloberror.ContainerNotFound, + bloberror.ResourceNotFound, + bloberror.CannotVerifyCopySource, + ) } type writer struct { From 0d20e7ae9ea7050f59bf526aa718bf4422c00be9 Mon Sep 17 00:00:00 2001 From: Flavian Missi Date: Tue, 30 May 2023 14:52:39 +0200 Subject: [PATCH 4/4] registry/storage/driver/testsuites: use 4MB for Azure append test Fixes #3931. Signed-off-by: Flavian Missi --- registry/storage/driver/testsuites/testsuites.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/registry/storage/driver/testsuites/testsuites.go b/registry/storage/driver/testsuites/testsuites.go index 27923aa3..9847bb38 100644 --- a/registry/storage/driver/testsuites/testsuites.go +++ b/registry/storage/driver/testsuites/testsuites.go @@ -388,7 +388,11 @@ func (suite *DriverSuite) TestReaderWithOffset(c *check.C) { // TestContinueStreamAppendLarge tests that a stream write can be appended to without // corrupting the data with a large chunk size. func (suite *DriverSuite) TestContinueStreamAppendLarge(c *check.C) { - suite.testContinueStreamAppend(c, int64(10*1024*1024)) + chunkSize := int64(10 * 1024 * 1024) + if suite.Name() == "azure" { + chunkSize = int64(4 * 1024 * 1024) + } + suite.testContinueStreamAppend(c, chunkSize) } // TestContinueStreamAppendSmall is the same as TestContinueStreamAppendLarge, but only