Add database interface for blob paths + DynamoDB integration + refactor BoltDB to use interface #588

chofnar · 2022-06-14T13:30:35Z

Closed as I split this PR into two: #667 and chofnar#18

What type of PR is this?

enhancement

Which issue does this PR fix:

What does this PR do / Why do we need it:

This PR adds a database interface that allows the usage of vendor-specific database services by implementing their drivers.
The usage of BoltDB was refactored to use this new interface.
Added AWS DynamoDB driver.

Will this break upgrades or downgrades?

If a client uses DynamoDB and decides to downgrade, they must make sure that the database is migrated to BoltDB to avoid data loss.

Does this PR introduce any user-facing change?:

Adds the possibility to use AWS DynamoDB instead of BoltDB

Problems
Right now, there are several problems with this PR:

database_factory.go uses global variable driverFactories to keep track of the available drivers (gochecknoglobals)
new entries to that variable are added with the Drivers' init() functions (gochecknoinits)
need to specify protocol in front of dynamodb endpoint (known issue)
checking for db interface value to be nil perhaps less than ideal way: fmt.Sprintf("%v", is.cache) == fmt.Sprintf("%v", nil)

Other comments
Inspiration from https://github.com/distribution/distribution

To do

check that the example config actually works
add coverage tests
rewrite some (if not all) cache tests for dynamodb

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

rchincha · 2022-06-14T15:14:14Z

why so?

rchincha · 2022-06-14T16:43:34Z

.github/workflows/ci-cd.yml

@@ -29,7 +29,7 @@ jobs:
      s3mock:
        image: ghcr.io/project-zot/localstack/localstack:0.13.2
        env:
-          SERVICES: s3
+          SERVICES: s3,dynamodb


This makes us a user of another aws service? :\

yes..I propose to keep backing up boltdb in s3, and locally we just download it to a temp dir, use it, and keep backing it up?

this will work for one zot but not for multiple, for multiple we should do the same for all GETs but that will add a lot of overhead as GETs are very frequent....

codecov · 2022-06-15T14:00:58Z

Codecov Report

Merging #588 (42f322a) into main (4a3c007) will decrease coverage by 0.87%.
The diff coverage is 67.41%.

❗ Current head 42f322a differs from pull request most recent head 8886f6f. Consider uploading reports for the commit 8886f6f to get more accurate results

@@            Coverage Diff             @@
##             main     #588      +/-   ##
==========================================
- Coverage   88.05%   87.18%   -0.88%     
==========================================
  Files          61       62       +1     
  Lines       11630    11712      +82     
==========================================
- Hits        10241    10211      -30     
- Misses       1090     1190     +100     
- Partials      299      311      +12

Impacted Files	Coverage Δ
pkg/api/config/config.go	`85.41% <ø> (ø)`
pkg/api/controller.go	`80.54% <24.70%> (-13.37%)`	⬇️
pkg/storage/dynamodatabase/dynamodatabase.go	`74.28% <74.28%> (ø)`
pkg/storage/cache.go	`67.22% <86.53%> (+5.28%)`	⬆️
pkg/storage/database/database_factory.go	`100.00% <100.00%> (ø)`
pkg/storage/local.go	`82.67% <100.00%> (-0.15%)`	⬇️
pkg/storage/s3/s3.go	`85.66% <100.00%> (+0.09%)`	⬆️
pkg/extensions/search/resolver.go	`78.58% <0.00%> (-7.08%)`	⬇️
pkg/extensions/search/common/oci_layout.go	`90.56% <0.00%> (-4.86%)`	⬇️
... and 4 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

rchincha · 2022-06-22T19:06:18Z

What happens when we interface with GCP instead? is there a dynamoDB equivalent for GCP?
Would prefer to have a IO driver per cloud provider instead.

pkg/storage/dynamodb/dynamodb.go

pkg/storage/cache.go

docs/aws_databases.md

pkg/storage/s3/s3.go

peusebiu

approved by mistake

peusebiu · 2022-07-08T15:30:55Z

pkg/storage/local.go

