Browse Source

Exec systemd-run without --user flag in Quobyte driver

Removes the --user flag from the systemd based mount command in the
Quobyte driver. This prevents mount failures due to:
- Older systemd releases not supporting the --user flag (e.g. CentOS)
- Systemd versions having a bug preventing running the --scope and
  --user flags together
- processutils context not allowing mount to run with this flag
(see bug referenced below for details)

Furthermore the systemd detection is fixed and
all mount and umount commands are moved to libvirt privsep.

Closes-Bug: #1756823

Change-Id: I53f3a062ce419d1142d7dd3103fab565bb105e05
tags/20.0.0.0rc1
Silvan Kaiser 1 year ago
parent
commit
7231f7dee1
4 changed files with 161 additions and 84 deletions
  1. +32
    -0
      nova/privsep/libvirt.py
  2. +93
    -67
      nova/tests/unit/virt/libvirt/volume/test_quobyte.py
  3. +31
    -17
      nova/virt/libvirt/volume/quobyte.py
  4. +5
    -0
      releasenotes/notes/bug-1756823-fix-d3a999a258019c54.yaml

+ 32
- 0
nova/privsep/libvirt.py View File

@@ -255,3 +255,35 @@ def create_mdev(physical_device, mdev_type, uuid=None):
with open(fpath, 'w') as f:
f.write(uuid)
return uuid


@nova.privsep.sys_admin_pctxt.entrypoint
def systemd_run_qb_mount(qb_vol, mnt_base, cfg_file=None):
"""Mount QB volume in separate CGROUP"""
sysdr_cmd = ['systemd-run', '--scope', 'mount.quobyte', '--disable-xattrs',
qb_vol, mnt_base]
if cfg_file:
sysdr_cmd.extend(['-c', cfg_file])
return processutils.execute(*sysdr_cmd)


# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp.
def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file=None):
"""Mount QB volume"""
mnt_cmd = ['mount.quobyte', '--disable-xattrs', qb_vol, mnt_base]
if cfg_file:
mnt_cmd.extend(['-c', cfg_file])
return processutils.execute(*mnt_cmd)


@nova.privsep.sys_admin_pctxt.entrypoint
def qb_umount(mnt_base):
"""Unmount QB volume"""
unprivileged_qb_umount(mnt_base)


# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp.
def unprivileged_qb_umount(mnt_base):
"""Unmount QB volume"""
umnt_cmd = ['umount.quobyte', mnt_base]
return processutils.execute(*umnt_cmd)

+ 93
- 67
nova/tests/unit/virt/libvirt/volume/test_quobyte.py View File

@@ -17,6 +17,7 @@
import os
import traceback

import ddt
import mock
from oslo_concurrency import processutils
from oslo_utils import fileutils
@@ -24,6 +25,7 @@ import psutil
import six

from nova import exception as nova_exception
from nova.privsep import libvirt
from nova import test
from nova.tests.unit.virt.libvirt.volume import test_volume
from nova import utils
@@ -31,6 +33,7 @@ from nova.virt.libvirt import utils as libvirt_utils
from nova.virt.libvirt.volume import quobyte


@ddt.ddt
class QuobyteTestCase(test.NoDBTestCase):
"""Tests the nova.virt.libvirt.volume.quobyte module utilities."""

@@ -59,86 +62,85 @@ class QuobyteTestCase(test.NoDBTestCase):
mypart.mountpoint = self.TEST_MNT_POINT
return [mypart]

@mock.patch.object(os.path, "exists", return_value=False)
@mock.patch.object(fileutils, "ensure_tree")
@mock.patch('oslo_concurrency.processutils.execute')
def test_quobyte_mount_volume_not_systemd(self, mock_execute,
mock_ensure_tree,
mock_exists):
mnt_base = '/mnt'
quobyte_volume = '192.168.1.1/volume-00001'
export_mnt_base = os.path.join(mnt_base,
utils.get_hash_str(quobyte_volume))
@ddt.data(u"starting", u"running", u"degraded")
@mock.patch.object(psutil.Process, "name", return_value="systemd")
@mock.patch.object(processutils, "execute")
def test_quobyte_is_systemd_ok(self, value, mock_execute, mock_proc):
mock_execute.return_value = [value, "fake stderr"]

