Merge pull request #4605 from shin-/4600-check-all-network-updates

Ensure network config matches remote for all properties

compose/network.py

@@ -126,22 +126,64 @@ def create_ipam_config_from_dict(ipam_dict):
     )
 
 
+class NetworkConfigChangedError(ConfigurationError):
+    def __init__(self, net_name, property_name):
+        super(NetworkConfigChangedError, self).__init__(
+            'Network "{}" needs to be recreated - {} has changed'.format(
+                net_name, property_name
+            )
+        )
+
+
+def check_remote_ipam_config(remote, local):
+    remote_ipam = remote.get('IPAM')
+    ipam_dict = create_ipam_config_from_dict(local.ipam)
+    if local.ipam.get('driver') and local.ipam.get('driver') != remote_ipam.get('Driver'):
+        raise NetworkConfigChangedError(local.full_name, 'IPAM driver')
+    if len(ipam_dict['Config']) != 0:
+        if len(ipam_dict['Config']) != len(remote_ipam['Config']):
+            raise NetworkConfigChangedError(local.full_name, 'IPAM configs')
+        remote_configs = sorted(remote_ipam['Config'], key='Subnet')
+        local_configs = sorted(ipam_dict['Config'], key='Subnet')
+        while local_configs:
+            lc = local_configs.pop()
+            rc = remote_configs.pop()
+            if lc.get('Subnet') != rc.get('Subnet'):
+                raise NetworkConfigChangedError(local.full_name, 'IPAM config subnet')
+            if lc.get('Gateway') is not None and lc.get('Gateway') != rc.get('Gateway'):
+                raise NetworkConfigChangedError(local.full_name, 'IPAM config gateway')
+            if lc.get('IPRange') != rc.get('IPRange'):
+                raise NetworkConfigChangedError(local.full_name, 'IPAM config ip_range')
+            if sorted(lc.get('AuxiliaryAddresses')) != sorted(rc.get('AuxiliaryAddresses')):
+                raise NetworkConfigChangedError(local.full_name, 'IPAM config aux_addresses')
+
+
 def check_remote_network_config(remote, local):
     if local.driver and remote.get('Driver') != local.driver:
-        raise ConfigurationError(
-            'Network "{}" needs to be recreated - driver has changed'
-            .format(local.full_name)
-        )
+        raise NetworkConfigChangedError(local.full_name, 'driver')
     local_opts = local.driver_opts or {}
     remote_opts = remote.get('Options') or {}
     for k in set.union(set(remote_opts.keys()), set(local_opts.keys())):
         if k in OPTS_EXCEPTIONS:
             continue
         if remote_opts.get(k) != local_opts.get(k):
-            raise ConfigurationError(
-                'Network "{}" needs to be recreated - options have changed'
-                .format(local.full_name)
-            )
+            raise NetworkConfigChangedError(local.full_name, 'option "{}"'.format(k))
+
+    if local.ipam is not None:
+        check_remote_ipam_config(remote, local)
+
+    if local.internal is not None and local.internal != remote.get('Internal', False):
+        raise NetworkConfigChangedError(local.full_name, 'internal')
+    if local.enable_ipv6 is not None and local.enable_ipv6 != remote.get('EnableIPv6', False):
+        raise NetworkConfigChangedError(local.full_name, 'enable_ipv6')
+
+    local_labels = local.labels or {}
+    remote_labels = remote.get('Labels', {})
+    for k in set.union(set(remote_labels.keys()), set(local_labels.keys())):
+        if k.startswith('com.docker.compose.'):  # We are only interested in user-specified labels
+            continue
+        if remote_labels.get(k) != local_labels.get(k):
+            raise NetworkConfigChangedError(local.full_name, 'label "{}"'.format(k))
 
 
 def build_networks(name, config_data, client):

tests/unit/network_test.py

@@ -4,20 +4,62 @@ from __future__ import unicode_literals
 import pytest
 
 from .. import unittest
