From 489a4ede69c72ce930a0909af5fffe2a9faa8d1a Mon Sep 17 00:00:00 2001 From: Nikhil Kshirsagar Date: Fri, 13 Aug 2021 16:59:42 +0530 Subject: [PATCH] Do not zap a disk if it is used by lvm2 If the disk being zapped is used by lvm (if it contains the lvm label and hasn't been pvremove'd) it's safer to simply bail out of zapping it than attempt teardown through a force pvremove, because the disk being zapped might be in fact in use by some LV. Closes-Bug: 1858519 Change-Id: I111475c5a4584a3e367c604ab51ce2ef3789ff7f --- actions/zap_disk.py | 14 ++++++++++++-- unit_tests/test_actions_zap_disk.py | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/actions/zap_disk.py b/actions/zap_disk.py index 1002c742..22733449 100755 --- a/actions/zap_disk.py +++ b/actions/zap_disk.py @@ -29,6 +29,8 @@ from charmhelpers.contrib.storage.linux.utils import ( from charmhelpers.core.unitdata import kv from charms_ceph.utils import is_active_bluestore_device from charms_ceph.utils import is_mapped_luks_device +from charmhelpers.contrib.storage.linux.lvm import is_lvm_physical_volume +from charmhelpers.core.hookenv import log class ZapDiskError(Exception): @@ -61,6 +63,7 @@ def zap(): failed_devices = [] not_block_devices = [] + lvm_devices = [] try: devices = get_devices() except ZapDiskError as error: @@ -68,6 +71,8 @@ def zap(): return for device in devices: + if is_lvm_physical_volume(device): + lvm_devices.append(device) if not is_block_device(device): not_block_devices.append(device) if (is_device_mounted(device) or @@ -75,10 +80,15 @@ def zap(): is_mapped_luks_device(device)): failed_devices.append(device) - if failed_devices or not_block_devices: + if lvm_devices or failed_devices or not_block_devices: message = "" + if lvm_devices: + log('Cannot zap a device used by lvm') + message = "{} devices are lvm devices: {}".format( + len(lvm_devices), + ", ".join(lvm_devices)) if failed_devices: - message = "{} devices are mounted: {}".format( + message += "{} devices are mounted: {}".format( len(failed_devices), ", ".join(failed_devices)) if not_block_devices: diff --git a/unit_tests/test_actions_zap_disk.py b/unit_tests/test_actions_zap_disk.py index 21a6ffcc..375b026f 100644 --- a/unit_tests/test_actions_zap_disk.py +++ b/unit_tests/test_actions_zap_disk.py @@ -27,11 +27,13 @@ class ZapDiskActionTests(CharmTestCase): 'is_device_mounted', 'is_active_bluestore_device', 'is_mapped_luks_device', + 'is_lvm_physical_volume', 'kv']) self.is_device_mounted.return_value = False self.is_block_device.return_value = True self.is_active_bluestore_device.return_value = False self.is_mapped_luks_device.return_value = False + self.is_lvm_physical_volume.return_value = False self.kv.return_value = self.kv self.hookenv.local_unit.return_value = "ceph-osd-test/0" @@ -215,3 +217,22 @@ class ZapDiskActionTests(CharmTestCase): self.hookenv.action_fail.assert_called_with( 'Failed due to: not-absolute: Not absolute path.') self.hookenv.action_set.assert_not_called() + + @mock.patch('os.path.exists', mock.MagicMock(return_value=True)) + @mock.patch.object(zap_disk, 'zap_disk') + def test_wont_zap_lvm_device(self, _zap_disk): + """Won't zap lvm disk""" + def side_effect(arg): + return { + 'devices': '/dev/vdb', + 'i-really-mean-it': True, + }.get(arg) + + self.hookenv.action_get.side_effect = side_effect + self.is_lvm_physical_volume.return_value = True + + zap_disk.zap() + _zap_disk.assert_not_called() + self.hookenv.action_fail.assert_called_with( + '1 devices are lvm devices: /dev/vdb') + self.hookenv.action_set.assert_not_called()