diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 14c2cceef..b87f1a04d 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -1018,6 +1018,18 @@ class GenericHardwareManager(HardwareManager): block_device.name) return info = node.get('driver_internal_info', {}) + if self._is_read_only_device(block_device): + if info.get('agent_erase_skip_read_only', False): + LOG.info("Skipping erase of read-only device %s", + block_device.name) + return + else: + msg = ('Failed to invoke erase of device %(device)s ' + 'as the device is flagged read-only, and the ' + 'conductor has not signaled this is a permitted ' + 'case.' % {'device': block_device.name}) + LOG.error(msg) + raise errors.BlockDeviceEraseError(msg) # Note(TheJulia) Use try/except to capture and log the failure # and then revert to attempting to shred the volume if enabled. try: @@ -1070,6 +1082,10 @@ class GenericHardwareManager(HardwareManager): LOG.info("Skipping metadata erase of RAID member device %s", dev.name) continue + if self._is_read_only_device(dev): + LOG.info("Skipping metadata erase of read-only device %s", + dev.name) + continue try: disk_utils.destroy_disk_metadata(dev.name, node['uuid']) @@ -1143,6 +1159,28 @@ class GenericHardwareManager(HardwareManager): return 'linux_raid_member' in out + def _is_read_only_device(self, block_device): + """Check if a block device is read-only. + + Checks the device read-only flag in order to identify virtual + and firmware driven devices that block write device access. + + :param block_device: a BlockDevice object + :returns: True if the device is read-only. + """ + try: + dev_name = str(block_device.name)[5:] + + with open('/sys/block/%s/ro' % dev_name, 'r') as f: + flag = f.read().strip() + if flag == '1': + return True + except IOError as e: + LOG.warning("Could not determine if %s is a read-only device. " + "Error: %s", + block_device.name, e) + return False + def _get_ata_security_lines(self, block_device): output = utils.execute('hdparm', '-I', block_device.name)[0] diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index af2045b66..86e3a432f 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -2593,6 +2593,33 @@ class TestGenericHardwareManager(base.IronicAgentTest): mocked_execute.return_value = 'md0', '' self.assertFalse(self.hardware._is_linux_raid_member(raid_member)) + def test__is_read_only_device(self): + fileobj = mock.mock_open(read_data='1\n') + device = hardware.BlockDevice('/dev/sdfake', 'fake', 1024, False) + with mock.patch( + 'builtins.open', fileobj, create=True) as mock_open: + self.assertTrue(self.hardware._is_read_only_device(device)) + mock_open.assert_called_once_with( + '/sys/block/sdfake/ro', 'r') + + def test__is_read_only_device_false(self): + fileobj = mock.mock_open(read_data='0\n') + device = hardware.BlockDevice('/dev/sdfake', 'fake', 1024, False) + with mock.patch( + 'builtins.open', fileobj, create=True) as mock_open: + self.assertFalse(self.hardware._is_read_only_device(device)) + mock_open.assert_called_once_with( + '/sys/block/sdfake/ro', 'r') + + def test__is_read_only_device_error(self): + device = hardware.BlockDevice('/dev/sdfake', 'fake', 1024, False) + with mock.patch( + 'builtins.open', side_effect=IOError, + autospec=True) as mock_open: + self.assertFalse(self.hardware._is_read_only_device(device)) + mock_open.assert_called_once_with( + '/sys/block/sdfake/ro', 'r') + @mock.patch.object(utils, 'execute', autospec=True) def test_get_bmc_address(self, mocked_execute): mocked_execute.return_value = '192.1.2.3\n', '' diff --git a/releasenotes/notes/fix-cleaning-read-only-device-c8a0f4cc2f434d99.yaml b/releasenotes/notes/fix-cleaning-read-only-device-c8a0f4cc2f434d99.yaml new file mode 100644 index 000000000..ece06338b --- /dev/null +++ b/releasenotes/notes/fix-cleaning-read-only-device-c8a0f4cc2f434d99.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixes an issue where metadata erasure cleaning would fail on devices + that are read-only at the hardware level. Typically these are virtual + devices being offered to the operating system for purposes like OS + self-installation. + + In the case of full device erasure, this is explicitly raised as + a hard failure requiring operator intervention.