mimic ceph-volume: reject devices that have existing GPT headers

src/ceph-volume/ceph_volume/devices/lvm/common.py

@@ -52,7 +52,7 @@ def common_parser(prog, description):
     required_group.add_argument(
         '--data',
         required=True,
-        type=arg_validators.LVPath(),
+        type=arg_validators.ValidDevice(as_string=True),
         help='OSD data path. A physical device or logical volume',
     )
 

src/ceph-volume/ceph_volume/tests/devices/lvm/test_create.py

@@ -17,14 +17,16 @@ def test_main_shows_full_help(self, capsys):
         assert 'Use the bluestore objectstore' in stdout
         assert 'A physical device or logical' in stdout
 
-    def test_excludes_filestore_bluestore_flags(self, capsys):
+    def test_excludes_filestore_bluestore_flags(self, capsys, device_info):
+        device_info()
         with pytest.raises(SystemExit):
             lvm.create.Create(argv=['--data', '/dev/sdfoo', '--filestore', '--bluestore']).main()
         stdout, sterr = capsys.readouterr()
         expected = 'Cannot use --filestore (filestore) with --bluestore (bluestore)'
         assert expected in stdout
 
-    def test_excludes_other_filestore_bluestore_flags(self, capsys):
+    def test_excludes_other_filestore_bluestore_flags(self, capsys, device_info):
+        device_info()
         with pytest.raises(SystemExit):
             lvm.create.Create(argv=[
                 '--bluestore', '--data', '/dev/sdfoo',
@@ -34,7 +36,8 @@ def test_excludes_other_filestore_bluestore_flags(self, capsys):
         expected = 'Cannot use --bluestore (bluestore) with --journal (filestore)'
         assert expected in stdout
 
-    def test_excludes_block_and_journal_flags(self, capsys):
+    def test_excludes_block_and_journal_flags(self, capsys, device_info):
+        device_info()
         with pytest.raises(SystemExit):
             lvm.create.Create(argv=[
                 '--bluestore', '--data', '/dev/sdfoo', '--block.db', 'vg/ceph1',

src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py

@@ -47,14 +47,16 @@ def test_main_shows_full_help(self, capsys):
         assert 'Use the bluestore objectstore' in stdout
         assert 'A physical device or logical' in stdout
 
-    def test_excludes_filestore_bluestore_flags(self, capsys):
+    def test_excludes_filestore_bluestore_flags(self, capsys, device_info):
+        device_info()
         with pytest.raises(SystemExit):
             lvm.prepare.Prepare(argv=['--data', '/dev/sdfoo', '--filestore', '--bluestore']).main()
         stdout, stderr = capsys.readouterr()
         expected = 'Cannot use --filestore (filestore) with --bluestore (bluestore)'
         assert expected in stdout
 
-    def test_excludes_other_filestore_bluestore_flags(self, capsys):
+    def test_excludes_other_filestore_bluestore_flags(self, capsys, device_info):
+        device_info()
         with pytest.raises(SystemExit):
             lvm.prepare.Prepare(argv=[
                 '--bluestore', '--data', '/dev/sdfoo',
@@ -64,7 +66,8 @@ def test_excludes_other_filestore_bluestore_flags(self, capsys):
         expected = 'Cannot use --bluestore (bluestore) with --journal (filestore)'
         assert expected in stdout
 
-    def test_excludes_block_and_journal_flags(self, capsys):
+    def test_excludes_block_and_journal_flags(self, capsys, device_info):
+        device_info()
         with pytest.raises(SystemExit):
             lvm.prepare.Prepare(argv=[
                 '--bluestore', '--data', '/dev/sdfoo', '--block.db', 'vg/ceph1',
@@ -74,8 +77,9 @@ def test_excludes_block_and_journal_flags(self, capsys):
         expected = 'Cannot use --block.db (bluestore) with --journal (filestore)'
         assert expected in stdout
 
-    def test_journal_is_required_with_filestore(self, is_root, monkeypatch):
-        monkeypatch.setattr('os.path.exists', lambda x: True)
+    def test_journal_is_required_with_filestore(self, is_root, monkeypatch, device_info):
+        monkeypatch.setattr("os.path.exists", lambda path: True)
+        device_info()
         with pytest.raises(SystemExit) as error:
             lvm.prepare.Prepare(argv=['--filestore', '--data', '/dev/sdfoo']).main()
         expected = '--journal is required when using --filestore'

src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py

@@ -4,31 +4,6 @@
 from ceph_volume.util import arg_validators
 
 
-invalid_lv_paths = [
-    '', 'lv_name', '/lv_name', 'lv_name/',
-    '/dev/lv_group/lv_name'
-]
-
-
-class TestLVPath(object):
-
-    def setup(self):
-        self.validator = arg_validators.LVPath()
-
-    @pytest.mark.parametrize('path', invalid_lv_paths)
-    def test_no_slash_is_an_error(self, path):
-        with pytest.raises(argparse.ArgumentError):
-            self.validator(path)
-
-    def test_is_valid(self):
-        path = 'vg/lv'
-        assert self.validator(path) == path
-
-    def test_abspath_is_valid(self):
-        path = '/'
-        assert self.validator(path) == path
-
-
 class TestOSDPath(object):
 
     def setup(self):

src/ceph-volume/ceph_volume/tests/util/test_device.py

@@ -13,7 +13,7 @@ def test_sys_api(self, device_info):
         assert "foo" in disk.sys_api
 
     def test_is_lv(self, device_info):
-        data = {"lv_path": "vg/lv", "vg_name": "vg"}
+        data = {"lv_path": "vg/lv", "vg_name": "vg", "name": "lv"}
         device_info(lv=data)
         disk = device.Device("vg/lv")
         assert disk.is_lv

src/ceph-volume/ceph_volume/util/arg_validators.py

@@ -6,51 +6,27 @@
 from ceph_volume.util.device import Device
 
 
-class LVPath(object):
-    """
-    A simple validator to ensure that a logical volume is specified like::
-
-        <vg name>/<lv name>
-
-    Or a full path to a device, like ``/dev/sda``
+class ValidDevice(object):
 
-    Because for LVM it is better to be specific on what group does an lv
-    belongs to.
-    """
+    def __init__(self, as_string=False):
+        self.as_string = as_string
 
     def __call__(self, string):
+        device = Device(string)
         error = None
-        if string.startswith('/'):
-            if not os.path.exists(string):
-                error = "Argument (device) does not exist: %s" % string
-                raise argparse.ArgumentError(None, error)
-            else:
-                return string
-        try:
-            vg, lv = string.split('/')
-        except ValueError:
-            error = "Logical volume must be specified as 'volume_group/logical_volume' but got: %s" % string
-            raise argparse.ArgumentError(None, error)
-
-        if not vg:
-            error = "Didn't specify a volume group like 'volume_group/logical_volume', got: %s" % string
-        if not lv:
-            error = "Didn't specify a logical volume like 'volume_group/logical_volume', got: %s" % string
+        if not device.exists:
+            error = "Unable to proceed with non-existing device: %s" % string
+        elif device.has_gpt_headers:
+            error = "GPT headers found, they must be removed on: %s" % string
 
         if error:
             raise argparse.ArgumentError(None, error)
-        return string
-
-
-class ValidDevice(object):
-
-    def __call__(self, string):
-        device = Device(string)
-        if not device.exists:
-            raise argparse.ArgumentError(
-                None, "Unable to proceed with non-existing device: %s" % string
-            )
 
+        if self.as_string:
+            if device.is_lv:
+                # all codepaths expect an lv path to be returned in this format
+                return "{}/{}".format(device.vg_name, device.lv_name)
+            return string
         return device
 
 

src/ceph-volume/ceph_volume/util/device.py

@@ -70,6 +70,7 @@ def __init__(self, path):
         self.lv_api = None
         self.lvs = []
         self.vg_name = None
+        self.lv_name = None
         self.pvs_api = []
         self.disk_api = {}
         self.blkid_api = {}
@@ -108,6 +109,7 @@ def _parse(self):
             self.lvs = [lv]
             self.abspath = lv.lv_path
             self.vg_name = lv.vg_name
+            self.lv_name = lv.name
         else:
             dev = disk.lsblk(self.path)
             self.blkid_api = disk.blkid(self.path)
@@ -211,6 +213,10 @@ def _get_pv_paths(self):
     def exists(self):
         return os.path.exists(self.abspath)
 
+    @property
+    def has_gpt_headers(self):
+        return self.blkid_api.get("PTTYPE") == "gpt"
+
     @property
     def rotational(self):
         return self.sys_api['rotational'] == '1'

src/ceph-volume/ceph_volume/util/disk.py

@@ -51,6 +51,7 @@ def _blkid_parser(output):
         'TYPE': 'TYPE',
         'PART_ENTRY_NAME': 'PARTLABEL',
         'PART_ENTRY_UUID': 'PARTUUID',
+        'PTTYPE': 'PTTYPE',
     }
 
     for pair in pairs: