From 75c91a8c6f37a38d69d5da8b1e7d49d9c636230b Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Wed, 23 Feb 2022 09:36:29 +0100 Subject: [PATCH] ceph-volume: abort when passed devices have partitions ceph-volume doesn't prevent from using db and/or wal devices with existing partitions on them. This can lead to a data loss situation. Fixes: https://tracker.ceph.com/issues/54376 Signed-off-by: Guillaume Abrioux --- src/ceph-volume/ceph_volume/devices/lvm/common.py | 3 +++ .../ceph_volume/tests/devices/lvm/test_batch.py | 1 + .../ceph_volume/tests/util/test_arg_validators.py | 8 ++++++++ src/ceph-volume/ceph_volume/util/arg_validators.py | 4 ++-- src/ceph-volume/ceph_volume/util/device.py | 11 +++++++++++ 5 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/ceph-volume/ceph_volume/devices/lvm/common.py b/src/ceph-volume/ceph_volume/devices/lvm/common.py index e09f7f0db91..614be0af6ad 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/common.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/common.py @@ -92,6 +92,7 @@ bluestore_args = { '--block.db': { 'dest': 'block_db', 'help': 'Path to bluestore block.db logical volume or device', + 'type': arg_validators.ValidDevice(as_string=True), }, '--block.db-size': { 'dest': 'block_db_size', @@ -109,6 +110,7 @@ bluestore_args = { '--block.wal': { 'dest': 'block_wal', 'help': 'Path to bluestore block.wal logical volume or device', + 'type': arg_validators.ValidDevice(as_string=True), }, '--block.wal-size': { 'dest': 'block_wal_size', @@ -132,6 +134,7 @@ filestore_args = { }, '--journal': { 'help': 'A logical volume (vg_name/lv_name), or path to a device', + 'type': arg_validators.ValidDevice(as_string=True), }, '--journal-size': { 'help': 'Size of journal LV in case a raw block device was passed in --journal', diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py index 1bb1858aec7..265a9b84ef4 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py @@ -33,6 +33,7 @@ class TestBatch(object): mocked_device.return_value = MagicMock( is_partition=True, has_gpt_headers=False, + has_partitions=False, ) with pytest.raises(ArgumentError): arg_validators.ValidBatchDevice()('foo') diff --git a/src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py b/src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py index b76f8f6e417..13dff80bfe9 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py @@ -3,6 +3,7 @@ import pytest import os from ceph_volume import exceptions from ceph_volume.util import arg_validators +from mock.mock import patch, PropertyMock class TestOSDPath(object): @@ -88,6 +89,13 @@ class TestValidDevice(object): with pytest.raises(argparse.ArgumentError): self.validator('/device/does/not/exist') + @patch('ceph_volume.util.arg_validators.Device.has_partitions', new_callable=PropertyMock, return_value=True) + @patch('ceph_volume.util.arg_validators.Device.exists', new_callable=PropertyMock, return_value=True) + @patch('ceph_volume.api.lvm.get_single_lv', return_value=None) + def test_dev_has_partitions(self, m_get_single_lv, m_exists, m_has_partitions, fake_call): + with pytest.raises(RuntimeError): + self.validator('/dev/foo') + class TestValidFraction(object): diff --git a/src/ceph-volume/ceph_volume/util/arg_validators.py b/src/ceph-volume/ceph_volume/util/arg_validators.py index 38a01198516..9793457d4e5 100644 --- a/src/ceph-volume/ceph_volume/util/arg_validators.py +++ b/src/ceph-volume/ceph_volume/util/arg_validators.py @@ -41,10 +41,10 @@ class ValidDevice(object): # __init__ elif device.has_gpt_headers and not self.gpt_ok: error = "GPT headers found, they must be removed on: %s" % dev_path - + if device.has_partitions: + raise RuntimeError("Device {} has partitions.".format(dev_path)) if error: raise argparse.ArgumentError(None, error) - return device diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py index 20ed448c22f..c30a094b6ad 100644 --- a/src/ceph-volume/ceph_volume/util/device.py +++ b/src/ceph-volume/ceph_volume/util/device.py @@ -476,6 +476,15 @@ class Device(object): vg_free -= extent_size return [vg_free] + @property + def has_partitions(self): + ''' + Boolean to determine if a given device has partitions. + ''' + if self.sys_api.get('partitions'): + return True + return False + def _check_generic_reject_reasons(self): reasons = [ ('removable', 1, 'removable'), @@ -514,6 +523,8 @@ class Device(object): if self.has_gpt_headers: rejected.append('Has GPT headers') + if self.has_partitions: + rejected.append('Has partitions') return rejected def _check_lvm_reject_reasons(self):