From 2c5a8c3b4066dd2aca47d719c9723850ce5f96fc Mon Sep 17 00:00:00 2001 From: Jan Fajerski Date: Wed, 12 Feb 2020 14:47:37 +0100 Subject: [PATCH] ceph-volume: use get_device_vgs in has_common_vg Fixes: https://tracker.ceph.com/issues/44099 Signed-off-by: Jan Fajerski --- .../devices/lvm/strategies/strategies.py | 11 ++--- .../devices/lvm/strategies/validators.py | 9 ++-- .../devices/lvm/strategies/test_filestore.py | 48 ++++++++----------- 3 files changed, 27 insertions(+), 41 deletions(-) diff --git a/src/ceph-volume/ceph_volume/devices/lvm/strategies/strategies.py b/src/ceph-volume/ceph_volume/devices/lvm/strategies/strategies.py index ad69ab7b32204..75d748fa8123c 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/strategies/strategies.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/strategies/strategies.py @@ -1,5 +1,6 @@ import json from ceph_volume.util.prepare import osd_id_available +from ceph_volume.api.lvm import get_device_vgs class Strategy(object): @@ -64,10 +65,6 @@ class MixedStrategy(Strategy): def get_common_vg(self, devs): # find all the vgs associated with the current device for dev in devs: - for pv in dev.pvs_api: - vg = self.system_vgs.get(vg_name=pv.vg_name) - if not vg: - continue - # this should give us just one VG, it would've been caught by - # the validator otherwise - return vg + vgs = get_device_vgs(dev.abspath) + if vgs: + return vgs[0] diff --git a/src/ceph-volume/ceph_volume/devices/lvm/strategies/validators.py b/src/ceph-volume/ceph_volume/devices/lvm/strategies/validators.py index f67d6ffaa9736..e631e10d3208d 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/strategies/validators.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/strategies/validators.py @@ -43,14 +43,13 @@ def has_common_vg(ssd_devices): Ensure that devices have a common VG between them """ msg = 'Could not find a common VG between devices: %s' - system_vgs = lvm.VolumeGroups() ssd_vgs = {} for ssd_device in ssd_devices: - for pv in ssd_device.pvs_api: - vg = system_vgs.get(vg_name=pv.vg_name) - if not vg: - continue + vgs = lvm.get_device_vgs(ssd_device.abspath) + if not vgs: + continue + for vg in vgs: try: ssd_vgs[vg.name].append(ssd_device.abspath) except KeyError: diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_filestore.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_filestore.py index 710c09288b77a..5bfc07086b4c3 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_filestore.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_filestore.py @@ -1,4 +1,5 @@ import pytest +from mock.mock import patch from ceph_volume.devices.lvm.strategies import filestore from ceph_volume.api import lvm @@ -155,21 +156,23 @@ def test_hdd_device_is_lvm_member_fails(self, stub_vgs, fakedevice, factory, con filestore.MixedType.with_auto_devices(args, devices) assert 'Unable to use device, already a member of LVM' in str(error.value) - def test_ssd_is_lvm_member_doesnt_fail(self, volumes, stub_vgs, fakedevice, factory, conf_ceph): - # fast PV, because ssd is an LVM member - CephPV = lvm.PVolume(vg_name='fast', pv_name='/dev/sda', pv_tags='') + @patch('ceph_volume.devices.lvm.strategies.strategies.MixedStrategy.get_common_vg') + def test_ssd_is_lvm_member_doesnt_fail(self, + patched_get_common_vg, + volumes, + fakedevice, + factory, + conf_ceph): ssd = fakedevice( - used_by_ceph=False, is_lvm_member=True, rotational=False, sys_api=dict(size=6073740000), pvs_api=[CephPV] + used_by_ceph=False, is_lvm_member=True, rotational=False, sys_api=dict(size=6073740000) ) hdd = fakedevice(used_by_ceph=False, is_lvm_member=False, rotational=True, sys_api=dict(size=6073740000)) - # when get_api_vgs() gets called, it will return this one VG - stub_vgs([ - dict( + vg = lvm.VolumeGroup( vg_name='fast', lv_name='foo', lv_path='/dev/vg/foo', lv_tags="ceph.type=data", - vg_extent_size=1024*1024*1024, vg_free_count=7 - ) - ]) + vg_extent_size=1024*1024*1024, vg_free_count=7) + patched_get_common_vg.return_value = vg + conf_ceph(get_safe=lambda *a: '5120') args = factory(filtered_devices=[], osds_per_device=1, @@ -180,28 +183,16 @@ def test_ssd_is_lvm_member_doesnt_fail(self, volumes, stub_vgs, fakedevice, fact assert result['journal']['percentage'] == 71 assert result['journal']['human_readable_size'] == '5.00 GB' - def test_no_common_vg(self, volumes, stub_vgs, fakedevice, factory, conf_ceph): - # fast PV, because ssd is an LVM member - CephPV1 = lvm.PVolume(vg_name='fast1', pv_name='/dev/sda', pv_tags='') - CephPV2 = lvm.PVolume(vg_name='fast2', pv_name='/dev/sdb', pv_tags='') + @patch('ceph_volume.api.lvm.get_device_vgs') + def test_no_common_vg(self, patched_get_device_vgs, volumes, fakedevice, factory, conf_ceph): + patched_get_device_vgs.side_effect = lambda x: [lvm.VolumeGroup(vg_name='{}'.format(x[-1]), vg_tags='')] ssd1 = fakedevice( - used_by_ceph=False, is_lvm_member=True, rotational=False, sys_api=dict(size=6073740000), pvs_api=[CephPV1] + used_by_ceph=False, is_lvm_member=True, rotational=False, sys_api=dict(size=6073740000) ) ssd2 = fakedevice( - used_by_ceph=False, is_lvm_member=True, rotational=False, sys_api=dict(size=6073740000), pvs_api=[CephPV2] + used_by_ceph=False, is_lvm_member=True, rotational=False, sys_api=dict(size=6073740000) ) hdd = fakedevice(used_by_ceph=False, is_lvm_member=False, rotational=True, sys_api=dict(size=6073740000)) - # when get_api_vgs() gets called, it will return this one VG - stub_vgs([ - dict( - vg_free='7g', vg_name='fast1', lv_name='foo', - lv_path='/dev/vg/fast1', lv_tags="ceph.type=data" - ), - dict( - vg_free='7g', vg_name='fast2', lv_name='foo', - lv_path='/dev/vg/fast2', lv_tags="ceph.type=data" - ) - ]) conf_ceph(get_safe=lambda *a: '5120') args = factory(filtered_devices=[], osds_per_device=1, @@ -209,8 +200,7 @@ def test_no_common_vg(self, volumes, stub_vgs, fakedevice, factory, conf_ceph): devices = [ssd1, ssd2, hdd] with pytest.raises(RuntimeError) as error: filestore.MixedType.with_auto_devices(args, devices) - - assert 'Could not find a common VG between devices' in str(error.value) + assert 'Could not find a common VG between devices' in str(error.value) def test_ssd_device_fails_multiple_osds(self, stub_vgs, fakedevice, factory, conf_ceph): conf_ceph(get_safe=lambda *a: '15120')