inspection: prepare for future deprecations

1. cleanly separate deprecated and non-deprecated properties
2. add root disk to inspection data, so that we can have a proper
   fallback when root device hints are not given.

Change-Id: Ie19b82ff2a914873ff4b2395b02643e086b934b1

ironic_python_agent/inspector.py

@@ -151,19 +151,12 @@ def setup_ipmi_credentials(resp):
 def discover_network_properties(inventory, data, failures):
     """Discover network and BMC related properties.
 
-    Populates 'boot_interface', 'ipmi_address' and 'interfaces' keys.
+    This logic should eventually move to inspector itself.
 
     :param inventory: hardware inventory from a hardware manager
     :param data: mutable data that we'll send to inspector
     :param failures: AccumulatedFailures object
     """
-    # Both boot interface and IPMI address might not be present,
-    # we don't count it as failure
-    data['boot_interface'] = utils.get_agent_params().get('BOOTIF')
-    LOG.info('boot devices was %s', data['boot_interface'])
-    data['ipmi_address'] = inventory.get('bmc_address')
-    LOG.info('BMC IP address: %s', data['ipmi_address'])
-
     data.setdefault('interfaces', {})
     for iface in inventory['interfaces']:
         is_loopback = (iface.ipv4_address and
@@ -190,27 +183,21 @@ def discover_network_properties(inventory, data, failures):
         failures.add('no network interfaces found')
 
 
-def discover_scheduling_properties(inventory, data):
+def discover_scheduling_properties(inventory, data, root_disk=None):
     """Discover properties required for nova scheduler.
 
     This logic should eventually move to inspector itself.
 
     :param inventory: hardware inventory from a hardware manager
     :param data: mutable data that we'll send to inspector
+    :param root_disk: root device (if it can be detected)
     """
     data['cpus'] = inventory['cpu'].count
     data['cpu_arch'] = inventory['cpu'].architecture
     data['memory_mb'] = inventory['memory'].physical_mb
-
+    if root_disk is not None:
-    # Replicate the same logic as in deploy. This logic will be moved to
-    # inspector itself, but we need it for backward compatibility.
-    try:
-        disk = utils.guess_root_disk(inventory['disks'])
-    except errors.DeviceNotFound:
-        LOG.warn('no suitable root device detected')
-    else:
         # -1 is required to give Ironic some spacing for partitioning
-        data['local_gb'] = disk.size / units.Gi - 1
+        data['local_gb'] = root_disk.size / units.Gi - 1
 
     for key in ('cpus', 'local_gb', 'memory_mb'):
         try:
@@ -230,14 +217,37 @@ def collect_default(data, failures):
         1. it collects exactly the same data as the old bash-based ramdisk
         2. it also posts the whole inventory which we'll eventually use.
 
+    In both cases it tries to get BMC address, PXE boot device and the expected
+    root device.
+
     :param data: mutable data that we'll send to inspector
     :param failures: AccumulatedFailures object
     """
     inventory = hardware.dispatch_to_managers('list_hardware_info')
+
+    # In the future we will only need the current version of inventory,
+    # a guessed root disk, PXE boot interface and IPMI address.
+    # Everything else will be done by inspector itself and its plugins.
+    data['inventory'] = inventory
+    # Replicate the same logic as in deploy. We need to make sure that when
+    # root device hints are not set, inspector will use the same root disk as
+    # will be used for deploy.
+    try:
+        root_disk = utils.guess_root_disk(inventory['disks'][:])
+    except errors.DeviceNotFound:
+        root_disk = None
+        LOG.warn('no suitable root device detected')
+    else:
+        data['root_disk'] = root_disk
+        LOG.debug('default root device is %s', root_disk.name)
+    # Both boot interface and IPMI address might not be present,
+    # we don't count it as failure
+    data['boot_interface'] = utils.get_agent_params().get('BOOTIF')
+    LOG.debug('boot devices was %s', data['boot_interface'])
+    data['ipmi_address'] = inventory.get('bmc_address')
+    LOG.debug('BMC IP address: %s', data['ipmi_address'])
+
     # These 2 calls are required for backward compatibility and should be
     # dropped after inspector is ready (probably in Mitaka cycle).
     discover_network_properties(inventory, data, failures)
-    discover_scheduling_properties(inventory, data)
+    discover_scheduling_properties(inventory, data, root_disk)
-    # In the future we will only need the current version of inventory,
-    # everything else will be done by inspector itself and its plugins
-    data['inventory'] = inventory

ironic_python_agent/tests/unit/test_inspector.py

@@ -254,8 +254,6 @@ class BaseDiscoverTest(unittest.TestCase):
         self.data = {}
 
 
-@mock.patch.object(utils, 'get_agent_params',
-                   lambda: {'BOOTIF': 'boot:if'})
 class TestDiscoverNetworkProperties(BaseDiscoverTest):
     def test_no_network_interfaces(self):
         self.inventory['interfaces'] = [
@@ -282,8 +280,6 @@ class TestDiscoverNetworkProperties(BaseDiscoverTest):
                           'em2': {'mac': '11:22:33:44:55:66',
                                   'ip': None}},
                          self.data['interfaces'])
-        self.assertEqual('1.2.3.4', self.data['ipmi_address'])
-        self.assertEqual('boot:if', self.data['boot_interface'])
         self.assertFalse(self.failures)
 
     def test_missing(self):
@@ -305,7 +301,9 @@ class TestDiscoverNetworkProperties(BaseDiscoverTest):
 
 class TestDiscoverSchedulingProperties(BaseDiscoverTest):
     def test_ok(self):
-        inspector.discover_scheduling_properties(self.inventory, self.data)
+        inspector.discover_scheduling_properties(
+            self.inventory, self.data,
+            root_disk=self.inventory['disks'][2])
 
         self.assertEqual({'cpus': 4, 'cpu_arch': 'x86_64', 'local_gb': 464,
                           'memory_mb': 12288}, self.data)
@@ -313,7 +311,6 @@ class TestDiscoverSchedulingProperties(BaseDiscoverTest):
     def test_no_local_gb(self):
         # Some DRAC servers do not have any visible hard drive until RAID is
         # built
-        self.inventory['disks'] = []
 
         inspector.discover_scheduling_properties(self.inventory, self.data)
 
@@ -321,6 +318,8 @@ class TestDiscoverSchedulingProperties(BaseDiscoverTest):
                          self.data)
 
 
+@mock.patch.object(utils, 'get_agent_params',
+                   lambda: {'BOOTIF': 'boot:if'})
 @mock.patch.object(inspector, 'discover_scheduling_properties', autospec=True)
 @mock.patch.object(inspector, 'discover_network_properties', autospec=True)
 @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True)
@@ -330,10 +329,37 @@ class TestCollectDefault(BaseDiscoverTest):
 
         inspector.collect_default(self.data, self.failures)
 
+        for key in ('memory', 'interfaces', 'cpu', 'disks'):
+            self.assertTrue(self.data['inventory'][key])
+
+        self.assertEqual('1.2.3.4', self.data['ipmi_address'])
+        self.assertEqual('boot:if', self.data['boot_interface'])
+        self.assertEqual(self.inventory['disks'][0].name,
+                         self.data['root_disk'].name)
+
         mock_dispatch.assert_called_once_with('list_hardware_info')
         mock_discover_net.assert_called_once_with(self.inventory, self.data,
                                                   self.failures)
-        mock_discover_sched.assert_called_once_with(self.inventory, self.data)
+        mock_discover_sched.assert_called_once_with(
+            self.inventory, self.data,
+            root_disk=self.inventory['disks'][0])
 
-        for key in ('memory', 'interfaces', 'cpu', 'disks'):
+    def test_no_root_disk(self, mock_dispatch, mock_discover_net,
+                          mock_discover_sched):
+        mock_dispatch.return_value = self.inventory
+        self.inventory['disks'] = []
+
+        inspector.collect_default(self.data, self.failures)
+
+        for key in ('memory', 'interfaces', 'cpu'):
             self.assertTrue(self.data['inventory'][key])
+
+        self.assertEqual('1.2.3.4', self.data['ipmi_address'])
+        self.assertEqual('boot:if', self.data['boot_interface'])
+        self.assertNotIn('root_disk', self.data)
+
+        mock_dispatch.assert_called_once_with('list_hardware_info')
+        mock_discover_net.assert_called_once_with(self.inventory, self.data,
+                                                  self.failures)
+        mock_discover_sched.assert_called_once_with(
+            self.inventory, self.data, root_disk=None)