Repository navigation
Conversation
Allow [resolver.host."*"] to configure mirrors for every registry without its own entry, matching the "*" mirror in containerd's CRI registry config. Registry mirrors running on the node, such as Spegel, are usually configured this way. Use plain HTTP when a mirror host is given with an http:// scheme, instead of silently switching it to https unless insecure = true is set. A host given without a scheme defaults to https; previously a host:port without a scheme failed to parse and broke host resolution for the image. Signed-off-by: Kiril Angov <kangov@seatgeek.com>
github-actions
Bot
added
go
Parallel pull built a single remote store for the image registry and only took the HTTP client from the first resolved host, so layers were always fetched from the image registry even when [resolver.host] mirrors were configured. Build one blob source per resolved host (mirrors first, then the image registry). For each layer, probe the mirrors in order with a single HEAD request and fetch from the first one that has the blob, falling back to the image registry. If the download fails, retry it from the next host; the layer is only unpacked and verified after the download completes, so decompression and unpack errors still fail immediately. Requests to mirrors carry the ns=<registry> query parameter, as containerd sends it. The image registry keeps its own scheme: an insecure mirror no longer makes the image registry plain HTTP, unless the registry is configured as an insecure mirror of itself. Mirrors with a custom path are skipped, since blob URLs are always built under /v2. This lets node-local registry mirrors such as Spegel serve layers to parallel pull. Signed-off-by: Kiril Angov <kangov@seatgeek.com>
4b4b73b to
9fd00ac
Compare
|
For context on overlap with other work:
What this PR adds on top of both: |
sondavidb
left a comment
sondavidb
left a comment
There was a problem hiding this comment.
Doing a quick first pass, will look closer on Monday, couple of concerns but this will be great once it's merged. Thanks!
| // download the target layer | ||
| s := src[0] | ||
| client := s.Hosts[0].Client | ||
| if len(s.Hosts) == 0 { |
There was a problem hiding this comment.
We can probably also apply this logic in MountLocal?
There was a problem hiding this comment.
MountLocal now builds the mirror and origin stores with newBlobSources and picks one with selectBlobSource. One limitation I see is that MountLocal doesn't retry on download failure the way the parallel path does, so a mirror that has the layer but fails mid-download won't fall back to the next host.
I can add that if you want it in this PR?
There was a problem hiding this comment.
a mirror that has the layer but fails mid-download won't fall back to the next host
Hm, that's a good point. I think this would be a good followup PR then, since I think we should separate this behavioral change. Though I suppose two commits for this PR would also suffice.
There was a problem hiding this comment.
Let me know which way you prefer because this change is already getting somewhat big and I do not want it to get held up on unrelated change. I can still open a new PR for it, that's not the problem :)
Probe mirrors with GetHeader so registries that reject HEAD still work, apply mirror selection in MountLocal, fold blobSource into orasBlobStore, drop 'parallel pull' from the custom-path log, and leave the documented mirror insecure default untouched. Signed-off-by: Kiril Angov <kangov@seatgeek.com>
|
CI looks to be failing on the new test |
GetHeader makes three requests on a miss. A 404 to the HEAD probe is final, so only fall back to GetHeader for other statuses, such as registries that do not allow HEAD. Signed-off-by: Kiril Angov <kangov@seatgeek.com>
(Hopefully) Fixed the CI failure and |
This branch has not been deployed
2 participants
Issue #, if available: Fixes #2094 (mirror part). Related to #1013 (same root cause, for SOCI artifacts instead of parallel pull layers)
Description of changes:
Parallel pull (
parallel_pull_unpack) fetched every layer from the image registry, even when[resolver.host]mirrors were configured.preloadAllLayerscreated a single ORAS store for the image registry and only reused the HTTP client of the first resolved host.This PR makes parallel pull use the configured mirrors:
service/resolver)[resolver.host."*"]applies to every registry without its own entry, like the"*"mirror in containerd's CRI registry config. Node-local mirrors such as Spegel are usually configured this way.http://scheme uses plain HTTP; before, it was switched to https unlessinsecure = truewas also set.mirror.example.com:5000) defaults to https; before,host:portwithout a scheme failed to parse and broke host resolution for the image.fs)HEADrequest; the layer is fetched from the first mirror that has it, otherwise from the image registry. Probing runs inside each layer's premount goroutine, so it is parallel.ns=<registry>query parameter, as containerd sends it, so a mirror can tell which registry the image belongs to.isInsecureHostmakes the image registry plain HTTP whenever any of its mirrors isinsecure = true; on this path that only happens if the registry is configured as an insecure mirror of itself./v2).Docs:
docs/config.md(hostscheme handling,"*", and theinsecuredefault, which isfalsein code) anddocs/parallel-mode.md(Mirrors).Testing performed:
TestAsRegistryHostsMirrors(wildcard, precedence, http scheme, scheme-less host);TestSelectBlobSource*(httptest mirror and origin: hit on mirror, miss falls back to origin, no probes without mirrors);TestNewBlobSourcesOriginScheme;TestParallelFetchFromMirrorWithFallback(mirror serves the layer; mirror GET fails and the image registry serves it;nssent only to the mirror). The resolver tests fail without the resolver change, and the fallback test fails with the fallback disabled.go test -racefor all non-integration packages andgolangci-lint run(v2.13.0): pass.http://<node>:30031(Spegel) first, then the ECR endpoint.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.