diff options
author | anguoming <agm_daydayup@foxmail.com> | 2022-08-12 14:05:52 +0800 |
---|---|---|
committer | anguoming <agm_daydayup@foxmail.com> | 2023-02-16 10:24:35 +0800 |
commit | 2c3442a397c97e78f53b27e44b610752035b6f09 (patch) | |
tree | 5ed5ed5b2dc0346522e4b89ac0fb53064204c1ac | |
parent | 64e25979a904e468e1e89d8727e9554662596acf (diff) | |
download | glance_store-2c3442a397c97e78f53b27e44b610752035b6f09.tar.gz |
move attachment_update to try block
move attachment_update to try block, so when there is some error calling
attachment_update the finally block will be called, then the attachment created could be deleted.
Closes-bug: #1983238
Change-Id: I2dc3888a56d802424c6d62a656b2e5fef9dabb3e
-rw-r--r-- | glance_store/_drivers/cinder/store.py | 30 | ||||
-rw-r--r-- | glance_store/tests/unit/cinder/test_cinder_base.py | 19 |
2 files changed, 31 insertions, 18 deletions
diff --git a/glance_store/_drivers/cinder/store.py b/glance_store/_drivers/cinder/store.py index 17fb96b..95bce57 100644 --- a/glance_store/_drivers/cinder/store.py +++ b/glance_store/_drivers/cinder/store.py @@ -722,18 +722,20 @@ class Store(glance_store.driver.Store): mode=attach_mode) LOG.debug('Attachment %(attachment_id)s created successfully.', {'attachment_id': attachment['id']}) - attachment = self.volume_api.attachment_update( - client, attachment['id'], connector_prop, - mountpoint='glance_store') - LOG.debug('Attachment %(attachment_id)s updated successfully with ' - 'connection info %(conn_info)s', - {'attachment_id': attachment.id, - 'conn_info': strutils.mask_dict_password( - attachment.connection_info)}) - volume = volume.manager.get(volume_id) - connection_info = attachment.connection_info + volume = volume.manager.get(volume_id) + attachment_id = attachment['id'] + connection_info = None try: + attachment = self.volume_api.attachment_update( + client, attachment_id, connector_prop, + mountpoint='glance_store') + LOG.debug('Attachment %(attachment_id)s updated successfully with ' + 'connection info %(conn_info)s', + {'attachment_id': attachment_id, + 'conn_info': strutils.mask_dict_password( + attachment.connection_info)}) + connection_info = attachment.connection_info conn = base.factory( connection_info['driver_volume_type'], volume=volume, @@ -747,9 +749,9 @@ class Store(glance_store.driver.Store): # Complete the attachment (marking the volume "in-use") after # the connection with os-brick is complete - self.volume_api.attachment_complete(client, attachment.id) + self.volume_api.attachment_complete(client, attachment_id) LOG.debug('Attachment %(attachment_id)s completed successfully.', - {'attachment_id': attachment.id}) + {'attachment_id': attachment_id}) self.volume_connector_map[volume.id] = conn if (connection_info['driver_volume_type'] == 'rbd' and @@ -768,7 +770,7 @@ class Store(glance_store.driver.Store): try: if volume.multiattach: attachment_state_manager.detach( - client, attachment.id, volume_id, host, conn, + client, attachment_id, volume_id, host, conn, connection_info, device) else: conn.disconnect_volume(device) @@ -780,7 +782,7 @@ class Store(glance_store.driver.Store): {'volume_id': volume.id}) if not volume.multiattach: - self.volume_api.attachment_delete(client, attachment.id) + self.volume_api.attachment_delete(client, attachment_id) def _cinder_volume_data_iterator(self, client, volume, max_size, offset=0, chunk_size=None, partial_length=None): diff --git a/glance_store/tests/unit/cinder/test_cinder_base.py b/glance_store/tests/unit/cinder/test_cinder_base.py index acdb70b..922b161 100644 --- a/glance_store/tests/unit/cinder/test_cinder_base.py +++ b/glance_store/tests/unit/cinder/test_cinder_base.py @@ -207,7 +207,8 @@ class TestCinderStoreBase(object): multipath_supported=False, enforce_multipath=False, encrypted_nfs=False, qcow2_vol=False, - multiattach=False): + multiattach=False, + update_attachment_error=None): fake_volume = mock.MagicMock(id=str(uuid.uuid4()), status='available', multiattach=multiattach) fake_volume.manager.get.return_value = fake_volume @@ -284,6 +285,9 @@ class TestCinderStoreBase(object): 'getaddrinfo') as mock_get_host_ip, \ mock.patch.object(cinder.strutils, 'mask_dict_password'): + if update_attachment_error: + attach_update.side_effect = update_attachment_error + fake_host = 'fake_host' fake_addr_info = [[0, 1, 2, 3, ['127.0.0.1']]] fake_ip = fake_addr_info[0][4][0] @@ -309,9 +313,14 @@ class TestCinderStoreBase(object): except exceptions.BackendException: attach_delete.assert_called_once_with( fake_client, fake_attachment_id) + elif update_attachment_error: + self.assertRaises(type(update_attachment_error), do_open) else: do_open() - if not (encrypted_nfs or qcow2_vol): + if update_attachment_error: + attach_delete.assert_called_once_with( + fake_client, fake_attachment_id) + elif not (encrypted_nfs or qcow2_vol): mock_conn.assert_called_once_with( root_helper, fake_ip, multipath_supported, enforce_multipath, @@ -353,8 +362,10 @@ class TestCinderStoreBase(object): def test_open_cinder_volume_ro(self): self._test_open_cinder_volume('rb', 'ro', None) - def test_open_cinder_volume_error(self): - self._test_open_cinder_volume('wb', 'rw', IOError) + def test_open_cinder_volume_update_attachment_error(self): + err = Exception("update attachment fake error") + self._test_open_cinder_volume('rb', 'ro', None, + update_attachment_error=err) def test_open_cinder_volume_nfs_encrypted(self): self._test_open_cinder_volume('rb', 'ro', None, encrypted_nfs=True) |