Merge "Disallow IP changes on undercloud update"

instack_undercloud/tests/test_undercloud.py

@@ -29,6 +29,7 @@ from oslotest import log
 from six.moves import configparser
 
 from instack_undercloud import undercloud
+from instack_undercloud import validator
 
 
 undercloud._configure_logging(undercloud.DEFAULT_LOG_LEVEL, None)
@@ -144,10 +145,12 @@ class TestUndercloud(BaseTestCase):
     @mock.patch('instack_undercloud.undercloud._check_memory')
     @mock.patch('instack_undercloud.undercloud._check_sysctl')
     @mock.patch('instack_undercloud.undercloud._validate_network')
-    def test_validate_configuration(self, mock_validate_network,
+    @mock.patch('instack_undercloud.undercloud._validate_no_ip_change')
+    def test_validate_configuration(self, mock_vnic, mock_validate_network,
                                     mock_check_memory, mock_check_hostname,
                                     mock_check_sysctl):
         undercloud._validate_configuration()
+        self.assertTrue(mock_vnic.called)
         self.assertTrue(mock_validate_network.called)
         self.assertTrue(mock_check_memory.called)
         self.assertTrue(mock_check_hostname.called)
@@ -262,6 +265,48 @@ class TestCheckSysctl(BaseTestCase):
         undercloud._check_sysctl()
 
 
+class TestNoIPChange(BaseTestCase):
+    @mock.patch('os.path.isfile', return_value=False)
+    def test_new_install(self, mock_isfile):
+        undercloud._validate_no_ip_change()
+
+    @mock.patch('instack_undercloud.undercloud.open')
+    @mock.patch('json.loads')
+    @mock.patch('os.path.isfile', return_value=True)
+    def test_update_matches(self, mock_isfile, mock_loads, mock_open):
+        mock_members = [{'name': 'eth0'},
+                        {'name': 'br-ctlplane',
+                         'addresses': [{'ip_netmask': '192.168.24.1/24'}]
+                         }
+                        ]
+        mock_config = {'network_config': mock_members}
+        mock_loads.return_value = mock_config
+        undercloud._validate_no_ip_change()
+
+    @mock.patch('instack_undercloud.undercloud.open')
+    @mock.patch('json.loads')
+    @mock.patch('os.path.isfile', return_value=True)
+    def test_update_mismatch(self, mock_isfile, mock_loads, mock_open):
+        mock_members = [{'name': 'eth0'},
+                        {'name': 'br-ctlplane',
+                         'addresses': [{'ip_netmask': '192.168.0.1/24'}]
+                         }
+                        ]
+        mock_config = {'network_config': mock_members}
+        mock_loads.return_value = mock_config
+        self.assertRaises(validator.FailedValidation,
+                          undercloud._validate_no_ip_change)
+
+    @mock.patch('instack_undercloud.undercloud.open')
+    @mock.patch('json.loads')
+    @mock.patch('os.path.isfile', return_value=True)
+    def test_update_no_network(self, mock_isfile, mock_loads, mock_open):
+        mock_members = [{'name': 'eth0'}]
+        mock_config = {'network_config': mock_members}
+        mock_loads.return_value = mock_config
+        undercloud._validate_no_ip_change()
+
+
 class TestGenerateEnvironment(BaseTestCase):
     def setUp(self):
         super(TestGenerateEnvironment, self).setUp()

instack_undercloud/undercloud.py

@@ -646,12 +646,42 @@ def _validate_network():
     validator.validate_config(params, error_handler)
 
 
+def _validate_no_ip_change():
+    """Disallow provisioning interface IP changes
+
+    Changing the provisioning network IP causes a number of issues, so we
+    need to disallow it early in the install before configurations start to
+    be changed.
+    """
+    os_net_config_file = '/etc/os-net-config/config.json'
+    # Nothing to do if we haven't already installed
+    if not os.path.isfile(
+            os.path.expanduser(os_net_config_file)):
+        return
+    with open(os_net_config_file) as f:
+        network_config = json.loads(f.read())
+    try:
+        ctlplane = [i for i in network_config.get('network_config', [])
+                    if i['name'] == 'br-ctlplane'][0]
+    except IndexError:
+        # Nothing to check if br-ctlplane wasn't configured
+        return
+    existing_ip = ctlplane['addresses'][0]['ip_netmask']
+    if existing_ip != CONF.local_ip:
+        message = ('Changing the local_ip is not allowed.  Existing IP: '
+                   '%s, Configured IP: %s') % (existing_ip,
+                                               CONF.network_cidr)
+        LOG.error(message)
+        raise validator.FailedValidation(message)
+
+
 def _validate_configuration():
     try:
         _check_hostname()
         _check_memory()
         _check_sysctl()
         _validate_network()
+        _validate_no_ip_change()
     except RuntimeError as e:
         LOG.error('An error occurred during configuration validation, '
                   'please check your host configuration and try again. '

releasenotes/notes/disallow-ip-changes-bde0e2528544c71b.yaml

@@ -0,0 +1,11 @@
+---
+upgrade:
+  - |
+    Network configuration changes are no longer allowed during undercloud
+    upgrades.  Changing the local_ip of a deployed undercloud causes problems
+    with some of the services, so a pre-deployment check was added to prevent
+    such changes.
+
+    Because the default CIDR was changed in this release, the check also
+    prevents accidental reconfiguration of the ctlplane network if the old
+    default is still in use, but not explicitly configured.