Merge pull request #2341 from dnephin/dont_create_default_network_sometimes

Only create the default network if at least one service needs it

compose/project.py

@@ -300,7 +300,7 @@ class Project(object):
 
         plans = self._get_convergence_plans(services, strategy)
 
-        if self.use_networking:
+        if self.use_networking and self.uses_default_network():
             self.ensure_network_exists()
 
         return [
@@ -383,7 +383,10 @@ class Project(object):
     def remove_network(self):
         network = self.get_network()
         if network:
-            self.client.remove_network(network['id'])
+            self.client.remove_network(network['Id'])
+
+    def uses_default_network(self):
+        return any(service.net.mode == self.name for service in self.services)
 
     def _inject_deps(self, acc, service):
         dep_names = service.get_dependency_names()

tests/integration/project_test.py

@@ -7,6 +7,7 @@ from compose.const import LABEL_PROJECT
 from compose.container import Container
 from compose.project import Project
 from compose.service import ConvergenceStrategy
+from compose.service import Net
 from compose.service import VolumeFromSpec
 
 
@@ -111,6 +112,7 @@ class ProjectTest(DockerClientTestCase):
         network_name = 'network_does_exist'
         project = Project(network_name, [], client)
         client.create_network(network_name)
+        self.addCleanup(client.remove_network, network_name)
         assert project.get_network()['Name'] == network_name
 
     def test_net_from_service(self):
@@ -398,6 +400,20 @@ class ProjectTest(DockerClientTestCase):
         self.assertEqual(len(project.get_service('data').containers(stopped=True)), 1)
         self.assertEqual(len(project.get_service('console').containers()), 0)
 
+    def test_project_up_with_custom_network(self):
+        self.require_api_version('1.21')
+        client = docker_client(version='1.21')
+        network_name = 'composetest-custom'
+
+        client.create_network(network_name)
+        self.addCleanup(client.remove_network, network_name)
+
+        web = self.create_service('web', net=Net(network_name))
+        project = Project('composetest', [web], client, use_networking=True)
+        project.up()
+
+        assert project.get_network() is None
+
     def test_unscale_after_restart(self):
         web = self.create_service('web')
         project = Project('composetest', [web], self.client)

tests/integration/testcases.py

@@ -42,34 +42,23 @@ class DockerClientTestCase(unittest.TestCase):
         if 'command' not in kwargs:
             kwargs['command'] = ["top"]
 
-        links = kwargs.get('links', None)
-        volumes_from = kwargs.get('volumes_from', None)
-        net = kwargs.get('net', None)
+        workaround_options = {}
+        for option in ['links', 'volumes_from', 'net']:
+            if option in kwargs:
+                workaround_options[option] = kwargs.pop(option, None)
 
-        workaround_options = ['links', 'volumes_from', 'net']
-        for key in workaround_options:
-            try:
-                del kwargs[key]
-            except KeyError:
-                pass
-
-        options = ServiceLoader(working_dir='.', filename=None, service_name=name, service_dict=kwargs).make_service_dict()
+        options = ServiceLoader(
+            working_dir='.',
+            filename=None,
+            service_name=name,
+            service_dict=kwargs
+        ).make_service_dict()
+        options.update(workaround_options)
 
         labels = options.setdefault('labels', {})
         labels['com.docker.compose.test-name'] = self.id()
 
-        if links:
-            options['links'] = links
-        if volumes_from:
-            options['volumes_from'] = volumes_from
-        if net:
-            options['net'] = net
-
-        return Service(
-            project='composetest',
-            client=self.client,
-            **options
-        )
+        return Service(project='composetest', client=self.client, **options)
 
     def check_build(self, *args, **kwargs):
         kwargs.setdefault('rm', True)

tests/unit/project_test.py

@@ -7,6 +7,8 @@ from .. import unittest
 from compose.const import LABEL_SERVICE
 from compose.container import Container
 from compose.project import Project
+from compose.service import ContainerNet
+from compose.service import Net
 from compose.service import Service
 
 
@@ -263,6 +265,32 @@ class ProjectTest(unittest.TestCase):
         service = project.get_service('test')
         self.assertEqual(service.net.mode, 'container:' + container_name)
 
+    def test_uses_default_network_true(self):
+        web = Service('web', project='test', image="alpine", net=Net('test'))
+        db = Service('web', project='test', image="alpine", net=Net('other'))
+        project = Project('test', [web, db], None)
+        assert project.uses_default_network()
+
+    def test_uses_default_network_custom_name(self):
+        web = Service('web', project='test', image="alpine", net=Net('other'))
+        project = Project('test', [web], None)
+        assert not project.uses_default_network()
+
+    def test_uses_default_network_host(self):
+        web = Service('web', project='test', image="alpine", net=Net('host'))
+        project = Project('test', [web], None)
+        assert not project.uses_default_network()
+
+    def test_uses_default_network_container(self):
+        container = mock.Mock(id='test')
+        web = Service(
+            'web',
+            project='test',
+            image="alpine",
+            net=ContainerNet(container))
+        project = Project('test', [web], None)
+        assert not project.uses_default_network()
+
     def test_container_without_name(self):
         self.mock_client.containers.return_value = [
             {'Image': 'busybox:latest', 'Id': '1', 'Name': '1'},