diff --git a/CHANGES.md b/CHANGES.md index 1f43d88d4..b87a2e7de 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,21 @@ Change log ========== +1.3.2 (2015-07-14) +------------------ + +The following bugs have been fixed: + +- When there were one-off containers created by running `docker-compose run` on an older version of Compose, `docker-compose run` would fail with a name collision. Compose now shows an error if you have leftover containers of this type lying around, and tells you how to remove them. +- Compose was not reading Docker authentication config files created in the new location, `~/docker/config.json`, and authentication against private registries would therefore fail. +- When a container had a pseudo-TTY attached, its output in `docker-compose up` would be truncated. +- `docker-compose up --x-smart-recreate` would sometimes fail when an image tag was updated. +- `docker-compose up` would sometimes create two containers with the same numeric suffix. +- `docker-compose rm` and `docker-compose ps` would sometimes list services that aren't part of the current project (though no containers were erroneously removed). +- Some `docker-compose` commands would not show an error if invalid service names were passed in. + +Thanks @dano, @josephpage, @kevinsimper, @lieryan, @phemmer, @soulrebel and @sschepens! + 1.3.1 (2015-06-21) ------------------ diff --git a/compose/__init__.py b/compose/__init__.py index f3ec6acb0..1f6957495 100644 --- a/compose/__init__.py +++ b/compose/__init__.py @@ -1,3 +1,3 @@ from __future__ import unicode_literals -__version__ = '1.3.1' +__version__ = '1.3.2' diff --git a/compose/cli/main.py b/compose/cli/main.py index 8aeb0459d..d5d15177c 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -11,6 +11,7 @@ from docker.errors import APIError import dockerpty from .. import legacy +from ..const import DEFAULT_TIMEOUT from ..project import NoSuchService, ConfigurationError from ..service import BuildError, NeedsBuildError from ..config import parse_environment @@ -32,7 +33,7 @@ def main(): except KeyboardInterrupt: log.error("\nAborting.") sys.exit(1) - except (UserError, NoSuchService, ConfigurationError, legacy.LegacyContainersError) as e: + except (UserError, NoSuchService, ConfigurationError, legacy.LegacyError) as e: log.error(e.msg) sys.exit(1) except NoSuchCommand as e: @@ -333,12 +334,22 @@ class TopLevelCommand(Command): if not options['--service-ports']: container_options['ports'] = [] - container = service.create_container( - quiet=True, - one_off=True, - insecure_registry=insecure_registry, - **container_options - ) + try: + container = service.create_container( + quiet=True, + one_off=True, + insecure_registry=insecure_registry, + **container_options + ) + except APIError as e: + legacy.check_for_legacy_containers( + project.client, + project.name, + [service.name], + allow_one_off=False, + ) + + raise e if options['-d']: service.start_container(container) @@ -392,9 +403,8 @@ class TopLevelCommand(Command): -t, --timeout TIMEOUT Specify a shutdown timeout in seconds. (default: 10) """ - timeout = options.get('--timeout') - params = {} if timeout is None else {'timeout': int(timeout)} - project.stop(service_names=options['SERVICE'], **params) + timeout = float(options.get('--timeout') or DEFAULT_TIMEOUT) + project.stop(service_names=options['SERVICE'], timeout=timeout) def restart(self, project, options): """ @@ -406,9 +416,8 @@ class TopLevelCommand(Command): -t, --timeout TIMEOUT Specify a shutdown timeout in seconds. (default: 10) """ - timeout = options.get('--timeout') - params = {} if timeout is None else {'timeout': int(timeout)} - project.restart(service_names=options['SERVICE'], **params) + timeout = float(options.get('--timeout') or DEFAULT_TIMEOUT) + project.restart(service_names=options['SERVICE'], timeout=timeout) def up(self, project, options): """ @@ -437,9 +446,9 @@ class TopLevelCommand(Command): image needs to be updated. (EXPERIMENTAL) --no-recreate If containers already exist, don't recreate them. --no-build Don't build an image, even if it's missing - -t, --timeout TIMEOUT When attached, use this timeout in seconds - for the shutdown. (default: 10) - + -t, --timeout TIMEOUT Use this timeout in seconds for container shutdown + when attached or when containers are already + running. (default: 10) """ insecure_registry = options['--allow-insecure-ssl'] detached = options['-d'] @@ -450,6 +459,7 @@ class TopLevelCommand(Command): allow_recreate = not options['--no-recreate'] smart_recreate = options['--x-smart-recreate'] service_names = options['SERVICE'] + timeout = float(options.get('--timeout') or DEFAULT_TIMEOUT) project.up( service_names=service_names, @@ -458,6 +468,7 @@ class TopLevelCommand(Command): smart_recreate=smart_recreate, insecure_registry=insecure_registry, do_build=not options['--no-build'], + timeout=timeout ) to_attach = [c for s in project.get_services(service_names) for c in s.containers()] @@ -475,9 +486,7 @@ class TopLevelCommand(Command): signal.signal(signal.SIGINT, handler) print("Gracefully stopping... (press Ctrl+C again to force)") - timeout = options.get('--timeout') - params = {} if timeout is None else {'timeout': int(timeout)} - project.stop(service_names=service_names, **params) + project.stop(service_names=service_names, timeout=timeout) def migrate_to_labels(self, project, _options): """ diff --git a/compose/const.py b/compose/const.py index f76fb572c..709c3a10d 100644 --- a/compose/const.py +++ b/compose/const.py @@ -1,4 +1,5 @@ +DEFAULT_TIMEOUT = 10 LABEL_CONTAINER_NUMBER = 'com.docker.compose.container-number' LABEL_ONE_OFF = 'com.docker.compose.oneoff' LABEL_PROJECT = 'com.docker.compose.project' diff --git a/compose/legacy.py b/compose/legacy.py index 340511a76..c9ec65817 100644 --- a/compose/legacy.py +++ b/compose/legacy.py @@ -1,6 +1,7 @@ import logging import re +from .const import LABEL_VERSION from .container import get_container_name, Container @@ -24,41 +25,82 @@ Alternatively, remove them: $ docker rm -f {rm_args} """ +ONE_OFF_ADDENDUM_FORMAT = """ +You should also remove your one-off containers: + + $ docker rm -f {rm_args} +""" + +ONE_OFF_ERROR_MESSAGE_FORMAT = """ +Compose found the following containers without labels: + +{names_list} + +As of Compose 1.3.0, containers are identified with labels instead of naming convention. + +Remove them before continuing: + + $ docker rm -f {rm_args} +""" + def check_for_legacy_containers( client, project, services, - stopped=False, - one_off=False): + allow_one_off=True): """Check if there are containers named using the old naming convention and warn the user that those containers may need to be migrated to using labels, so that compose can find them. """ - containers = list(get_legacy_containers( - client, - project, - services, - stopped=stopped, - one_off=one_off)) + containers = get_legacy_containers(client, project, services, one_off=False) if containers: - raise LegacyContainersError([c.name for c in containers]) + one_off_containers = get_legacy_containers(client, project, services, one_off=True) + + raise LegacyContainersError( + [c.name for c in containers], + [c.name for c in one_off_containers], + ) + + if not allow_one_off: + one_off_containers = get_legacy_containers(client, project, services, one_off=True) + + if one_off_containers: + raise LegacyOneOffContainersError( + [c.name for c in one_off_containers], + ) -class LegacyContainersError(Exception): - def __init__(self, names): +class LegacyError(Exception): + def __unicode__(self): + return self.msg + + __str__ = __unicode__ + + +class LegacyContainersError(LegacyError): + def __init__(self, names, one_off_names): self.names = names + self.one_off_names = one_off_names self.msg = ERROR_MESSAGE_FORMAT.format( names_list="\n".join(" {}".format(name) for name in names), rm_args=" ".join(names), ) - def __unicode__(self): - return self.msg + if one_off_names: + self.msg += ONE_OFF_ADDENDUM_FORMAT.format(rm_args=" ".join(one_off_names)) - __str__ = __unicode__ + +class LegacyOneOffContainersError(LegacyError): + def __init__(self, one_off_names): + self.one_off_names = one_off_names + + self.msg = ONE_OFF_ERROR_MESSAGE_FORMAT.format( + names_list="\n".join(" {}".format(name) for name in one_off_names), + rm_args=" ".join(one_off_names), + ) def add_labels(project, container): @@ -76,8 +118,8 @@ def migrate_project_to_labels(project): project.client, project.name, project.service_names, - stopped=True, - one_off=False) + one_off=False, + ) for container in containers: add_labels(project, container) @@ -87,13 +129,29 @@ def get_legacy_containers( client, project, services, - stopped=False, one_off=False): - containers = client.containers(all=stopped) + return list(_get_legacy_containers_iter( + client, + project, + services, + one_off=one_off, + )) + + +def _get_legacy_containers_iter( + client, + project, + services, + one_off=False): + + containers = client.containers(all=True) for service in services: for container in containers: + if LABEL_VERSION in container['Labels']: + continue + name = get_container_name(container) if has_container(project, service, name, one_off=one_off): yield Container.from_ps(client, container) diff --git a/compose/project.py b/compose/project.py index 6446a6d33..11c1e1ce9 100644 --- a/compose/project.py +++ b/compose/project.py @@ -6,7 +6,7 @@ from functools import reduce from docker.errors import APIError from .config import get_service_name_from_net, ConfigurationError -from .const import LABEL_PROJECT, LABEL_SERVICE, LABEL_ONE_OFF +from .const import LABEL_PROJECT, LABEL_SERVICE, LABEL_ONE_OFF, DEFAULT_TIMEOUT from .service import Service from .container import Container from .legacy import check_for_legacy_containers @@ -99,6 +99,16 @@ class Project(object): raise NoSuchService(name) + def validate_service_names(self, service_names): + """ + Validate that the given list of service names only contains valid + services. Raises NoSuchService if one of the names is invalid. + """ + valid_names = self.service_names + for name in service_names: + if name not in valid_names: + raise NoSuchService(name) + def get_services(self, service_names=None, include_deps=False): """ Returns a list of this project's services filtered @@ -211,10 +221,14 @@ class Project(object): allow_recreate=True, smart_recreate=False, insecure_registry=False, - do_build=True): + do_build=True, + timeout=DEFAULT_TIMEOUT): services = self.get_services(service_names, include_deps=start_deps) + for service in services: + service.remove_duplicate_containers() + plans = self._get_convergence_plans( services, allow_recreate=allow_recreate, @@ -228,6 +242,7 @@ class Project(object): plans[service.name], insecure_registry=insecure_registry, do_build=do_build, + timeout=timeout ) ] @@ -274,6 +289,11 @@ class Project(object): service.remove_stopped(**options) def containers(self, service_names=None, stopped=False, one_off=False): + if service_names: + self.validate_service_names(service_names) + else: + service_names = self.service_names + containers = [ Container.from_ps(self.client, container) for container in self.client.containers( @@ -281,8 +301,6 @@ class Project(object): filters={'label': self.labels(one_off=one_off)})] def matches_service_names(container): - if not service_names: - return True return container.labels.get(LABEL_SERVICE) in service_names if not containers: @@ -290,8 +308,7 @@ class Project(object): self.client, self.name, self.service_names, - stopped=stopped, - one_off=one_off) + ) return filter(matches_service_names, containers) diff --git a/compose/service.py b/compose/service.py index 6c2cc4da5..2bf22eac3 100644 --- a/compose/service.py +++ b/compose/service.py @@ -13,6 +13,7 @@ from docker.utils import create_host_config, LogConfig from . import __version__ from .config import DOCKER_CONFIG_KEYS, merge_environment from .const import ( + DEFAULT_TIMEOUT, LABEL_CONTAINER_NUMBER, LABEL_ONE_OFF, LABEL_PROJECT, @@ -64,6 +65,10 @@ class NeedsBuildError(Exception): self.service = service +class NoSuchImageError(Exception): + pass + + VolumeSpec = namedtuple('VolumeSpec', 'external internal mode') @@ -105,8 +110,7 @@ class Service(object): self.client, self.project, [self.name], - stopped=stopped, - one_off=one_off) + ) return containers @@ -224,8 +228,11 @@ class Service(object): do_build=True, insecure_registry=False): - if self.image(): + try: + self.image() return + except NoSuchImageError: + pass if self.can_be_built(): if do_build: @@ -240,7 +247,7 @@ class Service(object): return self.client.inspect_image(self.image_name) except APIError as e: if e.response.status_code == 404 and e.explanation and 'No such image' in str(e.explanation): - return None + raise NoSuchImageError("Image '{}' not found".format(self.image_name)) else: raise @@ -251,26 +258,6 @@ class Service(object): else: return self.options['image'] - def converge(self, - allow_recreate=True, - smart_recreate=False, - insecure_registry=False, - do_build=True): - """ - If a container for this service doesn't exist, create and start one. If there are - any, stop them, create+start new ones, and remove the old containers. - """ - plan = self.convergence_plan( - allow_recreate=allow_recreate, - smart_recreate=smart_recreate, - ) - - return self.execute_convergence_plan( - plan, - insecure_registry=insecure_registry, - do_build=do_build, - ) - def convergence_plan(self, allow_recreate=True, smart_recreate=False): @@ -294,7 +281,17 @@ class Service(object): return ConvergencePlan('recreate', containers) def _containers_have_diverged(self, containers): - config_hash = self.config_hash() + config_hash = None + + try: + config_hash = self.config_hash() + except NoSuchImageError as e: + log.debug( + 'Service %s has diverged: %s', + self.name, six.text_type(e), + ) + return True + has_diverged = False for c in containers: @@ -311,7 +308,8 @@ class Service(object): def execute_convergence_plan(self, plan, insecure_registry=False, - do_build=True): + do_build=True, + timeout=DEFAULT_TIMEOUT): (action, containers) = plan if action == 'create': @@ -328,6 +326,7 @@ class Service(object): self.recreate_container( c, insecure_registry=insecure_registry, + timeout=timeout ) for c in containers ] @@ -349,7 +348,8 @@ class Service(object): def recreate_container(self, container, - insecure_registry=False): + insecure_registry=False, + timeout=DEFAULT_TIMEOUT): """Recreate a container. The original container is renamed to a temporary name so that data @@ -358,7 +358,7 @@ class Service(object): """ log.info("Recreating %s..." % container.name) try: - container.stop() + container.stop(timeout=timeout) except APIError as e: if (e.response.status_code == 500 and e.explanation @@ -394,6 +394,26 @@ class Service(object): container.start() return container + def remove_duplicate_containers(self, timeout=DEFAULT_TIMEOUT): + for c in self.duplicate_containers(): + log.info('Removing %s...' % c.name) + c.stop(timeout=timeout) + c.remove() + + def duplicate_containers(self): + containers = sorted( + self.containers(stopped=True), + key=lambda c: c.get('Created'), + ) + + numbers = set() + + for c in containers: + if c.number in numbers: + yield c + else: + numbers.add(c.number) + def config_hash(self): return json_hash(self.config_dict()) diff --git a/docs/install.md b/docs/install.md index 96a4a2376..cdaac34f3 100644 --- a/docs/install.md +++ b/docs/install.md @@ -27,7 +27,7 @@ First, install Docker version 1.6 or greater: To install Compose, run the following commands: - curl -L https://github.com/docker/compose/releases/download/1.3.1/docker-compose-`uname -s`-`uname -m` > /usr/local/bin/docker-compose + curl -L https://github.com/docker/compose/releases/download/1.3.2/docker-compose-`uname -s`-`uname -m` > /usr/local/bin/docker-compose chmod +x /usr/local/bin/docker-compose > Note: If you get a "Permission denied" error, your `/usr/local/bin` directory probably isn't writable and you'll need to install Compose as the superuser. Run `sudo -i`, then the two commands above, then `exit`. diff --git a/requirements.txt b/requirements.txt index 69bd4c5f9..fc5b68489 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,8 +1,8 @@ PyYAML==3.10 -docker-py==1.2.3 +docker-py==1.3.0 dockerpty==0.3.4 docopt==0.6.1 requests==2.6.1 six==1.7.3 texttable==0.8.2 -websocket-client==0.11.0 +websocket-client==0.32.0 diff --git a/setup.py b/setup.py index d2e81e175..d0ec10679 100644 --- a/setup.py +++ b/setup.py @@ -30,7 +30,7 @@ install_requires = [ 'requests >= 2.6.1, < 2.7', 'texttable >= 0.8.1, < 0.9', 'websocket-client >= 0.11.0, < 1.0', - 'docker-py >= 1.2.3, < 1.3', + 'docker-py >= 1.3.0, < 1.4', 'dockerpty >= 0.3.4, < 0.4', 'six >= 1.3.0, < 2', ] diff --git a/tests/fixtures/build-ctx/Dockerfile b/tests/fixtures/build-ctx/Dockerfile index d1ceac6b7..dd864b838 100644 --- a/tests/fixtures/build-ctx/Dockerfile +++ b/tests/fixtures/build-ctx/Dockerfile @@ -1,2 +1,3 @@ FROM busybox:latest +LABEL com.docker.compose.test_image=true CMD echo "success" diff --git a/tests/fixtures/dockerfile-with-volume/Dockerfile b/tests/fixtures/dockerfile-with-volume/Dockerfile index 6e5d0a55e..0d376ec48 100644 --- a/tests/fixtures/dockerfile-with-volume/Dockerfile +++ b/tests/fixtures/dockerfile-with-volume/Dockerfile @@ -1,3 +1,4 @@ -FROM busybox +FROM busybox:latest +LABEL com.docker.compose.test_image=true VOLUME /data CMD top diff --git a/tests/fixtures/dockerfile_with_entrypoint/Dockerfile b/tests/fixtures/dockerfile_with_entrypoint/Dockerfile index 7d28d2933..e7454e59b 100644 --- a/tests/fixtures/dockerfile_with_entrypoint/Dockerfile +++ b/tests/fixtures/dockerfile_with_entrypoint/Dockerfile @@ -1,2 +1,3 @@ FROM busybox:latest +LABEL com.docker.compose.test_image=true ENTRYPOINT echo "From prebuilt entrypoint" diff --git a/tests/fixtures/simple-dockerfile/Dockerfile b/tests/fixtures/simple-dockerfile/Dockerfile index d1ceac6b7..dd864b838 100644 --- a/tests/fixtures/simple-dockerfile/Dockerfile +++ b/tests/fixtures/simple-dockerfile/Dockerfile @@ -1,2 +1,3 @@ FROM busybox:latest +LABEL com.docker.compose.test_image=true CMD echo "success" diff --git a/tests/integration/cli_test.py b/tests/integration/cli_test.py index 2d1f1f76e..421d59857 100644 --- a/tests/integration/cli_test.py +++ b/tests/integration/cli_test.py @@ -9,6 +9,7 @@ from mock import patch from .testcases import DockerClientTestCase from compose.cli.main import TopLevelCommand +from compose.project import NoSuchService class CLITestCase(DockerClientTestCase): @@ -25,6 +26,7 @@ class CLITestCase(DockerClientTestCase): self.project.remove_stopped() for container in self.project.containers(stopped=True, one_off=True): container.remove(force=True) + super(CLITestCase, self).tearDown() @property def project(self): @@ -162,6 +164,19 @@ class CLITestCase(DockerClientTestCase): self.assertEqual(old_ids, new_ids) + def test_up_with_timeout(self): + self.command.dispatch(['up', '-d', '-t', '1'], None) + service = self.project.get_service('simple') + another = self.project.get_service('another') + self.assertEqual(len(service.containers()), 1) + self.assertEqual(len(another.containers()), 1) + + # Ensure containers don't have stdin and stdout connected in -d mode + config = service.containers()[0].inspect()['Config'] + self.assertFalse(config['AttachStderr']) + self.assertFalse(config['AttachStdout']) + self.assertFalse(config['AttachStdin']) + @patch('dockerpty.start') def test_run_service_without_links(self, mock_stdout): self.command.base_dir = 'tests/fixtures/links-composefile' @@ -208,13 +223,10 @@ class CLITestCase(DockerClientTestCase): self.assertEqual(old_ids, new_ids) @patch('dockerpty.start') - def test_run_without_command(self, __): + def test_run_without_command(self, _): self.command.base_dir = 'tests/fixtures/commands-composefile' self.check_build('tests/fixtures/simple-dockerfile', tag='composetest_test') - for c in self.project.containers(stopped=True, one_off=True): - c.remove() - self.command.dispatch(['run', 'implicit'], None) service = self.project.get_service('implicit') containers = service.containers(stopped=True, one_off=True) @@ -351,6 +363,10 @@ class CLITestCase(DockerClientTestCase): self.assertEqual(len(service.containers(stopped=True)), 1) self.assertFalse(service.containers(stopped=True)[0].is_running) + def test_logs_invalid_service_name(self): + with self.assertRaises(NoSuchService): + self.command.dispatch(['logs', 'madeupname'], None) + def test_kill(self): self.command.dispatch(['up', '-d'], None) service = self.project.get_service('simple') diff --git a/tests/integration/legacy_test.py b/tests/integration/legacy_test.py index 6c52b68d3..806b9a457 100644 --- a/tests/integration/legacy_test.py +++ b/tests/integration/legacy_test.py @@ -1,12 +1,75 @@ +import unittest + +from docker.errors import APIError + from compose import legacy from compose.project import Project from .testcases import DockerClientTestCase -class ProjectTest(DockerClientTestCase): +class UtilitiesTestCase(unittest.TestCase): + def test_has_container(self): + self.assertTrue( + legacy.has_container("composetest", "web", "composetest_web_1", one_off=False), + ) + self.assertFalse( + legacy.has_container("composetest", "web", "composetest_web_run_1", one_off=False), + ) + + def test_has_container_one_off(self): + self.assertFalse( + legacy.has_container("composetest", "web", "composetest_web_1", one_off=True), + ) + self.assertTrue( + legacy.has_container("composetest", "web", "composetest_web_run_1", one_off=True), + ) + + def test_has_container_different_project(self): + self.assertFalse( + legacy.has_container("composetest", "web", "otherapp_web_1", one_off=False), + ) + self.assertFalse( + legacy.has_container("composetest", "web", "otherapp_web_run_1", one_off=True), + ) + + def test_has_container_different_service(self): + self.assertFalse( + legacy.has_container("composetest", "web", "composetest_db_1", one_off=False), + ) + self.assertFalse( + legacy.has_container("composetest", "web", "composetest_db_run_1", one_off=True), + ) + + def test_is_valid_name(self): + self.assertTrue( + legacy.is_valid_name("composetest_web_1", one_off=False), + ) + self.assertFalse( + legacy.is_valid_name("composetest_web_run_1", one_off=False), + ) + + def test_is_valid_name_one_off(self): + self.assertFalse( + legacy.is_valid_name("composetest_web_1", one_off=True), + ) + self.assertTrue( + legacy.is_valid_name("composetest_web_run_1", one_off=True), + ) + + def test_is_valid_name_invalid(self): + self.assertFalse( + legacy.is_valid_name("foo"), + ) + self.assertFalse( + legacy.is_valid_name("composetest_web_lol_1", one_off=True), + ) + + +class LegacyTestCase(DockerClientTestCase): def setUp(self): - super(ProjectTest, self).setUp() + super(LegacyTestCase, self).setUp() + self.containers = [] db = self.create_service('db') web = self.create_service('web', links=[(db, 'db')]) @@ -23,35 +86,105 @@ class ProjectTest(DockerClientTestCase): **service.options ) self.client.start(container) + self.containers.append(container) # Create a single one-off legacy container - self.client.create_container( - name='{}_{}_run_1'.format(self.project.name, self.services[0].name), + self.containers.append(self.client.create_container( + name='{}_{}_run_1'.format(self.project.name, db.name), **self.services[0].options - ) + )) + + def tearDown(self): + super(LegacyTestCase, self).tearDown() + for container in self.containers: + try: + self.client.kill(container) + except APIError: + pass + try: + self.client.remove_container(container) + except APIError: + pass def get_legacy_containers(self, **kwargs): - return list(legacy.get_legacy_containers( + return legacy.get_legacy_containers( self.client, self.project.name, [s.name for s in self.services], **kwargs - )) + ) def test_get_legacy_container_names(self): self.assertEqual(len(self.get_legacy_containers()), len(self.services)) def test_get_legacy_container_names_one_off(self): - self.assertEqual(len(self.get_legacy_containers(stopped=True, one_off=True)), 1) + self.assertEqual(len(self.get_legacy_containers(one_off=True)), 1) def test_migration_to_labels(self): + # Trying to get the container list raises an exception + with self.assertRaises(legacy.LegacyContainersError) as cm: - self.assertEqual(self.project.containers(stopped=True), []) + self.project.containers(stopped=True) self.assertEqual( set(cm.exception.names), set(['composetest_db_1', 'composetest_web_1', 'composetest_nginx_1']), ) + self.assertEqual( + set(cm.exception.one_off_names), + set(['composetest_db_run_1']), + ) + + # Migrate the containers + legacy.migrate_project_to_labels(self.project) - self.assertEqual(len(self.project.containers(stopped=True)), len(self.services)) + + # Getting the list no longer raises an exception + + containers = self.project.containers(stopped=True) + self.assertEqual(len(containers), len(self.services)) + + def test_migration_one_off(self): + # We've already migrated + + legacy.migrate_project_to_labels(self.project) + + # Trying to create a one-off container results in a Docker API error + + with self.assertRaises(APIError) as cm: + self.project.get_service('db').create_container(one_off=True) + + # Checking for legacy one-off containers raises an exception + + with self.assertRaises(legacy.LegacyOneOffContainersError) as cm: + legacy.check_for_legacy_containers( + self.client, + self.project.name, + ['db'], + allow_one_off=False, + ) + + self.assertEqual( + set(cm.exception.one_off_names), + set(['composetest_db_run_1']), + ) + + # Remove the old one-off container + + c = self.client.inspect_container('composetest_db_run_1') + self.client.remove_container(c) + + # Checking no longer raises an exception + + legacy.check_for_legacy_containers( + self.client, + self.project.name, + ['db'], + allow_one_off=False, + ) + + # Creating a one-off container no longer results in an API error + + self.project.get_service('db').create_container(one_off=True) + self.assertIsInstance(self.client.inspect_container('composetest_db_run_1'), dict) diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index 2976af823..505f509ea 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -1,5 +1,7 @@ from __future__ import unicode_literals + from compose import config +from compose.const import LABEL_PROJECT from compose.project import Project from compose.container import Container from .testcases import DockerClientTestCase @@ -29,6 +31,21 @@ class ProjectTest(DockerClientTestCase): [c.name for c in containers], ['composetest_web_1']) + def test_containers_with_extra_service(self): + web = self.create_service('web') + web_1 = web.create_container() + + db = self.create_service('db') + db_1 = db.create_container() + + self.create_service('extra').create_container() + + project = Project('composetest', [web, db], self.client) + self.assertEqual( + set(project.containers(stopped=True)), + set([web_1, db_1]), + ) + def test_volumes_from_service(self): service_dicts = config.from_dictionary({ 'data': { @@ -55,6 +72,7 @@ class ProjectTest(DockerClientTestCase): image='busybox:latest', volumes=['/var/data'], name='composetest_data_container', + labels={LABEL_PROJECT: 'composetest'}, ) project = Project.from_dicts( name='composetest', @@ -69,9 +87,6 @@ class ProjectTest(DockerClientTestCase): db = project.get_service('db') self.assertEqual(db.volumes_from, [data_container]) - project.kill() - project.remove_stopped() - def test_net_from_service(self): project = Project.from_dicts( name='composetest', @@ -95,15 +110,13 @@ class ProjectTest(DockerClientTestCase): net = project.get_service('net') self.assertEqual(web._get_net(), 'container:' + net.containers()[0].id) - project.kill() - project.remove_stopped() - def test_net_from_container(self): net_container = Container.create( self.client, image='busybox:latest', name='composetest_net_container', - command='top' + command='top', + labels={LABEL_PROJECT: 'composetest'}, ) net_container.start() @@ -123,9 +136,6 @@ class ProjectTest(DockerClientTestCase): web = project.get_service('web') self.assertEqual(web._get_net(), 'container:' + net_container.id) - project.kill() - project.remove_stopped() - def test_start_stop_kill_remove(self): web = self.create_service('web') db = self.create_service('db') @@ -171,9 +181,6 @@ class ProjectTest(DockerClientTestCase): self.assertEqual(len(db.containers()), 1) self.assertEqual(len(web.containers()), 0) - project.kill() - project.remove_stopped() - def test_project_up_starts_uncreated_services(self): db = self.create_service('db') web = self.create_service('web', links=[(db, 'db')]) @@ -205,9 +212,6 @@ class ProjectTest(DockerClientTestCase): self.assertNotEqual(db_container.id, old_db_id) self.assertEqual(db_container.get('Volumes./etc'), db_volume_path) - project.kill() - project.remove_stopped() - def test_project_up_with_no_recreate_running(self): web = self.create_service('web') db = self.create_service('db', volumes=['/var/db']) @@ -228,9 +232,6 @@ class ProjectTest(DockerClientTestCase): self.assertEqual(db_container.inspect()['Volumes']['/var/db'], db_volume_path) - project.kill() - project.remove_stopped() - def test_project_up_with_no_recreate_stopped(self): web = self.create_service('web') db = self.create_service('db', volumes=['/var/db']) @@ -258,9 +259,6 @@ class ProjectTest(DockerClientTestCase): self.assertEqual(db_container.inspect()['Volumes']['/var/db'], db_volume_path) - project.kill() - project.remove_stopped() - def test_project_up_without_all_services(self): console = self.create_service('console') db = self.create_service('db') @@ -273,9 +271,6 @@ class ProjectTest(DockerClientTestCase): self.assertEqual(len(db.containers()), 1) self.assertEqual(len(console.containers()), 1) - project.kill() - project.remove_stopped() - def test_project_up_starts_links(self): console = self.create_service('console') db = self.create_service('db', volumes=['/var/db']) @@ -291,9 +286,6 @@ class ProjectTest(DockerClientTestCase): self.assertEqual(len(db.containers()), 1) self.assertEqual(len(console.containers()), 0) - project.kill() - project.remove_stopped() - def test_project_up_starts_depends(self): project = Project.from_dicts( name='composetest', @@ -329,9 +321,6 @@ class ProjectTest(DockerClientTestCase): self.assertEqual(len(project.get_service('data').containers()), 1) self.assertEqual(len(project.get_service('console').containers()), 0) - project.kill() - project.remove_stopped() - def test_project_up_with_no_deps(self): project = Project.from_dicts( name='composetest', @@ -368,9 +357,6 @@ class ProjectTest(DockerClientTestCase): self.assertEqual(len(project.get_service('data').containers(stopped=True)), 1) self.assertEqual(len(project.get_service('console').containers()), 0) - project.kill() - project.remove_stopped() - def test_unscale_after_restart(self): web = self.create_service('web') project = Project('composetest', [web], self.client) @@ -395,5 +381,3 @@ class ProjectTest(DockerClientTestCase): project.up() service = project.get_service('web') self.assertEqual(len(service.containers()), 1) - project.kill() - project.remove_stopped() diff --git a/tests/integration/resilience_test.py b/tests/integration/resilience_test.py index 8229e9d3c..392490902 100644 --- a/tests/integration/resilience_test.py +++ b/tests/integration/resilience_test.py @@ -8,25 +8,36 @@ from .testcases import DockerClientTestCase class ResilienceTest(DockerClientTestCase): - def test_recreate_fails(self): - db = self.create_service('db', volumes=['/var/db'], command='top') - project = Project('composetest', [db], self.client) + def setUp(self): + self.db = self.create_service('db', volumes=['/var/db'], command='top') + self.project = Project('composetest', [self.db], self.client) - container = db.create_container() - db.start_container(container) - host_path = container.get('Volumes')['/var/db'] + container = self.db.create_container() + self.db.start_container(container) + self.host_path = container.get('Volumes')['/var/db'] - project.up() - container = db.containers()[0] - self.assertEqual(container.get('Volumes')['/var/db'], host_path) + def test_successful_recreate(self): + self.project.up() + container = self.db.containers()[0] + self.assertEqual(container.get('Volumes')['/var/db'], self.host_path) + def test_create_failure(self): with mock.patch('compose.service.Service.create_container', crash): with self.assertRaises(Crash): - project.up() + self.project.up() - project.up() - container = db.containers()[0] - self.assertEqual(container.get('Volumes')['/var/db'], host_path) + self.project.up() + container = self.db.containers()[0] + self.assertEqual(container.get('Volumes')['/var/db'], self.host_path) + + def test_start_failure(self): + with mock.patch('compose.service.Service.start_container', crash): + with self.assertRaises(Crash): + self.project.up() + + self.project.up() + container = self.db.containers()[0] + self.assertEqual(container.get('Volumes')['/var/db'], self.host_path) class Crash(Exception): diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 32de5fa47..bf12be7f2 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -2,8 +2,9 @@ from __future__ import unicode_literals from __future__ import absolute_import import os from os import path -import mock +from docker.errors import APIError +import mock import tempfile import shutil import six @@ -18,11 +19,11 @@ from compose.const import ( ) from compose.service import ( ConfigError, + ConvergencePlan, Service, build_extra_hosts, ) from compose.container import Container -from docker.errors import APIError from .testcases import DockerClientTestCase @@ -235,7 +236,12 @@ class ServiceTest(DockerClientTestCase): def test_create_container_with_volumes_from(self): volume_service = self.create_service('data') volume_container_1 = volume_service.create_container() - volume_container_2 = Container.create(self.client, image='busybox:latest', command=["top"]) + volume_container_2 = Container.create( + self.client, + image='busybox:latest', + command=["top"], + labels={LABEL_PROJECT: 'composetest'}, + ) host_service = self.create_service('host', volumes_from=[volume_service, volume_container_2]) host_container = host_service.create_container() host_service.start_container(host_container) @@ -244,7 +250,7 @@ class ServiceTest(DockerClientTestCase): self.assertIn(volume_container_2.id, host_container.get('HostConfig.VolumesFrom')) - def test_converge(self): + def test_execute_convergence_plan_recreate(self): service = self.create_service( 'db', environment={'FOO': '1'}, @@ -264,7 +270,8 @@ class ServiceTest(DockerClientTestCase): num_containers_before = len(self.client.containers(all=True)) service.options['environment']['FOO'] = '2' - new_container = service.converge()[0] + new_container, = service.execute_convergence_plan( + ConvergencePlan('recreate', [old_container])) self.assertEqual(new_container.get('Config.Entrypoint'), ['top']) self.assertEqual(new_container.get('Config.Cmd'), ['-d', '1']) @@ -281,7 +288,7 @@ class ServiceTest(DockerClientTestCase): self.client.inspect_container, old_container.id) - def test_converge_when_containers_are_stopped(self): + def test_execute_convergence_plan_when_containers_are_stopped(self): service = self.create_service( 'db', environment={'FOO': '1'}, @@ -290,11 +297,21 @@ class ServiceTest(DockerClientTestCase): command=['-d', '1'] ) service.create_container() - self.assertEqual(len(service.containers(stopped=True)), 1) - service.converge() - self.assertEqual(len(service.containers(stopped=True)), 1) - def test_converge_with_image_declared_volume(self): + containers = service.containers(stopped=True) + self.assertEqual(len(containers), 1) + container, = containers + self.assertFalse(container.is_running) + + service.execute_convergence_plan(ConvergencePlan('start', [container])) + + containers = service.containers() + self.assertEqual(len(containers), 1) + container.inspect() + self.assertEqual(container, containers[0]) + self.assertTrue(container.is_running) + + def test_execute_convergence_plan_with_image_declared_volume(self): service = Service( project='composetest', name='db', @@ -306,7 +323,8 @@ class ServiceTest(DockerClientTestCase): self.assertEqual(old_container.get('Volumes').keys(), ['/data']) volume_path = old_container.get('Volumes')['/data'] - new_container = service.converge()[0] + new_container, = service.execute_convergence_plan( + ConvergencePlan('recreate', [old_container])) self.assertEqual(new_container.get('Volumes').keys(), ['/data']) self.assertEqual(new_container.get('Volumes')['/data'], volume_path) @@ -408,7 +426,7 @@ class ServiceTest(DockerClientTestCase): self.assertEqual(len(self.client.images(name='composetest_test')), 1) def test_start_container_uses_tagged_image_if_it_exists(self): - self.client.build('tests/fixtures/simple-dockerfile', tag='composetest_test') + self.check_build('tests/fixtures/simple-dockerfile', tag='composetest_test') service = Service( name='test', client=self.client, @@ -705,3 +723,18 @@ class ServiceTest(DockerClientTestCase): self.assertEqual(1, len(device_config)) self.assertDictEqual(device_dict, device_config[0]) + + def test_duplicate_containers(self): + service = self.create_service('web') + + options = service._get_container_create_options({}, 1) + original = Container.create(service.client, **options) + + self.assertEqual(set(service.containers(stopped=True)), set([original])) + self.assertEqual(set(service.duplicate_containers()), set()) + + options['name'] = 'temporary_container_name' + duplicate = Container.create(service.client, **options) + + self.assertEqual(set(service.containers(stopped=True)), set([original, duplicate])) + self.assertEqual(set(service.duplicate_containers()), set([duplicate])) diff --git a/tests/integration/state_test.py b/tests/integration/state_test.py index 7a7d2b58f..95fcc49de 100644 --- a/tests/integration/state_test.py +++ b/tests/integration/state_test.py @@ -12,8 +12,8 @@ from .testcases import DockerClientTestCase class ProjectTestCase(DockerClientTestCase): def run_up(self, cfg, **kwargs): - if 'smart_recreate' not in kwargs: - kwargs['smart_recreate'] = True + kwargs.setdefault('smart_recreate', True) + kwargs.setdefault('timeout', 0.1) project = self.make_project(cfg) project.up(**kwargs) @@ -153,7 +153,31 @@ class ProjectWithDependenciesTest(ProjectTestCase): self.assertEqual(new_containers - old_containers, set()) +def converge(service, + allow_recreate=True, + smart_recreate=False, + insecure_registry=False, + do_build=True): + """ + If a container for this service doesn't exist, create and start one. If there are + any, stop them, create+start new ones, and remove the old containers. + """ + plan = service.convergence_plan( + allow_recreate=allow_recreate, + smart_recreate=smart_recreate, + ) + + return service.execute_convergence_plan( + plan, + insecure_registry=insecure_registry, + do_build=do_build, + timeout=0.1, + ) + + class ServiceStateTest(DockerClientTestCase): + """Test cases for Service.convergence_plan.""" + def test_trigger_create(self): web = self.create_service('web') self.assertEqual(('create', []), web.convergence_plan(smart_recreate=True)) @@ -191,6 +215,13 @@ class ServiceStateTest(DockerClientTestCase): web = self.create_service('web', command=["top", "-d", "1"]) self.assertEqual(('recreate', [container]), web.convergence_plan(smart_recreate=True)) + def test_trigger_recreate_with_nonexistent_image_tag(self): + web = self.create_service('web', image="busybox:latest") + container = web.create_container() + + web = self.create_service('web', image="nonexistent-image") + self.assertEqual(('recreate', [container]), web.convergence_plan(smart_recreate=True)) + def test_trigger_recreate_with_image_change(self): repo = 'composetest_myimage' tag = 'latest' @@ -216,18 +247,19 @@ class ServiceStateTest(DockerClientTestCase): def test_trigger_recreate_with_build(self): context = tempfile.mkdtemp() + base_image = "FROM busybox\nLABEL com.docker.compose.test_image=true\n" try: dockerfile = os.path.join(context, 'Dockerfile') with open(dockerfile, 'w') as f: - f.write('FROM busybox\n') + f.write(base_image) web = self.create_service('web', build=context) container = web.create_container() with open(dockerfile, 'w') as f: - f.write('FROM busybox\nCMD echo hello world\n') + f.write(base_image + 'CMD echo hello world\n') web.build() web = self.create_service('web', build=context) @@ -249,15 +281,15 @@ class ConfigHashTest(DockerClientTestCase): def test_config_hash_with_custom_labels(self): web = self.create_service('web', labels={'foo': '1'}) - container = web.converge()[0] + container = converge(web)[0] self.assertIn(LABEL_CONFIG_HASH, container.labels) self.assertIn('foo', container.labels) def test_config_hash_sticks_around(self): web = self.create_service('web', command=["top"]) - container = web.converge()[0] + container = converge(web)[0] self.assertIn(LABEL_CONFIG_HASH, container.labels) web = self.create_service('web', command=["top", "-d", "1"]) - container = web.converge()[0] + container = converge(web)[0] self.assertIn(LABEL_CONFIG_HASH, container.labels) diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index 48fcf3ef2..98c5876eb 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -2,6 +2,7 @@ from __future__ import unicode_literals from __future__ import absolute_import from compose.service import Service from compose.config import make_service_dict +from compose.const import LABEL_PROJECT from compose.cli.docker_client import docker_client from compose.progress_stream import stream_output from .. import unittest @@ -12,15 +13,15 @@ class DockerClientTestCase(unittest.TestCase): def setUpClass(cls): cls.client = docker_client() - # TODO: update to use labels in #652 - def setUp(self): - for c in self.client.containers(all=True): - if c['Names'] and 'composetest' in c['Names'][0]: - self.client.kill(c['Id']) - self.client.remove_container(c['Id']) - for i in self.client.images(): - if isinstance(i.get('Tag'), basestring) and 'composetest' in i['Tag']: - self.client.remove_image(i) + def tearDown(self): + for c in self.client.containers( + all=True, + filters={'label': '%s=composetest' % LABEL_PROJECT}): + self.client.kill(c['Id']) + self.client.remove_container(c['Id']) + for i in self.client.images( + filters={'label': 'com.docker.compose.test_image'}): + self.client.remove_image(i) def create_service(self, name, **kwargs): if 'image' not in kwargs and 'build' not in kwargs: @@ -36,5 +37,6 @@ class DockerClientTestCase(unittest.TestCase): ) def check_build(self, *args, **kwargs): + kwargs.setdefault('rm', True) build_output = self.client.build(*args, **kwargs) stream_output(build_output, open('/dev/null', 'w')) diff --git a/tests/unit/cli_test.py b/tests/unit/cli_test.py index d10cb9b30..ab3ea56a9 100644 --- a/tests/unit/cli_test.py +++ b/tests/unit/cli_test.py @@ -127,7 +127,7 @@ class CLITestCase(unittest.TestCase): def test_run_with_environment_merged_with_options_list(self, mock_dockerpty): command = TopLevelCommand() mock_client = mock.create_autospec(docker.Client) - mock_project = mock.Mock() + mock_project = mock.Mock(client=mock_client) mock_project.get_service.return_value = Service( 'service', client=mock_client, @@ -156,7 +156,7 @@ class CLITestCase(unittest.TestCase): def test_run_service_with_restart_always(self): command = TopLevelCommand() mock_client = mock.create_autospec(docker.Client) - mock_project = mock.Mock() + mock_project = mock.Mock(client=mock_client) mock_project.get_service.return_value = Service( 'service', client=mock_client, @@ -180,7 +180,7 @@ class CLITestCase(unittest.TestCase): command = TopLevelCommand() mock_client = mock.create_autospec(docker.Client) - mock_project = mock.Mock() + mock_project = mock.Mock(client=mock_client) mock_project.get_service.return_value = Service( 'service', client=mock_client, diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 88d301470..a63e39c65 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -12,6 +12,7 @@ from compose.const import LABEL_SERVICE, LABEL_PROJECT, LABEL_ONE_OFF from compose.service import ( ConfigError, NeedsBuildError, + NoSuchImageError, build_port_bindings, build_volume_binding, get_container_data_volumes, @@ -233,7 +234,7 @@ class ServiceTest(unittest.TestCase): images.append({'Id': 'abc123'}) return [] - service.image = lambda: images[0] if images else None + service.image = lambda *args, **kwargs: mock_get_image(images) self.mock_client.pull = pull service.create_container(insecure_registry=True) @@ -246,7 +247,7 @@ class ServiceTest(unittest.TestCase): service.image = lambda: {'Id': 'abc123'} new_container = service.recreate_container(mock_container) - mock_container.stop.assert_called_once_with() + 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)) @@ -254,6 +255,15 @@ class ServiceTest(unittest.TestCase): new_container.start.assert_called_once_with() mock_container.remove.assert_called_once_with() + @mock.patch('compose.service.Container', autospec=True) + def test_recreate_container_with_timeout(self, _): + mock_container = mock.create_autospec(Container) + self.mock_client.inspect_image.return_value = {'Id': 'abc123'} + service = Service('foo', client=self.mock_client, image='someimage') + service.recreate_container(mock_container, timeout=1) + + mock_container.stop.assert_called_once_with(timeout=1) + def test_parse_repository_tag(self): self.assertEqual(parse_repository_tag("root"), ("root", "")) self.assertEqual(parse_repository_tag("root:tag"), ("root", "tag")) @@ -273,7 +283,7 @@ class ServiceTest(unittest.TestCase): images.append({'Id': 'abc123'}) return [] - service.image = lambda: images[0] if images else None + service.image = lambda *args, **kwargs: mock_get_image(images) self.mock_client.pull = pull service.create_container() @@ -283,7 +293,7 @@ class ServiceTest(unittest.TestCase): service = Service('foo', client=self.mock_client, build='.') images = [] - service.image = lambda *args, **kwargs: images[0] if images else None + service.image = lambda *args, **kwargs: mock_get_image(images) service.build = lambda: images.append({'Id': 'abc123'}) service.create_container(do_build=True) @@ -298,7 +308,7 @@ class ServiceTest(unittest.TestCase): def test_create_container_no_build_but_needs_build(self): service = Service('foo', client=self.mock_client, build='.') - service.image = lambda: None + service.image = lambda *args, **kwargs: mock_get_image([]) with self.assertRaises(NeedsBuildError): service.create_container(do_build=False) @@ -315,6 +325,13 @@ class ServiceTest(unittest.TestCase): self.assertFalse(self.mock_client.build.call_args[1]['pull']) +def mock_get_image(images): + if images: + return images[0] + else: + raise NoSuchImageError() + + class ServiceVolumesTest(unittest.TestCase): def setUp(self):