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

getting image ID (sha) takes longer than docker cli #627

Closed
medyagh opened this issue Dec 10, 2019 · 9 comments · Fixed by #1121
Closed

getting image ID (sha) takes longer than docker cli #627

medyagh opened this issue Dec 10, 2019 · 9 comments · Fixed by #1121

Comments

@medyagh
Copy link
Member

medyagh commented Dec 10, 2019

I am trying to find a cheap way to get the image SHA, if I do docker inspect it takes 0.07 s

$ time docker inspect --format='{{.Id}}' ubuntu
sha256:775349758637aff77bf85e2ff0597e86e3e859183ef0baba8b3e8fc8d3cba51c

real	0m0.073s
user	0m0.035s
sys	0m0.033s

however if I use go-containerregistry it will take 4s,

ref, _ := name.ParseReference("ubuntu", name.WeakValidation)
img, _ := daemon.Image(ref)
m, _ := img.Manifest()

is there any cheaper way to get the image ID using go-containerregistry ?

I also tried crane.Digest that still takes 2.1 seconds.

fmt.Println(crane.Digest("ubuntu"))

Update: I also tried getting it through config but it is still 1.95 seconds

code:

	ref, _ := name.ParseReference("ubuntu", name.WeakValidation)
	img, _ := daemon.Image(ref)
	cf, _ := img.ConfigName()
	fmt.Println(cf)

time

$ time ./example1 
sha256:775349758637aff77bf85e2ff0597e86e3e859183ef0baba8b3e8fc8d3cba51c

real    0m1.954s

This would be an important improvment for iterative development on minikube. when users constantly rebuild an image and push.

@medyagh medyagh changed the title crane.Digest() takes longer than docker crane.Digest() takes longer than docker inspect Dec 10, 2019
@jonjohnsonjr
Copy link
Collaborator

is there any cheaper way to get the image ID using go-containerregistry ?

It might make sense to use the docker client directly? There's ImageInspectWithRaw, which I believe does what you want. We just thinly wrap the docker client library to make access slightly easier. Our daemon package isn't particularly optimized, because I mostly use it to load images into the daemon.

The reason this is really slow is that our daemon.Image implementation is very naive. We essentially take the output of docker save and parse it as a tarball.Image, which is really inefficient if all you care about is the Image ID.

Something like this should work:

func getImageID(ctx context.Context, ref string) (string, error) {
	cli, err := client.NewClientWithOpts(client.FromEnv)
	if err != nil {
		return "", err
	}
	cli.NegotiateAPIVersion(ctx)
	img, err := cli.ImageInspectWithRaw(ctx, ref)
	if err != nil {
		return "", err
	}
	return img.ID, nil
}

Hopefully that's sufficient for what you're trying to do? If you're trying to use go-containerregistry to accomplish something more complicated, and this is slowing everything down, we could also look at optimizing that use case by specializing the ConfigName method (and maybe Digest), but I'm reluctant to do that unless it's necessary.

@medyagh
Copy link
Member Author

medyagh commented Dec 10, 2019

@jonjohnsonjr I was hoping to not depend on docker cli exist in the path. I need this for implementing kic (kuberntes in container) in minikube kubernetes/minikube#5987

in which we will need a faster way of loading image from host to the minikube node (but avoid loading the duplicate images don't load if sha exists)

@medyagh medyagh changed the title crane.Digest() takes longer than docker inspect getting image ID (sha) takes longer than docker inspect Dec 10, 2019
@medyagh medyagh changed the title getting image ID (sha) takes longer than docker inspect getting image ID (sha) takes longer than docker cli Dec 10, 2019
@jonjohnsonjr
Copy link
Collaborator

I was hoping to not depend on docker cli exist in the path

I don't think the snippet I posted above requires the CLI on the path, just the github.com/docker/docker/client dependency, which it looks like minikube already depends on.

in which we will need a faster way of loading image from host to the minikube node (but avoid loading the duplicate images don't load if sha exists)

This is an interesting problem that's particularly bad with docker: moby/moby#38446

IMO the cleanest way to solve this is to run a registry in-cluster to avoid having to serialize images to tarballs in the first place... some related discussion in kind: kubernetes-sigs/kind#602

@medyagh
Copy link
Member Author

medyagh commented Dec 10, 2019

@jonjohnsonjr thanks for the response, yeah we like to provide both ways of registry and loading the image for various reasons, such as offline minikube experience,...

if I use the docker cli lib, I wonder if that would work nice with users who use crio or podman as their default OCI .... I doubt that docker support it. (but my undrestanding is go-containerregistery) would would work regardless of that

@jonjohnsonjr
Copy link
Collaborator

I wonder if that would work nice with users who use crio or podman as their default OCI

Probably not, I'm sure you'd have to do something different for each runtime

@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@jonjohnsonjr
Copy link
Collaborator

@medyagh it looks like you worked around this. Is this still something you want fixed?

@afbjorklund
Copy link
Contributor

afbjorklund commented Jan 2, 2021

We still need to do it, but will not use docker daemon - since it might not be available.

Most likely with some file hack, with deploying a registry server as the "real" solution.

i.e. store the digest in a separate file, when available

busybox@sha256:49dae530fd5fee674a6b0d3da89a380fc93746095e7eca0f1b70188a95fd5d71

and cache the id, to not have to recompute it from tar

sha256:a77dce18d0ecb0c1f368e336528ab8054567a9055269c07c0169cba15aec0291"

and resolve the server, to avoid issues with non-docker runtimes

docker.io/library/busybox:latest

and then add the platform to the path, to avoid arch collisions

linux/amd64/docker.io/library/busybox_latest

Now all we have to do is fix is the stupid old file format, and add some real compression to it...

https://www.cyphar.com/blog/post/20190121-ociv2-images-i-tar "What's Wrong with Tar?"

@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

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

Successfully merging a pull request may close this issue.

3 participants