From 29020b47f775cf62cc549d2b3f53691dd4cc1f24 Mon Sep 17 00:00:00 2001 From: Ben Swartzlander Date: Thu, 25 Feb 2016 23:57:21 -0500 Subject: [PATCH] Change sudo to run_as_root in LVM driver The LVM driver was using sudo for one command instead of proper rootwrap usage. This caused failures when the user running manila-share didn't have passwordless sudo access. Depends-On: Ib1896997f2e7a505b5bf8ec0c9e5ee35942f79a6 Change-Id: I48ea946c2b8de2ebf0e067ef4829b81cabd1464f Closes-Bug: 1550121 --- etc/manila/rootwrap.d/share.filters | 3 ++- manila/share/drivers/lvm.py | 8 +++++--- manila/tests/share/drivers/test_lvm.py | 10 ++++------ 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/etc/manila/rootwrap.d/share.filters b/etc/manila/rootwrap.d/share.filters index 82255dae00..197cbcd710 100644 --- a/etc/manila/rootwrap.d/share.filters +++ b/etc/manila/rootwrap.d/share.filters @@ -47,7 +47,8 @@ lvextend: CommandFilter, lvextend, root # manila/share/drivers/lvm.py: 'lvcreate', '-L', %s, '-n', %s lvcreate: CommandFilter, lvcreate, root -# manila/share/drivers/lvm.py: 'vgs', %s, '--rows' +# manila/share/drivers/lvm.py: 'vgs', '--noheadings', '-o', 'name' +# manila/share/drivers/lvm.py: 'vgs', %s, '--rows', '--units', 'g' vgs: CommandFilter, vgs, root # manila/share/drivers/glusterfs.py: 'mkdir', '%s' diff --git a/manila/share/drivers/lvm.py b/manila/share/drivers/lvm.py index 5ca5028a75..c60d109484 100644 --- a/manila/share/drivers/lvm.py +++ b/manila/share/drivers/lvm.py @@ -67,7 +67,8 @@ CONF.register_opts(generic.share_opts) class LVMMixin(driver.ExecuteMixin): def check_for_setup_error(self): """Returns an error if prerequisites aren't met.""" - out, err = self._execute('sudo', 'vgs', '--noheadings', '-o', 'name') + out, err = self._execute('vgs', '--noheadings', '-o', 'name', + run_as_root=True) volume_groups = out.split() if self.configuration.lvm_share_volume_group not in volume_groups: msg = (_("share volume group %s doesn't exist") @@ -190,9 +191,10 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): super(LVMShareDriver, self)._update_share_stats(data) def get_share_server_pools(self, share_server=None): - out, err = self._execute('sudo', 'vgs', + out, err = self._execute('vgs', self.configuration.lvm_share_volume_group, - '--rows', '--units', 'g') + '--rows', '--units', 'g', + run_as_root=True) total_size = re.findall("VSize\s[0-9.]+g", out)[0][6:-1] free_size = re.findall("VFree\s[0-9.]+g", out)[0][6:-1] return [{ diff --git a/manila/tests/share/drivers/test_lvm.py b/manila/tests/share/drivers/test_lvm.py index 289fc4e914..5c51bfdd50 100644 --- a/manila/tests/share/drivers/test_lvm.py +++ b/manila/tests/share/drivers/test_lvm.py @@ -124,9 +124,7 @@ class LVMShareDriverTestCase(test.TestCase): def exec_runner(*ignore_args, **ignore_kwargs): return '\n fake1\n fakevg\n fake2\n', '' - expected_exec = [ - 'sudo vgs --noheadings -o name', - ] + expected_exec = ['vgs --noheadings -o name'] fake_utils.fake_execute_set_repliers([(expected_exec[0], exec_runner)]) self._driver.check_for_setup_error() self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) @@ -135,7 +133,7 @@ class LVMShareDriverTestCase(test.TestCase): def exec_runner(*ignore_args, **ignore_kwargs): return '\n fake0\n fake1\n fake2\n', '' - fake_utils.fake_execute_set_repliers([('sudo vgs --noheadings -o name', + fake_utils.fake_execute_set_repliers([('vgs --noheadings -o name', exec_runner)]) self.assertRaises(exception.InvalidParameterValue, self._driver.check_for_setup_error) @@ -144,7 +142,7 @@ class LVMShareDriverTestCase(test.TestCase): def exec_runner(*ignore_args, **ignore_kwargs): return '\n fake1\n fakevg\n fake2\n', '' - fake_utils.fake_execute_set_repliers([('sudo vgs --noheadings -o name', + fake_utils.fake_execute_set_repliers([('vgs --noheadings -o name', exec_runner)]) CONF.set_default('lvm_share_export_ip', None) self.assertRaises(exception.InvalidParameterValue, @@ -485,7 +483,7 @@ class LVMShareDriverTestCase(test.TestCase): self.assertEqual(expected_result, self._driver.get_share_server_pools()) self._driver._execute.assert_called_once_with( - 'sudo', 'vgs', 'fakevg', '--rows', '--units', 'g') + 'vgs', 'fakevg', '--rows', '--units', 'g', run_as_root=True) def test_copy_volume_error(self): def _fake_exec(*args, **kwargs):