Merge pull request #21645 from alfredodeza/wip-rm23451

ceph-volume include physical devices associated with an LV when listing

Reviewed-by: Andrew Schoen <aschoen@redhat.com>

doc/ceph-volume/lvm/list.rst

@@ -39,6 +39,7 @@ one with a physical device may look similar to::
           data uuid                 SlEgHe-jX1H-QBQk-Sce0-RUls-8KlY-g8HgcZ
           journal device            /dev/journals/journal1
           data device               /dev/test_group/data-lv2
+          devices                   /dev/sda
 
       [data]    /dev/test_group/data-lv2
 
@@ -50,6 +51,7 @@ one with a physical device may look similar to::
           data uuid                 SlEgHe-jX1H-QBQk-Sce0-RUls-8KlY-g8HgcZ
           journal device            /dev/journals/journal1
           data device               /dev/test_group/data-lv2
+          devices                   /dev/sdb
 
     ====== osd.0 =======
 
@@ -63,11 +65,18 @@ one with a physical device may look similar to::
           data uuid                 TUpfel-Q5ZT-eFph-bdGW-SiNW-l0ag-f5kh00
           journal device            /dev/sdd1
           data device               /dev/test_group/data-lv1
+          devices                   /dev/sdc
 
       [journal]    /dev/sdd1
 
           PARTUUID                  cd72bd28-002a-48da-bdf6-d5b993e84f3f
 
+
+For logical volumes the ``devices`` key is populated with the physical devices
+associated with the logical volume. Since LVM allows multiple physical devices
+to be part of a logical volume, the value will be comma separated when using
+``pretty``, but an array when using ``json``.
+
 .. note:: Tags are displayed in a readable format. The ``osd id`` key is stored
           as a ``ceph.osd_id`` tag. For more information on lvm tag conventions
           see :ref:`ceph-volume-lvm-tag-api`
@@ -96,6 +105,7 @@ can be listed in the following way::
           data uuid                 SlEgHe-jX1H-QBQk-Sce0-RUls-8KlY-g8HgcZ
           journal device            /dev/journals/journal1
           data device               /dev/test_group/data-lv2
+          devices                   /dev/sdc
 
 
 .. note:: Tags are displayed in a readable format. The ``osd id`` key is stored
