Add support for root device hints

umago · umago · commit 5a529ca4893f · 2015-03-13T10:42:12.000Z
This patch add support for root device hints on IPA. Instead of picking
the first disk &gt;= 4G, if the hints are specified IPA will look at it
to decide which device it should pick for the deployment. The initial
patch supports the following hints: Size, model, WWN, serial, vendor.

Implements: blueprint ipa-as-default-ramdisk
Implements: blueprint root-device-hints
Change-Id: I2b00b3fb3b61001033750dd8951f9353d6f2e361
diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
@@ -18,7 +18,9 @@
 import shlex
 
 import netifaces
+from oslo_utils import units
 import psutil
+import pyudev
 import six
 import stevedore
 
@@ -256,15 +258,100 @@ def list_block_devices(self):
                                        rotational=bool(int(device['ROTA']))))
         return devices
 
+    def _get_device_vendor(self, dev):
+        """Get the vendor name of a given device."""
+        try:
+            devname = os.path.basename(dev)
+            with open('/sys/class/block/%s/device/vendor' % devname, 'r') as f:
+                return f.read().strip()
+        except IOError:
+            LOG.warning("Can't find the device vendor for device %s", dev)
+
     def get_os_install_device(self):
-        # Find the first device larger than 4GB, assume it is the OS disk
-        # TODO(russellhaering): This isn't a valid assumption in all cases,
-        #                       is there a more reasonable default behavior?
         block_devices = self.list_block_devices()
-        block_devices.sort(key=lambda device: device.size)
-        for device in block_devices:
-            if device.size >= (4 * pow(1024, 3)):
-                return device.name
+        root_device_hints = utils.parse_root_device_hints()
+
+        if not root_device_hints:
+            # If no hints are passed find the first device larger than
+            # 4GB, assume it is the OS disk
+            # TODO(russellhaering): This isn't a valid assumption in
+            # all cases, is there a more reasonable default behavior?
+            block_devices.sort(key=lambda device: device.size)
+            for device in block_devices:
+                if device.size >= (4 * pow(1024, 3)):
+                    return device.name
+        else:
+
+            def match(hint, current_value, device):
+                hint_value = root_device_hints[hint]
+                if hint_value != current_value:
+                    LOG.debug("Root device hint %(hint)s=%(value)s does not "
+                              "match the device %(device)s value of "
+                              "%(current)s", {'hint': hint,
+                              'value': hint_value, 'device': device,
+                              'current': current_value})
+                    return False
+                return True
+
+            context = pyudev.Context()
+            for dev in block_devices:
+                try:
+                    udev = pyudev.Device.from_device_file(context, dev.name)
+                except (ValueError, EnvironmentError) as e:
+                    LOG.warning("Device %(dev)s is inaccessible, skipping... "
+                    "Error: %(error)s", {'dev': dev, 'error': e})
+                    continue
+
+                # TODO(lucasagomes): Add support for operators <, >, =, etc...
+                # to better deal with sizes.
+                if 'size' in root_device_hints:
+                    # Since we don't support units yet we expect the size
+                    # in GiB for now
+                    size = dev.size / units.Gi
+                    if not match('size', size, dev.name):
+                        continue
+
+                if 'model' in root_device_hints:
+                    model = udev.get('ID_MODEL', None)
+                    if not model:
+                        continue
+                    model = utils.normalize(model)
+                    if not match('model', model, dev.name):
+                        continue
+
+                if 'wwn' in root_device_hints:
+                    wwn = udev.get('ID_WWN', None)
+                    if not wwn:
+                        continue
+                    wwn = utils.normalize(wwn)
+                    if not match('wwn', wwn, dev.name):
+                        continue
+
+                if 'serial' in root_device_hints:
+                    # TODO(lucasagomes): Since lsblk only supports
+                    # returning the short serial we are using
+                    # ID_SERIAL_SHORT here to keep compatibility with the
+                    # bash deploy ramdisk
+                    serial = udev.get('ID_SERIAL_SHORT', None)
+                    if not serial:
+                        continue
+                    serial = utils.normalize(serial)
+                    if not match('serial', serial, dev.name):
+                        continue
+
+                if 'vendor' in root_device_hints:
+                    vendor = self._get_device_vendor(dev.name)
+                    if not vendor:
+                        continue
+                    vendor = utils.normalize(vendor)
+                    if not match('vendor', vendor, dev.name):
+                        continue
+
+                return dev.name
+
+            else:
+                raise errors.DeviceNotFound("No suitable device was found for "
+                    "deployment using these hints %s" % root_device_hints)
 
     def erase_block_device(self, block_device):
         if self._ata_erase(block_device):
diff --git a/ironic_python_agent/tests/hardware.py b/ironic_python_agent/tests/hardware.py
@@ -14,6 +14,7 @@
 
 import mock
 from oslotest import base as test_base
