Compose: docker-compose run command continuously restarting

[nicph]

Description of problem:

An unexpected behaviour is encountered when using docker-compose run with services defining restart: always.

docker version:

Client version: 1.5.0
Client API version: 1.17
Go version (client): go1.4.1
Git commit (client): a8a31ef
OS/Arch (client): linux/amd64
Server version: 1.5.0
Server API version: 1.17
Go version (server): go1.4.1
Git commit (server): a8a31ef

uname -a:
Linux docker01 3.13.0-45-generic #74-Ubuntu SMP Tue Jan 13 19:36:28 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

Environment details :

Host running inside a VMWare VM.

How reproducible:

Use this docker-compose file :

db:
    image: mysql
    restart: always
    environment:
        MYSQL_ROOT_PASSWORD: mypass

Steps to Reproduce:

1.

$ docker-compose up -d
Creating mysql_db_1...

2.

$ docker-compose run --rm db sh -c 'exec mysql -h db -uroot -p"$DB_ENV_MYSQL_ROOT_PASSWORD"'
mysql> exit
Removing mysql_db_run_1...
Conflict, You cannot remove a running container. Stop the container before attempting removal or use -f

Actual Results:

mysql_db_run_1 is still running.

Expected Results:

mysql_db_run_1 stopped and removed.

Additional info:

If I don't specify the '--rm' option, the result is the same but without the conflict message.

It looks like docker-compose run started the container with 'restart: always' option. I did not find a way to override this behavior from the docker-compose run command. Maybe docker-compose run shall ignore restart: always in docker-compose.yml (at least since the --rm flag has been specified) or provide a way to override this ?

[aanand]

Oh dear. Yes, we should probably just override restart by default.

[thaJeztah]

Is this something to be fixed upstream as well? Ie, don't allow restart policies on non-detached containers?

[aanand]

@thaJeztah Assuming the client reattaches, I can see "restart + interactive" being a useful thing in some cases, so probably not.

[thaJeztah]

@aanand good point, will see if I can test if that actually works. Would probably result in a race condition as well if combined with --rm

[thaJeztah]

Just checked for --rm and --restart and they are incompatible options in docker, see: https://github.com/docker/docker/commit/e49c70109228d657790190007c8f9d7c55a25be2

Will check some further w.r.t. non-detached and --restart

[josephpage]

Just got the same issue.
I'm agree that restart: always policy should be overriden and disabled when --rm is specified in the docker-compose run command

[myhro]

Debugging this with @mbodock, we find out where to override the restart option:

diff --git a/compose/cli/main.py b/compose/cli/main.py
index 95dfb6c..dbf40f4 100644
--- a/compose/cli/main.py
+++ b/compose/cli/main.py
@@ -325,6 +325,9 @@ class TopLevelCommand(Command):
         if options['--entrypoint']:
             container_options['entrypoint'] = options.get('--entrypoint')

+        if options['--rm']:
+            container_options['rm'] = options.get('--rm')
+
         if options['--user']:
             container_options['user'] = options.get('--user')

diff --git a/compose/service.py b/compose/service.py
index 936e3f9..2bfed47 100644
--- a/compose/service.py
+++ b/compose/service.py
@@ -443,7 +443,9 @@ class Service(object):
         if isinstance(dns_search, six.string_types):
             dns_search = [dns_search]

-        restart = parse_restart_spec(options.get('restart', None))
+        restart = None
+        if not options.get('rm', False):
+            restart = parse_restart_spec(options.get('restart', None))

         return create_host_config(
             links=self._get_links(link_to_self=one_off),

The problem now is that if we leave the rm in the container_options dict, later in the process we get this error:

Traceback (most recent call last):
  File "/home/vagrant/.virtualenvs/compose/bin/docker-compose", line 9, in <module>
    load_entry_point('docker-compose==1.1.0', 'console_scripts', 'docker-compose')()
  File "/vagrant/docker-compose/compose/cli/main.py", line 31, in main
    command.sys_dispatch()
  File "/vagrant/docker-compose/compose/cli/docopt_command.py", line 21, in sys_dispatch
    self.dispatch(sys.argv[1:], None)
  File "/vagrant/docker-compose/compose/cli/command.py", line 27, in dispatch
    super(Command, self).dispatch(*args, **kwargs)
  File "/vagrant/docker-compose/compose/cli/docopt_command.py", line 24, in dispatch
    self.perform_command(*self.parse(argv, global_options))
  File "/vagrant/docker-compose/compose/cli/command.py", line 59, in perform_command
    handler(project, command_options)
  File "/vagrant/docker-compose/compose/cli/main.py", line 340, in run
    **container_options
  File "/vagrant/docker-compose/compose/service.py", line 191, in create_container
    return Container.create(self.client, **container_options)
  File "/vagrant/docker-compose/compose/container.py", line 36, in create
    response = client.create_container(**options)
TypeError: create_container() got an unexpected keyword argument 'rm'

Which is strange, as create_container() is receiving a **kwargs already. So, looks like we shouldn't do this, as it is kinda sensible with which arguments it accepts.

Any thoughts on how we can proceed from here?

[dnephin]

I would try just this:

diff --git a/compose/cli/main.py b/compose/cli/main.py
index 95dfb6c..85e6755 100644
--- a/compose/cli/main.py
+++ b/compose/cli/main.py
@@ -325,6 +325,9 @@ class TopLevelCommand(Command):
         if options['--entrypoint']:
             container_options['entrypoint'] = options.get('--entrypoint')

+        if options['--rm']:
+            container_options['restart'] = None
+
         if options['--user']:
             container_options['user'] = options.get('--user')

Not sure if it'll work, it's just a guess based on what you tried.

[myhro]

Daniel, your suggestion is even simpler and... It works! :-)

Could you please send a PR?

[dnephin]

I probably wont have time to write the tests for it for a little while. If you're interested, feel free to grab the patch and submit a PR!

[myhro]

Looks like Joseph already took care of it. Thanks Joseph, thanks Daniel!

[wernight]

--rm and --restart are incompatible but is it the right solution to change behavior in that edge case? I'd be more inclined to have docker-compose run never respect --restart, but I guess some people want to have a specific run command always restart (not using up for some reason).

This change looks good but should include clear documentation update.

[josephpage]

Thanks @aanand for merging my PR #1218 that fixes this issue.

[TylerRick]

If we're not going to change it so that docker-compose run _never_ respects the restart policy defined in your docker-compose.yml (as proposed in https://github.com/docker/compose/issues/1013#issuecomment-94374161), could we at least add an option to let you override the restart policy on your docker run container?

I have a use case where I want to detach (because it's a long process—a database restore) but don't want to --rm because I want to be able to later go in and see any log files or other evidence for debugging in case something went wrong...

Looks like I can work around it like this (based on https://github.com/docker/docker/issues/13743#issuecomment-218742019)...

container_id=$(docker-compose run -d app script)
docker update --restart=no $container_id

... but for a super-quick process that would be subject to a race condition.

[cgarnier]

The restart always on "run" is a little annoying.
I made a gitlab backup with docker-compose run... such a bad idea :D