Skip to content
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

Define an error to allow an external reference #1029

Closed

Conversation

zhangguanzhang
Copy link

Fixes: containers/podman#7337
more detail could see containers/podman#7337 (comment)
Signed-off-by: zhangguanzhang [email protected]

@rhatdan
Copy link
Member

rhatdan commented Aug 19, 2020

@vrothberg @mtrmac PTAL

@vrothberg
Copy link
Member

I think we need to fix it in podman.

$ podman load -i FOO.tar name:tag is supposed to load the image in FOO.tar and the tag it as name:tag.

@zhangguanzhang
Copy link
Author

I think we need to fix it in podman.

$ podman load -i FOO.tar name:tag is supposed to load the image in FOO.tar and the tag it as name:tag.

you could see this containers/podman#7337 (comment)
I use the dlv to debug this and found the reason, so I commit this pr

Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m afraid I have no idea what is the problem, the solution, or how this makes any sense.

  • Why make a check for len(index.Manifests), and use the [0] member inside a loop iterating through array?
  • If the user asks for a specific ref.image, how does it make sense to return an entry that does not have that name?
  • How does it make sense for a “find” operation to edit the data?

If this is to influence Podman’s heuristics for deciding how to tag the result of a pull, those are entirely up to Podman and c/image is trying fairly hard to stay away.

@zhangguanzhang
Copy link
Author

I change it,I need this error type to judge in podman

@zhangguanzhang zhangguanzhang changed the title fix podman load oci-archive use name Define an error to allow an external reference Aug 19, 2020
@mtrmac
Copy link
Collaborator

mtrmac commented Aug 19, 2020

I’m afraid I have no idea what is the problem, the solution

I can’t see how this relates to the linked Podman issue at all. If Podman is using file names instead of other data it was supplied, why can’t it just stop doing that?

@zhangguanzhang
Copy link
Author

zhangguanzhang commented Aug 19, 2020

$ podman pull alpine
$ podman save --format oci-archive alpine:latest >| /tmp/FOO.tar

this will be ok

$ podman load -i /tmp/FOO.tar 

and this image name will be docker.io/library/alpine:latest, it will run into this line https://github.com/containers/image/blob/master/oci/layout/oci_transport.go#L190

but if use a name to override, afer this , the image's name should be localhost/foo:latest

$ podman load -i /tmp/FOO.tar foo 
Error: error pulling "foo": unable to pull dir:/tmp/FOO.tar: error determining pull goal for image "dir:/tmp/FOO.tar": error parsing dest reference name "localhost/tmp/FOO.tar": error parsing named reference "localhost/tmp/FOO.tar": invalid reference format: repository name must be lowercase

it does not run ok in the function in https://github.com/containers/podman/blob/master/libpod/runtime_img.go#L288-L294
Because in this case, https://github.com/containers/image/blob/master/oci/layout/oci_transport.go#L205 ref.image is localhost/foo:latest, it is not equal to docker.io/library/alpine:latest

@mtrmac
Copy link
Collaborator

mtrmac commented Aug 19, 2020

Because in this case, https://github.com/containers/image/blob/master/oci/layout/oci_transport.go#L205 ref.image is localhost/foo:latest, it is not equal to docker.io/library/alpine:latest

So what’s actually wrong about that situation? If the ref is for $path:foo, and foo does not exist in the index, that’s that.

  • As far as the end user is concerned, what should happen instead?
  • How does that difference in actual vs. expected behavior manifest at the c/image API boundary?
  • How does adding a recognizable error type help? What is Podman going to do with that error? (I guess “try again with some other reference — but then why is it trying two ways instead of just the correct way?)

(Perhaps see also the conversation around containers/podman#6811 (comment) , but I suspect the questions I have are at a more basic level.)

@zhangguanzhang
Copy link
Author

I think when len(index.Manifests) == 1 and ref.image != "", the image's name will be the ref.image after load the oci-archive file

@rhatdan
Copy link
Member

rhatdan commented Aug 28, 2020

@mtrmac What is your latest feeling on this?

@mtrmac
Copy link
Collaborator

mtrmac commented Aug 28, 2020

containers/podman#7386 (comment) – I don’t really understand what’s going on, even what’s wrong, my best guess is that Podman is internally confused about the meaning of the “name” parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman load -i Path-With-Caps label : invalid reference format: repository name must be lowercase
4 participants