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

allow to push local docker images by digest #78

Merged
merged 1 commit into from
Oct 30, 2019

Conversation

eunomie
Copy link
Contributor

@eunomie eunomie commented Oct 28, 2019

In the case contentDigest is defined (for example after a build
by docker app) try to push local docker image from the digest.
In that case, the image of the service is not defined in the
bundle.json.

If contentDigest is empty, do the same thing as before, so try to resolve
and if it's not resolvable try to push.

The service name is now displayed in error messages to better understand
what's going on on failures.

Signed-off-by: Yves Brissaud yves.brissaud@docker.com

@thaJeztah
Copy link

Question (haven't looked closely, so bear with me); will the image that was pushed be tagged? If not, it may be worth checking with the Docker Hub team; I seem to recall that images that have never been tagged may be garbage-collected after some time.

Haven't looked what the UX for this looks like, but there has been prior discussion in docker about allowing push by digest; in that discussion, the format docker push <image-ID/digest> repository/name:tag was rejected to keep the path open for pushing multiple images (docker push image1 image2 image3).

There was an alternative proposal that was "accepted", but on hold, pending containerd integration; moby/moby#38880 (comment)

That alternative uses a flag (name to be decided on, but currently --local) to specify the local image as "source", thus allowing pushing a single image, and tag it under multiple names, e.g. (from that comment);

docker push --local=<local-image>  \
    myregistry/an-image:v1 \
    myregistry/an-image:v1.0 \
    myregistry/an-image:v1.0.2 \
    myregistry/an-image:latest

@eunomie
Copy link
Contributor Author

eunomie commented Oct 28, 2019

Question (haven't looked closely, so bear with me);

np 😉

will the image that was pushed be tagged?

Yes, images are tagged and then pushed, and then the index will allow to not be garbaged collected as they are referenced.

The main workflow was done in #76. This PR only adds the possibility to do it with a digest as the source and not an image.

https://github.com/docker/cnab-to-oci/blob/87394d4609cce25f8ac2e1ae569eee5887dfd1a9/remotes/fixup.go#L218-L228

Copy link
Member

@radu-matei radu-matei left a comment

Choose a reason for hiding this comment

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

Manually tested, LGTM.

@radu-matei radu-matei mentioned this pull request Oct 29, 2019
remotes/fixup.go Outdated Show resolved Hide resolved
remotes/fixup.go Outdated Show resolved Hide resolved
In the case `contentDigest` is defined (for example after a build
by docker app) try to push local docker image from the digest.
In that case, the `image` of the service is not defined in the
bundle.json.

If `contentDigest` is empty, do the same thing as before, so try to resolve
and if it's not resolvable try to push.

The service name is now displayed in error messages to better understand
what's going on on failures.

--

A `internal.ImageClient` interface has been added to only deal with
methods from Docker's `ImageAPIClient` we need. Easier to mock for
example.

Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>
Copy link
Collaborator

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

@silvin-lubecki silvin-lubecki merged commit 5a97c84 into cnabio:master Oct 30, 2019
eunomie added a commit to eunomie/cnab-to-oci that referenced this pull request Nov 4, 2019
With 4f3cb87 (cnabio#76) and 5a97c84 (cnabio#78) `cnab-to-oci` is now able
to push images from local docker daemon image store.

Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>
eunomie added a commit to eunomie/cnab-to-oci that referenced this pull request Nov 5, 2019
With 4f3cb87 (cnabio#76) and 5a97c84 (cnabio#78) `cnab-to-oci` is now able
to push images from local docker daemon image store.

Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>
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.

4 participants