From: Erik Johannes Date: Mon, 5 Oct 2015 21:24:55 +0000 (-0700) Subject: NetApp: Cleanup if E-Series volume create fails X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=a4bcb1e11bf48207c81e42ba3b9c96f5e13d3ef6;p=openstack-build%2Fcinder-build.git NetApp: Cleanup if E-Series volume create fails The creation of a volume may fail in such a manner that a volume in a partial state has been created. When Openstack comes back re-attempting the creation operation it fails because the partial volume with the name already exists. Solution: When there is a failure creating a volume, check if a partial volume was created and delete it so any retry will be successful and the storage device does not contain any dead volumes. Closes-Bug: 1506940 Change-Id: Id93527a4cab314f27a13d8c08b772fed538d1092 --- diff --git a/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py b/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py index de69391b6..90e5c27d4 100644 --- a/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py +++ b/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py @@ -878,6 +878,78 @@ class NetAppEseriesLibraryTestCase(test.TestCase): mock_invoke.assert_not_called() + @mock.patch.object(library, 'LOG', mock.Mock()) + def test_create_volume_fail_clean(self): + """Test volume creation fail w/o a partial volume being created. + + Test the failed creation of a volume where a partial volume with + the name has not been created, thus no cleanup is required. + """ + self.library._get_volume = mock.Mock( + side_effect = exception.VolumeNotFound(message='')) + self.library._client.create_volume = mock.Mock( + side_effect = exception.NetAppDriverException) + self.library._client.delete_volume = mock.Mock() + fake_volume = copy.deepcopy(get_fake_volume()) + + self.assertRaises(exception.NetAppDriverException, + self.library.create_volume, fake_volume) + + self.assertTrue(self.library._get_volume.called) + self.assertFalse(self.library._client.delete_volume.called) + self.assertEqual(1, library.LOG.error.call_count) + + @mock.patch.object(library, 'LOG', mock.Mock()) + def test_create_volume_fail_dirty(self): + """Test volume creation fail where a partial volume has been created. + + Test scenario where the creation of a volume fails and a partial + volume is created with the name/id that was supplied by to the + original creation call. In this situation the partial volume should + be detected and removed. + """ + fake_volume = copy.deepcopy(get_fake_volume()) + self.library._get_volume = mock.Mock(return_value=fake_volume) + self.library._client.list_volume = mock.Mock(return_value=fake_volume) + self.library._client.create_volume = mock.Mock( + side_effect = exception.NetAppDriverException) + self.library._client.delete_volume = mock.Mock() + + self.assertRaises(exception.NetAppDriverException, + self.library.create_volume, fake_volume) + + self.assertTrue(self.library._get_volume.called) + self.assertTrue(self.library._client.delete_volume.called) + self.library._client.delete_volume.assert_called_once_with( + fake_volume["id"]) + self.assertEqual(1, library.LOG.error.call_count) + + @mock.patch.object(library, 'LOG', mock.Mock()) + def test_create_volume_fail_dirty_fail_delete(self): + """Volume creation fail with partial volume deletion fails + + Test scenario where the creation of a volume fails and a partial + volume is created with the name/id that was supplied by to the + original creation call. The partial volume is detected but when + the cleanup deletetion of that fragment volume is attempted it fails. + """ + fake_volume = copy.deepcopy(get_fake_volume()) + self.library._get_volume = mock.Mock(return_value=fake_volume) + self.library._client.list_volume = mock.Mock(return_value=fake_volume) + self.library._client.create_volume = mock.Mock( + side_effect = exception.NetAppDriverException) + self.library._client.delete_volume = mock.Mock( + side_effect = exception.NetAppDriverException) + + self.assertRaises(exception.NetAppDriverException, + self.library.create_volume, fake_volume) + + self.assertTrue(self.library._get_volume.called) + self.assertTrue(self.library._client.delete_volume.called) + self.library._client.delete_volume.assert_called_once_with( + fake_volume["id"]) + self.assertEqual(2, library.LOG.error.call_count) + @ddt.ddt class NetAppEseriesLibraryMultiAttachTestCase(test.TestCase): diff --git a/cinder/volume/drivers/netapp/eseries/library.py b/cinder/volume/drivers/netapp/eseries/library.py index 323af2d6c..a7ef5c119 100644 --- a/cinder/volume/drivers/netapp/eseries/library.py +++ b/cinder/volume/drivers/netapp/eseries/library.py @@ -428,6 +428,26 @@ class NetAppESeriesLibrary(object): except exception.NetAppDriverException as e: with excutils.save_and_reraise_exception(): LOG.error(_LE("Error creating volume. Msg - %s."), e) + # There was some kind failure creating the volume, make sure no + # partial flawed work exists + try: + bad_vol = self._get_volume(eseries_volume_label) + except Exception: + # Swallowing the exception intentionally because this is + # emergency cleanup to make sure no intermediate volumes + # were left. In this whole error situation, the more + # common route would be for no volume to have been created. + pass + else: + # Some sort of partial volume was created despite the + # error. Lets clean it out so no partial state volumes or + # orphans are left. + try: + self._client.delete_volume(bad_vol["id"]) + except exception.NetAppDriverException as e2: + LOG.error(_LE( + "Error cleaning up failed volume creation. " + "Msg - %s."), e2) return vol