Switch to IPA as a primary ramdisk

* Stop requiring memory_mb, local_gb, cpu, cpu_arch, ipmi_address, interfaces;
  take them from inventory instead
* Issue a warning when inventory is not supplied
* Raise an error when root device hints are requested but inventory
  is not supplied
* Logging improvement around network interfaces handling

Closes-Bug: #1528831
Change-Id: Iaa1c34092463ff216379e30bcef55235517f6c92

CONTRIBUTING.rst

@@ -77,8 +77,8 @@ components. There is a plugin for installing **ironic-inspector** on DevStack.
 Example local.conf
 ------------------
 
-Using simple ramdisk
+Using IPA
-~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~
 
 ::
 
@@ -90,6 +90,60 @@ Using simple ramdisk
     disable_service heat h-api h-api-cfn h-api-cw h-eng
     disable_service cinder c-sch c-api c-vol
 
+    enable_plugin ironic https://github.com/openstack/ironic
+    enable_plugin ironic-inspector https://github.com/openstack/ironic-inspector
+
+    IRONIC_BAREMETAL_BASIC_OPS=True
+    IRONIC_VM_COUNT=2
+    IRONIC_VM_SPECS_RAM=1024
+
+    IRONIC_DEPLOY_DRIVER_ISCSI_WITH_IPA=True
+    IRONIC_BUILD_DEPLOY_RAMDISK=False
+    IRONIC_INSPECTOR_RAMDISK_ELEMENT=ironic-agent
+    IRONIC_INSPECTOR_BUILD_RAMDISK=False
+
+    VIRT_DRIVER=ironic
+
+    LOGDAYS=1
+    LOGFILE=~/logs/stack.sh.log
+    SCREEN_LOGDIR=~/logs/screen
+
+    DEFAULT_INSTANCE_TYPE=baremetal
+    TEMPEST_ALLOW_TENANT_ISOLATION=False
+
+Notes
+-----
+
+* Set IRONIC_INSPECTOR_BUILD_RAMDISK to True if you want to build ramdisk.
+  Default value is False and ramdisk will be downloaded instead of building.
+
+* 1024 MiB of RAM is a minimum required for the default build of IPA based on
+  CoreOS. If you plan to use another operating system and build IPA with
+  diskimage-builder 2048 MiB is recommended.
+
+* Network configuration is pretty sensitive, better not to touch it
+  without deep understanding.
+
+* This configuration disables Heat and Cinder, adjust it if you need these
+  services.
+
+Using simple ramdisk
+~~~~~~~~~~~~~~~~~~~~
+
+.. note::
+    This ramdisk is deprecated and should not be used.
+
+::
+
+    [[local|localrc]]
+    enable_service ironic ir-api ir-cond
+    disable_service n-net n-novnc
+    enable_service neutron q-svc q-agt q-dhcp q-l3 q-meta
+    enable_service s-proxy s-object s-container s-account
+    disable_service heat h-api h-api-cfn h-api-cw h-eng
+    disable_service cinder c-sch c-api c-vol
+
+    enable_plugin ironic https://github.com/openstack/ironic
     enable_plugin ironic-inspector https://github.com/openstack/ironic-inspector
 
     IRONIC_BAREMETAL_BASIC_OPS=True
@@ -115,58 +169,10 @@ Notes
 
 * You need at least 1G of RAM for VMs, default value of 512 MB won't work
 
-* Network configuration is pretty sensitive, better not to touch it
-  without deep understanding
-
-* This configuration disables Heat and Cinder, adjust it if you need these
-  services
-
 * Before restarting stack.sh::
 
     rm -rf /opt/stack/ironic-inspector
 
-Using IPA
-~~~~~~~~~
-
-::
-
-    [[local|localrc]]
-    enable_service ironic ir-api ir-cond
-    disable_service n-net n-novnc
-    enable_service neutron q-svc q-agt q-dhcp q-l3 q-meta
-    enable_service s-proxy s-object s-container s-account
-    disable_service heat h-api h-api-cfn h-api-cw h-eng
-    disable_service cinder c-sch c-api c-vol
-
-    enable_plugin ironic-inspector https://github.com/openstack/ironic-inspector
-
-    IRONIC_BAREMETAL_BASIC_OPS=True
-    IRONIC_VM_COUNT=2
-    IRONIC_VM_SPECS_RAM=1024
-
-    IRONIC_DEPLOY_DRIVER_ISCSI_WITH_IPA=True
-    IRONIC_BUILD_DEPLOY_RAMDISK=False
-    IRONIC_INSPECTOR_RAMDISK_ELEMENT=ironic-agent
-    IRONIC_INSPECTOR_BUILD_RAMDISK=False
-
-    VIRT_DRIVER=ironic
-
-    LOGDAYS=1
-    LOGFILE=~/logs/stack.sh.log
-    SCREEN_LOGDIR=~/logs/screen
-
-    DEFAULT_INSTANCE_TYPE=baremetal
-    TEMPEST_ALLOW_TENANT_ISOLATION=False
-
-Notes
------
-
-* Set IRONIC_INSPECTOR_BUILD_RAMDISK to True if you want to build ramdisk.
-  Default value is False and ramdisk will be download instead of building.
-* 1024 MiB of RAM is a minimum required for the default build of IPA based on
-  CoreOS. If you plan to use another operating system and build IPA with
-  diskimage-builder 2048 MiB is recommended.
-
 Test
 ----
 

