manifest push using yaml file

[clnperez]

Instead of: manifest create list image1 image2 manifest annotate list image1 manifest push list

You can do: manifest push --file=myfile.yaml list:tag

Signed-off-by: Christy Norman [email protected]

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

[clnperez]

I still need to add a test. If anyone wants to give this a quick once-over in the meantime feel free.

[clnperez]

Also, so I don't forget, I need to update the doc to add this, and and to include how to set the cli experimental. It's confusing, because it's "true" for the engine, but "enabled" for the cli. :D

[clnperez]

Redid this (no longer uses local fs) but haven't tested the original path yet, so I'll get to that on Monday, and also take the rest of @dnephin's comments into account. Sorry for abandoning this for so long!

[clnperez]

tests added for yaml. removing [WIP]

@vdemeester @thaJeztah the week before dockercon, so, you've got time to take a look at this now right? :angel:

[clnperez]

cc @tianon @StefanScherer @AceHack

[StefanScherer]

Thanks @clnperez I'll look into it in the next days. Haven't looked into the details, but agree that pushing a yaml should be done with the docker manifest push command.

[clnperez]

@vdemeester i wasn't a huge fan of the file change but it grew on me and did make validation easier. i'm open to changes for sure. but i'm not sure i like having someone specify any information on the command line that could be also in a yaml file (in this case, the target ref).

[vdemeester]

@clnperez I really like the file, but in the current proposal there is two things that I feel could be better

• cli UX, a flag should not change the meaning of the positional args ; e.g. in the current design, having --file=true makes the positionnal arg being either a list (from the manifest store) or a file.
• the list is a image reference, and I think even with the file, we should specify that image reference as positional arguments. It would make the file not tied to the image reference pushed — it would be possible to do something like : docker manifest push --file=foo.yml vdemeester/foo && docker manifest push --file=foo.yml grc.io/vdemeester/foo — and the command would not be far from the current usage (we just add the possibility to pass a file)

So my proposal is to make the file something like the following :

manifests:
    - image: test/hello-world-ppc64le:latest
      platform:
          architecture: ppc64le
    - image: test/hello-world-amd64:latest
      platform:
          architecture: amd64
          os: linux
    - image: test/hello-world-s390x:latest
      platform:
          architecture: s390x 
          os: linux
          osversion: 1.1
          variant: xyz
          osfeatures: [a,b,c]

And have the cli being docker manifest push --file=myfile.yaml list.

@thaJeztah wdyt ?

[dnephin]

I agree with @vdemeester

[clnperez]

@vdemeester can you clarify: the file not tied to the image reference pushed...? The image reference pushed is the first line in the file the way the manifest tool works, and the way I intended this one to work as well. What's the meaning of vdemeester/foo in your example? Would it overwrite the target image on the first line of the file?

[vdemeester]

@clnperez yes, my sugestion would be to not have the image: … first line in the file (so just listing in the manifest file the manifests).

I updated my example above as I realized I did not remove that first line 😅.

[clnperez]

Thanks @vdemeester. That's a simple enough code change. I'd like to hear from some others before we get attached. :D

[tianon]

I think it's relevant to bring up https://github.com/estesp/manifest-tool/issues/23 (multiple tags pushed at once; https://github.com/estesp/manifest-tool/pull/32) in the context of removing image: from the YAML itself -- being able to simply docker manifest push --file xyz.yml abc:1.2.3 abc:1.2 abc:1 abc:latest would be great (especially if it's efficient about skipping the expensive digest lookups and cross-blob mounts).

It probably won't be very useful for pushing to multiple registries though (hub+gcr.io for example), right? (since the manifest list image references still need to be on the same registry for cross-repository blob-mounting)

https://github.com/estesp/manifest-tool/issues/22 is also relevant for the official images use-case -- we have to wait to push the manifest lists until the full set of architectures have built (see https://github.com/docker-library/official-images/issues/3835), so having --ignore-missing helps us to be able to push a manifest list in the meantime, but it's only a workaround for a much larger problem in the way manifest list objects have to be created and updated (namely, the race conditions around "updating" a manifest list without API support for adding/updating entries directly to an existing tag's manifest list in an idempotent way at the registry level, which would allow us to build/update manifest lists more fluidly as architectures build and push like they were before manifest lists were implemented and we were single-arch).

It'd be neat to be able to specify this YAML content without needing a file on-disk, but that's more of a minor annoyance than a serious issue (since it's a matter of garbage collection after we build the appropriate YAML, push the bits, then clean up afterwards).

[clnperez]

Thanks @tianon . I think for this PR, I can look at just redoing the --file arg so that it points to the list name (instead of it living in the yaml file), and have two follow-on PRs:

• add support for extra optional args that specify other refs to name this same manifest list, so it's all done with only one need to cross-mount the layer blobs.
• add the --ignore-missing parameter

[clnperez]

@silvin-lubecki nits taken into account, thanks :)

@vdemeester i'll have to redo this again if we go with https://github.com/docker/cli/pull/1156. Unless we want to merge this one first and make @dmcgowan rebase his. ;)

for now though -- i can't get the vendor bit fixed make -f docker.Makefile vendor ?

[clnperez]

figured out vendor. i think i had switched a commit. PTAL

[clnperez]

Rebased against master post-merge of #1156

[clnperez]

ping @thaJeztah @vdemeester

[codecov-io]

Codecov Report

Merging #866 into master will increase coverage by 0.07%. The diff coverage is 82.14%.

@@            Coverage Diff            @@
##           master    #866      +/-   ##
=========================================
+ Coverage   54.63%   54.7%   +0.07%     
=========================================
  Files         293     293              
  Lines       19369   19418      +49     
=========================================
+ Hits        10582   10623      +41     
- Misses       8126    8130       +4     
- Partials      661     665       +4

[clnperez]

Rebased. PTAL.

[clnperez]

paging @thaJeztah & @vdemeester

[silvin-lubecki]

@thaJeztah are you ok to merge this PR?

[clnperez]

ping @thaJeztah . would love to close this :innocent:

[duglin]

@dnephin @vdemeester can chance of merging this one?

[clnperez]

ping @thaJeztah. I saw that https://github.com/docker/cli/pull/1252 was merged so I have a sliver of hope that someone will take a look at this again.

[olljanat]

Nice is very good idea. Would be nice to get this merged before #1355

[grooverdan]

Nice work @clnperez, great way to create manifests by yaml definition.