From b5c0ef66c4ec9a3f2c0637f259dad42263bceaf8 Mon Sep 17 00:00:00 2001 From: weikeyou Date: Wed, 25 Jul 2018 16:59:32 +0800 Subject: [PATCH] Delete error volume which auto_remove is True Create a container and create a volume at the same time. If the volume is error , it would not be deleted even if it is auto_removed. Because this error occured in _wait_for_volumes_available, VolumeMapping object has not created in sql at that time. But the _fail_container get the volumes from the sql. We can delete these volumes before calling _fail_container in _wait_for_volumes_available(). Change-Id: I3cbf78bd57dcf56da09c94965679583fd63cdbd5 Closes-bug: #1783517 --- zun/compute/manager.py | 8 ++++++++ zun/tests/unit/compute/test_compute_manager.py | 18 ++++++++++++++++++ zun/volume/driver.py | 9 +++++---- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/zun/compute/manager.py b/zun/compute/manager.py index 8cafa5156..04a21f69d 100644 --- a/zun/compute/manager.py +++ b/zun/compute/manager.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +import copy import itertools import six @@ -169,6 +170,7 @@ class Manager(periodic_task.PeriodicTasks): def _wait_for_volumes_available(self, context, volumes, container, timeout=60, poll_interval=1): start_time = time.time() + request_volumes = copy.deepcopy(volumes) try: volumes = itertools.chain(volumes) volume = next(volumes) @@ -178,6 +180,12 @@ class Manager(periodic_task.PeriodicTasks): time.sleep(poll_interval) except StopIteration: return + for volume in request_volumes: + if volume.auto_remove: + try: + self.driver.delete_volume(context, volume) + except Exception: + LOG.exception("Failed to delete volume") msg = _("Volumes did not reach available status after" "%d seconds") % (timeout) self._fail_container(context, container, msg, unset_host=True) diff --git a/zun/tests/unit/compute/test_compute_manager.py b/zun/tests/unit/compute/test_compute_manager.py index ae519c87c..44eeeafeb 100644 --- a/zun/tests/unit/compute/test_compute_manager.py +++ b/zun/tests/unit/compute/test_compute_manager.py @@ -1289,6 +1289,24 @@ class TestManager(base.TestCase): mock_is_volume_available.assert_called_once() mock_fail.assert_not_called() + @mock.patch.object(fake_driver, 'delete_volume') + @mock.patch.object(fake_driver, 'is_volume_available') + @mock.patch.object(manager.Manager, '_fail_container') + def test_wait_for_volumes_available_failed(self, mock_fail, + mock_is_volume_available, + mock_delete_volume): + mock_is_volume_available.return_value = False + container = Container(self.context, **utils.get_test_container()) + volume = FakeVolumeMapping() + volume.auto_remove = True + volumes = [volume] + self.assertRaises(exception.Conflict, + self.compute_manager._wait_for_volumes_available, + self.context, volumes, container, timeout=2) + self.assertTrue(mock_is_volume_available.called) + self.assertTrue(mock_fail.called) + self.assertTrue(mock_delete_volume.called) + @mock.patch.object(Network, 'save') @mock.patch.object(fake_driver, 'create_network') def test_network_create(self, mock_create, mock_save): diff --git a/zun/volume/driver.py b/zun/volume/driver.py index 9e29c4148..3631bb2d4 100644 --- a/zun/volume/driver.py +++ b/zun/volume/driver.py @@ -121,10 +121,11 @@ class Cinder(VolumeDriver): cinder.delete_volume(volume) def _unmount_device(self, volume): - conn_info = jsonutils.loads(volume.connection_info) - devpath = conn_info['data']['device_path'] - mountpoint = mount.get_mountpoint(volume.volume_id) - mount.do_unmount(devpath, mountpoint) + if hasattr(volume, 'connection_info'): + conn_info = jsonutils.loads(volume.connection_info) + devpath = conn_info['data']['device_path'] + mountpoint = mount.get_mountpoint(volume.volume_id) + mount.do_unmount(devpath, mountpoint) @validate_volume_provider(supported_providers) def bind_mount(self, context, volume):