Skip to content

Multipart download enhancements#4021

Merged
andaaron merged 2 commits into
project-zot:mainfrom
andaaron:multipart
Apr 28, 2026
Merged

Multipart download enhancements#4021
andaaron merged 2 commits into
project-zot:mainfrom
andaaron:multipart

Conversation

@andaaron

Copy link
Copy Markdown
Contributor

As a follow-up to #3995, to include enhancements in #3976

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

Blob HEAD responses had no Content-Type and GET responses echoed the
request's Accept header verbatim, which produced missing or malformed
media types and left multipart/byteranges parts without a per-part
Content-Type. This breaks OCI distribution-spec conformance and
consumers like stargz-snapshotter that need a well-formed layer media
type.

Add a blobResponseMediaType helper that resolves the descriptor's
MediaType via GetBlobDescriptorFromRepo and falls back to
application/octet-stream. Use it in CheckBlob (HEAD), GetBlob full
(200), GetBlob single-range (206), and per-part in
writeMultipartRanges (206 multipart). Lookup is deferred until after
the blob is known to exist.

Cover the new behaviour with mock-based unit tests in routes_test.go
and end-to-end assertions in TestPullRange.

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
@andaaron
andaaron requested a review from Copilot April 27, 2026 07:54
@andaaron andaaron changed the title Multipart download changes. Multipart download enhancements Apr 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Follow-up to earlier multipart range blob pull work, improving blob download responses to be descriptor-aware for Content-Type and optimizing multipart range delivery to reduce concurrent reader usage.

Changes:

  • Add descriptor-aware Content-Type resolution for blob HEAD/GET with application/octet-stream fallback.
  • Rework multipart range responses to stream ranges lazily (one reader at a time) and advertise a precomputed Content-Length.
  • Expand test coverage for descriptor-derived Content-Type, multipart part headers, streaming behavior, and truncation on mid-stream read errors.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
pkg/api/routes.go Adds blobResponseMediaType, sets Content-Type for blob responses, and rewrites multipart range streaming to be lazy with precomputed Content-Length.
pkg/api/routes_test.go Adds unit tests for descriptor-aware Content-Type and multipart range response behavior (per-part headers, content-length correctness, sequential reader opens, truncation on error).
pkg/api/controller_test.go Extends existing range-pull integration tests to assert correct fallback/per-part Content-Type behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/api/routes.go Outdated
Comment thread pkg/api/routes.go Outdated
Comment thread pkg/api/routes_test.go
@codecov

codecov Bot commented Apr 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.69767% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.48%. Comparing base (bb5b77a) to head (c2987f6).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/api/routes.go 90.69% 4 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4021      +/-   ##
==========================================
+ Coverage   91.39%   91.48%   +0.09%     
==========================================
  Files         197      197              
  Lines       28308    28353      +45     
==========================================
+ Hits        25872    25940      +68     
+ Misses       1581     1561      -20     
+ Partials      855      852       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/api/routes.go
Comment thread pkg/api/routes.go Outdated
Comment thread pkg/api/routes.go Outdated
…nt-Length

writeMultipartRanges previously opened every range reader up front
and emitted no Content-Length, so an N-range request held N
concurrent storage readers (and their fds / read buffers) per
response window and forced chunked encoding on HTTP/1.1 — neither
friendly to proxies nor to fan-out scenarios like stargz lazy pulls.

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@andaaron
andaaron marked this pull request as ready for review April 27, 2026 17:28
@andaaron
andaaron requested a review from rchincha as a code owner April 27, 2026 17:28
@andaaron

Copy link
Copy Markdown
Contributor Author

@jezell I have added the enhancements in #3976 on top of main (after #3995 was merged).
@jezell, @invy, do you want to try it out?

@rchincha

Copy link
Copy Markdown
Contributor

code coverage is dropping and trending in the wrong direction :(

@andaaron

Copy link
Copy Markdown
Contributor Author
image Something is wrong with coverage reporting in this case.

@andaaron
andaaron merged commit 1a1f84d into project-zot:main Apr 28, 2026
48 of 49 checks passed
@andaaron
andaaron deleted the multipart branch April 28, 2026 05:43
Comment thread pkg/api/routes.go
Comment on lines +1034 to +1035
// distribution-spec conformance and consumers like stargz-snapshotter that
// require a non-empty, well-formed media type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: specific tools probably shouldn't be mentioned in the source code as the implementation is aligned to dist-spec.

Suggested change
// distribution-spec conformance and consumers like stargz-snapshotter that
// require a non-empty, well-formed media type.
// distribution-spec conformance that
// requires a non-empty, well-formed media type.

@invy

invy commented May 26, 2026

Copy link
Copy Markdown

@jezell I have added the enhancements in #3976 on top of main (after #3995 was merged). @jezell, @invy, do you want to try it out?

We've tested newest ZOT 2.1.17 release, our stargz and zstd-chunked images work now perfectly (and reduce pull times significantly) :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants