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 <gabrioux@redhat.com>

diff --git a/src/ceph-volume/ceph_volume/devices/lvm/common.py b/src/ceph-volume/ceph_volume/devices/lvm/common.py
index e09f7f0db91acef22646c83e6d7174f5221b0216..614be0af6ad6115af7c91b4e5f64b00087840e90 100644 (file)
--- 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 1bb1858aec77c2f8fc6f3268ea6a4d205b32b689..265a9b84ef4b4d556d27b42afe6c02c4084b3505 100644 (file)
--- 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 b76f8f6e4176f567e2f8651e6fcea4f0fe34b258..13dff80bfe957d1a7b0f5ee1837150a96bbd0741 100644 (file)
--- 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 38a0119851615e1eccd0be479dd5a4d0a376b783..9793457d4e55dbcb3007b5a36daa5fc0301f357d 100644 (file)
--- 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 20ed448c22f7072ebb090d77fe1bbd022f44379f..c30a094b6adeadbc75789b7c2f872d959b9f2a15 100644 (file)
--- 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):