From: John Griffith Date: Tue, 2 Jul 2013 19:16:28 +0000 (-0600) Subject: Add execute wrapper to brick LVM code. X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=62fb1ac44a384016e81223de179f79cede6f8fad;p=openstack-build%2Fcinder-build.git 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 e01104427..0066a726e 100644 --- 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 9d3ac6b23..cb579b89b 100644 --- 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())