Merge "RBD: Default rbd_secret_uuid to the cluster FSID"

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': {

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,

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.