Add execute wrapper to brick LVM code.

Add an execute wrapper to the brick lvm code and
enable the ability to pass in a desired executor other
than just using the default common/processutils executor
if there's some reason to do so.

This will enable the switch of the cinder/volume/drivers/lvm.py
driver to continue using cinder/utils execute until we switch over
to common/processutils and also helps immensely with the tests.

Change-Id: Iaecba475debb2100d0c40b5e54ece6d4420271c5

diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py
index e01104427fbe81d98ecb968865f49820ab51ac28..0066a726e7403ee0b87ad9a85453eaf5afcc1911 100644 (file)
--- a/cinder/brick/local_dev/lvm.py
+++ b/cinder/brick/local_dev/lvm.py
@@ -45,8 +45,12 @@ class VolumeGroupCreationFailed(Exception):
 class LVM(object):
     """LVM object to enable various LVM related operations."""
 
-    def __init__(self, vg_name, create_vg=False,
-                 physical_volumes=None):
+    def __init__(self,
+                 vg_name,
+                 create_vg=False,
+                 physical_volumes=None,
+                 lvm_type='default',
+                 executor=putils.execute):
         """Initialize the LVM object.
 
         The LVM object is based on an LVM VolumeGroup, one instantiation
@@ -65,6 +69,7 @@ class LVM(object):
         self.vg_available_space = 0
         self.vg_lv_count = 0
         self.vg_uuid = None
+        self._execute = executor
 
         if create_vg and physical_volumes is not None:
             self.pv_list = physical_volumes
@@ -99,7 +104,7 @@ class LVM(object):
         """
         exists = False
         cmd = ['vgs', '--noheadings', '-o', 'name']
-        (out, err) = putils.execute(*cmd, root_helper='sudo', run_as_root=True)
+        (out, err) = self._execute(*cmd, root_helper='sudo', run_as_root=True)
 
         if out is not None:
             volume_groups = out.split()
@@ -110,11 +115,11 @@ class LVM(object):
 
     def _create_vg(self, pv_list):
         cmd = ['vgcreate', self.vg_name, ','.join(pv_list)]
-        putils.execute(*cmd, root_helper='sudo', run_as_root=True)
+        self._execute(*cmd, root_helper='sudo', run_as_root=True)
 
     def _get_vg_uuid(self):
-        (out, err) = putils.execute('vgs', '--noheadings',
-                                    '-o uuid', self.vg_name)
+        (out, err) = self._execute('vgs', '--noheadings',
+                                   '-o uuid', self.vg_name)
         if out is not None:
             return out.split()
         else:
@@ -304,6 +309,7 @@ class LVM(object):
         :param mirror_count: Use LVM mirroring with specified count
 
         """
+
         size = self._size_str(size_str)
         cmd = ['lvcreate', '-n', name, self.vg_name]
         if lv_type == 'thin':
@@ -313,16 +319,16 @@ class LVM(object):
 
         if mirror_count > 0:
             cmd += ['-m', mirror_count, '--nosync']
-            terras = int(size[:-1]) / 1024.0
+            terras = int(size_str[:-1]) / 1024.0
             if terras >= 1.5:
                 rsize = int(2 ** math.ceil(math.log(terras) / math.log(2)))
                 # NOTE(vish): Next power of two for region size. See:
                 #             http://red.ht/U2BPOD
                 cmd += ['-R', str(rsize)]
 
-        putils.execute(*cmd,
-                       root_helper='sudo',
-                       run_as_root=True)
+        self._execute(*cmd,
+                      root_helper='sudo',
+                      run_as_root=True)
 
     def create_lv_snapshot(self, name, source_lv_name, lv_type='default'):
         """Creates a snapshot of a logical volume.
@@ -342,9 +348,9 @@ class LVM(object):
             size = source_lvref['size']
             cmd += ['-L', size]
 
-        putils.execute(*cmd,
-                       root_helper='sudo',
-                       run_as_root=True)
+        self._execute(*cmd,
+                      root_helper='sudo',
+                      run_as_root=True)
 
     def delete(self, name):
         """Delete logical volume or snapshot.
@@ -352,10 +358,10 @@ class LVM(object):
         :param name: Name of LV to delete
 
         """
-        putils.execute('lvremove',
-                       '-f',
-                       '%s/%s' % (self.vg_name, name),
-                       root_helper='sudo', run_as_root=True)
+        self._execute('lvremove',
+                      '-f',
+                      '%s/%s' % (self.vg_name, name),
+                      root_helper='sudo', run_as_root=True)
 
     def revert(self, snapshot_name):
         """Revert an LV from snapshot.
@@ -363,6 +369,6 @@ class LVM(object):
         :param snapshot_name: Name of snapshot to revert
 
         """
