Merge "Small refactor in the root device loop matching logic"

2 files changed, 28 insertions, 10 deletions

diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index 87332916..edd1d78a 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -612,6 +612,16 @@ class GenericHardwareManager(HardwareManager):
                 if hint == 'rotational':
                     hint_value = strutils.bool_from_string(hint_value)
 
+                elif hint == 'size':
+                    try:
+                        hint_value = int(hint_value)
+                    except (ValueError, TypeError):
+                        LOG.warning(
+                            'Root device hint "size" is not an integer. '
+                            'Current value: "%(value)s"; and type: "%(type)s"',
+                            {'value': hint_value, 'type': type(hint_value)})
+                        return False
+
                 if hint_value != current_value:
                     LOG.debug("Root device hint %(hint)s=%(value)s does not "
                               "match the device %(device)s value of "
@@ -625,7 +635,7 @@ class GenericHardwareManager(HardwareManager):
             def check_device_attrs(device):
                 for key in ('model', 'wwn', 'serial', 'vendor',
                             'wwn_with_extension', 'wwn_vendor_extension',
-                            'name', 'rotational'):
+                            'name', 'rotational', 'size'):
                     if key not in root_device_hints:
                         continue
 
@@ -636,21 +646,17 @@ class GenericHardwareManager(HardwareManager):
                     if isinstance(value, six.string_types):
                         value = utils.normalize(value)
 
+                    if key == 'size':
+                        # Since we don't support units yet we expect the size
+                        # in GiB for now
+                        value = value / units.Gi
+
                     if not match(key, value, device.name):
                         return False
 
                 return True
 
             for dev in block_devices:
-                # 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 check_device_attrs(dev):
                     return dev.name
 
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index b84e2086..dac4bb66 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -477,6 +477,18 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
         self._get_os_install_device_root_device_hints(
             {'size': 10}, '/dev/sdb')
 
+    def test_get_os_install_device_root_device_hints_size_str(self):
+        self._get_os_install_device_root_device_hints(
+            {'size': '10'}, '/dev/sdb')
+
+    @mock.patch.object(hardware.LOG, 'warning')
+    def test_get_os_install_device_root_device_hints_size_not_int(
+            self, mock_log):
+        self.assertRaises(errors.DeviceNotFound,
+                          self._get_os_install_device_root_device_hints,
+                          {'size': 'not-int'}, '/dev/sdb')
+        self.assertTrue(mock_log.called)
+
     def test_get_os_install_device_root_device_hints_vendor(self):
         self._get_os_install_device_root_device_hints(
             {'vendor': 'fake-vendor'}, '/dev/sdb')