Disallow IP changes on undercloud update

Changing the ctlplane IP after an undercloud has been installed
causes various difficult to solve problems, and doesn't work
properly in any case.  Let's explicitly disallow it during the
config validation before we start reconfiguring services and break
something.

Note that this also prevents accidental changes due to the new
default CIDR in this release.

Change-Id: I7f3a436b0a4e44fdc1241ebd52003ec9f659e8ea
Closes-Bug: 1645267

instack_undercloud/tests/test_undercloud.py

@@ -30,6 +30,7 @@ from oslotest import mockpatch
 from six.moves import configparser
 
 from instack_undercloud import undercloud
+from instack_undercloud import validator
 
 
 undercloud._configure_logging(undercloud.DEFAULT_LOG_LEVEL, None)
@@ -141,10 +142,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)
@@ -259,6 +262,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

@@ -643,12 +643,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.