Add option to push missing images #76

eunomie · 2019-10-15T15:36:12Z

Allow push command and remotes.Fixup to be able to push missing
images from the local docker daemon image store.

A --push-images flag is added to the push command. This flag will
act during the fixup phase. If an image is not found, instead of
just printing a message saying you need to push the image before,
the image will be pushed if available in the local docker daemon
image store.

The fixup command doesn't change, there's no option to "fixup and push"
if you want to do that, use push.

remotes/fixup.go

remotes/push.go

silvin-lubecki · 2019-10-15T16:06:18Z

remotes/push.go

@@ -235,3 +245,75 @@ func pushBundleConfigDescriptor(ctx context.Context, name string, resolver remot
 	}
 	return descriptor, nil
 }
+
+func pushUnderTag(ctx context.Context, imageClient client.ImageAPIClient, image *bundle.BaseImage, targetRef reference.Named) error {
+	ref := image.Digest


I think we shouldn't use that content digest here, as explained by @glyn cnabio/cnab-go#145 (comment) at least until the specifications defines clearly what's in this field.

Oh, I think this is the cause for #76 (comment)

silvin-lubecki · 2019-10-15T16:06:52Z

remotes/push.go

@@ -235,3 +245,75 @@ func pushBundleConfigDescriptor(ctx context.Context, name string, resolver remot
 	}
 	return descriptor, nil
 }
+
+func pushUnderTag(ctx context.Context, imageClient client.ImageAPIClient, image *bundle.BaseImage, targetRef reference.Named) error {


Can you add some comments here, explaining the docker daemon limitation and why we push all the images on the same tag?

silvin-lubecki · 2019-10-15T16:17:19Z

remotes/push.go

+		return err
+	}
+	defer reader.Close()
+	if err := jsonmessage.DisplayJSONMessagesStream(reader, ioutil.Discard, 0, false, nil); err != nil {


I think we should let the user add its own iostream here so she can disable it or print it wherever she wants.
Maybe we can add it to the WithPushImages ?

Or maybe we should bind it with the event producer, like for the other images mounted or copied?

Something like this https://github.com/docker/cnab-to-oci/blob/master/remotes/fixup.go#L69 ? WDYT @eunomie ?

radu-matei · 2019-10-15T18:53:30Z

Ok, so I have this workflow:

really simple bundle built locally (invocation image not pushed), with the invocation image tagged like this:

{
    "description": "A short description of your bundle",
    "invocationImages": [
        {
            "image": "noaccesstothisregistry/push-test-cnab:742d5c8bd222737511ebc213cf7d99c40b32ef60",
            "imageType": "docker"
        }
    ],
    "keywords": [
        "push-test",
        "cnab",
        "tutorial"
    ],
    "maintainers": [
        {
            "email": "john.doe@example.com",
            "name": "John Doe",
            "url": "https://example.com"
        },
        {
            "email": "jane.doe@example.com",
            "name": "Jane Doe",
            "url": "https://example.com"
        }
    ],
    "name": "push-test",
    "schemaVersion": "v1.0.0-WD",
    "version": "0.1.0"
}

the image is built locally

 docker image ls
REPOSITORY                              TAG                                        IMAGE ID            CREATED              SIZE
noaccesstothisregistry/push-test-cnab   742d5c8bd222737511ebc213cf7d99c40b32ef60   3aeba8c4e1e8        About a minute ago   11.1MB
alpine                                  latest                                     961769676411        7 weeks ago          5.58MB
registry                                latest                                     f32a97de94e1        7 months ago         25.8MB

push bundle to the registry, then pull it back:

$ cnab-to-oci push ignore/push-test/bundle.json --push-images --auto-update-bundle -t localhost:5000/from-daemon:v1 --log-level debug
$ cnab-to-oci pull localhost:5000/from-daemon:v1 --log-level debug

try to push the bundle we just pulled:

cnab-to-oci push pulled.json --push-images -t localhost:5000/pulled-and-pushed:v1 --log-level debug
DEBU[0000] Fixing up bundle localhost:5000/pulled-and-pushed:v1
DEBU[0000] Updating entry in relocation map for "noaccesstothisregistry/push-test-cnab:742d5c8bd222737511ebc213cf7d99c40b32ef60"
Starting to copy image noaccesstothisregistry/push-test-cnab:742d5c8bd222737511ebc213cf7d99c40b32ef60...
Failed to copy image noaccesstothisregistry/push-test-cnab:742d5c8bd222737511ebc213cf7d99c40b32ef60: Error response from daemon: No such image: sha256:43b67c7bbc1dadd39e61ba9b0711c71035b0d0401d9a90a24cca34edfdb1659e
Error: Error response from daemon: No such image: sha256:43b67c7bbc1dadd39e61ba9b0711c71035b0d0401d9a90a24cca34edfdb1659e

Even if it is the same image, because we're trying to access it by SHA, that is not available locally - how is the SHA propagated locally after a push? Could we ensure the image is available in such scenarios?

radu-matei

This is great, thanks for picking it up!
The only immediate question is around how the SHA is used when pushing, and how / if it propagates after pushing an image.

cmd/cnab-to-oci/push.go

remotes/fixupoptions.go

radu-matei · 2019-10-15T19:06:09Z

remotes/push.go

@@ -235,3 +245,75 @@ func pushBundleConfigDescriptor(ctx context.Context, name string, resolver remot
 	}
 	return descriptor, nil
 }
