From 9780d28028a40ecbfc327dab779e8a37c9aaed51 Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Mon, 8 Mar 2021 09:59:26 +0100 Subject: [PATCH] ceph-volume: `get_first_*()` refactor As indicated by commit 17957d9beb42a04b8f180ccb7ba07d43179a41d3 those fuctions were meant to avoid writing something like following: ``` lvs = get_lvs() if len(lvs) >= 1: lvs = lv[0] ``` Those functions should return `None` if 0 or more than 1 item is returned. The current name of these functions are confusing and can lead to thinking that we just want the first item returned, even though it returns more than 1 item, let's rename them to `get_single_pv()`, `get_single_vg()` and `get_single_lv()` Closes: https://tracker.ceph.com/issues/49643 Signed-off-by: Guillaume Abrioux (cherry picked from commit a5e4216b49704783c55fb83b3ae6dde35b0082ad) --- src/ceph-volume/ceph_volume/api/lvm.py | 54 +++-- .../ceph_volume/devices/lvm/prepare.py | 18 +- .../ceph_volume/devices/lvm/zap.py | 6 +- .../ceph_volume/devices/simple/scan.py | 2 +- .../ceph_volume/tests/api/test_lvm.py | 227 ++++++++++-------- src/ceph-volume/ceph_volume/tests/conftest.py | 2 +- .../tests/devices/lvm/test_activate.py | 2 +- .../tests/devices/lvm/test_listing.py | 4 +- .../tests/devices/lvm/test_migrate.py | 98 ++++---- .../tests/devices/lvm/test_prepare.py | 12 +- src/ceph-volume/ceph_volume/util/device.py | 6 +- 11 files changed, 234 insertions(+), 197 deletions(-) diff --git a/src/ceph-volume/ceph_volume/api/lvm.py b/src/ceph-volume/ceph_volume/api/lvm.py index c6199fc5fb754..3945c4ffcb923 100644 --- a/src/ceph-volume/ceph_volume/api/lvm.py +++ b/src/ceph-volume/ceph_volume/api/lvm.py @@ -376,8 +376,12 @@ class PVolume(object): self.set_tag(k, v) # after setting all the tags, refresh them for the current object, use the # pv_* identifiers to filter because those shouldn't change - pv_object = self.get_first_pv(filter={'pv_name': self.pv_name, - 'pv_uuid': self.pv_uuid}) + pv_object = self.get_single_pv(filter={'pv_name': self.pv_name, + 'pv_uuid': self.pv_uuid}) + + if not pv_object: + raise RuntimeError('No PV was found.') + self.tags = pv_object.tags def set_tag(self, key, value): @@ -471,15 +475,21 @@ def get_pvs(fields=PV_FIELDS, filters='', tags=None): return [PVolume(**pv_report) for pv_report in pvs_report] -def get_first_pv(fields=PV_FIELDS, filters=None, tags=None): +def get_single_pv(fields=PV_FIELDS, filters=None, tags=None): """ - Wrapper of get_pv meant to be a convenience method to avoid the phrase:: + Wrapper of get_pvs() meant to be a convenience method to avoid the phrase:: pvs = get_pvs() if len(pvs) >= 1: pv = pvs[0] """ pvs = get_pvs(fields=fields, filters=filters, tags=tags) - return pvs[0] if len(pvs) > 0 else [] + + if len(pvs) == 0: + return None + if len(pvs) > 1: + raise RuntimeError('Filters {} matched more than 1 PV present on this host.'.format(str(filters))) + + return pvs[0] ################################ @@ -650,7 +660,7 @@ def create_vg(devices, name=None, name_prefix=None): name] + devices ) - return get_first_vg(filters={'vg_name': name}) + return get_single_vg(filters={'vg_name': name}) def extend_vg(vg, devices): @@ -674,7 +684,7 @@ def extend_vg(vg, devices): vg.name] + devices ) - return get_first_vg(filters={'vg_name': vg.name}) + return get_single_vg(filters={'vg_name': vg.name}) def reduce_vg(vg, devices): @@ -696,7 +706,7 @@ def reduce_vg(vg, devices): vg.name] + devices ) - return get_first_vg(filter={'vg_name': vg.name}) + return get_single_vg(filter={'vg_name': vg.name}) def remove_vg(vg_name): @@ -742,15 +752,21 @@ def get_vgs(fields=VG_FIELDS, filters='', tags=None): return [VolumeGroup(**vg_report) for vg_report in vgs_report] -def get_first_vg(fields=VG_FIELDS, filters=None, tags=None): +def get_single_vg(fields=VG_FIELDS, filters=None, tags=None): """ - Wrapper of get_vg meant to be a convenience method to avoid the phrase:: + Wrapper of get_vgs() meant to be a convenience method to avoid the phrase:: vgs = get_vgs() if len(vgs) >= 1: vg = vgs[0] """ vgs = get_vgs(fields=fields, filters=filters, tags=tags) - return vgs[0] if len(vgs) > 0 else [] + + if len(vgs) == 0: + return None + if len(vgs) > 1: + raise RuntimeError('Filters {} matched more than 1 VG present on this host.'.format(str(filters))) + + return vgs[0] def get_device_vgs(device, name_prefix=''): @@ -970,7 +986,7 @@ def create_lv(name_prefix, ] process.run(command) - lv = get_first_lv(filters={'lv_name': name, 'vg_name': vg.vg_name}) + lv = get_single_lv(filters={'lv_name': name, 'vg_name': vg.vg_name}) if tags is None: tags = { @@ -1095,15 +1111,21 @@ def get_lvs(fields=LV_FIELDS, filters='', tags=None): return [Volume(**lv_report) for lv_report in lvs_report] -def get_first_lv(fields=LV_FIELDS, filters=None, tags=None): +def get_single_lv(fields=LV_FIELDS, filters=None, tags=None): """ - Wrapper of get_lv meant to be a convenience method to avoid the phrase:: + Wrapper of get_lvs() meant to be a convenience method to avoid the phrase:: lvs = get_lvs() if len(lvs) >= 1: lv = lvs[0] """ lvs = get_lvs(fields=fields, filters=filters, tags=tags) - return lvs[0] if len(lvs) > 0 else [] + + if len(lvs) == 0: + return None + if len(lvs) > 1: + raise RuntimeError('Filters {} matched more than 1 LV present on this host.'.format(str(filters))) + + return lvs[0] def get_lv_by_name(name): @@ -1141,7 +1163,7 @@ def get_lv_by_fullname(full_name): """ try: vg_name, lv_name = full_name.split('/') - res_lv = get_first_lv(filters={'lv_name': lv_name, + res_lv = get_single_lv(filters={'lv_name': lv_name, 'vg_name': vg_name}) except ValueError: res_lv = None diff --git a/src/ceph-volume/ceph_volume/devices/lvm/prepare.py b/src/ceph-volume/ceph_volume/devices/lvm/prepare.py index f0c3959a62724..2f715fdba122c 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/prepare.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/prepare.py @@ -151,8 +151,8 @@ class Prepare(object): try: vg_name, lv_name = device_name.split('/') - lv = api.get_first_lv(filters={'lv_name': lv_name, - 'vg_name': vg_name}) + lv = api.get_single_lv(filters={'lv_name': lv_name, + 'vg_name': vg_name}) except ValueError: lv = None @@ -240,8 +240,8 @@ class Prepare(object): try: vgname, lvname = self.args.data.split('/') - lv = api.get_first_lv(filters={'lv_name': lvname, - 'vg_name': vgname}) + lv = api.get_single_lv(filters={'lv_name': lvname, + 'vg_name': vgname}) except ValueError: lv = None @@ -325,7 +325,7 @@ class Prepare(object): try: vg_name, lv_name = self.args.data.split('/') - data_lv = api.get_first_lv(filters={'lv_name': lv_name, + data_lv = api.get_single_lv(filters={'lv_name': lv_name, 'vg_name': vg_name}) except ValueError: data_lv = None @@ -340,8 +340,8 @@ class Prepare(object): data_lv.set_tags(tags) if not journal_device.startswith('/'): # we got a journal lv, set rest of the tags - api.get_first_lv(filters={'lv_name': lv_name, - 'vg_name': vg_name}).set_tags(tags) + api.get_single_lv(filters={'lv_name': lv_name, + 'vg_name': vg_name}).set_tags(tags) prepare_filestore( data_lv.lv_path, @@ -354,8 +354,8 @@ class Prepare(object): elif self.args.bluestore: try: vg_name, lv_name = self.args.data.split('/') - block_lv = api.get_first_lv(filters={'lv_name': lv_name, - 'vg_name': vg_name}) + block_lv = api.get_single_lv(filters={'lv_name': lv_name, + 'vg_name': vg_name}) except ValueError: block_lv = None diff --git a/src/ceph-volume/ceph_volume/devices/lvm/zap.py b/src/ceph-volume/ceph_volume/devices/lvm/zap.py index e95b8687c1d77..20023a27c9011 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/zap.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/zap.py @@ -167,8 +167,8 @@ class Zap(object): Device examples: vg-name/lv-name, /dev/vg-name/lv-name Requirements: Must be a logical volume (LV) """ - lv = api.get_first_lv(filters={'lv_name': device.lv_name, 'vg_name': - device.vg_name}) + lv = api.get_single_lv(filters={'lv_name': device.lv_name, 'vg_name': + device.vg_name}) self.unmount_lv(lv) wipefs(device.abspath) @@ -232,7 +232,7 @@ class Zap(object): mlogger.info('Zapping lvm member {}. lv_path is {}'.format(device.abspath, lv.lv_path)) self.zap_lv(Device(lv.lv_path)) else: - vg = api.get_first_vg(filters={'vg_name': lv.vg_name}) + vg = api.get_single_vg(filters={'vg_name': lv.vg_name}) if vg: mlogger.info('Found empty VG {}, removing'.format(vg.vg_name)) api.remove_vg(vg.vg_name) diff --git a/src/ceph-volume/ceph_volume/devices/simple/scan.py b/src/ceph-volume/ceph_volume/devices/simple/scan.py index 34da0962b0fc8..70e5256d27317 100644 --- a/src/ceph-volume/ceph_volume/devices/simple/scan.py +++ b/src/ceph-volume/ceph_volume/devices/simple/scan.py @@ -80,7 +80,7 @@ class Scan(object): device = os.readlink(path) else: device = path - lvm_device = lvm.get_first_lv(filters={'lv_path': device}) + lvm_device = lvm.get_single_lv(filters={'lv_path': device}) if lvm_device: device_uuid = lvm_device.lv_uuid else: diff --git a/src/ceph-volume/ceph_volume/tests/api/test_lvm.py b/src/ceph-volume/ceph_volume/tests/api/test_lvm.py index f01ceb4f337a5..16e586e02b22a 100644 --- a/src/ceph-volume/ceph_volume/tests/api/test_lvm.py +++ b/src/ceph-volume/ceph_volume/tests/api/test_lvm.py @@ -191,23 +191,23 @@ class TestCreateLV(object): @patch('ceph_volume.api.lvm.process.run') @patch('ceph_volume.api.lvm.process.call') - @patch('ceph_volume.api.lvm.get_first_lv') - def test_uses_size(self, m_get_first_lv, m_call, m_run, monkeypatch): - m_get_first_lv.return_value = self.foo_volume + @patch('ceph_volume.api.lvm.get_single_lv') + def test_uses_size(self, m_get_single_lv, m_call, m_run, monkeypatch): + m_get_single_lv.return_value = self.foo_volume api.create_lv('foo', 0, vg=self.foo_group, size=419430400, tags={'ceph.type': 'data'}) expected = ['lvcreate', '--yes', '-l', '100', '-n', 'foo-0', 'foo_group'] m_run.assert_called_with(expected) @patch('ceph_volume.api.lvm.process.run') @patch('ceph_volume.api.lvm.process.call') - @patch('ceph_volume.api.lvm.get_first_lv') - def test_uses_size_adjust_if_1percent_over(self, m_get_first_lv, m_call, m_run, monkeypatch): + @patch('ceph_volume.api.lvm.get_single_lv') + def test_uses_size_adjust_if_1percent_over(self, m_get_single_lv, m_call, m_run, monkeypatch): foo_volume = api.Volume(lv_name='foo', lv_path='/path', vg_name='foo_group', lv_tags='') foo_group = api.VolumeGroup(vg_name='foo_group', vg_extent_size="4194304", vg_extent_count="1000", vg_free_count="1000") - m_get_first_lv.return_value = foo_volume + m_get_single_lv.return_value = foo_volume # 423624704 should be just under 1% off of the available size 419430400 api.create_lv('foo', 0, vg=foo_group, size=4232052736, tags={'ceph.type': 'data'}) expected = ['lvcreate', '--yes', '-l', '1000', '-n', 'foo-0', 'foo_group'] @@ -215,17 +215,17 @@ class TestCreateLV(object): @patch('ceph_volume.api.lvm.process.run') @patch('ceph_volume.api.lvm.process.call') - @patch('ceph_volume.api.lvm.get_first_lv') - def test_uses_size_too_large(self, m_get_first_lv, m_call, m_run, monkeypatch): - m_get_first_lv.return_value = self.foo_volume + @patch('ceph_volume.api.lvm.get_single_lv') + def test_uses_size_too_large(self, m_get_single_lv, m_call, m_run, monkeypatch): + m_get_single_lv.return_value = self.foo_volume with pytest.raises(RuntimeError): api.create_lv('foo', 0, vg=self.foo_group, size=5368709120, tags={'ceph.type': 'data'}) @patch('ceph_volume.api.lvm.process.run') @patch('ceph_volume.api.lvm.process.call') - @patch('ceph_volume.api.lvm.get_first_lv') - def test_uses_extents(self, m_get_first_lv, m_call, m_run, monkeypatch): - m_get_first_lv.return_value = self.foo_volume + @patch('ceph_volume.api.lvm.get_single_lv') + def test_uses_extents(self, m_get_single_lv, m_call, m_run, monkeypatch): + m_get_single_lv.return_value = self.foo_volume api.create_lv('foo', 0, vg=self.foo_group, extents='50', tags={'ceph.type': 'data'}) expected = ['lvcreate', '--yes', '-l', '50', '-n', 'foo-0', 'foo_group'] m_run.assert_called_with(expected) @@ -235,18 +235,18 @@ class TestCreateLV(object): (3, 33),]) @patch('ceph_volume.api.lvm.process.run') @patch('ceph_volume.api.lvm.process.call') - @patch('ceph_volume.api.lvm.get_first_lv') - def test_uses_slots(self, m_get_first_lv, m_call, m_run, monkeypatch, test_input, expected): - m_get_first_lv.return_value = self.foo_volume + @patch('ceph_volume.api.lvm.get_single_lv') + def test_uses_slots(self, m_get_single_lv, m_call, m_run, monkeypatch, test_input, expected): + m_get_single_lv.return_value = self.foo_volume api.create_lv('foo', 0, vg=self.foo_group, slots=test_input, tags={'ceph.type': 'data'}) expected = ['lvcreate', '--yes', '-l', str(expected), '-n', 'foo-0', 'foo_group'] m_run.assert_called_with(expected) @patch('ceph_volume.api.lvm.process.run') @patch('ceph_volume.api.lvm.process.call') - @patch('ceph_volume.api.lvm.get_first_lv') - def test_uses_all(self, m_get_first_lv, m_call, m_run, monkeypatch): - m_get_first_lv.return_value = self.foo_volume + @patch('ceph_volume.api.lvm.get_single_lv') + def test_uses_all(self, m_get_single_lv, m_call, m_run, monkeypatch): + m_get_single_lv.return_value = self.foo_volume api.create_lv('foo', 0, vg=self.foo_group, tags={'ceph.type': 'data'}) expected = ['lvcreate', '--yes', '-l', '100%FREE', '-n', 'foo-0', 'foo_group'] m_run.assert_called_with(expected) @@ -254,9 +254,9 @@ class TestCreateLV(object): @patch('ceph_volume.api.lvm.process.run') @patch('ceph_volume.api.lvm.process.call') @patch('ceph_volume.api.lvm.Volume.set_tags') - @patch('ceph_volume.api.lvm.get_first_lv') - def test_calls_to_set_tags_default(self, m_get_first_lv, m_set_tags, m_call, m_run, monkeypatch): - m_get_first_lv.return_value = self.foo_volume + @patch('ceph_volume.api.lvm.get_single_lv') + def test_calls_to_set_tags_default(self, m_get_single_lv, m_set_tags, m_call, m_run, monkeypatch): + m_get_single_lv.return_value = self.foo_volume api.create_lv('foo', 0, vg=self.foo_group) tags = { "ceph.osd_id": "null", @@ -269,9 +269,9 @@ class TestCreateLV(object): @patch('ceph_volume.api.lvm.process.run') @patch('ceph_volume.api.lvm.process.call') @patch('ceph_volume.api.lvm.Volume.set_tags') - @patch('ceph_volume.api.lvm.get_first_lv') - def test_calls_to_set_tags_arg(self, m_get_first_lv, m_set_tags, m_call, m_run, monkeypatch): - m_get_first_lv.return_value = self.foo_volume + @patch('ceph_volume.api.lvm.get_single_lv') + def test_calls_to_set_tags_arg(self, m_get_single_lv, m_set_tags, m_call, m_run, monkeypatch): + m_get_single_lv.return_value = self.foo_volume api.create_lv('foo', 0, vg=self.foo_group, tags={'ceph.type': 'data'}) tags = { "ceph.type": "data", @@ -283,10 +283,10 @@ class TestCreateLV(object): @patch('ceph_volume.api.lvm.process.call') @patch('ceph_volume.api.lvm.get_device_vgs') @patch('ceph_volume.api.lvm.create_vg') - @patch('ceph_volume.api.lvm.get_first_lv') - def test_create_vg(self, m_get_first_lv, m_create_vg, m_get_device_vgs, m_call, + @patch('ceph_volume.api.lvm.get_single_lv') + def test_create_vg(self, m_get_single_lv, m_create_vg, m_get_device_vgs, m_call, m_run, monkeypatch): - m_get_first_lv.return_value = self.foo_volume + m_get_single_lv.return_value = self.foo_volume m_get_device_vgs.return_value = [] api.create_lv('foo', 0, device='dev/foo', size='5G', tags={'ceph.type': 'data'}) m_create_vg.assert_called_with('dev/foo', name_prefix='ceph') @@ -377,19 +377,19 @@ class TestExtendVG(object): self.foo_volume = api.VolumeGroup(vg_name='foo', lv_tags='') def test_uses_single_device_in_list(self, monkeypatch, fake_run): - monkeypatch.setattr(api, 'get_first_vg', lambda **kw: True) + monkeypatch.setattr(api, 'get_single_vg', lambda **kw: True) api.extend_vg(self.foo_volume, ['/dev/sda']) expected = ['vgextend', '--force', '--yes', 'foo', '/dev/sda'] assert fake_run.calls[0]['args'][0] == expected def test_uses_single_device(self, monkeypatch, fake_run): - monkeypatch.setattr(api, 'get_first_vg', lambda **kw: True) + monkeypatch.setattr(api, 'get_single_vg', lambda **kw: True) api.extend_vg(self.foo_volume, '/dev/sda') expected = ['vgextend', '--force', '--yes', 'foo', '/dev/sda'] assert fake_run.calls[0]['args'][0] == expected def test_uses_multiple_devices(self, monkeypatch, fake_run): - monkeypatch.setattr(api, 'get_first_vg', lambda **kw: True) + monkeypatch.setattr(api, 'get_single_vg', lambda **kw: True) api.extend_vg(self.foo_volume, ['/dev/sda', '/dev/sdb']) expected = ['vgextend', '--force', '--yes', 'foo', '/dev/sda', '/dev/sdb'] assert fake_run.calls[0]['args'][0] == expected @@ -401,19 +401,19 @@ class TestReduceVG(object): self.foo_volume = api.VolumeGroup(vg_name='foo', lv_tags='') def test_uses_single_device_in_list(self, monkeypatch, fake_run): - monkeypatch.setattr(api, 'get_first_vg', lambda **kw: True) + monkeypatch.setattr(api, 'get_single_vg', lambda **kw: True) api.reduce_vg(self.foo_volume, ['/dev/sda']) expected = ['vgreduce', '--force', '--yes', 'foo', '/dev/sda'] assert fake_run.calls[0]['args'][0] == expected def test_uses_single_device(self, monkeypatch, fake_run): - monkeypatch.setattr(api, 'get_first_vg', lambda **kw: True) + monkeypatch.setattr(api, 'get_single_vg', lambda **kw: True) api.reduce_vg(self.foo_volume, '/dev/sda') expected = ['vgreduce', '--force', '--yes', 'foo', '/dev/sda'] assert fake_run.calls[0]['args'][0] == expected def test_uses_multiple_devices(self, monkeypatch, fake_run): - monkeypatch.setattr(api, 'get_first_vg', lambda **kw: True) + monkeypatch.setattr(api, 'get_single_vg', lambda **kw: True) api.reduce_vg(self.foo_volume, ['/dev/sda', '/dev/sdb']) expected = ['vgreduce', '--force', '--yes', 'foo', '/dev/sda', '/dev/sdb'] assert fake_run.calls[0]['args'][0] == expected @@ -425,28 +425,28 @@ class TestCreateVG(object): self.foo_volume = api.VolumeGroup(vg_name='foo', lv_tags='') def test_no_name(self, monkeypatch, fake_run): - monkeypatch.setattr(api, 'get_first_vg', lambda **kw: True) + monkeypatch.setattr(api, 'get_single_vg', lambda **kw: True) api.create_vg('/dev/sda') result = fake_run.calls[0]['args'][0] assert '/dev/sda' in result assert result[-2].startswith('ceph-') def test_devices_list(self, monkeypatch, fake_run): - monkeypatch.setattr(api, 'get_first_vg', lambda **kw: True) + monkeypatch.setattr(api, 'get_single_vg', lambda **kw: True) api.create_vg(['/dev/sda', '/dev/sdb'], name='ceph') result = fake_run.calls[0]['args'][0] expected = ['vgcreate', '--force', '--yes', 'ceph', '/dev/sda', '/dev/sdb'] assert result == expected def test_name_prefix(self, monkeypatch, fake_run): - monkeypatch.setattr(api, 'get_first_vg', lambda **kw: True) + monkeypatch.setattr(api, 'get_single_vg', lambda **kw: True) api.create_vg('/dev/sda', name_prefix='master') result = fake_run.calls[0]['args'][0] assert '/dev/sda' in result assert result[-2].startswith('master-') def test_specific_name(self, monkeypatch, fake_run): - monkeypatch.setattr(api, 'get_first_vg', lambda **kw: True) + monkeypatch.setattr(api, 'get_single_vg', lambda **kw: True) api.create_vg('/dev/sda', name='master') result = fake_run.calls[0]['args'][0] assert '/dev/sda' in result @@ -780,91 +780,106 @@ class TestGetLVs(object): assert api.get_lvs() == [] -class TestGetFirstPV(object): +class TestGetSinglePV(object): - def test_get_first_pv(self, monkeypatch): - pv1 = api.PVolume(pv_name='/dev/sda', pv_uuid='0000', pv_tags={}, - vg_name='vg1') - pv2 = api.PVolume(pv_name='/dev/sdb', pv_uuid='0001', pv_tags={}, - vg_name='vg2') - stdout = ['{};{};{};{};;'.format(pv1.pv_name, pv1.pv_tags, pv1.pv_uuid, pv1.vg_name), - '{};{};{};{};;'.format(pv2.pv_name, pv2.pv_tags, pv2.pv_uuid, pv2.vg_name)] - monkeypatch.setattr(api.process, 'call', lambda x,**kw: (stdout, '', 0)) + @patch('ceph_volume.devices.lvm.prepare.api.get_pvs') + def test_get_single_pv_multiple_matches_raises_runtimeerror(self, m_get_pvs): + fake_pvs = [] + fake_pvs.append(api.PVolume(pv_name='/dev/sda', pv_tags={})) + fake_pvs.append(api.PVolume(pv_name='/dev/sdb', pv_tags={})) - pv_ = api.get_first_pv() - assert isinstance(pv_, api.PVolume) - assert pv_.pv_name == pv1.pv_name + m_get_pvs.return_value = fake_pvs - def test_get_first_pv_single_pv(self, monkeypatch): - pv = api.PVolume(pv_name='/dev/sda', pv_uuid='0000', pv_tags={}, - vg_name='vg1') - stdout = ['{};;;;;;'.format(pv.pv_name)] - monkeypatch.setattr(api.process, 'call', lambda x,**kw: (stdout, '', 0)) + with pytest.raises(RuntimeError) as e: + api.get_single_pv() + assert "matched more than 1 PV present on this host." in str(e.value) - pv_ = api.get_first_pv() - assert isinstance(pv_, api.PVolume) - assert pv_.pv_name == pv.pv_name + @patch('ceph_volume.devices.lvm.prepare.api.get_pvs') + def test_get_single_pv_no_match_returns_none(self, m_get_pvs): + m_get_pvs.return_value = [] - def test_get_first_pv_empty(self, monkeypatch): - monkeypatch.setattr(api.process, 'call', lambda x,**kw: ('', '', 0)) - assert api.get_first_pv() == [] + pv = api.get_single_pv() + assert pv == None + @patch('ceph_volume.devices.lvm.prepare.api.get_pvs') + def test_get_single_pv_one_match(self, m_get_pvs): + fake_pvs = [] + fake_pvs.append(api.PVolume(pv_name='/dev/sda', pv_tags={})) + m_get_pvs.return_value = fake_pvs -class TestGetFirstVG(object): + pv = api.get_single_pv() - def test_get_first_vg(self, monkeypatch): - vg1 = api.VolumeGroup(vg_name='vg1') - vg2 = api.VolumeGroup(vg_name='vg2') - stdout = ['{};;;;;;'.format(vg1.vg_name), '{};;;;;;'.format(vg2.vg_name)] - monkeypatch.setattr(api.process, 'call', lambda x,**kw: (stdout, '', 0)) + assert isinstance(pv, api.PVolume) + assert pv.name == '/dev/sda' - vg_ = api.get_first_vg() - assert isinstance(vg_, api.VolumeGroup) - assert vg_.vg_name == vg1.vg_name - def test_get_first_vg_single_vg(self, monkeypatch): - vg = api.VolumeGroup(vg_name='vg') - stdout = ['{};;;;;;'.format(vg.vg_name)] - monkeypatch.setattr(api.process, 'call', lambda x,**kw: (stdout, '', 0)) +class TestGetSingleVG(object): - vg_ = api.get_first_vg() - assert isinstance(vg_, api.VolumeGroup) - assert vg_.vg_name == vg.vg_name + @patch('ceph_volume.devices.lvm.prepare.api.get_vgs') + def test_get_single_vg_multiple_matches_raises_runtimeerror(self, m_get_vgs): + fake_vgs = [] + fake_vgs.append(api.VolumeGroup(vg_name='vg1')) + fake_vgs.append(api.VolumeGroup(vg_name='vg2')) - def test_get_first_vg_empty(self, monkeypatch): - monkeypatch.setattr(api.process, 'call', lambda x,**kw: ('', '', 0)) - vg_ = api.get_first_vg() - assert vg_ == [] + m_get_vgs.return_value = fake_vgs + with pytest.raises(RuntimeError) as e: + api.get_single_vg() + assert "matched more than 1 VG present on this host." in str(e.value) -class TestGetFirstLV(object): + @patch('ceph_volume.devices.lvm.prepare.api.get_vgs') + def test_get_single_vg_no_match_returns_none(self, m_get_vgs): + m_get_vgs.return_value = [] - def test_get_first_lv(self, monkeypatch): - lv1 = api.Volume(lv_tags='ceph.type=data', lv_path='/dev/vg1/lv1', - lv_name='lv1', vg_name='vg1') - lv2 = api.Volume(lv_tags='ceph.type=data', lv_path='/dev/vg2/lv2', - lv_name='lv2', vg_name='vg2') - stdout = ['{};{};{};{}'.format(lv1.lv_tags, lv1.lv_path, lv1.lv_name, - lv1.vg_name), - '{};{};{};{}'.format(lv2.lv_tags, lv2.lv_path, lv2.lv_name, - lv2.vg_name)] - monkeypatch.setattr(api.process, 'call', lambda x,**kw: (stdout, '', 0)) + vg = api.get_single_vg() + assert vg == None - lv_ = api.get_first_lv() - assert isinstance(lv_, api.Volume) - assert lv_.lv_name == lv1.lv_name + @patch('ceph_volume.devices.lvm.prepare.api.get_vgs') + def test_get_single_vg_one_match(self, m_get_vgs): + fake_vgs = [] + fake_vgs.append(api.VolumeGroup(vg_name='vg1')) + m_get_vgs.return_value = fake_vgs - def test_get_first_lv_single_lv(self, monkeypatch): - stdout = ['ceph.type=data;/dev/vg/lv;lv;vg'] - monkeypatch.setattr(api.process, 'call', lambda x,**kw: (stdout, '', 0)) - lv = api.Volume(lv_tags='ceph.type=data', - lv_path='/dev/vg/lv', - lv_name='lv', vg_name='vg') + vg = api.get_single_vg() - lv_ = api.get_first_lv() - assert isinstance(lv_, api.Volume) - assert lv_.lv_name == lv.lv_name + assert isinstance(vg, api.VolumeGroup) + assert vg.name == 'vg1' - def test_get_first_lv_empty(self, monkeypatch): - monkeypatch.setattr(api.process, 'call', lambda x,**kw: ('', '', 0)) - assert api.get_lvs() == [] +class TestGetSingleLV(object): + + @patch('ceph_volume.devices.lvm.prepare.api.get_lvs') + def test_get_single_lv_multiple_matches_raises_runtimeerror(self, m_get_lvs): + fake_lvs = [] + fake_lvs.append(api.Volume(lv_name='lv1', + lv_path='/dev/vg1/lv1', + vg_name='vg1', + lv_tags='', + lv_uuid='fake-uuid')) + fake_lvs.append(api.Volume(lv_name='lv1', + lv_path='/dev/vg2/lv1', + vg_name='vg2', + lv_tags='', + lv_uuid='fake-uuid')) + m_get_lvs.return_value = fake_lvs + + with pytest.raises(RuntimeError) as e: + api.get_single_lv() + assert "matched more than 1 LV present on this host" in str(e.value) + + @patch('ceph_volume.devices.lvm.prepare.api.get_lvs') + def test_get_single_lv_no_match_returns_none(self, m_get_lvs): + m_get_lvs.return_value = [] + + lv = api.get_single_lv() + assert lv == None + + @patch('ceph_volume.devices.lvm.prepare.api.get_lvs') + def test_get_single_lv_one_match(self, m_get_lvs): + fake_lvs = [] + fake_lvs.append(api.Volume(lv_name='lv1', lv_path='/dev/vg1/lv1', vg_name='vg1', lv_tags='', lv_uuid='fake-uuid')) + m_get_lvs.return_value = fake_lvs + + lv_ = api.get_single_lv() + + assert isinstance(lv_, api.Volume) + assert lv_.name == 'lv1' diff --git a/src/ceph-volume/ceph_volume/tests/conftest.py b/src/ceph-volume/ceph_volume/tests/conftest.py index 149afbbc65235..cf0f825686efe 100644 --- a/src/ceph-volume/ceph_volume/tests/conftest.py +++ b/src/ceph-volume/ceph_volume/tests/conftest.py @@ -287,7 +287,7 @@ def device_info(monkeypatch, patch_bluestore_label): monkeypatch.setattr("ceph_volume.sys_info.devices", {}) monkeypatch.setattr("ceph_volume.util.device.disk.get_devices", lambda: devices) if not devices: - monkeypatch.setattr("ceph_volume.util.device.lvm.get_first_lv", lambda filters: lv) + monkeypatch.setattr("ceph_volume.util.device.lvm.get_single_lv", lambda filters: lv) else: monkeypatch.setattr("ceph_volume.util.device.lvm.get_device_lvs", lambda path: [lv]) diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_activate.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_activate.py index 1fae5c4e0c34c..9b8fcbebeaef9 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_activate.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_activate.py @@ -51,7 +51,7 @@ class TestActivate(object): volumes = [] volumes.append(FooVolume) monkeypatch.setattr(api, 'get_lvs', lambda **kwargs: []) - monkeypatch.setattr(api, 'get_first_lv', lambda **kwargs: []) + monkeypatch.setattr(api, 'get_single_lv', lambda **kwargs: []) monkeypatch.setattr(activate, 'activate_filestore', capture) args = Args(osd_id=None, osd_fsid='2222') diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_listing.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_listing.py index cf4b68c714378..30d91b20f68b9 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_listing.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_listing.py @@ -106,7 +106,7 @@ class TestFullReport(object): lv_path='/dev/VolGroup/lv', vg_name='VolGroup') volumes = [] volumes.append(osd) - monkeypatch.setattr(lvm.listing.api, 'get_first_pv', lambda **kwargs: pv) + monkeypatch.setattr(lvm.listing.api, 'get_single_pv', lambda **kwargs: pv) monkeypatch.setattr(lvm.listing.api, 'get_lvs', lambda **kwargs: volumes) @@ -126,7 +126,7 @@ class TestFullReport(object): volumes = [] volumes.append(osd) volumes.append(journal) - monkeypatch.setattr(lvm.listing.api,'get_first_pv',lambda **kwargs:pv) + monkeypatch.setattr(lvm.listing.api,'get_single_pv',lambda **kwargs:pv) monkeypatch.setattr(lvm.listing.api, 'get_lvs', lambda **kwargs: volumes) diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py index 74d6589502b6f..1faaee05e6dda 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py @@ -47,7 +47,7 @@ class TestFindAssociatedDevices(object): return self.mock_volumes.pop(0) mock_single_volumes = {} - def mock_get_first_lv(self, *args, **kwargs): + def mock_get_single_lv(self, *args, **kwargs): p = kwargs['filters']['lv_path'] return self.mock_single_volumes[p] @@ -64,7 +64,7 @@ class TestFindAssociatedDevices(object): self.mock_single_volumes = {'/dev/VolGroup/lv1': vol} monkeypatch.setattr(migrate.api, 'get_lvs', self.mock_get_lvs) - monkeypatch.setattr(migrate.api, 'get_first_lv', self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', self.mock_get_single_lv) monkeypatch.setattr(process, 'call', lambda x, **kw: ('', '', 0)) result = migrate.find_associated_devices(osd_id='0', osd_fsid='1234') @@ -89,7 +89,7 @@ class TestFindAssociatedDevices(object): self.mock_single_volumes = {'/dev/VolGroup/lv1': vol, '/dev/VolGroup/lv2': vol2} monkeypatch.setattr(migrate.api, 'get_lvs', self.mock_get_lvs) - monkeypatch.setattr(migrate.api, 'get_first_lv', self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', self.mock_get_single_lv) monkeypatch.setattr(process, 'call', lambda x, **kw: ('', '', 0)) result = migrate.find_associated_devices(osd_id='0', osd_fsid='1234') @@ -126,7 +126,7 @@ class TestFindAssociatedDevices(object): '/dev/VolGroup/lv3': vol3} monkeypatch.setattr(migrate.api, 'get_lvs', self.mock_get_lvs) - monkeypatch.setattr(migrate.api, 'get_first_lv', self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', self.mock_get_single_lv) monkeypatch.setattr(process, 'call', lambda x, **kw: ('', '', 0)) result = migrate.find_associated_devices(osd_id='0', osd_fsid='1234') @@ -159,7 +159,7 @@ class TestFindAssociatedDevices(object): class TestVolumeTagTracker(object): mock_single_volumes = {} - def mock_get_first_lv(self, *args, **kwargs): + def mock_get_single_lv(self, *args, **kwargs): p = kwargs['filters']['lv_path'] return self.mock_single_volumes[p] @@ -185,7 +185,7 @@ class TestVolumeTagTracker(object): self.mock_single_volumes = {'/dev/VolGroup/lv1': data_vol, '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol} - monkeypatch.setattr(migrate.api, 'get_first_lv', self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', self.mock_get_single_lv) self.mock_process_input = [] monkeypatch.setattr(process, 'call', self.mock_process) @@ -233,7 +233,7 @@ class TestVolumeTagTracker(object): self.mock_single_volumes = {'/dev/VolGroup/lv1': data_vol, '/dev/VolGroup/lv2': db_vol} - monkeypatch.setattr(migrate.api, 'get_first_lv', self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', self.mock_get_single_lv) self.mock_process_input = [] monkeypatch.setattr(process, 'call', self.mock_process) @@ -297,7 +297,7 @@ class TestVolumeTagTracker(object): '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol} - monkeypatch.setattr(migrate.api, 'get_first_lv', self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', self.mock_get_single_lv) self.mock_process_input = [] monkeypatch.setattr(process, 'call', self.mock_process) @@ -355,7 +355,7 @@ class TestVolumeTagTracker(object): '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol} - monkeypatch.setattr(migrate.api, 'get_first_lv', self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', self.mock_get_single_lv) self.mock_process_input = [] monkeypatch.setattr(process, 'call', self.mock_process) @@ -426,7 +426,7 @@ class TestVolumeTagTracker(object): '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol} - monkeypatch.setattr(migrate.api, 'get_first_lv', self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', self.mock_get_single_lv) self.mock_process_input = [] monkeypatch.setattr(process, 'call', self.mock_process) @@ -512,7 +512,7 @@ class TestNew(object): return ('', '', 0) mock_single_volumes = {} - def mock_get_first_lv(self, *args, **kwargs): + def mock_get_single_lv(self, *args, **kwargs): p = kwargs['filters']['lv_path'] return self.mock_single_volumes[p] @@ -590,8 +590,8 @@ class TestNew(object): self.mock_single_volumes = {'/dev/VolGroup/lv1': data_vol, '/dev/VolGroup/lv3': wal_vol} - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_process_input = [] monkeypatch.setattr(process, 'call', self.mock_process) @@ -681,8 +681,8 @@ class TestNew(object): self.mock_single_volumes = {'/dev/VolGroup/lv1': data_vol, '/dev/VolGroup/lv3': wal_vol} - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_process_input = [] monkeypatch.setattr(process, 'call', self.mock_process) @@ -746,8 +746,8 @@ class TestNew(object): self.mock_single_volumes = {'/dev/VolGroup/lv1': data_vol, '/dev/VolGroup/lv3': wal_vol} - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_process_input = [] monkeypatch.setattr(process, 'call', self.mock_process) @@ -826,7 +826,7 @@ class TestNew(object): self.mock_single_volumes = {'/dev/VolGroup/lv1': data_vol} - monkeypatch.setattr(migrate.api, 'get_first_lv', self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', self.mock_get_single_lv) self.mock_process_input = [] monkeypatch.setattr(process, 'call', self.mock_process) @@ -889,7 +889,7 @@ class TestNew(object): self.mock_single_volumes = {'/dev/VolGroup/lv1': data_vol} - monkeypatch.setattr(migrate.api, 'get_first_lv', self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', self.mock_get_single_lv) self.mock_process_input = [] monkeypatch.setattr(process, 'call', self.mock_process) @@ -937,7 +937,7 @@ class TestNew(object): self.mock_single_volumes = {'/dev/VolGroup/lv1': data_vol} - monkeypatch.setattr(migrate.api, 'get_first_lv', self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', self.mock_get_single_lv) self.mock_process_input = [] monkeypatch.setattr(process, 'call', self.mock_process) @@ -1006,7 +1006,7 @@ class TestMigrate(object): return ('', '', 0) mock_single_volumes = {} - def mock_get_first_lv(self, *args, **kwargs): + def mock_get_single_lv(self, *args, **kwargs): p = kwargs['filters']['lv_path'] return self.mock_single_volumes[p] @@ -1042,8 +1042,8 @@ class TestMigrate(object): '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = api.Volume(lv_name='volume2', lv_uuid='y', vg_name='vg', @@ -1168,8 +1168,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv1': data_vol, '/dev/VolGroup/lv2': db_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = api.Volume(lv_name='volume2_new', lv_uuid='new-db-uuid', vg_name='vg', @@ -1266,8 +1266,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv1': data_vol, '/dev/VolGroup/lv2': db_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = api.Volume(lv_name='volume2_new', lv_uuid='new-db-uuid', vg_name='vg', @@ -1329,8 +1329,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv1': data_vol, '/dev/VolGroup/lv2': db_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = api.Volume(lv_name='volume2_new', lv_uuid='new-db-uuid', vg_name='vg', @@ -1437,8 +1437,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = api.Volume(lv_name='volume2_new', lv_uuid='new-db-uuid', vg_name='vg', @@ -1561,8 +1561,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = api.Volume(lv_name='volume2_new', lv_uuid='new-db-uuid', vg_name='vg', @@ -1680,8 +1680,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv1': data_vol, '/dev/VolGroup/lv2': db_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = api.Volume(lv_name='volume2_new', lv_uuid='new-db-uuid', vg_name='vg', @@ -1759,8 +1759,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = wal_vol monkeypatch.setattr(api, 'get_lv_by_fullname', @@ -1835,8 +1835,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = db_vol monkeypatch.setattr(api, 'get_lv_by_fullname', @@ -1911,8 +1911,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = db_vol monkeypatch.setattr(api, 'get_lv_by_fullname', @@ -1982,8 +1982,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = db_vol monkeypatch.setattr(api, 'get_lv_by_fullname', @@ -2063,8 +2063,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = db_vol monkeypatch.setattr(api, 'get_lv_by_fullname', @@ -2162,8 +2162,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = db_vol monkeypatch.setattr(api, 'get_lv_by_fullname', @@ -2234,8 +2234,8 @@ Example calls for supported scenarios: '/dev/VolGroup/lv2': db_vol, '/dev/VolGroup/lv3': wal_vol, } - monkeypatch.setattr(migrate.api, 'get_first_lv', - self.mock_get_first_lv) + monkeypatch.setattr(migrate.api, 'get_single_lv', + self.mock_get_single_lv) self.mock_volume = db_vol monkeypatch.setattr(api, 'get_lv_by_fullname', diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py index dff58cbabd690..fcbc276f0de1b 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py @@ -121,10 +121,10 @@ class TestPrepare(object): assert result == ('', '', {'ceph.type': 'data'}) @patch('ceph_volume.api.lvm.Volume.set_tags') - @patch('ceph_volume.devices.lvm.prepare.api.get_first_lv') - def test_setup_device_lv_passed(self, m_get_first_lv, m_set_tags): + @patch('ceph_volume.devices.lvm.prepare.api.get_single_lv') + def test_setup_device_lv_passed(self, m_get_single_lv, m_set_tags): fake_volume = api.Volume(lv_name='lv_foo', lv_path='/fake-path', vg_name='vg_foo', lv_tags='', lv_uuid='fake-uuid') - m_get_first_lv.return_value = fake_volume + m_get_single_lv.return_value = fake_volume result = lvm.prepare.Prepare([]).setup_device(device_type='data', device_name='vg_foo/lv_foo', tags={'ceph.type': 'data'}, size=0, slots=None) assert result == ('/fake-path', 'fake-uuid', {'ceph.type': 'data', @@ -147,9 +147,9 @@ class TestPrepare(object): 'ceph.data_device': '/fake-path'}) @patch('ceph_volume.devices.lvm.prepare.Prepare.get_ptuuid') - @patch('ceph_volume.devices.lvm.prepare.api.get_first_lv') - def test_setup_device_partition_passed(self, m_get_first_lv, m_get_ptuuid): - m_get_first_lv.side_effect = ValueError() + @patch('ceph_volume.devices.lvm.prepare.api.get_single_lv') + def test_setup_device_partition_passed(self, m_get_single_lv, m_get_ptuuid): + m_get_single_lv.side_effect = ValueError() m_get_ptuuid.return_value = 'fake-uuid' result = lvm.prepare.Prepare([]).setup_device(device_type='data', device_name='/dev/sdx', tags={'ceph.type': 'data'}, size=0, slots=None) diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py index b7d725bcdd5d9..f7912d3c7f2bc 100644 --- a/src/ceph-volume/ceph_volume/util/device.py +++ b/src/ceph-volume/ceph_volume/util/device.py @@ -162,11 +162,11 @@ class Device(object): # if the path is not absolute, we have 'vg/lv', let's use LV name # to get the LV. if self.path[0] == '/': - lv = lvm.get_first_lv(filters={'lv_path': self.path}) + lv = lvm.get_single_lv(filters={'lv_path': self.path}) else: vgname, lvname = self.path.split('/') - lv = lvm.get_first_lv(filters={'lv_name': lvname, - 'vg_name': vgname}) + lv = lvm.get_single_lv(filters={'lv_name': lvname, + 'vg_name': vgname}) if lv: self.lv_api = lv self.lvs = [lv] -- 2.39.5