From 5086d93b41f0178e820825af8ada53e15a95f177 Mon Sep 17 00:00:00 2001 From: Anton Arefiev Date: Wed, 17 Feb 2016 16:27:02 +0200 Subject: [PATCH] Add enroll_node_not_found hook Add new node_not_found_hook - enroll_node_not_found hook, which allows to enroll unknown nodes to Ironic automatically. Change-Id: If1528688504e4be4b2369b985bc576544d96868d Related-Bug: #1524753 --- doc/source/http-api.rst | 2 + doc/source/install.rst | 4 +- doc/source/usage.rst | 68 ++++++++++ example.conf | 11 ++ ironic_inspector/node_cache.py | 25 +++- ironic_inspector/plugins/discovery.py | 101 ++++++++++++++ ironic_inspector/test/test_node_cache.py | 41 ++++++ .../test/test_plugins_discovery.py | 127 ++++++++++++++++++ ironic_inspector/test/test_utils.py | 4 +- ironic_inspector/utils.py | 4 +- .../notes/enroll-hook-d8c32eba70848210.yaml | 6 + setup.cfg | 2 + tox.ini | 1 + 13 files changed, 389 insertions(+), 7 deletions(-) create mode 100644 ironic_inspector/plugins/discovery.py create mode 100644 ironic_inspector/test/test_plugins_discovery.py create mode 100644 releasenotes/notes/enroll-hook-d8c32eba70848210.yaml diff --git a/doc/source/http-api.rst b/doc/source/http-api.rst index 3199cec6f..8783151b3 100644 --- a/doc/source/http-api.rst +++ b/doc/source/http-api.rst @@ -150,6 +150,8 @@ authentication. * 204 - OK * 404 - not found +.. _ramdisk_callback: + Ramdisk Callback ~~~~~~~~~~~~~~~~ diff --git a/doc/source/install.rst b/doc/source/install.rst index f5132a607..ae92732ef 100644 --- a/doc/source/install.rst +++ b/doc/source/install.rst @@ -24,8 +24,8 @@ Note for Ubuntu users Version Support Matrix ~~~~~~~~~~~~~~~~~~~~~~ -**ironic-inspector** currently requires bare metal API version ``1.6`` to be -provided by Ironic. This version is available starting with Ironic Kilo +**ironic-inspector** currently requires bare metal API version ``1.11`` to be +provided by Ironic. This version is available starting with Ironic Liberty release. Here is a mapping between Ironic versions and supported **ironic-inspector** diff --git a/doc/source/usage.rst b/doc/source/usage.rst index b315ab996..c82efd550 100644 --- a/doc/source/usage.rst +++ b/doc/source/usage.rst @@ -207,3 +207,71 @@ Here are some plugins that can be additionally enabled: Refer to :ref:`contributing_link` for information on how to write your own plugin. + +Discovery +~~~~~~~~~ + +Starting from Mitaka, **ironic-inspector** is able to register new nodes +in Ironic. + +The existing ``node-not-found-hook`` handles what happens if +**ironic-inspector** receives inspection data from a node it can not identify. +This can happen if a node is manually booted without registering it with +Ironic first. + +For discovery, the configuration file option ``node_not_found_hook`` should be +set to load the hook called ``enroll``. This hook will enroll the unidentified +node into Ironic using the ``fake`` driver (this driver is a configurable +option, set ``enroll_node_driver`` in the **ironic-inspector** configuration +file, to the Ironic driver you want). + +The ``enroll`` hook will also set the ``ipmi_address`` property on the new +node, if its available in the introspection data we received, +see :ref:`ramdisk_callback`. + +Once the ``enroll`` hook is finished, **ironic-inspector** will process the +introspection data in the same way it would for an identified node. It runs +the processing plugins :ref:`_plugins`, and after that it runs introspection +rules, which would allow for more customisable node configuration, +see :ref:`_rules`. + +A rule to set a node's Ironic driver to the ``agent_ipmitool`` driver and +populate the required driver_info for that driver would look like:: + + "description": "Set IPMI driver_info if no credentials", + "actions": [ + {'action': 'set-attribute', 'path': 'driver', 'value': 'agent_ipmitool'}, + {'action': 'set-attribute', 'path': 'driver_info/ipmi_username', + 'value': 'username'}, + {'action': 'set-attribute', 'path': 'driver_info/ipmi_password', + 'value': 'password'} + ] + "conditions": [ + {'op': 'is-empty', 'field': 'node://driver_info.ipmi_password'}, + {'op': 'is-empty', 'field': 'node://driver_info.ipmi_username'} + ] + + "description": "Set deploy info if not already set on node", + "actions": [ + {'action': 'set-attribute', 'path': 'driver_info/deploy_kernel', + 'value': ''}, + {'action': 'set-attribute', 'path': 'driver_info/deploy_ramdisk', + 'value': ''}, + ] + "conditions": [ + {'op': 'is-empty', 'field': 'node://driver_info.deploy_ramdisk'}, + {'op': 'is-empty', 'field': 'node://driver_info.deploy_kernel'} + ] + +All nodes discovered and enrolled via the ``enroll`` hook, will contain an +``auto_discovered`` flag in the introspection data, this flag makes it +possible to distinguish between manually enrolled nodes and auto-discovered +nodes in the introspection rules using the rule condition ``eq``:: + + "description": "Enroll auto-discovered nodes with fake driver", + "actions": [ + {'action': 'set-attribute', 'path': 'driver', 'value': 'fake'} + ] + "conditions": [ + {'op': 'eq', 'field': 'data://auto_discovered', 'value': True} + ] diff --git a/example.conf b/example.conf index 4a5ba8a1d..c6db4917c 100644 --- a/example.conf +++ b/example.conf @@ -301,6 +301,17 @@ #database = +[discovery] + +# +# From ironic_inspector.plugins.discovery +# + +# The name of the Ironic driver used by the enroll hook when creating +# a new node in Ironic. (string value) +#enroll_node_driver = fake + + [firewall] # diff --git a/ironic_inspector/node_cache.py b/ironic_inspector/node_cache.py index 85e3491a0..325633107 100644 --- a/ironic_inspector/node_cache.py +++ b/ironic_inspector/node_cache.py @@ -25,7 +25,7 @@ from oslo_utils import excutils from sqlalchemy import text from ironic_inspector import db -from ironic_inspector.common.i18n import _, _LE, _LW +from ironic_inspector.common.i18n import _, _LE, _LW, _LI from ironic_inspector import utils CONF = cfg.CONF @@ -568,3 +568,26 @@ def clean_up(): node_info.release_lock() return uuids + + +def create_node(driver, ironic=None, **attributes): + """Create ironic node and cache it. + + * Create new node in ironic. + * Cache it in inspector. + + :param driver: driver for Ironic node. + :param ironic: ronic client instance. + :param attributes: dict, additional keyword arguments to pass + to the ironic client on node creation. + :return: NodeInfo, or None in case error happened. + """ + if ironic is None: + ironic = utils.get_client() + try: + node = ironic.node.create(driver=driver, **attributes) + except exceptions.InvalidAttribute as e: + LOG.error(_LE('Failed to create new node: %s'), e) + else: + LOG.info(_LI('Node %s was created successfully'), node.uuid) + return add_node(node.uuid, ironic=ironic) diff --git a/ironic_inspector/plugins/discovery.py b/ironic_inspector/plugins/discovery.py new file mode 100644 index 000000000..cb0f9c846 --- /dev/null +++ b/ironic_inspector/plugins/discovery.py @@ -0,0 +1,101 @@ +# 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. + +"""Enroll node not found hook hook.""" + +from oslo_config import cfg + +from ironic_inspector.common.i18n import _, _LW +from ironic_inspector import node_cache +from ironic_inspector import utils + + +DISCOVERY_OPTS = [ + cfg.StrOpt('enroll_node_driver', + default='fake', + help='The name of the Ironic driver used by the enroll ' + 'hook when creating a new node in Ironic.'), +] + + +def list_opts(): + return [ + ('discovery', DISCOVERY_OPTS) + ] + +CONF = cfg.CONF +CONF.register_opts(DISCOVERY_OPTS, group='discovery') + +LOG = utils.getProcessingLogger(__name__) + + +def _extract_node_driver_info(introspection_data): + node_driver_info = {} + ipmi_address = utils.get_ipmi_address_from_data(introspection_data) + if ipmi_address: + node_driver_info['ipmi_address'] = ipmi_address + else: + LOG.warning(_LW('No BMC address provided, discovered node will be ' + 'created without ipmi address')) + return node_driver_info + + +def _check_existing_nodes(introspection_data, node_driver_info, ironic): + macs = introspection_data.get('macs') + if macs: + # verify existing ports + for mac in macs: + ports = ironic.port.list(address=mac) + if not ports: + continue + raise utils.Error( + _('Port %(mac)s already exists, uuid: %(uuid)s') % + {'mac': mac, 'uuid': ports.uuid}, data=introspection_data) + else: + LOG.warning(_LW('No suitable interfaces found for discovered node. ' + 'Check that validate_interfaces hook is listed in ' + '[processing]default_processing_hooks config option')) + + # verify existing node with discovered ipmi address + ipmi_address = node_driver_info.get('ipmi_address') + if ipmi_address: + # FIXME(aarefiev): it's not effective to fetch all nodes, and may + # impact on performance on big clusters + nodes = ironic.node.list(fields=('uuid', 'driver_info'), limit=0) + for node in nodes: + if ipmi_address == utils.get_ipmi_address(node): + raise utils.Error( + _('Node %(uuid)s already has BMC address ' + '%(ipmi_address)s, not enrolling') % + {'ipmi_address': ipmi_address, 'uuid': node.uuid}, + data=introspection_data) + + +def enroll_node_not_found_hook(introspection_data, **kwargs): + node_attr = {} + ironic = utils.get_client() + + node_driver_info = _extract_node_driver_info(introspection_data) + node_attr['driver_info'] = node_driver_info + + node_driver = CONF.discovery.enroll_node_driver + + _check_existing_nodes(introspection_data, node_driver_info, ironic) + LOG.debug('Creating discovered node with driver %(driver)s and ' + 'attributes: %(attr)s', + {'driver': node_driver, 'attr': node_attr}, + data=introspection_data) + # NOTE(aarefiev): This flag allows to distinguish enrolled manually + # and auto-discovered nodes in the introspection rules. + introspection_data['auto_discovered'] = True + return node_cache.create_node(node_driver, ironic=ironic, **node_attr) diff --git a/ironic_inspector/test/test_node_cache.py b/ironic_inspector/test/test_node_cache.py index 070f31c06..c98d76af7 100644 --- a/ironic_inspector/test/test_node_cache.py +++ b/ironic_inspector/test/test_node_cache.py @@ -675,3 +675,44 @@ class TestLock(test_base.NodeTest): self.assertTrue(node_info._locked) get_lock_mock.return_value.acquire.assert_called_with(False) self.assertEqual(2, get_lock_mock.return_value.acquire.call_count) + + +@mock.patch.object(node_cache, 'add_node', autospec=True) +@mock.patch.object(utils, 'get_client', autospec=True) +class TestNodeCreate(test_base.NodeTest): + def setUp(self): + super(TestNodeCreate, self).setUp() + self.mock_client = mock.Mock() + + def test_default_create(self, mock_get_client, mock_add_node): + mock_get_client.return_value = self.mock_client + self.mock_client.node.create.return_value = self.node + + node_cache.create_node('fake') + + self.mock_client.node.create.assert_called_once_with(driver='fake') + mock_add_node.assert_called_once_with(self.node.uuid, + ironic=self.mock_client) + + def test_create_with_args(self, mock_get_client, mock_add_node): + mock_get_client.return_value = self.mock_client + self.mock_client.node.create.return_value = self.node + + node_cache.create_node('agent_ipmitool', ironic=self.mock_client) + + self.assertFalse(mock_get_client.called) + self.mock_client.node.create.assert_called_once_with( + driver='agent_ipmitool') + mock_add_node.assert_called_once_with(self.node.uuid, + ironic=self.mock_client) + + def test_create_client_error(self, mock_get_client, mock_add_node): + mock_get_client.return_value = self.mock_client + self.mock_client.node.create.side_effect = ( + node_cache.exceptions.InvalidAttribute) + + node_cache.create_node('fake') + + mock_get_client.assert_called_once_with() + self.mock_client.node.create.assert_called_once_with(driver='fake') + self.assertFalse(mock_add_node.called) diff --git a/ironic_inspector/test/test_plugins_discovery.py b/ironic_inspector/test/test_plugins_discovery.py new file mode 100644 index 000000000..d6f3c77c0 --- /dev/null +++ b/ironic_inspector/test/test_plugins_discovery.py @@ -0,0 +1,127 @@ +# 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 +import mock + +from ironic_inspector import node_cache +from ironic_inspector.plugins import discovery +from ironic_inspector.test import base as test_base +from ironic_inspector import utils + + +def copy_call_args(mock_arg): + new_mock = mock.Mock() + + def side_effect(*args, **kwargs): + args = copy.deepcopy(args) + kwargs = copy.deepcopy(kwargs) + new_mock(*args, **kwargs) + return mock.DEFAULT + mock_arg.side_effect = side_effect + return new_mock + + +class TestEnrollNodeNotFoundHook(test_base.NodeTest): + def setUp(self): + super(TestEnrollNodeNotFoundHook, self).setUp() + self.ironic = mock.MagicMock() + + @mock.patch.object(node_cache, 'create_node', autospec=True) + @mock.patch.object(utils, 'get_client', autospec=True) + @mock.patch.object(discovery, '_check_existing_nodes', autospec=True) + def test_enroll_default(self, mock_check_existing, mock_client, + mock_create_node): + mock_client.return_value = self.ironic + introspection_data = {'test': 'test'} + + discovery.enroll_node_not_found_hook(introspection_data) + + mock_create_node.assert_called_once_with('fake', ironic=self.ironic, + driver_info={}) + mock_check_existing.assert_called_once_with( + introspection_data, {}, self.ironic) + + @mock.patch.object(node_cache, 'create_node', autospec=True) + @mock.patch.object(utils, 'get_client', autospec=True) + @mock.patch.object(discovery, '_check_existing_nodes', autospec=True) + def test_enroll_with_ipmi_address(self, mock_check_existing, mock_client, + mock_create_node): + mock_client.return_value = self.ironic + introspection_data = {'ipmi_address': '1.2.3.4'} + expected_data = introspection_data.copy() + mock_check_existing = copy_call_args(mock_check_existing) + + discovery.enroll_node_not_found_hook(introspection_data) + + mock_create_node.assert_called_once_with( + 'fake', ironic=self.ironic, + driver_info={'ipmi_address': '1.2.3.4'}) + mock_check_existing.assert_called_once_with( + expected_data, {'ipmi_address': '1.2.3.4'}, self.ironic) + self.assertEqual({'ipmi_address': '1.2.3.4', 'auto_discovered': True}, + introspection_data) + + @mock.patch.object(node_cache, 'create_node', autospec=True) + @mock.patch.object(utils, 'get_client', autospec=True) + @mock.patch.object(discovery, '_check_existing_nodes', autospec=True) + def test_enroll_with_non_default_driver(self, mock_check_existing, + mock_client, mock_create_node): + mock_client.return_value = self.ironic + discovery.CONF.set_override('enroll_node_driver', 'fake2', + 'discovery') + mock_check_existing = copy_call_args(mock_check_existing) + introspection_data = {} + + discovery.enroll_node_not_found_hook(introspection_data) + + mock_create_node.assert_called_once_with('fake2', ironic=self.ironic, + driver_info={}) + mock_check_existing.assert_called_once_with( + {}, {}, self.ironic) + self.assertEqual({'auto_discovered': True}, introspection_data) + + def test__check_existing_nodes_new_mac(self): + self.ironic.port.list.return_value = [] + introspection_data = {'macs': self.macs} + node_driver_info = {} + + discovery._check_existing_nodes( + introspection_data, node_driver_info, self.ironic) + + def test__check_existing_nodes_existing_mac(self): + self.ironic.port.list.return_value = mock.MagicMock( + address=self.macs[0], uuid='fake_port') + introspection_data = {'macs': self.macs} + node_driver_info = {} + + self.assertRaises(utils.Error, + discovery._check_existing_nodes, + introspection_data, node_driver_info, self.ironic) + + def test__check_existing_nodes_new_node(self): + self.ironic.node.list.return_value = [mock.MagicMock( + driver_info={'ipmi_address': '1.2.4.3'}, uuid='fake_node')] + introspection_data = {} + node_driver_info = {'ipmi_address': self.bmc_address} + + discovery._check_existing_nodes(introspection_data, node_driver_info, + self.ironic) + + def test__check_existing_nodes_existing_node(self): + self.ironic.node.list.return_value = [mock.MagicMock( + driver_info={'ipmi_address': self.bmc_address}, uuid='fake_node')] + introspection_data = {} + node_driver_info = {'ipmi_address': self.bmc_address} + + self.assertRaises(utils.Error, discovery._check_existing_nodes, + introspection_data, node_driver_info, self.ironic) diff --git a/ironic_inspector/test/test_utils.py b/ironic_inspector/test/test_utils.py index c7bc9a90b..3ffe705d1 100644 --- a/ironic_inspector/test/test_utils.py +++ b/ironic_inspector/test/test_utils.py @@ -46,7 +46,7 @@ class TestCheckAuth(base.BaseTest): utils.get_client(fake_token) args = {'os_auth_token': fake_token, 'ironic_url': fake_ironic_url, - 'os_ironic_api_version': '1.6', + 'os_ironic_api_version': '1.11', 'max_retries': CONF.ironic.max_retries, 'retry_interval': CONF.ironic.retry_interval} mock_client.assert_called_once_with(1, **args) @@ -60,7 +60,7 @@ class TestCheckAuth(base.BaseTest): 'os_tenant_name': CONF.ironic.os_tenant_name, 'os_endpoint_type': CONF.ironic.os_endpoint_type, 'os_service_type': CONF.ironic.os_service_type, - 'os_ironic_api_version': '1.6', + 'os_ironic_api_version': '1.11', 'max_retries': CONF.ironic.max_retries, 'retry_interval': CONF.ironic.retry_interval} mock_client.assert_called_once_with(1, **args) diff --git a/ironic_inspector/utils.py b/ironic_inspector/utils.py index 2deee8760..75af8c5f9 100644 --- a/ironic_inspector/utils.py +++ b/ironic_inspector/utils.py @@ -34,8 +34,8 @@ SET_CREDENTIALS_VALID_STATES = {'enroll'} GREEN_POOL = None -# 1.6 is a Kilo API version, which has all we need and is pretty well tested -DEFAULT_IRONIC_API_VERSION = '1.6' +# 1.11 is API version, which support 'enroll' state +DEFAULT_IRONIC_API_VERSION = '1.11' def get_ipmi_address(node): diff --git a/releasenotes/notes/enroll-hook-d8c32eba70848210.yaml b/releasenotes/notes/enroll-hook-d8c32eba70848210.yaml new file mode 100644 index 000000000..8bd28cd57 --- /dev/null +++ b/releasenotes/notes/enroll-hook-d8c32eba70848210.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - Switch required Ironic API version to '1.11', which supports 'enroll' state. +features: + - Add a new node_not_found hook - enroll, which allows automatically discover + Ironic's node. diff --git a/setup.cfg b/setup.cfg index 8a20f0455..8130786cc 100644 --- a/setup.cfg +++ b/setup.cfg @@ -35,6 +35,7 @@ ironic_inspector.hooks.processing = root_device_hint = ironic_inspector.plugins.raid_device:RaidDeviceDetection ironic_inspector.hooks.node_not_found = example = ironic_inspector.plugins.example:example_not_found_hook + enroll = ironic_inspector.plugins.discovery:enroll_node_not_found_hook ironic_inspector.rules.conditions = eq = ironic_inspector.plugins.rules:EqCondition lt = ironic_inspector.plugins.rules:LtCondition @@ -54,6 +55,7 @@ ironic_inspector.rules.actions = oslo.config.opts = ironic_inspector = ironic_inspector.conf:list_opts ironic_inspector.common.swift = ironic_inspector.common.swift:list_opts + ironic_inspector.plugins.discovery = ironic_inspector.plugins.discovery:list_opts [compile_catalog] directory = ironic_inspector/locale diff --git a/tox.ini b/tox.ini index 7e8fc0049..c67ece267 100644 --- a/tox.ini +++ b/tox.ini @@ -48,6 +48,7 @@ commands = --namespace ironic_inspector \ --namespace keystonemiddleware.auth_token \ --namespace ironic_inspector.common.swift \ + --namespace ironic_inspector.plugins.discovery \ --namespace oslo.db \ --namespace oslo.log