diff -Nru nova-17.0.13/debian/changelog nova-17.0.13/debian/changelog --- nova-17.0.13/debian/changelog 2020-08-28 08:10:08.000000000 -0400 +++ nova-17.0.13/debian/changelog 2021-05-17 14:25:43.000000000 -0400 @@ -1,3 +1,13 @@ +nova (2:17.0.13-0ubuntu2) bionic; urgency=high + + * d/p/0001-Force-refresh-instance-info_cache-during-heal.patch: + Fixes LP: #1751923. + + * d/p/0002-remove-deprecated-test_list_vifs_neutron_notimplemented.patch: + Removed unused/deprecated test that Fixes LP: #1751923. + + -- Jorge Niedbalski Mon, 17 May 2021 14:25:43 -0400 + nova (2:17.0.13-0ubuntu1) bionic; urgency=medium * New stable point release for OpenStack Queens (LP: #1893234). diff -Nru nova-17.0.13/debian/patches/0001-Force-refresh-instance-info_cache-during-heal.patch nova-17.0.13/debian/patches/0001-Force-refresh-instance-info_cache-during-heal.patch --- nova-17.0.13/debian/patches/0001-Force-refresh-instance-info_cache-during-heal.patch 1969-12-31 21:00:00.000000000 -0300 +++ nova-17.0.13/debian/patches/0001-Force-refresh-instance-info_cache-during-heal.patch 2021-05-17 14:20:37.000000000 -0400 @@ -0,0 +1,372 @@ +From 0c2371b0a86414fd513d63ffdce9df7fd9075d23 Mon Sep 17 00:00:00 2001 +From: Matt Riedemann +Date: Tue, 14 Aug 2018 17:57:53 +0800 +Subject: [PATCH] Force refresh instance info_cache during heal + +If the instance info_cache is corrupted somehow, like during +a host reboot and the ports aren't wired up properly or +a mistaken policy change in neutron results in nova resetting +the info_cache to an empty list, the _heal_instance_info_cache +is meant to fix it (once the current state of the ports for +the instance in neutron is corrected). However, the task is +currently only refreshing the cache *based* on the current contents +of the cache, which defeats the purpose of neutron being the source +of truth for the ports attached to the instance. + +This change makes the _heal_instance_info_cache periodic task +pass a "force_refresh" kwarg, which defaults to False for backward +compatibility with other methods that refresh the cache after +operations like attach/detach interface, and if True will make +nova get the current state of the ports for the instance from neutron +and fully rebuild the info_cache. + +To not lose port order in info_cache this change takes original order +from nova historical data that are stored as VirtualInterfaceList +objects. For ports that are not registered as VirtualInterfaces +objects it will add them at the end of port_order list. Due to this +for instances older than Newton another patch was introduced to fill +missing VirtualInterface objects in the DB [1]. + +Long-term we should be able to refactor some of the older refresh +code which leverages the cache to instead use the refresh_vif_id +kwarg so that we do targeted cache updates when we do things like +attach and detach ports, but that's a change for another day. + +[1] https://review.openstack.org/#/c/614167 + +Co-Authored-By: Maciej Jozefczyk +Change-Id: I629415236b2447128ae9a980d4ebe730a082c461 +Closes-Bug: #1751923 +--- + nova/compute/manager.py | 3 +- + nova/network/neutronv2/api.py | 81 ++++++++++++-- + nova/tests/unit/compute/test_compute.py | 4 +- + nova/tests/unit/network/test_neutronv2.py | 126 ++++++++++++++++++++++ + 4 files changed, 203 insertions(+), 11 deletions(-) + +diff --git a/nova/compute/manager.py b/nova/compute/manager.py +index adc6696b3d..a4834bfa89 100644 +--- a/nova/compute/manager.py ++++ b/nova/compute/manager.py +@@ -7127,7 +7127,8 @@ class ComputeManager(manager.Manager): + try: + # Call to network API to get instance info.. this will + # force an update to the instance's info_cache +- self.network_api.get_instance_nw_info(context, instance) ++ self.network_api.get_instance_nw_info( ++ context, instance, force_refresh=True) + LOG.debug('Updated the network info_cache for instance', + instance=instance) + except exception.InstanceNotFound: +diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py +index 6ec851f597..3216cbfbee 100644 +--- a/nova/network/neutronv2/api.py ++++ b/nova/network/neutronv2/api.py +@@ -1445,7 +1445,8 @@ class API(base_api.NetworkAPI): + def _get_instance_nw_info(self, context, instance, networks=None, + port_ids=None, admin_client=None, + preexisting_port_ids=None, +- refresh_vif_id=None, **kwargs): ++ refresh_vif_id=None, force_refresh=False, ++ **kwargs): + # NOTE(danms): This is an inner method intended to be called + # by other code that updates instance nwinfo. It *must* be + # called with the refresh_cache-%(instance_uuid) lock held! +@@ -1457,12 +1458,17 @@ class API(base_api.NetworkAPI): + nw_info = self._build_network_info_model(context, instance, networks, + port_ids, admin_client, + preexisting_port_ids, +- refresh_vif_id) ++ refresh_vif_id, ++ force_refresh=force_refresh) + return network_model.NetworkInfo.hydrate(nw_info) + + def _gather_port_ids_and_networks(self, context, instance, networks=None, + port_ids=None, neutron=None): +- """Return an instance's complete list of port_ids and networks.""" ++ """Return an instance's complete list of port_ids and networks. ++ ++ The results are based on the instance info_cache in the nova db, not ++ the instance's current list of ports in neutron. ++ """ + + if ((networks is None and port_ids is not None) or + (port_ids is None and networks is not None)): +@@ -2097,7 +2103,8 @@ class API(base_api.NetworkAPI): + return port['device_id'] + + def get_vifs_by_instance(self, context, instance): +- raise NotImplementedError() ++ return objects.VirtualInterfaceList.get_by_instance_uuid(context, ++ instance.uuid) + + def get_vif_by_mac_address(self, context, mac_address): + raise NotImplementedError() +@@ -2426,7 +2433,7 @@ class API(base_api.NetworkAPI): + def _build_network_info_model(self, context, instance, networks=None, + port_ids=None, admin_client=None, + preexisting_port_ids=None, +- refresh_vif_id=None): ++ refresh_vif_id=None, force_refresh=False): + """Return list of ordered VIFs attached to instance. + + :param context: Request context. +@@ -2448,6 +2455,10 @@ class API(base_api.NetworkAPI): + cache rather than the entire cache. This can be + triggered via a "network-changed" server external event + from Neutron. ++ :param force_refresh: If ``networks`` and ``port_ids`` are both None, ++ by default the instance.info_cache will be used to ++ populate the network info. Pass ``True`` to force ++ collection of ports and networks from neutron directly. + """ + + search_opts = {'tenant_id': instance.project_id, +@@ -2519,11 +2530,28 @@ class API(base_api.NetworkAPI): + return nw_info + # else there is no existing cache and we need to build it + ++ # Determine if we're doing a full refresh (_heal_instance_info_cache) ++ # or if we are refreshing because we have attached/detached a port. ++ # TODO(mriedem); we should leverage refresh_vif_id in the latter case ++ # since we are unnecessarily rebuilding the entire cache for one port + nw_info_refresh = networks is None and port_ids is None +- networks, port_ids = self._gather_port_ids_and_networks( +- context, instance, networks, port_ids, client) +- nw_info = network_model.NetworkInfo() ++ if nw_info_refresh and force_refresh: ++ # Use the current set of ports from neutron rather than the cache. ++ port_ids = self._get_ordered_port_list(context, instance, ++ current_neutron_ports) ++ net_ids = [current_neutron_port_map.get(port_id).get('network_id') ++ for port_id in port_ids] ++ ++ # This is copied from _gather_port_ids_and_networks. ++ networks = self._get_available_networks( ++ context, instance.project_id, net_ids, client) ++ else: ++ # We are refreshing the full cache using the existing cache rather ++ # than what is currently in neutron. ++ networks, port_ids = self._gather_port_ids_and_networks( ++ context, instance, networks, port_ids, client) + ++ nw_info = network_model.NetworkInfo() + for port_id in port_ids: + current_neutron_port = current_neutron_port_map.get(port_id) + if current_neutron_port: +@@ -2539,6 +2567,43 @@ class API(base_api.NetworkAPI): + + return nw_info + ++ def _get_ordered_port_list(self, context, instance, current_neutron_ports): ++ """Returns ordered port list using nova virtual_interface data.""" ++ ++ # a dict, keyed by port UUID, of the port's "index" ++ # so that we can order the returned port UUIDs by the ++ # original insertion order followed by any newly-attached ++ # ports ++ port_uuid_to_index_map = {} ++ port_order_list = [] ++ ports_without_order = [] ++ ++ # Get set of ports from nova vifs ++ vifs = self.get_vifs_by_instance(context, instance) ++ for port in current_neutron_ports: ++ # NOTE(mjozefcz): For each port check if we have its index from ++ # nova virtual_interfaces objects. If not - it seems ++ # to be a new port - add it at the end of list. ++ ++ # Find port index if it was attached before. ++ for vif in vifs: ++ if vif.uuid == port['id']: ++ port_uuid_to_index_map[port['id']] = vif.id ++ break ++ ++ if port['id'] not in port_uuid_to_index_map: ++ # Assume that it's new port and add it to the end of port list. ++ ports_without_order.append(port['id']) ++ ++ # Lets sort created port order_list by given index. ++ port_order_list = sorted(port_uuid_to_index_map, ++ key=lambda k: port_uuid_to_index_map[k]) ++ ++ # Add ports without order to the end of list ++ port_order_list.extend(ports_without_order) ++ ++ return port_order_list ++ + def _get_subnets_from_port(self, context, port, client=None): + """Return the subnets for a given port.""" + +diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py +index f8dab8a86d..53face55e6 100644 +--- a/nova/tests/unit/compute/test_compute.py ++++ b/nova/tests/unit/compute/test_compute.py +@@ -7228,14 +7228,14 @@ class ComputeTestCase(BaseTestCase, + return instance_map[instance_uuid] + + # NOTE(comstud): Override the stub in setUp() +- def fake_get_instance_nw_info(cls, context, instance, +- use_slave=False): ++ def fake_get_instance_nw_info(cls, context, instance, **kwargs): + # Note that this exception gets caught in compute/manager + # and is ignored. However, the below increment of + # 'get_nw_info' won't happen, and you'll get an assert + # failure checking it below. + self.assertEqual(call_info['expected_instance']['uuid'], + instance['uuid']) ++ self.assertTrue(kwargs['force_refresh']) + call_info['get_nw_info'] += 1 + if _get_instance_nw_info_raise: + raise exception.InstanceNotFound(instance_id=instance['uuid']) +diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py +index 312b08763a..4d6b937616 100644 +--- a/nova/tests/unit/network/test_neutronv2.py ++++ b/nova/tests/unit/network/test_neutronv2.py +@@ -43,6 +43,7 @@ from nova.network.neutronv2 import api as neutronapi + from nova.network.neutronv2 import constants + from nova import objects + from nova.objects import network_request as net_req_obj ++from nova.objects import virtual_interface as obj_vif + from nova.pci import manager as pci_manager + from nova.pci import utils as pci_utils + from nova.pci import whitelist as pci_whitelist +@@ -6412,6 +6413,11 @@ class TestGetInstanceNetworkInfo(test.NoDBTestCase): + network_id = kwargs.get('network_id', uuids.network_id) + return {'id': port_id, 'network_id': network_id} + ++ @staticmethod ++ def _get_fake_vif(context, **kwargs): ++ """Returns VirtualInterface based on provided VIF ID""" ++ return obj_vif.VirtualInterface(context=context, **kwargs) ++ + def test_get_nw_info_refresh_vif_id_add_vif(self): + """Tests that a network-changed event occurred on a single port + which is not already in the cache so it's added. +@@ -6502,3 +6508,123 @@ class TestGetInstanceNetworkInfo(test.NoDBTestCase): + self.assertIsNotNone(old_vif) + removed_vif = self._get_vif_in_cache(nwinfo, uuids.removed_port) + self.assertIsNone(removed_vif) ++ ++ def test_get_instance_nw_info_force_refresh(self): ++ """Tests a full refresh of the instance info cache using information ++ from neutron rather than the instance's current info cache data. ++ """ ++ # Fake out an empty cache. ++ self.instance.info_cache = self._get_fake_info_cache([]) ++ # The instance has one attached port in neutron. ++ self.client.list_ports.return_value = { ++ 'ports': [self._get_fake_port(uuids.port_id)]} ++ ordered_port_list = [uuids.port_id] ++ ++ with test.nested( ++ mock.patch.object(self.api, '_get_available_networks', ++ return_value=[{'id': uuids.network_id}]), ++ mock.patch.object(self.api, '_build_vif_model', ++ return_value=model.VIF(uuids.port_id)), ++ # We should not call _gather_port_ids_and_networks since that uses ++ # the existing instance.info_cache when no ports/networks are ++ # passed to _build_network_info_model and what we want is a full ++ # refresh of the ports based on what neutron says is current. ++ mock.patch.object(self.api, '_gather_port_ids_and_networks', ++ new_callable=mock.NonCallableMock), ++ mock.patch.object(self.api, '_get_ordered_port_list', ++ return_value=ordered_port_list) ++ ) as ( ++ get_nets, build_vif, gather_ports, mock_port_map ++ ): ++ nwinfo = self.api._get_instance_nw_info( ++ self.context, self.instance, force_refresh=True) ++ get_nets.assert_called_once_with( ++ self.context, self.instance.project_id, ++ [uuids.network_id], self.client) ++ # Assert that the port is in the cache now. ++ self.assertIsNotNone(self._get_vif_in_cache(nwinfo, uuids.port_id)) ++ ++ def test__get_ordered_port_list(self): ++ """This test if port_list is sorted by VirtualInterface id ++ sequence. ++ """ ++ nova_vifs = [ ++ self._get_fake_vif(self.context, ++ uuid=uuids.port_id_1, id=0), ++ self._get_fake_vif(self.context, ++ uuid=uuids.port_id_2, id=1), ++ self._get_fake_vif(self.context, ++ uuid=uuids.port_id_3, id=2), ++ ] ++ # Random order. ++ current_neutron_ports = [ ++ self._get_fake_port(uuids.port_id_2), ++ self._get_fake_port(uuids.port_id_1), ++ self._get_fake_port(uuids.port_id_3), ++ ] ++ expected_port_list = [uuids.port_id_1, ++ uuids.port_id_2, ++ uuids.port_id_3] ++ with mock.patch.object(self.api, 'get_vifs_by_instance', ++ return_value=nova_vifs): ++ port_list = self.api._get_ordered_port_list( ++ self.context, self.instance, current_neutron_ports) ++ self.assertEqual(expected_port_list, ++ port_list) ++ ++ def test__get_ordered_port_list_new_port(self): ++ """This test if port_list is sorted by VirtualInterface id ++ sequence while new port appears. ++ """ ++ nova_vifs = [ ++ self._get_fake_vif(self.context, ++ uuid=uuids.port_id_1, id=0), ++ self._get_fake_vif(self.context, ++ uuid=uuids.port_id_3, id=2), ++ ] ++ # New port appears. ++ current_neutron_ports = [ ++ self._get_fake_port(uuids.port_id_1), ++ self._get_fake_port(uuids.port_id_4), ++ self._get_fake_port(uuids.port_id_3) ++ ] ++ expected_port_list = [uuids.port_id_1, ++ uuids.port_id_3, ++ uuids.port_id_4] ++ with mock.patch.object(self.api, 'get_vifs_by_instance', ++ return_value=nova_vifs): ++ port_list = self.api._get_ordered_port_list( ++ self.context, self.instance, current_neutron_ports) ++ self.assertEqual(expected_port_list, ++ port_list) ++ ++ def test__get_ordered_port_list_new_port_and_deleted_vif(self): ++ """This test if port_list is sorted by VirtualInterface id ++ sequence while new port appears along with deleted old ++ VirtualInterface objects. ++ """ ++ # Display also deleted VirtualInterface. ++ nova_vifs = [ ++ self._get_fake_vif(self.context, ++ uuid=uuids.port_id_1, id=0, ++ deleted=True), ++ self._get_fake_vif(self.context, ++ uuid=uuids.port_id_2, id=3), ++ self._get_fake_vif(self.context, ++ uuid=uuids.port_id_3, id=5), ++ ] ++ # Random order and new port. ++ current_neutron_ports = [ ++ self._get_fake_port(uuids.port_id_4), ++ self._get_fake_port(uuids.port_id_3), ++ self._get_fake_port(uuids.port_id_2), ++ ] ++ expected_port_list = [uuids.port_id_2, ++ uuids.port_id_3, ++ uuids.port_id_4] ++ with mock.patch.object(self.api, 'get_vifs_by_instance', ++ return_value=nova_vifs): ++ port_list = self.api._get_ordered_port_list( ++ self.context, self.instance, current_neutron_ports) ++ self.assertEqual(expected_port_list, ++ port_list) +-- +2.25.1 + diff -Nru nova-17.0.13/debian/patches/0002-remove-deprecated-test_list_vifs_neutron_notimplemented.patch nova-17.0.13/debian/patches/0002-remove-deprecated-test_list_vifs_neutron_notimplemented.patch --- nova-17.0.13/debian/patches/0002-remove-deprecated-test_list_vifs_neutron_notimplemented.patch 1969-12-31 21:00:00.000000000 -0300 +++ nova-17.0.13/debian/patches/0002-remove-deprecated-test_list_vifs_neutron_notimplemented.patch 2021-05-17 14:25:43.000000000 -0400 @@ -0,0 +1,28 @@ +Description: This test has been deprecated +as part of stable/rocky and breaks compatibility +with change pushed in upstream LP: #1751923 + +Author: Jorge Niedbalski +Bug-Ubuntu: https://bugs.launchpad.net/bugs/1751923 + +--- nova-17.0.13.orig/nova/tests/unit/api/openstack/compute/test_virtual_interfaces.py ++++ nova-17.0.13/nova/tests/unit/api/openstack/compute/test_virtual_interfaces.py +@@ -121,18 +121,6 @@ class ServerVirtualInterfaceTestV21(test + 'fake_uuid', + expected_attrs=None) + +- def test_list_vifs_neutron_notimplemented(self): +- """Tests that a 400 is returned when using neutron as the backend""" +- # unset the get_vifs_by_instance stub from setUp +- self.get_vifs_by_instance_patcher.stop() +- self.flags(use_neutron=True) +- # reset the controller to use the neutron network API +- self._set_controller() +- req = fakes.HTTPRequest.blank('', version=self.wsgi_api_version) +- self.assertRaises(webob.exc.HTTPBadRequest, +- self.controller.index, req, FAKE_UUID) +- self.get_vifs_by_instance_patcher.start() +- + + class ServerVirtualInterfaceTestV212(ServerVirtualInterfaceTestV21): + wsgi_api_version = '2.12' diff -Nru nova-17.0.13/debian/patches/series nova-17.0.13/debian/patches/series --- nova-17.0.13/debian/patches/series 2020-08-28 08:10:08.000000000 -0400 +++ nova-17.0.13/debian/patches/series 2021-05-17 14:25:43.000000000 -0400 @@ -3,3 +3,5 @@ arm-console-patch.patch revert-generalize-db-conf-group-copying.patch skip-double-word-hacking-test.patch +0001-Force-refresh-instance-info_cache-during-heal.patch +0002-remove-deprecated-test_list_vifs_neutron_notimplemented.patch