From 413b76e2285f5f6575601385f9b4af7503b41c3e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 9 Sep 2015 17:53:36 -0400 Subject: [PATCH] Fix warning message when a container uses a non-json log driver Signed-off-by: Daniel Nephin --- compose/cli/log_printer.py | 48 ++++++++++++-------- compose/cli/main.py | 8 +--- compose/container.py | 9 ++++ tests/unit/cli/log_printer_test.py | 73 ++++++++++++++++++------------ 4 files changed, 84 insertions(+), 54 deletions(-) diff --git a/compose/cli/log_printer.py b/compose/cli/log_printer.py index c2fcc54fd..49071dd4a 100644 --- a/compose/cli/log_printer.py +++ b/compose/cli/log_printer.py @@ -13,9 +13,9 @@ from compose import utils class LogPrinter(object): - def __init__(self, containers, attach_params=None, output=sys.stdout, monochrome=False): + # TODO: move logic to run + 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) @@ -25,6 +25,7 @@ class LogPrinter(object): for line in mux.loop(): self.output.write(line) + # TODO: doesn't use self, remove from class def _calculate_prefix_width(self, containers): """ Calculate the maximum width of container names so we can make the log @@ -56,14 +57,10 @@ class LogPrinter(object): 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)) + if container.has_api_logs: + return build_log_generator(container, prefix, color_fn) + return build_no_log_generator(container, prefix, color_fn) def _generate_prefix(self, container): """ @@ -73,12 +70,27 @@ class LogPrinter(object): 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 build_no_log_generator(container, prefix, color_fn): + """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_fn(wait_on_exit(container)) + + +def build_log_generator(container, prefix, color_fn): + # 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_fn(wait_on_exit(container)) + + +def wait_on_exit(container): + exit_code = container.wait() + return "%s exited with code %s\n" % (container.name, exit_code) diff --git a/compose/cli/main.py b/compose/cli/main.py index a7b918168..61461ae7b 100644 --- a/compose/cli/main.py +++ b/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): diff --git a/compose/container.py b/compose/container.py index 51b625890..28af093d7 100644 --- a/compose/container.py +++ b/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. diff --git a/tests/unit/cli/log_printer_test.py b/tests/unit/cli/log_printer_test.py index d8fbf94b9..2c9168980 100644 --- a/tests/unit/cli/log_printer_test.py +++ b/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)) + + expected = '{} exited with code {}\n'.format(mock_container.name, exit_status) + self.assertEqual(expected, wait_on_exit(mock_container)) + + 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)) + + output = run_log_printer([mock_container]) + self.assertIn( + "WARNING: no logs are available with the 'none' log driver\n", + output + ) + 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() - - -class MockContainer(object): - def __init__(self, reader): - self._reader = reader - - @property - def name(self): - return 'myapp_web_1' - - @property - def name_without_project(self): - return 'web_1' - - def attach(self, *args, **kwargs): - return self._reader() - - def wait(self, *args, **kwargs): - return 0 + output = six.StringIO() + LogPrinter(containers, output=output, monochrome=monochrome).run() + return output.getvalue()