diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index e3af49cf9..22ed99b44 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -568,8 +568,25 @@ class GenericHardwareManager(HardwareManager): block_device.name) return - if self._ata_erase(block_device): - return + # Note(TheJulia) Use try/except to capture and log the failure + # and then revert to attempting to shred the volume if enabled. + try: + if self._ata_erase(block_device): + return + except errors.BlockDeviceEraseError as e: + info = node.get('driver_internal_info', {}) + execute_shred = info.get( + 'agent_continue_if_ata_erase_failed', False) + if execute_shred: + LOG.warning('Failed to invoke ata_erase, ' + 'falling back to shred: %(err)s' + % {'err': e}) + else: + msg = ('Failed to invoke ata_erase, ' + 'fallback to shred is not enabled: %(err)s' + % {'err': e}) + LOG.error(msg) + raise errors.IncompatibleHardwareMethodError(msg) if self._shred_block_device(node, block_device): return @@ -643,6 +660,20 @@ class GenericHardwareManager(HardwareManager): if 'supported' not in security_lines: return False + if 'enabled' in security_lines: + # Attempt to unlock the drive in the event it has already been + # locked by a previous failed attempt. + try: + utils.execute('hdparm', '--user-master', 'u', + '--security-unlock', 'NULL', block_device.name) + security_lines = self._get_ata_security_lines(block_device) + except processutils.ProcessExecutionError as e: + raise errors.BlockDeviceEraseError('Security password set ' + 'failed for device ' + '%(name)s: %(err)s' % + {'name': block_device.name, + 'err': e}) + if 'enabled' in security_lines: raise errors.BlockDeviceEraseError( ('Block device {0} already has a security password set' @@ -653,16 +684,29 @@ class GenericHardwareManager(HardwareManager): ('Block device {0} is frozen and cannot be erased' ).format(block_device.name)) - utils.execute('hdparm', '--user-master', 'u', '--security-set-pass', - 'NULL', block_device.name) + try: + utils.execute('hdparm', '--user-master', 'u', + '--security-set-pass', 'NULL', block_device.name) + except processutils.ProcessExecutionError as e: + raise errors.BlockDeviceEraseError('Security password set ' + 'failed for device ' + '%(name)s: %(err)s' % + {'name': block_device.name, + 'err': e}) # Use the 'enhanced' security erase option if it's supported. erase_option = '--security-erase' if 'not supported: enhanced erase' not in security_lines: erase_option += '-enhanced' - utils.execute('hdparm', '--user-master', 'u', erase_option, - 'NULL', block_device.name) + try: + utils.execute('hdparm', '--user-master', 'u', erase_option, + 'NULL', block_device.name) + except processutils.ProcessExecutionError as e: + raise errors.BlockDeviceEraseError('Erase failed for device ' + '%(name)s: %(err)s' % + {'name': block_device.name, + 'err': e}) # Verify that security is now 'not enabled' security_lines = self._get_ata_security_lines(block_device) diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 9bf0b7bf4..91ae1e791 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -744,8 +744,10 @@ class TestGenericHardwareManager(test_base.BaseTestCase): 'shred', '--force', '--zero', '--verbose', '--iterations', '1', '/dev/sda') + @mock.patch.object(hardware.GenericHardwareManager, '_shred_block_device') @mock.patch.object(utils, 'execute') - def test_erase_block_device_ata_security_enabled(self, mocked_execute): + def test_erase_block_device_ata_security_enabled( + self, mocked_execute, mock_shred): hdparm_output = HDPARM_INFO_TEMPLATE % { 'supported': '\tsupported', 'enabled': '\tenabled', @@ -754,17 +756,143 @@ class TestGenericHardwareManager(test_base.BaseTestCase): } mocked_execute.side_effect = [ + (hdparm_output, ''), + None, (hdparm_output, '') ] block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824, True) - self.assertRaises(errors.BlockDeviceEraseError, - self.hardware.erase_block_device, - self.node, block_device) + + self.assertRaises( + errors.IncompatibleHardwareMethodError, + self.hardware.erase_block_device, + self.node, + block_device) + self.assertFalse(mock_shred.called) + + @mock.patch.object(hardware.GenericHardwareManager, '_shred_block_device') + @mock.patch.object(utils, 'execute') + def test_erase_block_device_ata_security_enabled_unlock_attempt( + self, mocked_execute, mock_shred): + hdparm_output = HDPARM_INFO_TEMPLATE % { + 'supported': '\tsupported', + 'enabled': '\tenabled', + 'frozen': 'not\tfrozen', + 'enhanced_erase': 'not\tsupported: enhanced erase', + } + + hdparm_output_not_enabled = HDPARM_INFO_TEMPLATE % { + 'supported': '\tsupported', + 'enabled': 'not\tenabled', + 'frozen': 'not\tfrozen', + 'enhanced_erase': 'not\tsupported: enhanced erase', + } + + mocked_execute.side_effect = [ + (hdparm_output, ''), + '', + (hdparm_output_not_enabled, ''), + '', + '', + (hdparm_output_not_enabled, '') + ] + + block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824, + True) + + self.hardware.erase_block_device(self.node, block_device) + self.assertFalse(mock_shred.called) @mock.patch.object(utils, 'execute') - def test_erase_block_device_ata_frozen(self, mocked_execute): + def test__ata_erase_security_enabled_unlock_exception( + self, mocked_execute): + hdparm_output = HDPARM_INFO_TEMPLATE % { + 'supported': '\tsupported', + 'enabled': '\tenabled', + 'frozen': 'not\tfrozen', + 'enhanced_erase': 'not\tsupported: enhanced erase', + } + + mocked_execute.side_effect = [ + (hdparm_output, ''), + processutils.ProcessExecutionError() + ] + + block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824, + True) + + self.assertRaises(errors.BlockDeviceEraseError, + self.hardware._ata_erase, + block_device) + + @mock.patch.object(utils, 'execute') + def test__ata_erase_security_enabled_set_password_exception( + self, mocked_execute): + hdparm_output = HDPARM_INFO_TEMPLATE % { + 'supported': '\tsupported', + 'enabled': '\tenabled', + 'frozen': 'not\tfrozen', + 'enhanced_erase': 'not\tsupported: enhanced erase', + } + + hdparm_output_not_enabled = HDPARM_INFO_TEMPLATE % { + 'supported': '\tsupported', + 'enabled': 'not\tenabled', + 'frozen': 'not\tfrozen', + 'enhanced_erase': 'not\tsupported: enhanced erase', + } + + mocked_execute.side_effect = [ + (hdparm_output, ''), + '', + (hdparm_output_not_enabled, ''), + '', + processutils.ProcessExecutionError() + ] + + block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824, + True) + + self.assertRaises(errors.BlockDeviceEraseError, + self.hardware._ata_erase, + block_device) + + @mock.patch.object(utils, 'execute') + def test__ata_erase_security_erase_exec_exception( + self, mocked_execute): + hdparm_output = HDPARM_INFO_TEMPLATE % { + 'supported': '\tsupported', + 'enabled': '\tenabled', + 'frozen': 'not\tfrozen', + 'enhanced_erase': 'not\tsupported: enhanced erase', + } + + hdparm_output_not_enabled = HDPARM_INFO_TEMPLATE % { + 'supported': '\tsupported', + 'enabled': 'not\tenabled', + 'frozen': 'not\tfrozen', + 'enhanced_erase': 'not\tsupported: enhanced erase', + } + + mocked_execute.side_effect = [ + (hdparm_output, '', '-1'), + '', + (hdparm_output_not_enabled, ''), + '', + processutils.ProcessExecutionError() + ] + + block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824, + True) + + self.assertRaises(errors.BlockDeviceEraseError, + self.hardware._ata_erase, + block_device) + + @mock.patch.object(hardware.GenericHardwareManager, '_shred_block_device') + @mock.patch.object(utils, 'execute') + def test_erase_block_device_ata_frozen(self, mocked_execute, mock_shred): hdparm_output = HDPARM_INFO_TEMPLATE % { 'supported': '\tsupported', 'enabled': 'not\tenabled', @@ -778,12 +906,16 @@ class TestGenericHardwareManager(test_base.BaseTestCase): block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824, True) - self.assertRaises(errors.BlockDeviceEraseError, - self.hardware.erase_block_device, - self.node, block_device) + self.assertRaises( + errors.IncompatibleHardwareMethodError, + self.hardware.erase_block_device, + self.node, + block_device) + self.assertFalse(mock_shred.called) + @mock.patch.object(hardware.GenericHardwareManager, '_shred_block_device') @mock.patch.object(utils, 'execute') - def test_erase_block_device_ata_failed(self, mocked_execute): + def test_erase_block_device_ata_failed(self, mocked_execute, mock_shred): hdparm_output_before = HDPARM_INFO_TEMPLATE % { 'supported': '\tsupported', 'enabled': 'not\tenabled', @@ -809,9 +941,52 @@ class TestGenericHardwareManager(test_base.BaseTestCase): block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824, True) - self.assertRaises(errors.BlockDeviceEraseError, - self.hardware.erase_block_device, - self.node, block_device) + + self.assertRaises( + errors.IncompatibleHardwareMethodError, + self.hardware.erase_block_device, + self.node, + block_device) + self.assertFalse(mock_shred.called) + + @mock.patch.object(hardware.GenericHardwareManager, '_shred_block_device') + @mock.patch.object(utils, 'execute') + def test_erase_block_device_ata_failed_continued( + self, + mocked_execute, + mock_shred): + + info = self.node.get('driver_internal_info') + info['agent_continue_if_ata_erase_failed'] = True + + hdparm_output_before = HDPARM_INFO_TEMPLATE % { + 'supported': '\tsupported', + 'enabled': 'not\tenabled', + 'frozen': 'not\tfrozen', + 'enhanced_erase': 'not\tsupported: enhanced erase', + } + + # If security mode remains enabled after the erase, it is indiciative + # of a failed erase. + hdparm_output_after = HDPARM_INFO_TEMPLATE % { + 'supported': '\tsupported', + 'enabled': '\tenabled', + 'frozen': 'not\tfrozen', + 'enhanced_erase': 'not\tsupported: enhanced erase', + } + + mocked_execute.side_effect = [ + (hdparm_output_before, ''), + ('', ''), + ('', ''), + (hdparm_output_after, ''), + ] + + block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824, + True) + + self.hardware.erase_block_device(self.node, block_device) + self.assertTrue(mock_shred.called) def test_normal_vs_enhanced_security_erase(self): @mock.patch.object(utils, 'execute') diff --git a/releasenotes/notes/enable-cleaning-fallback-57e8c9aa2f24e63d.yaml b/releasenotes/notes/enable-cleaning-fallback-57e8c9aa2f24e63d.yaml new file mode 100644 index 000000000..5c46cbadf --- /dev/null +++ b/releasenotes/notes/enable-cleaning-fallback-57e8c9aa2f24e63d.yaml @@ -0,0 +1,14 @@ +--- +features: + - The driver_internal_info internal setting + ``agent_continue_if_ata_erase_failed`` allows operators + to enable disk cleaning operations to fallback from a failed + ata_erase operation to disk shredding operations. +fixes: + - IPA will now attempt to unlock a security locked drive + with a 'NULL' password if it is found to be enabled, + however this will only work if the password was previously + set to a 'NULL' value, such as if a failure during a previous + ata_erase sequence. + - Potential command failures in the secure erase process are + now captured and raised as BlockDeviceEraseError exceptions.