From e9824d11d17215c280533b4bce1cd489b9fd2f4a Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 15 Oct 2019 16:11:27 +0200 Subject: [PATCH] Wire in in-band inspection for PXE boot and neutron-based networking This change implements in-band inspection support for PXE and iPXE boot interfaces and all in-tree network interfaces. Story: #1528920 Task: #23184 Change-Id: I470d55add73bae47a2755cde93d4b1e1f30e94a7 --- ironic/common/neutron.py | 24 +++ ironic/conf/neutron.py | 12 ++ ironic/drivers/modules/ipxe.py | 47 ++++-- ironic/drivers/modules/network/common.py | 1 + ironic/drivers/modules/network/flat.py | 76 ++++++--- ironic/drivers/modules/network/neutron.py | 143 +++++++++-------- ironic/drivers/modules/pxe.py | 47 ++++-- .../unit/drivers/modules/network/test_flat.py | 77 +++++++++- .../drivers/modules/network/test_neutron.py | 144 +++++++++++++----- .../unit/drivers/modules/network/test_noop.py | 8 + .../tests/unit/drivers/modules/test_ipxe.py | 13 ++ ironic/tests/unit/drivers/modules/test_pxe.py | 13 ++ .../inspector-pxe-boot-9ab9fede5671097e.yaml | 12 ++ 13 files changed, 449 insertions(+), 168 deletions(-) create mode 100644 releasenotes/notes/inspector-pxe-boot-9ab9fede5671097e.yaml diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index a62212ccdf..6734b32df4 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -770,3 +770,27 @@ class NeutronNetworkInterfaceMixin(object): return validate_network( rescuing_network, _('rescuing network'), context=task.context) + + def get_inspection_network_uuid(self, task): + inspection_network = ( + task.node.driver_info.get('inspection_network') + or CONF.neutron.inspection_network + ) + return validate_network( + inspection_network, _('inspection network'), + context=task.context) + + def validate_inspection(self, task): + """Validate that the node has required properties for inspection. + + :param task: A TaskManager instance with the node being checked + :raises: MissingParameterValue if node is missing one or more required + parameters + :raises: UnsupportedDriverExtension + """ + try: + self.get_inspection_network_uuid(task) + except exception.MissingParameterValue: + # Fall back to non-managed in-band inspection + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='inspection') diff --git a/ironic/conf/neutron.py b/ironic/conf/neutron.py index 50a9372f14..3edb73bc28 100644 --- a/ironic/conf/neutron.py +++ b/ironic/conf/neutron.py @@ -90,6 +90,18 @@ opts = [ 'cleaning, or rescue. This is done without IP ' 'addresses assigned to the port, and may be useful ' 'in some bonded network configurations.')), + cfg.StrOpt('inspection_network', + help=_('Neutron network UUID or name for the ramdisk to be ' + 'booted into for in-band inspection of nodes. ' + 'If a name is provided, it must be unique among all ' + 'networks or inspection will fail.')), + cfg.ListOpt('inspection_network_security_groups', + default=[], + help=_('List of Neutron Security Group UUIDs to be applied ' + 'during the node inspection process. Optional for the ' + '"neutron" network interface and not used for the ' + '"flat" or "noop" network interfaces. If not ' + 'specified, the default security group is used.')), ] diff --git a/ironic/drivers/modules/ipxe.py b/ironic/drivers/modules/ipxe.py index 49ac313481..696feb5180 100644 --- a/ironic/drivers/modules/ipxe.py +++ b/ironic/drivers/modules/ipxe.py @@ -48,20 +48,7 @@ class iPXEBoot(pxe_base.PXEBaseMixin, base.BootInterface): def __init__(self): pxe_utils.create_ipxe_boot_script() - @METRICS.timer('iPXEBoot.validate') - def validate(self, task): - """Validate the PXE-specific info for booting deploy/instance images. - - This method validates the PXE-specific info for booting the - ramdisk and instance on the node. If invalid, raises an - exception; otherwise returns None. - - :param task: a task from TaskManager. - :returns: None - :raises: InvalidParameterValue, if some parameters are invalid. - :raises: MissingParameterValue, if some required parameters are - missing. - """ + def _validate_common(self, task): node = task.node if not driver_utils.get_node_mac_addresses(task): @@ -93,11 +80,29 @@ class iPXEBoot(pxe_base.PXEBaseMixin, base.BootInterface): pxe.validate_boot_parameters_for_trusted_boot(node) pxe_utils.parse_driver_info(node) + + @METRICS.timer('iPXEBoot.validate') + def validate(self, task): + """Validate the PXE-specific info for booting deploy/instance images. + + This method validates the PXE-specific info for booting the + ramdisk and instance on the node. If invalid, raises an + exception; otherwise returns None. + + :param task: a task from TaskManager. + :returns: None + :raises: InvalidParameterValue, if some parameters are invalid. + :raises: MissingParameterValue, if some required parameters are + missing. + """ + self._validate_common(task) + # NOTE(TheJulia): If we're not writing an image, we can skip # the remainder of this method. if (not task.driver.storage.should_write_image(task)): return + node = task.node d_info = deploy_utils.get_image_instance_info(node) if (node.driver_internal_info.get('is_whole_disk_image') or deploy_utils.get_boot_option(node) == 'local'): @@ -108,6 +113,20 @@ class iPXEBoot(pxe_base.PXEBaseMixin, base.BootInterface): props = ['kernel', 'ramdisk'] deploy_utils.validate_image_properties(task.context, d_info, props) + @METRICS.timer('iPXEBoot.validate_inspection') + def validate_inspection(self, task): + """Validate that the node has required properties for inspection. + + :param task: A TaskManager instance with the node being checked + :raises: UnsupportedDriverExtension + """ + try: + self._validate_common(task) + except exception.MissingParameterValue: + # Fall back to non-managed in-band inspection + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='inspection') + @METRICS.timer('iPXEBoot.prepare_ramdisk') def prepare_ramdisk(self, task, ramdisk_params): """Prepares the boot of Ironic ramdisk using PXE. diff --git a/ironic/drivers/modules/network/common.py b/ironic/drivers/modules/network/common.py index 87859a1b5c..c1d5b4bfff 100644 --- a/ironic/drivers/modules/network/common.py +++ b/ironic/drivers/modules/network/common.py @@ -390,6 +390,7 @@ class VIFPortIDMixin(object): return (p_obj.internal_info.get('cleaning_vif_port_id') or p_obj.internal_info.get('provisioning_vif_port_id') or p_obj.internal_info.get('rescuing_vif_port_id') + or p_obj.internal_info.get('inspection_vif_port_id') or self._get_vif_id_by_port_like_obj(p_obj) or None) diff --git a/ironic/drivers/modules/network/flat.py b/ironic/drivers/modules/network/flat.py index 1b31aa5dfb..da5c646f82 100644 --- a/ironic/drivers/modules/network/flat.py +++ b/ironic/drivers/modules/network/flat.py @@ -126,6 +126,32 @@ class FlatNetwork(common.NeutronVIFPortIDMixin, """ self._unbind_flat_ports(task) + def _add_service_network(self, task, network, process): + # If we have left over ports from a previous process, remove them + neutron.rollback_ports(task, network) + LOG.info('Adding %s network to node %s', process, task.node.uuid) + vifs = neutron.add_ports_to_network(task, network) + field = '%s_vif_port_id' % process + for port in task.ports: + if port.uuid in vifs: + internal_info = port.internal_info + internal_info[field] = vifs[port.uuid] + port.internal_info = internal_info + port.save() + return vifs + + def _remove_service_network(self, task, network, process): + LOG.info('Removing ports from %s network for node %s', + process, task.node.uuid) + neutron.remove_ports_from_network(task, network) + field = '%s_vif_port_id' % process + for port in task.ports: + if field in port.internal_info: + internal_info = port.internal_info + del internal_info[field] + port.internal_info = internal_info + port.save() + def add_cleaning_network(self, task): """Add the cleaning network to a node. @@ -133,19 +159,8 @@ class FlatNetwork(common.NeutronVIFPortIDMixin, :returns: a dictionary in the form {port.uuid: neutron_port['id']} :raises: NetworkError, InvalidParameterValue """ - # If we have left over ports from a previous cleaning, remove them - neutron.rollback_ports(task, - self.get_cleaning_network_uuid(task)) - LOG.info('Adding cleaning network to node %s', task.node.uuid) - vifs = neutron.add_ports_to_network( - task, self.get_cleaning_network_uuid(task)) - for port in task.ports: - if port.uuid in vifs: - internal_info = port.internal_info - internal_info['cleaning_vif_port_id'] = vifs[port.uuid] - port.internal_info = internal_info - port.save() - return vifs + return self._add_service_network( + task, self.get_cleaning_network_uuid(task), 'cleaning') def remove_cleaning_network(self, task): """Remove the cleaning network from a node. @@ -153,16 +168,8 @@ class FlatNetwork(common.NeutronVIFPortIDMixin, :param task: A TaskManager instance. :raises: NetworkError """ - LOG.info('Removing ports from cleaning network for node %s', - task.node.uuid) - neutron.remove_ports_from_network( - task, self.get_cleaning_network_uuid(task)) - for port in task.ports: - if 'cleaning_vif_port_id' in port.internal_info: - internal_info = port.internal_info - del internal_info['cleaning_vif_port_id'] - port.internal_info = internal_info - port.save() + return self._remove_service_network( + task, self.get_cleaning_network_uuid(task), 'cleaning') def add_rescuing_network(self, task): """Add the rescuing network to a node. @@ -188,3 +195,26 @@ class FlatNetwork(common.NeutronVIFPortIDMixin, """ LOG.info('Unbind ports for rescuing node %s', task.node.uuid) self._unbind_flat_ports(task) + + def add_inspection_network(self, task): + """Add the inspection network to the node. + + :param task: A TaskManager instance. + :returns: a dictionary in the form {port.uuid: neutron_port['id']} + :raises: NetworkError + :raises: InvalidParameterValue, if the network interface configuration + is invalid. + """ + return self._add_service_network( + task, self.get_inspection_network_uuid(task), 'inspection') + + def remove_inspection_network(self, task): + """Removes the inspection network from a node. + + :param task: A TaskManager instance. + :raises: InvalidParameterValue, if the network interface configuration + is invalid. + :raises: MissingParameterValue, if some parameters are missing. + """ + return self._remove_service_network( + task, self.get_inspection_network_uuid(task), 'inspection') diff --git a/ironic/drivers/modules/network/neutron.py b/ironic/drivers/modules/network/neutron.py index 50507c9691..379be9e5cb 100644 --- a/ironic/drivers/modules/network/neutron.py +++ b/ironic/drivers/modules/network/neutron.py @@ -71,27 +71,43 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, 'option.') % node.uuid) raise exception.InvalidParameterValue(error_msg) + def _add_network(self, task, network, security_groups, process): + # If we have left over ports from a previous process, remove them + neutron.rollback_ports(task, network) + LOG.info('Adding %s network to node %s', process, task.node.uuid) + vifs = neutron.add_ports_to_network(task, network, + security_groups=security_groups) + field = '%s_vif_port_id' % process + for port in task.ports: + if port.uuid in vifs: + internal_info = port.internal_info + internal_info[field] = vifs[port.uuid] + port.internal_info = internal_info + port.save() + return vifs + + def _remove_network(self, task, network, process): + LOG.info('Removing ports from %s network for node %s', + process, task.node.uuid) + neutron.remove_ports_from_network(task, network) + field = '%s_vif_port_id' % process + for port in task.ports: + if field in port.internal_info: + internal_info = port.internal_info + del internal_info[field] + port.internal_info = internal_info + port.save() + def add_provisioning_network(self, task): """Add the provisioning network to a node. :param task: A TaskManager instance. :raises: NetworkError """ - # If we have left over ports from a previous provision attempt, remove - # them - neutron.rollback_ports( - task, self.get_provisioning_network_uuid(task)) - LOG.info('Adding provisioning network to node %s', - task.node.uuid) - vifs = neutron.add_ports_to_network( + self._add_network( task, self.get_provisioning_network_uuid(task), - security_groups=CONF.neutron.provisioning_network_security_groups) - for port in task.ports: - if port.uuid in vifs: - internal_info = port.internal_info - internal_info['provisioning_vif_port_id'] = vifs[port.uuid] - port.internal_info = internal_info - port.save() + CONF.neutron.provisioning_network_security_groups, + 'provisioning') def remove_provisioning_network(self, task): """Remove the provisioning network from a node. @@ -99,16 +115,8 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, :param task: A TaskManager instance. :raises: NetworkError """ - LOG.info('Removing provisioning network from node %s', - task.node.uuid) - neutron.remove_ports_from_network( - task, self.get_provisioning_network_uuid(task)) - for port in task.ports: - if 'provisioning_vif_port_id' in port.internal_info: - internal_info = port.internal_info - del internal_info['provisioning_vif_port_id'] - port.internal_info = internal_info - port.save() + return self._remove_network( + task, self.get_provisioning_network_uuid(task), 'provisioning') def add_cleaning_network(self, task): """Create neutron ports for each port on task.node to boot the ramdisk. @@ -117,21 +125,10 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, :raises: NetworkError :returns: a dictionary in the form {port.uuid: neutron_port['id']} """ - # If we have left over ports from a previous cleaning, remove them - neutron.rollback_ports(task, self.get_cleaning_network_uuid(task)) - LOG.info('Adding cleaning network to node %s', task.node.uuid) - security_groups = CONF.neutron.cleaning_network_security_groups - vifs = neutron.add_ports_to_network( - task, - self.get_cleaning_network_uuid(task), - security_groups=security_groups) - for port in task.ports: - if port.uuid in vifs: - internal_info = port.internal_info - internal_info['cleaning_vif_port_id'] = vifs[port.uuid] - port.internal_info = internal_info - port.save() - return vifs + return self._add_network( + task, self.get_cleaning_network_uuid(task), + CONF.neutron.cleaning_network_security_groups, + 'cleaning') def remove_cleaning_network(self, task): """Deletes the neutron port created for booting the ramdisk. @@ -139,16 +136,8 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, :param task: a TaskManager instance. :raises: NetworkError """ - LOG.info('Removing cleaning network from node %s', - task.node.uuid) - neutron.remove_ports_from_network( - task, self.get_cleaning_network_uuid(task)) - for port in task.ports: - if 'cleaning_vif_port_id' in port.internal_info: - internal_info = port.internal_info - del internal_info['cleaning_vif_port_id'] - port.internal_info = internal_info - port.save() + return self._remove_network( + task, self.get_cleaning_network_uuid(task), 'cleaning') def validate_rescue(self, task): """Validates the network interface for rescue operation. @@ -166,21 +155,10 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, :param task: a TaskManager instance. :returns: a dictionary in the form {port.uuid: neutron_port['id']} """ - # If we have left over ports from a previous rescue, remove them - neutron.rollback_ports(task, self.get_rescuing_network_uuid(task)) - LOG.info('Adding rescuing network to node %s', task.node.uuid) - security_groups = CONF.neutron.rescuing_network_security_groups - vifs = neutron.add_ports_to_network( - task, - self.get_rescuing_network_uuid(task), - security_groups=security_groups) - for port in task.ports: - if port.uuid in vifs: - internal_info = port.internal_info - internal_info['rescuing_vif_port_id'] = vifs[port.uuid] - port.internal_info = internal_info - port.save() - return vifs + return self._add_network( + task, self.get_rescuing_network_uuid(task), + CONF.neutron.rescuing_network_security_groups, + 'rescuing') def remove_rescuing_network(self, task): """Deletes neutron port created for booting the rescue ramdisk. @@ -188,16 +166,8 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, :param task: a TaskManager instance. :raises: NetworkError """ - LOG.info('Removing rescuing network from node %s', - task.node.uuid) - neutron.remove_ports_from_network( - task, self.get_rescuing_network_uuid(task)) - for port in task.ports: - if 'rescuing_vif_port_id' in port.internal_info: - internal_info = port.internal_info - del internal_info['rescuing_vif_port_id'] - port.internal_info = internal_info - port.save() + return self._remove_network( + task, self.get_rescuing_network_uuid(task), 'rescuing') def configure_tenant_networks(self, task): """Configure tenant networks for a node. @@ -277,3 +247,28 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, if neutron.is_smartnic_port(port): return True return False + + def add_inspection_network(self, task): + """Add the inspection network to the node. + + :param task: A TaskManager instance. + :returns: a dictionary in the form {port.uuid: neutron_port['id']} + :raises: NetworkError + :raises: InvalidParameterValue, if the network interface configuration + is invalid. + """ + return self._add_network( + task, self.get_inspection_network_uuid(task), + CONF.neutron.inspection_network_security_groups, + 'inspection') + + def remove_inspection_network(self, task): + """Removes the inspection network from a node. + + :param task: A TaskManager instance. + :raises: InvalidParameterValue, if the network interface configuration + is invalid. + :raises: MissingParameterValue, if some parameters are missing. + """ + return self._remove_network( + task, self.get_inspection_network_uuid(task), 'inspection') diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index a73f18abfe..1fc1ad6194 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -61,20 +61,7 @@ class PXEBoot(pxe_base.PXEBaseMixin, base.BootInterface): if CONF.pxe.ipxe_enabled: pxe_utils.create_ipxe_boot_script() - @METRICS.timer('PXEBoot.validate') - def validate(self, task): - """Validate the PXE-specific info for booting deploy/instance images. - - This method validates the PXE-specific info for booting the - ramdisk and instance on the node. If invalid, raises an - exception; otherwise returns None. - - :param task: a task from TaskManager. - :returns: None - :raises: InvalidParameterValue, if some parameters are invalid. - :raises: MissingParameterValue, if some required parameters are - missing. - """ + def _validate_common(self, task): node = task.node if not driver_utils.get_node_mac_addresses(task): @@ -99,11 +86,29 @@ class PXEBoot(pxe_base.PXEBaseMixin, base.BootInterface): validate_boot_parameters_for_trusted_boot(node) pxe_utils.parse_driver_info(node) + + @METRICS.timer('PXEBoot.validate') + def validate(self, task): + """Validate the PXE-specific info for booting deploy/instance images. + + This method validates the PXE-specific info for booting the + ramdisk and instance on the node. If invalid, raises an + exception; otherwise returns None. + + :param task: a task from TaskManager. + :returns: None + :raises: InvalidParameterValue, if some parameters are invalid. + :raises: MissingParameterValue, if some required parameters are + missing. + """ + self._validate_common(task) + # NOTE(TheJulia): If we're not writing an image, we can skip # the remainder of this method. if (not task.driver.storage.should_write_image(task)): return + node = task.node d_info = deploy_utils.get_image_instance_info(node) if (node.driver_internal_info.get('is_whole_disk_image') or deploy_utils.get_boot_option(node) == 'local'): @@ -114,6 +119,20 @@ class PXEBoot(pxe_base.PXEBaseMixin, base.BootInterface): props = ['kernel', 'ramdisk'] deploy_utils.validate_image_properties(task.context, d_info, props) + @METRICS.timer('PXEBoot.validate_inspection') + def validate_inspection(self, task): + """Validate that the node has required properties for inspection. + + :param task: A TaskManager instance with the node being checked + :raises: UnsupportedDriverExtension + """ + try: + self._validate_common(task) + except exception.MissingParameterValue: + # Fall back to non-managed in-band inspection + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='inspection') + @METRICS.timer('PXEBoot.prepare_ramdisk') def prepare_ramdisk(self, task, ramdisk_params): """Prepares the boot of Ironic ramdisk using PXE. diff --git a/ironic/tests/unit/drivers/modules/network/test_flat.py b/ironic/tests/unit/drivers/modules/network/test_flat.py index 7179f6590d..10edc6f9df 100644 --- a/ironic/tests/unit/drivers/modules/network/test_flat.py +++ b/ironic/tests/unit/drivers/modules/network/test_flat.py @@ -104,13 +104,9 @@ class TestFlatInterface(db_base.DbTestCase): task, CONF.neutron.cleaning_network) add_mock.assert_called_once_with( task, CONF.neutron.cleaning_network) - validate_calls = [ - mock.call(CONF.neutron.cleaning_network, 'cleaning network', - context=task.context), - mock.call(CONF.neutron.cleaning_network, 'cleaning network', - context=task.context) - ] - validate_mock.assert_has_calls(validate_calls) + validate_mock.assert_called_once_with( + CONF.neutron.cleaning_network, 'cleaning network', + context=task.context) self.port.refresh() self.assertEqual('vif-port-id', self.port.internal_info['cleaning_vif_port_id']) @@ -273,3 +269,70 @@ class TestFlatInterface(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.interface.remove_provisioning_network(task) unbind_mock.assert_called_once_with(task) + + @mock.patch.object(neutron, 'validate_network', + side_effect=lambda n, t, context=None: n) + @mock.patch.object(neutron, 'add_ports_to_network') + @mock.patch.object(neutron, 'rollback_ports') + def test_add_inspection_network(self, rollback_mock, add_mock, + validate_mock): + add_mock.return_value = {self.port.uuid: 'vif-port-id'} + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.add_inspection_network(task) + rollback_mock.assert_called_once_with( + task, CONF.neutron.inspection_network) + add_mock.assert_called_once_with( + task, CONF.neutron.inspection_network) + validate_mock.assert_called_once_with( + CONF.neutron.inspection_network, 'inspection network', + context=task.context) + self.port.refresh() + self.assertEqual('vif-port-id', + self.port.internal_info['inspection_vif_port_id']) + + @mock.patch.object(neutron, 'validate_network', + side_effect=lambda n, t, context=None: n) + @mock.patch.object(neutron, 'add_ports_to_network') + @mock.patch.object(neutron, 'rollback_ports') + def test_add_inspection_network_from_node(self, rollback_mock, add_mock, + validate_mock): + add_mock.return_value = {self.port.uuid: 'vif-port-id'} + # Make sure that changing the network UUID works + for inspection_network_uuid in [ + '3aea0de6-4b92-44da-9aa0-52d134c83fdf', + '438be438-6aae-4fb1-bbcb-613ad7a38286']: + driver_info = self.node.driver_info + driver_info['inspection_network'] = inspection_network_uuid + self.node.driver_info = driver_info + self.node.save() + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.add_inspection_network(task) + rollback_mock.assert_called_with( + task, inspection_network_uuid) + add_mock.assert_called_with(task, inspection_network_uuid) + validate_mock.assert_called_with( + inspection_network_uuid, + 'inspection network', context=task.context) + self.port.refresh() + self.assertEqual('vif-port-id', + self.port.internal_info['inspection_vif_port_id']) + + @mock.patch.object(neutron, 'validate_network', + side_effect=lambda n, t, context=None: n) + def test_validate_inspection(self, validate_mock): + inspection_network_uuid = '3aea0de6-4b92-44da-9aa0-52d134c83fdf' + driver_info = self.node.driver_info + driver_info['inspection_network'] = inspection_network_uuid + self.node.driver_info = driver_info + self.node.save() + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.validate_inspection(task) + validate_mock.assert_called_once_with( + inspection_network_uuid, 'inspection network', + context=task.context), + + def test_validate_inspection_exc(self): + self.config(inspection_network="", group='neutron') + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaises(exception.UnsupportedDriverExtension, + self.interface.validate_inspection, task) diff --git a/ironic/tests/unit/drivers/modules/network/test_neutron.py b/ironic/tests/unit/drivers/modules/network/test_neutron.py index 28872a5d91..5584b00a88 100644 --- a/ironic/tests/unit/drivers/modules/network/test_neutron.py +++ b/ironic/tests/unit/drivers/modules/network/test_neutron.py @@ -178,13 +178,9 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): add_ports_mock.assert_called_once_with( task, CONF.neutron.provisioning_network, security_groups=[]) - validate_calls = [ - mock.call(CONF.neutron.provisioning_network, - 'provisioning network', context=task.context), - mock.call(CONF.neutron.provisioning_network, - 'provisioning network', context=task.context) - ] - validate_mock.assert_has_calls(validate_calls) + validate_mock.assert_called_once_with( + CONF.neutron.provisioning_network, + 'provisioning network', context=task.context) self.port.refresh() self.assertEqual(self.neutron_port['id'], self.port.internal_info['provisioning_vif_port_id']) @@ -202,6 +198,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): for provisioning_network_uuid in [ '3aea0de6-4b92-44da-9aa0-52d134c83fdf', '438be438-6aae-4fb1-bbcb-613ad7a38286']: + validate_mock.reset_mock() driver_info = self.node.driver_info driver_info['provisioning_network'] = provisioning_network_uuid self.node.driver_info = driver_info @@ -213,13 +210,9 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): add_ports_mock.assert_called_with( task, provisioning_network_uuid, security_groups=[]) - validate_calls = [ - mock.call(provisioning_network_uuid, - 'provisioning network', context=task.context), - mock.call(provisioning_network_uuid, - 'provisioning network', context=task.context) - ] - validate_mock.assert_has_calls(validate_calls) + validate_mock.assert_called_once_with( + provisioning_network_uuid, + 'provisioning network', context=task.context) self.port.refresh() self.assertEqual(self.neutron_port['id'], self.port.internal_info['provisioning_vif_port_id']) @@ -300,13 +293,9 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): rollback_mock.assert_called_once_with( task, CONF.neutron.cleaning_network) self.assertEqual(res, add_ports_mock.return_value) - validate_calls = [ - mock.call(CONF.neutron.cleaning_network, - 'cleaning network', context=task.context), - mock.call(CONF.neutron.cleaning_network, - 'cleaning network', context=task.context) - ] - validate_mock.assert_has_calls(validate_calls) + validate_mock.assert_called_once_with( + CONF.neutron.cleaning_network, + 'cleaning network', context=task.context) self.port.refresh() self.assertEqual(self.neutron_port['id'], self.port.internal_info['cleaning_vif_port_id']) @@ -321,6 +310,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): # Make sure that changing the network UUID works for cleaning_network_uuid in ['3aea0de6-4b92-44da-9aa0-52d134c83fdf', '438be438-6aae-4fb1-bbcb-613ad7a38286']: + validate_mock.reset_mock() driver_info = self.node.driver_info driver_info['cleaning_network'] = cleaning_network_uuid self.node.driver_info = driver_info @@ -329,7 +319,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): res = self.interface.add_cleaning_network(task) rollback_mock.assert_called_with(task, cleaning_network_uuid) self.assertEqual(res, add_ports_mock.return_value) - validate_mock.assert_called_with( + validate_mock.assert_called_once_with( cleaning_network_uuid, 'cleaning network', context=task.context) self.port.refresh() @@ -441,13 +431,9 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): rollback_mock.assert_called_once_with( task, CONF.neutron.rescuing_network) self.assertEqual(add_ports_mock.return_value, res) - validate_calls = [ - mock.call(CONF.neutron.rescuing_network, - 'rescuing network', context=task.context), - mock.call(CONF.neutron.rescuing_network, - 'rescuing network', context=task.context) - ] - validate_mock.assert_has_calls(validate_calls) + validate_mock.assert_called_once_with( + CONF.neutron.rescuing_network, + 'rescuing network', context=task.context) other_port.refresh() self.assertEqual(neutron_other_port['id'], other_port.internal_info['rescuing_vif_port_id']) @@ -481,13 +467,9 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): rollback_mock.assert_called_once_with( task, rescuing_network_uuid) self.assertEqual(add_ports_mock.return_value, res) - validate_calls = [ - mock.call(rescuing_network_uuid, - 'rescuing network', context=task.context), - mock.call(rescuing_network_uuid, - 'rescuing network', context=task.context) - ] - validate_mock.assert_has_calls(validate_calls) + validate_mock.assert_called_once_with( + rescuing_network_uuid, + 'rescuing network', context=task.context) other_port.refresh() self.assertEqual(neutron_other_port['id'], other_port.internal_info['rescuing_vif_port_id']) @@ -779,3 +761,93 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): def test_need_power_on_false(self): with task_manager.acquire(self.context, self.node.id) as task: self.assertFalse(self.interface.need_power_on(task)) + + @mock.patch.object(neutron_common, 'validate_network', + side_effect=lambda n, t, context=None: n) + @mock.patch.object(neutron_common, 'rollback_ports') + @mock.patch.object(neutron_common, 'add_ports_to_network') + def test_add_inspection_network(self, add_ports_mock, rollback_mock, + validate_mock): + add_ports_mock.return_value = {self.port.uuid: self.neutron_port['id']} + with task_manager.acquire(self.context, self.node.id) as task: + res = self.interface.add_inspection_network(task) + rollback_mock.assert_called_once_with( + task, CONF.neutron.inspection_network) + self.assertEqual(res, add_ports_mock.return_value) + validate_mock.assert_called_once_with( + CONF.neutron.inspection_network, + 'inspection network', context=task.context) + self.port.refresh() + self.assertEqual(self.neutron_port['id'], + self.port.internal_info['inspection_vif_port_id']) + + @mock.patch.object(neutron_common, 'validate_network', + side_effect=lambda n, t, context=None: n) + @mock.patch.object(neutron_common, 'rollback_ports') + @mock.patch.object(neutron_common, 'add_ports_to_network') + def test_add_inspection_network_from_node(self, add_ports_mock, + rollback_mock, validate_mock): + add_ports_mock.return_value = {self.port.uuid: self.neutron_port['id']} + # Make sure that changing the network UUID works + for inspection_network_uuid in [ + '3aea0de6-4b92-44da-9aa0-52d134c83fdf', + '438be438-6aae-4fb1-bbcb-613ad7a38286']: + validate_mock.reset_mock() + driver_info = self.node.driver_info + driver_info['inspection_network'] = inspection_network_uuid + self.node.driver_info = driver_info + self.node.save() + with task_manager.acquire(self.context, self.node.id) as task: + res = self.interface.add_inspection_network(task) + rollback_mock.assert_called_with(task, inspection_network_uuid) + self.assertEqual(res, add_ports_mock.return_value) + validate_mock.assert_called_once_with( + inspection_network_uuid, + 'inspection network', context=task.context) + self.port.refresh() + self.assertEqual(self.neutron_port['id'], + self.port.internal_info['inspection_vif_port_id']) + + @mock.patch.object(neutron_common, 'validate_network', + lambda n, t, context=None: n) + @mock.patch.object(neutron_common, 'rollback_ports') + @mock.patch.object(neutron_common, 'add_ports_to_network') + def test_add_inspection_network_with_sg(self, add_ports_mock, + rollback_mock): + add_ports_mock.return_value = {self.port.uuid: self.neutron_port['id']} + sg_ids = [] + for i in range(2): + sg_ids.append(uuidutils.generate_uuid()) + self.config(inspection_network_security_groups=sg_ids, group='neutron') + sg = CONF.neutron.inspection_network_security_groups + with task_manager.acquire(self.context, self.node.id) as task: + res = self.interface.add_inspection_network(task) + add_ports_mock.assert_called_once_with( + task, CONF.neutron.inspection_network, + security_groups=sg) + rollback_mock.assert_called_once_with( + task, CONF.neutron.inspection_network) + self.assertEqual(res, add_ports_mock.return_value) + self.port.refresh() + self.assertEqual(self.neutron_port['id'], + self.port.internal_info['inspection_vif_port_id']) + + @mock.patch.object(neutron_common, 'validate_network', + side_effect=lambda n, t, context=None: n) + def test_validate_inspection(self, validate_mock): + inspection_network_uuid = '3aea0de6-4b92-44da-9aa0-52d134c83fdf' + driver_info = self.node.driver_info + driver_info['inspection_network'] = inspection_network_uuid + self.node.driver_info = driver_info + self.node.save() + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.validate_inspection(task) + validate_mock.assert_called_once_with( + inspection_network_uuid, 'inspection network', + context=task.context), + + def test_validate_inspection_exc(self): + self.config(inspection_network="", group='neutron') + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaises(exception.UnsupportedDriverExtension, + self.interface.validate_inspection, task) diff --git a/ironic/tests/unit/drivers/modules/network/test_noop.py b/ironic/tests/unit/drivers/modules/network/test_noop.py index 156af25df4..be432988e0 100644 --- a/ironic/tests/unit/drivers/modules/network/test_noop.py +++ b/ironic/tests/unit/drivers/modules/network/test_noop.py @@ -86,3 +86,11 @@ class NoopInterfaceTestCase(db_base.DbTestCase): def test_remove_cleaning_network(self): with task_manager.acquire(self.context, self.node.id) as task: self.interface.remove_cleaning_network(task) + + def test_add_inspection_network(self): + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.add_inspection_network(task) + + def test_remove_inspection_network(self): + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.remove_inspection_network(task) diff --git a/ironic/tests/unit/drivers/modules/test_ipxe.py b/ironic/tests/unit/drivers/modules/test_ipxe.py index 65f0194cdb..73b078817f 100644 --- a/ironic/tests/unit/drivers/modules/test_ipxe.py +++ b/ironic/tests/unit/drivers/modules/test_ipxe.py @@ -207,6 +207,19 @@ class iPXEBootTestCase(db_base.DbTestCase): self.assertRaises(exception.InvalidParameterValue, task.driver.boot.validate, task) + def test_validate_inspection(self): + with task_manager.acquire(self.context, self.node.uuid) as task: + task.driver.boot.validate_inspection(task) + + def test_validate_inspection_no_inspection_ramdisk(self): + driver_info = self.node.driver_info + del driver_info['deploy_ramdisk'] + self.node.driver_info = driver_info + self.node.save() + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertRaises(exception.UnsupportedDriverExtension, + task.driver.boot.validate_inspection, task) + # TODO(TheJulia): Many of the interfaces mocked below are private PXE # interface methods. As time progresses, these will need to be migrated # and refactored as we begin to separate PXE and iPXE interfaces. diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index 8d710b4d0e..af3defe163 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -207,6 +207,19 @@ class PXEBootTestCase(db_base.DbTestCase): self.assertRaises(exception.InvalidParameterValue, task.driver.boot.validate, task) + def test_validate_inspection(self): + with task_manager.acquire(self.context, self.node.uuid) as task: + task.driver.boot.validate_inspection(task) + + def test_validate_inspection_no_inspection_ramdisk(self): + driver_info = self.node.driver_info + del driver_info['deploy_ramdisk'] + self.node.driver_info = driver_info + self.node.save() + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertRaises(exception.UnsupportedDriverExtension, + task.driver.boot.validate_inspection, task) + @mock.patch.object(manager_utils, 'node_get_boot_mode', autospec=True) @mock.patch.object(manager_utils, 'node_set_boot_device', autospec=True) @mock.patch.object(dhcp_factory, 'DHCPFactory') diff --git a/releasenotes/notes/inspector-pxe-boot-9ab9fede5671097e.yaml b/releasenotes/notes/inspector-pxe-boot-9ab9fede5671097e.yaml new file mode 100644 index 0000000000..a05ad67f3e --- /dev/null +++ b/releasenotes/notes/inspector-pxe-boot-9ab9fede5671097e.yaml @@ -0,0 +1,12 @@ +--- +features: + - | + The ``pxe`` and ``ipxe`` boot interfaces, as well as all in-tree network + interfaces, now support managing in-band inspection boot. +upgrade: + - | + For the managed in-band inspection to work, make sure that the Bare Metal + Introspection endpoint (either in the service catalog or in the + ``[inspector]endpoint_override`` configuration option) is not set to + localhost. Alternatively, set the ``[inspector]callback_endpoint_override`` + option to a value with a real IP address.