Merge "move attachment_update to try block"

2 files changed, 31 insertions, 18 deletions

diff --git a/glance_store/_drivers/cinder/store.py b/glance_store/_drivers/cinder/store.py
index b587c7e..bfcadc6 100644
--- a/glance_store/_drivers/cinder/store.py
+++ b/glance_store/_drivers/cinder/store.py
@@ -728,18 +728,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,
@@ -753,9 +755,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
@@ -774,7 +776,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)
@@ -786,7 +788,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)