From: John Griffith <john.griffith@solidfire.com>
Date: Thu, 15 Jan 2015 15:56:28 +0000 (-0700)
Subject: Add retry for tgtadm update when tgt exists
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=4dc367f7d736ae506db9532270e92977c8b3954d;p=openstack-build%2Fcinder-build.git

Add retry for tgtadm update when tgt exists

For target creation using tgtadm driver, we create a persistence
file for the target, then send a tgt-admin --update 'name' where
name is the specific persistence file we want to read in and update
from.

It turns out that we can hit race conditions where the persistence
file is written, and an update is requested but target has already
done work to make it believe that the target has already been created.

One thought was to just use "update ALL" but this still seems to have
issues, and changes the error to an account exists failure.

This patch takes the brute force approach and adds the cinder.utils
retry decorator to the tgt-admin --update command.  To do this
we just break out the tgt-admin --update call into it's own method
and add the decorator to it.

Closes-Bug: #1398078

Change-Id: Ic72fb5f4fca70a7e7d1c306efe19674fd7e42cd6
---

diff --git a/cinder/volume/targets/tgt.py b/cinder/volume/targets/tgt.py
index 0704a8d52..4cae868e8 100644
--- a/cinder/volume/targets/tgt.py
+++ b/cinder/volume/targets/tgt.py
@@ -147,6 +147,13 @@ class TgtAdm(iscsi.ISCSITarget):
         LOG.debug('Failed to find CHAP auth from config for %s' % vol_id)
         return None
 
+    @utils.retry(putils.ProcessExecutionError)
+    def _do_tgt_update(self, name):
+            (out, err) = utils.execute('tgt-admin', '--update', name,
+                                       run_as_root=True)
+            LOG.debug("StdOut from tgt-admin --update: %s", out)
+            LOG.debug("StdErr from tgt-admin --update: %s", err)
+
     def ensure_export(self, context, volume, volume_path):
         chap_auth = None
         old_name = None
@@ -222,10 +229,8 @@ class TgtAdm(iscsi.ISCSITarget):
             # by creating the entry in the persist file
             # and then doing an update to get the target
             # created.
-            (out, err) = utils.execute('tgt-admin', '--update', name,
-                                       run_as_root=True)
-            LOG.debug("StdOut from tgt-admin --update: %s", out)
-            LOG.debug("StdErr from tgt-admin --update: %s", err)
+
+            self._do_tgt_update(name)
         except putils.ProcessExecutionError as e:
             if "target already exists" in e.stderr:
                 # Adding the additional Warning message below for a clear
@@ -233,15 +238,14 @@ class TgtAdm(iscsi.ISCSITarget):
                 LOG.warning(_LW('Could not create target because '
                                 'it already exists for volume: %s'), vol_id)
                 LOG.debug('Exception was: %s', e)
-                pass
-            else:
-                LOG.error(_LE("Failed to create iscsi target for volume "
-                              "id:%(vol_id)s: %(e)s"),
-                          {'vol_id': vol_id, 'e': e})
 
-                # Don't forget to remove the persistent file we created
-                os.unlink(volume_path)
-                raise exception.ISCSITargetCreateFailed(volume_id=vol_id)
+            LOG.error(_LE("Failed to create iscsi target for volume "
+                          "id:%(vol_id)s: %(e)s"),
+                      {'vol_id': vol_id, 'e': e})
+
+            # Don't forget to remove the persistent file we created
+            os.unlink(volume_path)
+            raise exception.ISCSITargetCreateFailed(volume_id=vol_id)
 
         # Grab targets list for debug
         # Consider adding a check for lun 0 and 1 for tgtadm