8000 Azure driver retry fix by milosgajdos · Pull Request #4576 · distribution/distribution · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Azure driver retry fix #4576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 14, 2025
Merged

Conversation

milosgajdos
Copy link
Member
@milosgajdos milosgajdos commented Feb 23, 2025

Most of this is a copy-pasta from the fix in GitLab container registry introduced by @vespian

  • fix the data corruption in Azure
  • refactor Azure storage driver client
  • update Azure storage driver docs
  • add Azure storage driver integration tests (!!!)
Click me!
=== RUN   TestAzureDriverSuite
=== RUN   TestAzureDriverSuite/TestConcurrentFileStreams
=== RUN   TestAzureDriverSuite/TestConcurrentStreamReads
=== RUN   TestAzureDriverSuite/TestContinueStreamAppendLarge
=== RUN   TestAzureDriverSuite/TestContinueStreamAppendSmall
=== RUN   TestAzureDriverSuite/TestDelete
=== RUN   TestAzureDriverSuite/TestDeleteFolder
=== RUN   TestAzureDriverSuite/TestDeleteNonexistent
=== RUN   TestAzureDriverSuite/TestDeleteOnlyDeletesSubpaths
=== RUN   TestAzureDriverSuite/TestInvalidPaths
=== RUN   TestAzureDriverSuite/TestList
=== RUN   TestAzureDriverSuite/TestMove
=== RUN   TestAzureDriverSuite/TestMoveInvalid
=== RUN   TestAzureDriverSuite/TestMoveNonexistent
=== RUN   TestAzureDriverSuite/TestMoveOverwrite
=== RUN   TestAzureDriverSuite/TestPutContentMultipleTimes
=== RUN   TestAzureDriverSuite/TestReadNonexistent
=== RUN   TestAzureDriverSuite/TestReadNonexistentStream
=== RUN   TestAzureDriverSuite/TestReaderWithOffset
=== RUN   TestAzureDriverSuite/TestRedirectURL
=== RUN   TestAzureDriverSuite/TestRootExists
=== RUN   TestAzureDriverSuite/TestStatCall
=== RUN   TestAzureDriverSuite/TestTruncate
=== RUN   TestAzureDriverSuite/TestValidPaths
=== RUN   TestAzureDriverSuite/TestWriteRead1
=== RUN   TestAzureDriverSuite/TestWriteRead2
=== RUN   TestAzureDriverSuite/TestWriteRead3
=== RUN   TestAzureDriverSuite/TestWriteRead4
=== RUN   TestAzureDriverSuite/TestWriteReadLargeStreams
=== RUN   TestAzureDriverSuite/TestWriteReadNonUTF8
=== RUN   TestAzureDriverSuite/TestWriteReadStreams1
=== RUN   TestAzureDriverSuite/TestWriteReadStreams2
=== RUN   TestAzureDriverSuite/TestWriteReadStreams3
=== RUN   TestAzureDriverSuite/TestWriteReadStreams4
=== RUN   TestAzureDriverSuite/TestWriteReadStreamsNonUTF8
=== RUN   TestAzureDriverSuite/TestWriteZeroByteContentThenAppend
=== RUN   TestAzureDriverSuite/TestWriteZeroByteStreamThenAppend
--- PASS: TestAzureDriverSuite (51.50s)
    --- PASS: TestAzureDriverSuite/TestConcurrentFileStreams (3.63s)
    --- PASS: TestAzureDriverSuite/TestConcurrentStreamReads (1.34s)
    --- PASS: TestAzureDriverSuite/TestContinueStreamAppendLarge (0.08s)
    --- PASS: TestAzureDriverSuite/TestContinueStreamAppendSmall (0.01s)
    --- PASS: TestAzureDriverSuite/TestDelete (0.00s)
    --- PASS: TestAzureDriverSuite/TestDeleteFolder (0.01s)
    --- PASS: TestAzureDriverSuite/TestDeleteNonexistent (0.00s)
    --- PASS: TestAzureDriverSuite/TestDeleteOnlyDeletesSubpaths (0.02s)
    --- PASS: TestAzureDriverSuite/TestInvalidPaths (0.00s)
    --- PASS: TestAzureDriverSuite/TestList (0.15s)
    --- PASS: TestAzureDriverSuite/TestMove (0.01s)
    --- PASS: TestAzureDriverSuite/TestMoveInvalid (0.00s)
    --- PASS: TestAzureDriverSuite/TestMoveNonexistent (0.00s)
    --- PASS: TestAzureDriverSuite/TestMoveOverwrite (0.01s)
    --- PASS: TestAzureDriverSuite/TestPutContentMultipleTimes (0.01s)
    --- PASS: TestAzureDriverSuite/TestReadNonexistent (0.00s)
    --- PASS: TestAzureDriverSuite/TestReadNonexistentStream (0.00s)
    --- PASS: TestAzureDriverSuite/TestReaderWithOffset (0.01s)
    --- PASS: TestAzureDriverSuite/TestRedirectURL (0.01s)
    --- PASS: TestAzureDriverSuite/TestRootExists (0.00s)
    --- PASS: TestAzureDriverSuite/TestStatCall (15.05s)
    --- PASS: TestAzureDriverSuite/TestTruncate (0.04s)
    --- PASS: TestAzureDriverSuite/TestValidPaths (0.11s)
    --- PASS: TestAzureDriverSuite/TestWriteRead1 (0.01s)
    --- PASS: TestAzureDriverSuite/TestWriteRead2 (0.01s)
    --- PASS: TestAzureDriverSuite/TestWriteRead3 (0.01s)
    --- PASS: TestAzureDriverSuite/TestWriteRead4 (0.02s)
    --- PASS: TestAzureDriverSuite/TestWriteReadLargeStreams (30.88s)
    --- PASS: TestAzureDriverSuite/TestWriteReadNonUTF8 (0.01s)
    --- PASS: TestAzureDriverSuite/TestWriteReadStreams1 (0.01s)
    --- PASS: TestAzureDriverSuite/TestWriteReadStreams2 (0.01s)
    --- PASS: TestAzureDriverSuite/TestWriteReadStreams3 (0.01s)
    --- PASS: TestAzureDriverSuite/TestWriteReadStreams4 (0.01s)
    --- PASS: TestAzureDriverSuite/TestWriteReadStreamsNonUTF8 (0.01s)
    --- PASS: TestAzureDriverSuite/TestWriteZeroByteContentThenAppend (0.01s)
    --- PASS: TestAzureDriverSuite/TestWriteZeroByteStreamThenAppend (0.01s)
