-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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
Support docker stack deploy
from a Compose file
#27998
Conversation
Yes! +1 on design 😉 |
d362295
to
a96f8b5
Compare
SGTM 👍 |
@@ -178,4 +178,13 @@ clone git github.com/flynn-archive/go-shlex 3f9db97f856818214da2e1057f8ad8480397 | |||
# metrics | |||
clone git github.com/docker/go-metrics 86138d05f285fd9737a99bee2d9be30866b59d72 | |||
|
|||
# composefile | |||
clone git github.com/aanand/compose-file 73f61511606f5b302eda1fe88be1c4d0656fe25b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This repo does not yet have a license @aanand can you add one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
License was added to the repo, so will be included if it's revendorded; aanand/compose-file@480a796
@AkihiroSuda a lot of people were wondering why the intermediate .dab file was required, and were requesting an option to directly deploy from a compose file. Having an option to directly create services from a compose file would answer that request. |
Cool 👍 |
I tried this, but got something very weird. Possibly I'm reading the logs wrong, or it's because I'm running inside a container, but this is what happened; I built this PR ( On an empty cache inside the container, I tried to run this docker-compose file (basically an example from the docs, but changed to format "3"); version: '3'
services:
db:
image: mysql:5.7
volumes:
- "./.data/db:/var/lib/mysql"
restart: always
environment:
MYSQL_ROOT_PASSWORD: wordpress
MYSQL_DATABASE: wordpress
MYSQL_USER: wordpress
MYSQL_PASSWORD: wordpress
wordpress:
depends_on:
- db
image: wordpress:latest
links:
- db
ports:
- "8000:80"
restart: always
environment:
WORDPRESS_DB_HOST: db:3306
WORDPRESS_DB_PASSWORD: wordpress Because it uses a bind-mount, I created the
However, I think docker went into an endless loop of pulling the same image(s). Maybe I'm reading the logs incorrectly, but there's multiple "download complete" messages; https://gist.github.com/thaJeztah/77f7c28100f9c5b35303554812191648#file-pull-loop-log-L235-L236 But Manually pulling the images, and then deploying the docker-compose file worked. I'm not sure it's related to this PR, or an existing bug |
Ok, ignore my previous comment; I'm on a very slow network, and the log-messages are just confusing (room for improvement there); these messages;
are shown for each layer, but I read them as "image download complete" |
a96f8b5
to
7e2b339
Compare
re-vendored to pickup the license |
fea081f
to
aa22d49
Compare
@dnephin build is failing;
|
Oh right, I didn't rebase the vendor changes correctly, oops |
aa22d49
to
dee3688
Compare
I tested this some more, and looks to be working as expected; both creating, and updating (i.e. run One thing I was worried about is if services would be created with an network alias, but that works as well 👏 |
I'm moving this to code review |
} | ||
|
||
// TODO: catch Windows paths here | ||
if strings.HasPrefix(source, "/") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😨
Can we be a little more precise with this?
See https://github.com/docker/docker/pull/24182/files for an approach to this.
Basically copied windows and linux versions of filepath.IsAbs
into something we can call from either platform.
As is this won't work on Windows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not planning on supporting windows at this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dnephin This is very very wrong.
There is filepath.IsAbs
, but you still need to be able to support Windows clients talking to Linux daemons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows client support was deemed not in scope for the first experimental release of Compose file support. There's platform-dependent complexity around splitting volume path mappings (of the C:\foo:D:\bar:ro
variety) that we didn't have time to go into.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this still works for relative paths on windows, right? It's just absolute paths that aren't supported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not even sure how this could work with relative paths.
Relative to what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now we expand relative paths in the compose-file
library, but I think that's probably wrong. We should move that to a later step.
Paths are relative to the current working directory of the client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dnephin Not sure I understand.
Services are scheduled across the cluster. How can it be relative to the client?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, I guess using relative paths is going to be pretty uncommon if you're running against a multi-node cluster. You would most likely use absolute paths in that case.
There is always the case where you are running against a single "local" node cluster, there the node is the same host. In that case supporting relative paths maintains backwards compatibility with how docker-compose
works today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not incredibly comfortable with this ambiguity, but at least we'll get an error if the service lands on a node where the requested bind path doesn't exist.
dee3688
to
5620854
Compare
Yes, this does not work. Help output is same for both operating modes (local daemon, current master): $ docker version | grep Experimental
Experimental: false
$ docker --help > normal.txt
# restart daemon with --experimental
$ docker version | grep Experimental
Experimental: true
$ docker --help > experimental.txt
$ diff normal.txt experimental.txt
$ |
|
||
func getBindOptions(mode []string) *mount.BindOptions { | ||
for _, item := range mode { | ||
if strings.Contains(item, "private") || strings.Contains(item, "shared") || strings.Contains(item, "slave") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the use of strings.Contains
here correct? This would also match strings that have extra characters ("privateer", "unshared", "slavery", etc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it would. It was kind of rushed, I need to look at a better way of doing it.
It shouldn't break anything though. The only other options that can be provided here are modes and nocopy
which wont contain these strings.
@thaJeztah Help output is fixed again (experimental features only showing up when daemon is running in experimental mode) in current HEAD. But is was broken in RC1. |
Docs is being tracked through docker/docs#685 |
Does this mean that reading docker-compose through docker directly, in 1.13, will support all the unsupported options that docker-compose bundle (up to 1.9.0) drops? |
@RRAlex some options are not supported (e.g. |
@thaJeztah Makes sense for container_name. Related: docker/compose#3680 (comment) |
@RRAlex things are going fast, so I'm not 100% sure if they are all supported, but you can download the release candidates to give it a spin 👍. Since you're interested in this feature, it would be welcome if you could test it, for your use case and see if things are missing, or where improvements can be made (feel free to break it anyway you can 😄) |
Seems like those 2 aren't supported in 1.13.0-rc2 yet, at least with our docker-compose.yml (which uses almost every options). It seems better than .dab support if they really are the only two left.
Any plans to support those before the final release? |
You can set the equivalent swarm mode option with |
Will compose support other options that docker service has, like --constraint? |
@liyaka yes, it's in |
@vdemeester is it in the new compose version? where can I found the updated documentation? |
Definitely not the documentation, but hope this helps: https://github.com/docker/compose/blob/master/compose/config/config_schema_v3.0.json |
WIP documentation can be found here; docker/docs#716 |
Pull request moby#27745 added support for the client to talk to older versions of the daemon. Various flags were added to docker 1.13 that are not compatible with older daemons. This PR adds annotations to those flags, so that they are automatically hidden if the daemon is older than docker 1.13 (API 1.25). Not all new flags affect the API (some are client-side only). The following PR's added new flags to docker 1.13 that affect the API; - moby#23430 added `--cpu-rt-period`and `--cpu-rt-runtime` - moby#27800 / moby#25317 added `--group` / `--group-add` / `--group-rm` - moby#27702 added `--network` to `docker build` - moby#25962 added `--attachable` to `docker network create` - moby#27998 added `--compose-file` to `docker stack deploy` - moby#22566 added `--stop-timeout` to `docker run` and `docker create` - moby#26061 added `--init` to `docker run` and `docker create` - moby#26941 added `--init-path` to `docker run` and `docker create` - moby#27958 added `--cpus` on `docker run` / `docker create` - moby#27567 added `--dns`, `--dns-opt`, and `--dns-search` to `docker service create` - moby#27596 added `--force` to `docker service update` - moby#27857 added `--hostname` to `docker service create` - moby#28031 added `--hosts`, `--host-add` / `--host-rm` to `docker service create` and `docker service update` - moby#28076 added `--tty` on `docker service create` / `docker service update` - moby#26421 added `--update-max-failure-ratio`, `--update-monitor` and `--rollback` on `docker service update` - moby#27369 added `--health-cmd`, `--health-interval`, `--health-retries`, `--health-timeout` and `--no-healthcheck` options to `docker service create` and `docker service update` Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Support `docker stack deploy` from a Compose file
Pull request moby/moby#27745 added support for the client to talk to older versions of the daemon. Various flags were added to docker 1.13 that are not compatible with older daemons. This PR adds annotations to those flags, so that they are automatically hidden if the daemon is older than docker 1.13 (API 1.25). Not all new flags affect the API (some are client-side only). The following PR's added new flags to docker 1.13 that affect the API; - moby/moby#23430 added `--cpu-rt-period`and `--cpu-rt-runtime` - moby/moby#27800 / moby/moby#25317 added `--group` / `--group-add` / `--group-rm` - moby/moby#27702 added `--network` to `docker build` - moby/moby#25962 added `--attachable` to `docker network create` - moby/moby#27998 added `--compose-file` to `docker stack deploy` - moby/moby#22566 added `--stop-timeout` to `docker run` and `docker create` - moby/moby#26061 added `--init` to `docker run` and `docker create` - moby/moby#26941 added `--init-path` to `docker run` and `docker create` - moby/moby#27958 added `--cpus` on `docker run` / `docker create` - moby/moby#27567 added `--dns`, `--dns-opt`, and `--dns-search` to `docker service create` - moby/moby#27596 added `--force` to `docker service update` - moby/moby#27857 added `--hostname` to `docker service create` - moby/moby#28031 added `--hosts`, `--host-add` / `--host-rm` to `docker service create` and `docker service update` - moby/moby#28076 added `--tty` on `docker service create` / `docker service update` - moby/moby#26421 added `--update-max-failure-ratio`, `--update-monitor` and `--rollback` on `docker service update` - moby/moby#27369 added `--health-cmd`, `--health-interval`, `--health-retries`, `--health-timeout` and `--no-healthcheck` options to `docker service create` and `docker service update` Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: 5d2722f83db9e301c6dcbe1c562c2051a52905db Component: cli
Read a Compose file using
aanand/compose-file
and create swarm mode services for each service in the file.