Merge pull request #2254 from mnowster/1756-dont-display-output-from-previous-runs

1756 dont display output from previous runs

compose/cli/log_printer.py

@@ -73,9 +73,13 @@ def build_no_log_generator(container, prefix, color_func):
 
 
 def build_log_generator(container, prefix, color_func):
-    # Attach to container before log printer starts running
+    # if the container doesn't have a log_stream we need to attach to container
+    # before log printer starts running
+    if container.log_stream is None:
         stream = container.attach(stdout=True, stderr=True,  stream=True, logs=True)
         line_generator = split_buffer(stream)
+    else:
+        line_generator = split_buffer(container.log_stream)
 
     for line in line_generator:
         yield prefix + line

compose/cli/main.py

@@ -565,16 +565,18 @@ class TopLevelCommand(DocoptCommand):
         start_deps = not options['--no-deps']
         service_names = options['SERVICE']
         timeout = int(options.get('--timeout') or DEFAULT_TIMEOUT)
+        detached = options.get('-d')
 
         to_attach = project.up(
             service_names=service_names,
             start_deps=start_deps,
             strategy=convergence_strategy_from_opts(options),
             do_build=not options['--no-build'],
-            timeout=timeout
+            timeout=timeout,
+            detached=detached
         )
 
-        if not options['-d']:
+        if not detached:
             log_printer = build_log_printer(to_attach, service_names, monochrome)
             attach_to_logs(project, log_printer, service_names, timeout)
 
@@ -636,7 +638,10 @@ def convergence_strategy_from_opts(options):
 
 def build_log_printer(containers, service_names, monochrome):
     if service_names:
-        containers = [c for c in containers if c.service in service_names]
+        containers = [
+            container
+            for container in containers if container.service in service_names
+        ]
     return LogPrinter(containers, monochrome=monochrome)
 
 

compose/container.py

@@ -19,6 +19,7 @@ class Container(object):
         self.client = client
         self.dictionary = dictionary
         self.has_been_inspected = has_been_inspected
+        self.log_stream = None
 
     @classmethod
     def from_ps(cls, client, dictionary, **kwargs):
@@ -146,6 +147,13 @@ class Container(object):
         log_type = self.log_driver
         return not log_type or log_type == 'json-file'
 
+    def attach_log_stream(self):
+        """A log stream can only be attached if the container uses a json-file
+        log driver.
+        """
+        if self.has_api_logs:
+            self.log_stream = self.attach(stdout=True, stderr=True, stream=True)
+
     def get(self, key):
         """Return a value from the container or None if the value is not set.
 
@@ -184,6 +192,15 @@ class Container(object):
     def remove(self, **options):
         return self.client.remove_container(self.id, **options)
 
+    def rename_to_tmp_name(self):
+        """Rename the container to a hopefully unique temporary container name
+        by prepending the short id.
+        """
+        self.client.rename(
+            self.id,
+            '%s_%s' % (self.short_id, self.name)
+        )
+
     def inspect_if_not_inspected(self):
         if not self.has_been_inspected:
             self.inspect()

compose/project.py

@@ -290,7 +290,8 @@ class Project(object):
            start_deps=True,
            strategy=ConvergenceStrategy.changed,
            do_build=True,
-           timeout=DEFAULT_TIMEOUT):
+           timeout=DEFAULT_TIMEOUT,
+           detached=False):
 
         services = self.get_services(service_names, include_deps=start_deps)
 
@@ -308,7 +309,8 @@ class Project(object):
             for container in service.execute_convergence_plan(
                 plans[service.name],
                 do_build=do_build,
-                timeout=timeout
+                timeout=timeout,
+                detached=detached
             )
         ]
 

compose/service.py

@@ -395,11 +395,17 @@ class Service(object):
     def execute_convergence_plan(self,
                                  plan,
                                  do_build=True,
-                                 timeout=DEFAULT_TIMEOUT):
+                                 timeout=DEFAULT_TIMEOUT,
+                                 detached=False):
         (action, containers) = plan
+        should_attach_logs = not detached
 
         if action == 'create':
             container = self.create_container(do_build=do_build)
+
+            if should_attach_logs:
+                container.attach_log_stream()
+
             self.start_container(container)
 
             return [container]
@@ -407,15 +413,16 @@ class Service(object):
         elif action == 'recreate':
             return [
                 self.recreate_container(
-                    c,
+                    container,
-                    timeout=timeout
+                    timeout=timeout,
+                    attach_logs=should_attach_logs
                 )
-                for c in containers
+                for container in containers
             ]
 
         elif action == 'start':
-            for c in containers:
+            for container in containers:
-                self.start_container_if_stopped(c)
+                self.start_container_if_stopped(container, attach_logs=should_attach_logs)
 
             return containers
 
@@ -430,7 +437,8 @@ class Service(object):
 
     def recreate_container(self,
                            container,
-                           timeout=DEFAULT_TIMEOUT):
+                           timeout=DEFAULT_TIMEOUT,
+                           attach_logs=False):
         """Recreate a container.
 
         The original container is renamed to a temporary name so that data
