openstack/ironic
Code Issues Proposed changes

Removed hardcoded IDs from "chassis" test resources

Browse Source 
Test resources are generated using certain helper methods to provide
key/value data. These methods hardcode ID fields which map to primary
key in the database. Subsequent creates on these resources force the
database to assign hardcoded primary keys, ignoring DB's internal
mechanisms. Though unlikely, this could lead to primary key conflicts.

This commit removes hardcoded IDs from chassis resources. As a side
effect, nodes are created without chassis association by default
because chassis referenced in default test node dictionary might not
exist.

Co-Authored-By: Vladyslav Drok <vdrok@mirantis.com>
Closes-bug: #1268589
Change-Id: I1f336055a02f11798cf6e20a86d3ba59f4aec080
This commit is contained in:
Vladyslav Drok 2015-03-20 17:52:38 +02:00
parent f734945746
commit 3aec8cae70
6 changed files with 63 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
ironic/tests/api/v1/test_chassis.py
View File

@ -77,7 +77,7 @@ class TestListChassis(api_base.FunctionalTest):
ch_list = [] ch_list = []
for id_ in range(5): for id_ in range(5):
chassis = obj_utils.create_test_chassis( chassis = obj_utils.create_test_chassis(
self.context, id=id_, uuid=uuidutils.generate_uuid()) self.context, uuid=uuidutils.generate_uuid())
ch_list.append(chassis.uuid) ch_list.append(chassis.uuid)
data = self.get_json('/chassis') data = self.get_json('/chassis')
self.assertEqual(len(ch_list), len(data['chassis'])) self.assertEqual(len(ch_list), len(data['chassis']))
@ -86,7 +86,7 @@ class TestListChassis(api_base.FunctionalTest):
def test_links(self): def test_links(self):
uuid = uuidutils.generate_uuid() uuid = uuidutils.generate_uuid()
obj_utils.create_test_chassis(self.context, id=1, uuid=uuid) obj_utils.create_test_chassis(self.context, uuid=uuid)
data = self.get_json('/chassis/%s' % uuid) data = self.get_json('/chassis/%s' % uuid)
self.assertIn('links', data.keys()) self.assertIn('links', data.keys())
self.assertEqual(2, len(data['links'])) self.assertEqual(2, len(data['links']))
@ -97,7 +97,7 @@ class TestListChassis(api_base.FunctionalTest):
def test_collection_links(self): def test_collection_links(self):
for id in range(5): for id in range(5):
obj_utils.create_test_chassis(self.context, id=id, obj_utils.create_test_chassis(self.context,
uuid=uuidutils.generate_uuid()) uuid=uuidutils.generate_uuid())
data = self.get_json('/chassis/?limit=3') data = self.get_json('/chassis/?limit=3')
self.assertEqual(3, len(data['chassis'])) self.assertEqual(3, len(data['chassis']))
@ -108,7 +108,7 @@ class TestListChassis(api_base.FunctionalTest):
def test_collection_links_default_limit(self): def test_collection_links_default_limit(self):
cfg.CONF.set_override('max_limit', 3, 'api') cfg.CONF.set_override('max_limit', 3, 'api')
for id_ in range(5): for id_ in range(5):
obj_utils.create_test_chassis(self.context, id=id_, obj_utils.create_test_chassis(self.context,
uuid=uuidutils.generate_uuid()) uuid=uuidutils.generate_uuid())
data = self.get_json('/chassis') data = self.get_json('/chassis')
self.assertEqual(3, len(data['chassis'])) self.assertEqual(3, len(data['chassis']))
@ -186,8 +186,7 @@ class TestPatch(api_base.FunctionalTest):
def test_replace_multi(self): def test_replace_multi(self):
extra = {"foo1": "bar1", "foo2": "bar2", "foo3": "bar3"} extra = {"foo1": "bar1", "foo2": "bar2", "foo3": "bar3"}
chassis = obj_utils.create_test_chassis(self.context, extra=extra, chassis = obj_utils.create_test_chassis(self.context, extra=extra,
uuid=uuidutils.generate_uuid(), uuid=uuidutils.generate_uuid())
id=1)
new_value = 'new value' new_value = 'new value'
response = self.patch_json('/chassis/%s' % chassis.uuid, response = self.patch_json('/chassis/%s' % chassis.uuid,
[{'path': '/extra/foo2', [{'path': '/extra/foo2',
@ -201,8 +200,7 @@ class TestPatch(api_base.FunctionalTest):
def test_remove_singular(self): def test_remove_singular(self):
chassis = obj_utils.create_test_chassis(self.context, extra={'a': 'b'}, chassis = obj_utils.create_test_chassis(self.context, extra={'a': 'b'},
uuid=uuidutils.generate_uuid(), uuid=uuidutils.generate_uuid())
id=1)
response = self.patch_json('/chassis/%s' % chassis.uuid, response = self.patch_json('/chassis/%s' % chassis.uuid,
[{'path': '/description', 'op': 'remove'}]) [{'path': '/description', 'op': 'remove'}])
self.assertEqual('application/json', response.content_type) self.assertEqual('application/json', response.content_type)
@ -218,8 +216,7 @@ class TestPatch(api_base.FunctionalTest):
extra = {"foo1": "bar1", "foo2": "bar2", "foo3": "bar3"} extra = {"foo1": "bar1", "foo2": "bar2", "foo3": "bar3"}
chassis = obj_utils.create_test_chassis(self.context, extra=extra, chassis = obj_utils.create_test_chassis(self.context, extra=extra,
description="foobar", description="foobar",
uuid=uuidutils.generate_uuid(), uuid=uuidutils.generate_uuid())
id=1)
# Removing one item from the collection # Removing one item from the collection
response = self.patch_json('/chassis/%s' % chassis.uuid, response = self.patch_json('/chassis/%s' % chassis.uuid,
@ -343,7 +340,7 @@ class TestPost(api_base.FunctionalTest):
def test_post_nodes_subresource(self): def test_post_nodes_subresource(self):
chassis = obj_utils.create_test_chassis(self.context) chassis = obj_utils.create_test_chassis(self.context)
ndict = apiutils.node_post_data(chassis_id=None) ndict = apiutils.node_post_data()
ndict['chassis_uuid'] = chassis.uuid ndict['chassis_uuid'] = chassis.uuid
response = self.post_json('/chassis/nodes', ndict, response = self.post_json('/chassis/nodes', ndict,
expect_errors=True) expect_errors=True)

22
ironic/tests/api/v1/test_nodes.py
View File

@ -120,7 +120,7 @@ class TestTopLevelFunctions(base.TestCase):
class TestNodeObject(base.TestCase): class TestNodeObject(base.TestCase):
def test_node_init(self): def test_node_init(self):
node_dict = test_api_utils.node_post_data(chassis_id=None) node_dict = test_api_utils.node_post_data()
del node_dict['instance_uuid'] del node_dict['instance_uuid']
node = api_node.Node(**node_dict) node = api_node.Node(**node_dict)
self.assertEqual(wtypes.Unset, node.instance_uuid) self.assertEqual(wtypes.Unset, node.instance_uuid)
@ -159,7 +159,8 @@ class TestListNodes(test_api_base.FunctionalTest):
self.assertEqual([], data['nodes']) self.assertEqual([], data['nodes'])
def test_one(self): def test_one(self):
node = obj_utils.create_test_node(self.context) node = obj_utils.create_test_node(self.context,
chassis_id=self.chassis.id)
data = self.get_json('/nodes', data = self.get_json('/nodes',
headers={api_base.Version.string: str(api_v1.MAX_VER)}) headers={api_base.Version.string: str(api_v1.MAX_VER)})
self.assertIn('instance_uuid', data['nodes'][0]) self.assertIn('instance_uuid', data['nodes'][0])
@ -184,7 +185,8 @@ class TestListNodes(test_api_base.FunctionalTest):
self.assertNotIn('chassis_id', data['nodes'][0]) self.assertNotIn('chassis_id', data['nodes'][0])
def test_get_one(self): def test_get_one(self):
node = obj_utils.create_test_node(self.context) node = obj_utils.create_test_node(self.context,
chassis_id=self.chassis.id)
data = self.get_json('/nodes/%s' % node.uuid, data = self.get_json('/nodes/%s' % node.uuid,
headers={api_base.Version.string: str(api_v1.MAX_VER)}) headers={api_base.Version.string: str(api_v1.MAX_VER)})
self.assertEqual(node.uuid, data['uuid']) self.assertEqual(node.uuid, data['uuid'])
@ -205,7 +207,8 @@ class TestListNodes(test_api_base.FunctionalTest):
self.assertNotIn('chassis_id', data) self.assertNotIn('chassis_id', data)
def test_detail(self): def test_detail(self):
node = obj_utils.create_test_node(self.context) node = obj_utils.create_test_node(self.context,
chassis_id=self.chassis.id)
data = self.get_json('/nodes/detail', data = self.get_json('/nodes/detail',
headers={api_base.Version.string: str(api_v1.MAX_VER)}) headers={api_base.Version.string: str(api_v1.MAX_VER)})
self.assertEqual(node.uuid, data['nodes'][0]["uuid"]) self.assertEqual(node.uuid, data['nodes'][0]["uuid"])
@ -530,7 +533,8 @@ class TestListNodes(test_api_base.FunctionalTest):
node = obj_utils.create_test_node( node = obj_utils.create_test_node(
self.context, self.context,
uuid=uuidutils.generate_uuid(), uuid=uuidutils.generate_uuid(),
instance_uuid=uuidutils.generate_uuid()) instance_uuid=uuidutils.generate_uuid(),
chassis_id=self.chassis.id)
instance_uuid = node.instance_uuid instance_uuid = node.instance_uuid
data = self.get_json('/nodes/detail?instance_uuid=%s' % instance_uuid) data = self.get_json('/nodes/detail?instance_uuid=%s' % instance_uuid)
@ -734,9 +738,11 @@ class TestPatch(test_api_base.FunctionalTest):
def setUp(self): def setUp(self):
super(TestPatch, self).setUp() super(TestPatch, self).setUp()
self.chassis = obj_utils.create_test_chassis(self.context) self.chassis = obj_utils.create_test_chassis(self.context)
self.node = obj_utils.create_test_node(self.context, name='node-57') self.node = obj_utils.create_test_node(self.context, name='node-57',
chassis_id=self.chassis.id)
self.node_no_name = obj_utils.create_test_node(self.context, self.node_no_name = obj_utils.create_test_node(self.context,
uuid='deadbeef-0000-1111-2222-333333333333') uuid='deadbeef-0000-1111-2222-333333333333',
chassis_id=self.chassis.id)
p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for') p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for')
self.mock_gtf = p.start() self.mock_gtf = p.start()
self.mock_gtf.return_value = 'test-topic' self.mock_gtf.return_value = 'test-topic'
@ -1392,7 +1398,6 @@ class TestDelete(test_api_base.FunctionalTest):
def setUp(self): def setUp(self):
super(TestDelete, self).setUp() super(TestDelete, self).setUp()
self.chassis = obj_utils.create_test_chassis(self.context)
p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for') p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for')
self.mock_gtf = p.start() self.mock_gtf = p.start()
self.mock_gtf.return_value = 'test-topic' self.mock_gtf.return_value = 'test-topic'
@ -1508,7 +1513,6 @@ class TestPut(test_api_base.FunctionalTest):
def setUp(self): def setUp(self):
super(TestPut, self).setUp() super(TestPut, self).setUp()
self.chassis = obj_utils.create_test_chassis(self.context)
self.node = obj_utils.create_test_node(self.context, self.node = obj_utils.create_test_node(self.context,
provision_state=states.AVAILABLE, name='node-39') provision_state=states.AVAILABLE, name='node-39')
p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for') p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for')

47
ironic/tests/db/test_chassis.py
View File

@ -25,40 +25,36 @@ from ironic.tests.db import utils
class DbChassisTestCase(base.DbTestCase): class DbChassisTestCase(base.DbTestCase):
def _create_test_chassis(self, **kwargs): def setUp(self):
ch = utils.get_test_chassis(**kwargs) super(DbChassisTestCase, self).setUp()
self.dbapi.create_chassis(ch) self.chassis = utils.create_test_chassis()
return ch
def test_get_chassis_list(self): def test_get_chassis_list(self):
uuids = [] uuids = [self.chassis.uuid]
for i in range(1, 6): for i in range(1, 6):
n = utils.get_test_chassis(id=i, uuid=uuidutils.generate_uuid()) ch = utils.create_test_chassis(uuid=uuidutils.generate_uuid())
self.dbapi.create_chassis(n) uuids.append(six.text_type(ch.uuid))
uuids.append(six.text_type(n['uuid']))
res = self.dbapi.get_chassis_list() res = self.dbapi.get_chassis_list()
res_uuids = [r.uuid for r in res] res_uuids = [r.uuid for r in res]
self.assertEqual(uuids.sort(), res_uuids.sort()) self.assertEqual(uuids.sort(), res_uuids.sort())
def test_get_chassis_by_id(self): def test_get_chassis_by_id(self):
ch = self._create_test_chassis() chassis = self.dbapi.get_chassis_by_id(self.chassis.id)
chassis = self.dbapi.get_chassis_by_id(ch['id'])
self.assertEqual(ch['uuid'], chassis.uuid) self.assertEqual(self.chassis.uuid, chassis.uuid)
def test_get_chassis_by_uuid(self): def test_get_chassis_by_uuid(self):
ch = self._create_test_chassis() chassis = self.dbapi.get_chassis_by_uuid(self.chassis.uuid)
chassis = self.dbapi.get_chassis_by_uuid(ch['uuid'])
self.assertEqual(ch['id'], chassis.id) self.assertEqual(self.chassis.id, chassis.id)
def test_get_chassis_that_does_not_exist(self): def test_get_chassis_that_does_not_exist(self):
self.assertRaises(exception.ChassisNotFound, self.assertRaises(exception.ChassisNotFound,
self.dbapi.get_chassis_by_id, 666) self.dbapi.get_chassis_by_id, 666)
def test_update_chassis(self): def test_update_chassis(self):
ch = self._create_test_chassis() res = self.dbapi.update_chassis(self.chassis.id,
res = self.dbapi.update_chassis(ch['id'], {'description': 'hello'}) {'description': 'hello'})
self.assertEqual('hello', res.description) self.assertEqual('hello', res.description)
@ -67,32 +63,27 @@ class DbChassisTestCase(base.DbTestCase):
self.dbapi.update_chassis, 666, {'description': ''}) self.dbapi.update_chassis, 666, {'description': ''})
def test_update_chassis_uuid(self): def test_update_chassis_uuid(self):
ch = self._create_test_chassis()
self.assertRaises(exception.InvalidParameterValue, self.assertRaises(exception.InvalidParameterValue,
self.dbapi.update_chassis, ch['id'], self.dbapi.update_chassis, self.chassis.id,
{'uuid': 'hello'}) {'uuid': 'hello'})
def test_destroy_chassis(self): def test_destroy_chassis(self):
ch = self._create_test_chassis() self.dbapi.destroy_chassis(self.chassis.id)
self.dbapi.destroy_chassis(ch['id'])
self.assertRaises(exception.ChassisNotFound, self.assertRaises(exception.ChassisNotFound,
self.dbapi.get_chassis_by_id, ch['id']) self.dbapi.get_chassis_by_id, self.chassis.id)
def test_destroy_chassis_that_does_not_exist(self): def test_destroy_chassis_that_does_not_exist(self):
self.assertRaises(exception.ChassisNotFound, self.assertRaises(exception.ChassisNotFound,
self.dbapi.destroy_chassis, 666) self.dbapi.destroy_chassis, 666)
def test_destroy_chassis_with_nodes(self): def test_destroy_chassis_with_nodes(self):
ch = self._create_test_chassis() utils.create_test_node(chassis_id=self.chassis.id)
utils.create_test_node(chassis_id=ch['id'])
self.assertRaises(exception.ChassisNotEmpty, self.assertRaises(exception.ChassisNotEmpty,
self.dbapi.destroy_chassis, ch['id']) self.dbapi.destroy_chassis, self.chassis.id)
def test_create_chassis_already_exists(self): def test_create_chassis_already_exists(self):
uuid = uuidutils.generate_uuid()
self._create_test_chassis(id=1, uuid=uuid)
self.assertRaises(exception.ChassisAlreadyExists, self.assertRaises(exception.ChassisAlreadyExists,
self._create_test_chassis, utils.create_test_chassis,
id=2, uuid=uuid) uuid=self.chassis.uuid)

9
ironic/tests/db/test_nodes.py
View File

@ -33,9 +33,6 @@ class DbNodeTestCase(base.DbTestCase):
def test_create_node(self): def test_create_node(self):
utils.create_test_node() utils.create_test_node()
def test_create_node_nullable_chassis_id(self):
utils.create_test_node(chassis_id=None)
def test_create_node_already_exists(self): def test_create_node_already_exists(self):
utils.create_test_node() utils.create_test_node()
self.assertRaises(exception.NodeAlreadyExists, self.assertRaises(exception.NodeAlreadyExists,
@ -200,10 +197,8 @@ class DbNodeTestCase(base.DbTestCase):
self.assertEqual(uuids.sort(), res_uuids.sort()) self.assertEqual(uuids.sort(), res_uuids.sort())
def test_get_node_list_with_filters(self): def test_get_node_list_with_filters(self):
ch1 = utils.get_test_chassis(id=1, uuid=uuidutils.generate_uuid()) ch1 = utils.create_test_chassis(uuid=uuidutils.generate_uuid())
ch2 = utils.get_test_chassis(id=2, uuid=uuidutils.generate_uuid()) ch2 = utils.create_test_chassis(uuid=uuidutils.generate_uuid())
self.dbapi.create_chassis(ch1)
self.dbapi.create_chassis(ch2)
node1 = utils.create_test_node(driver='driver-one', node1 = utils.create_test_node(driver='driver-one',
instance_uuid=uuidutils.generate_uuid(), instance_uuid=uuidutils.generate_uuid(),

19
ironic/tests/db/utils.py
View File

@ -187,7 +187,7 @@ def get_test_node(**kw):
'id': kw.get('id', 123), 'id': kw.get('id', 123),
'name': kw.get('name', None), 'name': kw.get('name', None),
'uuid': kw.get('uuid', '1be26c0b-03f2-4d2e-ae87-c02d7f33c123'), 'uuid': kw.get('uuid', '1be26c0b-03f2-4d2e-ae87-c02d7f33c123'),
'chassis_id': kw.get('chassis_id', 42), 'chassis_id': kw.get('chassis_id', None),
'conductor_affinity': kw.get('conductor_affinity', None), 'conductor_affinity': kw.get('conductor_affinity', None),
'power_state': kw.get('power_state', states.NOSTATE), 'power_state': kw.get('power_state', states.NOSTATE),
'target_power_state': kw.get('target_power_state', states.NOSTATE), 'target_power_state': kw.get('target_power_state', states.NOSTATE),
@ -272,6 +272,23 @@ def get_test_chassis(**kw):
} }
def create_test_chassis(**kw):
"""Create test chassis entry in DB and return Chassis DB object.
Function to be used to create test Chassis objects in the database.
:param kw: kwargs with overriding values for chassis's attributes.
:returns: Test Chassis DB object.
"""
chassis = get_test_chassis(**kw)
# Let DB generate ID if it isn't specified explicitly
if 'id' not in kw:
del chassis['id']
dbapi = db_api.get_instance()
return dbapi.create_chassis(chassis)
def get_test_conductor(**kw): def get_test_conductor(**kw):
return { return {
'id': kw.get('id', 6), 'id': kw.get('id', 6),

3
ironic/tests/objects/utils.py
View File

@ -79,6 +79,9 @@ def get_test_chassis(ctxt, **kw):
that a create() could be used to commit it to the DB. that a create() could be used to commit it to the DB.
""" """
db_chassis = db_utils.get_test_chassis(**kw) db_chassis = db_utils.get_test_chassis(**kw)
# Let DB generate ID if it isn't specified explicitly
if 'id' not in kw:
del db_chassis['id']
chassis = objects.Chassis(ctxt) chassis = objects.Chassis(ctxt)
for key in db_chassis: for key in db_chassis:
setattr(chassis, key, db_chassis[key]) setattr(chassis, key, db_chassis[key])