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>
(cherry picked from commit 75c91a8c6f37a38d69d5da8b1e7d49d9c636230b)

diff --git a/src/ceph-volume/ceph_volume/devices/lvm/common.py b/src/ceph-volume/ceph_volume/devices/lvm/common.py
index 05f83383f0ea13300e8a06abc117afa55295f914..30d627e03814806542e4ec7f5a669c76f5f669e4 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 2a334dad95d078a61a23984527025be729632759..0f90a2369a88d90995dd06eb424049b5728fbb03 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 1e8a49e2624c246b0a2f9ff03008dc00283eee8a..b0111accc72cde069b04d81fcf7ea5893ee02053 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):
@@ -87,3 +88,11 @@ class TestValidDevice(object):
     def test_path_is_invalid(self, fake_call, patch_bluestore_label):
         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')
+

diff --git a/src/ceph-volume/ceph_volume/util/arg_validators.py b/src/ceph-volume/ceph_volume/util/arg_validators.py
index 94cb4f691dbee180a0f095b95c989a32f8f1945f..5a7e82f071745e52b72b6743a2a82f4a08d6bf11 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/arg_validators.py
+++ b/src/ceph-volume/ceph_volume/util/arg_validators.py
@@ -37,10 +37,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 958872ba3563bc7ea917fa0f0426bea401dea7b8..32edc1541defa011d96531d16ecf5f0b865c0c9d 100644 (file)
--- a/src/ceph-volume/ceph_volume/util/device.py
+++ b/src/ceph-volume/ceph_volume/util/device.py
@@ -474,6 +474,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'),
@@ -512,6 +521,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):