From 294453433debb14215267046894bace07c90d1a5 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sat, 2 Aug 2014 16:31:08 -0400 Subject: [PATCH] Remove extra calls to docker server. Fix broken integration tests Signed-off-by: Daniel Nephin --- fig/container.py | 8 +++----- fig/project.py | 12 ++++++------ fig/service.py | 20 +++++++++++--------- tests/integration/project_test.py | 15 +++++++++------ tests/integration/service_test.py | 8 ++++++-- tests/unit/container_test.py | 18 ++++++++++++++++++ 6 files changed, 53 insertions(+), 28 deletions(-) diff --git a/fig/container.py b/fig/container.py index e9bbd59d3..dc3366a6e 100644 --- a/fig/container.py +++ b/fig/container.py @@ -97,11 +97,8 @@ class Container(object): @property def environment(self): self.inspect_if_not_inspected() - out = {} - for var in self.dictionary.get('Config', {}).get('Env', []): - k, v = var.split('=', 1) - out[k] = v - return out + return dict(var.split("=", 1) + for var in self.dictionary.get('Config', {}).get('Env', [])) @property def is_running(self): @@ -132,6 +129,7 @@ class Container(object): def inspect(self): self.dictionary = self.client.inspect_container(self.id) + self.has_been_inspected = True return self.dictionary def links(self): diff --git a/fig/project.py b/fig/project.py index 614b04899..d0c556c48 100644 --- a/fig/project.py +++ b/fig/project.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals from __future__ import absolute_import import logging + from .service import Service from .container import Container from .packages.docker.errors import APIError @@ -179,12 +180,11 @@ class Project(object): for service in self.get_services(service_names): service.remove_stopped(**options) - def containers(self, service_names=None, *args, **kwargs): - l = [] - for service in self.get_services(service_names): - for container in service.containers(*args, **kwargs): - l.append(container) - return l + def containers(self, service_names=None, stopped=False, one_off=False): + return [Container.from_ps(self.client, container) + for container in self.client.containers(all=stopped) + for service in self.get_services(service_names) + if service.has_container(container, one_off=one_off)] def _inject_links(self, acc, service): linked_names = service.get_linked_names() diff --git a/fig/service.py b/fig/service.py index 65bcf5197..48f63e6a5 100644 --- a/fig/service.py +++ b/fig/service.py @@ -65,15 +65,17 @@ class Service(object): self.options = options def containers(self, stopped=False, one_off=False): - l = [] - for container in self.client.containers(all=stopped): - name = get_container_name(container) - if not name or not is_valid_name(name, one_off): - continue - project, name, number = parse_name(name) - if project == self.project and name == self.name: - l.append(Container.from_ps(self.client, container)) - return l + return [Container.from_ps(self.client, container) + for container in self.client.containers(all=stopped) + if self.has_container(container, one_off=one_off)] + + def has_container(self, container, one_off=False): + """Return True if `container` was created to fulfill this service.""" + name = get_container_name(container) + if not name or not is_valid_name(name, one_off): + return False + project, name, number = parse_name(name) + return project == self.project and name == self.name def start(self, **options): for c in self.containers(stopped=True): diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index 83a988a14..a9123af85 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -108,8 +108,9 @@ class ProjectTest(DockerClientTestCase): self.assertEqual(len(project.containers()), 2) db_container = [c for c in project.containers() if 'db' in c.name][0] - self.assertNotEqual(c.id, old_db_id) - self.assertEqual(c.inspect()['Volumes']['/var/db'], db_volume_path) + self.assertNotEqual(db_container.id, old_db_id) + self.assertEqual(db_container.inspect()['Volumes']['/var/db'], + db_volume_path) project.kill() project.remove_stopped() @@ -130,8 +131,9 @@ class ProjectTest(DockerClientTestCase): self.assertEqual(len(project.containers()), 2) db_container = [c for c in project.containers() if 'db' in c.name][0] - self.assertEqual(c.id, old_db_id) - self.assertEqual(c.inspect()['Volumes']['/var/db'], db_volume_path) + self.assertEqual(db_container.id, old_db_id) + self.assertEqual(db_container.inspect()['Volumes']['/var/db'], + db_volume_path) project.kill() project.remove_stopped() @@ -158,8 +160,9 @@ class ProjectTest(DockerClientTestCase): self.assertEqual(len(new_containers), 2) db_container = [c for c in new_containers if 'db' in c.name][0] - self.assertEqual(c.id, old_db_id) - self.assertEqual(c.inspect()['Volumes']['/var/db'], db_volume_path) + self.assertEqual(db_container.id, old_db_id) + self.assertEqual(db_container.inspect()['Volumes']['/var/db'], + db_volume_path) project.kill() project.remove_stopped() diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 75275d8a7..6dc551a8d 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -1,11 +1,13 @@ from __future__ import unicode_literals from __future__ import absolute_import +import os + from fig import Service from fig.service import CannotBeScaledError from fig.container import Container from fig.packages.docker.errors import APIError from .testcases import DockerClientTestCase -import os + class ServiceTest(DockerClientTestCase): def test_containers(self): @@ -143,7 +145,9 @@ class ServiceTest(DockerClientTestCase): self.assertEqual(len(self.client.containers(all=True)), num_containers_before) self.assertNotEqual(old_container.id, new_container.id) - self.assertRaises(APIError, lambda: self.client.inspect_container(intermediate_container.id)) + self.assertRaises(APIError, + self.client.inspect_container, + intermediate_container.id) def test_recreate_containers_when_containers_are_stopped(self): service = self.create_service( diff --git a/tests/unit/container_test.py b/tests/unit/container_test.py index 6e5b9884e..9f4861891 100644 --- a/tests/unit/container_test.py +++ b/tests/unit/container_test.py @@ -1,7 +1,12 @@ from __future__ import unicode_literals from .. import unittest + +import mock +from fig.packages import docker + from fig.container import Container + class ContainerTest(unittest.TestCase): def test_from_ps(self): container = Container.from_ps(None, { @@ -67,3 +72,16 @@ class ContainerTest(unittest.TestCase): "Names":["/figtest_db_1"] }, has_been_inspected=True) self.assertEqual(container.name_without_project, "db_1") + + def test_inspect_if_not_inspected(self): + mock_client = mock.create_autospec(docker.Client) + container = Container(mock_client, dict(Id="the_id")) + + container.inspect_if_not_inspected() + mock_client.inspect_container.assert_called_once_with("the_id") + self.assertEqual(container.dictionary, + mock_client.inspect_container.return_value) + self.assertTrue(container.has_been_inspected) + + container.inspect_if_not_inspected() + self.assertEqual(mock_client.inspect_container.call_count, 1)