Merge lp:~raharper/charm-helpers/use-dd-in-zap-disk into lp:charm-helpers

[Ryan Harper]

Update zap_disk to use dd to ensure complete wipe of MBR and GPT data on a disk; sgdisk doesn't always remove the data from the disk even if the command returns without error. This was observed on a cinder install which attempts to pvcreate on a disk, but refuses if there is any GPT related partition table (as observed via fdisk -l /dev/XXX which will complain about GPT tables). This patch has been in use in branches of the cinder/ceph charm here:

lp:~raharper/charms/precise/ceph/use-dd-zap-disk
lp:~raharper/charms/precise/cinder/use-dd-zap-disk

and I've successfully deployed each charm hundreds of times since the change.

=== modified file 'charmhelpers/contrib/storage/linux/utils.py'
--- charmhelpers/contrib/storage/linux/utils.py	2014-03-05 20:46:13 +0000
+++ charmhelpers/contrib/storage/linux/utils.py	2014-03-25 16:07:21 +0000
@@ -2,7 +2,8 @@
 from stat import S_ISBLK
 
 from subprocess import (
-    check_call
+    check_call,
+    check_output,
 )
 
 
@@ -22,5 +23,12 @@
 
     :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
+    check_output(['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)])

=== modified file 'tests/contrib/storage/test_linux_storage_utils.py'
--- tests/contrib/storage/test_linux_storage_utils.py	2014-03-05 20:46:13 +0000
+++ tests/contrib/storage/test_linux_storage_utils.py	2014-03-25 16:07:21 +0000
@@ -8,12 +8,19 @@
 
 
 class MiscStorageUtilsTests(unittest.TestCase):
-    def test_zap_disk(self):
+    @patch(STORAGE_LINUX_UTILS + '.check_call')
+    def test_zap_disk(self, check_call):
         '''It calls sgdisk correctly to zap disk'''
-        with patch(STORAGE_LINUX_UTILS + '.check_call') as check_call:
+        with patch(STORAGE_LINUX_UTILS + '.check_output') as check_output:
+            check_output.return_value = '200\n'
             storage_utils.zap_disk('/dev/foo')
-            check_call.assert_called_with(['sgdisk', '--zap-all', '--clear',
-                                           '--mbrtogpt', '/dev/foo'])
+            check_output.assert_any_call(['sgdisk', '--zap-all', '--mbrtogpt',
+                                          '--clear', '/dev/foo'])
+            check_output.assert_any_call(['blockdev', '--getsz', '/dev/foo'])
+            check_call.assert_any_call(['dd', 'if=/dev/zero', 'of=/dev/foo',
+                                        'bs=1M', 'count=1'])
+            check_call.assert_any_call(['dd', 'if=/dev/zero', 'of=/dev/foo',
+                                        'bs=512', 'count=100', 'seek=100'])
 
     @patch(STORAGE_LINUX_UTILS + '.stat')
     @patch(STORAGE_LINUX_UTILS + '.S_ISBLK')