diff --git a/releasenotes/notes/block-uc-upgrade-for-network-plugin-change-ba8459b171b8e37e.yaml b/releasenotes/notes/block-uc-upgrade-for-network-plugin-change-ba8459b171b8e37e.yaml new file mode 100644 index 000000000..19c9d5e92 --- /dev/null +++ b/releasenotes/notes/block-uc-upgrade-for-network-plugin-change-ba8459b171b8e37e.yaml @@ -0,0 +1,7 @@ +--- +upgrade: + - | + Undercloud upgrade is not allowed if a change in network plugin + i.e ovs to ovn or ovn to ovs is detected. Such a change will break + the undercloud as just switching is not enough it also requires + network resources to be migrated. diff --git a/tripleoclient/tests/test_utils.py b/tripleoclient/tests/test_utils.py index b8159755e..1283145d9 100644 --- a/tripleoclient/tests/test_utils.py +++ b/tripleoclient/tests/test_utils.py @@ -874,6 +874,82 @@ class TestWaitForStackUtil(TestCase): utils.check_swift_and_rgw(mock_stack.environment(), env, 'UpgradePrepare') + @mock.patch('os.path.isfile', return_value=False) + def test_check_network_plugin_no_neutron(self, mock_file): + fake_env = { + 'parameter_defaults': { + 'NeutronMechanismDrivers': ['ovn']}, + } + utils.check_network_plugin('/tmp', + fake_env) + mock_file.assert_not_called() + + @mock.patch('os.path.isfile', return_value=False) + def test_check_network_plugin_inventory_missing(self, mock_file): + fake_env = { + 'parameter_defaults': { + 'NeutronMechanismDrivers': ['ovn']}, + 'resource_registry': { + 'OS::TripleO::Services::NeutronApi': 'foo'} + } + with self.assertRaises(exceptions.InvalidConfiguration): + utils.check_network_plugin('/tmp', + fake_env) + + @mock.patch('os.path.isfile', return_value=True) + def test_check_network_plugin_inventory_ovs_match(self, mock_file): + fake_env = { + 'parameter_defaults': { + 'NeutronMechanismDrivers': ['openvswitch']}, + 'resource_registry': { + 'OS::TripleO::Services::NeutronApi': 'foo'} + } + mock_open_ctx = mock.mock_open(read_data='neutron_ovs_agent') + with mock.patch('builtins.open', mock_open_ctx): + utils.check_network_plugin('/tmp', + fake_env) + + @mock.patch('os.path.isfile', return_value=True) + def test_check_network_plugin_inventory_ovs_mismatch(self, mock_file): + fake_env = { + 'parameter_defaults': { + 'NeutronMechanismDrivers': ['ovn']}, + 'resource_registry': { + 'OS::TripleO::Services::NeutronApi': 'foo'} + } + with self.assertRaises(exceptions.InvalidConfiguration): + mock_open_ctx = mock.mock_open(read_data='neutron_ovs_agent') + with mock.patch('builtins.open', mock_open_ctx): + utils.check_network_plugin('/tmp', + fake_env) + + @mock.patch('os.path.isfile', return_value=True) + def test_check_network_plugin_inventory_ovn_match(self, mock_file): + fake_env = { + 'parameter_defaults': { + 'NeutronMechanismDrivers': ['ovn']}, + 'resource_registry': { + 'OS::TripleO::Services::NeutronApi': 'foo'} + } + mock_open_ctx = mock.mock_open(read_data='ovn_controller') + with mock.patch('builtins.open', mock_open_ctx): + utils.check_network_plugin('/tmp', + fake_env) + + @mock.patch('os.path.isfile', return_value=True) + def test_check_network_plugin_inventory_ovn_mismatch(self, mock_file): + fake_env = { + 'parameter_defaults': { + 'NeutronMechanismDrivers': ['openvswitch']}, + 'resource_registry': { + 'OS::TripleO::Services::NeutronApi': 'foo'} + } + with self.assertRaises(exceptions.InvalidConfiguration): + mock_open_ctx = mock.mock_open(read_data='ovn_controller') + with mock.patch('builtins.open', mock_open_ctx): + utils.check_network_plugin('/tmp', + fake_env) + @mock.patch('subprocess.check_call') @mock.patch('os.path.exists') def test_remove_known_hosts(self, mock_exists, mock_check_call): diff --git a/tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py b/tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py index 232b3f159..ea1327168 100644 --- a/tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py +++ b/tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py @@ -456,6 +456,94 @@ class TestDeployUndercloud(TestPluginV1): mock_yaml_dump.assert_has_calls([mock.call(rewritten_env, default_flow_style=False)]) + @mock.patch('tripleoclient.utils.check_network_plugin') + @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy._is_undercloud_deploy') + @mock.patch('tripleoclient.utils.fetch_roles_file', + return_value={}, autospec=True) + @mock.patch('heatclient.common.template_utils.' + 'process_environment_and_files', return_value=({}, {}), + autospec=True) + @mock.patch('heatclient.common.template_utils.' + 'get_template_contents', return_value=({}, {}), + autospec=True) + @mock.patch('heatclient.common.environment_format.' + 'parse', autospec=True, return_value=dict()) + @mock.patch('heatclient.common.template_format.' + 'parse', autospec=True, return_value=dict()) + @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.' + '_setup_heat_environments', autospec=True) + @mock.patch('tripleo_common.image.kolla_builder.' + 'container_images_prepare_multi') + def test_deploy_tripleo_heat_templates_nw_plugin_uc(self, + mock_cipm, + mock_setup_heat_envs, + mock_hc_templ_parse, + mock_hc_env_parse, + mock_hc_get_templ_cont, + mock_hc_process, + mock_role_data, + mock_is_uc, + mock_check_nw_plugin): + + with tempfile.NamedTemporaryFile(delete=False) as roles_file: + self.addCleanup(os.unlink, roles_file.name) + + mock_cipm.return_value = {} + mock_is_uc.return_value = True + + parsed_args = self.check_parser(self.cmd, + ['--local-ip', '127.0.0.1/8', + '--upgrade', '--output-dir', '/my', + '--roles-file', roles_file.name], []) + + self.cmd._deploy_tripleo_heat_templates(self.orc, parsed_args) + + mock_check_nw_plugin.assert_called_once_with('/my', {}) + + @mock.patch('tripleoclient.utils.check_network_plugin') + @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy._is_undercloud_deploy') + @mock.patch('tripleoclient.utils.fetch_roles_file', + return_value={}, autospec=True) + @mock.patch('heatclient.common.template_utils.' + 'process_environment_and_files', return_value=({}, {}), + autospec=True) + @mock.patch('heatclient.common.template_utils.' + 'get_template_contents', return_value=({}, {}), + autospec=True) + @mock.patch('heatclient.common.environment_format.' + 'parse', autospec=True, return_value=dict()) + @mock.patch('heatclient.common.template_format.' + 'parse', autospec=True, return_value=dict()) + @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.' + '_setup_heat_environments', autospec=True) + @mock.patch('tripleo_common.image.kolla_builder.' + 'container_images_prepare_multi') + def test_deploy_tripleo_heat_templates_nw_plugin_st(self, + mock_cipm, + mock_setup_heat_envs, + mock_hc_templ_parse, + mock_hc_env_parse, + mock_hc_get_templ_cont, + mock_hc_process, + mock_role_data, + mock_is_uc, + mock_check_nw_plugin): + + with tempfile.NamedTemporaryFile(delete=False) as roles_file: + self.addCleanup(os.unlink, roles_file.name) + + mock_cipm.return_value = {} + mock_is_uc.return_value = False + + parsed_args = self.check_parser(self.cmd, + ['--local-ip', '127.0.0.1/8', + '--upgrade', '--output-dir', '/my', + '--roles-file', roles_file.name], []) + + self.cmd._deploy_tripleo_heat_templates(self.orc, parsed_args) + + mock_check_nw_plugin.assert_not_called() + @mock.patch('shutil.copy') @mock.patch('os.path.exists', return_value=False) def test_normalize_user_templates(self, mock_exists, mock_copy): diff --git a/tripleoclient/utils.py b/tripleoclient/utils.py index ad568d2df..3d28e65d8 100644 --- a/tripleoclient/utils.py +++ b/tripleoclient/utils.py @@ -1186,6 +1186,54 @@ def check_swift_and_rgw(old_env, env, stage): 'RGW)') +def check_network_plugin(output_dir, env): + """Disallow upgrade if change in network plugin detected + + If the undercloud is upgraded with a change in network plugin + i.e ovs to ovn or ovn to ovs it will break the undercloud as + just switching is not enough it needs network resources to be + migrated, so we detect if there is change in network and block + the upgrade + """ + + neutron_env = env.get('resource_registry', + {}).get('OS::TripleO::Services::NeutronApi', + 'OS::Heat::None') + + # Neutron is not deployed so just return + if neutron_env == "OS::Heat::None": + return + + parameters = env.get('parameter_defaults', {}) + + file_name = constants.TRIPLEO_STATIC_INVENTORY + + inventory_path = os.path.join(output_dir, file_name) + + if not os.path.isfile(inventory_path): + message = (_("The %s inventory file is missing. Without it " + "network plugin change can't be detected, and upgrade " + "will have issues if there is a change" % inventory_path)) + LOG.error(message) + raise exceptions.InvalidConfiguration(message) + + with open(inventory_path, 'r') as f: + inventory_data = yaml.safe_load(f) + + if ('neutron_ovs_agent' in inventory_data and + 'ovn' in parameters.get('NeutronMechanismDrivers')): + message = _("Network Plugin mismatch detected, " + "Upgrade from ml2 ovs to ml2 ovn is not allowed") + LOG.error(message) + raise exceptions.InvalidConfiguration(message) + elif ("ovn_controller" in inventory_data and + "openvswitch" in parameters.get('NeutronMechanismDrivers')): + message = _("Network Plugin mismatch detected, " + "Upgrade from ml2 ovn to ml2 ovs is not allowed") + LOG.error(message) + raise exceptions.InvalidConfiguration(message) + + def check_nic_config_with_ansible(environment): registry = environment.get('resource_registry', {}) is_ansible_config = environment.get( diff --git a/tripleoclient/v1/tripleo_deploy.py b/tripleoclient/v1/tripleo_deploy.py index b25f78fbd..25ad94e8e 100644 --- a/tripleoclient/v1/tripleo_deploy.py +++ b/tripleoclient/v1/tripleo_deploy.py @@ -758,6 +758,10 @@ class Deploy(command.Command): # check if we're trying to deploy ceph during the overcloud deployment utils.check_deployed_ceph_stage(env) + # check network plugin with undercloud upgrade + if parsed_args.upgrade and self._is_undercloud_deploy(parsed_args): + utils.check_network_plugin(parsed_args.output_dir, env) + roles_data = utils.fetch_roles_file( roles_file_path, parsed_args.templates)