Merge pull request #3337 from dnephin/check_for_short_id_alias

Check for short id alias, and don't disconnect if it already exists

compose/service.py

@@ -453,20 +453,20 @@ def connect_container_to_networks(self, container):
         connected_networks = container.get('NetworkSettings.Networks')
 
         for network, netdefs in self.networks.items():
-            aliases = netdefs.get('aliases', [])
-            ipv4_address = netdefs.get('ipv4_address', None)
-            ipv6_address = netdefs.get('ipv6_address', None)
             if network in connected_networks:
+                if short_id_alias_exists(container, network):
+                    continue
+
                 self.client.disconnect_container_from_network(
-                    container.id, network)
+                    container.id,
+                    network)
 
             self.client.connect_container_to_network(
                 container.id, network,
-                aliases=list(self._get_aliases(container).union(aliases)),
-                ipv4_address=ipv4_address,
-                ipv6_address=ipv6_address,
-                links=self._get_links(False)
-            )
+                aliases=self._get_aliases(netdefs, container),
+                ipv4_address=netdefs.get('ipv4_address', None),
+                ipv6_address=netdefs.get('ipv6_address', None),
+                links=self._get_links(False))
 
     def remove_duplicate_containers(self, timeout=DEFAULT_TIMEOUT):
         for c in self.duplicate_containers():
@@ -533,11 +533,32 @@ def _next_container_number(self, one_off=False):
         numbers = [c.number for c in containers]
         return 1 if not numbers else max(numbers) + 1
 
-    def _get_aliases(self, container):
-        if container.labels.get(LABEL_ONE_OFF) == "True":
-            return set()
+    def _get_aliases(self, network, container=None):
+        if container and container.labels.get(LABEL_ONE_OFF) == "True":
+            return []
+
+        return list(
+            {self.name} |
+            ({container.short_id} if container else set()) |
+            set(network.get('aliases', ()))
+        )
+
+    def build_default_networking_config(self):
+        if not self.networks:
+            return {}
 
-        return {self.name, container.short_id}
+        network = self.networks[self.network_mode.id]
+        endpoint = {
+            'Aliases': self._get_aliases(network),
+            'IPAMConfig': {},
+        }
+
+        if network.get('ipv4_address'):
+            endpoint['IPAMConfig']['IPv4Address'] = network.get('ipv4_address')
+        if network.get('ipv6_address'):
+            endpoint['IPAMConfig']['IPv6Address'] = network.get('ipv6_address')
+
+        return {"EndpointsConfig": {self.network_mode.id: endpoint}}
 
     def _get_links(self, link_to_self):
         links = {}
@@ -633,6 +654,10 @@ def _get_container_create_options(
             override_options,
             one_off=one_off)
 
+        networking_config = self.build_default_networking_config()
+        if networking_config:
+            container_options['networking_config'] = networking_config
+
         container_options['environment'] = format_environment(
             container_options['environment'])
         return container_options
@@ -796,6 +821,12 @@ def pull(self, ignore_pull_failures=False):
                 log.error(six.text_type(e))
 
 
+def short_id_alias_exists(container, network):
+    aliases = container.get(
+        'NetworkSettings.Networks.{net}.Aliases'.format(net=network)) or ()
+    return container.short_id in aliases
+
+
 class NetworkMode(object):
     """A `standard` network mode (ex: host, bridge)"""
 

tests/integration/project_test.py

@@ -565,7 +565,11 @@ def test_project_up_networks(self):
                 'name': 'web',
                 'image': 'busybox:latest',
                 'command': 'top',
-                'networks': {'foo': None, 'bar': None, 'baz': None},
+                'networks': {
+                    'foo': None,
+                    'bar': None,
+                    'baz': {'aliases': ['extra']},
+                },
             }],
             volumes={},
             networks={
@@ -581,15 +585,23 @@ def test_project_up_networks(self):
             config_data=config_data,
         )
         project.up()
-        self.assertEqual(len(project.containers()), 1)
+
+        containers = project.containers()
+        assert len(containers) == 1
+        container, = containers
 
         for net_name in ['foo', 'bar', 'baz']:
             full_net_name = 'composetest_{}'.format(net_name)
             network_data = self.client.inspect_network(full_net_name)
-            self.assertEqual(network_data['Name'], full_net_name)
+            assert network_data['Name'] == full_net_name
+
+        aliases_key = 'NetworkSettings.Networks.{net}.Aliases'
+        assert 'web' in container.get(aliases_key.format(net='composetest_foo'))
+        assert 'web' in container.get(aliases_key.format(net='composetest_baz'))
+        assert 'extra' in container.get(aliases_key.format(net='composetest_baz'))
 
         foo_data = self.client.inspect_network('composetest_foo')
-        self.assertEqual(foo_data['Driver'], 'bridge')
+        assert foo_data['Driver'] == 'bridge'
 
     @v2_only()
     def test_up_with_ipam_config(self):

tests/unit/service_test.py

@@ -663,6 +663,35 @@ def test_only_log_warning_when_host_ports_clash(self, mock_log):
             'for this service are created on a single host, the port will clash.'.format(name))
 
 
+class TestServiceNetwork(object):
+
+    def test_connect_container_to_networks_short_aliase_exists(self):
+        mock_client = mock.create_autospec(docker.Client)
+        service = Service(
+            'db',
+            mock_client,
+            'myproject',
+            image='foo',
+            networks={'project_default': {}})
+        container = Container(
+            None,
+            {
+                'Id': 'abcdef',
+                'NetworkSettings': {
+                    'Networks': {
+                        'project_default': {
+                            'Aliases': ['analias', 'abcdef'],
+                        },
+                    },
+                },
+            },
+            True)
+        service.connect_container_to_networks(container)
+
+        assert not mock_client.disconnect_container_from_network.call_count
+        assert not mock_client.connect_container_to_network.call_count
+
+
 def sort_by_name(dictionary_list):
     return sorted(dictionary_list, key=lambda k: k['name'])