From c1a8606095d4021c0f484d2bfa86fc15602503eb Mon Sep 17 00:00:00 2001 From: Zhenguo Niu Date: Wed, 21 Oct 2015 18:04:35 +0800 Subject: [PATCH] Add db api layer for CRUD operations on node tags - set/unset node tags - get node tags - add/delete single tag - check whether node tag exists - Delete all tags attached to the node when it's destroyed This will not support creating node with tags, just ignore the specified tags. Change-Id: Ibe83a726d904fd33b8550c8bec134cf05644560e Partial-bug: #1526266 --- ironic/common/exception.py | 4 + ironic/db/api.py | 61 ++++++++++++++ ironic/db/sqlalchemy/api.py | 68 +++++++++++++++ ironic/tests/unit/api/utils.py | 1 + ironic/tests/unit/db/test_node_tags.py | 111 +++++++++++++++++++++++++ ironic/tests/unit/db/test_nodes.py | 24 ++++++ ironic/tests/unit/db/utils.py | 24 ++++++ 7 files changed, 293 insertions(+) create mode 100644 ironic/tests/unit/db/test_node_tags.py diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 24b557ae9d..38079f5459 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -601,3 +601,7 @@ class CIMCException(IronicException): class OneViewError(IronicException): _msg_fmt = _("OneView exception occurred. Error: %(error)s") + + +class NodeTagNotFound(IronicException): + _msg_fmt = _("Node %(node_id)s doesn't have a tag '%(tag)s'") diff --git a/ironic/db/api.py b/ironic/db/api.py index e90010d3e1..ca27153453 100644 --- a/ironic/db/api.py +++ b/ironic/db/api.py @@ -543,3 +543,64 @@ class Connection(object): :param node_id: The id of a node. :raises: NodeNotFound """ + + @abc.abstractmethod + def set_node_tags(self, node_id, tags): + """Replace all of the node tags with specified list of tags. + + This ignores duplicate tags in the specified list. + + :param node_id: The id of a node. + :param tags: List of tags. + :returns: A list of NodeTag objects. + :raises: NodeNotFound if the node is not found. + """ + + @abc.abstractmethod + def unset_node_tags(self, node_id): + """Remove all tags of the node. + + :param node_id: The id of a node. + :raises: NodeNotFound if the node is not found. + """ + + @abc.abstractmethod + def get_node_tags_by_node_id(self, node_id): + """Get node tags based on its id. + + :param node_id: The id of a node. + :returns: A list of NodeTag objects. + :raises: NodeNotFound if the node is not found. + """ + + @abc.abstractmethod + def add_node_tag(self, node_id, tag): + """Add tag to the node. + + If the node_id and tag pair already exists, this should still + succeed. + + :param node_id: The id of a node. + :param tag: A tag string. + :returns: the NodeTag object. + :raises: NodeNotFound if the node is not found. + """ + + @abc.abstractmethod + def delete_node_tag(self, node_id, tag): + """Delete specified tag from the node. + + :param node_id: The id of a node. + :param tag: A tag string. + :raises: NodeNotFound if the node is not found. + :raises: NodeTagNotFound if the tag is not found. + """ + + @abc.abstractmethod + def node_tag_exists(self, node_id, tag): + """Check if the specified tag exist on the node. + + :param node_id: The id of a node. + :param tag: A tag string. + :returns: True if the tag exists otherwise False. + """ diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index d84d6a2a56..5e25c0d3c0 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -289,6 +289,14 @@ class Connection(api.Connection): if 'provision_state' not in values: values['provision_state'] = states.ENROLL + # TODO(zhenguo): Support creating node with tags + if 'tags' in values: + LOG.warning( + _LW('Ignore the specified tags %(tags)s when creating node: ' + '%(node)s.'), {'tags': values['tags'], + 'node': values['uuid']}) + del values['tags'] + node = models.Node() node.update(values) with _session_for_write() as session: @@ -364,6 +372,10 @@ class Connection(api.Connection): node_id) portgroup_query.delete() + # Delete all tags attached to the node + tag_query = model_query(models.NodeTag).filter_by(node_id=node_id) + tag_query.delete() + query.delete() def update_node(self, node_id, values): @@ -763,3 +775,59 @@ class Connection(api.Connection): count = query.update({'provision_updated_at': timeutils.utcnow()}) if count == 0: raise exception.NodeNotFound(node_id) + + def _check_node_exists(self, node_id): + if not model_query(models.Node).filter_by(id=node_id).scalar(): + raise exception.NodeNotFound(node=node_id) + + def set_node_tags(self, node_id, tags): + # remove duplicate tags + tags = set(tags) + with _session_for_write() as session: + self.unset_node_tags(node_id) + node_tags = [] + for tag in tags: + node_tag = models.NodeTag(tag=tag, node_id=node_id) + session.add(node_tag) + node_tags.append(node_tag) + + return node_tags + + def unset_node_tags(self, node_id): + self._check_node_exists(node_id) + with _session_for_write(): + model_query(models.NodeTag).filter_by(node_id=node_id).delete() + + def get_node_tags_by_node_id(self, node_id): + self._check_node_exists(node_id) + result = (model_query(models.NodeTag) + .filter_by(node_id=node_id) + .all()) + return result + + def add_node_tag(self, node_id, tag): + node_tag = models.NodeTag(tag=tag, node_id=node_id) + + self._check_node_exists(node_id) + try: + with _session_for_write() as session: + session.add(node_tag) + session.flush() + except db_exc.DBDuplicateEntry: + # NOTE(zhenguo): ignore tags duplicates + pass + + return node_tag + + def delete_node_tag(self, node_id, tag): + self._check_node_exists(node_id) + with _session_for_write(): + result = model_query(models.NodeTag).filter_by( + node_id=node_id, tag=tag).delete() + + if not result: + raise exception.NodeTagNotFound(node_id=node_id, tag=tag) + + def node_tag_exists(self, node_id, tag): + q = model_query(models.NodeTag).filter_by(node_id=node_id, tag=tag) + return model_query(q.exists()).scalar() diff --git a/ironic/tests/unit/api/utils.py b/ironic/tests/unit/api/utils.py index d44db2098c..369a163074 100644 --- a/ironic/tests/unit/api/utils.py +++ b/ironic/tests/unit/api/utils.py @@ -95,6 +95,7 @@ def node_post_data(**kw): node.pop('chassis_id') node.pop('target_raid_config') node.pop('raid_config') + node.pop('tags') internal = node_controller.NodePatchType.internal_attrs() return remove_internal(node, internal) diff --git a/ironic/tests/unit/db/test_node_tags.py b/ironic/tests/unit/db/test_node_tags.py new file mode 100644 index 0000000000..c4388cd2f7 --- /dev/null +++ b/ironic/tests/unit/db/test_node_tags.py @@ -0,0 +1,111 @@ +# 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. + +"""Tests for manipulating NodeTags via the DB API""" + +from ironic.common import exception + +from ironic.tests.unit.db import base +from ironic.tests.unit.db import utils as db_utils + + +class DbNodeTagTestCase(base.DbTestCase): + + def setUp(self): + super(DbNodeTagTestCase, self).setUp() + self.node = db_utils.create_test_node() + + def test_set_node_tags(self): + tags = self.dbapi.set_node_tags(self.node.id, ['tag1', 'tag2']) + self.assertEqual(self.node.id, tags[0].node_id) + self.assertItemsEqual(['tag1', 'tag2'], [tag.tag for tag in tags]) + + tags = self.dbapi.set_node_tags(self.node.id, []) + self.assertEqual([], tags) + + def test_set_node_tags_duplicate(self): + tags = self.dbapi.set_node_tags(self.node.id, + ['tag1', 'tag2', 'tag2']) + self.assertEqual(self.node.id, tags[0].node_id) + self.assertItemsEqual(['tag1', 'tag2'], [tag.tag for tag in tags]) + + def test_set_node_tags_node_not_exist(self): + self.assertRaises(exception.NodeNotFound, + self.dbapi.set_node_tags, '1234', ['tag1', 'tag2']) + + def test_get_node_tags_by_node_id(self): + self.dbapi.set_node_tags(self.node.id, ['tag1', 'tag2']) + tags = self.dbapi.get_node_tags_by_node_id(self.node.id) + self.assertEqual(self.node.id, tags[0].node_id) + self.assertItemsEqual(['tag1', 'tag2'], [tag.tag for tag in tags]) + + def test_get_node_tags_empty(self): + tags = self.dbapi.get_node_tags_by_node_id(self.node.id) + self.assertEqual([], tags) + + def test_get_node_tags_node_not_exist(self): + self.assertRaises(exception.NodeNotFound, + self.dbapi.get_node_tags_by_node_id, '123') + + def test_unset_node_tags(self): + self.dbapi.set_node_tags(self.node.id, ['tag1', 'tag2']) + self.dbapi.unset_node_tags(self.node.id) + tags = self.dbapi.get_node_tags_by_node_id(self.node.id) + self.assertEqual([], tags) + + def test_unset_empty_node_tags(self): + self.dbapi.unset_node_tags(self.node.id) + tags = self.dbapi.get_node_tags_by_node_id(self.node.id) + self.assertEqual([], tags) + + def test_unset_node_tags_node_not_exist(self): + self.assertRaises(exception.NodeNotFound, + self.dbapi.unset_node_tags, '123') + + def test_add_node_tag(self): + tag = self.dbapi.add_node_tag(self.node.id, 'tag1') + self.assertEqual(self.node.id, tag.node_id) + self.assertEqual('tag1', tag.tag) + + def test_add_node_tag_duplicate(self): + tag = self.dbapi.add_node_tag(self.node.id, 'tag1') + tag = self.dbapi.add_node_tag(self.node.id, 'tag1') + self.assertEqual(self.node.id, tag.node_id) + self.assertEqual('tag1', tag.tag) + + def test_add_node_tag_node_not_exist(self): + self.assertRaises(exception.NodeNotFound, + self.dbapi.add_node_tag, '123', 'tag1') + + def test_delete_node_tag(self): + self.dbapi.set_node_tags(self.node.id, ['tag1', 'tag2']) + self.dbapi.delete_node_tag(self.node.id, 'tag1') + tags = self.dbapi.get_node_tags_by_node_id(self.node.id) + self.assertEqual(1, len(tags)) + self.assertEqual('tag2', tags[0].tag) + + def test_delete_node_tag_not_found(self): + self.assertRaises(exception.NodeTagNotFound, + self.dbapi.delete_node_tag, self.node.id, 'tag1') + + def test_delete_node_tag_node_not_found(self): + self.assertRaises(exception.NodeNotFound, + self.dbapi.delete_node_tag, '123', 'tag1') + + def test_node_tag_exists(self): + self.dbapi.set_node_tags(self.node.id, ['tag1', 'tag2']) + ret = self.dbapi.node_tag_exists(self.node.id, 'tag1') + self.assertTrue(ret) + + def test_node_tag_not_exists(self): + ret = self.dbapi.node_tag_exists(self.node.id, 'tag1') + self.assertFalse(ret) diff --git a/ironic/tests/unit/db/test_nodes.py b/ironic/tests/unit/db/test_nodes.py index cbb4c10d63..3587024758 100644 --- a/ironic/tests/unit/db/test_nodes.py +++ b/ironic/tests/unit/db/test_nodes.py @@ -24,6 +24,7 @@ import six from ironic.common import exception from ironic.common import states +from ironic.db.sqlalchemy import api from ironic.tests.unit.db import base from ironic.tests.unit.db import utils @@ -33,6 +34,11 @@ class DbNodeTestCase(base.DbTestCase): def test_create_node(self): utils.create_test_node() + @mock.patch.object(api.LOG, 'warning', autospec=True) + def test_create_node_with_tags(self, mock_log): + utils.create_test_node(tags=['tag1', 'tag2']) + self.assertTrue(mock_log.called) + def test_create_node_already_exists(self): utils.create_test_node() self.assertRaises(exception.NodeAlreadyExists, @@ -322,6 +328,24 @@ class DbNodeTestCase(base.DbTestCase): self.assertRaises(exception.PortNotFound, self.dbapi.get_port_by_id, port.id) + def test_tags_get_destroyed_after_destroying_a_node(self): + node = utils.create_test_node() + + tag = utils.create_test_node_tag(node_id=node.id) + + self.assertTrue(self.dbapi.node_tag_exists(node.id, tag.tag)) + self.dbapi.destroy_node(node.id) + self.assertFalse(self.dbapi.node_tag_exists(node.id, tag.tag)) + + def test_tags_get_destroyed_after_destroying_a_node_by_uuid(self): + node = utils.create_test_node() + + tag = utils.create_test_node_tag(node_id=node.id) + + self.assertTrue(self.dbapi.node_tag_exists(node.id, tag.tag)) + self.dbapi.destroy_node(node.uuid) + self.assertFalse(self.dbapi.node_tag_exists(node.id, tag.tag)) + def test_update_node(self): node = utils.create_test_node() diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index 8337b8c6c5..2b1ab636e6 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -224,6 +224,7 @@ def get_test_node(**kw): 'inspection_started_at': kw.get('inspection_started_at'), 'raid_config': kw.get('raid_config'), 'target_raid_config': kw.get('target_raid_config'), + 'tags': kw.get('tags', []), } @@ -380,3 +381,26 @@ def create_test_portgroup(**kw): del portgroup['id'] dbapi = db_api.get_instance() return dbapi.create_portgroup(portgroup) + + +def get_test_node_tag(**kw): + return { + "tag": kw.get("tag", "tag1"), + "node_id": kw.get("node_id", "123"), + 'created_at': kw.get('created_at'), + 'updated_at': kw.get('updated_at'), + } + + +def create_test_node_tag(**kw): + """Create test node tag entry in DB and return NodeTag DB object. + + Function to be used to create test NodeTag objects in the database. + + :param kw: kwargs with overriding values for tag's attributes. + :returns: Test NodeTag DB object. + + """ + tag = get_test_node_tag(**kw) + dbapi = db_api.get_instance() + return dbapi.add_node_tag(tag['node_id'], tag['tag'])