openstack
/
charm-ceph-osd
Code Issues Proposed changes

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
This commit is contained in:
Nikhil Kshirsagar 2021-08-13 16:59:42 +05:30
parent e5ac333a97
commit 489a4ede69
2 changed files with 33 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
actions/zap_disk.py
View File

@ -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:

21
unit_tests/test_actions_zap_disk.py
View File

@ -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()