-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Fix manifest list never being stored in proxy-cache projects #19910
Conversation
e4c10b5
to
fa10d89
Compare
fa10d89
to
240af51
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting observation, and hard to spot edge case. Do you have a step-by-step guide on how to reproduce the bug?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19910 +/- ##
==========================================
- Coverage 67.56% 67.45% -0.11%
==========================================
Files 991 996 +5
Lines 109181 109777 +596
Branches 2719 2720 +1
==========================================
+ Hits 73768 74051 +283
- Misses 31449 31744 +295
- Partials 3964 3982 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
|
We detected this because we have a Harbor / Docker-registry setup that is sensitive the filesystem layout that Harbor owns. We have a primary Harbor that runs in one primary kubernetes cluster. We configure Harbor to use S3 as the filesystem, and we have other clusters which run docker-registry's which are given limited read access out of that S3 bucket. Those docker-registry can read Harbor's filesystem, because both are the same fundamental component, and so registries are affected by the filesystem, rather than the contents of Harbor's middleware (redis, in our case). In our case, we detected that manifest-list images were only partially mirrored, that the manifest layers pointed to by the manifests in the manifest-list were copied into the S3 bucket, but the manifest-list itself was absent. To give a concrete example, if I inspect the current ubuntu focal manifest-list, it returns something like this:
When we made requests to Harbor it would have no problem proxying the ubuntu:focal manifest-list and containers to the client, and the S3 bucket would contain the architecture of the image we requested, for example, the amd arch and "sha256:48c35f3de33487442af224ed4aabac19fd9bfbd91ee90e9471d412706b20ba73". This is expected, but what was missing is that the manifest-list wasn't stored. So pulling directly from the S3 bucket failed, but pulling through Harbor succeeded, even though Harbor was replying with layers that pointed at the S3 bucket. Harbor was caching the container layers, but not the manifest-list. This confused me initially because when I read the Harbor proxy cache code, the code seems to explicitly handle manifest-lists correctly. But when I inspected the layers in the docker-registry after pulling a manifest-list image for the first time through the proxy cache, you will find the layers for the inner container, but not the manifest list or any of the file pointers created for that resource. And the reason I found for this is that the docker client at some point in the past, used to make a request flow for an image that looked something like this: client -> HEAD $MANIFEST, client -> GET $MANIFEST, client fetches the layers. And when docker added manifest-lists, they added a step of indirection on the client side. So the client -> HEAD $MANIFESTLIST, then client GET -> $MANIFEST, and never executes a get on a manifest list resource directly. The client uses the list to pick out it's arch specific manifest and never refers to the list again. Harbor never sees the GET request on the manifest-list, and never saves the manifest list to disk. |
…k up the manifest in case it was a manifest list request, as there will be no subsequent GET request Signed-off-by: Cameron McAvoy <[email protected]>
240af51
to
c859215
Compare
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
This PR was closed because it has been stalled for 30 days with no activity. If this PR is still relevant, please re-open a new PR against main. |
Fix manifest list never being stored in proxy-cache projects by looking up the manifest on HEAD requests
Comprehensive Summary of your change
My understanding is that when docker (or containerd) requests a manifest list in a proxy-cache project, the client never makes a GET request for the manifest list. It makes a HEAD request for the manifest list, then makes a GET request for the platform architecture specific digest. As a result, the manifest list is never persisted in the storage layer, only in the redis cache layer. When the cache expires, this forces a new remote lookup for future requests instead of being able to rely on the storage, which is the purpose of the proxy-cache.
Issue being fixed
This may or may not also fix #19609, but it does seem related.
Please indicate you've done the following: