openstack
/
nova
Code Issues Proposed changes

Add missing test for None in sqlalchemy query filter 

In sqlalchemy a comparison against a non-null value will not match
null values, so the existing code at the end of
db.sqlalchemy.api.instance_get_all_by_filters() will not return
instances where vm_state is NULL.

This would be fine if the vm_state could never be null, but it
is declared as "nullable" in the Instance object. In many cases
"vm_state" will in fact have a value, but not all--in
get_test_instance() in test/utils.py the value of "vm_state" is not
specified.  There may be other similar cases.

Accordingly, this commit updates the test to explicitly check for None.

Without this fix the unit tests for bug 1292963 will fail because
the test instances have a vm_state of None.

Closes-Bug: 1294756
Related-Bug: 1292963
Change-Id: I271cff22dec160fd0e76abadefd0fe06d32c3227
This commit is contained in:
Chris Friesen 2014-03-25 08:29:58 -06:00
parent 0722a8fb23
commit 0aecc86987
2 changed files with 18 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
nova/db/sqlalchemy/api.py
View File

@ -1898,8 +1898,13 @@ def instance_get_all_by_filters(context, filters, sort_key, sort_dir,
query_prefix = query_prefix.\
filter_by(deleted=0)
if not filters.pop('soft_deleted', False):
query_prefix = query_prefix.\
filter(models.Instance.vm_state != vm_states.SOFT_DELETED)
# It would be better to have vm_state not be nullable
# but until then we test it explicitly as a workaround.
not_soft_deleted = or_(
models.Instance.vm_state != vm_states.SOFT_DELETED,
models.Instance.vm_state == None
)
query_prefix = query_prefix.filter(not_soft_deleted)
if 'cleaned' in filters:
if filters.pop('cleaned'):

11
nova/tests/db/test_db_api.py
View File

@ -1738,6 +1738,17 @@ class InstanceTestCase(test.TestCase, ModelsObjectComparatorMixin):
'soft_deleted': True})
self._assertEqualListsOfInstances([inst2, inst3], result)
def test_instance_get_all_by_filters_not_deleted(self):
inst1 = self.create_instance_with_args()
inst2 = self.create_instance_with_args(vm_state=vm_states.SOFT_DELETED)
inst3 = self.create_instance_with_args()
inst4 = self.create_instance_with_args(vm_state=vm_states.ACTIVE)
db.instance_destroy(self.ctxt, inst1['uuid'])
result = db.instance_get_all_by_filters(self.ctxt,
{'deleted': False})
self.assertIsNone(inst3.vm_state)
self._assertEqualListsOfInstances([inst3, inst4], result)
def test_instance_get_all_by_filters_cleaned(self):
inst1 = self.create_instance_with_args()
inst2 = self.create_instance_with_args(reservation_id='b')