From e5e866e8ecaf0522f3ebac83fa6fbc5a666acd58 Mon Sep 17 00:00:00 2001 From: Andrew Schoen Date: Wed, 25 Jul 2018 11:47:27 -0500 Subject: [PATCH] ceph-volume: PVolumes.get() should return one PV when using name or uuid It is possible to get duplicated pv entires from the 'pvs' lvm command. If we're using PVolumes.get() with either pv_name or pv_uuid we can safely return a single PVolume object. However, if we use pv_tags with PVolumes.get() we must still raise a MultiplePVsError if many pvs are found that have the tags, because they are not guaranteed to be the same pv, as would be the case with pv_name or pv_uuid. Fixes: http://tracker.ceph.com/issues/24784 Signed-off-by: Andrew Schoen --- src/ceph-volume/ceph_volume/api/lvm.py | 2 +- .../ceph_volume/tests/api/test_lvm.py | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/ceph-volume/ceph_volume/api/lvm.py b/src/ceph-volume/ceph_volume/api/lvm.py index 2c656988bb1..7525ba883d0 100644 --- a/src/ceph-volume/ceph_volume/api/lvm.py +++ b/src/ceph-volume/ceph_volume/api/lvm.py @@ -905,7 +905,7 @@ class PVolumes(list): ) if not pvs: return None - if len(pvs) > 1: + if len(pvs) > 1 and pv_tags: raise MultiplePVsError(pv_name) return pvs[0] 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 8b6450a2999..be3b5c1bd79 100644 --- a/src/ceph-volume/ceph_volume/tests/api/test_lvm.py +++ b/src/ceph-volume/ceph_volume/tests/api/test_lvm.py @@ -143,6 +143,31 @@ class TestGetPV(object): monkeypatch.setattr(api, 'PVolumes', lambda: pvolumes) assert api.get_pv(pv_uuid='0000') == FooPVolume + def test_multiple_pvs_is_matched_by_uuid(self, pvolumes, monkeypatch): + FooPVolume = api.PVolume(vg_name="vg", pv_name='/dev/sda', pv_uuid="0000", pv_tags={}, lv_uuid="0000000") + BarPVolume = api.PVolume(vg_name="vg", pv_name='/dev/sda', pv_uuid="0000", pv_tags={}) + pvolumes.append(FooPVolume) + pvolumes.append(BarPVolume) + monkeypatch.setattr(api, 'PVolumes', lambda: pvolumes) + assert api.get_pv(pv_uuid='0000') == FooPVolume + + def test_multiple_pvs_is_matched_by_name(self, pvolumes, monkeypatch): + FooPVolume = api.PVolume(vg_name="vg", pv_name='/dev/sda', pv_uuid="0000", pv_tags={}, lv_uuid="0000000") + BarPVolume = api.PVolume(vg_name="vg", pv_name='/dev/sda', pv_uuid="0000", pv_tags={}) + pvolumes.append(FooPVolume) + pvolumes.append(BarPVolume) + monkeypatch.setattr(api, 'PVolumes', lambda: pvolumes) + assert api.get_pv(pv_name='/dev/sda') == FooPVolume + + def test_multiple_pvs_is_matched_by_tags(self, pvolumes, monkeypatch): + FooPVolume = api.PVolume(vg_name="vg1", pv_name='/dev/sdc', pv_uuid="1000", pv_tags="ceph.foo=bar", lv_uuid="0000000") + BarPVolume = api.PVolume(vg_name="vg", pv_name='/dev/sda', pv_uuid="0000", pv_tags="ceph.foo=bar") + pvolumes.append(FooPVolume) + pvolumes.append(BarPVolume) + monkeypatch.setattr(api, 'PVolumes', lambda: pvolumes) + with pytest.raises(exceptions.MultiplePVsError): + api.get_pv(pv_tags={"ceph.foo": "bar"}) + def test_single_pv_is_matched_by_uuid(self, pvolumes, monkeypatch): FooPVolume = api.PVolume( pv_name='/dev/vg/foo', -- 2.39.5