openstack
/
nova
Code Issues Proposed changes

Get rid of args to RBDDriver.__init__() 

While reviewing [1] I noticed an annoying and unnecessary
coupling/duplication of references to the CONF variables for RBD. This
commit consolidates them down to a single source, the guts of
RBDDriver.__init__().

As part of this a workaround is also removed concerning the passing of
conffile=None to RADOS via python-rbd. The following change merged in
2014 so we can now return None when ``[libvirt]/images_rbd_ceph_conf``
is not set:

Make python rados.Rados accept conffile=None
https://github.com/ceph/ceph/pull/1787

[1] https://review.opendev.org/667421

Co-Authored-By: Lee Yarwood <lyarwood@redhat.com>
Change-Id: I361af845d6a733618ecd056aa7df973191184ae9
This commit is contained in:
Eric Fried 2019-07-01 17:09:15 -05:00 committed by Lee Yarwood
parent 1599e3cf68
commit 45026cb5c1
6 changed files with 41 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
nova/tests/unit/virt/libvirt/storage/test_rbd.py
View File

@ -60,6 +60,12 @@ class RbdTestCase(test.NoDBTestCase):
def setUp(self):
super(RbdTestCase, self).setUp()
self.rbd_pool = 'rbd'
self.rbd_connect_timeout = 5
self.flags(images_rbd_pool=self.rbd_pool, group='libvirt')
self.flags(rbd_connect_timeout=self.rbd_connect_timeout,
group='libvirt')
rados_patcher = mock.patch.object(rbd_utils, 'rados')
self.mock_rados = rados_patcher.start()
self.addCleanup(rados_patcher.stop)
@ -82,10 +88,7 @@ class RbdTestCase(test.NoDBTestCase):
self.mock_rbd.ImageBusy = FakeException
self.mock_rbd.ImageHasSnapshots = FakeException
self.rbd_pool = 'rbd'
self.rbd_connect_timeout = 5
self.driver = rbd_utils.RBDDriver(self.rbd_pool, None, None,
self.rbd_connect_timeout)
self.driver = rbd_utils.RBDDriver()
self.volume_name = u'volume-00000001'
self.snap_name = u'test-snap'

2
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -20015,9 +20015,9 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
params={'vm_state': vm_states.SHELVED_OFFLOADED})
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
mock_rbd_driver = mock.Mock(spec=rbd_utils.RBDDriver)
mock_rbd_driver.pool = mock.sentinel.rbd_pool
mock_rbd_imagebackend = mock.Mock(spec=imagebackend.Rbd)
mock_rbd_imagebackend.rbd_name = mock.sentinel.rbd_name
mock_rbd_imagebackend.pool = mock.sentinel.rbd_pool
# This is logged so we can't use a sentinel
mock_rbd_imagebackend.path = 'rbd:pool/vol_disk'
mock_rbd_imagebackend.driver = mock_rbd_driver

12
nova/tests/unit/virt/libvirt/test_imagebackend.py
View File