quobyte.mount_volume(quobyte_volume, export_mnt_base)
self.assertTrue(quobyte.is_systemd())
mock_execute.assert_called_once_with("systemctl",
"is-system-running",
check_exit_code=[0, 1])
mock_proc.assert_called_once()

mock_ensure_tree.assert_called_once_with(export_mnt_base)
expected_commands = [mock.call('mount.quobyte',
'--disable-xattrs',
quobyte_volume,
export_mnt_base)
]
mock_execute.assert_has_calls(expected_commands)
mock_exists.assert_called_once_with(" /run/systemd/system")
@mock.patch.object(psutil.Process, "name", return_value="NOT_systemd")
@mock.patch.object(os.path, "exists", return_value=False)
def test_quobyte_is_systemd_not(self, mock_exists, mock_proc):
self.assertFalse(quobyte.is_systemd())
mock_exists.assert_called_once_with(quobyte.SYSTEMCTL_CHECK_PATH)
mock_proc.assert_called_once_with()

@mock.patch.object(psutil.Process, "name", return_value="NOT_systemd")
@mock.patch.object(processutils, "execute")
@mock.patch.object(os.path, "exists", return_value=True)
def test_quobyte_is_systemd_invalid_state(self, mock_exists, mock_execute,
mock_proc):
mock_execute.return_value = ["FAKE_INACCEPTABLE_STATE", "fake stderr"]

self.assertFalse(quobyte.is_systemd())

mock_exists.assert_called_once_with(quobyte.SYSTEMCTL_CHECK_PATH)
mock_execute.assert_called_once_with("systemctl",
"is-system-running",
check_exit_code=[0, 1])
mock_proc.assert_called_once_with()

@ddt.data(None, '/some/arbitrary/path')
@mock.patch.object(quobyte, "is_systemd", return_value=False)
@mock.patch.object(fileutils, "ensure_tree")
@mock.patch('oslo_concurrency.processutils.execute')
def test_quobyte_mount_volume_systemd(self, mock_execute,
mock_ensure_tree,
mock_exists):
@mock.patch.object(libvirt, "unprivileged_qb_mount")
def test_quobyte_mount_volume_not_systemd(self, value, mock_mount,
mock_ensure_tree, mock_is_sysd):
mnt_base = '/mnt'
quobyte_volume = '192.168.1.1/volume-00001'
export_mnt_base = os.path.join(mnt_base,
utils.get_hash_str(quobyte_volume))

quobyte.mount_volume(quobyte_volume, export_mnt_base)
quobyte.mount_volume(quobyte_volume, export_mnt_base, value)

mock_ensure_tree.assert_called_once_with(export_mnt_base)
expected_commands = [mock.call('systemd-run',
'--scope',
'--user',
'mount.quobyte',
'--disable-xattrs',
quobyte_volume,
export_mnt_base)
]
mock_execute.assert_has_calls(expected_commands)
mock_exists.assert_called_once_with(" /run/systemd/system")

@mock.patch.object(os.path, "exists", return_value=False)
expected_commands = [mock.call(quobyte_volume, export_mnt_base,
cfg_file=value)]
mock_mount.assert_has_calls(expected_commands)
mock_is_sysd.assert_called_once_with()

@ddt.data(None, '/some/arbitrary/path')
@mock.patch.object(libvirt, 'systemd_run_qb_mount')
@mock.patch.object(quobyte, "is_systemd", return_value=True)
@mock.patch.object(fileutils, "ensure_tree")
@mock.patch('oslo_concurrency.processutils.execute')
def test_quobyte_mount_volume_with_config(self,
mock_execute,
mock_ensure_tree,
mock_exists):
def test_quobyte_mount_volume_systemd(self, value, mock_ensure_tree,
mock_exists, mock_privsep_sysdr):
mnt_base = '/mnt'
quobyte_volume = '192.168.1.1/volume-00001'
export_mnt_base = os.path.join(mnt_base,
utils.get_hash_str(quobyte_volume))
config_file_dummy = "/etc/quobyte/dummy.conf"

