diff --git a/glance_store/_drivers/cinder/store.py b/glance_store/_drivers/cinder/store.py index 6c208414..6281e923 100644 --- a/glance_store/_drivers/cinder/store.py +++ b/glance_store/_drivers/cinder/store.py @@ -569,16 +569,20 @@ class Store(glance_store.driver.Store): # image-volume to modification by users which might lead # to corruption of image. try: - volume = cinder_client.volumes.get(volume_id) - except cinder_exception.NotFound: - reason = (_LW("Image-Volume %s not found. If you have " - "upgraded your environment from single store " - "to multi store, transfer all your " - "Image-Volumes from user projects to service " - "project." - % volume_id)) - LOG.warning(reason) - return False + volume = self.volume_api.get(cinder_client, volume_id) + # NotFound Exception is translated to BackendException + except exceptions.BackendException as e: + # 'Volume could not be found. + # (HTTP 404) (Request-ID: )' + if 'HTTP 404' in str(e): + reason = (_LW("Image-Volume %s not found. If you have " + "upgraded your environment from single " + "store to multi store, transfer all your " + "Image-Volumes from user projects to " + "service project." + % volume_id)) + LOG.warning(reason) + return False if cinder_volume_type and volume.volume_type == cinder_volume_type: return True elif not cinder_volume_type: @@ -687,10 +691,12 @@ class Store(glance_store.driver.Store): raise exceptions.BadStoreConfiguration(store_name="cinder", reason=reason) - def _wait_volume_status(self, volume, status_transition, status_expected): + def _wait_volume_status(self, volume, status_transition, status_expected, + client): + volume_id = volume.id max_recheck_wait = 15 timeout = self.store_conf.cinder_state_transition_timeout - volume = volume.manager.get(volume.id) + volume = self.volume_api.get(client, volume_id) tries = 0 elapsed = 0 while volume.status == status_transition: @@ -705,7 +711,7 @@ class Store(glance_store.driver.Store): time.sleep(wait) tries += 1 elapsed += wait - volume = volume.manager.get(volume.id) + volume = self.volume_api.get(client, volume_id) if volume.status != status_expected: msg = (_('The status of volume %(volume_id)s is unexpected: ' 'status = %(status)s, expected = %(expected)s.') @@ -746,7 +752,7 @@ class Store(glance_store.driver.Store): LOG.debug('Attachment %(attachment_id)s created successfully.', {'attachment_id': attachment['id']}) - volume = volume.manager.get(volume_id) + volume = self.volume_api.get(client, volume_id) attachment_id = attachment['id'] connection_info = None try: @@ -853,7 +859,7 @@ class Store(glance_store.driver.Store): self._check_context(context) try: client = self.get_cinderclient(context, version='3.54') - volume = client.volumes.get(loc.volume_id) + volume = self.volume_api.get(client, loc.volume_id) size = int(volume.metadata.get('image_size', volume.size * units.Gi)) iterator = self._cinder_volume_data_iterator( @@ -896,7 +902,7 @@ class Store(glance_store.driver.Store): "internal error.")) return 0 - def _call_offline_extend(self, volume, size_gb): + def _call_offline_extend(self, volume, size_gb, client): size_gb += 1 LOG.debug("Extending (offline) volume %(volume_id)s to %(size)s GB.", {'volume_id': volume.id, 'size': size_gb}) @@ -904,7 +910,8 @@ class Store(glance_store.driver.Store): try: volume = self._wait_volume_status(volume, 'extending', - 'available') + 'available', + client) size_gb = volume.size return size_gb except exceptions.BackendException: @@ -918,7 +925,8 @@ class Store(glance_store.driver.Store): try: volume = self._wait_volume_status(volume, 'extending', - 'in-use') + 'in-use', + client) size_gb = volume.size return size_gb except exceptions.BackendException: @@ -965,7 +973,7 @@ class Store(glance_store.driver.Store): self._write_data(f, write_props) if write_props.need_extend: write_props.size_gb = self._call_offline_extend( - volume, write_props.size_gb) + volume, write_props.size_gb, client) def _online_extend(self, client, volume, write_props): with self._open_cinder_volume(client, volume, 'wb') as f: @@ -1048,7 +1056,8 @@ class Store(glance_store.driver.Store): metadata=metadata, volume_type=volume_type) - volume = self._wait_volume_status(volume, 'creating', 'available') + volume = self._wait_volume_status(volume, 'creating', 'available', + client) size_gb = volume.size failed = True diff --git a/glance_store/common/cinder_utils.py b/glance_store/common/cinder_utils.py index c1c4186b..5f7044d5 100644 --- a/glance_store/common/cinder_utils.py +++ b/glance_store/common/cinder_utils.py @@ -67,6 +67,15 @@ class API(object): volume = client.volumes.create(size, **kwargs) return volume + @handle_exceptions + def get(self, client, volume_id): + """Get the details about a volume given it's ID. + + :param client: cinderclient object + :param volume_id: UUID of the volume to get + """ + return client.volumes.get(volume_id) + def delete(self, client, volume_id): client.volumes.delete(volume_id) diff --git a/glance_store/tests/unit/cinder/test_cinder_base.py b/glance_store/tests/unit/cinder/test_cinder_base.py index 971bde61..59169284 100644 --- a/glance_store/tests/unit/cinder/test_cinder_base.py +++ b/glance_store/tests/unit/cinder/test_cinder_base.py @@ -163,43 +163,45 @@ class TestCinderStoreBase(object): @mock.patch.object(time, 'sleep') def test_wait_volume_status(self, mock_sleep): - fake_manager = mock.MagicMock(get=mock.Mock()) - volume_available = mock.MagicMock(manager=fake_manager, - id='fake-id', + volume_available = mock.MagicMock(id='fake-id', status='available') - volume_in_use = mock.MagicMock(manager=fake_manager, - id='fake-id', + volume_in_use = mock.MagicMock(id='fake-id', status='in-use') - fake_manager.get.side_effect = [volume_available, volume_in_use] + fake_volumes = mock.MagicMock() + fake_volumes.get.side_effect = [volume_available, volume_in_use] + fake_client = mock.MagicMock(volumes=fake_volumes) self.assertEqual(volume_in_use, self.store._wait_volume_status( - volume_available, 'available', 'in-use')) - fake_manager.get.assert_called_with('fake-id') + volume_available, 'available', 'in-use', + fake_client)) + fake_volumes.get.assert_called_with('fake-id') mock_sleep.assert_called_once_with(0.5) @mock.patch.object(time, 'sleep') def test_wait_volume_status_unexpected(self, mock_sleep): - fake_manager = mock.MagicMock(get=mock.Mock()) - volume_available = mock.MagicMock(manager=fake_manager, - id='fake-id', + volume_available = mock.MagicMock(id='fake-id', status='error') - fake_manager.get.return_value = volume_available + fake_volumes = mock.MagicMock( + get=mock.Mock(return_value=volume_available)) + fake_client = mock.MagicMock(volumes=fake_volumes) self.assertRaises(exceptions.BackendException, self.store._wait_volume_status, - volume_available, 'available', 'in-use') - fake_manager.get.assert_called_with('fake-id') + volume_available, 'available', 'in-use', + fake_client) + fake_volumes.get.assert_called_with('fake-id') @mock.patch.object(time, 'sleep') def test_wait_volume_status_timeout(self, mock_sleep): - fake_manager = mock.MagicMock(get=mock.Mock()) - volume_available = mock.MagicMock(manager=fake_manager, - id='fake-id', + volume_available = mock.MagicMock(id='fake-id', status='available') - fake_manager.get.return_value = volume_available + fake_volumes = mock.MagicMock( + get=mock.Mock(return_value=volume_available)) + fake_client = mock.MagicMock(volumes=fake_volumes) self.assertRaises(exceptions.BackendException, self.store._wait_volume_status, - volume_available, 'available', 'in-use') - fake_manager.get.assert_called_with('fake-id') + volume_available, 'available', 'in-use', + fake_client) + fake_volumes.get.assert_called_with('fake-id') def _test_open_cinder_volume(self, open_mode, attach_mode, error, multipath_supported=False, @@ -209,7 +211,6 @@ class TestCinderStoreBase(object): update_attachment_error=None): fake_volume = mock.MagicMock(id=str(uuid.uuid4()), status='available', multiattach=multiattach) - fake_volume.manager.get.return_value = fake_volume fake_attachment_id = str(uuid.uuid4()) fake_attachment_create = {'id': fake_attachment_id} if encrypted_nfs or qcow2_vol: @@ -457,8 +458,10 @@ class TestCinderStoreBase(object): with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc: mocked_cc.return_value = mock.MagicMock(volumes=fake_volumes) - self.assertRaises(exceptions.NotFound, method_call, loc, - context=self.context) + exc = exceptions.NotFound + if method_call == self.store.get: + exc = exceptions.BackendException + self.assertRaises(exc, method_call, loc, context=self.context) def test_cinder_get_volume_not_found(self): self._test_cinder_volume_not_found(self.store.get, 'get') @@ -546,9 +549,9 @@ class TestCinderStoreBase(object): self.config(cinder_volume_type='some_type', group=backend) fake_client = mock.MagicMock(auth_token=None, management_url=None) - fake_volume.manager.get.return_value = fake_volume - fake_volumes = mock.MagicMock(create=mock.Mock( - return_value=fake_volume)) + fake_volumes = mock.MagicMock( + create=mock.Mock(return_value=fake_volume), + get=mock.Mock(return_value=fake_volume)) @contextlib.contextmanager def fake_open(client, volume, mode): @@ -603,9 +606,9 @@ class TestCinderStoreBase(object): self.config(cinder_volume_type='some_type', group=backend) fake_client = mock.MagicMock(auth_token=None, management_url=None) - fake_volume.manager.get.return_value = fake_volume - fake_volumes = mock.MagicMock(create=mock.Mock( - return_value=fake_volume)) + fake_volumes = mock.MagicMock( + create=mock.Mock(return_value=fake_volume), + get=mock.Mock(return_value=fake_volume)) @contextlib.contextmanager def fake_open(client, volume, mode): @@ -684,9 +687,9 @@ class TestCinderStoreBase(object): self.store.volume_connector_map = fake_vol_connector_map fake_client = mock.MagicMock(auth_token=None, management_url=None) - fake_volume.manager.get.return_value = fake_volume - fake_volumes = mock.MagicMock(create=mock.Mock( - return_value=fake_volume)) + fake_volumes = mock.MagicMock( + create=mock.Mock(return_value=fake_volume), + get=mock.Mock(return_value=fake_volume)) @contextlib.contextmanager def fake_open(client, volume, mode): @@ -727,14 +730,18 @@ class TestCinderStoreBase(object): mock_cc_return_val, fake_volume, expected_volume_size // units.Gi) mock_wait.assert_has_calls( - [mock.call(fake_volume, 'creating', 'available'), - mock.call(fake_volume, 'extending', 'in-use')]) + [mock.call(fake_volume, 'creating', 'available', + mock_cc_return_val), + mock.call(fake_volume, 'extending', 'in-use', + mock_cc_return_val)]) else: fake_volume.extend.assert_called_once_with( fake_volume, expected_volume_size // units.Gi) mock_wait.assert_has_calls( - [mock.call(fake_volume, 'creating', 'available'), - mock.call(fake_volume, 'extending', 'available')]) + [mock.call(fake_volume, 'creating', 'available', + mock_cc_return_val), + mock.call(fake_volume, 'extending', 'available', + mock_cc_return_val)]) def test_cinder_add_extend_storage_full(self): @@ -757,9 +764,9 @@ class TestCinderStoreBase(object): verifier = None fake_client = mock.MagicMock() - fake_volume.manager.get.return_value = fake_volume - fake_volumes = mock.MagicMock(create=mock.Mock( - return_value=fake_volume)) + fake_volumes = mock.MagicMock( + create=mock.Mock(return_value=fake_volume), + get=mock.Mock(return_value=fake_volume)) with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc, \ mock.patch.object(self.store, '_open_cinder_volume'), \ @@ -797,9 +804,9 @@ class TestCinderStoreBase(object): delete=mock.MagicMock(side_effect=Exception())) fake_client = mock.MagicMock() - fake_volume.manager.get.return_value = fake_volume - fake_volumes = mock.MagicMock(create=mock.Mock( - return_value=fake_volume)) + fake_volumes = mock.MagicMock( + create=mock.Mock(return_value=fake_volume), + get=mock.Mock(return_value=fake_volume)) verifier = None with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc, \