From d7a2fb846ebaa0c661bcd041a394450a3b93fed6 Mon Sep 17 00:00:00 2001 From: Rishabh Dave Date: Thu, 3 Oct 2019 17:40:37 +0530 Subject: [PATCH 1/3] ceph-volume: allow creating empty VolumeGroup objects Signed-off-by: Rishabh Dave (cherry picked from commit 2dc4f7de96e51c8117b719640ec4c09388a1412e) --- src/ceph-volume/ceph_volume/api/lvm.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ceph-volume/ceph_volume/api/lvm.py b/src/ceph-volume/ceph_volume/api/lvm.py index 949d614252bfb..48d5e5bf47ae9 100644 --- a/src/ceph-volume/ceph_volume/api/lvm.py +++ b/src/ceph-volume/ceph_volume/api/lvm.py @@ -680,8 +680,9 @@ class VolumeGroups(list): to filter them via keyword arguments. """ - def __init__(self): - self._populate() + def __init__(self, populate=True): + if populate: + self._populate() def _populate(self): # get all the vgs in the current system From da5471e240f96048e18454f3706b15a9e88ffd86 Mon Sep 17 00:00:00 2001 From: Rishabh Dave Date: Thu, 3 Oct 2019 17:39:37 +0530 Subject: [PATCH 2/3] ceph-volume: VolumeGroups.filter shouldn't purge itself VolumeGroups.filter remove VGs from the list that do no match filter. Instead of doing that, return a new list that contains VGs that match the fiter so that VolumeGroups object held by code calling it is not modified. Fixes: https://tracker.ceph.com/issues/42171 Signed-off-by: Rishabh Dave (cherry picked from commit 028757813282f764ebcce05572f9e4b76ea4e552) --- src/ceph-volume/ceph_volume/api/lvm.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/ceph-volume/ceph_volume/api/lvm.py b/src/ceph-volume/ceph_volume/api/lvm.py index 48d5e5bf47ae9..7d1e9b690244f 100644 --- a/src/ceph-volume/ceph_volume/api/lvm.py +++ b/src/ceph-volume/ceph_volume/api/lvm.py @@ -734,15 +734,10 @@ def filter(self, vg_name=None, vg_tags=None): """ if not any([vg_name, vg_tags]): raise TypeError('.filter() requires vg_name or vg_tags (none given)') - # first find the filtered volumes with the values in self - filtered_groups = self._filter( - vg_name=vg_name, - vg_tags=vg_tags - ) - # then purge everything - self._purge() - # and add the filtered items - self.extend(filtered_groups) + + filtered_vgs = VolumeGroups(populate=False) + filtered_vgs.extend(self._filter(vg_name, vg_tags)) + return filtered_vgs def get(self, vg_name=None, vg_tags=None): """ From 9db8dd30ce38a8495ea9761962f5c81eb849ead6 Mon Sep 17 00:00:00 2001 From: Rishabh Dave Date: Thu, 3 Oct 2019 17:44:37 +0530 Subject: [PATCH 3/3] ceph-volume: update tests since VolumeGroups.filter returns a list VolumeGroups.filter returns VolumeGroups object that contains VGs matching the filter. Update the tests to hold the list returned by the Volumes.filter() call. Signed-off-by: Rishabh Dave (cherry picked from commit da58d239a0e067cd3ea5fd26aac24a2551b871c1) --- src/ceph-volume/ceph_volume/tests/api/test_lvm.py | 8 ++++---- src/ceph-volume/ceph_volume/tests/conftest.py | 4 ++++ 2 files changed, 8 insertions(+), 4 deletions(-) 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 4cedb4a56c001..7d31fa521ffe8 100644 --- a/src/ceph-volume/ceph_volume/tests/api/test_lvm.py +++ b/src/ceph-volume/ceph_volume/tests/api/test_lvm.py @@ -337,7 +337,7 @@ def test_filter_by_tag(self, volume_groups): journal = api.VolumeGroup(vg_name='volume2', vg_tags='ceph.group=plain') volume_groups.append(osd) volume_groups.append(journal) - volume_groups.filter(vg_tags={'ceph.group': 'dmcache'}) + volume_groups = volume_groups.filter(vg_tags={'ceph.group': 'dmcache'}) assert len(volume_groups) == 1 assert volume_groups[0].vg_name == 'volume1' @@ -345,7 +345,7 @@ def test_filter_by_tag_does_not_match_one(self, volume_groups): vg_tags = "ceph.group=dmcache,ceph.disk_type=ssd" osd = api.VolumeGroup(vg_name='volume1', vg_path='/dev/vg/lv', vg_tags=vg_tags) volume_groups.append(osd) - volume_groups.filter(vg_tags={'ceph.group': 'data', 'ceph.disk_type': 'ssd'}) + volume_groups = volume_groups.filter(vg_tags={'ceph.group': 'data', 'ceph.disk_type': 'ssd'}) assert volume_groups == [] def test_filter_by_vg_name(self, volume_groups): @@ -354,13 +354,13 @@ def test_filter_by_vg_name(self, volume_groups): journal = api.VolumeGroup(vg_name='volume2', vg_tags='ceph.type=journal') volume_groups.append(osd) volume_groups.append(journal) - volume_groups.filter(vg_name='ceph_vg') + volume_groups = volume_groups.filter(vg_name='ceph_vg') assert len(volume_groups) == 1 assert volume_groups[0].vg_name == 'ceph_vg' def test_filter_requires_params(self, volume_groups): with pytest.raises(TypeError): - volume_groups.filter() + volume_groups = volume_groups.filter() class TestVolumeGroupFree(object): diff --git a/src/ceph-volume/ceph_volume/tests/conftest.py b/src/ceph-volume/ceph_volume/tests/conftest.py index fad7df44e305d..07864da70726c 100644 --- a/src/ceph-volume/ceph_volume/tests/conftest.py +++ b/src/ceph-volume/ceph_volume/tests/conftest.py @@ -153,6 +153,10 @@ def volume_groups(monkeypatch): vgs._purge() return vgs +def volume_groups_empty(monkeypatch): + monkeypatch.setattr('ceph_volume.process.call', lambda x, **kw: ('', '', 0)) + vgs = lvm_api.VolumeGroups(populate=False) + return vgs @pytest.fixture def stub_vgs(monkeypatch, volume_groups):