From efd83675780a76422436d47c4622e22a0645cc30 Mon Sep 17 00:00:00 2001 From: Sofia Enriquez Date: Mon, 28 Sep 2020 20:20:34 +0000 Subject: [PATCH] Log information about the Ceph v2 clone API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starting with the Mimic release and the Ceph RBD clone v2 feature, it is no longer necessary to manage snapshot protection status. The usage of clone v2 is controlled via the Ceph “rbd default clone format” configuration option which defaults to “auto”, but can be overridden to “1” to force the legacy clone v1 behavior or “2” to force the new clonev2 behavior. This must be configured in Ceph, it's not something that can be configured on the Cinder side. As an alternative, setting the minimum client to mimic "$ ceph osd set-require-min-compat-client mimic" Above all with Ceph RBD clone v2 support enabled, image snapshots can be cloned without marking the snapshot as protected because RBD automatically tracks the usage of the snapshot by clones. As a result Ceph RBD clone v2 API improves performance. This proposed patch adds logging messages for the operator indicating whether Ceph has been configured to use the Ceph RBD v2 clone feature. In this way the operator can consult with the Ceph administrator to take appropriate action. You can determine which volumes are using the clone v2 feature thanks to volume.volume.op_features(). It returns a value depending on the v2 clone api being enabled or not. Co-authored-by: Eric Harney Change-Id: Ib52879a270a4ae4cdd3cb5fc18f2b7bdbccd8ab5 --- cinder/tests/unit/volume/drivers/test_rbd.py | 65 ++++++++++++++++++++ cinder/volume/drivers/rbd.py | 19 ++++++ 2 files changed, 84 insertions(+) diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 6e8e65fe30d..16ca0e77de2 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -869,6 +869,71 @@ class RBDTestCase(test.TestCase): proxy.create_snap.assert_called_with(*args) proxy.protect_snap.assert_called_with(*args) + @common_mocks + @mock.patch('cinder.objects.Volume.get_by_id') + @mock.patch.object(driver.RBDDriver, '_resize', mock.Mock()) + def test_log_create_vol_from_snap_w_v2_clone_api(self, volume_get_by_id): + volume_get_by_id.return_value = self.volume_a + + self.mock_proxy().__enter__().volume.op_features.return_value = 1 + self.mock_rbd.RBD_OPERATION_FEATURE_CLONE_PARENT = 1 + + snapshot = mock.Mock() + self.cfg.rbd_flatten_volume_from_snapshot = False + + with mock.patch.object(driver, 'LOG') as \ + mock_log: + + self.driver.create_volume_from_snapshot(self.volume_a, snapshot) + + mock_log.info.assert_called_once_with(mock.ANY) + mock_log.warning.assert_not_called() + self.assertTrue(self.driver._clone_v2_api_checked) + + @common_mocks + @mock.patch('cinder.objects.Volume.get_by_id') + @mock.patch.object(driver.RBDDriver, '_resize', mock.Mock()) + def test_log_create_vol_from_snap_without_v2_clone_api(self, + volume_get_by_id): + volume_get_by_id.return_value = self.volume_a + + self.mock_proxy().__enter__().volume.op_features.return_value = 0 + self.mock_rbd.RBD_OPERATION_FEATURE_CLONE_PARENT = 0 + + snapshot = mock.Mock() + self.cfg.rbd_flatten_volume_from_snapshot = False + + with mock.patch.object(driver, 'LOG') as \ + mock_log: + + self.driver.create_volume_from_snapshot(self.volume_a, snapshot) + + mock_log.warning.assert_called_once_with(mock.ANY) + mock_log.info.assert_not_called() + self.assertTrue(self.driver._clone_v2_api_checked) + + @common_mocks + @mock.patch('cinder.objects.Volume.get_by_id') + @mock.patch.object(driver.RBDDriver, '_resize', mock.Mock()) + def test_log_create_vol_from_snap_without_v2_clone_api_2(self, + volume_get_by_id): + volume_get_by_id.return_value = self.volume_a + + self.mock_proxy().__enter__().volume.op_features.return_value = 0 + self.mock_rbd.RBD_OPERATION_FEATURE_CLONE_PARENT = 1 + + snapshot = mock.Mock() + self.cfg.rbd_flatten_volume_from_snapshot = False + + with mock.patch.object(driver, 'LOG') as \ + mock_log: + + self.driver.create_volume_from_snapshot(self.volume_a, snapshot) + + mock_log.warning.assert_called_once_with(mock.ANY) + mock_log.info.assert_not_called() + self.assertTrue(self.driver._clone_v2_api_checked) + @common_mocks @mock.patch('cinder.objects.Volume.get_by_id') def test_delete_snapshot(self, volume_get_by_id): diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index e5c3a89522f..ffccb62940f 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -246,6 +246,7 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, self._is_replication_enabled = False self._replication_targets = [] self._target_names = [] + self._clone_v2_api_checked = False if self.rbd is not None: self.RBD_FEATURE_LAYERING = self.rbd.RBD_FEATURE_LAYERING @@ -268,6 +269,22 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, 'max_over_subscription_ratio', 'volume_dd_blocksize') return RBD_OPTS + additional_opts + def _show_msg_check_clone_v2_api(self, volume_name): + if not self._clone_v2_api_checked: + self._clone_v2_api_checked = True + with RBDVolumeProxy(self, volume_name) as volume: + try: + if (volume.volume.op_features() & + self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT): + LOG.info('Using v2 Clone API') + return + except AttributeError: + pass + LOG.warning('Not using v2 clone API, please upgrade to' + ' mimic+ and set the OSD minimum client' + ' compat version to mimic for better' + ' performance, fewer deletion issues') + def _get_target_config(self, target_id): """Get a replication target from known replication targets.""" for target in self._replication_targets: @@ -1000,6 +1017,8 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, self._flatten(self.configuration.rbd_pool, volume.name) if int(volume.size): self._resize(volume) + + self._show_msg_check_clone_v2_api(snapshot.volume_name) return volume_update def _delete_backup_snaps(self, rbd_image):