doc/source/http-api.rst

@@ -139,40 +139,56 @@ Ramdisk Callback
 discovered data. Should not be used for anything other than implementing
 the ramdisk. Request body: JSON dictionary with at least these keys:
 
-* ``cpus`` number of CPU
+* ``inventory`` full hardware inventory from the ironic-python-agent with at
-* ``cpu_arch`` architecture of the CPU
+  least the following keys:
-* ``memory_mb`` RAM in MiB
-* ``local_gb`` hard drive size in GiB
-* ``interfaces`` dictionary filled with data from all NIC's, keys being
-  interface names, values being dictionaries with keys:
 
-  * ``mac`` MAC address
+  * ``memory`` memory information containing at least key ``physical_mb`` -
-  * ``ip`` IP address
+    physical memory size as reported by dmidecode,
 
-* ``ipmi_address`` IP address of BMC, may be missing on VM
+  * ``cpu`` CPU infromation containing at least keys ``count`` (CPU count) and
-* ``boot_interface`` optional MAC address of the NIC that the machine
+    ``architecture`` (CPU architecture, e.g. ``x86_64``),
-  PXE booted from either in standard format ``11:22:33:44:55:66`` or
-  in *PXELinux* ``BOOTIF`` format ``01-11-22-33-44-55-66``.
 
-Optionally the following keys might be provided:
+  * ``bmc_address`` IP address of the node's BMC,
 
-* ``inventory`` full hardware inventory from the ironic-python-agent (will be
+  * ``interfaces`` list of dictionaries with the following keys:
-  required in the future). Required for root device hints support.
+
+    * ``name`` interface name,
+
+    * ``ipv4_address`` IPv4 address of the interface,
+
+    * ``mac_address`` MAC (physical) address of the interface.
 
 * ``root_disk`` default deployment root disk as calculated by the
-  ironic-python-agent algorithm. Will be required in the future.
+  ironic-python-agent algorithm.
+
+* ``boot_interface`` MAC address of the NIC that the machine PXE booted from
+  either in standard format ``11:22:33:44:55:66`` or in *PXELinux* ``BOOTIF``
+  format ``01-11-22-33-44-55-66``. Strictly speaking, this key is optional,
+  but some features will now work as expected, if it is not provided.
+
+Optionally the following keys might be provided:
 
 * ``error`` error happened during ramdisk run, interpreted by
   ``ramdisk_error`` plugin.
 
 * ``logs`` base64-encoded logs from the ramdisk.
 
+The following keys are supported for backward compatibility with the old
+bash-based ramdisk, when ``inventory`` is not provided:
+
+* ``cpus`` number of CPU
+
+* ``cpu_arch`` architecture of the CPU
+
+* ``memory_mb`` RAM in MiB
+
+* ``local_gb`` hard drive size in GiB
+
+* ``ipmi_address`` IP address of BMC, may be missing on VM
+
 * ``block_devices`` block devices information for the ``raid_device`` plugin,
   dictionary with one key: ``serials`` list of serial numbers of block devices.
 
-  This is deprecated and only supported for backward compatibility. Full
-  ``inventory`` should be provided instead.
-
 .. note::
     This list highly depends on enabled plugins, provided above are
     expected keys for the default set of plugins. See :ref:`plugins`

doc/source/install.rst

@@ -142,9 +142,74 @@ Here is *inspector.conf* you may end up with::
 .. note::
     Set ``debug = true`` if you want to see complete logs.
 