-from compose.config import ConfigurationError
 from compose.network import check_remote_network_config
 from compose.network import Network
+from compose.network import NetworkConfigChangedError
 
 
 class NetworkTest(unittest.TestCase):
     def test_check_remote_network_config_success(self):
         options = {'com.docker.network.driver.foo': 'bar'}
+        ipam_config = {
+            'driver': 'default',
+            'config': [
+                {'subnet': '172.0.0.1/16', },
+                {
+                    'subnet': '156.0.0.1/25',
+                    'gateway': '156.0.0.1',
+                    'aux_addresses': ['11.0.0.1', '24.25.26.27'],
+                    'ip_range': '156.0.0.1-254'
+                }
+            ]
+        }
+        labels = {
+            'com.project.tests.istest': 'true',
+            'com.project.sound.track': 'way out of here',
+        }
+        remote_labels = labels.copy()
+        remote_labels.update({
+            'com.docker.compose.project': 'compose_test',
+            'com.docker.compose.network': 'net1',
+        })
         net = Network(
             None, 'compose_test', 'net1', 'bridge',
-            options
+            options, enable_ipv6=True, ipam=ipam_config,
+            labels=labels
         )
         check_remote_network_config(
-            {'Driver': 'bridge', 'Options': options}, net
+            {
+                'Driver': 'bridge',
+                'Options': options,
+                'EnableIPv6': True,
+                'Internal': False,
+                'Attachable': True,
+                'IPAM': {
+                    'Driver': 'default',
+                    'Config': [{
+                        'Subnet': '156.0.0.1/25',
+                        'Gateway': '156.0.0.1',
+                        'AuxiliaryAddresses': ['24.25.26.27', '11.0.0.1'],
+                        'IPRange': '156.0.0.1-254'
+                    }, {
+                        'Subnet': '172.0.0.1/16',
+                        'Gateway': '172.0.0.1'
+                    }],
+                },
+                'Labels': remote_labels
+            },
+            net
         )
 
     def test_check_remote_network_config_whitelist(self):
@@ -36,20 +78,42 @@ class NetworkTest(unittest.TestCase):
 
     def test_check_remote_network_config_driver_mismatch(self):
         net = Network(None, 'compose_test', 'net1', 'overlay')
-        with pytest.raises(ConfigurationError):
+        with pytest.raises(NetworkConfigChangedError) as e:
             check_remote_network_config(
                 {'Driver': 'bridge', 'Options': {}}, net
             )
 
+        assert 'driver has changed' in str(e.value)
+
     def test_check_remote_network_config_options_mismatch(self):
         net = Network(None, 'compose_test', 'net1', 'overlay')
-        with pytest.raises(ConfigurationError):
+        with pytest.raises(NetworkConfigChangedError) as e:
             check_remote_network_config({'Driver': 'overlay', 'Options': {
                 'com.docker.network.driver.foo': 'baz'
             }}, net)
 
+        assert 'option "com.docker.network.driver.foo" has changed' in str(e.value)
+
     def test_check_remote_network_config_null_remote(self):
         net = Network(None, 'compose_test', 'net1', 'overlay')
         check_remote_network_config(
             {'Driver': 'overlay', 'Options': None}, net
         )
+
+    def test_check_remote_network_labels_mismatch(self):
+        net = Network(None, 'compose_test', 'net1', 'overlay', labels={
+            'com.project.touhou.character': 'sakuya.izayoi'
+        })
+        remote = {
+            'Driver': 'overlay',
+            'Options': None,
+            'Labels': {
+                'com.docker.compose.network': 'net1',
+                'com.docker.compose.project': 'compose_test',
+                'com.project.touhou.character': 'marisa.kirisame',
+            }
+        }
+        with pytest.raises(NetworkConfigChangedError) as e:
+            check_remote_network_config(remote, net)
+
+        assert 'label "com.project.touhou.character" has changed' in str(e.value)