@ -1683,7 +1683,7 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase):
def test_direct_snapshot_cleans_up_on_failures(self):
image = self.image_class(self.INSTANCE, self.NAME)
test_snap = 'rbd://%s/%s/%s/snap' % (self.FSID, image.pool,
test_snap = 'rbd://%s/%s/%s/snap' % (self.FSID, image.driver.pool,
image.rbd_name)
with test.nested(
mock.patch.object(rbd_utils.RBDDriver, 'get_fsid',
@ -1707,7 +1707,7 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase):
def test_cleanup_direct_snapshot(self):
image = self.image_class(self.INSTANCE, self.NAME)
test_snap = 'rbd://%s/%s/%s/snap' % (self.FSID, image.pool,
test_snap = 'rbd://%s/%s/%s/snap' % (self.FSID, image.driver.pool,
image.rbd_name)
with test.nested(
mock.patch.object(rbd_utils.RBDDriver, 'remove_snap'),
@ -1721,12 +1721,12 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase):
image.cleanup_direct_snapshot(dict(url=test_snap))
mock_rm.assert_called_once_with(image.rbd_name, 'snap', force=True,
ignore_errors=False,
pool=image.pool)
pool=image.driver.pool)
self.assertFalse(mock_destroy.called)
def test_cleanup_direct_snapshot_destroy_volume(self):
image = self.image_class(self.INSTANCE, self.NAME)
test_snap = 'rbd://%s/%s/%s/snap' % (self.FSID, image.pool,
test_snap = 'rbd://%s/%s/%s/snap' % (self.FSID, image.driver.pool,
image.rbd_name)
with test.nested(
mock.patch.object(rbd_utils.RBDDriver, 'remove_snap'),
@ -1738,9 +1738,9 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase):
mock_rm.assert_called_once_with(image.rbd_name, 'snap',
force=True,
ignore_errors=False,
pool=image.pool)
pool=image.driver.pool)
mock_destroy.assert_called_once_with(image.rbd_name,
pool=image.pool)
pool=image.driver.pool)
class PloopTestCase(_ImageTestCase, test.NoDBTestCase):

14
nova/virt/libvirt/driver.py
View File

@ -1239,14 +1239,6 @@ class LibvirtDriver(driver.ComputeDriver):
ret.append(int(obj.get('unit', 0)))
return max(ret)
@staticmethod
def _get_rbd_driver():
return rbd_utils.RBDDriver(
pool=CONF.libvirt.images_rbd_pool,
ceph_conf=CONF.libvirt.images_rbd_ceph_conf,
rbd_user=CONF.libvirt.rbd_user,
rbd_connect_timeout=CONF.libvirt.rbd_connect_timeout)
def _cleanup_rbd(self, instance):
# NOTE(nic): On revert_resize, the cleanup steps for the root
# volume are handled with an "rbd snap rollback" command,
@ -1257,7 +1249,7 @@ class LibvirtDriver(driver.ComputeDriver):
disk.endswith('disk.local'))
else:
filter_fn = lambda disk: disk.startswith(instance.uuid)
LibvirtDriver._get_rbd_driver().cleanup_volumes(filter_fn)
rbd_utils.RBDDriver().cleanup_volumes(filter_fn)
def _cleanup_lvm(self, instance, block_device_info):
"""Delete all LVM disks for given instance object."""
@ -3197,7 +3189,7 @@ class LibvirtDriver(driver.ComputeDriver):
if CONF.libvirt.images_type == 'rbd':
filter_fn = lambda disk: (disk.startswith(instance.uuid) and
disk.endswith('.rescue'))
LibvirtDriver._get_rbd_driver().cleanup_volumes(filter_fn)
rbd_utils.RBDDriver().cleanup_volumes(filter_fn)
def poll_rebooting_instances(self, timeout, instances):
pass
@ -5866,7 +5858,7 @@ class LibvirtDriver(driver.ComputeDriver):
info = lvm.get_volume_group_info(
CONF.libvirt.images_volume_group)
elif CONF.libvirt.images_type == 'rbd':
info = LibvirtDriver._get_rbd_driver().get_pool_info()
info = rbd_utils.RBDDriver().get_pool_info()
else:
info = libvirt_utils.get_fs_info(CONF.instances_path)

37
nova/virt/libvirt/imagebackend.py
View File