+Using IPA
+^^^^^^^^^
+
+ironic-python-agent_ is a ramdisk developed for Ironic. During the Liberty
+cycle support for **ironic-inspector** was added. This is the default ramdisk
+starting with the Mitaka release.
+
+.. note::
+    You need at least 1.5 GiB of RAM on the machines to use this ramdisk,
+    2 GiB is recommended.
+
+To build an ironic-python-agent ramdisk, do the following:
+
+* Get the new enough version of diskimage-builder_::
+
+    sudo pip install -U "diskimage-builder>=1.1.2"
+
+* Build the ramdisk::
+
+    disk-image-create ironic-agent fedora -o ironic-agent
+
+  .. note::
+    Replace "fedora" with your distribution of choice.
+
+* Copy resulting files ``ironic-agent.vmlinuz`` and ``ironic-agent.initramfs``
+  to the TFTP root directory.
+
+Alternatively, you can download a `prebuilt IPA image
+<http://tarballs.openstack.org/ironic-python-agent/coreos/files/>`_ or use
+the `CoreOS-based IPA builder
+<http://docs.openstack.org/developer/ironic-python-agent/#coreos>`_.
+
+Next, set up ``$TFTPROOT/pxelinux.cfg/default`` as follows::
+
+    default introspect
+
+    label introspect
+    kernel ironic-agent.vmlinuz
+    append initrd=ironic-agent.initramfs ipa-inspection-callback-url=http://{IP}:5050/v1/continue systemd.journald.forward_to_console=yes
+
+    ipappend 3
+
+Replace ``{IP}`` with IP of the machine (do not use loopback interface, it
+will be accessed by ramdisk on a booting machine).
+
+.. note::
+    While ``systemd.journald.forward_to_console=yes`` is not actually
+    required, it will substantially simplify debugging if something goes wrong.
+
+This ramdisk is pluggable: you can insert introspection plugins called
+*collectors* into it. For example, to enable a very handy ``logs`` collector
+(sending ramdisk logs to **ironic-inspector**), modify the ``append`` line in
+``$TFTPROOT/pxelinux.cfg/default``::
+
+    append initrd=ironic-agent.initramfs ipa-inspection-callback-url=http://{IP}:5050/v1/continue ipa-inspection-collectors=default,logs systemd.journald.forward_to_console=yes
+
+.. note::
+    You probably want to always keep ``default`` collector, as it provides the
+    basic information required for introspection.
+
+.. _diskimage-builder: https://github.com/openstack/diskimage-builder
+.. _ironic-python-agent: https://github.com/openstack/ironic-python-agent
+
 Using simple ramdisk
 ^^^^^^^^^^^^^^^^^^^^
 
+This ramdisk is deprecated, its use is not recommended.
+
 * Build and put into your TFTP the kernel and ramdisk created using the
   diskimage-builder_ `ironic-discoverd-ramdisk element`_::
 
@@ -166,57 +231,7 @@ Using simple ramdisk
   Replace ``{IP}`` with IP of the machine (do not use loopback interface, it
   will be accessed by ramdisk on a booting machine).
 
-  .. note::
-    There are some prebuilt images which use obsolete ``ironic_callback_url``
-    instead of ``discoverd_callback_url``. Modify ``pxelinux.cfg/default``
-    accordingly if you have one of these.
-
-Using IPA
-^^^^^^^^^
-
-ironic-python-agent_ is a new ramdisk developed for Ironic. During the Liberty
-cycle support for **ironic-inspector** was added. This is experimental
-for now, but we plan on making IPA the default ramdisk in Mitaka cycle.
-
-.. note::
-    You need at least 1.5 GiB of RAM on the machines to use this ramdisk.
-
-To build an ironic-python-agent ramdisk, do the following:
-
-* Get the latest diskimage-builder_::
-
-    sudo pip install -U "diskimage-builder>=1.1.2"
-
-* Build the ramdisk::
-
-    disk-image-create ironic-agent fedora -o ironic-agent
-
-  .. note::
-    Replace "fedora" with your distribution of choice.
-
-* Copy resulting files ``ironic-agent.vmlinuz`` and ``ironic-agent.initramfs``
-  to the TFTP root directory.
-
-Next, set up ``$TFTPROOT/pxelinux.cfg/default`` as follows::
-
-    default introspect
-
-    label introspect
-    kernel ironic-agent.vmlinuz
-    append initrd=ironic-agent.initramfs ipa-inspection-callback-url=http://{IP}:5050/v1/continue systemd.journald.forward_to_console=yes
-
-    ipappend 3
-
-Replace ``{IP}`` with IP of the machine (do not use loopback interface, it
-will be accessed by ramdisk on a booting machine).
-
-.. note::
-    While ``systemd.journald.forward_to_console=yes`` is not actually
-    required, it will substantially simplify debugging if something goes wrong.
-
-.. _diskimage-builder: https://github.com/openstack/diskimage-builder
 .. _ironic-discoverd-ramdisk element: https://github.com/openstack/diskimage-builder/tree/master/elements/ironic-discoverd-ramdisk
