diff --git a/doc/source/usage.rst b/doc/source/usage.rst
index 607e27b79..b315ab996 100644
--- a/doc/source/usage.rst
+++ b/doc/source/usage.rst
@@ -74,6 +74,16 @@ A condition is represented by an object with fields:
``field`` a `JSON path `_ to the field
in the introspection data to use in comparison.
+Starting with the Mitaka release, you can also apply conditions to ironic node
+field. Prefix field with schema (``data://`` or ``node://``) to distinguish
+between values from introspection data and node. Both schemes use JSON path::
+
+ {'field': 'node://property.path', 'op': 'eq', 'value': 'val'}
+ {'field': 'data://introspection.path', 'op': 'eq', 'value': 'val'}
+
+if scheme (node or data) is missing, condition compares data with
+introspection data.
+
``multiple`` how to treat situations where the ``field`` query returns multiple
results (e.g. the field contains a list), available options are:
@@ -110,6 +120,13 @@ Default available actions include:
value as a list and appends value to it. If optional ``unique`` parameter is
set to ``True``, nothing will be added if given value is already in a list.
+Starting from Mitaka release, ``value`` field in actions supports fetching data
+from introspection, it's using `python string formatting notation
+`_ ::
+
+ {'action': 'set-attribute', 'path': '/driver_info/ipmi_address',
+ 'value': '{data[inventory][bmc_address]}'}
+
.. _setting-ipmi-creds:
Setting IPMI Credentials
diff --git a/ironic_inspector/plugins/base.py b/ironic_inspector/plugins/base.py
index 6cc252906..c7834c52c 100644
--- a/ironic_inspector/plugins/base.py
+++ b/ironic_inspector/plugins/base.py
@@ -116,6 +116,9 @@ class RuleConditionPlugin(WithValidation): # pragma: no cover
class RuleActionPlugin(WithValidation): # pragma: no cover
"""Abstract base class for rule action plugins."""
+ FORMATTED_PARAMS = []
+ """List of params will be formatted with python format."""
+
@abc.abstractmethod
def apply(self, node_info, params, **kwargs):
"""Run action on successful rule match.
diff --git a/ironic_inspector/plugins/rules.py b/ironic_inspector/plugins/rules.py
index ae4ab781d..04ecc9a64 100644
--- a/ironic_inspector/plugins/rules.py
+++ b/ironic_inspector/plugins/rules.py
@@ -113,6 +113,8 @@ class SetAttributeAction(base.RuleActionPlugin):
REQUIRED_PARAMS = {'path', 'value'}
# TODO(dtantsur): proper validation of path
+ FORMATTED_PARAMS = ['value']
+
def apply(self, node_info, params, **kwargs):
node_info.patch([{'op': 'add', 'path': params['path'],
'value': params['value']}])
@@ -132,6 +134,8 @@ class SetCapabilityAction(base.RuleActionPlugin):
REQUIRED_PARAMS = {'name'}
OPTIONAL_PARAMS = {'value'}
+ FORMATTED_PARAMS = ['value']
+
def apply(self, node_info, params, **kwargs):
node_info.update_capabilities(
**{params['name']: params.get('value')})
@@ -145,6 +149,8 @@ class ExtendAttributeAction(base.RuleActionPlugin):
OPTIONAL_PARAMS = {'unique'}
# TODO(dtantsur): proper validation of path
+ FORMATTED_PARAMS = ['value']
+
def apply(self, node_info, params, **kwargs):
def _replace(values):
value = params['value']
diff --git a/ironic_inspector/rules.py b/ironic_inspector/rules.py
index 5b7341e04..005d8664a 100644
--- a/ironic_inspector/rules.py
+++ b/ironic_inspector/rules.py
@@ -131,7 +131,14 @@ class IntrospectionRule(object):
node_info=node_info, data=data)
ext_mgr = plugins_base.rule_conditions_manager()
for cond in self._conditions:
- field_values = jsonpath.parse(cond.field).find(data)
+ scheme, path = _parse_path(cond.field)
+
+ if scheme == 'node':
+ source_data = node_info.node().dict()
+ elif scheme == 'data':
+ source_data = data
+
+ field_values = jsonpath.parse(path).find(source_data)
field_values = [x.value for x in field_values]
cond_ext = ext_mgr[cond.op].obj
@@ -172,7 +179,7 @@ class IntrospectionRule(object):
:param node_info: NodeInfo instance
:param rollback: if True, rollback actions are executed
- :param data: introspection data (only used for logging)
+ :param data: introspection data
"""
if rollback:
method = 'rollback'
@@ -185,11 +192,26 @@ class IntrospectionRule(object):
ext_mgr = plugins_base.rule_actions_manager()
for act in self._actions:
+ ext = ext_mgr[act.action].obj
+ for formatted_param in ext.FORMATTED_PARAMS:
+ value = act.params.get(formatted_param)
+ if not value:
+ continue
+
+ # NOTE(aarefiev): verify provided value with introspection
+ # data format specifications.
+ # TODO(aarefiev): simple verify on import rule time.
+ try:
+ act.params[formatted_param] = value.format(data=data)
+ except KeyError as e:
+ raise utils.Error(_('Invalid formatting variable key '
+ 'provided: %s') % e,
+ node_info=node_info, data=data)
+
LOG.debug('Running %(what)s action `%(action)s %(params)s`',
{'action': act.action, 'params': act.params,
'what': method},
node_info=node_info, data=data)
- ext = ext_mgr[act.action].obj
getattr(ext, method)(node_info, act.params)
LOG.debug('Successfully applied %s',
@@ -197,6 +219,27 @@ class IntrospectionRule(object):
node_info=node_info, data=data)
+def _parse_path(path):
+ """Parse path, extract scheme and path.
+
+ Parse path with 'node' and 'data' scheme, which links on
+ introspection data and node info respectively. If scheme is
+ missing in path, default is 'data'.
+
+ :param path: data or node path
+ :return: tuple (scheme, path)
+ """
+ try:
+ index = path.index('://')
+ except ValueError:
+ scheme = 'data'
+ path = path
+ else:
+ scheme = path[:index]
+ path = path[index + 3:]
+ return scheme, path
+
+
def create(conditions_json, actions_json, uuid=None,
description=None):
"""Create a new rule in database.
@@ -235,8 +278,15 @@ def create(conditions_json, actions_json, uuid=None,
conditions = []
for cond_json in conditions_json:
field = cond_json['field']
+
+ scheme, path = _parse_path(field)
+
+ if scheme not in ('node', 'data'):
+ raise utils.Error(_('Unsupported scheme for field: %s, valid '
+ 'values are node:// or data://') % scheme)
+ # verify field as JSON path
try:
- jsonpath.parse(field)
+ jsonpath.parse(path)
except Exception as exc:
raise utils.Error(_('Unable to parse field JSON path %(field)s: '
'%(error)s') % {'field': field, 'error': exc})
@@ -364,7 +414,7 @@ def apply(node_info, data):
if to_rollback:
LOG.debug('Running rollback actions', node_info=node_info, data=data)
for rule in to_rollback:
- rule.apply_actions(node_info, rollback=True)
+ rule.apply_actions(node_info, rollback=True, data=data)
else:
LOG.debug('No rollback actions to apply',
node_info=node_info, data=data)
@@ -372,7 +422,7 @@ def apply(node_info, data):
if to_apply:
LOG.debug('Running actions', node_info=node_info, data=data)
for rule in to_apply:
- rule.apply_actions(node_info, rollback=False)
+ rule.apply_actions(node_info, rollback=False, data=data)
else:
LOG.debug('No actions to apply', node_info=node_info, data=data)
diff --git a/ironic_inspector/test/base.py b/ironic_inspector/test/base.py
index 171de4e6b..1ca14ebd6 100644
--- a/ironic_inspector/test/base.py
+++ b/ironic_inspector/test/base.py
@@ -88,15 +88,23 @@ class NodeTest(BaseTest):
self.uuid = uuidutils.generate_uuid()
self.bmc_address = '1.2.3.4'
self.macs = ['11:22:33:44:55:66', '66:55:44:33:22:11']
- self.node = mock.Mock(driver='pxe_ipmitool',
- driver_info={'ipmi_address': self.bmc_address},
- properties={'cpu_arch': 'i386', 'local_gb': 40},
- uuid=self.uuid,
- power_state='power on',
- provision_state='inspecting',
- extra={},
- instance_uuid=None,
- maintenance=False)
+ fake_node = {
+ 'driver': 'pxe_ipmitool',
+ 'driver_info': {'ipmi_address': self.bmc_address},
+ 'properties': {'cpu_arch': 'i386', 'local_gb': 40},
+ 'uuid': self.uuid,
+ 'power_state': 'power on',
+ 'provision_state': 'inspecting',
+ 'extra': {},
+ 'instance_uuid': None,
+ 'maintenance': False
+ }
+ mock_to_dict = mock.Mock(return_value=fake_node)
+
+ self.node = mock.Mock(**fake_node)
+ self.node.dict = mock_to_dict
+
self.ports = []
self.node_info = node_cache.NodeInfo(uuid=self.uuid, started_at=0,
node=self.node, ports=self.ports)
+ self.node_info.node = mock.Mock(return_value=self.node)
diff --git a/ironic_inspector/test/functional.py b/ironic_inspector/test/functional.py
index 821bd3bcd..2536aa327 100644
--- a/ironic_inspector/test/functional.py
+++ b/ironic_inspector/test/functional.py
@@ -342,6 +342,49 @@ class Test(Base):
self.uuid,
[{'op': 'add', 'path': '/extra/foo', 'value': 'bar'}])
+ def test_conditions_scheme_actions_path(self):
+ rules = [
+ {
+ 'conditions': [
+ {'field': 'node://properties.local_gb', 'op': 'eq',
+ 'value': 40},
+ {'field': 'node://driver_info.ipmi_address', 'op': 'eq',
+ 'value': self.bmc_address},
+ ],
+ 'actions': [
+ {'action': 'set-attribute', 'path': '/extra/foo',
+ 'value': 'bar'}
+ ]
+ },
+ {
+ 'conditions': [
+ {'field': 'data://inventory.cpu.count', 'op': 'eq',
+ 'value': self.data['inventory']['cpu']['count']},
+ ],
+ 'actions': [
+ {'action': 'set-attribute',
+ 'path': '/driver_info/ipmi_address',
+ 'value': '{data[inventory][bmc_address]}'}
+ ]
+ }
+ ]
+ for rule in rules:
+ self.call_add_rule(rule)
+
+ self.call_introspect(self.uuid)
+ eventlet.greenthread.sleep(DEFAULT_SLEEP)
+ self.call_continue(self.data)
+ eventlet.greenthread.sleep(DEFAULT_SLEEP)
+
+ self.cli.node.update.assert_any_call(
+ self.uuid,
+ [{'op': 'add', 'path': '/extra/foo', 'value': 'bar'}])
+
+ self.cli.node.update.assert_any_call(
+ self.uuid,
+ [{'op': 'add', 'path': '/driver_info/ipmi_address',
+ 'value': self.data['inventory']['bmc_address']}])
+
def test_root_device_hints(self):
self.node.properties['root_device'] = {'size': 20}
diff --git a/ironic_inspector/test/test_rules.py b/ironic_inspector/test/test_rules.py
index 0c17eb35d..b954f224c 100644
--- a/ironic_inspector/test/test_rules.py
+++ b/ironic_inspector/test/test_rules.py
@@ -18,14 +18,13 @@ import mock
from oslo_utils import uuidutils
from ironic_inspector import db
-from ironic_inspector import node_cache
from ironic_inspector.plugins import base as plugins_base
from ironic_inspector import rules
from ironic_inspector.test import base as test_base
from ironic_inspector import utils
-class BaseTest(test_base.BaseTest):
+class BaseTest(test_base.NodeTest):
def setUp(self):
super(BaseTest, self).setUp()
self.uuid = uuidutils.generate_uuid()
@@ -41,7 +40,6 @@ class BaseTest(test_base.BaseTest):
'memory_mb': 1024,
'local_gb': 42,
}
- self.node_info = node_cache.NodeInfo(uuid=self.uuid, started_at=42)
class TestCreateRule(BaseTest):
@@ -321,6 +319,38 @@ class TestCheckConditionsMultiple(BaseTest):
data)
+class TestCheckConditionsSchemePath(BaseTest):
+ def test_conditions_data_path(self):
+ self.data_set = [
+ ([{'op': 'eq', 'field': 'data://memory_mb', 'value': 1024}],
+ True),
+ ([{'op': 'gt', 'field': 'data://local_gb', 'value': 42}],
+ False)
+ ]
+
+ for condition, res in self.data_set:
+ rule = rules.create(conditions_json=condition,
+ actions_json=self.actions_json)
+ self.assertIs(res,
+ rule.check_conditions(self.node_info, self.data),
+ self.data)
+
+ def test_conditions_node_path(self):
+ self.node_set = [
+ ([{'op': 'eq', 'field': 'node://driver_info.ipmi_address',
+ 'value': self.bmc_address}],
+ True),
+ ([{'op': 'eq', 'field': 'node://driver', 'value': 'fake'}],
+ False)
+ ]
+
+ for condition, res in self.node_set:
+ rule = rules.create(conditions_json=condition,
+ actions_json=self.actions_json)
+ self.assertIs(res,
+ rule.check_conditions(self.node_info, self.data))
+
+
@mock.patch.object(plugins_base, 'rule_actions_manager', autospec=True)
class TestApplyActions(BaseTest):
def setUp(self):
@@ -330,12 +360,13 @@ class TestApplyActions(BaseTest):
self.rule = rules.create(conditions_json=self.conditions_json,
actions_json=self.actions_json)
self.act_mock = mock.Mock(spec=plugins_base.RuleActionPlugin)
+ self.act_mock.FORMATTED_PARAMS = ['value']
self.ext_mock = mock.Mock(spec=['obj'], obj=self.act_mock)
def test_apply(self, mock_ext_mgr):
mock_ext_mgr.return_value.__getitem__.return_value = self.ext_mock
- self.rule.apply_actions(self.node_info)
+ self.rule.apply_actions(self.node_info, data=self.data)
self.act_mock.apply.assert_any_call(self.node_info,
{'message': 'boom!'})
@@ -344,6 +375,33 @@ class TestApplyActions(BaseTest):
self.act_mock.apply.call_count)
self.assertFalse(self.act_mock.rollback.called)
+ def test_apply_data_format_value(self, mock_ext_mgr):
+ self.rule = rules.create(actions_json=[
+ {'action': 'set-attribute',
+ 'path': '/driver_info/ipmi_address',
+ 'value': '{data[memory_mb]}'}],
+ conditions_json=self.conditions_json
+ )
+ mock_ext_mgr.return_value.__getitem__.return_value = self.ext_mock
+
+ self.rule.apply_actions(self.node_info, data=self.data)
+
+ self.assertEqual(1, self.act_mock.apply.call_count)
+ self.assertFalse(self.act_mock.rollback.called)
+
+ def test_apply_data_format_value_fail(self, mock_ext_mgr):
+ self.rule = rules.create(
+ actions_json=[
+ {'action': 'set-attribute',
+ 'path': '/driver_info/ipmi_address',
+ 'value': '{data[inventory][bmc_address]}'}],
+ conditions_json=self.conditions_json
+ )
+ mock_ext_mgr.return_value.__getitem__.return_value = self.ext_mock
+
+ self.assertRaises(utils.Error, self.rule.apply_actions,
+ self.node_info, data=self.data)
+
def test_rollback(self, mock_ext_mgr):
mock_ext_mgr.return_value.__getitem__.return_value = self.ext_mock
@@ -380,7 +438,7 @@ class TestApply(BaseTest):
rule.check_conditions.assert_called_once_with(self.node_info,
self.data)
rule.apply_actions.assert_called_once_with(
- self.node_info, rollback=bool(idx))
+ self.node_info, rollback=bool(idx), data=self.data)
def test_actions(self, mock_get_all):
mock_get_all.return_value = self.rules
@@ -393,7 +451,7 @@ class TestApply(BaseTest):
rule.check_conditions.assert_called_once_with(self.node_info,
self.data)
rule.apply_actions.assert_called_once_with(
- self.node_info, rollback=bool(idx))
+ self.node_info, rollback=bool(idx), data=self.data)
def test_no_rollback(self, mock_get_all):
mock_get_all.return_value = self.rules
@@ -406,7 +464,7 @@ class TestApply(BaseTest):
rule.check_conditions.assert_called_once_with(self.node_info,
self.data)
rule.apply_actions.assert_called_once_with(
- self.node_info, rollback=False)
+ self.node_info, rollback=False, data=self.data)
def test_only_rollback(self, mock_get_all):
mock_get_all.return_value = self.rules
@@ -419,4 +477,4 @@ class TestApply(BaseTest):
rule.check_conditions.assert_called_once_with(self.node_info,
self.data)
rule.apply_actions.assert_called_once_with(
- self.node_info, rollback=True)
+ self.node_info, rollback=True, data=self.data)
diff --git a/releasenotes/notes/extend-rules-9a9d38701e970611.yaml b/releasenotes/notes/extend-rules-9a9d38701e970611.yaml
new file mode 100644
index 000000000..34fe63638
--- /dev/null
+++ b/releasenotes/notes/extend-rules-9a9d38701e970611.yaml
@@ -0,0 +1,5 @@
+---
+features:
+ - Conditions now support comparing fields from node info;
+ - Actions support formatting to fetch values from introspection data.
+ See http://docs.openstack.org/developer/ironic-inspector/usage.html#introspection-rules
\ No newline at end of file