Merge pull request #2020 from dnephin/fix_compose_with_logs_none

Fix compose up with other log drivers

compose/cli/log_printer.py

@@ -4,8 +4,6 @@ from __future__ import unicode_literals
 import sys
 from itertools import cycle
 
-from six import next
-
 from . import colors
 from .multiplexer import Multiplexer
 from .utils import split_buffer
@@ -13,72 +11,77 @@ from compose import utils
 
 
 class LogPrinter(object):
-    def __init__(self, containers, attach_params=None, output=sys.stdout, monochrome=False):
+    """Print logs from many containers to a single output stream."""
+
+    def __init__(self, containers, output=sys.stdout, monochrome=False):
         self.containers = containers
-        self.attach_params = attach_params or {}
-        self.prefix_width = self._calculate_prefix_width(containers)
-        self.generators = self._make_log_generators(monochrome)
         self.output = utils.get_output_stream(output)
+        self.monochrome = monochrome
 
     def run(self):
-        mux = Multiplexer(self.generators)
-        for line in mux.loop():
+        if not self.containers:
+            return
+
+        prefix_width = max_name_width(self.containers)
+        generators = list(self._make_log_generators(self.monochrome, prefix_width))
+        for line in Multiplexer(generators).loop():
             self.output.write(line)
 
-    def _calculate_prefix_width(self, containers):
-        """
-        Calculate the maximum width of container names so we can make the log
-        prefixes line up like so:
+    def _make_log_generators(self, monochrome, prefix_width):
+        def no_color(text):
+            return text
+
+        if monochrome:
+            color_funcs = cycle([no_color])
+        else:
+            color_funcs = cycle(colors.rainbow())
 
-        db_1  | Listening
-        web_1 | Listening
-        """
-        prefix_width = 0
-        for container in containers:
-            prefix_width = max(prefix_width, len(container.name_without_project))
-        return prefix_width
+        for color_func, container in zip(color_funcs, self.containers):
+            generator_func = get_log_generator(container)
+            prefix = color_func(build_log_prefix(container, prefix_width))
+            yield generator_func(container, prefix, color_func)
 
-    def _make_log_generators(self, monochrome):
-        color_fns = cycle(colors.rainbow())
-        generators = []
 
-        def no_color(text):
-            return text
+def build_log_prefix(container, prefix_width):
+    return container.name_without_project.ljust(prefix_width) + ' | '
+
+
+def max_name_width(containers):
+    """Calculate the maximum width of container names so we can make the log
+    prefixes line up like so:
+
+    db_1  | Listening
+    web_1 | Listening
+    """
+    return max(len(container.name_without_project) for container in containers)
+
+
+def get_log_generator(container):
+    if container.has_api_logs:
+        return build_log_generator
+    return build_no_log_generator
+
+
+def build_no_log_generator(container, prefix, color_func):
+    """Return a generator that prints a warning about logs and waits for
+    container to exit.
+    """
+    yield "{} WARNING: no logs are available with the '{}' log driver\n".format(
+        prefix,
+        container.log_driver)
+    yield color_func(wait_on_exit(container))
+
+
+def build_log_generator(container, prefix, color_func):
+    # Attach to container before log printer starts running
+    stream = container.attach(stdout=True, stderr=True,  stream=True, logs=True)
+    line_generator = split_buffer(stream, u'\n')
+
+    for line in line_generator:
+        yield prefix + line
+    yield color_func(wait_on_exit(container))
+
 
-        for container in self.containers:
-            if monochrome:
-                color_fn = no_color
-            else:
-                color_fn = next(color_fns)
-            generators.append(self._make_log_generator(container, color_fn))
-
-        return generators
-
-    def _make_log_generator(self, container, color_fn):
-        prefix = color_fn(self._generate_prefix(container))
-        # Attach to container before log printer starts running
-        line_generator = split_buffer(self._attach(container), u'\n')
-
-        for line in line_generator:
-            yield prefix + line
-
-        exit_code = container.wait()
-        yield color_fn("%s exited with code %s\n" % (container.name, exit_code))
-
-    def _generate_prefix(self, container):
-        """
-        Generate the prefix for a log line without colour
-        """
-        name = container.name_without_project
-        padding = ' ' * (self.prefix_width - len(name))
-        return ''.join([name, padding, ' | '])
-
-    def _attach(self, container):
-        params = {
-            'stdout': True,
-            'stderr': True,
-            'stream': True,
-        }
-        params.update(self.attach_params)
-        params = dict((name, 1 if value else 0) for (name, value) in list(params.items()))
-        return container.attach(**params)
+def wait_on_exit(container):
+    exit_code = container.wait()
+    return "%s exited with code %s\n" % (container.name, exit_code)

compose/cli/main.py

@@ -193,7 +193,7 @@ class TopLevelCommand(Command):
 
         monochrome = options['--no-color']
         print("Attaching to", list_containers(containers))
