From cd7b2693f873bde72706f8b6ab8c1f21e68f0fd1 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Tue, 28 Jan 2020 22:41:47 -0600 Subject: [PATCH] Skip read-only devices with metadata erase HPE "Virtual Install Devices" appear as read-only block devices, and may... or may not be visible depending on the bios configuration state. These devices can no longer be disabled from the bios settings so the simplest course of action seems to be that we should handle the existence of a read-only device. In the event of secure erase, this is treated as a hard failure case and a driver_internal_info flag has been added to enable a future bypass method for knowledgable operators. Change-Id: Ief8b360d11e654d8fae3a04a2a9f8d474a06e167 Story: 2007229 Task: 38502 --- ironic_python_agent/hardware.py | 38 +++++++++++++++++++ .../tests/unit/test_hardware.py | 27 +++++++++++++ ...ing-read-only-device-c8a0f4cc2f434d99.yaml | 10 +++++ 3 files changed, 75 insertions(+) create mode 100644 releasenotes/notes/fix-cleaning-read-only-device-c8a0f4cc2f434d99.yaml 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.