Move priority option to service network reference

Add ordering to networks.get_networks
Fix priority test

Signed-off-by: Joffrey F <joffrey@docker.com>

compose/config/config_schema_v2.0.json

@@ -191,7 +191,8 @@
                       "properties": {
                         "aliases": {"$ref": "#/definitions/list_of_strings"},
                         "ipv4_address": {"type": "string"},
-                        "ipv6_address": {"type": "string"}
+                        "ipv6_address": {"type": "string"},
+                        "priority": {"type": "number"}
                       },
                       "additionalProperties": false
                     },
@@ -299,8 +300,7 @@
           },
           "additionalProperties": false
         },
-        "internal": {"type": "boolean"},
+        "internal": {"type": "boolean"}
-        "priority": {"type": "number"}
       },
       "additionalProperties": false
     },

compose/config/config_schema_v2.1.json

@@ -217,7 +217,8 @@
                         "aliases": {"$ref": "#/definitions/list_of_strings"},
                         "ipv4_address": {"type": "string"},
                         "ipv6_address": {"type": "string"},
-                        "link_local_ips": {"$ref": "#/definitions/list_of_strings"}
+                        "link_local_ips": {"$ref": "#/definitions/list_of_strings"},
+                        "priority": {"type": "number"}
                       },
                       "additionalProperties": false
                     },
@@ -351,8 +352,7 @@
         "internal": {"type": "boolean"},
         "enable_ipv6": {"type": "boolean"},
         "labels": {"$ref": "#/definitions/list_or_dict"},
-        "name": {"type": "string"},
+        "name": {"type": "string"}
-        "priority": {"type": "number"}
       },
       "additionalProperties": false
     },

compose/config/config_schema_v2.2.json

@@ -223,7 +223,8 @@
                         "aliases": {"$ref": "#/definitions/list_of_strings"},
                         "ipv4_address": {"type": "string"},
                         "ipv6_address": {"type": "string"},
-                        "link_local_ips": {"$ref": "#/definitions/list_of_strings"}
+                        "link_local_ips": {"$ref": "#/definitions/list_of_strings"},
+                        "priority": {"type": "number"}
                       },
                       "additionalProperties": false
                     },
@@ -358,8 +359,7 @@
         "internal": {"type": "boolean"},
         "enable_ipv6": {"type": "boolean"},
         "labels": {"$ref": "#/definitions/list_or_dict"},
-        "name": {"type": "string"},
+        "name": {"type": "string"}
-        "priority": {"type": "number"}
       },
       "additionalProperties": false
     },

compose/config/config_schema_v2.3.json

@@ -226,7 +226,8 @@
                         "aliases": {"$ref": "#/definitions/list_of_strings"},
                         "ipv4_address": {"type": "string"},
                         "ipv6_address": {"type": "string"},
-                        "link_local_ips": {"$ref": "#/definitions/list_of_strings"}
+                        "link_local_ips": {"$ref": "#/definitions/list_of_strings"},
+                        "priority": {"type": "number"}
                       },
                       "additionalProperties": false
                     },
@@ -395,8 +396,7 @@
         "internal": {"type": "boolean"},
         "enable_ipv6": {"type": "boolean"},
         "labels": {"$ref": "#/definitions/list_or_dict"},
-        "name": {"type": "string"},
+        "name": {"type": "string"}
-        "priority": {"type": "number"}
       },
       "additionalProperties": false
     },

compose/config/config_schema_v3.0.json

@@ -310,8 +310,7 @@
           "additionalProperties": false
         },
         "internal": {"type": "boolean"},
-        "labels": {"$ref": "#/definitions/list_or_dict"},
+        "labels": {"$ref": "#/definitions/list_or_dict"}
-        "priority": {"type": "number"}
       },
       "additionalProperties": false
     },

compose/config/config_schema_v3.1.json

@@ -339,8 +339,7 @@
           "additionalProperties": false
         },
         "internal": {"type": "boolean"},