@@ -1032,7 +1041,7 @@ func (is *ImageStoreLocal) FinishBlobUpload(repo, uuid string, body io.Reader, d

 	dst := is.BlobPath(repo, dstDigest)

-	if is.dedupe && is.cache != nil {
+	if is.dedupe && fmt.Sprintf("%v", is.cache) != fmt.Sprintf("%v", nil) {


what if we use a switch where we use type check for each implementation we know we have, now we only have dynamodatabase and boltdb, if it's not one of these types assume cache is nil

later when adding more databases we add checks for them.

Thoughts?

pkg/storage/s3/s3.go

Signed-off-by: Catalin Hofnar <catalin.hofnar@gmail.com>

Added proper endpoint handling, logging, tests Implemented more functions, added more tests, updated errors Added aws databases documentation Added DB driver and factories, need to implement Implemented Factory and Driver interfaces for dynamodb Refactor cache.go and changed some stuff Rewrote some stuff, TODO: fix PutBlob Fixed dynamoDB functions Refactor boltDB to use new interface Fixed gotcha Lint Point S3 to use DynamoDB database instead of boltdb Reverted s3 use only boltdb Added more dynamodb tests, fixed errors lint Added database factory tests, removed Scan and DescribeTable functions Added loading of db parameters from either config or env Renamed packages and moved files around to avoid cyclic import Added more coverage tests Removed env var usage for config Added benchmark tests Signed-off-by: Catalin Hofnar <catalin.hofnar@gmail.com>

Signed-off-by: Catalin Hofnar <catalin.hofnar@gmail.com>

chofnar force-pushed the dynamodb branch 5 times, most recently from af1d119 to f0e76e7 Compare June 14, 2022 15:11

chofnar force-pushed the dynamodb branch from f0e76e7 to c3d816e Compare June 14, 2022 15:17

rchincha reviewed Jun 14, 2022

View reviewed changes

chofnar force-pushed the dynamodb branch 10 times, most recently from 0016f01 to 3aaa919 Compare June 15, 2022 13:11

chofnar linked an issue Jun 15, 2022 that may be closed by this pull request

storage: root directory directly on cloud storage #564

Closed

2 tasks

chofnar force-pushed the dynamodb branch 2 times, most recently from 0d8300d to c575305 Compare June 17, 2022 12:49

chofnar force-pushed the dynamodb branch 4 times, most recently from 5f4432a to 30eacda Compare June 27, 2022 15:19

peusebiu approved these changes Jun 27, 2022

View reviewed changes

pkg/storage/dynamodb/dynamodb.go Outdated Show resolved Hide resolved

pkg/storage/cache.go Show resolved Hide resolved

docs/aws_databases.md Outdated Show resolved Hide resolved

pkg/storage/s3/s3.go Outdated Show resolved Hide resolved

peusebiu requested changes Jun 27, 2022

View reviewed changes

chofnar force-pushed the dynamodb branch from 30eacda to 5076bd4 Compare June 28, 2022 12:28

chofnar force-pushed the dynamodb branch 6 times, most recently from cde8aaa to 2c338a8 Compare July 8, 2022 12:10

peusebiu reviewed Jul 8, 2022

View reviewed changes

pkg/storage/s3/s3.go Outdated Show resolved Hide resolved

chofnar force-pushed the dynamodb branch 12 times, most recently from d40fa6e to 42f322a Compare July 15, 2022 12:18

chofnar force-pushed the dynamodb branch from 42f322a to ce8479e Compare July 21, 2022 12:28

chofnar added 4 commits July 21, 2022 16:27

Refactor cachedb code to use interface

de165ec

Signed-off-by: Catalin Hofnar <catalin.hofnar@gmail.com>

Added example config, modified documentation

45a97ea

Signed-off-by: Catalin Hofnar <catalin.hofnar@gmail.com>

Fixed error messages

8886f6f

Signed-off-by: Catalin Hofnar <catalin.hofnar@gmail.com>

chofnar force-pushed the dynamodb branch from 5175606 to 8886f6f Compare July 21, 2022 13:33

chofnar mentioned this pull request Jul 21, 2022

refactor(cache): Add database interface for blob paths + refactor BoltDB to use interface #667

Merged

chofnar closed this Jul 21, 2022

zot-core automation moved this from In progress to Done Jul 21, 2022

chofnar removed a link to an issue Jul 21, 2022

storage: root directory directly on cloud storage #564

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add database interface for blob paths + DynamoDB integration + refactor BoltDB to use interface #588

Add database interface for blob paths + DynamoDB integration + refactor BoltDB to use interface #588

chofnar commented Jun 14, 2022 •

edited

rchincha commented Jun 14, 2022

rchincha Jun 14, 2022

peusebiu Jun 16, 2022

peusebiu Jun 16, 2022

codecov bot commented Jun 15, 2022 •

edited

rchincha commented Jun 22, 2022

peusebiu left a comment

peusebiu Jul 8, 2022 •

edited

Add database interface for blob paths + DynamoDB integration + refactor BoltDB to use interface #588

Add database interface for blob paths + DynamoDB integration + refactor BoltDB to use interface #588

Conversation

chofnar commented Jun 14, 2022 • edited

rchincha commented Jun 14, 2022

rchincha Jun 14, 2022

Choose a reason for hiding this comment

peusebiu Jun 16, 2022

Choose a reason for hiding this comment

peusebiu Jun 16, 2022

Choose a reason for hiding this comment

codecov bot commented Jun 15, 2022 • edited

Codecov Report

rchincha commented Jun 22, 2022

peusebiu left a comment

Choose a reason for hiding this comment

peusebiu Jul 8, 2022 • edited

Choose a reason for hiding this comment

chofnar commented Jun 14, 2022 •

edited

codecov bot commented Jun 15, 2022 •

edited

peusebiu Jul 8, 2022 •

edited