Use stop grace period for container stop.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

compose/cli/main.py

@@ -24,7 +24,6 @@ from ..config import ConfigurationError
 from ..config import parse_environment
 from ..config.environment import Environment
 from ..config.serialize import serialize_config
-from ..const import DEFAULT_TIMEOUT
 from ..const import IS_WINDOWS_PLATFORM
 from ..errors import StreamParseError
 from ..progress_stream import StreamOutputError
@@ -726,7 +725,7 @@ class TopLevelCommand(object):
           -t, --timeout TIMEOUT      Specify a shutdown timeout in seconds.
                                      (default: 10)
         """
-        timeout = int(options.get('--timeout') or DEFAULT_TIMEOUT)
+        timeout = timeout_from_opts(options)
 
         for s in options['SERVICE=NUM']:
             if '=' not in s:
@@ -760,7 +759,7 @@ class TopLevelCommand(object):
           -t, --timeout TIMEOUT      Specify a shutdown timeout in seconds.
                                      (default: 10)
         """
-        timeout = int(options.get('--timeout') or DEFAULT_TIMEOUT)
+        timeout = timeout_from_opts(options)
         self.project.stop(service_names=options['SERVICE'], timeout=timeout)
 
     def restart(self, options):
@@ -773,7 +772,7 @@ class TopLevelCommand(object):
           -t, --timeout TIMEOUT      Specify a shutdown timeout in seconds.
                                      (default: 10)
         """
-        timeout = int(options.get('--timeout') or DEFAULT_TIMEOUT)
+        timeout = timeout_from_opts(options)
         containers = self.project.restart(service_names=options['SERVICE'], timeout=timeout)
         exit_if(not containers, 'No containers to restart', 1)
 
@@ -831,7 +830,7 @@ class TopLevelCommand(object):
         start_deps = not options['--no-deps']
         cascade_stop = options['--abort-on-container-exit']
         service_names = options['SERVICE']
-        timeout = int(options.get('--timeout') or DEFAULT_TIMEOUT)
+        timeout = timeout_from_opts(options)
         remove_orphans = options['--remove-orphans']
         detached = options.get('-d')
 
@@ -896,6 +895,11 @@ def convergence_strategy_from_opts(options):
     return ConvergenceStrategy.changed
 
 
+def timeout_from_opts(options):
+    timeout = options.get('--timeout')
+    return None if timeout is None else int(timeout)
+
+
 def image_type_from_opt(flag, value):
     if not value:
         return ImageType.none

compose/parallel.py

@@ -248,7 +248,3 @@ def parallel_unpause(containers, options):
 
 def parallel_kill(containers, options):
     parallel_operation(containers, 'kill', options, 'Killing')
-
-
-def parallel_restart(containers, options):
-    parallel_operation(containers, 'restart', options, 'Restarting')

compose/project.py

@@ -14,7 +14,6 @@ from .config import ConfigurationError
 from .config.config import V1
 from .config.sort_services import get_container_name_from_network_mode
 from .config.sort_services import get_service_name_from_network_mode
-from .const import DEFAULT_TIMEOUT
 from .const import IMAGE_EVENTS
 from .const import LABEL_ONE_OFF
 from .const import LABEL_PROJECT
@@ -250,7 +249,7 @@ class Project(object):
 
         parallel.parallel_execute(
             containers,
-            operator.methodcaller('stop', **options),
+            self.build_container_operation_with_timeout_func('stop', options),
             operator.attrgetter('name'),
             'Stopping',
             get_deps)
@@ -291,7 +290,12 @@ class Project(object):
 
     def restart(self, service_names=None, **options):
         containers = self.containers(service_names, stopped=True)
-        parallel.parallel_restart(containers, options)
+
+        parallel.parallel_execute(
+            containers,
+            self.build_container_operation_with_timeout_func('restart', options),
+            operator.attrgetter('name'),
+            'Restarting')
         return containers
 
     def build(self, service_names=None, no_cache=False, pull=False, force_rm=False):
@@ -365,7 +369,7 @@ class Project(object):
            start_deps=True,
            strategy=ConvergenceStrategy.changed,
            do_build=BuildAction.none,
-           timeout=DEFAULT_TIMEOUT,
+           timeout=None,
            detached=False,
            remove_orphans=False):
 
@@ -506,6 +510,14 @@ class Project(object):
         dep_services.append(service)
         return acc + dep_services
 
+    def build_container_operation_with_timeout_func(self, operation, options):
+        def container_operation_with_timeout(container):
+            if options.get('timeout') is None:
+                service = self.get_service(container.service)
+                options['timeout'] = service.stop_timeout(None)
+            return getattr(container, operation)(**options)
+        return container_operation_with_timeout
+
 
 def get_volumes_from(project, service_dict):
     volumes_from = service_dict.pop('volumes_from', None)

compose/service.py

@@ -17,6 +17,7 @@ from docker.utils.ports import split_port
 
 from . import __version__
 from . import progress_stream
+from . import timeparse
 from .config import DOCKER_CONFIG_KEYS
 from .config import merge_environment
 from .config.types import VolumeSpec
@@ -169,7 +170,7 @@ class Service(object):
             self.start_container_if_stopped(c, **options)
         return containers
 
-    def scale(self, desired_num, timeout=DEFAULT_TIMEOUT):
+    def scale(self, desired_num, timeout=None):
         """
         Adjusts the number of containers to the specified number and ensures
         they are running.
