From e223f2a0fe97fae46e0fbb1f102dcdca980a52f1 Mon Sep 17 00:00:00 2001 From: Lauren Taylor Date: Wed, 20 Apr 2016 11:21:46 -0700 Subject: [PATCH] Log warning for failed deletion of NVRAM Currently, if NVRAM deletion fails, an exception is raised and causes a failure. If we fail to remove the entry from swift, the worst case scenario is if the data is really in swift, it sticks around and never gets cleaned out. Instead, if NVRAM deletion fails, we should log a warning message. Change-Id: I671f492c2b6640b4a8ce150524d95a09ca5f86b0 Closes-Bug: 1568936 --- nova_powervm/tests/virt/powervm/nvram/fake_api.py | 11 +++++++++++ nova_powervm/tests/virt/powervm/nvram/test_manager.py | 9 +++++++++ nova_powervm/virt/powervm/nvram/manager.py | 7 ++++++- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/nova_powervm/tests/virt/powervm/nvram/fake_api.py b/nova_powervm/tests/virt/powervm/nvram/fake_api.py index 7a06865c..5edd2d11 100644 --- a/nova_powervm/tests/virt/powervm/nvram/fake_api.py +++ b/nova_powervm/tests/virt/powervm/nvram/fake_api.py @@ -53,4 +53,15 @@ class ExpNvramStore(NoopNvramStore): :param instance: instance object :returns: the NVRAM data base64 encoded string """ + # Raise exception. This is to ensure fetch causes a failure + # when an exception is raised + raise Exception('Error') + + def delete(self, instance): + """Delete the NVRAM from the storage service. + + :param instance: instance object + """ + # Raise excpetion. This is to ensure delete does not fail + # despite an exception being raised raise Exception('Error') diff --git a/nova_powervm/tests/virt/powervm/nvram/test_manager.py b/nova_powervm/tests/virt/powervm/nvram/test_manager.py index 715c5e36..778428e6 100644 --- a/nova_powervm/tests/virt/powervm/nvram/test_manager.py +++ b/nova_powervm/tests/virt/powervm/nvram/test_manager.py @@ -35,6 +35,8 @@ class TestNvramManager(test.TestCase): fixtures.MockPatchObject(self.fake_store, 'store')).mock self.mock_fetch = self.useFixture( fixtures.MockPatchObject(self.fake_store, 'fetch')).mock + self.mock_remove = self.useFixture( + fixtures.MockPatchObject(self.fake_store, 'delete')).mock @mock.patch.object(vm, 'get_instance_wrapper') def test_manager(self, mock_get_inst): @@ -45,6 +47,8 @@ class TestNvramManager(test.TestCase): mgr.fetch(powervm.TEST_INST2) + mgr.remove(powervm.TEST_INST2) + # Simulate a quick repeated stores of the same LPAR by poking the Q. mgr._queue.put(powervm.TEST_INST1) mgr._queue.put(powervm.TEST_INST1) @@ -56,9 +60,14 @@ class TestNvramManager(test.TestCase): [mock.call(powervm.TEST_INST1, mock.ANY), mock.call(powervm.TEST_INST2, mock.ANY)]) self.mock_fetch.assert_called_with(powervm.TEST_INST2) + self.mock_remove.assert_called_with(powervm.TEST_INST2) # Test when fetch returns an exception mgr_exp = manager.NvramManager(self.fake_exp_store, mock.Mock(), mock.Mock()) self.assertRaises(api.NVRAMDownloadException, mgr_exp.fetch, powervm.TEST_INST2) + + # Test exception being logged but not raised during remove + mgr_exp.remove(powervm.TEST_INST2) + self.mock_remove.assert_called_with(powervm.TEST_INST2) diff --git a/nova_powervm/virt/powervm/nvram/manager.py b/nova_powervm/virt/powervm/nvram/manager.py index 489e48bd..71f92c61 100644 --- a/nova_powervm/virt/powervm/nvram/manager.py +++ b/nova_powervm/virt/powervm/nvram/manager.py @@ -120,7 +120,12 @@ class NvramManager(object): # Remove any pending updates self._pop_from_list(uuid=instance.uuid) # Remove it from the store - self._api.delete(instance) + try: + self._api.delete(instance) + except Exception as e: + # Delete exceptions should not end the operation + LOG.warning(_LW('Could not delete NVRAM: %s'), e, + instance=instance) @lockutils.synchronized(LOCK_NVRAM_UPDT_LIST) def _add_to_list(self, instance):