Do not allow uc upgrade when network plugin changes

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.

[1] is switching default network plugin on Undercloud
from ovs to ovn. But as direct upgrade is not possible
from ovs to ovn adding a check for the same and block
the undercloud upgrade.

[1] https://review.opendev.org/850158

Change-Id: I4a80380ab28edf318019cb0bf05765d329ed0405

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.

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):

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):

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(

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)