ceph-volume: fix nvme preformat wiping VG on osds-per-device > 1

prepare_data_device() was calling nvme_utils.preformat() unconditionally
for every OSD slot. With --osds-per-device N, this means the second osd
reformats the device after the first OSD had already created its vg
on it which makes vgcreate fail with "device has a signature".

this commit fixes this issue by checking api.get_device_vgs() before
preformatting: if a ceph VG already exists on the device, skip the format.
This mirrors the logic create_lv() already uses to reuse an existing VG.

Fixes: https://tracker.ceph.com/issues/78615
Signed-off-by: Guillaume Abrioux <gabrioux@ibm.com>

diff --git a/src/ceph-volume/ceph_volume/objectstore/lvm.py b/src/ceph-volume/ceph_volume/objectstore/lvm.py
index b8f63853001041d58cc3711ae77c3e54fc82f719..aae347406ced0bd5fd2bf82cd457aa48d6bb224f 100644 (file)
--- a/src/ceph-volume/ceph_volume/objectstore/lvm.py
+++ b/src/ceph-volume/ceph_volume/objectstore/lvm.py
@@ -86,7 +86,10 @@ class Lvm(BaseObjectStore):
         if disk.is_partition(device) or disk.is_device(device):
             if device_type == 'block' and self.objectstore == 'bluestore':
                 # NVMe preformat already discards, skip mkfs discard.
-                if nvme_utils.preformat(device):
+                # Only preformat when no ceph VG already exists on the device.
+                # With --osds-per-device > 1, the first OSD creates the VG;
+                # subsequent OSDs reuse it, so reformatting would destroy it.
+                if not api.get_device_vgs(device, 'ceph') and nvme_utils.preformat(device):
                     self.skip_mkfs_discard = True
             # we must create a vg, and then a single lv
             lv_name_prefix = "osd-{}".format(device_type)

diff --git a/src/ceph-volume/ceph_volume/tests/objectstore/test_lvm.py b/src/ceph-volume/ceph_volume/tests/objectstore/test_lvm.py
index 31662750ebccc0d00a1bd0559c8b26aaa421d452..98382d3f56890523abc87b88d3d62cf5fc5b0387 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/objectstore/test_lvm.py
+++ b/src/ceph-volume/ceph_volume/tests/objectstore/test_lvm.py
@@ -150,11 +150,13 @@ class TestLvm:
         assert self.lvm.args.data_size == 102400
 
     @patch('ceph_volume.objectstore.lvm.nvme_utils.preformat', Mock(return_value=True))
+    @patch('ceph_volume.objectstore.lvm.api.get_device_vgs', Mock(return_value=[]))
     @patch('ceph_volume.util.disk.is_device', Mock(return_value=True))
     @patch('ceph_volume.api.lvm.create_lv')
     def test_prepare_data_device_preformats_nvme_and_skips_mkfs_discard(self,
                                                                        m_create_lv: MagicMock,
                                                                        factory: Callable[..., Namespace]) -> None:
+        """No existing VG on a fresh device: preformat runs, discard flag is set."""
         args = factory(data='/dev/nvme0n1',
                        data_slots=1,
                        data_size=0)
@@ -168,6 +170,30 @@ class TestLvm:
         self.lvm.prepare_data_device('block', 'abcd')
         assert self.lvm.skip_mkfs_discard is True
 
+    @patch('ceph_volume.util.disk.is_device', Mock(return_value=True))
+    @patch('ceph_volume.api.lvm.create_lv')
+    def test_prepare_data_device_skips_preformat_when_vg_exists(self,
+                                                                 m_create_lv: MagicMock,
+                                                                 factory: Callable[..., Namespace]) -> None:
+        """Existing ceph VG on device (--osds-per-device > 1): preformat must not run."""
+        m_preformat = Mock(return_value=True)
+        fake_vg = Mock()
+        with patch('ceph_volume.objectstore.lvm.nvme_utils.preformat', m_preformat), \
+             patch('ceph_volume.objectstore.lvm.api.get_device_vgs', return_value=[fake_vg]):
+            args = factory(data='/dev/nvme0n1',
+                           data_slots=1,
+                           data_size=0)
+            self.lvm.args = args
+            self.lvm.objectstore = 'bluestore'
+            m_create_lv.return_value = Volume(lv_name='lv_foo',
+                                              lv_path='/fake-path',
+                                              vg_name='vg_foo',
+                                              lv_tags='',
+                                              lv_uuid='abcd')
+            self.lvm.prepare_data_device('block', 'abcd')
+            m_preformat.assert_not_called()
+            assert self.lvm.skip_mkfs_discard is False
+
     @patch('ceph_volume.util.disk.is_device', Mock(return_value=False))
     @patch('ceph_volume.util.disk.is_partition', Mock(return_value=False))
     def test_prepare_data_device_fails(self, factory):