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

@@ -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')

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)

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