-        "labels": {"$ref": "#/definitions/list_or_dict"},
+        "labels": {"$ref": "#/definitions/list_or_dict"}
-        "priority": {"type": "number"}
       },
       "additionalProperties": false
     },

compose/config/config_schema_v3.2.json

@@ -387,8 +387,7 @@
         },
         "internal": {"type": "boolean"},
         "attachable": {"type": "boolean"},
-        "labels": {"$ref": "#/definitions/list_or_dict"},
+        "labels": {"$ref": "#/definitions/list_or_dict"}
-        "priority": {"type": "number"}
       },
       "additionalProperties": false
     },

compose/config/config_schema_v3.3.json

@@ -430,8 +430,7 @@
         },
         "internal": {"type": "boolean"},
         "attachable": {"type": "boolean"},
-        "labels": {"$ref": "#/definitions/list_or_dict"},
+        "labels": {"$ref": "#/definitions/list_or_dict"}
-        "priority": {"type": "number"}
       },
       "additionalProperties": false
     },

compose/config/config_schema_v3.4.json

@@ -438,8 +438,7 @@
         },
         "internal": {"type": "boolean"},
         "attachable": {"type": "boolean"},
-        "labels": {"$ref": "#/definitions/list_or_dict"},
+        "labels": {"$ref": "#/definitions/list_or_dict"}
-        "priority": {"type": "number"}
       },
       "additionalProperties": false
     },

compose/config/config_schema_v3.5.json

@@ -464,8 +464,7 @@
         },
         "internal": {"type": "boolean"},
         "attachable": {"type": "boolean"},
-        "labels": {"$ref": "#/definitions/list_or_dict"},
+        "labels": {"$ref": "#/definitions/list_or_dict"}
-        "priority": {"type": "number"}
       },
       "additionalProperties": false
     },

compose/network.py

@@ -2,6 +2,7 @@ from __future__ import absolute_import
 from __future__ import unicode_literals
 
 import logging
+from collections import OrderedDict
 
 from docker.errors import NotFound
 from docker.types import IPAMConfig
@@ -26,7 +27,7 @@ OPTS_EXCEPTIONS = [
 class Network(object):
     def __init__(self, client, project, name, driver=None, driver_opts=None,
                  ipam=None, external=False, internal=False, enable_ipv6=False,
-                 labels=None, custom_name=False, priority=0):
+                 labels=None, custom_name=False):
         self.client = client
         self.project = project
         self.name = name
@@ -38,7 +39,6 @@ class Network(object):
         self.enable_ipv6 = enable_ipv6
         self.labels = labels
         self.custom_name = custom_name
-        self.priority = priority
 
     def ensure(self):
         if self.external:
@@ -215,7 +215,6 @@ def build_networks(name, config_data, client):
             enable_ipv6=data.get('enable_ipv6'),
             labels=data.get('labels'),
             custom_name=data.get('name') is not None,
-            priority=data.get('priority'),
         )
         for network_name, data in network_config.items()
     }
@@ -282,11 +281,13 @@ def get_networks(service_dict, network_definitions):
     for name, netdef in get_network_defs_for_service(service_dict).items():
         network = network_definitions.get(name)
         if network:
-            netdef['priority'] = network.priority
             networks[network.full_name] = netdef
         else:
             raise ConfigurationError(
                 'Service "{}" uses an undefined network "{}"'
                 .format(service_dict['name'], name))
 
-    return networks
+    return OrderedDict(sorted(
+        networks.items(),
+        key=lambda t: t[1].get('priority') or 0, reverse=True
+    ))

compose/service.py

@@ -558,24 +558,25 @@ class Service(object):
             raise OperationFailedError("Cannot start service %s: %s" % (self.name, ex.explanation))
         return container
 
+    @property
     def prioritized_networks(self):
-        prioritized_networks = OrderedDict(
+        return OrderedDict(
-            sorted(self.networks.items(), key=lambda t: t[1].get('priority', 0) or 0, reverse=True))
+            sorted(
-        return prioritized_networks
+                self.networks.items(),
+                key=lambda t: t[1].get('priority') or 0, reverse=True
+            )
+        )
 
     def connect_container_to_networks(self, container):
         connected_networks = container.get('NetworkSettings.Networks')
 
