Add remote.MultiWrite #798
Conversation
Codecov Report
@@ Coverage Diff @@
## master #798 +/- ##
==========================================
- Coverage 75.60% 74.96% -0.64%
==========================================
Files 102 103 +1
Lines 4185 4306 +121
==========================================
+ Hits 3164 3228 +64
- Misses 566 601 +35
- Partials 455 477 +22
Continue to review full report at Codecov.
|
pkg/v1/remote/multi_write.go
Outdated
| return err | ||
| } | ||
| default: | ||
| return fmt.Errorf("unknown media type: %v", desc.MediaType) |
There was a problem hiding this comment.
We need something like v1.ImageIndex.Blob to access non-manifest artifacts that are referenced by an index. Making the interface wider would be a breaking change, which is annoying, but I doubt it would affect much code outside of this repo...
|
Talked with Jon yesterday, some of this is more complex than we'd expected, since some registries (e.g., GCR) require that all of a manifest list's referenced manifests already be uploaded, so we can't just blindly copy manifests up at once, we have to do it in an order. Since a manifest list can reference other manifest lists, etc., we get a tree of them that has to be uploaded, leafs before roots. Each level can be uploaded in parallel, but has to complete before the next level up starts. I'm adding this check to our test registry in |
This method takes multiple images or manifest lists, and attempts to efficiently upload all of them in parallel, deduplicating shared layer and config blobs and avoiding unnecessary auth token exchanges. Blobs are uploaded in parallel first, then manifests are tagged in order with as much parallelism as we can support. This method is currently limited in that: - all images must be getting uploaded to the same repository (to simplify auth and transports), and - no image may contain streaming layers (to simplify digests), but both of these can be removed in future changes. - blobs are uploaded and manifests are tagged in parallel with a constant max parallelism, which should be a user-controlled option.
| @@ -23,6 +23,9 @@ import ( | |||
| "net/http" | |||
| "strings" | |||
| "sync" | |||
|
|
|||
| v1 "github.com/google/go-containerregistry/pkg/v1" | |||
There was a problem hiding this comment.
I'd prefer it if we inlined some of this stuff to avoid taking a dependency outside of the stdlib for this package, but I might just fix it myself before we cut a new release.
| // list's constituent manifests are already uploaded. | ||
| // This isn't strictly required by the registry API, but some | ||
| // registries require this. | ||
| if mf.contentType == string(types.OCIImageIndex) || |
There was a problem hiding this comment.
86.20% <7.69%> (-13.80%)
This used to have 100% coverage 😄
| // Current limitations: | ||
| // - All refs must share the same repository. | ||
| // - Images cannot consist of stream.Layers. | ||
| func MultiWrite(m map[name.Reference]Taggable, options ...Option) error { |
There was a problem hiding this comment.
This API is good. I have some issues with the implementation and limitations, which we've discussed offline. Given that the interface is correct, I'm fine with merging this and improving it as we have time.
This method takes multiple images or manifest lists, and attempts to
efficiently upload all of them in parallel, deduplicating shared layer
and config blobs and avoiding unnecessary auth token exchanges.
Blobs are uploaded in parallel first, then manifests are tagged in order
with as much parallelism as we can support.
This method is currently limited in that:
simplify auth and transports), and
of these can be removed in future changes.
constant max parallelism, which should be a user-controlled option.