-
Notifications
You must be signed in to change notification settings - Fork 395
[WIP] Replace docker.io as the default registry with registries[0] #449
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
Conversation
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
|
This is an alternative to #448 |
|
This is more in line with what I was thinking you were going to do. Looks like you've a whitespace error somewhere. I'd go forward with this and kill the earlier one. |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if len(registries) == 0 { |
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.
Should we default to quay.io in this case? Or fedora?
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.
I really don't think the containers/image should have a default. In the perfect world we would just allow a "" as the registry.
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.
A minimal comment for now:
c/i/docker/reference.Named, !IsNameOnly, is a type representing a specific image in a specific registry.
Maybe there needs to be a type representing the “unqualified” inputs to search.
Maybe some uses of reference.Named should not use it at all, and accept more lax input. (In particular I was wrong about containers-storage:, it uses reference.ParseNormalizedNamed and does not allow hostname-less references.)
But, if hostname-qualified and hostname-less references should live side by side, they need to be distinct types. The experience, both with projectatomic/docker’s reference, and with podman’s string uses, is that mixing them together makes it all to easy to be careless about the semantics and results in surprising and unexpected corner case behaviors.
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.
Ok, I would like you to work on the solution for this, We need to get a way to allow us to have images stored without requiring a default registry.
| return nil, err | ||
| } | ||
| if len(registries) == 0 { | ||
| return nil, errors.New("no registries specified") |
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.
"no registries specified in /etc/containers/registries.conf" perhaps?
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.
The problem is I am not sure if that is the path.
|
I think I prefer the original. No registry at all. |
Signed-off-by: Daniel J Walsh dwalsh@redhat.com