-.. _ironic-python-agent: https://github.com/openstack/ironic-python-agent
 
 Managing the **ironic-inspector** database
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

ironic_inspector/plugins/standard.py

@@ -18,10 +18,12 @@ import datetime
 import os
 import sys
 
+import netaddr
 from oslo_config import cfg
 from oslo_utils import units
+import six
 
-from ironic_inspector.common.i18n import _, _LC, _LI, _LW
+from ironic_inspector.common.i18n import _, _LC, _LE, _LI, _LW
 from ironic_inspector import conf
 from ironic_inspector.plugins import base
 from ironic_inspector import utils
@@ -30,8 +32,6 @@ CONF = cfg.CONF
 
 
 LOG = utils.getProcessingLogger('ironic_inspector.plugins.standard')
-KNOWN_ROOT_DEVICE_HINTS = ('model', 'vendor', 'serial', 'wwn', 'hctl',
-                           'size')
 
 
 class RootDiskSelectionHook(base.ProcessingHook):
@@ -52,11 +52,10 @@ class RootDiskSelectionHook(base.ProcessingHook):
 
         inventory = introspection_data.get('inventory')
         if not inventory:
-            LOG.error(_LW('Root device selection require ironic-python-agent '
+            raise utils.Error(
-                          'as an inspection ramdisk'),
+                _('Root device selection requires ironic-python-agent '
-                      node_info=node_info, data=introspection_data)
+                  'as an inspection ramdisk'),
-            # TODO(dtantsur): make it a real error in Mitaka cycle
+                node_info=node_info, data=introspection_data)
-            return
 
         disks = inventory.get('disks', [])
         if not disks:
@@ -96,17 +95,53 @@ class SchedulerHook(base.ProcessingHook):
 
     def before_update(self, introspection_data, node_info, **kwargs):
         """Update node with scheduler properties."""
+        inventory = introspection_data.get('inventory')
+        errors = []
+
         root_disk = introspection_data.get('root_disk')
         if root_disk:
             introspection_data['local_gb'] = root_disk['size'] // units.Gi
             if CONF.processing.disk_partitioning_spacing:
                 introspection_data['local_gb'] -= 1
+        elif inventory:
+            errors.append(_('root disk is not supplied by the ramdisk and '
+                            'root_disk_selection hook is not enabled'))
 
-        missing = [key for key in self.KEYS if not introspection_data.get(key)]
+        if inventory:
-        if missing:
+            try:
-            raise utils.Error(
+                introspection_data['cpus'] = int(inventory['cpu']['count'])
-                _('The following required parameters are missing: %s') %
+                introspection_data['cpu_arch'] = six.text_type(
-                missing, node_info=node_info, data=introspection_data)
+                    inventory['cpu']['architecture'])
+            except (KeyError, ValueError, TypeError):
+                errors.append(_('malformed or missing CPU information: %s') %
+                              inventory.get('cpu'))
+
+            try:
+                introspection_data['memory_mb'] = int(
+                    inventory['memory']['physical_mb'])
+            except (KeyError, ValueError, TypeError):
+                errors.append(_('malformed or missing memory information: %s; '
+                                'introspection requires physical memory size '
+                                'from dmidecode') %
+                              inventory.get('memory'))
+        else:
+            LOG.warning(_LW('No inventory provided: using old bash ramdisk '
+                            'is deprecated, please switch to '
+                            'ironic-python-agent'),
+                        node_info=node_info, data=introspection_data)
+
+            missing = [key for key in self.KEYS
+                       if not introspection_data.get(key)]
+            if missing:
+                raise utils.Error(
+                    _('The following required parameters are missing: %s') %
+                    missing,
+                    node_info=node_info, data=introspection_data)
+
+        if errors:
+            raise utils.Error(_('The following problems encountered: %s') %
+                              '; '.join(errors),
+                              node_info=node_info, data=introspection_data)
 
         LOG.info(_LI('Discovered data: CPUs: %(cpus)s %(cpu_arch)s, '
                      'memory %(memory_mb)s MiB, disk %(local_gb)s GiB'),
@@ -138,53 +173,118 @@ class ValidateInterfacesHook(base.ProcessingHook):
                           'actual': CONF.processing.keep_ports})
             sys.exit(1)
 