-        LogPrinter(containers, attach_params={'logs': True}, monochrome=monochrome).run()
+        LogPrinter(containers, monochrome=monochrome).run()
 
     def pause(self, project, options):
         """
@@ -602,11 +602,7 @@ 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]
-
-    return LogPrinter(
-        containers,
-        attach_params={"logs": True},
-        monochrome=monochrome)
+    return LogPrinter(containers, monochrome=monochrome)
 
 
 def attach_to_logs(project, log_printer, service_names, timeout):

compose/container.py

@@ -137,6 +137,15 @@ class Container(object):
     def is_paused(self):
         return self.get('State.Paused')
 
+    @property
+    def log_driver(self):
+        return self.get('HostConfig.LogConfig.Type')
+
+    @property
+    def has_api_logs(self):
+        log_type = self.log_driver
+        return not log_type or log_type == 'json-file'
+
     def get(self, key):
         """Return a value from the container or None if the value is not set.
 

docs/yml.md

@@ -289,11 +289,10 @@ Because Docker container names must be unique, you cannot scale a service
 beyond 1 container if you have specified a custom name. Attempting to do so
 results in an error.
 
-### log driver
+### log_driver
 
-Specify a logging driver for the service's containers, as with the ``--log-driver`` option for docker run ([documented here](http://docs.docker.com/reference/run/#logging-drivers-log-driver)).
-
-Allowed values are currently ``json-file``, ``syslog`` and ``none``. The list will change over time as more drivers are added to the Docker engine.
+Specify a logging driver for the service's containers, as with the ``--log-driver``
+option for docker run ([documented here](https://docs.docker.com/reference/logging/overview/)).
 
 The default value is json-file.
 
@@ -301,6 +300,12 @@ The default value is json-file.
     log_driver: "syslog"
     log_driver: "none"
 
+> **Note:** Only the `json-file` driver makes the logs available directly from
+> `docker-compose up` and `docker-compose logs`. Using any other driver will not
+> print any logs.
+
+### log_opt
+
 Specify logging options with `log_opt` for the logging driver, as with the ``--log-opt`` option for `docker run`.
 
 Logging options are key value pairs. An example of `syslog` options:

tests/unit/cli/log_printer_test.py

@@ -1,20 +1,31 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
-import os
-
+import mock
 import six
 
 from compose.cli.log_printer import LogPrinter
+from compose.cli.log_printer import wait_on_exit
+from compose.container import Container
 from tests import unittest
 
 
+def build_mock_container(reader):
+    return mock.Mock(
+        spec=Container,
+        name='myapp_web_1',
+        name_without_project='web_1',
+        has_api_logs=True,
+        attach=reader,
+        wait=mock.Mock(return_value=0),
+    )
+
+
 class LogPrinterTest(unittest.TestCase):
     def get_default_output(self, monochrome=False):
         def reader(*args, **kwargs):
             yield b"hello\nworld"
-
-        container = MockContainer(reader)
+        container = build_mock_container(reader)
         output = run_log_printer([container], monochrome=monochrome)
         return output
 
@@ -38,37 +49,39 @@ class LogPrinterTest(unittest.TestCase):
         def reader(*args, **kwargs):
             yield glyph.encode('utf-8') + b'\n'
 
-        container = MockContainer(reader)
+        container = build_mock_container(reader)
         output = run_log_printer([container])
         if six.PY2:
             output = output.decode('utf-8')
 
         self.assertIn(glyph, output)
 
+    def test_wait_on_exit(self):
+        exit_status = 3
+        mock_container = mock.Mock(
+            spec=Container,
+            name='cname',
+            wait=mock.Mock(return_value=exit_status))
 
-def run_log_printer(containers, monochrome=False):
-    r, w = os.pipe()
-    reader, writer = os.fdopen(r, 'r'), os.fdopen(w, 'w')
-    printer = LogPrinter(containers, output=writer, monochrome=monochrome)
-    printer.run()
-    writer.close()
-    return reader.read()
-
+        expected = '{} exited with code {}\n'.format(mock_container.name, exit_status)
+        self.assertEqual(expected, wait_on_exit(mock_container))
 
-class MockContainer(object):
-    def __init__(self, reader):
-        self._reader = reader
+    def test_generator_with_no_logs(self):
+        mock_container = mock.Mock(
+            spec=Container,
+            has_api_logs=False,
+            log_driver='none',
+            name_without_project='web_1',
+            wait=mock.Mock(return_value=0))
 
-    @property
-    def name(self):
-        return 'myapp_web_1'
+        output = run_log_printer([mock_container])
+        self.assertIn(
+            "WARNING: no logs are available with the 'none' log driver\n",
+            output
+        )
 
-    @property
-    def name_without_project(self):
-        return 'web_1'
 
-    def attach(self, *args, **kwargs):
-        return self._reader()
-
-    def wait(self, *args, **kwargs):
-        return 0
+def run_log_printer(containers, monochrome=False):
+    output = six.StringIO()
+    LogPrinter(containers, output=output, monochrome=monochrome).run()
+    return output.getvalue()