From b4190c3cb407c22921eac7ed249631ec0c05003e Mon Sep 17 00:00:00 2001 From: James Page Date: Mon, 14 Apr 2014 15:41:11 +0100 Subject: [PATCH] Revert local changes, use better disk cleaning --- .../contrib/storage/linux/utils.py | 15 ++++++++++++--- hooks/cinder_utils.py | 17 ++++++----------- unit_tests/test_cinder_utils.py | 19 +++++++++++++------ 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/hooks/charmhelpers/contrib/storage/linux/utils.py b/hooks/charmhelpers/contrib/storage/linux/utils.py index 5349c3ea..eed99ae3 100644 --- a/hooks/charmhelpers/contrib/storage/linux/utils.py +++ b/hooks/charmhelpers/contrib/storage/linux/utils.py @@ -2,7 +2,9 @@ from os import stat from stat import S_ISBLK from subprocess import ( - check_call + check_call, + check_output, + call ) @@ -22,5 +24,12 @@ def zap_disk(block_device): :param block_device: str: Full path of block device to clean. ''' - check_call(['sgdisk', '--zap-all', '--clear', - '--mbrtogpt', block_device]) + # sometimes sgdisk exits non-zero; this is OK, dd will clean up + call(['sgdisk', '--zap-all', '--mbrtogpt', + '--clear', block_device]) + dev_end = check_output(['blockdev', '--getsz', block_device]) + gpt_end = int(dev_end.split()[0]) - 100 + check_call(['dd', 'if=/dev/zero', 'of=%s'%(block_device), + 'bs=1M', 'count=1']) + check_call(['dd', 'if=/dev/zero', 'of=%s'%(block_device), + 'bs=512', 'count=100', 'seek=%s'%(gpt_end)]) diff --git a/hooks/cinder_utils.py b/hooks/cinder_utils.py index 86fb701b..ee60b426 100644 --- a/hooks/cinder_utils.py +++ b/hooks/cinder_utils.py @@ -319,16 +319,6 @@ def configure_lvm_storage(block_devices, volume_group, overwrite=False): extend_lvm_volume_group(volume_group, new_device) -def lvm_zap_disk(block_device): - ''' - Clear a block device of partition table. Relies on sgdisk, which is - installed as pat of the 'gdisk' package in Ubuntu. - - :param block_device: str: Full path of block device to clean. - ''' - subprocess.check_call(['sgdisk', '--zap-all', block_device]) - - def clean_storage(block_device): '''Ensures a block device is clean. That is: - unmounted @@ -343,7 +333,12 @@ def clean_storage(block_device): juju_log('clean_storage(): Found %s mounted @ %s, unmounting.' % (d, mp)) umount(mp, persist=True) - lvm_zap_disk(block_device) + + if is_lvm_physical_volume(block_device): + deactivate_lvm_volume_group(block_device) + remove_lvm_physical_volume(block_device) + + zap_disk(block_device) def _parse_block_device(block_device): diff --git a/unit_tests/test_cinder_utils.py b/unit_tests/test_cinder_utils.py index 29433658..b6cf09e4 100644 --- a/unit_tests/test_cinder_utils.py +++ b/unit_tests/test_cinder_utils.py @@ -172,22 +172,29 @@ class TestCinderUtils(CharmTestCase): ]) self.assertEquals(cinder_utils.restart_map(), ex_map) - @patch.object(cinder_utils, 'lvm_zap_disk') - def test_clean_storage_unmount(self, zap_disk): + def test_clean_storage_unmount(self): 'It unmounts block device when cleaning storage' self.is_lvm_physical_volume.return_value = False + self.zap_disk.return_value = True self.mounts.return_value = MOUNTS cinder_utils.clean_storage('/dev/vdb') self.umount.called_with('/dev/vdb', True) - zap_disk.assert_called_with('/dev/vdb') - @patch.object(cinder_utils, 'lvm_zap_disk') - def test_clean_storage_zap_disk(self, zap_disk): + def test_clean_storage_lvm_wipe(self): + 'It removes traces of LVM when cleaning storage' + self.mounts.return_value = [] + self.is_lvm_physical_volume.return_value = True + cinder_utils.clean_storage('/dev/vdb') + self.remove_lvm_physical_volume.assert_called_with('/dev/vdb') + self.deactivate_lvm_volume_group.assert_called_with('/dev/vdb') + self.zap_disk.assert_called_with('/dev/vdb') + + def test_clean_storage_zap_disk(self): 'It removes traces of LVM when cleaning storage' self.mounts.return_value = [] self.is_lvm_physical_volume.return_value = False cinder_utils.clean_storage('/dev/vdb') - zap_disk.assert_called_with('/dev/vdb') + self.zap_disk.assert_called_with('/dev/vdb') def test_parse_block_device(self): self.assertTrue(cinder_utils._parse_block_device(None),