+    def _get_interfaces(self, inventory, data=None):
+        """Convert inventory to a dict with interfaces.
+
+        :return: dict interface name -> dict with keys 'mac' and 'ip'
+        """
+        result = {}
+
+        for iface in inventory.get('interfaces', ()):
+            name = iface.get('name')
+            mac = iface.get('mac_address')
+            ip = iface.get('ipv4_address')
+
+            if not name:
+                LOG.error(_LE('Malformed interface record: %s'),
+                          iface, data=data)
+                continue
+
+            LOG.debug('Found interface %(name)s with MAC "%(mac)s" and '
+                      'IP address "%(ip)s"',
+                      {'name': name, 'mac': mac, 'ip': ip}, data=data)
+            result[name] = {'ip': ip, 'mac': mac}
+
+        return result
+
+    def _validate_interfaces(self, interfaces, pxe_mac, data=None):
+        """Validate interfaces on correctness and suitability.
+
+        :return: dict interface name -> dict with keys 'mac' and 'ip'
+        """
+        if not interfaces:
+            raise utils.Error(_('No interfaces supplied by the ramdisk'),
+                              data=data)
+
+        result = {}
+
+        for name, iface in interfaces.items():
+            mac = iface.get('mac')
+            ip = iface.get('ip')
+
+            if not mac:
+                LOG.debug('Skipping interface %s without link information',
+                          name, data=data)
+                continue
+
+            if not utils.is_valid_mac(mac):
+                LOG.warning(_LW('MAC %(mac)s for interface %(name)s is not '
+                                'valid, skipping'),
+                            {'mac': mac, 'name': name},
+                            data=data)
+                continue
+
+            mac = mac.lower()
+
+            if name == 'lo' or (ip and netaddr.IPAddress(ip).is_loopback()):
+                LOG.debug('Skipping local interface %s', name, data=data)
+                continue
+
+            if (CONF.processing.add_ports == 'pxe' and pxe_mac
+                    and mac != pxe_mac):
+                LOG.debug('Skipping interface %s as it was not PXE booting',
+                          name, data=data)
+                continue
+            elif CONF.processing.add_ports != 'all' and not ip:
+                LOG.debug('Skipping interface %s as it did not have '
+                          'an IP address assigned during the ramdisk run',
+                          name, data=data)
+                continue
+
+            result[name] = {'ip': ip, 'mac': mac.lower()}
+
+        return result
+
     def before_processing(self, introspection_data, **kwargs):
         """Validate information about network interfaces."""
-        if not introspection_data.get('interfaces'):
+        inventory = introspection_data.get('inventory', {})
-            raise utils.Error(_('No interfaces supplied by the ramdisk'),
-                              data=introspection_data)
 
-        valid_interfaces = {
+        bmc_address = utils.get_ipmi_address_from_data(introspection_data)
-            n: iface for n, iface in introspection_data['interfaces'].items()
+        if bmc_address:
-            if utils.is_valid_mac(iface.get('mac'))
+            introspection_data['ipmi_address'] = bmc_address
-        }
+        else:
+            LOG.debug('No BMC address provided in introspection data, '
+                      'assuming virtual environment', data=introspection_data)
+
+        if inventory:
+            all_interfaces = self._get_interfaces(inventory,
+                                                  introspection_data)
+        else:
+            LOG.warning(_LW('No inventory provided: using old bash ramdisk '
+                            'is deprecated, please switch to '
+                            'ironic-python-agent'),
+                        data=introspection_data)
+            all_interfaces = introspection_data.get('interfaces')
 
         pxe_mac = utils.get_pxe_mac(introspection_data)
-
+        if not pxe_mac and CONF.processing.add_ports == 'pxe':
-        if CONF.processing.add_ports == 'pxe' and not pxe_mac:
             LOG.warning(_LW('No boot interface provided in the introspection '
-                            'data, will add all ports with IP addresses'),
+                            'data, will add all ports with IP addresses'))
-                        data=introspection_data)
 
-        if CONF.processing.add_ports == 'pxe' and pxe_mac:
+        interfaces = self._validate_interfaces(all_interfaces, pxe_mac,
-            valid_interfaces = {
+                                               introspection_data)
-                n: iface for n, iface in valid_interfaces.items()
+        if not interfaces:
-                if iface['mac'].lower() == pxe_mac
+            raise utils.Error(_('No suitable interfaces found in %s') %
-            }
+                              all_interfaces, data=introspection_data)
-        elif CONF.processing.add_ports != 'all':
-            valid_interfaces = {
-                n: iface for n, iface in valid_interfaces.items()
-                if iface.get('ip')
-            }
 