@@ -438,36 +446,28 @@ class Service(object):
         container is removed.
         """
         log.info("Recreating %s" % container.name)
-        try:
+
         container.stop(timeout=timeout)
-        except APIError as e:
+        container.rename_to_tmp_name()
-            if (e.response.status_code == 500
-                    and e.explanation
-                    and 'no such process' in str(e.explanation)):
-                pass
-            else:
-                raise
-
-        # Use a hopefully unique container name by prepending the short id
-        self.client.rename(
-            container.id,
-            '%s_%s' % (container.short_id, container.name))
-
         new_container = self.create_container(
             do_build=False,
             previous_container=container,
             number=container.labels.get(LABEL_CONTAINER_NUMBER),
             quiet=True,
         )
+        if attach_logs:
+            new_container.attach_log_stream()
         self.start_container(new_container)
         container.remove()
         return new_container
 
-    def start_container_if_stopped(self, container):
+    def start_container_if_stopped(self, container, attach_logs=False):
         if container.is_running:
             return container
         else:
             log.info("Starting %s" % container.name)
+            if attach_logs:
+                container.attach_log_stream()
             return self.start_container(container)
 
     def start_container(self, container):

tests/integration/service_test.py

@@ -362,6 +362,7 @@ class ServiceTest(DockerClientTestCase):
 
         new_container, = service.execute_convergence_plan(
             ConvergencePlan('recreate', [old_container]))
+
         self.assertEqual(list(new_container.get('Volumes')), ['/data'])
         self.assertEqual(new_container.get('Volumes')['/data'], volume_path)
 

tests/integration/state_test.py

@@ -18,6 +18,7 @@ from compose.service import ConvergenceStrategy
 class ProjectTestCase(DockerClientTestCase):
     def run_up(self, cfg, **kwargs):
         kwargs.setdefault('timeout', 1)
+        kwargs.setdefault('detached', True)
 
         project = self.make_project(cfg)
         project.up(**kwargs)

tests/unit/cli/log_printer_test.py

@@ -16,6 +16,7 @@ def build_mock_container(reader):
         name='myapp_web_1',
         name_without_project='web_1',
         has_api_logs=True,
+        log_stream=None,
         attach=reader,
         wait=mock.Mock(return_value=0),
     )

tests/unit/service_test.py

@@ -323,9 +323,7 @@ class ServiceTest(unittest.TestCase):
         new_container = service.recreate_container(mock_container)
 
         mock_container.stop.assert_called_once_with(timeout=10)
-        self.mock_client.rename.assert_called_once_with(
+        mock_container.rename_to_tmp_name.assert_called_once_with()
-            mock_container.id,
-            '%s_%s' % (mock_container.short_id, mock_container.name))
 
         new_container.start.assert_called_once_with()
         mock_container.remove.assert_called_once_with()