diff --git a/compose/cli/log_printer.py b/compose/cli/log_printer.py index 6e1499e1d..66920726c 100644 --- a/compose/cli/log_printer.py +++ b/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 - stream = container.attach(stdout=True, stderr=True, stream=True, logs=True) - line_generator = split_buffer(stream) + # 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 diff --git a/compose/cli/main.py b/compose/cli/main.py index c800d95f9..5505b89f5 100644 --- a/compose/cli/main.py +++ b/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) diff --git a/compose/container.py b/compose/container.py index a03acf56f..dd69e8ddd 100644 --- a/compose/container.py +++ b/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() diff --git a/compose/project.py b/compose/project.py index d4934c268..68edaddcb 100644 --- a/compose/project.py +++ b/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 ) ] diff --git a/compose/service.py b/compose/service.py index 370ab1eb5..ad29f87f4 100644 --- a/compose/service.py +++ b/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, - timeout=timeout + container, + timeout=timeout, + attach_logs=should_attach_logs ) - for c in containers + for container in containers ] elif action == 'start': - for c in containers: - self.start_container_if_stopped(c) + for container in containers: + 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: - 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)) + container.stop(timeout=timeout) + container.rename_to_tmp_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): diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 8a8e4d54d..38d7d5b55 100644 --- a/tests/integration/service_test.py +++ b/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) diff --git a/tests/integration/state_test.py b/tests/integration/state_test.py index ef7276bd8..3230aefc6 100644 --- a/tests/integration/state_test.py +++ b/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) diff --git a/tests/unit/cli/log_printer_test.py b/tests/unit/cli/log_printer_test.py index 2c9168980..575fcaf7b 100644 --- a/tests/unit/cli/log_printer_test.py +++ b/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), ) diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index c7a5a3557..8c5c888fa 100644 --- a/tests/unit/service_test.py +++ b/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.id, - '%s_%s' % (mock_container.short_id, mock_container.name)) + mock_container.rename_to_tmp_name.assert_called_once_with() new_container.start.assert_called_once_with() mock_container.remove.assert_called_once_with()