-        if not valid_interfaces:
+        LOG.info(_LI('Using network interface(s): %s'),
-            raise utils.Error(_('No valid interfaces found in %s') %
+                 ', '.join('%s %s' % (name, items)
-                              introspection_data['interfaces'],
+                           for (name, items) in interfaces.items()),
-                              data=introspection_data)
+                 data=introspection_data)
-        elif valid_interfaces != introspection_data['interfaces']:
-            invalid = {n: iface
-                       for n, iface in introspection_data['interfaces'].items()
-                       if n not in valid_interfaces}
-            LOG.warning(_LW(
-                'The following interfaces were invalid or not eligible in '
-                'introspection data and were excluded: %s'), invalid,
-                data=introspection_data)
-            LOG.info(_LI('Eligible interfaces are %s'), valid_interfaces,
-                     data=introspection_data)
 
-        introspection_data['all_interfaces'] = introspection_data['interfaces']
+        introspection_data['all_interfaces'] = all_interfaces
-        introspection_data['interfaces'] = valid_interfaces
+        introspection_data['interfaces'] = interfaces
-        valid_macs = [iface['mac'] for iface in valid_interfaces.values()]
+        valid_macs = [iface['mac'] for iface in interfaces.values()]
         introspection_data['macs'] = valid_macs
 
     def before_update(self, introspection_data, node_info, **kwargs):

ironic_inspector/test/functional.py

@@ -69,24 +69,29 @@ class Base(base.NodeTest):
 
         # https://github.com/openstack/ironic-inspector/blob/master/HTTP-API.rst  # noqa
         self.data = {
-            'cpus': 4,
-            'cpu_arch': 'x86_64',
-            'memory_mb': 12288,
-            'local_gb': 464,
-            'interfaces': {
-                'eth1': {'mac': self.macs[0], 'ip': '1.2.1.2'},
-                'eth2': {'mac': '12:12:21:12:21:12'},
-                'eth3': {'mac': self.macs[1], 'ip': '1.2.1.1'},
-            },
             'boot_interface': '01-' + self.macs[0].replace(':', '-'),
-            'ipmi_address': self.bmc_address,
             'inventory': {
+                'interfaces': [
+                    {'name': 'eth1', 'mac_address': self.macs[0],
+                     'ipv4_address': '1.2.1.2'},
+                    {'name': 'eth2', 'mac_address': '12:12:21:12:21:12'},
+                    {'name': 'eth3', 'mac_address': self.macs[1],
+                     'ipv4_address': '1.2.1.1'},
+                ],
                 'disks': [
                     {'name': '/dev/sda', 'model': 'Big Data Disk',
                      'size': 1000 * units.Gi},
                     {'name': '/dev/sdb', 'model': 'Small OS Disk',
                      'size': 20 * units.Gi},
-                ]
+                ],
+                'cpu': {
+                    'count': 4,
+                    'architecture': 'x86_64'
+                },
+                'memory': {
+                    'physical_mb': 12288
+                },
+                'bmc_address': self.bmc_address
             },
             'root_disk': {'name': '/dev/sda', 'model': 'Big Data Disk',
                           'size': 1000 * units.Gi},

ironic_inspector/test/test_plugins_standard.py

@@ -35,13 +35,21 @@ class TestSchedulerHook(test_base.NodeTest):
         super(TestSchedulerHook, self).setUp()
         self.hook = std_plugins.SchedulerHook()
         self.data = {
+            'inventory': {
+                'cpu': {'count': 2, 'architecture': 'x86_64'},
+                'memory': {'physical_mb': 1024},
+            },
+            'root_disk': {
+                'name': '/dev/sda',
+                'size': 21 * units.Gi
+            }
+        }
+        self.old_data = {
             'local_gb': 20,
             'memory_mb': 1024,
             'cpus': 2,
             'cpu_arch': 'x86_64'
         }
-        self.node_patches = []
-        self.ports_patches = {}
         self.node_info = node_cache.NodeInfo(uuid=self.uuid, started_at=0,
                                              node=self.node)
 
@@ -50,14 +58,19 @@ class TestSchedulerHook(test_base.NodeTest):
         ext = base.processing_hooks_manager()['scheduler']
         self.assertIsInstance(ext.obj, std_plugins.SchedulerHook)
 
-    def test_missing(self):
+    def test_compat_missing(self):
-        for key in self.data:
+        for key in self.old_data:
-            new_data = self.data.copy()
+            new_data = self.old_data.copy()
             del new_data[key]
             self.assertRaisesRegexp(utils.Error, key,
                                     self.hook.before_update, new_data,
                                     self.node_info)
 
