diff --git a/magnum/common/exception.py b/magnum/common/exception.py index 431ba0dbf7..363770726c 100644 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -381,7 +381,7 @@ class ContainerAssociated(InvalidState): class ContainerAlreadyExists(Conflict): - message = _("A node with UUID %(uuid)s already exists.") + message = _("A container with UUID %(uuid)s already exists.") class PodNotFound(ResourceNotFound): diff --git a/magnum/db/sqlalchemy/api.py b/magnum/db/sqlalchemy/api.py index c6d1dbb092..cb3edb482e 100644 --- a/magnum/db/sqlalchemy/api.py +++ b/magnum/db/sqlalchemy/api.py @@ -365,27 +365,10 @@ class Connection(api.Connection): if filters is None: filters = [] - if 'associated' in filters: - if filters['associated']: - query = query.filter(models.Container.instance_uuid is not - None) - else: - query = query.filter(models.Container.instance_uuid is None) - if 'reserved' in filters: - if filters['reserved']: - query = query.filter(models.Container.reservation is not None) - else: - query = query.filter(models.Container.reservation is None) - if 'maintenance' in filters: - query = query.filter_by(maintenance=filters['maintenance']) - if 'driver' in filters: - query = query.filter_by(driver=filters['driver']) - if 'provision_state' in filters: - query = query.filter_by(provision_state=filters['provision_state']) - if 'provisioned_before' in filters: - limit = timeutils.utcnow() - datetime.timedelta( - seconds=filters['provisioned_before']) - query = query.filter(models.Container.provision_updated_at < limit) + if 'name' in filters: + query = query.filter_by(name=filters['name']) + if 'image_id' in filters: + query = query.filter_by(image_id=filters['image_id']) return query @@ -446,7 +429,9 @@ class Connection(api.Connection): with session.begin(): query = model_query(models.Container, session=session) query = add_identity_filter(query, container_id) - query.delete() + count = query.delete() + if count != 1: + raise exception.ContainerNotFound(container_id) def update_container(self, container_id, values): # NOTE(dtantsur): this can lead to very strange errors @@ -488,24 +473,13 @@ class Connection(api.Connection): if 'associated' in filters: if filters['associated']: - query = query.filter(models.Node.instance_uuid is not None) + query = query.filter(models.Node.ironic_node_id != None) else: - query = query.filter(models.Node.instance_uuid is None) - if 'reserved' in filters: - if filters['reserved']: - query = query.filter(models.Node.reservation is not None) - else: - query = query.filter(models.Node.reservation is None) - if 'maintenance' in filters: - query = query.filter_by(maintenance=filters['maintenance']) - if 'driver' in filters: - query = query.filter_by(driver=filters['driver']) - if 'provision_state' in filters: - query = query.filter_by(provision_state=filters['provision_state']) - if 'provisioned_before' in filters: - limit = timeutils.utcnow() - datetime.timedelta( - seconds=filters['provisioned_before']) - query = query.filter(models.Node.provision_updated_at < limit) + query = query.filter(models.Node.ironic_node_id == None) + if 'type' in filters: + query = query.filter_by(type=filters['type']) + if 'image_id' in filters: + query = query.filter_by(image_id=filters['image_id']) return query @@ -540,9 +514,9 @@ class Connection(api.Connection): try: node.save() except db_exc.DBDuplicateEntry as exc: - if 'instance_uuid' in exc.columns: + if 'ironic_node_id' in exc.columns: raise exception.InstanceAssociated( - instance_uuid=values['instance_uuid'], + instance_uuid=values['ironic_node_id'], node=values['uuid']) raise exception.NodeAlreadyExists(uuid=values['uuid']) return node @@ -566,7 +540,9 @@ class Connection(api.Connection): with session.begin(): query = model_query(models.Node, session=session) query = add_identity_filter(query, node_id) - query.delete() + count = query.delete() + if count != 1: + raise exception.NodeNotFound(node_id) def update_node(self, node_id, values): # NOTE(dtantsur): this can lead to very strange errors @@ -578,7 +554,7 @@ class Connection(api.Connection): return self._do_update_node(node_id, values) except db_exc.DBDuplicateEntry: raise exception.InstanceAssociated( - instance_uuid=values['instance_uuid'], + instance_uuid=values['ironic_node_id'], node=node_id) def _do_update_node(self, node_id, values): @@ -591,13 +567,10 @@ class Connection(api.Connection): except NoResultFound: raise exception.NodeNotFound(node=node_id) - # Prevent instance_uuid overwriting - if values.get("instance_uuid") and ref.instance_uuid: + # Prevent ironic_node_id overwriting + if values.get("ironic_node_id") and ref.ironic_node_id: raise exception.NodeAssociated(node=node_id, - instance=ref.instance_uuid) - - if 'provision_state' in values: - values['provision_updated_at'] = timeutils.utcnow() + instance=ref.ironic_node_id) ref.update(values) return ref diff --git a/magnum/db/sqlalchemy/models.py b/magnum/db/sqlalchemy/models.py index b20aff70ab..de736a8f03 100644 --- a/magnum/db/sqlalchemy/models.py +++ b/magnum/db/sqlalchemy/models.py @@ -167,6 +167,8 @@ class Node(Base): __tablename__ = 'node' __table_args__ = ( schema.UniqueConstraint('uuid', name='uniq_node0uuid'), + schema.UniqueConstraint('ironic_node_id', + name='uniq_node0ironic_node_id'), table_args() ) id = Column(Integer, primary_key=True) diff --git a/magnum/tests/db/test_container.py b/magnum/tests/db/test_container.py new file mode 100644 index 0000000000..07880307ff --- /dev/null +++ b/magnum/tests/db/test_container.py @@ -0,0 +1,156 @@ +# Copyright 2015 OpenStack Foundation +# All Rights Reserved. +# +# 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 Containers via the DB API""" + +import six + +from magnum.common import exception +from magnum.common import utils as magnum_utils +from magnum.tests.db import base +from magnum.tests.db import utils + + +class DbContainerTestCase(base.DbTestCase): + + def test_create_container(self): + utils.create_test_container() + + def test_create_container_already_exists(self): + utils.create_test_container() + self.assertRaises(exception.ContainerAlreadyExists, + utils.create_test_container) + + def test_get_container_by_id(self): + container = utils.create_test_container() + res = self.dbapi.get_container_by_id(container.id) + self.assertEqual(container.id, res.id) + self.assertEqual(container.uuid, res.uuid) + + def test_get_container_by_uuid(self): + container = utils.create_test_container() + res = self.dbapi.get_container_by_uuid(container.uuid) + self.assertEqual(container.id, res.id) + self.assertEqual(container.uuid, res.uuid) + + def test_get_container_that_does_not_exist(self): + self.assertRaises(exception.ContainerNotFound, + self.dbapi.get_container_by_id, 99) + self.assertRaises(exception.ContainerNotFound, + self.dbapi.get_container_by_uuid, + magnum_utils.generate_uuid()) + + def test_get_containerinfo_list_defaults(self): + container_id_list = [] + for i in range(1, 6): + container = utils.create_test_container( + uuid=magnum_utils.generate_uuid()) + container_id_list.append(container.id) + res = [i[0] for i in self.dbapi.get_containerinfo_list()] + self.assertEqual(sorted(res), sorted(container_id_list)) + + def test_get_containerinfo_list_with_cols(self): + uuids = {} + images = {} + for i in range(1, 6): + uuid = magnum_utils.generate_uuid() + image = 'image' + str(i) + container = utils.create_test_container(image_id=image, uuid=uuid) + uuids[container.id] = uuid + images[container.id] = image + res = self.dbapi.get_containerinfo_list(columns=['id', 'image_id', + 'uuid']) + self.assertEqual(images, dict((r[0], r[1]) for r in res)) + self.assertEqual(uuids, dict((r[0], r[2]) for r in res)) + + def test_get_containerinfo_list_with_filters(self): + container1 = utils.create_test_container(name='c1', + uuid=magnum_utils.generate_uuid()) + container2 = utils.create_test_container(name='c2', + uuid=magnum_utils.generate_uuid()) + + res = self.dbapi.get_containerinfo_list(filters={'name': 'c1'}) + self.assertEqual([container1.id], [r[0] for r in res]) + + res = self.dbapi.get_containerinfo_list(filters={'name': 'c2'}) + self.assertEqual([container2.id], [r[0] for r in res]) + + res = self.dbapi.get_containerinfo_list(filters={'name': 'bad-name'}) + self.assertEqual([], [r[0] for r in res]) + + def test_get_container_list(self): + uuids = [] + for i in range(1, 6): + container = utils.create_test_container( + uuid=magnum_utils.generate_uuid()) + uuids.append(six.text_type(container['uuid'])) + res = self.dbapi.get_container_list() + res_uuids = [r.uuid for r in res] + self.assertEqual(sorted(uuids), sorted(res_uuids)) + + def test_get_container_list_with_filters(self): + container1 = utils.create_test_container(name='container-one', + uuid=magnum_utils.generate_uuid()) + container2 = utils.create_test_container(name='container-two', + uuid=magnum_utils.generate_uuid()) + + res = self.dbapi.get_container_list(filters={'name': 'container-one'}) + self.assertEqual([container1.id], [r.id for r in res]) + + res = self.dbapi.get_container_list(filters={'name': 'container-two'}) + self.assertEqual([container2.id], [r.id for r in res]) + + res = self.dbapi.get_container_list(filters={'name': 'bad-container'}) + self.assertEqual([], [r.id for r in res]) + + def test_destroy_container(self): + container = utils.create_test_container() + self.dbapi.destroy_container(container.id) + self.assertRaises(exception.ContainerNotFound, + self.dbapi.get_container_by_id, container.id) + + def test_destroy_container_by_uuid(self): + container = utils.create_test_container() + self.dbapi.destroy_container(container.uuid) + self.assertRaises(exception.ContainerNotFound, + self.dbapi.get_container_by_uuid, container.uuid) + + def test_destroy_container_that_does_not_exist(self): + self.assertRaises(exception.ContainerNotFound, + self.dbapi.destroy_container, + magnum_utils.generate_uuid()) + + def test_update_container(self): + container = utils.create_test_container() + old_image = container.image_id + new_image = 'new-image' + self.assertNotEqual(old_image, new_image) + + res = self.dbapi.update_container(container.id, + {'image_id': new_image}) + self.assertEqual(new_image, res.image_id) + + def test_update_container_not_found(self): + container_uuid = magnum_utils.generate_uuid() + new_image = 'new-image' + self.assertRaises(exception.ContainerNotFound, + self.dbapi.update_container, + container_uuid, {'image_id': new_image}) + + def test_update_container_uuid(self): + container = utils.create_test_container() + self.assertRaises(exception.InvalidParameterValue, + self.dbapi.update_container, container.id, + {'uuid': ''}) \ No newline at end of file diff --git a/magnum/tests/db/test_node.py b/magnum/tests/db/test_node.py new file mode 100644 index 0000000000..29b1e375f1 --- /dev/null +++ b/magnum/tests/db/test_node.py @@ -0,0 +1,194 @@ +# Copyright 2015 OpenStack Foundation +# All Rights Reserved. +# +# 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 Nodes via the DB API""" + +import six + +from magnum.common import exception +from magnum.common import utils as magnum_utils +from magnum.tests.db import base +from magnum.tests.db import utils + + +class DbNodeTestCase(base.DbTestCase): + + def test_create_node(self): + utils.create_test_node() + + def test_create_node_already_exists(self): + utils.create_test_node() + self.assertRaises(exception.NodeAlreadyExists, + utils.create_test_node) + + def test_create_node_instance_already_associated(self): + instance_uuid = magnum_utils.generate_uuid() + utils.create_test_node(uuid=magnum_utils.generate_uuid(), + ironic_node_id=instance_uuid) + self.assertRaises(exception.InstanceAssociated, + utils.create_test_node, + uuid=magnum_utils.generate_uuid(), + ironic_node_id=instance_uuid) + + def test_get_node_by_id(self): + node = utils.create_test_node() + res = self.dbapi.get_node_by_id(node.id) + self.assertEqual(node.id, res.id) + self.assertEqual(node.uuid, res.uuid) + + def test_get_node_by_uuid(self): + node = utils.create_test_node() + res = self.dbapi.get_node_by_uuid(node.uuid) + self.assertEqual(node.id, res.id) + self.assertEqual(node.uuid, res.uuid) + + def test_get_node_that_does_not_exist(self): + self.assertRaises(exception.NodeNotFound, + self.dbapi.get_node_by_id, 99) + self.assertRaises(exception.NodeNotFound, + self.dbapi.get_node_by_uuid, + magnum_utils.generate_uuid()) + + def test_get_nodeinfo_list_defaults(self): + node_id_list = [] + for i in range(1, 6): + node = utils.create_test_node(uuid=magnum_utils.generate_uuid()) + node_id_list.append(node.id) + res = [i[0] for i in self.dbapi.get_nodeinfo_list()] + self.assertEqual(sorted(res), sorted(node_id_list)) + + def test_get_nodeinfo_list_with_cols(self): + uuids = {} + images = {} + for i in range(1, 6): + uuid = magnum_utils.generate_uuid() + image = 'image' + str(i) + node = utils.create_test_node(image_id=image, uuid=uuid) + uuids[node.id] = uuid + images[node.id] = image + res = self.dbapi.get_nodeinfo_list(columns=['id', 'image_id', 'uuid']) + self.assertEqual(images, dict((r[0], r[1]) for r in res)) + self.assertEqual(uuids, dict((r[0], r[2]) for r in res)) + + def test_get_nodeinfo_list_with_filters(self): + node1 = utils.create_test_node(type='virt', + ironic_node_id=magnum_utils.generate_uuid(), + uuid=magnum_utils.generate_uuid()) + node2 = utils.create_test_node(type='bare', + uuid=magnum_utils.generate_uuid()) + + res = self.dbapi.get_nodeinfo_list(filters={'type': 'virt'}) + self.assertEqual([node1.id], [r[0] for r in res]) + + res = self.dbapi.get_nodeinfo_list(filters={'type': 'bad-type'}) + self.assertEqual([], [r[0] for r in res]) + + res = self.dbapi.get_nodeinfo_list(filters={'associated': True}) + self.assertEqual([node1.id], [r[0] for r in res]) + + res = self.dbapi.get_nodeinfo_list(filters={'associated': False}) + self.assertEqual([node2.id], [r[0] for r in res]) + + def test_get_node_list(self): + uuids = [] + for i in range(1, 6): + node = utils.create_test_node(uuid=magnum_utils.generate_uuid()) + uuids.append(six.text_type(node['uuid'])) + res = self.dbapi.get_node_list() + res_uuids = [r.uuid for r in res] + self.assertEqual(sorted(uuids), sorted(res_uuids)) + + def test_get_node_list_with_filters(self): + node1 = utils.create_test_node(type='virt', + ironic_node_id=magnum_utils.generate_uuid(), + uuid=magnum_utils.generate_uuid()) + node2 = utils.create_test_node(type='bare', + uuid=magnum_utils.generate_uuid()) + + res = self.dbapi.get_node_list(filters={'type': 'virt'}) + self.assertEqual([node1.id], [r.id for r in res]) + + res = self.dbapi.get_node_list(filters={'type': 'bad-type'}) + self.assertEqual([], [r.id for r in res]) + + res = self.dbapi.get_node_list(filters={'associated': True}) + self.assertEqual([node1.id], [r.id for r in res]) + + res = self.dbapi.get_node_list(filters={'associated': False}) + self.assertEqual([node2.id], [r.id for r in res]) + + def test_destroy_node(self): + node = utils.create_test_node() + self.dbapi.destroy_node(node.id) + self.assertRaises(exception.NodeNotFound, + self.dbapi.get_node_by_id, node.id) + + def test_destroy_node_by_uuid(self): + node = utils.create_test_node() + self.dbapi.destroy_node(node.uuid) + self.assertRaises(exception.NodeNotFound, + self.dbapi.get_node_by_uuid, node.uuid) + + def test_destroy_node_that_does_not_exist(self): + self.assertRaises(exception.NodeNotFound, + self.dbapi.destroy_node, + magnum_utils.generate_uuid()) + + def test_update_node(self): + node = utils.create_test_node() + old_image = node.image_id + new_image = 'new-image' + self.assertNotEqual(old_image, new_image) + + res = self.dbapi.update_node(node.id, {'image_id': new_image}) + self.assertEqual(new_image, res.image_id) + + def test_update_node_not_found(self): + node_uuid = magnum_utils.generate_uuid() + new_image = 'new-image' + self.assertRaises(exception.NodeNotFound, self.dbapi.update_node, + node_uuid, {'image_id': new_image}) + + def test_update_node_uuid(self): + node = utils.create_test_node() + self.assertRaises(exception.InvalidParameterValue, + self.dbapi.update_node, node.id, + {'uuid': ''}) + + def test_update_node_associate_and_disassociate(self): + node = utils.create_test_node() + new_i_uuid = magnum_utils.generate_uuid() + res = self.dbapi.update_node(node.id, {'ironic_node_id': new_i_uuid}) + self.assertEqual(new_i_uuid, res.ironic_node_id) + res = self.dbapi.update_node(node.id, {'ironic_node_id': None}) + self.assertIsNone(res.ironic_node_id) + + def test_update_node_already_associated(self): + node = utils.create_test_node() + new_i_uuid_one = magnum_utils.generate_uuid() + self.dbapi.update_node(node.id, {'ironic_node_id': new_i_uuid_one}) + new_i_uuid_two = magnum_utils.generate_uuid() + self.assertRaises(exception.NodeAssociated, + self.dbapi.update_node, node.id, + {'ironic_node_id': new_i_uuid_two}) + + def test_update_node_instance_already_associated(self): + node1 = utils.create_test_node(uuid=magnum_utils.generate_uuid()) + new_i_uuid = magnum_utils.generate_uuid() + self.dbapi.update_node(node1.id, {'ironic_node_id': new_i_uuid}) + node2 = utils.create_test_node(uuid=magnum_utils.generate_uuid()) + self.assertRaises(exception.InstanceAssociated, + self.dbapi.update_node, node2.id, + {'ironic_node_id': new_i_uuid}) \ No newline at end of file diff --git a/magnum/tests/db/utils.py b/magnum/tests/db/utils.py index 754f104eed..6228d3ba32 100644 --- a/magnum/tests/db/utils.py +++ b/magnum/tests/db/utils.py @@ -122,6 +122,57 @@ def create_test_service(**kw): return dbapi.create_service(service) +def get_test_node(**kw): + return { + 'id': kw.get('id', 42), + 'uuid': kw.get('uuid', 'ea8e2a25-2901-438d-8157-de7ffd68d051'), + 'type': kw.get('type', 'virt'), + 'image_id': kw.get('image_id', 'ubuntu'), + 'ironic_node_id': kw.get('ironic_node_id'), + 'created_at': kw.get('created_at'), + 'updated_at': kw.get('updated_at'), + } + + +def create_test_node(**kw): + """Create test node entry in DB and return Node DB object. + Function to be used to create test Node objects in the database. + :param kw: kwargs with overriding values for node's attributes. + :returns: Test Node DB object. + """ + node = get_test_node(**kw) + # Let DB generate ID if it isn't specified explicitly + if 'id' not in kw: + del node['id'] + dbapi = db_api.get_instance() + return dbapi.create_node(node) + + +def get_test_container(**kw): + return { + 'id': kw.get('id', 42), + 'uuid': kw.get('uuid', 'ea8e2a25-2901-438d-8157-de7ffd68d051'), + 'name': kw.get('name', 'container1'), + 'image_id': kw.get('image_id', 'ubuntu'), + 'created_at': kw.get('created_at'), + 'updated_at': kw.get('updated_at'), + } + + +def create_test_container(**kw): + """Create test container entry in DB and return Container DB object. + Function to be used to create test Container objects in the database. + :param kw: kwargs with overriding values for container's attributes. + :returns: Test Container DB object. + """ + container = get_test_container(**kw) + # Let DB generate ID if it isn't specified explicitly + if 'id' not in kw: + del container['id'] + dbapi = db_api.get_instance() + return dbapi.create_container(container) + + def get_test_rc(**kw): return { 'id': kw.get('id', 42), diff --git a/tox.ini b/tox.ini index 5e217ff4ef..c21b2cdafc 100644 --- a/tox.ini +++ b/tox.ini @@ -36,9 +36,10 @@ commands = [flake8] # H803 skipped on purpose per list discussion. # E125 is deliberately excluded. See https://github.com/jcrocholl/pep8/issues/126 +# E711 is ignored because it is normal to use "column == None" in sqlalchemy # The rest of the ignores are TODOs # New from hacking 0.9: E129, E131, H407, H405, H904 # E251 Skipped due to https://github.com/jcrocholl/pep8/issues/301 -ignore = E121,E122,E123,E124,E125,E126,E127,E128,E129,E131,E251,H302,H405,H803,H904 +ignore = E121,E122,E123,E124,E125,E126,E127,E128,E129,E131,E251,H302,H405,H803,H904,E711 exclude = .venv,.git,.tox,dist,doc,*openstack/common*,*lib/python*,*egg,build,tools