From dacae335e4c538f54a32f431469bd7059cba48c3 Mon Sep 17 00:00:00 2001 From: Alexey Stupnikov Date: Mon, 3 Apr 2023 16:19:35 +0200 Subject: [PATCH] Process unlimited exceptions raised by unplug_vifs Currently compute manager's _cleanup_allocated_networks() method expects NotImplementedError or exception.NovaException when calling self.driver.unplug_vifs. In reality, other class of exception could be raised. It could break the Nova Compute flow and leave instance in inconsistent state. This patch switches from exception.NovaException to all kinds of exceptions. Closes-bug: #2015092 Change-Id: Icaf3cc93edfea97ee4fa497bdeb5f7d631c8ae55 --- nova/compute/manager.py | 2 +- nova/tests/unit/compute/test_compute_mgr.py | 25 ++++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 5ea71827fcf3..d25ca17e33e8 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -2985,7 +2985,7 @@ class ComputeManager(manager.Manager): 'Virt driver does not provide unplug_vifs method, so it ' 'is not possible determine if VIFs should be unplugged.' ) - except exception.NovaException as exc: + except Exception as exc: # It's possible that the instance never got as far as plugging # VIFs, in which case we would see an exception which can be # mostly ignored diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 1c69cd8f1c05..1d1500731486 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -8478,7 +8478,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): 'False', self.instance.system_metadata['network_allocated']) @mock.patch('nova.compute.manager.LOG') - def test__cleanup_allocated_networks__error(self, mock_log): + def test__cleanup_allocated_networks_neutron_error(self, mock_log): with test.nested( mock.patch.object( self.compute.network_api, 'get_instance_nw_info', @@ -8497,6 +8497,29 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): ) mock_unplug.assert_not_called() + @mock.patch('nova.compute.manager.LOG.warning') + def test__cleanup_allocated_networks_osvif_error(self, mock_log): + with test.nested( + mock.patch.object(self.compute.network_api, + 'get_instance_nw_info'), + mock.patch.object(self.compute.driver, 'unplug_vifs', + side_effect=ValueError('Malformed MAC 40:28:0:00:2:6')), + mock.patch.object(self.compute, '_deallocate_network'), + mock.patch.object(self.instance, 'save'), + ) as (mock_nwinfo, mock_unplug, mock_deallocate_network, mock_save): + self.compute._cleanup_allocated_networks( + self.context, self.instance, self.requested_networks) + + mock_nwinfo.assert_called_once_with(self.context, self.instance) + self.assertEqual(1, mock_log.call_count) + self.assertIn( + 'Cleaning up VIFs failed for instance. Error: %s', + mock_log.call_args[0][0], + ) + mock_deallocate_network.assert_called_once_with( + self.context, self.instance, self.requested_networks) + mock_save.assert_called_once_with() + def test_split_network_arqs(self): arqs = [ {'uuid': uuids.arq_uuid1},