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:

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

$ 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: moby/moby@e49c701

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.