From 9326dbd0978968441a08f54bad2028b04c0cb77c Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Tue, 12 May 2015 11:11:36 +0100 Subject: [PATCH] Basic implementation of smart recreate Signed-off-by: Aanand Prasad --- compose/cli/main.py | 4 ++ compose/const.py | 1 + compose/container.py | 5 +- compose/project.py | 18 +++--- compose/service.py | 94 ++++++++++++++++++++++++--- compose/state.py | 0 compose/utils.py | 9 +++ tests/integration/project_test.py | 3 +- tests/integration/service_test.py | 19 +++--- tests/integration/state_test.py | 102 ++++++++++++++++++++++++++++++ 10 files changed, 227 insertions(+), 28 deletions(-) create mode 100644 compose/const.py create mode 100644 compose/state.py create mode 100644 compose/utils.py create mode 100644 tests/integration/state_test.py diff --git a/compose/cli/main.py b/compose/cli/main.py index 592ef37fc6..027f2959ce 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -442,6 +442,8 @@ def up(self, project, options): print new container names. --no-color Produce monochrome output. --no-deps Don't start linked services. + --x-smart-recreate Only recreate containers whose configuration 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 @@ -455,12 +457,14 @@ def up(self, project, options): start_deps = not options['--no-deps'] recreate = not options['--no-recreate'] + smart_recreate = options['--x-smart-recreate'] service_names = options['SERVICE'] project.up( service_names=service_names, start_deps=start_deps, recreate=recreate, + smart_recreate=smart_recreate, insecure_registry=insecure_registry, do_build=not options['--no-build'], ) diff --git a/compose/const.py b/compose/const.py new file mode 100644 index 0000000000..15166b26c0 --- /dev/null +++ b/compose/const.py @@ -0,0 +1 @@ +LABEL_CONFIG_HASH = 'com.docker.compose.config-hash' diff --git a/compose/container.py b/compose/container.py index fc3370d9e4..6094d16473 100644 --- a/compose/container.py +++ b/compose/container.py @@ -171,13 +171,16 @@ def attach_socket(self, **kwargs): return self.client.attach_socket(self.id, **kwargs) def __repr__(self): - return '' % self.name + return '' % (self.name, self.id[:6]) def __eq__(self, other): if type(self) != type(other): return False return self.id == other.id + def __hash__(self): + return self.id.__hash__() + def get_container_name(container): if not container.get('Name') and not container.get('Names'): diff --git a/compose/project.py b/compose/project.py index 4e28d27a05..c66f3169aa 100644 --- a/compose/project.py +++ b/compose/project.py @@ -196,19 +196,19 @@ def up(self, service_names=None, start_deps=True, recreate=True, + smart_recreate=False, insecure_registry=False, do_build=True): + running_containers = [] - for service in self.get_services(service_names, include_deps=start_deps): - if recreate: - create_func = service.recreate_containers - else: - create_func = service.start_or_create_containers - for container in create_func( - insecure_registry=insecure_registry, - do_build=do_build): - running_containers.append(container) + for service in self.get_services(service_names, include_deps=start_deps): + running_containers += service.converge( + allow_recreate=recreate, + smart_recreate=smart_recreate, + insecure_registry=insecure_registry, + do_build=do_build, + ) return running_containers diff --git a/compose/service.py b/compose/service.py index 4ae11a90e6..5145b71fdf 100644 --- a/compose/service.py +++ b/compose/service.py @@ -11,8 +11,10 @@ from docker.utils import create_host_config, LogConfig from .config import DOCKER_CONFIG_KEYS +from .const import LABEL_CONFIG_HASH from .container import Container, get_container_name from .progress_stream import stream_output, StreamOutputError +from .utils import json_hash log = logging.getLogger(__name__) @@ -201,26 +203,89 @@ def create_container(self, return Container.create(self.client, **container_options) raise - def recreate_containers(self, insecure_registry=False, do_build=True): + 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. """ - containers = self.containers(stopped=True) - if not containers: + (action, containers) = self.convergence_plan( + allow_recreate=allow_recreate, + smart_recreate=smart_recreate, + ) + + if action == 'create': log.info("Creating %s..." % self._next_container_name(containers)) container = self.create_container( insecure_registry=insecure_registry, - do_build=do_build) + do_build=do_build, + ) self.start_container(container) + return [container] - return [ - self.recreate_container(c, insecure_registry=insecure_registry) - for c in containers - ] + elif action == 'recreate': + return [ + self.recreate_container( + c, + insecure_registry=insecure_registry, + ) + for c in containers + ] + + elif action == 'start': + for c in containers: + self.start_container_if_stopped(c) + + return containers + + elif action == 'noop': + for c in containers: + log.info("%s is up-to-date" % c.name) + + return containers + + else: + raise Exception("Invalid action: {}".format(action)) + + def convergence_plan(self, + allow_recreate=True, + smart_recreate=False): - def recreate_container(self, container, insecure_registry=False): + containers = self.containers(stopped=True) + + if not containers: + return ('create', []) + + if smart_recreate and not self._containers_have_diverged(containers): + return ('noop', containers) + + if not allow_recreate: + return ('start', containers) + + return ('recreate', containers) + + def _containers_have_diverged(self, containers): + config_hash = self.config_hash() + has_diverged = False + + for c in containers: + container_config_hash = c.labels.get(LABEL_CONFIG_HASH, None) + if container_config_hash != config_hash: + log.debug( + '%s has diverged: %s != %s', + c.name, container_config_hash, config_hash, + ) + has_diverged = True + + return has_diverged + + def recreate_container(self, + container, + insecure_registry=False): """Recreate a container. The original container is renamed to a temporary name so that data @@ -278,6 +343,9 @@ def start_or_create_containers( else: return [self.start_container_if_stopped(c) for c in containers] + def config_hash(self): + return json_hash(self.options) + def get_linked_names(self): return [s.name for (s, _) in self.links] @@ -369,6 +437,14 @@ def _get_container_create_options(self, for k in DOCKER_CONFIG_KEYS if k in self.options) container_options.update(override_options) + # TODO: add tests for this conditional + if not one_off and not override_options: + config_hash = self.config_hash() + if 'labels' not in container_options: + container_options['labels'] = {} + container_options['labels'][LABEL_CONFIG_HASH] = config_hash + log.debug("Added config hash: %s" % config_hash) + if 'detach' not in container_options: container_options['detach'] = True diff --git a/compose/state.py b/compose/state.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/compose/utils.py b/compose/utils.py new file mode 100644 index 0000000000..d441a2daee --- /dev/null +++ b/compose/utils.py @@ -0,0 +1,9 @@ +import json +import hashlib + + +def json_hash(obj): + dump = json.dumps(obj, sort_keys=True) + h = hashlib.sha256() + h.update(dump) + return h.hexdigest() diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index 00d156b370..57aae8ecb1 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -204,7 +204,7 @@ def test_project_up_with_no_recreate_stopped(self): self.assertEqual(len(project.containers()), 0) project.up(['db']) - project.stop() + project.kill() old_containers = project.containers(stopped=True) @@ -216,6 +216,7 @@ def test_project_up_with_no_recreate_stopped(self): new_containers = project.containers(stopped=True) self.assertEqual(len(new_containers), 2) + self.assertEqual([c.is_running for c in new_containers], [True, True]) db_container = [c for c in new_containers if 'db' in c.name][0] self.assertEqual(db_container.id, old_db_id) diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index dbb4a609c2..1bcdaca1bc 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -15,6 +15,7 @@ ConfigError, ) from compose.container import Container +from compose.const import LABEL_CONFIG_HASH from docker.errors import APIError from .testcases import DockerClientTestCase @@ -230,7 +231,7 @@ def test_create_container_with_volumes_from(self): self.assertIn(volume_container_2.id, host_container.get('HostConfig.VolumesFrom')) - def test_recreate_containers(self): + def test_converge(self): service = self.create_service( 'db', environment={'FOO': '1'}, @@ -249,7 +250,7 @@ def test_recreate_containers(self): num_containers_before = len(self.client.containers(all=True)) service.options['environment']['FOO'] = '2' - new_container, = service.recreate_containers() + new_container, = service.converge() self.assertEqual(new_container.dictionary['Config']['Entrypoint'], ['sleep']) self.assertEqual(new_container.dictionary['Config']['Cmd'], ['300']) @@ -264,7 +265,7 @@ def test_recreate_containers(self): self.client.inspect_container, old_container.id) - def test_recreate_containers_when_containers_are_stopped(self): + def test_converge_when_containers_are_stopped(self): service = self.create_service( 'db', environment={'FOO': '1'}, @@ -274,10 +275,10 @@ def test_recreate_containers_when_containers_are_stopped(self): ) service.create_container() self.assertEqual(len(service.containers(stopped=True)), 1) - service.recreate_containers() + service.converge() self.assertEqual(len(service.containers(stopped=True)), 1) - def test_recreate_containers_with_image_declared_volume(self): + def test_converge_with_image_declared_volume(self): service = Service( project='composetest', name='db', @@ -289,7 +290,7 @@ def test_recreate_containers_with_image_declared_volume(self): self.assertEqual(old_container.get('Volumes').keys(), ['/data']) volume_path = old_container.get('Volumes')['/data'] - service.recreate_containers() + service.converge() new_container = service.containers()[0] service.start_container(new_container) self.assertEqual(new_container.get('Volumes').keys(), ['/data']) @@ -635,14 +636,16 @@ def test_labels(self): service = self.create_service('web', labels=labels_dict) labels = create_and_start_container(service).labels.items() for pair in labels_dict.items(): - self.assertIn(pair, labels) + if pair[0] != LABEL_CONFIG_HASH: + self.assertIn(pair, labels) labels_list = ["%s=%s" % pair for pair in labels_dict.items()] service = self.create_service('web', labels=labels_list) labels = create_and_start_container(service).labels.items() for pair in labels_dict.items(): - self.assertIn(pair, labels) + if pair[0] != LABEL_CONFIG_HASH: + self.assertIn(pair, labels) def test_empty_labels(self): labels_list = ['foo', 'bar'] diff --git a/tests/integration/state_test.py b/tests/integration/state_test.py new file mode 100644 index 0000000000..cec4d13325 --- /dev/null +++ b/tests/integration/state_test.py @@ -0,0 +1,102 @@ +from __future__ import unicode_literals + +from compose import config +from compose.project import Project + +from .testcases import DockerClientTestCase + + +class ProjectStateTest(DockerClientTestCase): + def test_no_change(self): + cfg = { + 'db': {'image': 'busybox:latest'}, + 'web': {'image': 'busybox:latest'}, + } + + old_containers = self.run_up(cfg) + self.assertEqual(len(old_containers), 2) + + new_containers = self.run_up(cfg) + self.assertEqual(len(new_containers), 2) + + self.assertEqual(old_containers, new_containers) + + def test_partial_change(self): + cfg = { + 'db': {'image': 'busybox:latest'}, + 'web': {'image': 'busybox:latest'}, + } + + old_containers = self.run_up(cfg) + self.assertEqual(len(old_containers), 2) + + cfg['web']['command'] = '/bin/true' + + new_containers = self.run_up(cfg) + self.assertEqual(len(new_containers), 2) + + unchanged = list(old_containers & new_containers) + self.assertEqual(len(unchanged), 1) + self.assertEqual(unchanged[0].name_without_project, 'db_1') + + old = list(old_containers - new_containers) + self.assertEqual(len(old), 1) + self.assertEqual(old[0].name_without_project, 'web_1') + + new = list(new_containers - old_containers) + self.assertEqual(len(new), 1) + self.assertEqual(new[0].name_without_project, 'web_1') + self.assertEqual(new[0].get('Config.Cmd'), ['/bin/true']) + + def test_all_change(self): + cfg = { + 'db': {'image': 'busybox:latest'}, + 'web': {'image': 'busybox:latest'}, + } + + old_containers = self.run_up(cfg) + self.assertEqual(len(old_containers), 2) + + cfg['web']['command'] = '/bin/true' + cfg['db']['command'] = '/bin/true' + + new_containers = self.run_up(cfg) + self.assertEqual(len(new_containers), 2) + + unchanged = old_containers & new_containers + self.assertEqual(len(unchanged), 0) + + new = new_containers - old_containers + self.assertEqual(len(new), 2) + + def run_up(self, cfg): + project = self.make_project(cfg) + project.up(smart_recreate=True) + return set(project.containers(stopped=True)) + + def make_project(self, cfg): + return Project.from_dicts( + name='composetest', + client=self.client, + service_dicts=config.from_dictionary(cfg), + ) + + +class ServiceStateTest(DockerClientTestCase): + def test_trigger_create(self): + web = self.create_service('web') + self.assertEqual(('create', []), web.convergence_plan(smart_recreate=True)) + + def test_trigger_noop(self): + web = self.create_service('web') + container = web.create_container() + + web = self.create_service('web') + self.assertEqual(('noop', [container]), web.convergence_plan(smart_recreate=True)) + + def test_trigger_recreate(self): + web = self.create_service('web', command=["/bin/sleep", "300"]) + container = web.create_container() + + web = self.create_service('web', command=["/bin/sleep", "400"]) + self.assertEqual(('recreate', [container]), web.convergence_plan(smart_recreate=True))