diff --git a/ironic/common/exception.py b/ironic/common/exception.py index d482f43d15..ce7f4bd3d0 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -879,3 +879,7 @@ class FirmwareComponentAlreadyExists(Conflict): class FirmwareComponentNotFound(NotFound): _msg_fmt = _("Node %(node)s doesn't have Firmware component %(name)s") + + +class InvalidNodeInventory(Invalid): + _msg_fmt = _("Inventory for node %(node)s is invalid: %(reason)s") diff --git a/ironic/conf/inspector.py b/ironic/conf/inspector.py index ce9ece205b..e987c7a8e9 100644 --- a/ironic/conf/inspector.py +++ b/ironic/conf/inspector.py @@ -55,6 +55,20 @@ opts = [ 'to False if you need to inspect nodes that are not ' 'supported by boot interfaces (e.g. because they ' 'don\'t have ports).')), + cfg.StrOpt('add_ports', + default='pxe', + help=_('Which MAC addresses to add as ports during ' + 'inspection.'), + choices=list(VALID_ADD_PORTS_VALUES.items())), + cfg.StrOpt('keep_ports', + default='all', + help=_('Which ports (already present on a node) to keep after ' + 'inspection.'), + choices=list(VALID_KEEP_PORTS_VALUES.items())), + cfg.BoolOpt('update_pxe_enabled', + default=True, + help=_('Whether to update the ports\' pxe_enabled field ' + 'according to the inspection data.')), ] diff --git a/ironic/drivers/modules/inspector/agent.py b/ironic/drivers/modules/inspector/agent.py index f4886972b8..bc5bc08b1a 100644 --- a/ironic/drivers/modules/inspector/agent.py +++ b/ironic/drivers/modules/inspector/agent.py @@ -23,6 +23,7 @@ from ironic.conductor import utils as cond_utils from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import inspect_utils from ironic.drivers.modules.inspector import interface as common +from ironic.drivers.modules.inspector import nics LOG = logging.getLogger(__name__) @@ -78,6 +79,5 @@ class AgentInspect(common.Common): :param plugin_data: optional plugin-specific data. """ # TODO(dtantsur): migrate the whole pipeline from ironic-inspector - LOG.error('Meaningful inspection is not implemented yet in the "agent"' - ' inspect interface') + nics.process_interfaces(task, inventory, plugin_data) common.clean_up(task, finish=False) diff --git a/ironic/drivers/modules/inspector/nics.py b/ironic/drivers/modules/inspector/nics.py new file mode 100644 index 0000000000..12dc00dbe5 --- /dev/null +++ b/ironic/drivers/modules/inspector/nics.py @@ -0,0 +1,209 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_log import log as logging +from oslo_utils import netutils + +from ironic.common import exception +from ironic.common.i18n import _ +from ironic.common import utils +from ironic.conf import CONF +from ironic import objects + +LOG = logging.getLogger(__name__) + + +def get_pxe_mac(inventory): + """Get MAC address of the PXE interface.""" + pxe_mac = inventory.get('boot', {}).get('pxe_interface') + if pxe_mac and '-' in pxe_mac: + # pxelinux format: 01-aa-bb-cc-dd-ee-ff + pxe_mac = pxe_mac.split('-', 1)[1] + pxe_mac = pxe_mac.replace('-', ':').lower() + return pxe_mac + + +def get_interfaces(node, inventory): + """Convert inventory to a dict with interfaces. + + :return: dict interface name -> interface (for valid interfaces). + """ + result = {} + pxe_mac = get_pxe_mac(inventory) + + for iface in inventory['interfaces']: + name = iface.get('name') + mac = iface.get('mac_address') + ipv4_address = iface.get('ipv4_address') + ipv6_address = iface.get('ipv6_address') + # NOTE(kaifeng) ipv6 address may in the form of fd00::1%enp2s0, + # which is not supported by netaddr, remove the suffix if exists. + if ipv6_address and '%' in ipv6_address: + ipv6_address = ipv6_address.split('%')[0] + ip = ipv4_address or ipv6_address + client_id = iface.get('client_id') + + if not name: + LOG.error('Malformed interface record for node %(node)s: %(if)s', + {'node': node.uuid, 'if': iface}) + continue + + if not mac: + LOG.debug('Skipping interface %(if)s for node %(node)s without ' + 'link information', + {'node': node.uuid, 'if': iface}) + continue + + if not netutils.is_valid_mac(mac): + LOG.warning('MAC address of interface %(if)s of node %(node)s ' + 'is not valid, skipping', + {'node': node.uuid, 'if': iface}) + continue + + mac = mac.lower() + + LOG.debug('Found interface %(name)s with MAC "%(mac)s", ' + 'IP address "%(ip)s" and client_id "%(client_id)s" ' + 'for node %(node)s', + {'name': name, 'mac': mac, 'ip': ip, + 'client_id': client_id, 'node': node.uuid}) + result[name] = dict(iface, pxe_enabled=(mac == pxe_mac), + # IPv6 address without scope. + ipv6_address=ipv6_address) + + return result + + +def validate_interfaces(node, inventory, interfaces): + """Validate interfaces on correctness and suitability. + + :return: dict interface name -> interface. + """ + if not interfaces: + raise exception.InvalidNodeInventory( + node=node.uuid, + reason=_('no valid network interfaces')) + + pxe_mac = get_pxe_mac(inventory) + if not pxe_mac and CONF.inspector.add_ports == 'pxe': + LOG.warning('No boot interface provided in the inventory for node ' + '%s, will add all ports with IP addresses', node.uuid) + + result = {} + + for name, iface in interfaces.items(): + ip = iface.get('ipv4_address') or iface.get('ipv6_address') + pxe = iface.get('pxe_enabled', True) + + if name == 'lo' or (ip and utils.is_loopback(ip)): + LOG.debug('Skipping local interface %(iface)s for node %(node)s', + {'iface': name, 'node': node.uuid}) + continue + + if CONF.inspector.add_ports == 'pxe' and pxe_mac and not pxe: + LOG.debug('Skipping interface %(iface)s for node %(node)s as it ' + 'was not PXE booting and add_ports is set to "pxe"', + {'iface': name, 'node': node.uuid}) + continue + + if CONF.inspector.add_ports == 'active' and not ip: + LOG.debug('Skipping interface %(iface)s for node %(node)s as it ' + 'did not have an IP address assigned during the ramdisk ' + 'run and add_ports is set to "active"', + {'iface': name, 'node': node.uuid}) + continue + + result[name] = iface.copy() + + if not result: + raise exception.InvalidNodeInventory( + node=node.uuid, + reason=_('no network interfaces match the configuration ' + '(add_ports set to "%s")') % CONF.inspector.add_ports) + return result + + +def add_ports(task, interfaces): + """Add ports for all previously validated interfaces.""" + for iface in interfaces.values(): + mac = iface['mac_address'] + extra = {} + if iface.get('client_id'): + extra['client-id'] = iface['client_id'] + port_dict = {'address': mac, 'node_id': task.node.id, + 'pxe_enabled': iface['pxe_enabled'], 'extra': extra} + port = objects.Port(task.context, **port_dict) + try: + port.create() + LOG.info("Port created for MAC address %(address)s for " + "node %(node)s%(pxe)s", + {'address': mac, 'node': task.node.uuid, + 'pxe': ' (PXE booting)' if iface['pxe_enabled'] else ''}) + except exception.MACAlreadyExists: + LOG.info("Port already exists for MAC address %(address)s " + "for node %(node)s", + {'address': mac, 'node': task.node.uuid}) + + +def update_ports(task, all_interfaces, valid_macs): + """Update ports to match the valid MACs. + + Depending on the value of ``[inspector]keep_ports``, some ports may be + removed. + """ + # TODO(dtantsur): no port update for active nodes (when supported) + + if CONF.inspector.keep_ports == 'present': + expected_macs = {iface['mac_address'] + for iface in all_interfaces.values()} + elif CONF.inspector.keep_ports == 'added': + expected_macs = set(valid_macs) + else: + expected_macs = None # unused + + pxe_macs = {iface['mac_address'] for iface in all_interfaces.values() + if iface['pxe_enabled']} + + for port in objects.Port.list_by_node_id(task.context, task.node.id): + if expected_macs and port.address not in expected_macs: + expected_str = ', '.join(sorted(expected_macs)) + LOG.info("Deleting port %(port)s of node %(node)s as its MAC " + "%(mac)s is not in the expected MAC list [%(expected)s]", + {'port': port.uuid, 'mac': port.address, + 'node': task.node.uuid, 'expected': expected_str}) + port.destroy() + elif CONF.inspector.update_pxe_enabled: + pxe_enabled = port.address in pxe_macs + if pxe_enabled != port.pxe_enabled: + LOG.debug('Changing pxe_enabled=%(val)s on port %(port)s ' + 'of node %(node)s to match the inventory', + {'port': port.address, 'val': pxe_enabled, + 'node': task.node.uuid}) + port.pxe_enabled = pxe_enabled + port.save() + + +def process_interfaces(task, inventory, plugin_data): + """Process network interfaces in the inventory.""" + # TODO(dtantsur): this function will become two hooks in the future. + all_interfaces = get_interfaces(task.node, inventory) + interfaces = validate_interfaces(task.node, inventory, all_interfaces) + valid_macs = [iface['mac_address'] for iface in interfaces.values()] + + plugin_data['all_interfaces'] = all_interfaces + plugin_data['valid_interfaces'] = interfaces + plugin_data['macs'] = valid_macs + + if CONF.inspector.add_ports != 'disabled': + add_ports(task, interfaces) + + update_ports(task, all_interfaces, valid_macs) diff --git a/ironic/tests/unit/drivers/modules/inspector/test_agent.py b/ironic/tests/unit/drivers/modules/inspector/test_agent.py index fbd08b9013..6eea58e05e 100644 --- a/ironic/tests/unit/drivers/modules/inspector/test_agent.py +++ b/ironic/tests/unit/drivers/modules/inspector/test_agent.py @@ -21,6 +21,7 @@ from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import inspect_utils from ironic.drivers.modules.inspector import agent as inspector from ironic.drivers.modules.inspector import interface as common +from ironic.drivers.modules.inspector import nics from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as obj_utils @@ -102,6 +103,7 @@ class InspectHardwareTestCase(db_base.DbTestCase): @mock.patch.object(common, 'tear_down_managed_boot', autospec=True) +@mock.patch.object(nics, 'process_interfaces', autospec=True) class ContinueInspectionTestCase(db_base.DbTestCase): def setUp(self): super().setUp() @@ -113,11 +115,13 @@ class ContinueInspectionTestCase(db_base.DbTestCase): provision_state=states.INSPECTING) self.iface = inspector.AgentInspect() - def test(self, mock_tear_down): + def test(self, mock_process, mock_tear_down): mock_tear_down.return_value = None with task_manager.acquire(self.context, self.node.id) as task: result = self.iface.continue_inspection( task, mock.sentinel.inventory, mock.sentinel.plugin_data) + mock_process.assert_called_once_with( + task, mock.sentinel.inventory, mock.sentinel.plugin_data) mock_tear_down.assert_called_once_with(task) self.assertEqual(states.INSPECTING, task.node.provision_state) diff --git a/ironic/tests/unit/drivers/modules/inspector/test_nics.py b/ironic/tests/unit/drivers/modules/inspector/test_nics.py new file mode 100644 index 0000000000..7a567322dc --- /dev/null +++ b/ironic/tests/unit/drivers/modules/inspector/test_nics.py @@ -0,0 +1,311 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import copy +from unittest import mock + +from oslo_utils import uuidutils + +from ironic.common import exception +from ironic.conductor import task_manager +from ironic.conf import CONF +from ironic.drivers.modules.inspector import nics +from ironic import objects +from ironic.tests.unit.db import base as db_base +from ironic.tests.unit.objects import utils as obj_utils + + +_PXE_INTERFACE = 'aa:bb:cc:dd:ee:ff' + + +_INVENTORY = { + 'boot': { + 'pxe_interface': _PXE_INTERFACE, + }, + 'interfaces': [ + { + 'name': 'lo', + 'mac_address': '00:00:00:00:00:00', + 'ipv4_address': '127.0.0.1', + 'ipv6_address': '::1', + 'has_carrier': True, + }, + { + 'name': 'em0', + 'mac_address': _PXE_INTERFACE, + 'ipv4_address': '192.0.2.1', + 'ipv6_address': '2001:db8::1', + 'has_carrier': True, + }, + { + 'name': 'em1', + 'mac_address': '11:11:11:11:11:11', + 'ipv4_address': '192.0.2.2', + 'ipv6_address': '2001:db8::2', + 'has_carrier': True, + }, + { + 'name': 'em2', + 'mac_address': '22:22:22:22:22:22', + 'ipv4_address': None, + 'ipv6_address': '2001:db8::3', + 'has_carrier': True, + }, + { + 'name': 'em3', + 'mac_address': '33:33:33:33:33:33', + 'ipv4_address': '192.0.2.4', + 'ipv6_address': '2001:db8::4%em4', + 'has_carrier': True, + }, + { + 'name': 'em4', + 'mac_address': '44:44:44:44:44:44', + 'ipv4_address': None, + 'ipv6_address': None, + 'has_carrier': False, + }, + ], +} + + +_VALID = { + 'lo': { + 'name': 'lo', + 'mac_address': '00:00:00:00:00:00', + 'ipv4_address': '127.0.0.1', + 'ipv6_address': '::1', + 'has_carrier': True, + 'pxe_enabled': False, + }, + 'em0': { + 'name': 'em0', + 'mac_address': _PXE_INTERFACE, + 'ipv4_address': '192.0.2.1', + 'ipv6_address': '2001:db8::1', + 'has_carrier': True, + 'pxe_enabled': True, + }, + 'em1': { + 'name': 'em1', + 'mac_address': '11:11:11:11:11:11', + 'ipv4_address': '192.0.2.2', + 'ipv6_address': '2001:db8::2', + 'has_carrier': True, + 'pxe_enabled': False, + }, + 'em2': { + 'name': 'em2', + 'mac_address': '22:22:22:22:22:22', + 'ipv4_address': None, + 'ipv6_address': '2001:db8::3', + 'has_carrier': True, + 'pxe_enabled': False, + }, + 'em3': { + 'name': 'em3', + 'mac_address': '33:33:33:33:33:33', + 'ipv4_address': '192.0.2.4', + 'ipv6_address': '2001:db8::4', # note: no scope + 'has_carrier': True, + 'pxe_enabled': False, + }, + 'em4': { + 'name': 'em4', + 'mac_address': '44:44:44:44:44:44', + 'ipv4_address': None, + 'ipv6_address': None, + 'has_carrier': False, + 'pxe_enabled': False, + }, +} + + +class GetInterfacesTestCase(db_base.DbTestCase): + def setUp(self): + super().setUp() + self.node = obj_utils.create_test_node(self.context, + inspect_interface='agent') + self.inventory = copy.deepcopy(_INVENTORY) + self.inventory['interfaces'].extend([ + # Broken records + { + 'mac_address': '55:55:55:55:55:55', + 'ipv4_address': None, + 'ipv6_address': None, + 'has_carrier': False, + }, + { + 'name': 'broken', + 'mac_address': 'banana!', + 'ipv4_address': None, + 'ipv6_address': None, + 'has_carrier': False, + }, + ]) + + def test_get_interfaces(self): + result = nics.get_interfaces(self.node, self.inventory) + self.assertEqual(_VALID, result) + + +class ValidateInterfacesTestCase(db_base.DbTestCase): + def setUp(self): + super().setUp() + self.node = obj_utils.create_test_node(self.context, + inspect_interface='agent') + self.inventory = copy.deepcopy(_INVENTORY) + self.interfaces = copy.deepcopy(_VALID) + + def test_pxe_only(self): + expected = {'em0': _VALID['em0']} + result = nics.validate_interfaces( + self.node, self.inventory, self.interfaces) + self.assertEqual(expected, result) + # Make sure we don't modify the initial structures + self.assertEqual(_INVENTORY, self.inventory) + self.assertEqual(_VALID, self.interfaces) + + def test_all_interfaces(self): + CONF.set_override('add_ports', 'all', group='inspector') + expected = {key: value.copy() for key, value in _VALID.items() + if key != 'lo'} + result = nics.validate_interfaces( + self.node, self.inventory, self.interfaces) + self.assertEqual(expected, result) + + @mock.patch.object(nics.LOG, 'warning', autospec=True) + def test_no_pxe_fallback_to_all(self, mock_warn): + del self.inventory['boot'] + expected = {key: value.copy() for key, value in _VALID.items() + if key != 'lo'} + result = nics.validate_interfaces( + self.node, self.inventory, self.interfaces) + self.assertEqual(expected, result) + self.assertTrue(mock_warn.called) + + def test_active_interfaces(self): + CONF.set_override('add_ports', 'active', group='inspector') + expected = {key: value.copy() for key, value in _VALID.items() + if key not in ('lo', 'em4')} + result = nics.validate_interfaces( + self.node, self.inventory, self.interfaces) + self.assertEqual(expected, result) + + def test_nothing_to_add(self): + CONF.set_override('add_ports', 'active', group='inspector') + self.interfaces = {key: value.copy() for key, value in _VALID.items() + if key in ('lo', 'em4')} + self.assertRaises(exception.InvalidNodeInventory, + nics.validate_interfaces, + self.node, self.inventory, self.interfaces) + + +class AddPortsTestCase(db_base.DbTestCase): + def setUp(self): + super().setUp() + CONF.set_override('enabled_inspect_interfaces', + ['agent', 'no-inspect']) + self.node = obj_utils.create_test_node(self.context, + inspect_interface='agent') + self.interfaces = {key: value.copy() for key, value in _VALID.items() + if key not in ('lo', 'em4')} + self.macs = { + _PXE_INTERFACE, + '11:11:11:11:11:11', + '22:22:22:22:22:22', + '33:33:33:33:33:33', + } + + def test_add(self): + with task_manager.acquire(self.context, self.node.id) as task: + nics.add_ports(task, self.interfaces) + ports = objects.Port.list_by_node_id(self.context, self.node.id) + self.assertEqual( + {port.address: port.pxe_enabled for port in ports}, + {mac: (mac == _PXE_INTERFACE) for mac in self.macs}) + + def test_duplicates(self): + obj_utils.create_test_port(self.context, + node_id=self.node.id, + address=_PXE_INTERFACE, + pxe_enabled=False) + with task_manager.acquire(self.context, self.node.id) as task: + nics.add_ports(task, self.interfaces) + ports = objects.Port.list_by_node_id(self.context, self.node.id) + self.assertEqual( + {port.address: port.pxe_enabled for port in ports}, + # Always False because the PXE port already existed + {mac: False for mac in self.macs}) + + +class UpdatePortsTestCase(db_base.DbTestCase): + def setUp(self): + super().setUp() + CONF.set_override('enabled_inspect_interfaces', + ['agent', 'no-inspect']) + self.node = obj_utils.create_test_node(self.context, + inspect_interface='agent') + self.macs = { + _PXE_INTERFACE, + '11:11:11:11:11:11', + '22:22:22:22:22:22', + '33:33:33:33:33:33', + } + self.present_macs = {i['mac_address'] for i in _VALID.values()} + self.extra_mac = '00:11:00:11:00:11' + self.all_macs = self.present_macs | {self.extra_mac} + for mac in self.all_macs: + obj_utils.create_test_port(self.context, + uuid=uuidutils.generate_uuid(), + node_id=self.node.id, + address=mac, + pxe_enabled=(mac == self.extra_mac)) + + def test_keep_all(self): + with task_manager.acquire(self.context, self.node.id) as task: + nics.update_ports(task, _VALID, self.macs) + ports = objects.Port.list_by_node_id(self.context, self.node.id) + self.assertEqual( + {port.address: port.pxe_enabled for port in ports}, + # Nothing is removed by default, pxe_enabled updated + {mac: (mac == _PXE_INTERFACE) for mac in self.all_macs}) + + def test_keep_pxe_enabled(self): + CONF.set_override('update_pxe_enabled', False, group='inspector') + with task_manager.acquire(self.context, self.node.id) as task: + nics.update_ports(task, _VALID, self.macs) + ports = objects.Port.list_by_node_id(self.context, self.node.id) + self.assertEqual( + {port.address: port.pxe_enabled for port in ports}, + # Nothing is removed by default, pxe_enabled kept intact + {mac: (mac == self.extra_mac) for mac in self.all_macs}) + + def test_keep_added(self): + CONF.set_override('keep_ports', 'added', group='inspector') + with task_manager.acquire(self.context, self.node.id) as task: + nics.update_ports(task, _VALID, self.macs) + ports = objects.Port.list_by_node_id(self.context, self.node.id) + self.assertEqual( + {port.address: port.pxe_enabled for port in ports}, + # Extra ports removed, pxe_enabled updated + {mac: (mac == _PXE_INTERFACE) for mac in self.macs}) + + def test_keep_present(self): + CONF.set_override('keep_ports', 'present', group='inspector') + with task_manager.acquire(self.context, self.node.id) as task: + nics.update_ports(task, _VALID, self.macs) + ports = objects.Port.list_by_node_id(self.context, self.node.id) + self.assertEqual( + {port.address: port.pxe_enabled for port in ports}, + # Extra port removed, pxe_enabled updated + {mac: (mac == _PXE_INTERFACE) for mac in self.present_macs})