Skip read-only devices with metadata erase3.6.3

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.

Backport note: the unit tests have been modified to account for Python 2

Conflicts:
	ironic_python_agent/hardware.py
	ironic_python_agent/tests/unit/test_hardware.py

Change-Id: Ief8b360d11e654d8fae3a04a2a9f8d474a06e167
Story: 2007229
Task: 38502
(cherry picked from commit cd7b2693f873bde72706f8b6ab8c1f21e68f0fd1)

3 files changed, 75 insertions, 0 deletions

diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index a1a92fd1..eae0bcf8 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -843,6 +843,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:
@@ -891,6 +903,10 @@ class GenericHardwareManager(HardwareManager):
                 LOG.info("Skipping the erase of virtual media 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'])
@@ -945,6 +961,28 @@ class GenericHardwareManager(HardwareManager):
                 return True
         return False
 
+    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 1687af1c..4b75cfe0 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -2247,6 +2247,33 @@ class TestGenericHardwareManager(base.IronicAgentTest):
                           mock.call(self.hardware, block_devices[0])],
                          mock__is_vmedia.call_args_list)
 
+    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(
+                'six.moves.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(
+                'six.moves.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(
+                'six.moves.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 00000000..ece06338
--- /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.