Try to unlock failed device before proceeding

When a hard error has occured with secure erase,
we should attempt an unlock of the device becuase
the current mode can prevent disk IO. This may upset
some things like raid controllers even if they are
in a pass-through mode.

Change-Id: I32e1d962fbbb4a305d5dbebea92ac48ebd9b67ca
Story: #2002546
Task: #22107

ironic_python_agent/hardware.py

@@ -889,6 +889,31 @@ class GenericHardwareManager(HardwareManager):
         return security_lines
 
     def _ata_erase(self, block_device):
+
+        def __attempt_unlock_drive(block_device, security_lines=None):
+            # Attempt to unlock the drive in the event it has already been
+            # locked by a previous failed attempt. We try the empty string as
+            # versions of hdparm < 9.51, interpreted NULL as the literal
+            # string, "NULL", as opposed to the empty string.
+            if not security_lines:
+                security_lines = self._get_ata_security_lines(block_device)
+            unlock_passwords = ['NULL', '']
+            for password in unlock_passwords:
+                if 'not locked' in security_lines:
+                    break
+                try:
+                    utils.execute('hdparm', '--user-master', 'u',
+                                  '--security-unlock', password,
+                                  block_device.name)
+                except processutils.ProcessExecutionError as e:
+                    LOG.info('Security unlock failed for device '
+                             '%(name)s using password "%(password)s": %(err)s',
+                             {'name': block_device.name,
+                              'password': password,
+                              'err': e})
+                security_lines = self._get_ata_security_lines(block_device)
+            return security_lines
+
         security_lines = self._get_ata_security_lines(block_device)
 
         # If secure erase isn't supported return False so erase_block_device
@@ -907,26 +932,8 @@ class GenericHardwareManager(HardwareManager):
                  ).format(block_device.name))
 
         # At this point, we could be in SEC1,4,5
-
-        # Attempt to unlock the drive in the event it has already been
-        # locked by a previous failed attempt. We try the empty string as
-        # versions of hdparm < 9.51, interpreted NULL as the literal string,
-        # "NULL", as opposed to the empty string.
-        unlock_passwords = ['NULL', '']
-        for password in unlock_passwords:
-            if 'not locked' in security_lines:
-                break
-            try:
-                utils.execute('hdparm', '--user-master', 'u',
-                              '--security-unlock', password,
-                              block_device.name)
-            except processutils.ProcessExecutionError as e:
-                LOG.info('Security unlock failed for device '
-                         '%(name)s using password "%(password)s": %(err)s',
-                         {'name': block_device.name,
-                          'password': password,
-                          'err': e})
-            security_lines = self._get_ata_security_lines(block_device)
+        # Attempt to unlock the drive if it has failed in a prior attempt.
+        security_lines = __attempt_unlock_drive(block_device, security_lines)
 
         # If the unlock failed we will still be in SEC4, otherwise, we will be
         # in SEC1 or SEC5
@@ -959,6 +966,10 @@ class GenericHardwareManager(HardwareManager):
             utils.execute('hdparm', '--user-master', 'u', erase_option,
                           'NULL', block_device.name)
         except processutils.ProcessExecutionError as e:
+            # NOTE(TheJulia): Attempt unlock to allow fallback to shred
+            # to occur, otherwise shred will fail as well, as the security
+            # mode will prevent IO operations to the disk.
+            __attempt_unlock_drive(block_device)
             raise errors.BlockDeviceEraseError('Erase failed for device '
                                                '%(name)s: %(err)s' %
                                                {'name': block_device.name,

ironic_python_agent/tests/unit/test_hardware.py

@@ -1576,11 +1576,15 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         # Exception on security erase
         hdparm_output = create_hdparm_info(
             supported=True, enabled=False, frozen=False, enhanced_erase=False)
-
+        hdparm_unlocked_output = create_hdparm_info(
+            supported=True, locked=True, frozen=False, enhanced_erase=False)
         mocked_execute.side_effect = [
             (hdparm_output, '', '-1'),
             '',  # security-set-pass
-            processutils.ProcessExecutionError()  # security-erase
+            processutils.ProcessExecutionError(),  # security-erase
+            (hdparm_unlocked_output, '', '-1'),
+            '',  # attempt security unlock
+            (hdparm_output, '', '-1')
         ]
 
         block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824,

releasenotes/notes/attempts-ata-disk-unlock-897d76c494ec2976.yaml

@@ -0,0 +1,16 @@
+---
+fixes:
+  - |
+    Fixes the ATA Secure Erase logic to attempt an immediate unlock in the
+    event of a failed attempt to Secure Erase. This is required to permit
+    fallback to make use of the ``shred`` disk utility.
+
+    In the event that an ATA Secure Erase operation fails during cleaning,
+    the disk will be write locked. In this case, the disk must be explicitly
+    unlocked.
+
+    This should also prevent failures where an ATA Secure Erase operation
+    fails with a pass-through disk controller, which may prevent the disk
+    from being available after a reboot operation. For additional information,
+    please see
+    `story 2002546 <https://storyboard.openstack.org/#!/story/2002546>`_.