@@ -134,6 +144,7 @@ output (note how tags aren't modified)::
     {
         "0": [
             {
+                "devices": ["/dev/sda"],
                 "lv_name": "data-lv1",
                 "lv_path": "/dev/test_group/data-lv1",
                 "lv_tags": "ceph.cluster_fsid=ce454d91-d748-4751-a318-ff7f7aa18ffd,ceph.data_device=/dev/test_group/data-lv1,ceph.data_uuid=TUpfel-Q5ZT-eFph-bdGW-SiNW-l0ag-f5kh00,ceph.journal_device=/dev/sdd1,ceph.journal_uuid=cd72bd28-002a-48da-bdf6-d5b993e84f3f,ceph.osd_fsid=943949f0-ce37-47ca-a33c-3413d46ee9ec,ceph.osd_id=0,ceph.type=data",

src/ceph-volume/ceph_volume/api/lvm.py

@@ -220,7 +220,7 @@ def get_api_pvs():
           /dev/sdv;;07A4F654-4162-4600-8EB3-88D1E42F368D
 
     """
-    fields = 'pv_name,pv_tags,pv_uuid,vg_name'
+    fields = 'pv_name,pv_tags,pv_uuid,vg_name,lv_uuid'
 
     stdout, stderr, returncode = process.call(
         ['pvs', '--no-heading', '--readonly', '--separator=";"', '-o', fields]

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

@@ -49,6 +49,9 @@ def pretty_report(report):
                         value=value
                     )
                 )
+            output.append(
+                device_metadata_item_template.format(tag_name='devices', value=','.join(device['devices'])))
+
     print(''.join(output))
 
 
@@ -74,6 +77,30 @@ class List(object):
     def __init__(self, argv):
         self.argv = argv
 
+    @property
+    def pvs(self):
+        """
+        To avoid having to make an LVM API call for every single item being
+        reported, the call gets set only once, using that stored call for
+        subsequent calls
+        """
+        if getattr(self, '_pvs', None) is not None:
+            return self._pvs
+        self._pvs = api.get_api_pvs()
+        return self._pvs
+
+    def match_devices(self, lv_uuid):
+        """
+        It is possible to have more than one PV reported *with the same name*,
+        to avoid incorrect or duplicate contents we correlated the lv uuid to
+        the one on the physical device.
+        """
+        devices = []
+        for device in self.pvs:
+            if device.get('lv_uuid') == lv_uuid:
+                devices.append(device['pv_name'])
+        return devices
+
     @decorators.needs_root
     def list(self, args):
         # ensure everything is up to date before calling out
@@ -152,16 +179,17 @@ def single_report(self, device):
                 return self.full_report(lvs=lvs)
 
         if lv:
+
             try:
                 _id = lv.tags['ceph.osd_id']
             except KeyError:
                 logger.warning('device is not part of ceph: %s', device)
                 return report
 
             report.setdefault(_id, [])
-            report[_id].append(
-                lv.as_dict()
-            )
+            lv_report = lv.as_dict()
+            lv_report['devices'] = self.match_devices(lv.lv_uuid)
+            report[_id].append(lv_report)
 
         else:
             # this has to be a journal/wal/db device (not a logical volume) so try
@@ -202,9 +230,9 @@ def full_report(self, lvs=None):
                 continue
 
             report.setdefault(_id, [])
-            report[_id].append(
-                lv.as_dict()
-            )
+            lv_report = lv.as_dict()
+            lv_report['devices'] = self.match_devices(lv.lv_uuid)
+            report[_id].append(lv_report)
 
             for device_type in ['journal', 'block', 'wal', 'db']:
                 device_uuid = lv.tags.get('ceph.%s_uuid' % device_type)

src/ceph-volume/ceph_volume/tests/devices/lvm/test_listing.py

@@ -22,12 +22,16 @@ def test_is_empty(self, capsys):
         assert stdout == '\n'
 
     def test_type_and_path_are_reported(self, capsys):
-        lvm.listing.pretty_report({0: [{'type': 'data', 'path': '/dev/sda1'}]})
+        lvm.listing.pretty_report({0: [
+            {'type': 'data', 'path': '/dev/sda1', 'devices': ['/dev/sda']}
+        ]})
         stdout, stderr = capsys.readouterr()
         assert '[data]    /dev/sda1' in stdout
 
     def test_osd_id_header_is_reported(self, capsys):
-        lvm.listing.pretty_report({0: [{'type': 'data', 'path': '/dev/sda1'}]})
+        lvm.listing.pretty_report({0: [
+            {'type': 'data', 'path': '/dev/sda1', 'devices': ['/dev/sda']}
+        ]})
         stdout, stderr = capsys.readouterr()
         assert '====== osd.0 =======' in stdout
 
@@ -36,12 +40,20 @@ def test_tags_are_included(self, capsys):
             {0: [{
                 'type': 'data',
                 'path': '/dev/sda1',
-                'tags': {'ceph.osd_id': '0'}
+                'tags': {'ceph.osd_id': '0'},
+                'devices': ['/dev/sda'],
             }]}
         )
         stdout, stderr = capsys.readouterr()
         assert 'osd id' in stdout
 
+    def test_devices_are_comma_separated(self, capsys):
+        lvm.listing.pretty_report({0: [
+            {'type': 'data', 'path': '/dev/sda1', 'devices': ['/dev/sda', '/dev/sdb1']}
+        ]})
+        stdout, stderr = capsys.readouterr()
+        assert '/dev/sda,/dev/sdb1' in stdout
+
 
 class TestList(object):
 
@@ -155,22 +167,85 @@ def test_report_a_ceph_lv(self, volumes, monkeypatch):
         # ceph lvs are detected by looking into its tags
         tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=data'
         lv = api.Volume(
-            lv_name='lv', vg_name='VolGroup', lv_path='/dev/VolGroup/lv', lv_tags=tags)
+            lv_name='lv', vg_name='VolGroup',
+            lv_uuid='aaaa', lv_path='/dev/VolGroup/lv', lv_tags=tags
+        )
         volumes.append(lv)
         monkeypatch.setattr(lvm.listing.api, 'Volumes', lambda: volumes)
         result = lvm.listing.List([]).single_report('VolGroup/lv')
         assert result['0'][0]['name'] == 'lv'
         assert result['0'][0]['lv_tags'] == tags
         assert result['0'][0]['path'] == '/dev/VolGroup/lv'
+        assert result['0'][0]['devices'] == []
 
     def test_report_a_ceph_journal_device(self, volumes, monkeypatch):
         # ceph lvs are detected by looking into its tags
         tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=data,ceph.journal_device=/dev/sda1'
         lv = api.Volume(
-            lv_name='lv', vg_name='VolGroup', lv_path='/dev/VolGroup/lv', lv_tags=tags)
+            lv_name='lv', vg_name='VolGroup', lv_path='/dev/VolGroup/lv',
+            lv_uuid='aaa', lv_tags=tags)
         volumes.append(lv)
         monkeypatch.setattr(lvm.listing.api, 'Volumes', lambda: volumes)
         result = lvm.listing.List([]).single_report('/dev/sda1')
         assert result['0'][0]['tags'] == {'PARTUUID': 'x'}
         assert result['0'][0]['type'] == 'journal'
         assert result['0'][0]['path'] == '/dev/sda1'
+
+    def test_report_a_ceph_lv_with_devices(self, volumes, monkeypatch):
+        tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=data'
+        lv = api.Volume(
+            lv_name='lv', vg_name='VolGroup',
+            lv_uuid='aaaa', lv_path='/dev/VolGroup/lv', lv_tags=tags
+        )
+        volumes.append(lv)
+        monkeypatch.setattr(lvm.listing.api, 'Volumes', lambda: volumes)
+        listing = lvm.listing.List([])
+        listing._pvs = [
+            {'lv_uuid': 'aaaa', 'pv_name': '/dev/sda1', 'pv_tags': '', 'pv_uuid': ''},
+            {'lv_uuid': 'aaaa', 'pv_name': '/dev/sdb1', 'pv_tags': '', 'pv_uuid': ''},
+        ]
+        result = listing.single_report('VolGroup/lv')
+        assert result['0'][0]['name'] == 'lv'
+        assert result['0'][0]['lv_tags'] == tags
+        assert result['0'][0]['path'] == '/dev/VolGroup/lv'
+        assert result['0'][0]['devices'] == ['/dev/sda1', '/dev/sdb1']
+
+    def test_report_a_ceph_lv_with_no_matching_devices(self, volumes, monkeypatch):
+        tags = 'ceph.osd_id=0,ceph.journal_uuid=x,ceph.type=data'
+        lv = api.Volume(
+            lv_name='lv', vg_name='VolGroup',
+            lv_uuid='aaaa', lv_path='/dev/VolGroup/lv', lv_tags=tags
+        )
+        volumes.append(lv)
+        monkeypatch.setattr(lvm.listing.api, 'Volumes', lambda: volumes)
+        listing = lvm.listing.List([])
+        listing._pvs = [
+            {'lv_uuid': 'ffff', 'pv_name': '/dev/sda1', 'pv_tags': '', 'pv_uuid': ''},
+            {'lv_uuid': 'ffff', 'pv_name': '/dev/sdb1', 'pv_tags': '', 'pv_uuid': ''},
+        ]
+        result = listing.single_report('VolGroup/lv')
+        assert result['0'][0]['name'] == 'lv'
+        assert result['0'][0]['lv_tags'] == tags
+        assert result['0'][0]['path'] == '/dev/VolGroup/lv'
+        assert result['0'][0]['devices'] == []
+
+
+class TestListingPVs(object):
+
+    def setup(self):
+        self.default_pvs = [
+            {'lv_uuid': 'ffff', 'pv_name': '/dev/sda1', 'pv_tags': '', 'pv_uuid': ''},
+            {'lv_uuid': 'ffff', 'pv_name': '/dev/sdb1', 'pv_tags': '', 'pv_uuid': ''},
+        ]
+
+    def test_pvs_is_unset(self, monkeypatch):
+        monkeypatch.setattr(lvm.listing.api, 'get_api_pvs', lambda: self.default_pvs)
+        listing = lvm.listing.List([])
+        assert listing.pvs == self.default_pvs
+
+    def test_pvs_is_set(self, monkeypatch):
+        # keep it patched so that we can fail if this gets returned
+        monkeypatch.setattr(lvm.listing.api, 'get_api_pvs', lambda: self.default_pvs)
+        listing = lvm.listing.List([])
+        listing._pvs = []
+        assert listing.pvs == []