From db573863aaf5a836ac63acbd55c42ba7677c0294 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Fri, 3 Jul 2020 11:15:27 +0200 Subject: [PATCH] Prevent duplicated backend names With this patch we prevent us from having duplicated backend names for different driver configurations during the same run. If we pass the same name and configuration to Backend a second time we get the Backend instance that was created on the first call. If we pass the same name with a different configuration then we raise an exception on the second call. Change-Id: I87c14d236d7f2685315e4088db527090680c9523 Closes-Bug: #1886164 --- cinderlib/cinderlib.py | 22 ++++++++- cinderlib/tests/unit/test_cinderlib.py | 45 +++++++++++++++++++ .../backend-same-name-86a06844b57600a0.yaml | 6 +++ 3 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/backend-same-name-86a06844b57600a0.yaml diff --git a/cinderlib/cinderlib.py b/cinderlib/cinderlib.py index 5e94f28..4a40f68 100644 --- a/cinderlib/cinderlib.py +++ b/cinderlib/cinderlib.py @@ -69,11 +69,30 @@ class Backend(object): # With this dictionary the DB class can get the necessary data _volumes_inflight = {} + def __new__(cls, volume_backend_name, **driver_cfg): + # Prevent redefinition of an already initialized backend on the same + # persistence storage with a different configuration. + backend = Backend.backends.get(volume_backend_name) + if backend: + # If we are instantiating the same backend return the one we have + # saved (singleton pattern). + if driver_cfg == backend._original_driver_cfg: + return backend + raise ValueError('Backend named %s already exists with a different' + ' configuration' % volume_backend_name) + + return super(Backend, cls).__new__(cls) + def __init__(self, volume_backend_name, **driver_cfg): if not self.global_initialization: self.global_setup() + # Instance already initialized + if volume_backend_name in Backend.backends: + return + # Save the original config before we add the backend name and template + # the values. + self._original_driver_cfg = driver_cfg.copy() driver_cfg['volume_backend_name'] = volume_backend_name - Backend.backends[volume_backend_name] = self conf = self._get_backend_config(driver_cfg) self._apply_backend_workarounds(conf) @@ -101,6 +120,7 @@ class Backend(object): self._stats = self._transform_legacy_stats(stats) self._pool_names = tuple(pool['pool_name'] for pool in stats['pools']) + Backend.backends[volume_backend_name] = self @property def pool_names(self): diff --git a/cinderlib/tests/unit/test_cinderlib.py b/cinderlib/tests/unit/test_cinderlib.py index 80d613f..cd79258 100644 --- a/cinderlib/tests/unit/test_cinderlib.py +++ b/cinderlib/tests/unit/test_cinderlib.py @@ -101,6 +101,51 @@ class TestCinderlib(base.BaseTest): self.assertEqual(('default',), backend.pool_names) mock_workarounds.assert_called_once_with(mock_config.return_value) + @mock.patch.object(objects.Backend, 'global_initialization', True) + @mock.patch.object(objects.Backend, '_apply_backend_workarounds') + @mock.patch('oslo_utils.importutils.import_object') + @mock.patch.object(objects.Backend, '_get_backend_config') + def test_init_call_twice(self, mock_config, mock_import, mock_workarounds): + cinderlib.Backend.global_initialization = False + driver_cfg = {'k': 'v', 'k2': 'v2', 'volume_backend_name': 'Test'} + driver = mock_import.return_value + driver.capabilities = {'pools': [{'pool_name': 'default'}]} + + backend = objects.Backend(**driver_cfg) + self.assertEqual(1, mock_config.call_count) + self.assertEqual(1, mock_import.call_count) + self.assertEqual(1, mock_workarounds.call_count) + + # When initiallizing a Backend with the same configuration the Backend + # class must behave as a singleton and we won't initialize it again + backend_second = objects.Backend(**driver_cfg) + self.assertIs(backend, backend_second) + self.assertEqual(1, mock_config.call_count) + self.assertEqual(1, mock_import.call_count) + self.assertEqual(1, mock_workarounds.call_count) + + @mock.patch.object(objects.Backend, 'global_initialization', True) + @mock.patch.object(objects.Backend, '_apply_backend_workarounds') + @mock.patch('oslo_utils.importutils.import_object') + @mock.patch.object(objects.Backend, '_get_backend_config') + def test_init_call_twice_different_config(self, mock_config, mock_import, + mock_workarounds): + cinderlib.Backend.global_initialization = False + driver_cfg = {'k': 'v', 'k2': 'v2', 'volume_backend_name': 'Test'} + driver = mock_import.return_value + driver.capabilities = {'pools': [{'pool_name': 'default'}]} + + objects.Backend(**driver_cfg) + self.assertEqual(1, mock_config.call_count) + self.assertEqual(1, mock_import.call_count) + self.assertEqual(1, mock_workarounds.call_count) + + # It should fail if we reuse the backend name but change the config + self.assertRaises(ValueError, objects.Backend, k3='v3', **driver_cfg) + self.assertEqual(1, mock_config.call_count) + self.assertEqual(1, mock_import.call_count) + self.assertEqual(1, mock_workarounds.call_count) + @mock.patch('cinderlib.Backend._validate_and_set_options') @mock.patch.object(cfg, 'CONF') def test__set_cinder_config(self, conf_mock, validate_mock): diff --git a/releasenotes/notes/backend-same-name-86a06844b57600a0.yaml b/releasenotes/notes/backend-same-name-86a06844b57600a0.yaml new file mode 100644 index 0000000..6d05d20 --- /dev/null +++ b/releasenotes/notes/backend-same-name-86a06844b57600a0.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Prevent the creation of multiple backends with the same name during the + same code run. + (Bug #1886164).