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

Drop experimental tag for manifest commands #1355

Conversation

dims
Copy link

@dims dims commented Sep 8, 2018

Signed-off-by: Davanum Srinivas davanum@gmail.com

- What I did

Some of us working on Kubernetes have been using the docker manifest
experimental commands to build images for some time now. The last
problem we had was fixed by one of the commits a few months ago:
69e1743

Could we please promote these commands from experimental to GA

- How I did it

Delete the annotation for the command that sets it to experimental

- How to verify it

Just run docker manifest commands without the config.json or environment flag for experimental mode

- Description for the changelog

docker manifest commands are now no longer considered experimental and available for general use

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

2282173

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "promote-docker-manifest-commands-and-drop-experimental-tag" git@github.com:dims/cli.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

Some of us working on Kubernetes have been using the docker manifest
experimental commands to build images for some time now. The last
problem we had was fixed by one of the commits a few months ago:
docker@69e1743

Could we please promote these commands from experimental to GA

Change-Id: Iaa3ac7e1b56e1b6b4f091d047bf50c6794fa3d65
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
@dims dims force-pushed the promote-docker-manifest-commands-and-drop-experimental-tag branch from 79e0dc3 to 3f8dcb8 Compare September 8, 2018 01:39
@dims
Copy link
Author

dims commented Sep 8, 2018

cc @estesp

@clnperez
Copy link
Contributor

clnperez commented Sep 9, 2018

I'm for it! ;)

This is also a headache for our internal teams who have to add bits to their Travis files to add the experimental json info after installing docker.

@ijc
Copy link
Contributor

ijc commented Sep 11, 2018

In my (rather light) use of it so far it seems rather rough around the edges still. For example shouldn't issues such as #910 and #1358 (and I'm sure there must be others) be fixed before this becomes non-experimental?

@dims
Copy link
Author

dims commented Sep 11, 2018

@ijc

@dims
Copy link
Author

dims commented Jan 11, 2019

@thaJeztah @estesp @ijc as part of the kubernetes ci/build process, we have a few months under our belt now. So can we please promote manifest now?

Also moby/moby#37619 is now fixed as well so situation is even better than before.

@olljanat
Copy link
Contributor

@vdemeester PTAL

@dims
Copy link
Author

dims commented Feb 19, 2019

@thaJeztah Can we please get this in?

@grooverdan
Copy link

grooverdan commented Feb 26, 2019

I'm also been asked by sysadmins not to use docker manifest in automated build processes because of its experimental status. My frequent use of docker manifest over the last year has been stable and useful. The code history shows minor bug fixes but no significant churn and no outstanding bugs, and to me that seem to make it an ideal candidate fro removing the experimental tag. Related issues mentioned above aren't part of docker manifest as @dims mentioned 6 months ago.

Please merge this. I'd really appreciate it.

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

@dims
Copy link
Author

dims commented Feb 27, 2019

w00t!!! thanks @vdemeester @grooverdan

@cpuguy83
Copy link
Collaborator

cpuguy83 commented Mar 5, 2019

ping @tiborvass @estesp WDYT?

@estesp
Copy link
Contributor

estesp commented Mar 5, 2019

I think this is totally reasonable and agree that the functionality seems stable at this point. However, I've never been a docker/cli maintainer, so was assuming one of them would take @vdemeester's ping and do a review.

@tiborvass
Copy link
Collaborator

cc @tonistiigi who had concerns about the design.
Question: now that we have CLI plugins, why not make it a CLI plugin instead? Could have faster iterations too.

@docwhat
Copy link

docwhat commented Mar 5, 2019

Would the cli plugins be distributed in the docker-ce apt & yum repositories?

@tonistiigi
Copy link
Member

Next version of CLI will come with CLI plugins support #1564 for allowing docker cli to be extended with functionality that doesn't directly expose the Engine capabilities. I think that's the right place where the manifest command should go and it doesn't require experimental after that.

The main reasons for that are that manifest command does not use or integrate with anything from the Engine/remote API. It's a completely standalone tool where the CLI directly communicates with a registry. For example when you do docker build or pull you can't even use these images with manifest command because they are local.

Buildkit already supports directly building manifest lists, including the functionality of stitching already available images into the manifest list. That is currently only available in BuildKit-standalone and not in Docker integration (DOCKER_BUILDKIT=1), but it will change in the future. With the containerd image integration in engine moby/moby#38043 docker images will start to manage manifest lists locally, and push/save/load will be able to work directly with manifest lists. I'm not saying that current manifest command would be completely useless then, but there will be alternatives that are much more tightly integrated.

