Allow registry_mirrors to apply to unofficial registries.#32771
Allow registry_mirrors to apply to unofficial registries.#32771Syntaxide wants to merge 1 commit intomoby:masterfrom
Conversation
c9ed7b9 to
26b8c14
Compare
There was a problem hiding this comment.
It is a panic?
And will it be too strict?
There was a problem hiding this comment.
Panic seemed reasonable to me, since mirrors are only read when the daemon starts up. The alternative seems to be failing silently, and ignoring invalid mirrors. What do you think is the best way to handle this?
There was a problem hiding this comment.
Just to check whether LoadMirror will be called when a user reloads a daemon's config ?
There was a problem hiding this comment.
Looks like the "TestDaemonReloadMirrors " test that was removed was to allow reloading mirrors at runtime (signup)
There was a problem hiding this comment.
FWIW, I don't think we panic in other cases like this; I don't think a panic is really suitable here; we validated the format, and the format was invalid, so the error should be printed, but a full dump
|
Looks like there's a gofmt issue; |
|
I'm -1 on introducing a new micro format for this feature, and would rather have we'd either use the advanced (CSV) syntax, e.g. The CSV-syntax could easily be made backward-compatible with the "old" approach (although erroring out if the new format is used should not be a problem). |
26b8c14 to
98eb71b
Compare
Tested with unit tests, and ran locally: registry_mirrors="someregistry.com->index.docker.io", and docker pull someregistry.com/library/ubuntu succeeded. Signed-off-by: Alexander Midlash <[email protected]>
98eb71b to
9904a72
Compare
|
@thaJeztah How would you want to specify multiple registry mirrors? |
|
@Syntaxide the flag would be specified multiple times then; |
|
@thaJeztah What do you think is the best approach for handling the json? I see the approach taken in 273eeb8 works well for the command line flags. (un)fortunately the other commit did not impact the daemon.json, so I'm not sure what the best approach here is. It seems we'll need to add additional behavior wherever we unmarshal the daemon.json. What would you propose as the type for ServiceOptions.Mirrors? I'm tempted to use json.RawMessage, which we can either continue to unmarshal, or just treat as the raw bytes in the case where the value isn't json. Is there a more elegant approach that I am missing? |
|
Discussed offline. This functionality is better handled by deploying a proxy which has these forwarding rules. This does not need to be implemented in the engine. |
|
@Syntaxide Can you explain more for the conclusion? I believe this is one very important missing feature in Docker. Thanks |
|
@denverdino |
|
We have exactly the requirement for the local cache in remote Data Center for the centralized docker registry in corp headquarter. The HTTP proxy approach is OK, but it is hard for auth and handling the private image. Another case is user cannot access Docker images from gcr.io. The local mirror with some VPN can solve the problem. Pls consider those requirements Thanks a lot |
Tested with unit tests, and ran locally:
registry_mirrors="someregistry.com->index.docker.io", and docker pull
someregistry.com/library/ubuntu succeeded.
Signed-off-by: Alexander Midlash [email protected]
- What I did
Allow registry_mirrors flag to additionally set mirrors for unofficial registries.
- How I did it
For backwards compatibility, registry_mirrors entries can either be old-style ("some-host"), or new style ("original-host->new-mirror"). All mirroring information is stored in the IndexConfig, so that we can set mirrors per index.
- How to verify it
Besides unit tests, try setting registry_mirrors="somedomain.com->index.docker.io"
You can then `docker pull somedomain.com/library/ubuntu" and receive the image from the official registry.
In practice, this would be used to redirect from an organizations private registry, to per-datacenter caches.
- Description for the changelog
Allow registry_mirrors to apply to unofficial registries.
- A picture of a cute animal (not mandatory but encouraged)