openstack/ironic-inspector
Code Issues Proposed changes

Added scope to introspection rules.

Browse Source 
Added 'scope' property to IntrospectionRule and logic to check if a node
falls in the same scope.This allows introspection rules to be applied on
selected nodes instead of every one of them.

Story: 2006995
Task: 37763

Change-Id: I77034f032ea0ec16886afdd928546eb801f7a90a
This commit is contained in:
Zygimantas Matonis 2020-02-12 16:06:20 +01:00
parent e6ece052ec
commit e2c8f9fd7b
14 changed files with 214 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
api-ref/source/introspection-api-v1-rules.inc
View File

@ -33,6 +33,7 @@ Request
- conditions: conditions
- actions: actions
- description: description
- scope: scope
**Example creating rule request:**
@ -54,6 +55,7 @@ section may contain additional default fields, like ``invert``,
- conditions: conditions
- actions: actions
- description: description
- scope: scope
**Example JSON representation:**
@ -77,6 +79,7 @@ Response
- uuid: uuid
- description: description
- scope: scope
- links: links
**Example JSON representation:**
@ -117,6 +120,7 @@ The response will contain full rule object:
- conditions: conditions
- actions: actions
- description: description
- scope: scope
**Example JSON representation:**

7
api-ref/source/parameters.yaml
View File

@ -214,6 +214,13 @@ root_disk:
in: body
required: true
type: string
scope:
description: |
Scope of an introspection rule. If set, the rule is only applied to nodes
that have matching ``inspection_scope`` property.
in: body
required: false
type: string
started_at:
description: |
UTC ISO8601 timestamp of introspection start.

3
api-ref/source/samples/api-v1-create-rule-request.json
View File

@ -22,5 +22,6 @@
"op":"is-empty",
"field":"node://driver_info.deploy_kernel"
}
]
],
"scope":"Delivery_1"
}

3
api-ref/source/samples/api-v1-create-rule-response.json
View File

@ -32,5 +32,6 @@
"rel": "self"
}
],
"uuid": "7459bf7c-9ff9-43a8-ba9f-48542ecda66c"
"uuid": "7459bf7c-9ff9-43a8-ba9f-48542ecda66c",
"scope": ""
}

1
doc/source/user/http-api.rst
View File

@ -402,3 +402,4 @@ Version History
* **1.14** allows formatting to be applied to strings nested in dicts and lists
in the actions of introspection rules.
* **1.15** allows reapply with provided introspection data from request.
* **1.16** adds ``scope`` field to introspection rule.

26
doc/source/user/usage.rst
View File

@ -103,6 +103,32 @@ results (e.g. the field contains a list), available options are:
All other fields are passed to the condition plugin, e.g. numeric comparison
operations require a ``value`` field to compare against.
Scope
^^^^^
By default, introspection rules are applied to all nodes being inspected.
In order for the rule to be applied only to specific nodes, a matching scope
variable must be set to both the rule and the node. To set the scope for a
rule include field ``"scope"`` in JSON file before importing. For example::
cat <json file>
{
"description": "...",
"actions": [...],
"conditions": [...],
"scope": "SCOPE"
}
Set the property ``inspection_scope`` on the node you want the rule to be
applied to::
openstack baremetal node set --property inspection_scope="SCOPE" <node>
Now, when inspecting, the rule will be applied only to nodes with matching
scope value. It will also ignore nodes that do not have ``inspection_scope``
property set. Note that if a rule has no scope set, it will be applied to all
nodes, regardless if they have ``inspection_scope`` set or not.
Actions
^^^^^^^

1
ironic_inspector/db.py
View File

