From 4042121f6e8e302206189bd110c46791ce72bc28 Mon Sep 17 00:00:00 2001 From: Ian Glen Neal Date: Fri, 12 Jan 2018 06:29:16 +0000 Subject: [PATCH] The updated unit test simulates the error in the bug. What's happening is that the container has an image sha (represented by 'shaDOES_NOT_EXIST') which causes an error when client.inspect_image is called on it, because the image has actually been removed. Signed-off-by: Ian Glen Neal --- compose/cli/main.py | 7 +++---- compose/service.py | 24 +++++------------------- tests/unit/service_test.py | 12 ++++++++++-- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/compose/cli/main.py b/compose/cli/main.py index 14f7a745c..ef7ea0c9d 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -971,10 +971,9 @@ class TopLevelCommand(object): if ignore_orphans and remove_orphans: raise UserError("COMPOSE_IGNORE_ORPHANS and --remove-orphans cannot be combined.") - if no_start: - opts = ['-d', '--abort-on-container-exit', '--exit-code-from'] - for excluded in [x for x in opts if options.get(x)]: - raise UserError('--no-start and {} cannot be combined.'.format(excluded)) + opts = ['-d', '--abort-on-container-exit', '--exit-code-from'] + for excluded in [x for x in opts if options.get(x) and no_start]: + raise UserError('--no-start and {} cannot be combined.'.format(excluded)) with up_shutdown_context(self.project, service_names, timeout, detached): def up(rebuild): diff --git a/compose/service.py b/compose/service.py index 195257c39..97455f6c2 100644 --- a/compose/service.py +++ b/compose/service.py @@ -1306,24 +1306,6 @@ def merge_volume_bindings(volumes, tmpfs, previous_container, mounts): return list(volume_bindings.values()), affinity -def try_get_image_volumes(container): - """ - Try to get the volumes from the existing container. If the image does - not exist, raise an exception that will be caught at the CLI level to - prompt user for a rebuild. - """ - - try: - image_volumes = [ - VolumeSpec.parse(volume) - for volume in - container.image_config['ContainerConfig'].get('Volumes') or {} - ] - return image_volumes - except ImageNotFound: - raise - - def get_container_data_volumes(container, volumes_option, tmpfs_option, mounts_option): """ Find the container data volumes that are in `volumes_option`, and return @@ -1339,7 +1321,11 @@ def get_container_data_volumes(container, volumes_option, tmpfs_option, mounts_o for mount in container.get('Mounts') or {} ) - image_volumes = try_get_image_volumes(container) + image_volumes = [ + VolumeSpec.parse(volume) + for volume in + container.image_config['ContainerConfig'].get('Volumes') or {} + ] for volume in set(volumes_option + image_volumes): # No need to preserve host volumes diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index ef250a4d7..bdeaebdd7 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -5,6 +5,7 @@ import docker import pytest from docker.constants import DEFAULT_DOCKER_API_VERSION from docker.errors import APIError +from docker.errors import ImageNotFound from .. import mock from .. import unittest @@ -926,7 +927,7 @@ class ServiceVolumesTest(unittest.TestCase): volumes, _ = get_container_data_volumes(container, options, ['/dev/tmpfs'], []) assert sorted(volumes) == sorted(expected) - def test_get_container_data_volumes_image_is_none(self): + def test_get_container_data_volumes_image_does_not_exist(self): # Issue 5465, check for non-existant image. options = [VolumeSpec.parse(v) for v in [ '/host/volume:/host/volume:ro', @@ -936,8 +937,15 @@ class ServiceVolumesTest(unittest.TestCase): '/dev/tmpfs' ]] + def inspect_fn(image): + if image == 'shaDOES_NOT_EXIST': + raise ImageNotFound("inspect_fn: {}".format(image)) + return {'ContainerConfig': None} + + self.mock_client.inspect_image = inspect_fn + container = Container(self.mock_client, { - 'Image': None, + 'Image': 'shaDOES_NOT_EXIST', 'Mounts': [] }, has_been_inspected=True)