=== RUN   TestCommitAfterMove
--- PASS: TestCommitAfterMove (0.11s)
=== RUN   TestParamParsing
--- PASS: TestParamParsing (0.00s)
PASS
ok      github.com/distribution/distribution/v3/registry/storage/driver/azure   52.006s```

</details>

We might also bump Azure SDK _in the future PR_: I tried doing it in this PR but the diff is bonkers, so I opted to do it in the future PR; we now _finally_ have integration tests for Azure driver so bumping the SDK should be easier in terms of verifying the driver's basic functionality!

Closes #4571

PTAL @thaJeztah @Jamstah @wy65701436 @squizzi 

@vespian
Copy link
vespian commented Feb 23, 2025

FWIW, since last time we have spoken, I have also added ETag support for better consistency guarantees in Writer() . See https://gitlab.com/gitlab-org/container-registry/-/blob/856d0c6063cd19643c158f1e9c0a69c68c88301d/registry/storage/driver/azure/v2/azure.go#L783 for the latest and greatest version of the code.

@milosgajdos milosgajdos marked this pull request as ready for review March 9, 2025 19:25
@milosgajdos milosgajdos changed the title [WIP] Azure driver retry fix Azure driver retry fix Mar 9, 2025
Copy link
Collaborator
@davidspek davidspek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given it's mostly a copy/paste LGTM.

* Make copy poll max retry, a global driver max retry
* Get support for etags in Azure
* Fix storage driver tests
* Fix auth mess and update docs
* Refactor Azure client and enable Azure storage tests

We use Azurite for integration testing which requires TLS,
so we had to figure out how to skip TLS verification when running tests locally:
this required updating testsuites Driver and constructor due to TestRedirectURL
sending GET and HEAD requests to remote storage which in this case is Azurite.

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
@milosgajdos milosgajdos merged commit ebd20d3 into distribution:main Mar 14, 2025
21 checks passed
@jayaddison
Copy link

Hi @milosgajdos - was this intended to close/resolve bug #4571? (the text Closes #4571 appears within the detailed notes in the pull request description, but it appears that GitHub did not link this PR to that issue)

@milosgajdos
Copy link
Member Author

Thanks @jayaddison, just closed it

@jayaddison
Copy link

You're welcome - thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0