From 66d12ccfff0a4110c8928a2c832921804c89c61b Mon Sep 17 00:00:00 2001 From: Ben Nemec Date: Fri, 10 Feb 2017 17:08:43 +0000 Subject: [PATCH] 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 | 47 ++++++++++++++++++- instack_undercloud/undercloud.py | 30 ++++++++++++ .../disallow-ip-changes-bde0e2528544c71b.yaml | 11 +++++ 3 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/disallow-ip-changes-bde0e2528544c71b.yaml diff --git a/instack_undercloud/tests/test_undercloud.py b/instack_undercloud/tests/test_undercloud.py index 2b2430f75..3db58ae5d 100644 --- a/instack_undercloud/tests/test_undercloud.py +++ b/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() diff --git a/instack_undercloud/undercloud.py b/instack_undercloud/undercloud.py index ce5d5d53b..3a0dcb1d9 100644 --- a/instack_undercloud/undercloud.py +++ b/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. ' diff --git a/releasenotes/notes/disallow-ip-changes-bde0e2528544c71b.yaml b/releasenotes/notes/disallow-ip-changes-bde0e2528544c71b.yaml new file mode 100644 index 000000000..e6b28341e --- /dev/null +++ b/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.