diff --git a/requirements.txt b/requirements.txt index 606865123..e16d935cd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -20,6 +20,7 @@ oslo.config>=5.1.0 # Apache-2.0 oslo.messaging>=5.29.0 # Apache-2.0 oslo.middleware>=3.31.0 # Apache-2.0 oslo.policy>=1.30.0 # Apache-2.0 +oslo.privsep>=1.23.0 # Apache-2.0 oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 oslo.service!=1.28.1,>=1.24.0 # Apache-2.0 oslo.versionedobjects>=1.28.0 # Apache-2.0 diff --git a/zun/common/exception.py b/zun/common/exception.py index b28818f95..c68c7248e 100644 --- a/zun/common/exception.py +++ b/zun/common/exception.py @@ -494,7 +494,7 @@ class EntityNotFound(ZunException): class CommandError(ZunException): - message = _("The command: %(cmd)s failed on the system.") + message = _("The command: %(cmd)s failed on the system, due to %(error)s") class NoValidHost(ZunException): diff --git a/zun/common/mount.py b/zun/common/mount.py index 59c5f2af7..2fe4757de 100644 --- a/zun/common/mount.py +++ b/zun/common/mount.py @@ -12,7 +12,6 @@ import os -from oslo_concurrency import processutils from oslo_log import log as logging from oslo_utils import excutils @@ -44,7 +43,7 @@ class Mounter(object): try: utils.execute('mkfs', '-t', fstype, '-F', devpath, run_as_root=True) - except processutils.ProcessExecutionError as e: + except exception.CommandError as e: raise exception.MakeFileSystemException(_( "Unexpected error while make filesystem. " "Devpath: %(devpath)s, " @@ -56,7 +55,7 @@ class Mounter(object): try: utils.execute('mount', '-t', fstype, devpath, mountpoint, run_as_root=True) - except processutils.ProcessExecutionError as e: + except exception.CommandError as e: raise exception.MountException(_( "Unexpected error while mount block device. " "Devpath: %(devpath)s, " @@ -67,7 +66,7 @@ class Mounter(object): def unmount(self, mountpoint): try: utils.execute('umount', mountpoint, run_as_root=True) - except processutils.ProcessExecutionError as e: + except exception.CommandError as e: raise exception.UnmountException(_( "Unexpected err while unmount block device. " "Mountpoint: %(mountpoint)s, " @@ -90,9 +89,9 @@ class Mounter(object): filter_fstype = () try: - (out, err) = processutils.execute('cat', PROC_MOUNTS_PATH, - check_exit_code=0) - except processutils.ProcessExecutionError: + (out, err) = utils.execute('cat', PROC_MOUNTS_PATH, + check_exit_code=0) + except exception.CommandError: msg = _("Failed to read mounts.") raise exception.FileNotFound(msg) diff --git a/zun/common/privileged.py b/zun/common/privileged.py new file mode 100644 index 000000000..e2c75978b --- /dev/null +++ b/zun/common/privileged.py @@ -0,0 +1,22 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_privsep import capabilities as c +from oslo_privsep import priv_context + + +default = priv_context.PrivContext( + 'zun.common', + cfg_section='privsep', + pypath=__name__ + '.default', + capabilities=[c.CAP_SYS_ADMIN], +) diff --git a/zun/common/utils.py b/zun/common/utils.py index ca35351aa..f8cf4ca36 100644 --- a/zun/common/utils.py +++ b/zun/common/utils.py @@ -35,6 +35,7 @@ from zun.common import clients from zun.common import consts from zun.common import exception from zun.common.i18n import _ +from zun.common import privileged import zun.conf CONF = zun.conf.CONF @@ -305,16 +306,34 @@ def get_security_group_ids(context, security_groups, **kwargs): security_groups) -def get_root_helper(): - # TODO(hongbin): Use rootwrap instead - return 'sudo' +def custom_execute(*cmd, **kwargs): + try: + return processutils.execute(*cmd, **kwargs) + except processutils.ProcessExecutionError as e: + sanitized_cmd = strutils.mask_password(' '.join(cmd)) + raise exception.CommandError(cmd=sanitized_cmd, + error=six.text_type(e)) + + +@privileged.default.entrypoint +def execute_root(*cmd, **kwargs): + # NOTE(kiennt): Set run_as_root=False because if it is set to True, the + # command is prefixed by the command specified in the + # root_helper kwargs [1]. But we use oslo.privsep instead + # of rootwrap so set run_as_root=False. + # [1] https://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/processutils.py#L218 # noqa + return custom_execute(*cmd, shell=False, run_as_root=False, **kwargs) def execute(*cmd, **kwargs): - if 'run_as_root' in kwargs and 'root_helper' not in kwargs: - kwargs['root_helper'] = get_root_helper() - - return processutils.execute(*cmd, **kwargs) + run_as_root = kwargs.pop('run_as_root', False) + # NOTE(kiennt): Root_helper is unnecessary when use privsep, + # therefore pop it! + kwargs.pop('root_helper', None) + if run_as_root: + return execute_root(*cmd, **kwargs) + else: + return custom_execute(*cmd, **kwargs) def check_capsule_template(tpl): diff --git a/zun/container/os_capability/host_capability.py b/zun/container/os_capability/host_capability.py index 59335804c..04789eefe 100644 --- a/zun/container/os_capability/host_capability.py +++ b/zun/container/os_capability/host_capability.py @@ -77,7 +77,7 @@ class Host(object): def get_pci_resources(self): addresses = [] try: - output, status = processutils.execute('lspci', '-D', '-nnmm') + output, status = utils.execute('lspci', '-D', '-nnmm') lines = output.split('\n') for line in lines: if not line: @@ -105,27 +105,21 @@ class Host(object): Function (VF). Only normal PCI devices or SR-IOV VFs are assignable. """ - try: - path = '/sys/bus/pci/devices/' + address + '/' - output, status = processutils.execute('ls', path) - if "physfn" in output: - phys_address = None - upath = '/sys/bus/pci/devices/%s/physfn/uevent' % address - try: - ou, st = processutils.execute('cat', upath) - lines = ou.split('\n') - for line in lines: - if 'PCI_SLOT_NAME' in line: - columns = line.split("=") - phys_address = columns[1] - except processutils.ProcessExecutionError: - raise exception.CommandError(cmd='cat') - return {'dev_type': fields.PciDeviceType.SRIOV_VF, - 'parent_addr': phys_address} - if "virtfn" in output: - return {'dev_type': fields.PciDeviceType.SRIOV_PF} - except processutils.ProcessExecutionError: - raise exception.CommandError(cmd='ls') + path = '/sys/bus/pci/devices/' + address + '/' + output, status = utils.execute('ls', path) + if "physfn" in output: + phys_address = None + upath = '/sys/bus/pci/devices/%s/physfn/uevent' % address + ou, st = utils.execute('cat', upath) + lines = ou.split('\n') + for line in lines: + if 'PCI_SLOT_NAME' in line: + columns = line.split("=") + phys_address = columns[1] + return {'dev_type': fields.PciDeviceType.SRIOV_VF, + 'parent_addr': phys_address} + if "virtfn" in output: + return {'dev_type': fields.PciDeviceType.SRIOV_PF} return {'dev_type': fields.PciDeviceType.STANDARD} def _get_device_capabilities(device, address): @@ -143,26 +137,18 @@ class Host(object): return {} def _get_product_and_vendor(address): - try: - output, status = processutils.execute('lspci', '-n', '-s', - address) - value = output.split()[2] - result = value.split(":") - return result[0], result[1] - except processutils.ProcessExecutionError: - raise exception.CommandError(cmd='lspci') + output, status = utils.execute('lspci', '-n', '-s', address) + value = output.split()[2] + result = value.split(":") + return result[0], result[1] def _get_numa_node(address): numa_node = None - try: - output, status = processutils.execute('lspci', '-vmm', '-s', - address) - lines = output.split('\n') - for line in lines: - if 'NUMANode' in line: - numa_node = int(line.split(":")[1]) - except processutils.ProcessExecutionError: - raise exception.CommandError(cmd='lspci') + output, status = utils.execute('lspci', '-vmm', '-s', address) + lines = output.split('\n') + for line in lines: + if 'NUMANode' in line: + numa_node = int(line.split(":")[1]) return numa_node dev_name = 'pci_' + address.replace(":", "_").replace(".", "_") @@ -212,15 +198,12 @@ class Host(object): 'rdma': 'rdma'} features = [] - try: - output, status = processutils.execute('ethtool', '-k', ifname) - lines = output.split('\n') - for line in lines: - columns = line.split(":") - if columns[0].strip() in FEATURES_LIST: - if "on" in columns[1].strip(): - features.append(FEATURES_MAP.get(columns[0].strip())) - except processutils.ProcessExecutionError: - raise exception.CommandError(cmd='ethtool -k') + output, status = utils.execute('ethtool', '-k', ifname) + lines = output.split('\n') + for line in lines: + columns = line.split(":") + if columns[0].strip() in FEATURES_LIST: + if "on" in columns[1].strip(): + features.append(FEATURES_MAP.get(columns[0].strip())) return {'name': devname, 'capabilities': features} diff --git a/zun/container/os_capability/linux/os_capability_linux.py b/zun/container/os_capability/linux/os_capability_linux.py index 57ff05030..7589c8b2d 100644 --- a/zun/container/os_capability/linux/os_capability_linux.py +++ b/zun/container/os_capability/linux/os_capability_linux.py @@ -15,11 +15,10 @@ from collections import defaultdict import re -import six -from oslo_concurrency import processutils from oslo_log import log as logging from zun.common import exception +from zun.common import utils from zun.container.os_capability import host_capability @@ -32,19 +31,15 @@ class LinuxHost(host_capability.Host): # TODO(sbiswas7): rootwrap changes for zun required. old_lscpu = False try: - output = processutils.execute('lscpu', '-p=socket,cpu,online') - except processutils.ProcessExecutionError as e: + output = utils.execute('lscpu', '-p=socket,cpu,online') + except exception.CommandError: LOG.info("There was a problem while executing lscpu -p=socket" ",cpu,online. Try again without the online column.") # There is a possibility that an older version of lscpu is used # So let's try without the online column - try: - output = processutils.execute('lscpu', '-p=socket,cpu') - old_lscpu = True - except processutils.ProcessExecutionError as e: - LOG.exception("There was a problem while executing lscpu " - "-p=socket,cpu : %s", six.text_type(e)) - raise exception.CommandError(cmd="lscpu") + output = utils.execute('lscpu', '-p=socket,cpu') + old_lscpu = True + if old_lscpu: cpu_sock_pair = re.findall("\d+(?:,\d+)?", str(output)) else: diff --git a/zun/tests/unit/common/test_mount.py b/zun/tests/unit/common/test_mount.py index 95d7f3823..4d11560b7 100644 --- a/zun/tests/unit/common/test_mount.py +++ b/zun/tests/unit/common/test_mount.py @@ -12,8 +12,6 @@ import mock -from oslo_concurrency import processutils - from zun.common import exception from zun.common import mount from zun.tests import base @@ -25,12 +23,14 @@ class TestMounter(base.BaseTestCase): self.mountinfo = "/dev/0 /path/to/0 type0 flags 0 0\n" \ "/dev/1 /path/to/1 type1 flags 0 0\n" \ "/dev/2 /path/to/2 type2 flags,1,2=3 0 0\n" - self.mounts = [str(mount.MountInfo('/dev/0', '/path/to/0', - 'type0', 'flags')), - str(mount.MountInfo('/dev/1', '/path/to/1', - 'type1', 'flags')), - str(mount.MountInfo('/dev/2', '/path/to/2', - 'type2', 'flags,1,2=3'))] + self.mounts = [ + mount.MountInfo(device='/dev/0', mountpoint='/path/to/0', + fstype='type0', opts='flags'), + mount.MountInfo(device='/dev/1', mountpoint='/path/to/1', + fstype='type1', opts='flags'), + mount.MountInfo(device='/dev/2', mountpoint='/path/to/2', + fstype='type2', opts='flags,1,2=3'), + ] @mock.patch('zun.common.utils.execute') def test_mount(self, mock_execute): @@ -48,30 +48,32 @@ class TestMounter(base.BaseTestCase): fake_devpath = '/dev/3' fake_mp = '/path/to/3' fake_fstype = 'ext4' - mock_execute.side_effect = processutils.ProcessExecutionError() + mock_execute.side_effect = exception.CommandError() mounter = mount.Mounter() self.assertRaises(exception.MountException, mounter.mount, fake_devpath, fake_mp, fake_fstype) - @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('zun.common.utils.execute') def test_read_mounts(self, mock_execute): mock_execute.return_value = (self.mountinfo, '') - expected_mounts = self.mounts + expected_mounts = [str(m) for m in self.mounts] mounter = mount.Mounter() mounts = [str(m) for m in mounter.read_mounts()] self.assertEqual(len(expected_mounts), len(mounts)) for m in mounts: self.assertIn(m, expected_mounts) - @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('zun.common.utils.execute') def test_read_mounts_error(self, mock_execute): - mock_execute.side_effect = processutils.ProcessExecutionError() + mock_execute.side_effect = exception.CommandError() mounter = mount.Mounter() self.assertRaises(exception.FileNotFound, mounter.read_mounts) - @mock.patch('oslo_concurrency.processutils.execute') - def test_get_mps_by_device(self, mock_execute): + @mock.patch('zun.common.utils.execute') + @mock.patch('zun.common.mount.Mounter.read_mounts') + def test_get_mps_by_device(self, mock_read_mounts, mock_execute): mock_execute.return_value = (self.mountinfo, '') + mock_read_mounts.return_value = self.mounts mounter = mount.Mounter() self.assertEqual(['/path/to/0'], mounter.get_mps_by_device('/dev/0')) diff --git a/zun/tests/unit/container/docker/test_docker_driver.py b/zun/tests/unit/container/docker/test_docker_driver.py index 0bbb7b75c..6a7c68ad1 100644 --- a/zun/tests/unit/container/docker/test_docker_driver.py +++ b/zun/tests/unit/container/docker/test_docker_driver.py @@ -615,7 +615,7 @@ class TestDockerDriver(base.DriverTestCase): requested_network[0], security_groups=test_sec_group_id) - @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('zun.common.utils.execute') @mock.patch('zun.container.driver.ContainerDriver.get_host_mem') @mock.patch( 'zun.container.docker.driver.DockerDriver.get_host_info') diff --git a/zun/tests/unit/container/os_capability/linux/os_capability_linux/test_os_capability_linux.py b/zun/tests/unit/container/os_capability/linux/os_capability_linux/test_os_capability_linux.py index b6927b701..924dbc99c 100644 --- a/zun/tests/unit/container/os_capability/linux/os_capability_linux/test_os_capability_linux.py +++ b/zun/tests/unit/container/os_capability/linux/os_capability_linux/test_os_capability_linux.py @@ -16,8 +16,6 @@ import mock import six from mock import mock_open - -from oslo_concurrency import processutils from oslo_serialization import jsonutils from zun.common import exception @@ -45,22 +43,22 @@ LSCPU_NO_ONLINE = """# The following is the parsable format, which can be fed to class TestOSCapability(base.BaseTestCase): - @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('zun.common.utils.execute') def test_get_cpu_numa_info_with_online(self, mock_output): mock_output.return_value = LSCPU_ON output = os_capability_linux.LinuxHost().get_cpu_numa_info() expected_output = {'0': [0, 8], '1': [16, 24], '2': [32]} self.assertEqual(expected_output, output) - @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('zun.common.utils.execute') def test_get_cpu_numa_info_exception(self, mock_output): - mock_output.side_effect = processutils.ProcessExecutionError() + mock_output.side_effect = exception.CommandError() self.assertRaises(exception.CommandError, os_capability_linux.LinuxHost().get_cpu_numa_info) - @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('zun.common.utils.execute') def test_get_cpu_numa_info_without_online(self, mock_output): - mock_output.side_effect = [processutils.ProcessExecutionError(), + mock_output.side_effect = [exception.CommandError(), LSCPU_NO_ONLINE] expected_output = {'0': [0, 1], '1': [2, 3]} output = os_capability_linux.LinuxHost().get_cpu_numa_info() @@ -78,7 +76,7 @@ class TestOSCapability(base.BaseTestCase): @mock.patch('zun.pci.utils.get_ifname_by_pci_address') @mock.patch('zun.pci.utils.get_net_name_by_vf_pci_address') - @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('zun.common.utils.execute') def test_get_pci_resource(self, mock_output, mock_netname, mock_ifname): mock_netname.return_value = 'net_enp2s0f3_ec_38_8f_79_11_2b' diff --git a/zun/tests/unit/volume/test_driver.py b/zun/tests/unit/volume/test_driver.py index 87b2b4361..615d6a17b 100644 --- a/zun/tests/unit/volume/test_driver.py +++ b/zun/tests/unit/volume/test_driver.py @@ -177,8 +177,9 @@ class VolumeDriverTestCase(base.TestCase): self.assertEqual(self.fake_container_path, destination) mock_get_mountpoint.assert_called_once_with(self.fake_volume_id) + @mock.patch('zun.common.mount.Mounter.read_mounts') @mock.patch('zun.volume.cinder_workflow.CinderWorkflow') - def test_delete(self, mock_cinder_workflow_cls): + def test_delete(self, mock_cinder_workflow_cls, mock_read_mounts): mock_cinder_workflow = mock.MagicMock() mock_cinder_workflow_cls.return_value = mock_cinder_workflow mock_cinder_workflow.delete_volume.return_value = self.fake_volume_id diff --git a/zun/volume/cinder_workflow.py b/zun/volume/cinder_workflow.py index 3ac5d7500..30ad27668 100644 --- a/zun/volume/cinder_workflow.py +++ b/zun/volume/cinder_workflow.py @@ -19,7 +19,6 @@ from oslo_utils import excutils from zun.common import exception from zun.common.i18n import _ -from zun.common import utils import zun.conf from zun.volume import cinder_api as cinder @@ -40,9 +39,8 @@ def get_volume_connector_properties(): when multipathd is not running. """ - root_helper = utils.get_root_helper() return brick_connector.get_connector_properties( - root_helper, + None, CONF.my_block_storage_ip, CONF.volume.use_multipath, enforce_multipath=True, @@ -59,11 +57,10 @@ def get_volume_connector(protocol, driver=None, as the root_helper needed to execute commands. """ - root_helper = utils.get_root_helper() if protocol.upper() == "RBD": kwargs['do_local_attach'] = True return brick_connector.InitiatorConnector.factory( - protocol, root_helper, + protocol, None, driver=driver, use_multipath=use_multipath, device_scan_attempts=device_scan_attempts,