Merge pull request #43948 from guits/wip-53278-pacific

pacific: ceph-volume: fix bug with miscalculation of required db/wal slot size for VGs with multiple PVs

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

@@ -112,35 +112,55 @@ def get_physical_fast_allocs(devices, type_, fast_slots_per_device, new_osds, ar
         requested_size = get_size_fct(lv_format=False)
 
     ret = []
-    for dev in devices:
-        if not dev.available_lvm:
-            continue
-        # any LV present is considered a taken slot
-        occupied_slots = len(dev.lvs)
-        # this only looks at the first vg on device, unsure if there is a better
-        # way
-        dev_size = dev.vg_size[0]
-        abs_size = disk.Size(b=int(dev_size / requested_slots))
-        free_size = dev.vg_free[0]
-        relative_size = int(abs_size) / dev_size
-        if requested_size:
-            if requested_size <= abs_size:
-                abs_size = requested_size
-                relative_size = int(abs_size) / dev_size
-            else:
-                mlogger.error(
-                    '{} was requested for {}, but only {} can be fulfilled'.format(
-                        requested_size,
-                        '{}_size'.format(type_),
-                        abs_size,
-                    ))
-                exit(1)
-        while abs_size <= free_size and len(ret) < new_osds and occupied_slots < fast_slots_per_device:
-            free_size -= abs_size.b
-            occupied_slots += 1
-            ret.append((dev.path, relative_size, abs_size, requested_slots))
+    vg_device_map = group_devices_by_vg(devices)
+    for vg_devices in vg_device_map.values():
+        for dev in vg_devices:
+            if not dev.available_lvm:
+                continue
+            # any LV present is considered a taken slot
+            occupied_slots = len(dev.lvs)
+            # prior to v15.2.8, db/wal deployments were grouping multiple fast devices into single VGs - we need to
+            # multiply requested_slots (per device) by the number of devices in the VG in order to ensure that
+            # abs_size is calculated correctly from vg_size
+            slots_for_vg = len(vg_devices) * requested_slots
+            dev_size = dev.vg_size[0]
+            # this only looks at the first vg on device, unsure if there is a better
+            # way
+            abs_size = disk.Size(b=int(dev_size / slots_for_vg))
+            free_size = dev.vg_free[0]
+            relative_size = int(abs_size) / dev_size
+            if requested_size:
+                if requested_size <= abs_size:
+                    abs_size = requested_size
+                    relative_size = int(abs_size) / dev_size
+                else:
+                    mlogger.error(
+                        '{} was requested for {}, but only {} can be fulfilled'.format(
+                            requested_size,
+                            '{}_size'.format(type_),
+                            abs_size,
+                        ))
+                    exit(1)
+            while abs_size <= free_size and len(ret) < new_osds and occupied_slots < fast_slots_per_device:
+                free_size -= abs_size.b
+                occupied_slots += 1
+                ret.append((dev.path, relative_size, abs_size, requested_slots))
     return ret
 
+def group_devices_by_vg(devices):
+    result = dict()
+    result['unused_devices'] = []
+    for dev in devices:
+        if len(dev.vgs) > 0:
+            # already using assumption that a PV only belongs to single VG in other places
+            vg_name = dev.vgs[0].name
+            if vg_name in result:
+                result[vg_name].append(dev)
+            else:
+                result[vg_name] = [dev]
+        else:
+            result['unused_devices'].append(dev)
+    return result
 
 def get_lvm_fast_allocs(lvs):
     return [("{}/{}".format(d.vg_name, d.lv_name), 100.0,

src/ceph-volume/ceph_volume/tests/conftest.py

@@ -63,6 +63,9 @@ def mock_lv():
 def mock_devices_available():
     dev = create_autospec(device.Device)
     dev.path = '/dev/foo'
+    dev.vg_name = 'vg_foo'
+    dev.lv_name = 'lv_foo'
+    dev.vgs = [lvm.VolumeGroup(vg_name=dev.vg_name, lv_name=dev.lv_name)]
     dev.available_lvm = True
     dev.vg_size = [21474836480]
     dev.vg_free = dev.vg_size
@@ -73,6 +76,9 @@ def mock_device_generator():
     def mock_device():
         dev = create_autospec(device.Device)
         dev.path = '/dev/foo'
+        dev.vg_name = 'vg_foo'
+        dev.lv_name = 'lv_foo'
+        dev.vgs = [lvm.VolumeGroup(vg_name=dev.vg_name, lv_name=dev.lv_name)]
         dev.available_lvm = True
         dev.vg_size = [21474836480]
         dev.vg_free = dev.vg_size

src/ceph-volume/ceph_volume/tests/functional/Vagrantfile

@@ -390,7 +390,7 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
         # always make /dev/sd{a/b/c/d} so that CI can ensure that
         # virtualbox and libvirt will have the same devices to use for OSDs
         (0..3).each do |d|
-          lv.storage :file, :device => "sd#{driverletters[d]}", :size => '12G'
+          lv.storage :file, :device => "sd#{driverletters[d]}", :size => '100G'
         end
         lv.memory = MEMORY
         lv.random_hostname = true

src/ceph-volume/ceph_volume/tests/functional/batch/playbooks/setup_mixed_type.yml

@@ -1,4 +1,4 @@
-
+---
 - hosts: osds
   become: yes
   tasks:
@@ -119,8 +119,8 @@
         - /opt/vdd/loop0_nvme0
         - /opt/vde/loop1_nvme1
 
-    - name: create 11GB sparse files for NVMe
-      command: "fallocate -l 11G {{ item }}"
+    - name: create 20GB sparse files for NVMe
+      command: "fallocate -l 20G {{ item }}"
       loop:
         - /opt/vdd/loop0_nvme0
         - /opt/vde/loop1_nvme1