@ -91,6 +91,7 @@ class Rule(Base):
description = Column(Text)
# NOTE(dtantsur): in the future we might need to temporary disable a rule
disabled = Column(Boolean, default=False)
scope = Column(String(255), nullable=True)
conditions = orm.relationship('RuleCondition', lazy='joined',
order_by='RuleCondition.id',

9
ironic_inspector/main.py
View File

@ -41,7 +41,7 @@ _app = flask.Flask(__name__)
LOG = utils.getProcessingLogger(__name__)
MINIMUM_API_VERSION = (1, 0)
CURRENT_API_VERSION = (1, 15)
CURRENT_API_VERSION = (1, 16)
DEFAULT_API_VERSION = CURRENT_API_VERSION
_LOGGING_EXCLUDED_KEYS = ('logs',)
@ -413,11 +413,16 @@ def api_rules():
body = flask.request.get_json(force=True)
if body.get('uuid') and not uuidutils.is_uuid_like(body['uuid']):
raise utils.Error(_('Invalid UUID value'), code=400)
if body.get('scope') and len(body.get('scope')) > 255:
raise utils.Error(
_("Invalid scope: the length of the scope should be within "
"255 characters"), code=400)
rule = rules.create(conditions_json=body.get('conditions', []),
actions_json=body.get('actions', []),
uuid=body.get('uuid'),
description=body.get('description'))
description=body.get('description'),
scope=body.get('scope'))
response_code = (200 if _get_version() < (1, 6) else 201)
return flask.make_response(

33
ironic_inspector/migrations/versions/b55109d5063a_added_scope_column_to_rules_table.py Normal file
View File

@ -0,0 +1,33 @@
# 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.
"""Added 'scope' column to 'Rules' table
Revision ID: b55109d5063a
Revises: bf8dec16023c
Create Date: 2019-12-11 14:15:57.510289
"""
from alembic import op
import sqlalchemy as sa
# revision identifiers, used by Alembic.
revision = 'b55109d5063a'
down_revision = 'bf8dec16023c'
branch_labels = None
depends_on = None
def upgrade():
op.add_column('rules', sa.Column('scope', sa.String(255),
nullable=True, default=None))

45
ironic_inspector/rules.py
View File

@ -101,17 +101,19 @@ def actions_schema():
class IntrospectionRule(object):
"""High-level class representing an introspection rule."""
def __init__(self, uuid, conditions, actions, description):
def __init__(self, uuid, conditions, actions, description, scope=None):
"""Create rule object from database data."""
self._uuid = uuid
self._conditions = conditions
self._actions = actions
self._description = description
self._scope = scope
def as_dict(self, short=False):
result = {
'uuid': self._uuid,
'description': self._description,
'scope': self._scope
}
if not short:
@ -211,6 +213,17 @@ class IntrospectionRule(object):
LOG.debug('Successfully applied actions',
node_info=node_info, data=data)
def check_scope(self, node_info):
"""Check if node's scope falls under rule._scope and rule is applicable
:param node_info: a NodeInfo object
:returns: True if conditions match, otherwise False
"""
if not self._scope:
return True
return self._scope == \
node_info.node().properties.get('inspection_scope')
def _format_value(value, data):
"""Apply parameter formatting to a value.
@ -338,7 +351,7 @@ def _validate_actions(actions_json):
def create(conditions_json, actions_json, uuid=None,
description=None):
description=None, scope=None):
"""Create a new rule in database.
:param conditions_json: list of dicts with the following keys:
@ -350,13 +363,16 @@ def create(conditions_json, actions_json, uuid=None,
Other keys are stored as is.
:param uuid: rule UUID, will be generated if empty
:param description: human-readable rule description
:param scope: if scope on node and rule matches, rule applies;
if its empty, rule applies to all nodes.
:returns: new IntrospectionRule object
:raises: utils.Error on failure
"""
uuid = uuid or uuidutils.generate_uuid()
LOG.debug('Creating rule %(uuid)s with description "%(descr)s", '
'conditions %(conditions)s and actions %(actions)s',
{'uuid': uuid, 'descr': description,
'conditions %(conditions)s, scope "%(scope)s"'
' and actions %(actions)s',
{'uuid': uuid, 'descr': description, 'scope': scope,
'conditions': conditions_json, 'actions': actions_json})
conditions = _validate_conditions(conditions_json)
@ -364,8 +380,8 @@ def create(conditions_json, actions_json, uuid=None,
try:
with db.ensure_transaction() as session:
rule = db.Rule(uuid=uuid, description=description,
disabled=False, created_at=timeutils.utcnow())
rule = db.Rule(uuid=uuid, description=description, disabled=False,
created_at=timeutils.utcnow(), scope=scope)
for field, op, multiple, invert, params in conditions:
rule.conditions.append(db.RuleCondition(op=op,
@ -385,12 +401,14 @@ def create(conditions_json, actions_json, uuid=None,
raise utils.Error(_('Rule with UUID %s already exists') % uuid,
code=409)
LOG.info('Created rule %(uuid)s with description "%(descr)s"',
{'uuid': uuid, 'descr': description})
LOG.info('Created rule %(uuid)s with description "%(descr)s" '
'and scope "%(scope)s"',
{'uuid': uuid, 'descr': description, 'scope': scope})
return IntrospectionRule(uuid=uuid,
conditions=rule.conditions,
actions=rule.actions,
description=description)
description=description,
scope=rule.scope)
def get(uuid):
@ -402,7 +420,8 @@ def get(uuid):
return IntrospectionRule(uuid=rule.uuid, actions=rule.actions,
conditions=rule.conditions,
description=rule.description)
description=rule.description,
scope=rule.scope)
def get_all():
@ -410,7 +429,8 @@ def get_all():
query = db.model_query(db.Rule).order_by(db.Rule.created_at)
return [IntrospectionRule(uuid=rule.uuid, actions=rule.actions,
conditions=rule.conditions,
description=rule.description)
description=rule.description,
scope=rule.scope)
for rule in query]
@ -452,7 +472,8 @@ def apply(node_info, data):
to_apply = []
for rule in rules:
if rule.check_conditions(node_info, data):
if (rule.check_scope(node_info) and
rule.check_conditions(node_info, data)):
to_apply.append(rule)
if to_apply:

8
ironic_inspector/test/functional.py
View File

@ -393,7 +393,8 @@ class Test(Base):
{'op': 'eq', 'field': 'memory_mb', 'value': 1024},
],
'actions': [{'action': 'fail', 'message': 'boom'}],
'description': 'Cool actions'
'description': 'Cool actions',
'scope': "sniper's scope"
}
res = self.call_add_rule(rule)
@ -411,7 +412,8 @@ class Test(Base):
res = self.call_list_rules()
self.assertEqual(rule['links'], res[0].pop('links'))
self.assertEqual([{'uuid': rule['uuid'],
'description': 'Cool actions'}],
'description': rule['description'],
'scope': rule['scope']}],
res)
res = self.call_get_rule(rule['uuid'])
@ -466,7 +468,7 @@ class Test(Base):
'value': 'foo'},
{'action': 'fail', 'message': 'boom'}
]
}
},
]
for rule in rules:
self.call_add_rule(rule)

6
ironic_inspector/test/unit/test_main.py
View File

@ -465,7 +465,8 @@ class TestApiRules(BaseAPITest):
create_mock.assert_called_once_with(conditions_json='cond',
actions_json='act',
uuid=self.uuid,
description=None)
description=None,
scope=None)
self.assertEqual(exp, json.loads(res.data.decode('utf-8')))
@mock.patch.object(rules, 'create', autospec=True)
@ -487,7 +488,8 @@ class TestApiRules(BaseAPITest):
create_mock.assert_called_once_with(conditions_json='cond',
actions_json='act',
uuid=self.uuid,
description=None)
description=None,
scope=None)
self.assertEqual(exp, json.loads(res.data.decode('utf-8')))
@mock.patch.object(rules, 'create', autospec=True)

83
ironic_inspector/test/unit/test_rules.py
View File

@ -40,6 +40,8 @@ class BaseTest(test_base.NodeTest):
'local_gb': 42,
}
self.scope = "inner circle"
@staticmethod
def condition_defaults(condition):
condition = condition.copy()
@ -56,7 +58,8 @@ class TestCreateRule(BaseTest):
self.assertTrue(rule_json.pop('uuid'))
self.assertEqual({'description': None,
'conditions': [],
'actions': self.actions_json},
'actions': self.actions_json,
'scope': None},
rule_json)
def test_create_action_none_value(self):
@ -68,7 +71,8 @@ class TestCreateRule(BaseTest):
self.assertTrue(rule_json.pop('uuid'))
self.assertEqual({'description': None,
'conditions': [],
'actions': self.actions_json},
'actions': self.actions_json,
'scope': None},
rule_json)
def test_duplicate_uuid(self):
@ -94,7 +98,8 @@ class TestCreateRule(BaseTest):
self.assertEqual({'description': None,
'conditions': [BaseTest.condition_defaults(cond)
for cond in self.conditions_json],
'actions': self.actions_json},
'actions': self.actions_json,
'scope': None},
rule_json)
def test_invalid_condition(self):
@ -157,6 +162,17 @@ class TestCreateRule(BaseTest):
rules.create,
self.conditions_json, self.actions_json)
def test_scope(self):
rule = rules.create([], self.actions_json, scope=self.scope)
rule_json = rule.as_dict()
self.assertTrue(rule_json.pop('uuid'))
self.assertEqual({'description': None,
'conditions': [],
'actions': self.actions_json,
'scope': self.scope},
rule_json)
class TestGetRule(BaseTest):
def setUp(self):
@ -170,7 +186,8 @@ class TestGetRule(BaseTest):
self.assertEqual({'description': None,
'conditions': [BaseTest.condition_defaults(cond)
for cond in self.conditions_json],
'actions': self.actions_json},
'actions': self.actions_json,
'scope': None},
rule_json)
def test_not_found(self):
@ -558,3 +575,61 @@ class TestApply(BaseTest):
self.node_info, data=self.data)
else:
self.assertFalse(rule.apply_actions.called)
@mock.patch.object(rules, 'get_all', autospec=True)
class TestRuleScope(BaseTest):
"""Test that rules are only applied on the nodes that fall in their scope.
Check that:
- global rule is applied to all nodes
- different rules with scopes are applied to different nodes
- rule without matching scope is not applied
"""
def setUp(self):
super(TestRuleScope, self).setUp()
"""
rule_global
rule_scope_1
rule_scope_2
rule_out_scope
"""
self.rules = [rules.IntrospectionRule("", "", "", "", None),
rules.IntrospectionRule("", "", "", "", "scope_1"),
rules.IntrospectionRule("", "", "", "", "scope_2"),
rules.IntrospectionRule("", "", "", "", "scope_3")]
for r in self.rules:
r.check_conditions = mock.Mock()
r.check_conditions.return_value = True
r.apply_actions = mock.Mock()
r.apply_actions.return_value = True
def test_node_no_scope(self, mock_get_all):
mock_get_all.return_value = self.rules
self.node_info.node().properties['inspection_scope'] = None
rules.apply(self.node_info, self.data)
self.rules[0].apply_actions.assert_called_once() # global
self.rules[1].apply_actions.assert_not_called() # scope_1
self.rules[2].apply_actions.assert_not_called() # scope_2
self.rules[3].apply_actions.assert_not_called() # scope_3
def test_node_scope_1(self, mock_get_all):
mock_get_all.return_value = self.rules
self.node_info.node().properties['inspection_scope'] = "scope_1"
rules.apply(self.node_info, self.data)
self.rules[0].apply_actions.assert_called_once() # global
self.rules[1].apply_actions.assert_called_once() # scope_1
self.rules[2].apply_actions.assert_not_called() # scope_2
self.rules[3].apply_actions.assert_not_called() # scope_3
def test_node_scope_2(self, mock_get_all):
mock_get_all.return_value = self.rules
self.node_info.node().properties['inspection_scope'] = "scope_2"
rules.apply(self.node_info, self.data)
self.rules[0].apply_actions.assert_called_once() # global
self.rules[1].apply_actions.assert_not_called() # scope_1
self.rules[2].apply_actions.assert_called_once() # scope_2
self.rules[3].apply_actions.assert_not_called() # scope_3

10
releasenotes/notes/introspection_rules_scope-9b06c3ad4e273a52.yaml Normal file
View File

@ -0,0 +1,10 @@
---
features:
- |
Added the capability to define a scope for the inspection process.
Previously, all introspection rules were applied when inspecting
any node. There was no mechanism to apply only a selected set of
rules. This change introduces a ``scope`` field to introspection rules.
If a scope is set on an introspection rule, it will only apply to nodes
that have a matching ``inspection_scope`` property. If not set, it will
apply to all nodes.