fix(repodb): make table reset syncronous for consistent testing (#1111)

Signed-off-by: Laurentiu Niculae <niculae.laurentiu1@gmail.com>
(cherry picked from commit bea0eabcaa)
(cherry picked from commit 0f02d625331987126326a28731f14d3139061adc)

fix(repodb): use dynamodb sdk functions for waiting on tables to be created or deleted

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>

fix(ci): make sure the dynamodb tables used in testing have unique names

There were cases of failures when tests were run for the entire repodb package
which did not reproduce when tests were run for each sub-folder individually
This change should make sure there are no collisions between the names
used in concurrent tests.

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
Co-authored-by: Laurentiu Niculae <niculae.laurentiu1@gmail.com>

errors/errors.go

@@ -76,4 +76,5 @@ var (
 	ErrLimitIsNegative          = errors.New("pageturner: limit has negative value")
 	ErrOffsetIsNegative         = errors.New("pageturner: offset has negative value")
 	ErrSortCriteriaNotSupported = errors.New("pageturner: the sort criteria is not supported")
+	ErrTimeout                  = errors.New("operation timeout")
 )

pkg/meta/repodb/dynamodb-wrapper/dynamo_internal_test.go

@@ -0,0 +1,102 @@
+package dynamo
+
+import (
+	"context"
+	"os"
+	"testing"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/config"
+	"github.com/aws/aws-sdk-go-v2/service/dynamodb"
+	guuid "github.com/gofrs/uuid"
+	"github.com/rs/zerolog"
+	. "github.com/smartystreets/goconvey/convey"
+
+	"zotregistry.io/zot/pkg/log" //nolint:go-staticcheck
+	"zotregistry.io/zot/pkg/meta/repodb/version"
+)
+
+func TestWrapperErrors(t *testing.T) {
+	const (
+		endpoint = "http://localhost:4566"
+		region   = "us-east-2"
+	)
+
+	uuid, err := guuid.NewV4()
+	if err != nil {
+		panic(err)
+	}
+
+	repoMetaTablename := "RepoMetadataTable" + uuid.String()
+	manifestDataTablename := "ManifestDataTable" + uuid.String()
+	versionTablename := "Version" + uuid.String()
+
+	Convey("Create table errors", t, func() {
+		badEndpoint := endpoint + "1"
+
+		customResolver := aws.EndpointResolverWithOptionsFunc(
+			func(service, region string, options ...interface{}) (aws.Endpoint, error) {
+				return aws.Endpoint{
+					PartitionID:   "aws",
+					URL:           badEndpoint,
+					SigningRegion: region,
+				}, nil
+			},
+		)
+
+		cfg, err := config.LoadDefaultConfig(context.Background(), config.WithRegion(region),
+			config.WithEndpointResolverWithOptions(customResolver))
+		So(err, ShouldBeNil)
+
+		dynamoWrapper := DBWrapper{
+			Client:                dynamodb.NewFromConfig(cfg),
+			RepoMetaTablename:     repoMetaTablename,
+			ManifestDataTablename: manifestDataTablename,
+			VersionTablename:      versionTablename,
+			Patches:               version.GetDynamoDBPatches(),
+			Log:                   log.Logger{Logger: zerolog.New(os.Stdout)},
+		}
+
+		// The table creation should fail as the endpoint is not configured correctly
+		err = dynamoWrapper.createRepoMetaTable()
+		So(err, ShouldNotBeNil)
+
+		err = dynamoWrapper.createManifestDataTable()
+		So(err, ShouldNotBeNil)
+
+		err = dynamoWrapper.createVersionTable()
+		So(err, ShouldNotBeNil)
+	})
+
+	Convey("Delete table errors", t, func() {
+		customResolver := aws.EndpointResolverWithOptionsFunc(
+			func(service, region string, options ...interface{}) (aws.Endpoint, error) {
+				return aws.Endpoint{
+					PartitionID:   "aws",
+					URL:           endpoint,
+					SigningRegion: region,
+				}, nil
+			},
+		)
+
+		cfg, err := config.LoadDefaultConfig(context.Background(), config.WithRegion(region),
+			config.WithEndpointResolverWithOptions(customResolver))
+		So(err, ShouldBeNil)
+
+		dynamoWrapper := DBWrapper{
+			Client:                dynamodb.NewFromConfig(cfg),
+			RepoMetaTablename:     repoMetaTablename,
+			ManifestDataTablename: manifestDataTablename,
+			VersionTablename:      versionTablename,
+			Patches:               version.GetDynamoDBPatches(),
+			Log:                   log.Logger{Logger: zerolog.New(os.Stdout)},
+		}
+
+		// The tables were not created so delete calls fail, but dynamoWrapper should not error
+		err = dynamoWrapper.deleteRepoMetaTable()
+		So(err, ShouldBeNil)
+
+		err = dynamoWrapper.deleteManifestDataTable()
+		So(err, ShouldBeNil)
+	})
+}

pkg/meta/repodb/dynamodb-wrapper/dynamo_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue"
 	"github.com/aws/aws-sdk-go-v2/service/dynamodb"
 	"github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
+	guuid "github.com/gofrs/uuid"
 	"github.com/rs/zerolog"
 	. "github.com/smartystreets/goconvey/convey"
 
@@ -27,13 +28,22 @@ func TestIterator(t *testing.T) {
 		region   = "us-east-2"
 	)
 
+	uuid, err := guuid.NewV4()
+	if err != nil {
+		panic(err)
+	}
+
+	repoMetaTablename := "RepoMetadataTable" + uuid.String()
+	manifestDataTablename := "ManifestDataTable" + uuid.String()
+	versionTablename := "Version" + uuid.String()
+
 	Convey("TestIterator", t, func() {
 		dynamoWrapper, err := dynamo.NewDynamoDBWrapper(dynamoParams.DBDriverParameters{
 			Endpoint:              endpoint,
 			Region:                region,
-			RepoMetaTablename:     "RepoMetadataTable",
+			RepoMetaTablename:     repoMetaTablename,
-			ManifestDataTablename: "ManifestDataTable",
+			ManifestDataTablename: manifestDataTablename,
-			VersionTablename:      "Version",
+			VersionTablename:      versionTablename,
 		})
 		So(err, ShouldBeNil)
 
@@ -51,7 +61,7 @@ func TestIterator(t *testing.T) {
 
 		repoMetaAttributeIterator := iterator.NewBaseDynamoAttributesIterator(
 			dynamoWrapper.Client,
-			"RepoMetadataTable",
+			repoMetaTablename,
 			"RepoMetadata",
 			1,
 			log.Logger{Logger: zerolog.New(os.Stdout)},
@@ -109,15 +119,24 @@ func TestWrapperErrors(t *testing.T) {
 		region   = "us-east-2"
 	)
 
+	uuid, err := guuid.NewV4()
+	if err != nil {
+		panic(err)
+	}
+
+	repoMetaTablename := "RepoMetadataTable" + uuid.String()
+	manifestDataTablename := "ManifestDataTable" + uuid.String()
+	versionTablename := "Version" + uuid.String()
+
 	ctx := context.Background()
 
 	Convey("Errors", t, func() {
 		dynamoWrapper, err := dynamo.NewDynamoDBWrapper(dynamoParams.DBDriverParameters{ //nolint:contextcheck
 			Endpoint:              endpoint,
 			Region:                region,
-			RepoMetaTablename:     "RepoMetadataTable",
+			RepoMetaTablename:     repoMetaTablename,
-			ManifestDataTablename: "ManifestDataTable",
+			ManifestDataTablename: manifestDataTablename,
-			VersionTablename:      "Version",
+			VersionTablename:      versionTablename,
 		})
 		So(err, ShouldBeNil)
 
@@ -139,7 +158,7 @@ func TestWrapperErrors(t *testing.T) {
 		})
 
 		Convey("GetManifestData unmarshal error", func() {
-			err := setBadManifestData(dynamoWrapper.Client, "dig")
+			err := setBadManifestData(dynamoWrapper.Client, manifestDataTablename, "dig")
 			So(err, ShouldBeNil)
 
 			_, err = dynamoWrapper.GetManifestData("dig")
@@ -147,7 +166,7 @@ func TestWrapperErrors(t *testing.T) {
 		})
 
 		Convey("SetManifestMeta GetRepoMeta error", func() {
-			err := setBadRepoMeta(dynamoWrapper.Client, "repo1")
+			err := setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo1")
 			So(err, ShouldBeNil)
 
 			err = dynamoWrapper.SetManifestMeta("repo1", "dig", repodb.ManifestMetadata{})
@@ -174,7 +193,7 @@ func TestWrapperErrors(t *testing.T) {
 			err := dynamoWrapper.SetManifestData("dig", repodb.ManifestData{})
 			So(err, ShouldBeNil)
 
-			err = setBadRepoMeta(dynamoWrapper.Client, "repo")
+			err = setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo")
 			So(err, ShouldBeNil)
 
 			_, err = dynamoWrapper.GetManifestMeta("repo", "dig")
@@ -200,7 +219,7 @@ func TestWrapperErrors(t *testing.T) {
 		})
 
 		Convey("DeleteRepoTag unmarshal error", func() {
-			err = setBadRepoMeta(dynamoWrapper.Client, "repo")
+			err = setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo")
 			So(err, ShouldBeNil)
 
 			err = dynamoWrapper.DeleteRepoTag("repo", "tag")
@@ -216,7 +235,7 @@ func TestWrapperErrors(t *testing.T) {
 		})
 
 		Convey("GetRepoMeta unmarshal error", func() {
-			err = setBadRepoMeta(dynamoWrapper.Client, "repo")
+			err = setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo")
 			So(err, ShouldBeNil)
 
 			_, err = dynamoWrapper.GetRepoMeta("repo")
@@ -276,7 +295,7 @@ func TestWrapperErrors(t *testing.T) {
 		})
 
 		Convey("DeleteSignature sigDigest.SignatureManifestDigest != sigMeta.SignatureDigest true", func() {
-			err := setRepoMeta(dynamoWrapper.Client, repodb.RepoMetadata{
+			err := setRepoMeta(dynamoWrapper.Client, repoMetaTablename, repodb.RepoMetadata{
 				Name: "repo",
 				Signatures: map[string]repodb.ManifestSignatures{
 					"tag1": {
@@ -297,7 +316,7 @@ func TestWrapperErrors(t *testing.T) {
 		})
 
 		Convey("GetMultipleRepoMeta unmarshal error", func() {
-			err = setBadRepoMeta(dynamoWrapper.Client, "repo") //nolint:contextcheck
+			err = setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo") //nolint:contextcheck
 			So(err, ShouldBeNil)
 
 			_, err = dynamoWrapper.GetMultipleRepoMeta(ctx, func(repoMeta repodb.RepoMetadata) bool { return true },
@@ -307,7 +326,7 @@ func TestWrapperErrors(t *testing.T) {
 		})
 
 		Convey("SearchRepos repoMeta unmarshal error", func() {
-			err = setBadRepoMeta(dynamoWrapper.Client, "repo") //nolint:contextcheck
+			err = setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo") //nolint:contextcheck
 			So(err, ShouldBeNil)
 
 			_, _, err = dynamoWrapper.SearchRepos(ctx, "", repodb.Filter{}, repodb.PageInput{})
@@ -340,7 +359,7 @@ func TestWrapperErrors(t *testing.T) {
 		})
 
 		Convey("SearchTags repoMeta unmarshal error", func() {
-			err = setBadRepoMeta(dynamoWrapper.Client, "repo") //nolint:contextcheck
+			err = setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo") //nolint:contextcheck
 			So(err, ShouldBeNil)
 
 			_, _, err = dynamoWrapper.SearchTags(ctx, "repo:", repodb.Filter{}, repodb.PageInput{})
@@ -377,7 +396,7 @@ func TestWrapperErrors(t *testing.T) {
 	})
 }
 
-func setBadManifestData(client *dynamodb.Client, digest string) error {
+func setBadManifestData(client *dynamodb.Client, manifestDataTableName, digest string) error {
 	mdAttributeValue, err := attributevalue.Marshal("string")
 	if err != nil {
 		return err
@@ -395,14 +414,14 @@ func setBadManifestData(client *dynamodb.Client, digest string) error {
 				Value: digest,
 			},
 		},
-		TableName:        aws.String("ManifestDataTable"),
+		TableName:        aws.String(manifestDataTableName),
 		UpdateExpression: aws.String("SET #MD = :ManifestData"),
 	})
 
 	return err
 }
 
-func setBadRepoMeta(client *dynamodb.Client, repoName string) error {
+func setBadRepoMeta(client *dynamodb.Client, repoMetadataTableName, repoName string) error {
 	repoAttributeValue, err := attributevalue.Marshal("string")
 	if err != nil {
 		return err
@@ -420,14 +439,14 @@ func setBadRepoMeta(client *dynamodb.Client, repoName string) error {
 				Value: repoName,
 			},
 		},
-		TableName:        aws.String("RepoMetadataTable"),
+		TableName:        aws.String(repoMetadataTableName),
 		UpdateExpression: aws.String("SET #RM = :RepoMetadata"),
 	})
 
 	return err
 }
 
-func setRepoMeta(client *dynamodb.Client, repoMeta repodb.RepoMetadata) error {
+func setRepoMeta(client *dynamodb.Client, repoMetadataTableName string, repoMeta repodb.RepoMetadata) error {
 	repoAttributeValue, err := attributevalue.Marshal(repoMeta)
 	if err != nil {
 		return err
@@ -445,7 +464,7 @@ func setRepoMeta(client *dynamodb.Client, repoMeta repodb.RepoMetadata) error {
 				Value: repoMeta.Name,
 			},
 		},
-		TableName:        aws.String("RepoMetadataTable"),
+		TableName:        aws.String(repoMetadataTableName),
 		UpdateExpression: aws.String("SET #RM = :RepoMetadata"),
 	})
 

pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go

@@ -833,11 +833,11 @@ func (dwr DBWrapper) createRepoMetaTable() error {
 		BillingMode: types.BillingModePayPerRequest,
 	})
 
-	if err != nil && strings.Contains(err.Error(), "Table already exists") {
+	if err != nil && !strings.Contains(err.Error(), "Table already exists") {
-		return nil
+		return err
 	}
 
-	return err
+	return dwr.waitTableToBeCreated(dwr.RepoMetaTablename)
 }
 
 func (dwr DBWrapper) deleteRepoMetaTable() error {
@@ -845,7 +845,11 @@ func (dwr DBWrapper) deleteRepoMetaTable() error {
 		TableName: aws.String(dwr.RepoMetaTablename),
 	})
 
-	return err
+	if temp := new(types.ResourceNotFoundException); errors.As(err, &temp) {
+		return nil
+	}
+
+	return dwr.waitTableToBeDeleted(dwr.RepoMetaTablename)
 }
 
 func (dwr DBWrapper) ResetRepoMetaTable() error {
@@ -857,6 +861,26 @@ func (dwr DBWrapper) ResetRepoMetaTable() error {
 	return dwr.createRepoMetaTable()
 }
 
+func (dwr DBWrapper) waitTableToBeCreated(tableName string) error {
+	const maxWaitTime = 20 * time.Second
+
+	waiter := dynamodb.NewTableExistsWaiter(dwr.Client)
+
+	return waiter.Wait(context.Background(), &dynamodb.DescribeTableInput{
+		TableName: &tableName,
+	}, maxWaitTime)
+}
+
+func (dwr DBWrapper) waitTableToBeDeleted(tableName string) error {
+	const maxWaitTime = 20 * time.Second
+
+	waiter := dynamodb.NewTableNotExistsWaiter(dwr.Client)
+
+	return waiter.Wait(context.Background(), &dynamodb.DescribeTableInput{
+		TableName: &tableName,
+	}, maxWaitTime)
+}
+
 func (dwr DBWrapper) createManifestDataTable() error {
 	_, err := dwr.Client.CreateTable(context.Background(), &dynamodb.CreateTableInput{
 		TableName: aws.String(dwr.ManifestDataTablename),
@@ -875,11 +899,11 @@ func (dwr DBWrapper) createManifestDataTable() error {
 		BillingMode: types.BillingModePayPerRequest,
 	})
 
-	if err != nil && strings.Contains(err.Error(), "Table already exists") {
+	if err != nil && !strings.Contains(err.Error(), "Table already exists") {
-		return nil
+		return err
 	}
 
-	return err
+	return dwr.waitTableToBeCreated(dwr.ManifestDataTablename)
 }
 
 func (dwr *DBWrapper) createVersionTable() error {
@@ -899,9 +923,17 @@ func (dwr *DBWrapper) createVersionTable() error {
 		},
 		BillingMode: types.BillingModePayPerRequest,
 	})
+	if err != nil {
+		if strings.Contains(err.Error(), "Table already exists") {
+			return nil
+		}
 
-	if err != nil && strings.Contains(err.Error(), "Table already exists") {
+		return err
-		return nil
+	}
+
+	err = dwr.waitTableToBeCreated(dwr.VersionTablename)
+	if err != nil {
+		return err
 	}
 
 	if err == nil {
@@ -931,7 +963,7 @@ func (dwr *DBWrapper) createVersionTable() error {
 		}
 	}
 
-	return err
+	return nil
 }
 
 func (dwr *DBWrapper) getDBVersion() (string, error) {
@@ -964,7 +996,11 @@ func (dwr DBWrapper) deleteManifestDataTable() error {
 		TableName: aws.String(dwr.ManifestDataTablename),
 	})
 
-	return err
+	if temp := new(types.ResourceNotFoundException); errors.As(err, &temp) {
+		return nil
+	}
+
+	return dwr.waitTableToBeDeleted(dwr.ManifestDataTablename)
 }
 
 func (dwr DBWrapper) ResetManifestDataTable() error {

pkg/meta/repodb/repodb_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 	"time"
 
+	guuid "github.com/gofrs/uuid"
 	"github.com/opencontainers/go-digest"
 	"github.com/opencontainers/image-spec/specs-go"
 	ispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -65,12 +66,21 @@ func TestBoltDBWrapper(t *testing.T) {
 func TestDynamoDBWrapper(t *testing.T) {
 	skipIt(t)
 
+	uuid, err := guuid.NewV4()
+	if err != nil {
+		panic(err)
+	}
+
+	repoMetaTablename := "RepoMetadataTable" + uuid.String()
+	manifestDataTablename := "ManifestDataTable" + uuid.String()
+	versionTablename := "Version" + uuid.String()
+
 	Convey("DynamoDB Wrapper", t, func() {
 		dynamoDBDriverParams := dynamoParams.DBDriverParameters{
 			Endpoint:              os.Getenv("DYNAMODBMOCK_ENDPOINT"),
-			RepoMetaTablename:     "RepoMetadataTable",
+			RepoMetaTablename:     repoMetaTablename,
-			ManifestDataTablename: "ManifestDataTable",
+			ManifestDataTablename: manifestDataTablename,
-			VersionTablename:      "Version",
+			VersionTablename:      versionTablename,
 			Region:                "us-east-2",
 		}