From d0abb60fe7798c6fea4bfb9247db25cf15591340 Mon Sep 17 00:00:00 2001 From: Michael Dovgal Date: Tue, 20 Dec 2016 15:16:03 +0200 Subject: [PATCH] Attach/Delete volume for tgt driver race condition fix We have race condition when remove target operation still isn't completed and we try to run create target operation. This patch fixed this race condition by retrying create iscsi target operation run. Change-Id: I151990cc5148eb3f56e27e01fce71d87da2d9759 Closes-Bug: #1335889 --- cinder/tests/unit/targets/test_tgt_driver.py | 18 ++++++++++++++++++ cinder/volume/targets/tgt.py | 17 ++++++++++------- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/cinder/tests/unit/targets/test_tgt_driver.py b/cinder/tests/unit/targets/test_tgt_driver.py index 8c8fcdd03ee..21caab32470 100644 --- a/cinder/tests/unit/targets/test_tgt_driver.py +++ b/cinder/tests/unit/targets/test_tgt_driver.py @@ -77,6 +77,7 @@ class TestTgtAdmDriver(tf.TargetDriverFixture): ' ACL information:\n' ' ALL"\n' % {'test_vol': self.test_vol, 'bspath': self.testvol_path}) + self.patch('time.sleep') def fake_get(self, value, default): if value in ('iscsi_target_flags', 'iscsi_write_cache'): @@ -386,3 +387,20 @@ class TestTgtAdmDriver(tf.TargetDriverFixture): old_name=None, portals_ips=[self.configuration.iscsi_ip_address], portals_port=self.configuration.iscsi_port) + + @test.testtools.skipIf(sys.platform == "darwin", "SKIP on OSX") + def test_create_iscsi_target_retry(self): + with mock.patch('cinder.utils.execute', return_value=('', '')),\ + mock.patch.object(self.target, '_get_target', + side_effect=[None, None, 1]) as get_target,\ + mock.patch.object(self.target, '_verify_backing_lun', + side_effect=lambda x, y: True): + self.assertEqual( + 1, + self.target.create_iscsi_target( + self.test_vol, + 1, + 0, + self.fake_volumes_dir)) + # 3 - default retries count value for utils.retry + self.assertEqual(3, get_target.call_count) diff --git a/cinder/volume/targets/tgt.py b/cinder/volume/targets/tgt.py index 95014138056..570d769eaee 100644 --- a/cinder/volume/targets/tgt.py +++ b/cinder/volume/targets/tgt.py @@ -118,6 +118,7 @@ class TgtAdm(iscsi.ISCSITarget): LOG.debug("StdOut from tgt-admin --update: %s", out) LOG.debug("StdErr from tgt-admin --update: %s", err) + @utils.retry(exception.NotFound) def create_iscsi_target(self, name, tid, lun, path, chap_auth=None, **kwargs): @@ -160,8 +161,8 @@ class TgtAdm(iscsi.ISCSITarget): volume_path = os.path.join(volumes_dir, vol_id) if os.path.exists(volume_path): - LOG.warning(_LW('Persistence file already exists for volume, ' - 'found file at: %s'), volume_path) + LOG.debug(('Persistence file already exists for volume, ' + 'found file at: %s'), volume_path) utils.robust_file_write(volumes_dir, vol_id, volume_conf) LOG.debug(('Created volume path %(vp)s,\n' 'content: %(vc)s'), @@ -215,11 +216,13 @@ class TgtAdm(iscsi.ISCSITarget): iqn = '%s%s' % (self.iscsi_target_prefix, vol_id) tid = self._get_target(iqn) if tid is None: - LOG.error(_LE("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, }) + LOG.warning(_LW("Failed to create iscsi target for Volume " + "ID: %(vol_id)s. It could be caused by problem " + "with concurrency. " + "Also please ensure your tgtd config " + "file contains 'include %(volumes_dir)s/*'"), { + 'vol_id': vol_id, + 'volumes_dir': volumes_dir, }) raise exception.NotFound() # NOTE(jdg): Sometimes we have some issues with the backing lun