Merge "Erase expected GPT locations in metadata wipe" into stable/rocky

ironic_lib/disk_utils.py

@@ -411,7 +411,20 @@ def destroy_disk_metadata(dev, node_uuid):
                 utils.execute('wipefs', '--all', dev,
                               run_as_root=True,
                               use_standard_locale=True)
-
+    # NOTE(TheJulia): sgdisk attempts to load and make sense of the
+    # partition tables in advance of wiping the partition data.
+    # This means when a CRC error is found, sgdisk fails before
+    # erasing partition data.
+    # This is the same bug as
+    # https://bugs.launchpad.net/ironic-python-agent/+bug/1737556
+    dd_device = 'of=%s' % dev
+    gpt_backup = get_dev_block_size(dev) - 33
+    dd_seek = 'seek=%i' % gpt_backup
+    utils.execute('dd', 'bs=512', 'if=/dev/zero', dd_device, 'count=33',
+                  run_as_root=True, use_standard_locale=True)
+    utils.execute('dd', 'bs=512', 'if=/dev/zero', dd_device, 'count=33',
+                  dd_seek, run_as_root=True, use_standard_locale=True)
+    # Go ahead and let sgdisk run as well.
     utils.execute('sgdisk', '-Z', dev, run_as_root=True,
                   use_standard_locale=True)
 

ironic_lib/tests/test_disk_utils.py

@@ -569,7 +569,7 @@ class MakePartitionsTestCase(base.IronicLibTestCase):
         self.assertEqual(expected_result, result)
 
 
-@mock.patch.object(utils, 'execute', autospec=True, return_value=('', ''))
+@mock.patch.object(utils, 'execute', autospec=True)
 class DestroyMetaDataTestCase(base.IronicLibTestCase):
 
     def setUp(self):
@@ -578,9 +578,31 @@ class DestroyMetaDataTestCase(base.IronicLibTestCase):
         self.node_uuid = "12345678-1234-1234-1234-1234567890abcxyz"
 
     def test_destroy_disk_metadata(self, mock_exec):
+        # Note(TheJulia): This list will get-reused, but only the second
+        # execution returning a string is needed for the test as otherwise
+        # command output is not used.
+        mock_exec.side_effect = iter([
+            (None, None),
+            ('1024\n', None),
+            (None, None),
+            (None, None),
+            (None, None),
+            (None, None)])
+
         expected_calls = [mock.call('wipefs', '--force', '--all', 'fake-dev',
                                     run_as_root=True,
                                     use_standard_locale=True),
+                          mock.call('blockdev', '--getsz', 'fake-dev',
+                                    check_exit_code=[0],
+                                    run_as_root=True),
+                          mock.call('dd', 'bs=512', 'if=/dev/zero',
+                                    'of=fake-dev', 'count=33',
+                                    run_as_root=True,
+                                    use_standard_locale=True),
+                          mock.call('dd', 'bs=512', 'if=/dev/zero',
+                                    'of=fake-dev', 'count=33', 'seek=991',
+                                    run_as_root=True,
+                                    use_standard_locale=True),
                           mock.call('sgdisk', '-Z', 'fake-dev',
                                     run_as_root=True,
                                     use_standard_locale=True),
@@ -606,11 +628,26 @@ class DestroyMetaDataTestCase(base.IronicLibTestCase):
         expected_calls = [mock.call('wipefs', '--force', '--all', 'fake-dev',
                                     run_as_root=True,
                                     use_standard_locale=True),
+                          mock.call('blockdev', '--getsz', 'fake-dev',
+                                    check_exit_code=[0],
+                                    run_as_root=True),
+                          mock.call('dd', 'bs=512', 'if=/dev/zero',
+                                    'of=fake-dev', 'count=33',
+                                    run_as_root=True,
+                                    use_standard_locale=True),
+                          mock.call('dd', 'bs=512', 'if=/dev/zero',
+                                    'of=fake-dev', 'count=33', 'seek=991',
+                                    run_as_root=True,
+                                    use_standard_locale=True),
                           mock.call('sgdisk', '-Z', 'fake-dev',
                                     run_as_root=True,
                                     use_standard_locale=True)]
-        mock_exec.side_effect = [(None, None),
+        mock_exec.side_effect = iter([
-                                 processutils.ProcessExecutionError()]
+            (None, None),
+            ('1024\n', None),
+            (None, None),
+            (None, None),
+            processutils.ProcessExecutionError()])
         self.assertRaises(processutils.ProcessExecutionError,
                           disk_utils.destroy_disk_metadata,
                           self.dev,
@@ -618,11 +655,14 @@ class DestroyMetaDataTestCase(base.IronicLibTestCase):
         mock_exec.assert_has_calls(expected_calls)
 
     def test_destroy_disk_metadata_wipefs_not_support_force(self, mock_exec):
-        mock_exec.side_effect = iter(
+        mock_exec.side_effect = iter([
-            [processutils.ProcessExecutionError(description='--force'),
+            processutils.ProcessExecutionError(description='--force'),
-             (None, None),
+            (None, None),
-             (None, None),
+            ('1024\n', None),
-             ('', '')])
+            (None, None),
+            (None, None),
+            (None, None),
+            (None, None)])
 
         expected_call = [mock.call('wipefs', '--force', '--all', 'fake-dev',
                                    run_as_root=True,

releasenotes/notes/wipe-gpt-on-metadata-wipe-ac0a93b16e00893f.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes an issue where CRC errors in the GPT partition information would
+    cause cleaning to fail. A similar issue was previously encountered with
+    ironic-python-agent. See `Story 1737556 <https://storyboard.openstack.org/#!/story/1737556>`_
+    for details.