openstack/magnum
Code Issues Proposed changes

Add unit tests for dbapi of Node and Container

Browse Source 
Do the following fixes to pass the tests:
* Add unique constraint to 'ironic_node_id'.
* Throw an exception on deleting a non-existent Node or Container.
* Throw an exception on associating an ironic node to an already
  associated magnum node.
* Throw an exception on associating an already associated ironic
  node to a magnum node.
* Fix the methods _add_containers_filters and _add_nodes_filters.

Change-Id: Ibf71853e2468ff95d4055be09cb0b460be28a3db
This commit is contained in:
Hongbin Lu 2015-01-06 02:53:37 +00:00
parent 068874d0db
commit 2cf34a243a
7 changed files with 428 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
magnum/common/exception.py
View File

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

71
magnum/db/sqlalchemy/api.py
View File

@ -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

2
magnum/db/sqlalchemy/models.py
View File

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

156
magnum/tests/db/test_container.py Normal file
View File

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

194
magnum/tests/db/test_node.py Normal file
View File

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

51
magnum/tests/db/utils.py
View File

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

3
tox.ini
View File

@ -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