As @tiborvass mentioned this would also allow faster iterations for the functionality and maybe allow exposing more similar useful functionality.

If people agree we can create a repo under the docker org to make sure manifest cli plugin will be part of the official plugins.

For distribution of plugins, I don't know for sure, but I assume they will be distributed on the same channels where you can get the CLI (packages+static builds).

@docwhat
Copy link

docwhat commented Mar 5, 2019

If people agree we can create a repo under the docker org to make sure manifest cli plugin will be part of the official plugins.

Yes, please. I don't mind it being moved into a plugin as long as I can continue to use Chef to point at the docker repositories to get all the packages I need.

@dims dims closed this Apr 18, 2019
@docwhat
Copy link

docwhat commented Apr 18, 2019

Why was this closed? Was something merged that addresses it?

@dims
Copy link
Author

dims commented Apr 18, 2019

@docwhat it did not look like there was any support for merging this patch as-is as folks were talking about moving this to a CLI plugin. If one of the maintainers wants to merge, i'll be happy to reopen the PR. (also, if anyone wants to pick up this patch into their own PR, that's fine with me as well)

@tonistiigi
Copy link
Member

If people agree we can create a repo

Facing some internal issues for creating a dedicated repo for this at the moment. I am trying to integrate support for this use case in the builder cli plugin currently in development. https://github.com/tonistiigi/buildx/pull/25 Maybe this could be used as an alternative. The plan is to include it with docker installation but I can’t guarantee a timeline atm.

I’ll leave it to other maintainers to decide. I know it is frustrating to wait but it is also bad to add backward compatibility guarantees to something that may not fit the overall design direction and would probably increase confusion in the future. As mentioned before moby/moby#38043 will add native manifest list support in the daemon and a lot of user flows will change because of it.

@pjh
Copy link

pjh commented Jun 26, 2020

It looks like this PR was rejected because of forthcoming CLI plugin support last year. I followed some of the links and it looks like #2534 deprecated support for the CLI Plugins API last month though. That PR did not link to any discussion or context or other issues explaining why it was deprecated.

Does anyone know the fate of the "experimental" docker manifest commands now?

@olljanat
Copy link
Contributor

olljanat commented Oct 1, 2020

@dims can you reopen this one so we can continue discussing if it can be merged now when concept of CLI plugins was rejected?

@dims dims reopened this Oct 1, 2020
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@ce4a9f8). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1355   +/-   ##
=========================================
  Coverage          ?   54.63%           
=========================================
  Files             ?      293           
  Lines             ?    19369           
  Branches          ?        0           
=========================================
  Hits              ?    10582           
  Misses            ?     8126           
  Partials          ?      661           

@dims
Copy link
Author

dims commented Oct 1, 2020

@olljanat Done!

@justincormack justincormack added this to the 20.03.0 milestone Oct 2, 2020
@justaugustus
Copy link

IT'S HAPPENING! 🎉

@thaJeztah
Copy link
Member

We had some internal discussion on this, and I opened alternative pull requests in #2774 (and related: #2773)

@dims
Copy link
Author

dims commented Oct 2, 2020

@thaJeztah those pesky "Enterprise users"!!! :) thanks! i'll close this now.

@dims dims closed this Oct 2, 2020
@thaJeztah
Copy link
Member

Haha, yeah, well, it made sense for some things perhaps. Overall, documentation should be good enough.

FWIW; we'd still like to improve the UX for handling the manifest commands, which was the main reason for keeping it in experimental; such changes could potentially break scripts that rely on the UX / output. That said, not having to configure the CLI to be able to use these features definitely makes things easier to use, and is better to collect feedback on these

@olljanat
Copy link
Contributor

olljanat commented Oct 2, 2020

@thaJeztah hmm, but is Windows Server version still considered as enterprise version? (even when support is actually much worse than on community edition on Linux).

You can see my old and still valid use case (which why especially would like this to be enabled by default on Windows Server) on https://github.com/olljanat/multi-win-version-aspnet

@thaJeztah
Copy link
Member

@olljanat Docker currently doesn't build packages for Windows (besides Docker Desktop); if we would build packages again, then windows packages would have the same configuration (i.e., no special "enterprise" variant for Windows)

@olljanat
Copy link
Contributor

olljanat commented Oct 3, 2020

@thaJeztah yep, that why manifest commands shouldn't be experimental feature IMO.

/cc @StefanScherer you probably have seen this dilemma with Windows containers.

@clnperez
Copy link
Contributor

clnperez commented Oct 3, 2020

Very nice! 😀

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

Successfully merging this pull request may close these issues.

None yet