Fixup persistence file not found on tgt remove

After commit: beecd769af02ba5915be827d28a7b46d970e41b0
we're experiencing a race in the tempest gating tests.

It appears that there are certain cases where a detach
and a delete call can overlap causing a race in the
remove_iscsi_target method.  The result is that when
the slower method gets around to the final removal of
the persistence file it's no longer there, so we
hit an unhandled exception.

This patch does two things:
1. It adds the volume lock to the detach_volume
method in the manager (already in place for delete).
This should mitigate the race and allow us to avoid it.

2. In the iscsi_remove_target method check if the
persist file has already been removed when we get
to the last check.  The fact is that if it was somehow
removed before we got here we don't really care, we're
just going to delete it anyway.

Change-Id: Ied45e1a66c8fb79c95ada66f8c59bd2839e200ad
Closes-Bug: 1277362

diff --git a/cinder/brick/iscsi/iscsi.py b/cinder/brick/iscsi/iscsi.py
index 236e4d0a8fd4450fc8bb95efe5237cf0141808b0..0858979eee01f522d685bae8fb748fd2a1aef033 100644 (file)
--- a/cinder/brick/iscsi/iscsi.py
+++ b/cinder/brick/iscsi/iscsi.py
@@ -223,9 +223,8 @@ class TgtAdm(TargetAdmin):
             LOG.error(_("Failed to create iscsi target for volume "
                         "id:%(vol_id)s. Please ensure your tgtd config file "
                         "contains 'include %(volumes_dir)s/*'") % {
-                            'vol_id': vol_id,
-                            'volumes_dir': volumes_dir,
-                        })
+                      'vol_id': vol_id,
+                      'volumes_dir': volumes_dir, })
             raise exception.NotFound()
 
         # NOTE(jdg): Sometimes we have some issues with the backing lun
@@ -278,7 +277,15 @@ class TgtAdm(TargetAdmin):
                       % {'vol_id': vol_id, 'e': str(e)})
             raise exception.ISCSITargetRemoveFailed(volume_id=vol_id)
 
-        os.unlink(volume_path)
+        # NOTE(jdg): This *should* be there still but incase
+        # it's not we don't care, so just ignore it if was
+        # somehow deleted between entry of this method
+        # and here
+        if os.path.exists(volume_path):
+            os.unlink(volume_path)
+        else:
+            LOG.debug('Volume path %s not found at end, '
+                      'of remove_iscsi_target.' % volume_path)
 
     def show_target(self, tid, iqn=None, **kwargs):
         if iqn is None:

diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index f10f855fb4af589bb56143615576c06ca180c156..0e893e45b37c0798437bbc2c12c011fec22250dd 100644 (file)
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -637,6 +637,7 @@ class VolumeManager(manager.SchedulerDependentManager):
             self._notify_about_volume_usage(context, volume, "attach.end")
         return do_attach()
 
+    @locked_volume_operation
     def detach_volume(self, context, volume_id):
         """Updates db to show volume is detached."""
         # TODO(vish): refactor this into a more general "unreserve"