+import pyudev
 import six
 from stevedore import extension
 
@@ -115,7 +116,7 @@
 BLK_DEVICE_TEMPLATE = (
     'KNAME="sda" MODEL="TinyUSB Drive" SIZE="3116853504" '
     'ROTA="0" TYPE="disk"\n'
-    'KNAME="sdb" MODEL="Fastable SD131 7" SIZE="31016853504" '
+    'KNAME="sdb" MODEL="Fastable SD131 7" SIZE="10737418240" '
     'ROTA="0" TYPE="disk"\n'
     'KNAME="sdc" MODEL="NWD-BLP4-1600   " SIZE="1765517033472" '
     ' ROTA="0" TYPE="disk"\n'
@@ -188,6 +189,61 @@ def test_get_os_install_device(self, mocked_execute):
         mocked_execute.assert_called_once_with(
             'lsblk', '-PbdioKNAME,MODEL,SIZE,ROTA,TYPE', check_exit_code=[0])
 
+    @mock.patch.object(hardware.GenericHardwareManager, '_get_device_vendor')
+    @mock.patch.object(pyudev.Device, 'from_device_file')
+    @mock.patch.object(utils, 'parse_root_device_hints')
+    @mock.patch.object(utils, 'execute')
+    def test_get_os_install_device_root_device_hints(self, mocked_execute,
+                                                     mock_root_device,
+                                                     mock_pyudev,
+                                                     mock_dev_vendor):
+        model = 'fastable sd131 7'
+        mock_root_device.return_value = {'model': model,
+                                         'wwn': 'fake-wwn',
+                                         'serial': 'fake-serial',
+                                         'vendor': 'fake-vendor',
+                                         'size': 10}
+        mock_dev_vendor.return_value = 'fake-vendor'
+        mock_pyudev.side_effect = ({}, {'ID_MODEL': model,
+                                        'ID_WWN': 'fake-wwn',
+                                        'ID_SERIAL_SHORT': 'fake-serial'})
+        mocked_execute.return_value = (BLK_DEVICE_TEMPLATE, '')
+
+        self.assertEqual('/dev/sdb', self.hardware.get_os_install_device())
+        mocked_execute.assert_called_once_with(
+            'lsblk', '-PbdioKNAME,MODEL,SIZE,ROTA,TYPE', check_exit_code=[0])
+        mock_root_device.assert_called_once_with()
+        expected = [mock.call(mock.ANY, '/dev/sda'),
+                    mock.call(mock.ANY, '/dev/sdb')]
+        mock_pyudev.assert_has_calls(expected)
+
+    @mock.patch.object(pyudev.Device, 'from_device_file')
+    @mock.patch.object(utils, 'parse_root_device_hints')
+    @mock.patch.object(utils, 'execute')
+    def test_get_os_install_device_root_device_hints_no_device_found(self,
+            mocked_execute, mock_root_device, mock_pyudev):
+        mock_root_device.return_value = {'model': 'endo-sym armor'}
+        mock_pyudev.return_value = {'ID_MODEL': 'Saturn V Armor'}
+        mocked_execute.return_value = (BLK_DEVICE_TEMPLATE, '')
+        self.assertRaises(errors.DeviceNotFound,
+                          self.hardware.get_os_install_device)
+        mocked_execute.assert_called_once_with(
+            'lsblk', '-PbdioKNAME,MODEL,SIZE,ROTA,TYPE', check_exit_code=[0])
+        mock_root_device.assert_called_once_with()
+        expected = [mock.call(mock.ANY, '/dev/sda'),
+                    mock.call(mock.ANY, '/dev/sdb'),
+                    mock.call(mock.ANY, '/dev/sdc'),
+                    mock.call(mock.ANY, '/dev/sdd')]
+        mock_pyudev.assert_has_calls(expected)
+
+    def test__get_device_vendor(self):
+        fileobj = mock.mock_open(read_data='fake-vendor')
+        with mock.patch(OPEN_FUNCTION_NAME, fileobj, create=True) as mock_open:
+            vendor = self.hardware._get_device_vendor('/dev/sdfake')
+            mock_open.assert_called_once_with(
+                '/sys/class/block/sdfake/device/vendor', 'r')
+            self.assertEqual('fake-vendor', vendor)
+
     @mock.patch('ironic_python_agent.hardware.GenericHardwareManager.'
                 '_get_cpu_count')
     @mock.patch(OPEN_FUNCTION_NAME)
@@ -298,7 +354,7 @@ def test_list_block_device(self, mocked_execute):
                                  rotational=False),
             hardware.BlockDevice(name='/dev/sdb',
                                  model='Fastable SD131 7',
-                                 size=31016853504,
+                                 size=10737418240,
                                  rotational=False),
             hardware.BlockDevice(name='/dev/sdc',
                                  model='NWD-BLP4-1600',
diff --git a/ironic_python_agent/tests/utils.py b/ironic_python_agent/tests/utils.py
@@ -268,3 +268,39 @@ def test__get_vmedia_params_umount_fails(self, execute_mock, mkdir_mock,
         read_params_mock.assert_called_once_with("/vmedia_mnt/parameters.txt")
         execute_mock.assert_any_call('umount', vmedia_mount_point)
         self.assertEqual(expected_params, returned_params)
+
+    @mock.patch.object(utils, 'get_agent_params')
+    def test_parse_root_device_hints(self, mock_get_params):
+        mock_get_params.return_value = {
+            'root_device': 'vendor=SpongeBob,model=Square%20Pants',
+            'ipa-api-url': 'http://1.2.3.4:1234'
+        }
+        expected = {'vendor': 'spongebob', 'model': 'square pants'}
+        result = utils.parse_root_device_hints()
+        self.assertEqual(expected, result)
+
+    @mock.patch.object(utils, 'get_agent_params')
+    def test_parse_root_device_hints_no_hints(self, mock_get_params):
+        mock_get_params.return_value = {
+            'ipa-api-url': 'http://1.2.3.4:1234'
+        }
+        result = utils.parse_root_device_hints()
+        self.assertEqual({}, result)
+
+    @mock.patch.object(utils, 'get_agent_params')
+    def test_parse_root_device_size(self, mock_get_params):
+        mock_get_params.return_value = {
+            'root_device': 'size=12345',
+            'ipa-api-url': 'http://1.2.3.4:1234'
+        }
+        result = utils.parse_root_device_hints()
+        self.assertEqual(12345, result['size'])
+
+    @mock.patch.object(utils, 'get_agent_params')
+    def test_parse_root_device_not_supported(self, mock_get_params):
+        mock_get_params.return_value = {
+            'root_device': 'foo=bar,size=12345',
+            'ipa-api-url': 'http://1.2.3.4:1234'
+        }
+        self.assertRaises(errors.DeviceNotFound,
+                          utils.parse_root_device_hints)
diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py
@@ -17,6 +17,7 @@
 import os
 
 from oslo_concurrency import processutils
+from six.moves.urllib import parse
 
 from ironic_python_agent import errors
 from ironic_python_agent.openstack.common import _i18n as gtu
@@ -25,6 +26,9 @@
 LOG = logging.getLogger(__name__)
 
 
+SUPPORTED_ROOT_DEVICE_HINTS = set(('size', 'model', 'wwn', 'serial', 'vendor'))
+
+
 def get_ordereddict(*args, **kwargs):
     """A fix for py26 not having ordereddict."""
     try:
@@ -140,3 +144,48 @@ def get_agent_params():
         params.update(vmedia_params)
 
     return params
+
+
+def normalize(string):
+    """Return a normalized string."""
+    # Since we can't use space on the kernel cmdline, Ironic will
+    # urlencode the values.
+    return parse.unquote(string).lower().strip()
+
+
+def parse_root_device_hints():
+    """Parse the root device hints.
+
+    Parse the root device hints given by Ironic via kernel cmdline
+    or vmedia.
+
+    :returns: A dict with the hints or an empty dict if no hints are
+              passed.
+    :raises: DeviceNotFound if there are unsupported hints.
+
+    """
+    root_device = get_agent_params().get('root_device')
+    if not root_device:
+        return {}
+
+    hints = dict((item.split('=') for item in root_device.split(',')))
+
+    # Find invalid hints for logging
+    not_supported = set(hints) - SUPPORTED_ROOT_DEVICE_HINTS
+    if not_supported:
+        error_msg = ('No device can be found because the following hints: '
+                     '"%(not_supported)s" are not supported by this version '
+                     'of IPA. Supported hints are: "%(supported)s"',
+                    {'not_supported': ', '.join(not_supported),
+                     'supported': ', '.join(SUPPORTED_ROOT_DEVICE_HINTS)})
+        raise errors.DeviceNotFound(error_msg)
+
+    # Normalise the values
+    hints = {k: normalize(v) for k, v in hints.iteritems()}
+
+    if 'size' in hints:
+        # NOTE(lucasagomes): Ironic should validate before passing to
+        # the deploy ramdisk
+        hints['size'] = int(hints['size'])
+
+    return hints
diff --git a/requirements.txt b/requirements.txt
@@ -13,6 +13,7 @@ oslo.serialization>=1.2.0               # Apache-2.0
 oslo.utils>=1.2.0                       # Apache-2.0
 pecan>=0.8.0
 psutil>=1.1.1,<2.0.0
+pyudev
 requests>=2.2.0,!=2.4.0
 six>=1.9.0
 stevedore>=1.1.0  # Apache-2.0