-        for network, netdefs in self.prioritized_networks().items():
+        for network, netdefs in self.prioritized_networks.items():
             if network in connected_networks:
                 if short_id_alias_exists(container, network):
                     continue
+                self.client.disconnect_container_from_network(container.id, network)
 
-                self.client.disconnect_container_from_network(
+            log.debug('Connecting to {}'.format(network))
-                    container.id,
-                    network)
-
-            print('Connecting to {}'.format(network))
             self.client.connect_container_to_network(
                 container.id, network,
                 aliases=self._get_aliases(netdefs, container),

tests/acceptance/cli_test.py

@@ -1274,13 +1274,6 @@ class CLITestCase(DockerClientTestCase):
             bar_container.id
         )
 
-    def test_up_ordered_networks(self):
-        self.base_dir = 'tests/fixtures/networks'
-        result = self.dispatch(['-f', 'ordered-networks.yml', 'up', '-d'])
-
-        assert 'Connecting to networks_buzz\nConnecting to networks_foo' \
-               '\nConnecting to networks_bar' in result.stdout
-
     @v3_only()
     def test_up_with_healthcheck(self):
         def wait_on_health_status(container, status):

tests/fixtures/networks/ordered-networks.yml

@@ -1,15 +0,0 @@
-version: "2.3"
-
-services:
-  web:
-    image: busybox
-    command: top
-    networks: ["foo", "bar", "buzz"]
-
-networks:
-  foo:
-    priority: 2
-  bar:
-     priority: 1
-  buzz:
-    priority: 3

tests/integration/project_test.py

@@ -829,6 +829,71 @@ class ProjectTest(DockerClientTestCase):
         assert ipam_config.get('IPv4Address') == '172.16.100.100'
         assert ipam_config.get('IPv6Address') == 'fe80::1001:102'
 
+    @v2_3_only()
+    def test_up_with_network_priorities(self):
+        mac_address = '74:6f:75:68:6f:75'
+
+        def get_config_data(p1, p2, p3):
+            return build_config(
+                version=V2_3,
+                services=[{
+                    'name': 'web',
+                    'image': 'busybox:latest',
+                    'networks': {
+                        'n1': {
+                            'priority': p1,
+                        },
+                        'n2': {
+                            'priority': p2,
+                        },
+                        'n3': {
+                            'priority': p3,
+                        }
+                    },
+                    'command': 'top',
+                    'mac_address': mac_address
+                }],
+                networks={
+                    'n1': {},
+                    'n2': {},
+                    'n3': {}
+                }
+            )
+
+        config1 = get_config_data(1000, 1, 1)
+        config2 = get_config_data(2, 3, 1)
+        config3 = get_config_data(5, 40, 100)
+
+        project = Project.from_config(
+            client=self.client,
+            name='composetest',
+            config_data=config1
+        )
+        project.up(detached=True)
+        service_container = project.get_service('web').containers()[0]
+        net_config = service_container.inspect()['NetworkSettings']['Networks']['composetest_n1']
+        assert net_config['MacAddress'] == mac_address
+
+        project = Project.from_config(
+            client=self.client,
+            name='composetest',
+            config_data=config2
+        )
+        project.up(detached=True)
+        service_container = project.get_service('web').containers()[0]
+        net_config = service_container.inspect()['NetworkSettings']['Networks']['composetest_n2']
+        assert net_config['MacAddress'] == mac_address
+
+        project = Project.from_config(
+            client=self.client,
+            name='composetest',
+            config_data=config3
+        )
+        project.up(detached=True)
+        service_container = project.get_service('web').containers()[0]
+        net_config = service_container.inspect()['NetworkSettings']['Networks']['composetest_n3']
+        assert net_config['MacAddress'] == mac_address
+
     @v2_1_only()
     def test_up_with_enable_ipv6(self):
         self.require_api_version('1.23')