diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index bd65f37d8b5..f60bc30ef77 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -423,18 +423,19 @@ class RBDTestCase(test.TestCase): 'image.features()'will be both called for 'exclusive-lock' and 'journaling' in this order. """ - journaling_feat = 1 - exclusive_lock_feat = 2 - self.driver.rbd.RBD_FEATURE_JOURNALING = journaling_feat - self.driver.rbd.RBD_FEATURE_EXCLUSIVE_LOCK = exclusive_lock_feat image = self.mock_proxy.return_value.__enter__.return_value - image.features.return_value = 0 + + image_features = 0 if exclusive_lock_enabled: - image.features.return_value += exclusive_lock_feat + image_features |= self.driver.RBD_FEATURE_EXCLUSIVE_LOCK if journaling_enabled: - image.features.return_value += journaling_feat + image_features |= self.driver.RBD_FEATURE_JOURNALING + + image.features.return_value = image_features + journaling_status = str(journaling_enabled).lower() exclusive_lock_status = str(exclusive_lock_enabled).lower() + expected = { 'replication_driver_data': ('{"had_exclusive_lock":%s,' '"had_journaling":%s}' % @@ -444,14 +445,15 @@ class RBDTestCase(test.TestCase): } res = self.driver._enable_replication(self.volume_a) self.assertEqual(expected, res) + if exclusive_lock_enabled and journaling_enabled: image.update_features.assert_not_called() elif exclusive_lock_enabled and not journaling_enabled: - image.update_features.assert_called_once_with(journaling_feat, - True) + image.update_features.assert_called_once_with( + self.driver.RBD_FEATURE_JOURNALING, True) else: - calls = [call(exclusive_lock_feat, True), - call(journaling_feat, True)] + calls = [call(self.driver.RBD_FEATURE_EXCLUSIVE_LOCK, True), + call(self.driver.RBD_FEATURE_JOURNALING, True)] image.update_features.assert_has_calls(calls, any_order=False) image.mirror_image_enable.assert_called_once_with() @@ -474,10 +476,10 @@ class RBDTestCase(test.TestCase): image.update_features.assert_not_called() elif had_journaling == 'false' and had_exclusive_lock == 'true': image.update_features.assert_called_once_with( - self.driver.rbd.RBD_FEATURE_JOURNALING, False) + self.driver.RBD_FEATURE_JOURNALING, False) else: - calls = [call(self.driver.rbd.RBD_FEATURE_JOURNALING, False), - call(self.driver.rbd.RBD_FEATURE_EXCLUSIVE_LOCK, + calls = [call(self.driver.RBD_FEATURE_JOURNALING, False), + call(self.driver.RBD_FEATURE_EXCLUSIVE_LOCK, False)] image.update_features.assert_has_calls(calls, any_order=False) @@ -2459,6 +2461,55 @@ class RBDTestCase(test.TestCase): ret = driver.get_backup_device(self.context, backup) self.assertEqual(ret, (self.volume_b, False)) + @common_mocks + def test_multiattach_exclusions(self): + self.assertEqual( + self.driver.RBD_FEATURE_JOURNALING | + self.driver.RBD_FEATURE_FAST_DIFF | + self.driver.RBD_FEATURE_OBJECT_MAP | + self.driver.RBD_FEATURE_EXCLUSIVE_LOCK, + self.driver.MULTIATTACH_EXCLUSIONS) + + MULTIATTACH_FULL_FEATURES = ( + driver.RBDDriver.RBD_FEATURE_LAYERING | + driver.RBDDriver.RBD_FEATURE_EXCLUSIVE_LOCK | + driver.RBDDriver.RBD_FEATURE_OBJECT_MAP | + driver.RBDDriver.RBD_FEATURE_FAST_DIFF | + driver.RBDDriver.RBD_FEATURE_JOURNALING) + + MULTIATTACH_REDUCED_FEATURES = ( + driver.RBDDriver.RBD_FEATURE_LAYERING | + driver.RBDDriver.RBD_FEATURE_EXCLUSIVE_LOCK) + + @ddt.data(MULTIATTACH_FULL_FEATURES, MULTIATTACH_REDUCED_FEATURES) + @common_mocks + def test_enable_multiattach(self, features): + image = self.mock_proxy.return_value.__enter__.return_value + image_features = features + image.features.return_value = image_features + + ret = self.driver._enable_multiattach(self.volume_a) + + image.update_features.assert_called_once_with( + self.driver.MULTIATTACH_EXCLUSIONS & image_features, False) + + self.assertEqual( + {'provider_location': + "{\"saved_features\":%s}" % image_features}, ret) + + @ddt.data(MULTIATTACH_FULL_FEATURES, MULTIATTACH_REDUCED_FEATURES) + @common_mocks + def test_disable_multiattach(self, features): + image = self.mock_proxy.return_value.__enter__.return_value + self.volume_a.provider_location = '{"saved_features": %s}' % features + + ret = self.driver._disable_multiattach(self.volume_a) + + image.update_features.assert_called_once_with( + self.driver.MULTIATTACH_EXCLUSIONS & features, True) + + self.assertEqual({'provider_location': None}, ret) + class ManagedRBDTestCase(test_driver.BaseDriverTestCase): driver_name = "cinder.volume.drivers.rbd.RBDDriver" diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index 70adce85b73..4174ca1ac8e 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -204,7 +204,7 @@ class RADOSClient(object): def features(self): features = self.cluster.conf_get('rbd_default_features') if ((features is None) or (int(features) == 0)): - features = self.driver.rbd.RBD_FEATURE_LAYERING + features = self.driver.RBD_FEATURE_LAYERING return int(features) @@ -223,6 +223,12 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, SYSCONFDIR = '/etc/ceph/' + RBD_FEATURE_LAYERING = 1 + RBD_FEATURE_EXCLUSIVE_LOCK = 4 + RBD_FEATURE_OBJECT_MAP = 8 + RBD_FEATURE_FAST_DIFF = 16 + RBD_FEATURE_JOURNALING = 64 + def __init__(self, active_backend_id=None, *args, **kwargs): super(RBDDriver, self).__init__(*args, **kwargs) self.configuration.append_config_values(RBD_OPTS) @@ -247,6 +253,20 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, self._replication_targets = [] self._target_names = [] + if self.rbd is not None: + self.RBD_FEATURE_LAYERING = self.rbd.RBD_FEATURE_LAYERING + self.RBD_FEATURE_EXCLUSIVE_LOCK = \ + self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK + self.RBD_FEATURE_OBJECT_MAP = self.rbd.RBD_FEATURE_OBJECT_MAP + self.RBD_FEATURE_FAST_DIFF = self.rbd.RBD_FEATURE_FAST_DIFF + self.RBD_FEATURE_JOURNALING = self.rbd.RBD_FEATURE_JOURNALING + + self.MULTIATTACH_EXCLUSIONS = ( + self.RBD_FEATURE_JOURNALING | + self.RBD_FEATURE_FAST_DIFF | + self.RBD_FEATURE_OBJECT_MAP | + self.RBD_FEATURE_EXCLUSIVE_LOCK) + @staticmethod def get_driver_options(): return RBD_OPTS @@ -752,13 +772,13 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, vol_name = utils.convert_str(volume.name) with RBDVolumeProxy(self, vol_name) as image: had_exclusive_lock = (image.features() & - self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK) - had_journaling = image.features() & self.rbd.RBD_FEATURE_JOURNALING + self.RBD_FEATURE_EXCLUSIVE_LOCK) + had_journaling = image.features() & self.RBD_FEATURE_JOURNALING if not had_exclusive_lock: - image.update_features(self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK, + image.update_features(self.RBD_FEATURE_EXCLUSIVE_LOCK, True) if not had_journaling: - image.update_features(self.rbd.RBD_FEATURE_JOURNALING, True) + image.update_features(self.RBD_FEATURE_JOURNALING, True) image.mirror_image_enable() driver_data = self._dumps({ @@ -769,53 +789,49 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, 'replication_driver_data': driver_data} def _enable_multiattach(self, volume): - multipath_feature_exclusions = [ - self.rbd.RBD_FEATURE_JOURNALING, - self.rbd.RBD_FEATURE_FAST_DIFF, - self.rbd.RBD_FEATURE_OBJECT_MAP, - self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK, - ] vol_name = utils.convert_str(volume.name) - image_features = None with RBDVolumeProxy(self, vol_name) as image: image_features = image.features() - for feature in multipath_feature_exclusions: - if image_features & feature: - image.update_features(feature, False) + change_features = self.MULTIATTACH_EXCLUSIONS & image_features + image.update_features(change_features, False) return {'provider_location': self._dumps({'saved_features': image_features})} def _disable_multiattach(self, volume): - multipath_feature_exclusions = [ - self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK, - self.rbd.RBD_FEATURE_OBJECT_MAP, - self.rbd.RBD_FEATURE_FAST_DIFF, - self.rbd.RBD_FEATURE_JOURNALING, - ] vol_name = utils.convert_str(volume.name) with RBDVolumeProxy(self, vol_name) as image: try: provider_location = json.loads(volume.provider_location) image_features = provider_location['saved_features'] - except Exception: - msg = _('Could not find saved image features.') + change_features = self.MULTIATTACH_EXCLUSIONS & image_features + image.update_features(change_features, True) + except IndexError: + msg = "Could not find saved image features." + raise RBDDriverException(reason=msg) + except self.rbd.InvalidArgument: + msg = "Failed to restore image features." raise RBDDriverException(reason=msg) - for feature in multipath_feature_exclusions: - if image_features & feature: - image.update_features(feature, True) return {'provider_location': None} def _is_replicated_type(self, volume_type): - # We do a safe attribute get because volume_type could be None - specs = getattr(volume_type, 'extra_specs', {}) - return specs.get(EXTRA_SPECS_REPL_ENABLED) == " True" + try: + extra_specs = volume_type.extra_specs + LOG.debug('extra_specs: %s', extra_specs) + return extra_specs.get(EXTRA_SPECS_REPL_ENABLED) == " True" + except Exception: + LOG.debug('Unable to retrieve extra specs info') + return False def _is_multiattach_type(self, volume_type): - # We do a safe attribute get because volume_type could be None - specs = getattr(volume_type, 'extra_specs', {}) - return specs.get(EXTRA_SPECS_MULTIATTACH) == " True" + try: + extra_specs = volume_type.extra_specs + LOG.debug('extra_specs: %s', extra_specs) + return extra_specs.get(EXTRA_SPECS_MULTIATTACH) == " True" + except Exception: + LOG.debug('Unable to retrieve extra specs info') + return False def _setup_volume(self, volume, volume_type=None): @@ -1224,10 +1240,9 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, # they will be disabled here. If not, it will keep # what it was before. if not driver_data['had_journaling']: - image.update_features(self.rbd.RBD_FEATURE_JOURNALING, False) + image.update_features(self.RBD_FEATURE_JOURNALING, False) if not driver_data['had_exclusive_lock']: - image.update_features(self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK, - False) + image.update_features(self.RBD_FEATURE_EXCLUSIVE_LOCK, False) return {'replication_status': fields.ReplicationStatus.DISABLED, 'replication_driver_data': None} diff --git a/releasenotes/notes/rbd-multiattach-exceptions-43066312f3b527f5.yaml b/releasenotes/notes/rbd-multiattach-exceptions-43066312f3b527f5.yaml new file mode 100644 index 00000000000..d5b19f3bf49 --- /dev/null +++ b/releasenotes/notes/rbd-multiattach-exceptions-43066312f3b527f5.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Catch argument exceptions when configuring multiattach for rbd volumes. + This allows multiattach images with flags already set to continue instead + of raising an exception and failing.