quobyte.mount_volume(quobyte_volume,
export_mnt_base,
config_file_dummy)
quobyte.mount_volume(quobyte_volume, export_mnt_base, value)

mock_ensure_tree.assert_called_once_with(export_mnt_base)
expected_commands = [mock.call('mount.quobyte',
'--disable-xattrs',
quobyte_volume,
export_mnt_base,
'-c',
config_file_dummy)
]
mock_execute.assert_has_calls(expected_commands)
mock_exists.assert_called_once_with(" /run/systemd/system")
mock_exists.assert_called_once_with()
mock_privsep_sysdr.assert_called_once_with(quobyte_volume,
export_mnt_base,
cfg_file=value)

@mock.patch.object(quobyte, "is_systemd", return_value=False)
@mock.patch.object(fileutils, "ensure_tree")
@mock.patch('oslo_concurrency.processutils.execute',
side_effect=(processutils.
ProcessExecutionError))
def test_quobyte_mount_volume_fails(self, mock_execute, mock_ensure_tree):
@mock.patch.object(libvirt, 'unprivileged_qb_mount',
side_effect=(processutils.
ProcessExecutionError))
def test_quobyte_mount_volume_fails(self, mock_mount, mock_ensure_tree,
mock_is_sysd):
mnt_base = '/mnt'
quobyte_volume = '192.168.1.1/volume-00001'
export_mnt_base = os.path.join(mnt_base,
@@ -148,9 +150,26 @@ class QuobyteTestCase(test.NoDBTestCase):
quobyte.mount_volume,
quobyte_volume,
export_mnt_base)
mock_ensure_tree.assert_called_once_with(export_mnt_base)
mock_is_sysd.assert_called_once()

@mock.patch.object(quobyte, "is_systemd", return_value=False)
@mock.patch.object(libvirt, 'unprivileged_qb_umount')
def test_quobyte_umount_volume_non_sysd(self,
mock_lv_umount, mock_is_sysd):
mnt_base = '/mnt'
quobyte_volume = '192.168.1.1/volume-00001'
export_mnt_base = os.path.join(mnt_base,
utils.get_hash_str(quobyte_volume))

quobyte.umount_volume(export_mnt_base)

mock_lv_umount.assert_called_once_with(export_mnt_base)
mock_is_sysd.assert_called_once()

