Use cinder.utils.execute directly

When we were in the process of working on brick libs
we did quite a bit of funky stuff with setting a member
variable for each class to point to an executor that was
passed in during init.

There's no longer any reason to do this with the target drivers,
so we should simplify our lives a bit and go back to using the
good old cinder.utils wrapper.

It's also believed that the use of the member variable is
susceptible to some concurrency issue that was causing the
wrong cmd string to be executed.  We'll mark this as closing
that bug and reopen if we still see the signature in Kibana.

Change-Id: I7a5648d496d15eec5d7ac3643411198226f1ffdf
Closes-Bug: #1398078

diff --git a/cinder/tests/targets/test_base_iscsi_driver.py b/cinder/tests/targets/test_base_iscsi_driver.py
index 2608e5be318668aec274f1c73b5fa52f8441660c..1414ed5beb1d5779556c42f09f29686ea3e26d86 100644 (file)
--- a/cinder/tests/targets/test_base_iscsi_driver.py
+++ b/cinder/tests/targets/test_base_iscsi_driver.py
@@ -116,8 +116,8 @@ class TestBaseISCSITargetDriver(test.TestCase):
                        'safe_get',
                        _fake_safe_get)
 
-        self.stubs.Set(self.target,
-                       '_execute',
+        self.stubs.Set(utils,
+                       'execute',
                        _fake_execute)
 
         self.assertEqual(target_string,

diff --git a/cinder/tests/targets/test_tgt_driver.py b/cinder/tests/targets/test_tgt_driver.py
index 9682b489180aa080f67f05f129d22b807eb76119..72bee11abb7cd878befaa8504ed568ad0436a9cb 100644 (file)
--- a/cinder/tests/targets/test_tgt_driver.py
+++ b/cinder/tests/targets/test_tgt_driver.py
@@ -115,8 +115,8 @@ class TestTgtAdmDriver(test.TestCase):
         def _fake_execute(*args, **kwargs):
             return self.fake_iscsi_scan, None
 
-        self.stubs.Set(self.target,
-                       '_execute',
+        self.stubs.Set(utils,
+                       'execute',
                        _fake_execute)
 
         self.assertEqual('1',
@@ -129,8 +129,8 @@ class TestTgtAdmDriver(test.TestCase):
         def _fake_execute(*args, **kwargs):
             return self.fake_iscsi_scan, None
 
-        self.stubs.Set(self.target,
-                       '_execute',
+        self.stubs.Set(utils,
+                       'execute',
                        _fake_execute)
 
         self.assertTrue(self.target._verify_backing_lun(
@@ -144,8 +144,8 @@ class TestTgtAdmDriver(test.TestCase):
         def _fake_execute_bad_lun(*args, **kwargs):
             return bad_scan, None
 
-        self.stubs.Set(self.target,
-                       '_execute',
+        self.stubs.Set(utils,
+                       'execute',
                        _fake_execute_bad_lun)
 
         self.assertFalse(self.target._verify_backing_lun(
@@ -176,8 +176,8 @@ class TestTgtAdmDriver(test.TestCase):
         def _fake_execute(*args, **kwargs):
             return '', ''
 
-        self.stubs.Set(self.target,
-                       '_execute',
+        self.stubs.Set(utils,
+                       'execute',
                        _fake_execute)
 
         self.stubs.Set(self.target,
@@ -206,8 +206,8 @@ class TestTgtAdmDriver(test.TestCase):
                 stderr='target already exists',
                 cmd='tgtad --lld iscsi --op show --mode target')
 
-        self.stubs.Set(self.target,
-                       '_execute',
+        self.stubs.Set(utils,
+                       'execute',
                        _fake_execute)
 
         self.stubs.Set(self.target,
@@ -233,8 +233,8 @@ class TestTgtAdmDriver(test.TestCase):
         def _fake_execute(*args, **kwargs):
             return '', ''
 
-        self.stubs.Set(self.target,
-                       '_execute',
+        self.stubs.Set(utils,
+                       'execute',
                        _fake_execute)
 
         self.stubs.Set(self.target,

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index c5dc25aeba782421a2093d6c4a6de3612cebc1dc..54e99cde2226d84320a60c7fec119fd9fe15e05f 100644 (file)
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -4027,7 +4027,8 @@ class ISCSITestCase(DriverTestCase):
         iscsi_driver = \
             cinder.volume.targets.tgt.TgtAdm(
                 configuration=self.configuration)
-        iscsi_driver._execute = lambda *a, **kw: \
+
+        utils.execute = lambda *a, **kw: \
             ("%s dummy" % CONF.iscsi_ip_address, '')
         volume = {"name": "dummy",
                   "host": "0.0.0.0",

diff --git a/cinder/volume/targets/driver.py b/cinder/volume/targets/driver.py
index bfa636d55cfdbdb6468015dacf3a483175f576c4..9fabfd8dfd751cba8339d5691277a677b86a57eb 100644 (file)
--- a/cinder/volume/targets/driver.py
+++ b/cinder/volume/targets/driver.py
@@ -12,9 +12,11 @@
 
 import abc
 
-from oslo_concurrency import processutils as putils
+from oslo.config import cfg
 import six
 
+CONF = cfg.CONF
+
 
 @six.add_metaclass(abc.ABCMeta)
 class Target(object):
@@ -33,8 +35,9 @@ class Target(object):
     def __init__(self, *args, **kwargs):
         self.db = kwargs.get('db')
         self.configuration = kwargs.get('configuration')
-        self._execute = kwargs.get('executor', putils.execute)
-        self._root_helper = kwargs.get('root_helper')
+        self._root_helper = kwargs.get('root_helper',
+                                       'sudo cinder-rootwrap %s' %
+                                       CONF.rootwrap_config)
 
     @abc.abstractmethod
     def ensure_export(self, context, volume,

diff --git a/cinder/volume/targets/iscsi.py b/cinder/volume/targets/iscsi.py
index 0ae0a4512b995b4f2abc908cdf13108c83dd1bd0..f8eae6ddd89fb2bddff3213996885fe9fa81e6a0 100644 (file)
--- a/cinder/volume/targets/iscsi.py
+++ b/cinder/volume/targets/iscsi.py
@@ -15,6 +15,7 @@ from oslo.concurrency import processutils
 from cinder import exception
 from cinder.i18n import _, _LW, _LE
 from cinder.openstack.common import log as logging
+from cinder import utils
 from cinder.volume.targets import driver
 
 LOG = logging.getLogger(__name__)
@@ -135,10 +136,9 @@ class ISCSITarget(driver.Target):
             # NOTE(griff) We're doing the split straight away which should be
             # safe since using '@' in hostname is considered invalid
 
-            (out, _err) = self._execute('iscsiadm', '-m', 'discovery',
+            (out, _err) = utils.execute('iscsiadm', '-m', 'discovery',
                                         '-t', 'sendtargets', '-p',
                                         volume['host'].split('@')[0],
-                                        root_helper=self._root_helper,
                                         run_as_root=True)
         except processutils.ProcessExecutionError as ex:
             LOG.error(_LE("ISCSI discovery attempt failed for:%s") %

diff --git a/cinder/volume/targets/lio.py b/cinder/volume/targets/lio.py
index 3f38dacd2129bccb6b895d42ee396d6b732b5d8b..4c66e2bcb2e0f71c0bcad1092cd0cfdbec046e0d 100644 (file)
--- a/cinder/volume/targets/lio.py
+++ b/cinder/volume/targets/lio.py
@@ -15,6 +15,7 @@ from oslo_concurrency import processutils as putils
 from cinder import exception
 from cinder.i18n import _, _LE, _LI, _LW
 from cinder.openstack.common import log as logging
+from cinder import utils
 from cinder.volume.targets.tgt import TgtAdm
 
 LOG = logging.getLogger(__name__)
@@ -71,13 +72,13 @@ class LioAdm(TgtAdm):
 
     def _verify_rtstool(self):
         try:
-            self._execute('cinder-rtstool', 'verify')
+            utils.execute('cinder-rtstool', 'verify')
         except (OSError, putils.ProcessExecutionError):
             LOG.error(_LE('cinder-rtstool is not installed correctly'))
             raise
 
     def _get_target(self, iqn):
-        (out, err) = self._execute('cinder-rtstool',
+        (out, err) = utils.execute('cinder-rtstool',
                                    'get-targets',
                                    run_as_root=True)
         lines = out.split('\n')
@@ -105,7 +106,7 @@ class LioAdm(TgtAdm):
                             name,
                             chap_auth_userid,
                             chap_auth_password]
-            self._execute(*command_args, run_as_root=True)
+            utils.execute(*command_args, run_as_root=True)
         except putils.ProcessExecutionError as e:
             LOG.error(_LE("Failed to create iscsi target for volume "
                           "id:%s.") % vol_id)
@@ -128,7 +129,7 @@ class LioAdm(TgtAdm):
         iqn = '%s%s' % (self.iscsi_target_prefix, vol_uuid_name)
 
         try:
-            self._execute('cinder-rtstool',
+            utils.execute('cinder-rtstool',
                           'delete',
                           iqn,
                           run_as_root=True)
@@ -155,7 +156,7 @@ class LioAdm(TgtAdm):
 
         # Add initiator iqns to target ACL
         try:
-            self._execute('cinder-rtstool', 'add-initiator',
+            utils.execute('cinder-rtstool', 'add-initiator',
                           volume_iqn,
                           auth_user,
                           auth_pass,
@@ -183,7 +184,7 @@ class LioAdm(TgtAdm):
 
         # Delete initiator iqns from target ACL
         try:
-            self._execute('cinder-rtstool', 'delete-initiator',
+            utils.execute('cinder-rtstool', 'delete-initiator',
                           volume_iqn,
                           connector['initiator'],
                           run_as_root=True)

diff --git a/cinder/volume/targets/tgt.py b/cinder/volume/targets/tgt.py
index f5cf3b8466312e1b3483b2eb806d1728add1408f..02ff1d385061ba0fca8fe287af69021c3447f9b2 100644 (file)
--- a/cinder/volume/targets/tgt.py
+++ b/cinder/volume/targets/tgt.py
@@ -22,6 +22,7 @@ from cinder import exception
 from cinder.openstack.common import fileutils
 from cinder.i18n import _, _LI, _LW, _LE
 from cinder.openstack.common import log as logging
+from cinder import utils
 from cinder.volume.targets import iscsi
 from cinder.volume import utils as vutils
 
@@ -58,7 +59,7 @@ class TgtAdm(iscsi.ISCSITarget):
         self.volumes_dir = self.configuration.safe_get('volumes_dir')
 
     def _get_target(self, iqn):
-        (out, err) = self._execute('tgt-admin', '--show', run_as_root=True)
+        (out, err) = utils.execute('tgt-admin', '--show', run_as_root=True)
         lines = out.split('\n')
         for line in lines:
             if iqn in line:
@@ -73,7 +74,7 @@ class TgtAdm(iscsi.ISCSITarget):
         capture = False
         target_info = []
 
-        (out, err) = self._execute('tgt-admin', '--show', run_as_root=True)
+        (out, err) = utils.execute('tgt-admin', '--show', run_as_root=True)
         lines = out.split('\n')
 
         for line in lines:
@@ -99,7 +100,7 @@ class TgtAdm(iscsi.ISCSITarget):
         # and error on the side of caution
         time.sleep(10)
         try:
-            (out, err) = self._execute('tgtadm', '--lld', 'iscsi',
+            (out, err) = utils.execute('tgtadm', '--lld', 'iscsi',
                                        '--op', 'new', '--mode',
                                        'logicalunit', '--tid',
                                        tid, '--lun', '1', '-b',
@@ -214,7 +215,7 @@ class TgtAdm(iscsi.ISCSITarget):
             # by creating the entry in the persist file
             # and then doing an update to get the target
             # created.
-            (out, err) = self._execute('tgt-admin', '--update', 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)
@@ -222,7 +223,7 @@ class TgtAdm(iscsi.ISCSITarget):
             # Grab targets list for debug
             # Consider adding a check for lun 0 and 1 for tgtadm
             # before considering this as valid
-            (out, err) = self._execute('tgtadm',
+            (out, err) = utils.execute('tgtadm',
                                        '--lld',
                                        'iscsi',
                                        '--op',
@@ -357,7 +358,7 @@ class TgtAdm(iscsi.ISCSITarget):
         try:
             # NOTE(vish): --force is a workaround for bug:
             #             https://bugs.launchpad.net/cinder/+bug/1159948
-            self._execute('tgt-admin',
+            utils.execute('tgt-admin',
                           '--force',
                           '--delete',
                           iqn,
@@ -381,7 +382,7 @@ class TgtAdm(iscsi.ISCSITarget):
             try:
                 LOG.warning(_LW('Silent failure of target removal '
                                 'detected, retry....'))
-                self._execute('tgt-admin',
+                utils.execute('tgt-admin',
                               '--delete',
                               iqn,
                               run_as_root=True)