@@ -196,7 +197,7 @@ class Service(object):
             return container
 
         def stop_and_remove(container):
-            container.stop(timeout=timeout)
+            container.stop(timeout=self.stop_timeout(timeout))
             container.remove()
 
         running_containers = self.containers(stopped=False)
@@ -374,7 +375,7 @@ class Service(object):
 
     def execute_convergence_plan(self,
                                  plan,
-                                 timeout=DEFAULT_TIMEOUT,
+                                 timeout=None,
                                  detached=False,
                                  start=True):
         (action, containers) = plan
@@ -421,7 +422,7 @@ class Service(object):
     def recreate_container(
             self,
             container,
-            timeout=DEFAULT_TIMEOUT,
+            timeout=None,
             attach_logs=False,
             start_new_container=True):
         """Recreate a container.
@@ -432,7 +433,7 @@ class Service(object):
         """
         log.info("Recreating %s" % container.name)
 
-        container.stop(timeout=timeout)
+        container.stop(timeout=self.stop_timeout(timeout))
         container.rename_to_tmp_name()
         new_container = self.create_container(
             previous_container=container,
@@ -446,6 +447,14 @@ class Service(object):
         container.remove()
         return new_container
 
+    def stop_timeout(self, timeout):
+        if timeout is not None:
+            return timeout
+        timeout = timeparse.timeparse(self.options.get('stop_grace_period') or '')
+        if timeout is not None:
+            return timeout
+        return DEFAULT_TIMEOUT
+
     def start_container_if_stopped(self, container, attach_logs=False, quiet=False):
         if not container.is_running:
             if not quiet:
@@ -483,10 +492,10 @@ class Service(object):
                 link_local_ips=netdefs.get('link_local_ips', None),
             )
 
-    def remove_duplicate_containers(self, timeout=DEFAULT_TIMEOUT):
+    def remove_duplicate_containers(self, timeout=None):
         for c in self.duplicate_containers():
             log.info('Removing %s' % c.name)
-            c.stop(timeout=timeout)
+            c.stop(timeout=self.stop_timeout(timeout))
             c.remove()
 
     def duplicate_containers(self):

tests/unit/timeparse_test.py

@@ -50,3 +50,7 @@ def test_invalid_with_space():
 
 def test_invalid_with_comma():
     assert timeparse.timeparse('5h,34m,56s') is None
+
+
+def test_invalid_with_empty_string():
+    assert timeparse.timeparse('') is None