@mock.patch('oslo_concurrency.processutils.execute')
def test_quobyte_umount_volume(self, mock_execute):
@mock.patch.object(quobyte, "is_systemd", return_value=True)
@mock.patch.object(libvirt, 'qb_umount')
def test_quobyte_umount_volume_sysd(self, mock_lv_umount, mock_exists):
mnt_base = '/mnt'
quobyte_volume = '192.168.1.1/volume-00001'
export_mnt_base = os.path.join(mnt_base,
@@ -158,14 +177,16 @@ class QuobyteTestCase(test.NoDBTestCase):

quobyte.umount_volume(export_mnt_base)

mock_execute.assert_called_once_with('umount.quobyte',
export_mnt_base)
mock_lv_umount.assert_called_once_with(export_mnt_base)
mock_exists.assert_called_once_with()

@mock.patch.object(quobyte, "is_systemd", return_value=True)
@mock.patch.object(quobyte.LOG, "error")
@mock.patch('oslo_concurrency.processutils.execute')
@mock.patch.object(libvirt, 'qb_umount')
def test_quobyte_umount_volume_warns(self,
mock_execute,
mock_debug):
mock_debug,
mock_issysd):
mnt_base = '/mnt'
quobyte_volume = '192.168.1.1/volume-00001'
export_mnt_base = os.path.join(mnt_base,
@@ -182,13 +203,16 @@ class QuobyteTestCase(test.NoDBTestCase):
(mock_debug.
assert_called_once_with("The Quobyte volume at %s is still in use.",
export_mnt_base))
mock_issysd.assert_called_once_with()

@mock.patch.object(quobyte, "is_systemd", return_value=True)
@mock.patch.object(quobyte.LOG, "exception")
@mock.patch('oslo_concurrency.processutils.execute',
side_effect=(processutils.ProcessExecutionError))
@mock.patch.object(libvirt, 'qb_umount',
side_effect=(processutils.ProcessExecutionError))
def test_quobyte_umount_volume_fails(self,
mock_execute,
mock_exception):
mock_exception,
mock_issysd):
mnt_base = '/mnt'
quobyte_volume = '192.168.1.1/volume-00001'
export_mnt_base = os.path.join(mnt_base,
@@ -196,10 +220,12 @@ class QuobyteTestCase(test.NoDBTestCase):

quobyte.umount_volume(export_mnt_base)

mock_execute.assert_called_once_with(export_mnt_base)
(mock_exception.
assert_called_once_with("Couldn't unmount "
"the Quobyte Volume at %s",
export_mnt_base))
mock_issysd.assert_called_once_with()

@mock.patch.object(psutil, "disk_partitions")
@mock.patch.object(os, "stat")

+ 31
- 17
nova/virt/libvirt/volume/quobyte.py View File

@@ -25,6 +25,7 @@ import six
import nova.conf
from nova import exception as nova_exception
from nova.i18n import _
from nova.privsep import libvirt
from nova import utils
from nova.virt.libvirt import utils as libvirt_utils
from nova.virt.libvirt.volume import fs
@@ -37,35 +38,48 @@ SOURCE_PROTOCOL = 'quobyte'
SOURCE_TYPE = 'file'
DRIVER_CACHE = 'none'
DRIVER_IO = 'native'
VALID_SYSD_STATES = ["starting", "running", "degraded"]
SYSTEMCTL_CHECK_PATH = "/run/systemd/system"


def is_systemd():
"""Checks if the host is running systemd"""
if psutil.Process(1).name() == "systemd" or os.path.exists(
SYSTEMCTL_CHECK_PATH):
sysdout, sysderr = processutils.execute("systemctl",
"is-system-running",
check_exit_code=[0, 1])
for state in VALID_SYSD_STATES:
if state in sysdout:
return True
return False


def mount_volume(volume, mnt_base, configfile=None):
"""Wraps execute calls for mounting a Quobyte volume"""
fileutils.ensure_tree(mnt_base)

# NOTE(kaisers): disable xattrs to speed up io as this omits
# additional metadata requests in the backend. xattrs can be
# enabled without issues but will reduce performance.
command = ['mount.quobyte', '--disable-xattrs', volume, mnt_base]
if os.path.exists(" /run/systemd/system"):
# Note(kaisers): with systemd this requires a separate CGROUP to
# prevent Nova service stop/restarts from killing the mount.
command = ['systemd-run', '--scope', '--user', 'mount.quobyte',
'--disable-xattrs', volume, mnt_base]
if configfile:
command.extend(['-c', configfile])

LOG.debug('Mounting volume %s at mount point %s ...',
volume,
mnt_base)
processutils.execute(*command)
# Note(kaisers): with systemd this requires a separate CGROUP to
# prevent Nova service stop/restarts from killing the mount.
if is_systemd():
LOG.debug('Mounting volume %s at mount point %s via systemd-run',
volume, mnt_base)
libvirt.systemd_run_qb_mount(volume, mnt_base, cfg_file=configfile)
else:
LOG.debug('Mounting volume %s at mount point %s via mount.quobyte',
volume, mnt_base, cfg_file=configfile)

libvirt.unprivileged_qb_mount(volume, mnt_base, cfg_file=configfile)
LOG.info('Mounted volume: %s', volume)


def umount_volume(mnt_base):
"""Wraps execute calls for unmouting a Quobyte volume"""
try:
processutils.execute('umount.quobyte', mnt_base)
if is_systemd():
libvirt.qb_umount(mnt_base)
else:
libvirt.unprivileged_qb_umount(mnt_base)
except processutils.ProcessExecutionError as exc:
if 'Device or resource busy' in six.text_type(exc):
LOG.error("The Quobyte volume at %s is still in use.", mnt_base)

+ 5
- 0
releasenotes/notes/bug-1756823-fix-d3a999a258019c54.yaml View File

@@ -0,0 +1,5 @@
---
fixes:
- |
Fixes a bug causing mount failures on systemd based systems that are
using the systemd-run based mount with the Nova Quobyte driver.

Loading…
Cancel
Save