+
+func pushUnderTag(ctx context.Context, imageClient client.ImageAPIClient, image *bundle.BaseImage, targetRef reference.Named) error {
+	ref := image.Digest


Oh, I think this is the cause for #76 (comment)

remotes/push.go

radu-matei · 2019-10-15T20:58:24Z

remotes/push.go

+	return base64.URLEncoding.EncodeToString(buf), nil
+}
+
+func resolveAuthConfig(index *registrytypes.IndexInfo) types.AuthConfig {


Also, this is going to be really painful to vendor if you don't have the exact revisions...

# github.com/docker/cnab-to-oci/remotes vendor/github.com/docker/cnab-to-oci/remotes/push.go:318:2: cannot use authConfig (type "github.com/docker/cli/cli/config/types".AuthConfig) as type "github.com/docker/docker/api/types".AuthConfig in return argument

Right, thank you for catching that @radu-matei 👍
We need to be more strict on our dependencies 😅

I've pushed a new bunch of commits to improve dependencies of cnab-to-oci. I'm not sure this is the right way of doing it.

I mainly upgraded the cnab-go to the latest tag and let it to get docker/cli and docker/docker (cli is the same as before for cnab-to-oci, docker is a little newer)

From what I saw it will not fix this issue, because the AuthConfig type has been introduced in docker/cli more recently.
I supposed the better way to fix this issue should be to upgrade both cnab-go and cnab-to-oci to latest releases of docker/cli and docker/docker but at least for cnab-to-oci it requires some more work.

Dependency upgrades are in dedicated commits so we can easily choose to not to do it.

@radu-matei can you check it works on your side?

Will be sure to check this out today - that being said, if we do end up bumping docker/docker, we are probably going to need some help untangling the dependencies in (at least) cnab-go, there are quite a few revisions we had to pin in order to compile (I think mostly introduced by a combination of docker/docker, docker/cli, docker/compose-on-kubernetes, and cli's dependency on Kubernetes.

Thanks for the work here!

cc @jeremyrickard

About cnab-go, we are working with @silvin-lubecki to remove the dep on the cli, we only need the api client. Will post something there soon (next week).

Ok - thanks a lot!
Then I think we can go ahead and merge this, then try to import again (and release) after the changes in cnab-go.

Does that make sense?

Opened cnabio/cnab-go#146 to track this in cnab-go.

Allow `push` command and `remotes.Fixup` to be able to push missing images from the local docker daemon image store. A `--push-images` flag is added to the `push` command. This flag will act during the `fixup` phase. If an image is not found, instead of just printing a message saying you need to push the image before, the image will be pushed if available in the local docker daemon image store. The `fixup` command doesn't change, there's no option to "fixup and push" if you want to do that, use `push`. Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>

remotes/fixup.go

silvin-lubecki

Looks better 👍

eunomie · 2019-10-17T13:29:39Z

Typos are fixed.
We can now configure output (discarded by default).
Dependencies are updated.

eunomie · 2019-10-17T13:30:41Z

I was also able to perform a full push - pull - push cycle as in #76 (comment) without issue.

silvin-lubecki

LGTM

silvin-lubecki · 2019-10-17T13:58:16Z

^ @radu-matei I let you merge 😄

remotes/push.go

remotes/fixupoptions.go

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

Allow to define a writer to get the output of image push during fixup. The `push` command defines it to `stdout` but if using as a library the output is configurable. By default (or nil) output is discared. Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>

-> v1.3.0 Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>

-> v0.6.3 Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>

-> v2.7.1 Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>

-> v0.7.1-beta1 Rely on cnab-go for docker/cli, docker/docker Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>

radu-matei · 2019-10-17T15:15:49Z

I just tried again the same workflow from #76 (comment) and now it works - most likely it was because of #76 (comment).

radu-matei

LGTM

-> 23446275646041f9b598d64c51be24d5d0e49376 This target commit is the one that copies `types.AuthConfig` from docker api to docker cli. By itself it's not an issue but it can be if `cnab-to-oci` is integrated in a context there a newer version of `docker/cli` is required. See cnabio#76 (comment) for example, and the same kind of issue exists in `docker/app` where both `cnab-to-oci` and a more recent `docker/cli` are used. Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>

-> 23446275646041f9b598d64c51be24d5d0e49376 This target commit is the one that copies `types.AuthConfig` from docker api to docker cli. By itself it's not an issue but it can be if `cnab-to-oci` is integrated in a context where a newer version of `docker/cli` is required. See cnabio#76 (comment) for example, and the same kind of issue exists in `docker/app` where both `cnab-to-oci` and a more recent `docker/cli` are used. Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>

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 force-pushed the push-images branch from 0f82727 to 8db33e1 Compare October 15, 2019 15:43

silvin-lubecki reviewed Oct 15, 2019

View reviewed changes

remotes/fixup.go Outdated Show resolved Hide resolved

silvin-lubecki reviewed Oct 15, 2019

View reviewed changes

remotes/push.go Outdated Show resolved Hide resolved

silvin-lubecki reviewed Oct 15, 2019

View reviewed changes

radu-matei reviewed Oct 15, 2019

View reviewed changes

radu-matei mentioned this pull request Oct 16, 2019

Refactor signing and verifying TUF metadata cnabio/signy#28

Merged

eunomie force-pushed the push-images branch 3 times, most recently from 9f83f8a to dc02dbd Compare October 16, 2019 15:18

silvin-lubecki reviewed Oct 16, 2019

View reviewed changes

remotes/fixup.go Outdated Show resolved Hide resolved

silvin-lubecki reviewed Oct 16, 2019

View reviewed changes

radu-matei mentioned this pull request Oct 17, 2019

duffle push should also push all referenced images cnabio/duffle#734

Open

eunomie force-pushed the push-images branch from dc02dbd to 6e51dd3 Compare October 17, 2019 13:21

eunomie marked this pull request as ready for review October 17, 2019 13:28

silvin-lubecki approved these changes Oct 17, 2019

View reviewed changes

rumpl reviewed Oct 17, 2019

View reviewed changes

remotes/push.go Outdated Show resolved Hide resolved

rumpl reviewed Oct 17, 2019

View reviewed changes

remotes/fixupoptions.go Outdated Show resolved Hide resolved

eunomie added 5 commits October 17, 2019 16:33

Add dependencies

ad71b2a

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

Update containerd/containerd

1297acc

-> v1.3.0 Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>

Update docker/docker-credential-helpers

b9a2348

-> v0.6.3 Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>

Update docker/distribution

3a76bd2

-> v2.7.1 Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>

Update deislabs/cnab-go

0cc26a8

-> v0.7.1-beta1 Rely on cnab-go for docker/cli, docker/docker Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>

eunomie force-pushed the push-images branch from 6e51dd3 to 0cc26a8 Compare October 17, 2019 14:35

radu-matei approved these changes Oct 17, 2019

View reviewed changes

radu-matei merged commit 4f3cb87 into cnabio:master Oct 17, 2019

eunomie mentioned this pull request Oct 18, 2019

Update docker/cli to handle types.AuthConfig #77

Merged

vdice mentioned this pull request Oct 18, 2019

Unhide the archive command, add some docs about archvice/publish -a getporter/porter#751

Merged

eunomie mentioned this pull request Oct 28, 2019

allow to push local docker images by digest #78

Merged

This was referenced Nov 4, 2019

update README about new push behavior #80

Merged

Push images if not present in registry #67

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to push missing images #76

Add option to push missing images #76

eunomie commented Oct 15, 2019

silvin-lubecki Oct 15, 2019

radu-matei Oct 15, 2019

silvin-lubecki Oct 15, 2019

silvin-lubecki Oct 15, 2019

silvin-lubecki Oct 15, 2019

silvin-lubecki Oct 15, 2019

radu-matei commented Oct 15, 2019

radu-matei left a comment

radu-matei Oct 15, 2019

radu-matei Oct 15, 2019 •

edited

Loading

silvin-lubecki Oct 15, 2019

eunomie Oct 17, 2019

silvin-lubecki Oct 17, 2019

radu-matei Oct 17, 2019 •

edited

Loading

rumpl Oct 17, 2019

radu-matei Oct 17, 2019

radu-matei Oct 17, 2019

silvin-lubecki left a comment

eunomie commented Oct 17, 2019

eunomie commented Oct 17, 2019

silvin-lubecki left a comment

silvin-lubecki commented Oct 17, 2019

radu-matei commented Oct 17, 2019

radu-matei left a comment

Add option to push missing images #76

Add option to push missing images #76

Conversation

eunomie commented Oct 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radu-matei commented Oct 15, 2019

radu-matei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radu-matei Oct 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radu-matei Oct 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

silvin-lubecki left a comment

Choose a reason for hiding this comment

eunomie commented Oct 17, 2019

eunomie commented Oct 17, 2019

silvin-lubecki left a comment

Choose a reason for hiding this comment

silvin-lubecki commented Oct 17, 2019

radu-matei commented Oct 17, 2019

radu-matei left a comment

Choose a reason for hiding this comment

radu-matei Oct 15, 2019 •

edited

Loading

radu-matei Oct 17, 2019 •

edited

Loading