+    def test_no_root_disk(self):
+        self.assertRaisesRegexp(utils.Error, 'root disk is not supplied',
+                                self.hook.before_update,
+                                {'inventory': {'disks': []}}, self.node_info)
+
     @mock.patch.object(node_cache.NodeInfo, 'patch')
     def test_ok(self, mock_patch):
         patch = [
@@ -70,6 +83,18 @@ class TestSchedulerHook(test_base.NodeTest):
         self.hook.before_update(self.data, self.node_info)
         self.assertCalledWithPatch(patch, mock_patch)
 
+    @mock.patch.object(node_cache.NodeInfo, 'patch')
+    def test_compat_ok(self, mock_patch):
+        patch = [
+            {'path': '/properties/cpus', 'value': '2', 'op': 'add'},
+            {'path': '/properties/cpu_arch', 'value': 'x86_64', 'op': 'add'},
+            {'path': '/properties/memory_mb', 'value': '1024', 'op': 'add'},
+            {'path': '/properties/local_gb', 'value': '20', 'op': 'add'}
+        ]
+
+        self.hook.before_update(self.old_data, self.node_info)
+        self.assertCalledWithPatch(patch, mock_patch)
+
     @mock.patch.object(node_cache.NodeInfo, 'patch')
     def test_no_overwrite(self, mock_patch):
         CONF.set_override('overwrite_existing', False, 'processing')
@@ -86,8 +111,9 @@ class TestSchedulerHook(test_base.NodeTest):
         self.assertCalledWithPatch(patch, mock_patch)
 
     @mock.patch.object(node_cache.NodeInfo, 'patch')
-    def test_root_disk(self, mock_patch):
+    def test_compat_root_disk(self, mock_patch):
-        self.data['root_disk'] = {'name': '/dev/sda', 'size': 42 * units.Gi}
+        self.old_data['root_disk'] = {'name': '/dev/sda',
+                                      'size': 42 * units.Gi}
         patch = [
             {'path': '/properties/cpus', 'value': '2', 'op': 'add'},
             {'path': '/properties/cpu_arch', 'value': 'x86_64', 'op': 'add'},
@@ -95,7 +121,7 @@ class TestSchedulerHook(test_base.NodeTest):
             {'path': '/properties/local_gb', 'value': '41', 'op': 'add'}
         ]
 
-        self.hook.before_update(self.data, self.node_info)
+        self.hook.before_update(self.old_data, self.node_info)
         self.assertCalledWithPatch(patch, mock_patch)
 
     @mock.patch.object(node_cache.NodeInfo, 'patch')
@@ -118,6 +144,19 @@ class TestValidateInterfacesHook(test_base.NodeTest):
         super(TestValidateInterfacesHook, self).setUp()
         self.hook = std_plugins.ValidateInterfacesHook()
         self.data = {
+            'inventory': {
+                'interfaces': [
+                    {'name': 'em1', 'mac_address': '11:11:11:11:11:11',
+                     'ipv4_address': '1.1.1.1'},
+                    {'name': 'em2', 'mac_address': '22:22:22:22:22:22',
+                     'ipv4_address': '2.2.2.2'},
+                    {'name': 'em3', 'mac_address': '33:33:33:33:33:33',
+                     'ipv4_address': None},
+                ],
+            },
+            'boot_interface': '01-22-22-22-22-22-22'
+        }
+        self.old_data = {
             'interfaces': {
                 'em1': {'mac': '11:11:11:11:11:11', 'ip': '1.1.1.1'},
                 'em2': {'mac': '22:22:22:22:22:22', 'ip': '2.2.2.2'},
@@ -125,8 +164,9 @@ class TestValidateInterfacesHook(test_base.NodeTest):
             },
             'boot_interface': '01-22-22-22-22-22-22',
         }
-        self.orig_interfaces = self.data['interfaces'].copy()
+        self.orig_interfaces = self.old_data['interfaces'].copy()
-        self.pxe_interface = self.data['interfaces']['em2']
+        self.orig_interfaces['em3']['ip'] = None
+        self.pxe_interface = self.old_data['interfaces']['em2']
         self.active_interfaces = {
             'em1': {'mac': '11:11:11:11:11:11', 'ip': '1.1.1.1'},
             'em2': {'mac': '22:22:22:22:22:22', 'ip': '2.2.2.2'},
@@ -157,6 +197,12 @@ class TestValidateInterfacesHook(test_base.NodeTest):
     def test_no_interfaces(self):
         self.assertRaisesRegexp(utils.Error, 'No interfaces',
                                 self.hook.before_processing, {})
+        self.assertRaisesRegexp(utils.Error, 'No interfaces',
+                                self.hook.before_processing, {'inventory': {}})
+        self.assertRaisesRegexp(utils.Error, 'No interfaces',
+                                self.hook.before_processing, {'inventory': {
+                                    'interfaces': []
+                                }})
 
     def test_only_pxe(self):
         self.hook.before_processing(self.data)
@@ -175,7 +221,7 @@ class TestValidateInterfacesHook(test_base.NodeTest):
 
     def test_only_pxe_not_found(self):
         self.data['boot_interface'] = 'aa:bb:cc:dd:ee:ff'
-        self.assertRaisesRegexp(utils.Error, 'No valid interfaces',
+        self.assertRaisesRegexp(utils.Error, 'No suitable interfaces',
                                 self.hook.before_processing, self.data)
 
     def test_only_pxe_no_boot_interface(self):
@@ -208,6 +254,43 @@ class TestValidateInterfacesHook(test_base.NodeTest):
                          sorted(self.data['macs']))
         self.assertEqual(self.orig_interfaces, self.data['all_interfaces'])
 
+    def test_malformed_interfaces(self):
+        self.data = {
+            'inventory': {
+                'interfaces': [
+                    # no name
+                    {'mac_address': '11:11:11:11:11:11',
+                     'ipv4_address': '1.1.1.1'},
+                    # empty
+                    {},
+                ],
+            },
+        }
+        self.assertRaisesRegexp(utils.Error, 'No interfaces supplied',
+                                self.hook.before_processing, self.data)
+
+    def test_skipped_interfaces(self):
+        CONF.set_override('add_ports', 'all', 'processing')
+        self.data = {
+            'inventory': {
+                'interfaces': [
+                    # local interface (by name)
+                    {'name': 'lo', 'mac_address': '11:11:11:11:11:11',
+                     'ipv4_address': '1.1.1.1'},
+                    # local interface (by IP address)
+                    {'name': 'em1', 'mac_address': '22:22:22:22:22:22',
+                     'ipv4_address': '127.0.0.1'},
+                    # no MAC provided
+                    {'name': 'em3', 'ipv4_address': '2.2.2.2'},
+                    # malformed MAC provided
+                    {'name': 'em4', 'mac_address': 'foobar',
+                     'ipv4_address': '2.2.2.2'},
+                ],
+            },
+        }
+        self.assertRaisesRegexp(utils.Error, 'No suitable interfaces found',
+                                self.hook.before_processing, self.data)
+
     @mock.patch.object(node_cache.NodeInfo, 'delete_port', autospec=True)
     def test_keep_all(self, mock_delete_port):
         self.hook.before_update(self.data, self.node_info)
@@ -262,16 +345,17 @@ class TestRootDiskSelection(test_base.NodeTest):
         self.assertNotIn('local_gb', self.data)
         self.assertNotIn('root_disk', self.data)
 
-    @mock.patch.object(std_plugins.LOG, 'error')
+    def test_no_inventory(self):
-    def test_no_inventory(self, mock_log):
         self.node.properties['root_device'] = {'model': 'foo'}
         del self.data['inventory']
 
-        self.hook.before_update(self.data, self.node_info, None, None)
+        self.assertRaisesRegexp(utils.Error,
+                                'requires ironic-python-agent',
+                                self.hook.before_update,
+                                self.data, self.node_info, None, None)
 
         self.assertNotIn('local_gb', self.data)
         self.assertNotIn('root_disk', self.data)
-        self.assertTrue(mock_log.called)
 
     def test_no_disks(self):
         self.node.properties['root_device'] = {'size': 10}

releasenotes/notes/ipa-inventory-0a1e8d644da850ff.yaml

@@ -0,0 +1,15 @@
+---
+prelude: >
+    Starting with this release, ironic-python-agent becomes the default
+    introspection ramdisk, with the old bash-based ramdisk being deprecated.
+features:
+  - Inspector no longer requires old-style "local_gb", "memory_mb", "cpus"
+    and "cpu_arch" fields from the introspection ramdisk. They are still
+    supported, though, for compatibility with the old ramdisk.
+upgrade:
+  - The root_disk_selection processing hook will now error out if root device
+    hints are specified on ironic node, but ironic-python-agent is not used
+    as an introspection ramdisk.
+deprecations:
+  - Using old bash-based ramdisk is deprecated, please switch to
+    ironic-python-agent as soon as possible.