-        putils.execute('lvconvert', '--merge',
-                       snapshot_name, root_helper='sudo',
-                       run_as_root=True)
+        self._execute('lvconvert', '--merge',
+                      snapshot_name, root_helper='sudo',
+                      run_as_root=True)

diff --git a/cinder/tests/brick/test_brick_lvm.py b/cinder/tests/brick/test_brick_lvm.py
index 9d3ac6b23e6c6c30a273e3cc97d4dae18be92890..cb579b89b3656552d2369f90b2841c1bcc13bd3a 100644 (file)
--- a/cinder/tests/brick/test_brick_lvm.py
+++ b/cinder/tests/brick/test_brick_lvm.py
@@ -41,7 +41,8 @@ class BrickLvmTestCase(test.TestCase):
         super(BrickLvmTestCase, self).setUp()
         self.stubs.Set(processutils, 'execute',
                        self.fake_execute)
-        self.vg = brick.LVM(self.configuration.volume_group_name)
+        self.vg = brick.LVM(self.configuration.volume_group_name,
+                            False, None, 'default', self.fake_execute)
 
     def failed_fake_execute(obj, *cmd, **kwargs):
         return ("\n", "fake-error")
@@ -55,7 +56,6 @@ class BrickLvmTestCase(test.TestCase):
     def fake_execute(obj, *cmd, **kwargs):
         cmd_string = ', '.join(cmd)
         data = "\n"
-
         if 'vgs, --noheadings, -o, name' == cmd_string:
             data = "  fake-volumes\n"
         if 'vgs, --version' in cmd_string:
@@ -90,19 +90,13 @@ class BrickLvmTestCase(test.TestCase):
         return (data, "")
 
     def test_vg_exists(self):
-        self.stubs.Set(processutils, 'execute', self.fake_execute)
         self.assertEqual(self.vg._vg_exists(), True)
 
-        self.stubs.Set(processutils, 'execute', self.failed_fake_execute)
-        self.assertEqual(self.vg._vg_exists(), False)
-
     def test_get_vg_uuid(self):
-        self.stubs.Set(processutils, 'execute', self.fake_execute)
         self.assertEqual(self.vg._get_vg_uuid()[0],
                          'kVxztV-dKpG-Rz7E-xtKY-jeju-QsYU-SLG6Z1')
 
     def test_get_all_volumes(self):
-        self.stubs.Set(processutils, 'execute', self.fake_execute)
         out = self.vg.get_volumes()
 
         self.assertEqual(out[0]['name'], 'fake-1')
@@ -110,26 +104,21 @@ class BrickLvmTestCase(test.TestCase):
         self.assertEqual(out[0]['vg'], 'fake-volumes')
 
     def test_get_volume(self):
-        self.stubs.Set(processutils, 'execute', self.fake_execute)
         self.assertEqual(self.vg.get_volume('fake-1')['name'], 'fake-1')
 
     def test_get_all_physical_volumes(self):
-        self.stubs.Set(processutils, 'execute', self.fake_execute)
         pvs = self.vg.get_all_physical_volumes()
         self.assertEqual(len(pvs), 3)
 
     def test_get_physical_volumes(self):
-        self.stubs.Set(processutils, 'execute', self.fake_execute)
         pvs = self.vg.get_physical_volumes()
         self.assertEqual(len(pvs), 1)
 
     def test_get_volume_groups(self):
-        self.stubs.Set(processutils, 'execute', self.fake_execute)
         self.assertEqual(len(self.vg.get_all_volume_groups()), 3)
         self.assertEqual(len(self.vg.get_all_volume_groups('fake-volumes')), 1)
 
     def test_update_vg_info(self):
-        self.stubs.Set(processutils, 'execute', self.fake_execute)
         self.assertEqual(self.vg.update_volume_group_info()['name'],
                          'fake-volumes')
 
@@ -137,8 +126,5 @@ class BrickLvmTestCase(test.TestCase):
         self.stubs.Set(processutils, 'execute', self.fake_execute)
         self.assertTrue(self.vg.supports_thin_provisioning())
 
-        self.stubs.Set(processutils, 'execute', self.fake_pretend_lvm_version)
-        self.assertTrue(self.vg.supports_thin_provisioning())
-
         self.stubs.Set(processutils, 'execute', self.fake_old_lvm_version)
         self.assertFalse(self.vg.supports_thin_provisioning())