diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 2ac50b5f04c..21645886f07 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -19,6 +19,7 @@ import math import os import tempfile import time +import types from unittest import mock from unittest.mock import call import uuid @@ -176,14 +177,14 @@ class MockDriverConfig(object): @ddt.ddt class RBDTestCase(test.TestCase): - @staticmethod - def _make_configuration(conf_in=None): + @classmethod + def _make_configuration(cls, conf_in=None): cfg = mock.Mock(spec=conf.Configuration) cfg.image_conversion_dir = None cfg.rbd_cluster_name = 'nondefault' cfg.rbd_pool = 'rbd' cfg.rbd_ceph_conf = '/etc/ceph/my_ceph.conf' - cfg.rbd_secret_uuid = None + cfg.rbd_secret_uuid = '5fe62cc7-0392-4a32-8466-081ce0ea970f' cfg.rbd_user = 'cinder' cfg.volume_backend_name = None cfg.volume_dd_blocksize = '1M' @@ -193,12 +194,22 @@ class RBDTestCase(test.TestCase): cfg.backup_use_temp_snapshot = False cfg.enable_deferred_deletion = False + # Because the mocked conf doesn't actually have an underlying oslo conf + # it doesn't have the set_default method, so we use a fake one. + cfg.set_default = types.MethodType(cls._set_default, cfg) + if conf_in is not None: for k in conf_in: setattr(cfg, k, conf_in[k]) return cfg + @staticmethod + def _set_default(cfg, name, value, group=None): + # Ignore the group for now + if not getattr(cfg, name): + setattr(cfg, name, value) + @staticmethod def _make_drv(conf_in): cfg = RBDTestCase._make_configuration(conf_in) @@ -337,8 +348,11 @@ class RBDTestCase(test.TestCase): def test_do_setup_replication_disabled(self): with mock.patch.object(self.driver.configuration, 'safe_get', - return_value=None): + return_value=None), \ + mock.patch.object(self.driver, + '_set_default_secret_uuid') as mock_secret: self.driver.do_setup(self.context) + mock_secret.assert_called_once_with() self.assertFalse(self.driver._is_replication_enabled) self.assertEqual([], self.driver._replication_targets) self.assertEqual([], self.driver._target_names) @@ -348,6 +362,36 @@ class RBDTestCase(test.TestCase): 'secret_uuid': self.cfg.rbd_secret_uuid}, self.driver._active_config) + @ddt.data('', None) + @mock.patch.object(driver.RBDDriver, '_get_fsid') + def test__set_default_secret_uuid_missing(self, secret_uuid, mock_fsid): + # Clear the current values + self.cfg.rbd_secret_uuid = secret_uuid + self.driver._active_config['secret_uuid'] = secret_uuid + # Fake fsid value returned by the cluster + fsid = str(uuid.uuid4()) + mock_fsid.return_value = fsid + + self.driver._set_default_secret_uuid() + + mock_fsid.assert_called_once_with() + self.assertEqual(fsid, self.driver._active_config['secret_uuid']) + self.assertEqual(fsid, self.cfg.rbd_secret_uuid) + + @mock.patch.object(driver.RBDDriver, '_get_fsid') + def test__set_default_secret_uuid_present(self, mock_fsid): + # Set secret_uuid like _get_target_config does on do_setup + secret_uuid = self.cfg.rbd_secret_uuid + self.driver._active_config['secret_uuid'] = secret_uuid + # Fake fsid value returned by the cluster (should not be callled) + mock_fsid.return_value = str(uuid.uuid4()) + self.driver._set_default_secret_uuid() + mock_fsid.assert_not_called() + # Values must not have changed + self.assertEqual(secret_uuid, + self.driver._active_config['secret_uuid']) + self.assertEqual(secret_uuid, self.cfg.rbd_secret_uuid) + def test_do_setup_replication(self): cfg = [{'backend_id': 'secondary-backend', 'conf': 'foo', @@ -1983,7 +2027,8 @@ class RBDTestCase(test.TestCase): self.driver._active_config = {'name': 'secondary_id', 'user': 'foo', - 'conf': 'bar'} + 'conf': 'bar', + 'secret_uuid': self.cfg.rbd_secret_uuid} expected = { 'driver_volume_type': 'rbd', 'data': { diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index e4ec408b729..07cc72174dc 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -81,7 +81,7 @@ RBD_OPTS = [ 'dependency from volume to snapshot'), cfg.StrOpt('rbd_secret_uuid', help='The libvirt uuid of the secret for the rbd_user ' - 'volumes'), + 'volumes. Defaults to the cluster FSID.'), cfg.IntOpt('rbd_max_clone_depth', default=5, help='Maximum number of nested volume clones that are ' @@ -424,6 +424,17 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, """Performs initialization steps that could raise exceptions.""" self._do_setup_replication() self._active_config = self._get_target_config(self._active_backend_id) + self._set_default_secret_uuid() + + def _set_default_secret_uuid(self): + # Set secret_uuid to the cluster FSID if missing, should only happen + # with the primary/default configuration + if not self._active_config['secret_uuid']: + # self._active_config must be set before this call + fsid = self._get_fsid() + self._active_config['secret_uuid'] = fsid + LOG.info('Secret UUID defaulting to cluster FSID: %s', fsid) + self.configuration.set_default('rbd_secret_uuid', fsid) def _do_setup_replication(self) -> None: replication_devices = self.configuration.safe_get( @@ -1719,6 +1730,7 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, self._active_backend_id = secondary_id self._active_config = remote + self._set_default_secret_uuid() LOG.info('RBD driver failover completion completed.') def failover_host(self, diff --git a/releasenotes/notes/rbd-rbd_secret_uuid-fsid-95daee128f59c8e4.yaml b/releasenotes/notes/rbd-rbd_secret_uuid-fsid-95daee128f59c8e4.yaml new file mode 100644 index 00000000000..b8e067f6389 --- /dev/null +++ b/releasenotes/notes/rbd-rbd_secret_uuid-fsid-95daee128f59c8e4.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + RBD driver: Sets the Ceph cluster FSID as the default value for the + ``rbd_secret_uuid`` configuration option.