@ -845,25 +845,16 @@ class Rbd(Image):
else:
self.rbd_name = '%s_%s' % (instance.uuid, disk_name)
self.pool = CONF.libvirt.images_rbd_pool
self.rbd_user = CONF.libvirt.rbd_user
self.rbd_connect_timeout = CONF.libvirt.rbd_connect_timeout
self.ceph_conf = CONF.libvirt.images_rbd_ceph_conf
self.driver = rbd_utils.RBDDriver()
path = 'rbd:%s/%s' % (self.pool, self.rbd_name)
if self.rbd_user:
path += ':id=' + self.rbd_user
if self.ceph_conf:
path += ':conf=' + self.ceph_conf
path = 'rbd:%s/%s' % (self.driver.pool, self.rbd_name)
if self.driver.rbd_user:
path += ':id=' + self.driver.rbd_user
if self.driver.ceph_conf:
path += ':conf=' + self.driver.ceph_conf
super(Rbd, self).__init__(path, "block", "rbd", is_block_dev=False)
self.driver = rbd_utils.RBDDriver(
pool=self.pool,
ceph_conf=self.ceph_conf,
rbd_user=self.rbd_user,
rbd_connect_timeout=self.rbd_connect_timeout)
self.discard_mode = CONF.libvirt.hw_disk_discard
def libvirt_info(self, disk_info, cache_mode,
@ -889,16 +880,16 @@ class Rbd(Image):
info.target_dev = disk_info['dev']
info.source_type = 'network'
info.source_protocol = 'rbd'
info.source_name = '%s/%s' % (self.pool, self.rbd_name)
info.source_name = '%s/%s' % (self.driver.pool, self.rbd_name)
info.source_hosts = hosts
info.source_ports = ports
info.boot_order = boot_order
auth_enabled = (CONF.libvirt.rbd_user is not None)
auth_enabled = (self.driver.rbd_user is not None)
if CONF.libvirt.rbd_secret_uuid:
info.auth_secret_uuid = CONF.libvirt.rbd_secret_uuid
auth_enabled = True # Force authentication locally
if CONF.libvirt.rbd_user:
info.auth_username = CONF.libvirt.rbd_user
if self.driver.rbd_user:
info.auth_username = self.driver.rbd_user
if auth_enabled:
info.auth_secret_type = 'ceph'
info.auth_secret_uuid = CONF.libvirt.rbd_secret_uuid
@ -989,7 +980,7 @@ class Rbd(Image):
reason=reason)
def flatten(self):
self.driver.flatten(self.rbd_name, pool=self.pool)
self.driver.flatten(self.rbd_name, pool=self.driver.pool)
def get_model(self, connection):
secret = None
@ -1002,8 +993,8 @@ class Rbd(Image):
servers = [str(':'.join(k)) for k in zip(hosts, ports)]
return imgmodel.RBDImage(self.rbd_name,
self.pool,
self.rbd_user,
self.driver.pool,
self.driver.rbd_user,
secret,
servers)
@ -1077,7 +1068,7 @@ class Rbd(Image):
self.driver.create_snap(self.rbd_name, snapshot_name, protect=True)
location = {'url': 'rbd://%(fsid)s/%(pool)s/%(image)s/%(snap)s' %
dict(fsid=fsid,
pool=self.pool,
pool=self.driver.pool,
image=self.rbd_name,
snap=snapshot_name)}
try:

17
nova/virt/libvirt/storage/rbd_utils.py
View File

@ -32,10 +32,14 @@ from oslo_utils import encodeutils
from oslo_utils import excutils
from oslo_utils import units
import nova.conf
from nova import exception
from nova.i18n import _
from nova.virt.libvirt import utils as libvirt_utils
CONF = nova.conf.CONF
LOG = logging.getLogger(__name__)
@ -118,16 +122,15 @@ class RADOSClient(object):
class RBDDriver(object):
def __init__(self, pool, ceph_conf, rbd_user, rbd_connect_timeout):
self.pool = pool
# NOTE(angdraug): rados.Rados fails to connect if ceph_conf is None:
# https://github.com/ceph/ceph/pull/1787
self.ceph_conf = ceph_conf or ''
self.rbd_user = rbd_user or None
self.rbd_connect_timeout = rbd_connect_timeout
def __init__(self):
if rbd is None:
raise RuntimeError(_('rbd python libraries not found'))
self.pool = CONF.libvirt.images_rbd_pool
self.rbd_user = CONF.libvirt.rbd_user
self.rbd_connect_timeout = CONF.libvirt.rbd_connect_timeout
self.ceph_conf = CONF.libvirt.images_rbd_ceph_conf
def _connect_to_rados(self, pool=None):
client = rados.Rados(rados_id=self.rbd_user,
conffile=self.ceph_conf)