Merge "Add unit tests for dbapi of Node and Container"

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):

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

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)

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': ''})

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})

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),

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