From 7a158ac4488ac86c742e15c714dda7dd76772366 Mon Sep 17 00:00:00 2001 From: LisaLi Date: Fri, 26 Feb 2016 10:41:19 +0800 Subject: [PATCH] Make nullable of fields in db model and object match Currently there are some fields whose nullable in db model and object don't match. This may lead object query fails. For example, for volume, if attach_status is nullable in db schema, but is not nullable in object schema. When writting data to db using db interface, the attach_status can be null. But later when reading data from db to construct volume object, it raises exception. As a result, all the query related to the volume fails. This patch is to make all the fields in corresponding db model and object match. Change-Id: I84fb16f0a67f9a1f7a09e91bb8ebe6298f3f49c4 Closes-Bug: #1549669 Related-Bug: #1549673 --- cinder/objects/backup.py | 6 ++-- cinder/objects/base.py | 3 +- cinder/objects/service.py | 2 +- cinder/objects/snapshot.py | 4 +-- cinder/objects/volume.py | 14 ++++----- cinder/objects/volume_type.py | 2 +- cinder/tests/unit/objects/test_objects.py | 38 +++++++++++++++++------ 7 files changed, 45 insertions(+), 24 deletions(-) diff --git a/cinder/objects/backup.py b/cinder/objects/backup.py index 826388280..19674116c 100644 --- a/cinder/objects/backup.py +++ b/cinder/objects/backup.py @@ -54,7 +54,7 @@ class Backup(base.CinderPersistentObject, base.CinderObject, 'parent_id': fields.StringField(nullable=True), 'status': c_fields.BackupStatusField(nullable=True), 'fail_reason': fields.StringField(nullable=True), - 'size': fields.IntegerField(), + 'size': fields.IntegerField(nullable=True), 'display_name': fields.StringField(nullable=True), 'display_description': fields.StringField(nullable=True), @@ -64,11 +64,11 @@ class Backup(base.CinderPersistentObject, base.CinderObject, 'service_metadata': fields.StringField(nullable=True), 'service': fields.StringField(nullable=True), - 'object_count': fields.IntegerField(), + 'object_count': fields.IntegerField(nullable=True), 'temp_volume_id': fields.StringField(nullable=True), 'temp_snapshot_id': fields.StringField(nullable=True), - 'num_dependent_backups': fields.IntegerField(), + 'num_dependent_backups': fields.IntegerField(nullable=True), 'snapshot_id': fields.StringField(nullable=True), 'data_timestamp': fields.DateTimeField(nullable=True), 'restore_volume_id': fields.StringField(nullable=True), diff --git a/cinder/objects/base.py b/cinder/objects/base.py index 034a6083e..5e7fd5c21 100644 --- a/cinder/objects/base.py +++ b/cinder/objects/base.py @@ -350,7 +350,8 @@ class CinderPersistentObject(object): 'created_at': fields.DateTimeField(nullable=True), 'updated_at': fields.DateTimeField(nullable=True), 'deleted_at': fields.DateTimeField(nullable=True), - 'deleted': fields.BooleanField(default=False), + 'deleted': fields.BooleanField(default=False, + nullable=True), } @contextlib.contextmanager diff --git a/cinder/objects/service.py b/cinder/objects/service.py index b8f833e62..43d21e015 100644 --- a/cinder/objects/service.py +++ b/cinder/objects/service.py @@ -43,7 +43,7 @@ class Service(base.CinderPersistentObject, base.CinderObject, 'binary': fields.StringField(nullable=True), 'topic': fields.StringField(nullable=True), 'report_count': fields.IntegerField(default=0), - 'disabled': fields.BooleanField(default=False), + 'disabled': fields.BooleanField(default=False, nullable=True), 'availability_zone': fields.StringField(nullable=True, default='cinder'), 'disabled_reason': fields.StringField(nullable=True), diff --git a/cinder/objects/snapshot.py b/cinder/objects/snapshot.py index 7a9806574..1c7cc64af 100644 --- a/cinder/objects/snapshot.py +++ b/cinder/objects/snapshot.py @@ -45,11 +45,11 @@ class Snapshot(base.CinderPersistentObject, base.CinderObject, 'user_id': fields.UUIDField(nullable=True), 'project_id': fields.UUIDField(nullable=True), - 'volume_id': fields.UUIDField(), + 'volume_id': fields.UUIDField(nullable=True), 'cgsnapshot_id': fields.UUIDField(nullable=True), 'status': fields.StringField(nullable=True), 'progress': fields.StringField(nullable=True), - 'volume_size': fields.IntegerField(), + 'volume_size': fields.IntegerField(nullable=True), 'display_name': fields.StringField(nullable=True), 'display_description': fields.StringField(nullable=True), diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py index d50175ef5..acad84678 100644 --- a/cinder/objects/volume.py +++ b/cinder/objects/volume.py @@ -74,10 +74,10 @@ class Volume(base.CinderPersistentObject, base.CinderObject, 'snapshot_id': fields.UUIDField(nullable=True), 'host': fields.StringField(nullable=True), - 'size': fields.IntegerField(), - 'availability_zone': fields.StringField(), - 'status': fields.StringField(), - 'attach_status': fields.StringField(), + 'size': fields.IntegerField(nullable=True), + 'availability_zone': fields.StringField(nullable=True), + 'status': fields.StringField(nullable=True), + 'attach_status': fields.StringField(nullable=True), 'migration_status': fields.StringField(nullable=True), 'scheduled_at': fields.DateTimeField(nullable=True), @@ -98,9 +98,9 @@ class Volume(base.CinderPersistentObject, base.CinderObject, 'consistencygroup_id': fields.UUIDField(nullable=True), - 'deleted': fields.BooleanField(default=False), - 'bootable': fields.BooleanField(default=False), - 'multiattach': fields.BooleanField(default=False), + 'deleted': fields.BooleanField(default=False, nullable=True), + 'bootable': fields.BooleanField(default=False, nullable=True), + 'multiattach': fields.BooleanField(default=False, nullable=True), 'replication_status': fields.StringField(nullable=True), 'replication_extended_status': fields.StringField(nullable=True), diff --git a/cinder/objects/volume_type.py b/cinder/objects/volume_type.py index 646796722..b6471fd77 100644 --- a/cinder/objects/volume_type.py +++ b/cinder/objects/volume_type.py @@ -37,7 +37,7 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject, 'id': fields.UUIDField(), 'name': fields.StringField(nullable=True), 'description': fields.StringField(nullable=True), - 'is_public': fields.BooleanField(default=True), + 'is_public': fields.BooleanField(default=True, nullable=True), 'projects': fields.ListOfStringsField(nullable=True), 'extra_specs': fields.DictOfStringsField(nullable=True), } diff --git a/cinder/tests/unit/objects/test_objects.py b/cinder/tests/unit/objects/test_objects.py index f8a97c9a5..c6c74d0d9 100644 --- a/cinder/tests/unit/objects/test_objects.py +++ b/cinder/tests/unit/objects/test_objects.py @@ -14,6 +14,7 @@ from oslo_versionedobjects import fixture +from cinder import db from cinder.objects import base from cinder import test @@ -21,22 +22,22 @@ from cinder import test # NOTE: The hashes in this list should only be changed if they come with a # corresponding version bump in the affected objects. object_data = { - 'Backup': '1.4-1002c50b6e31938583c95c4c4889286c', - 'BackupImport': '1.4-1002c50b6e31938583c95c4c4889286c', + 'Backup': '1.4-cae44fe34d5a870110ba93adebc1edca', + 'BackupImport': '1.4-cae44fe34d5a870110ba93adebc1edca', 'BackupList': '1.0-24591dabe26d920ce0756fe64cd5f3aa', - 'CGSnapshot': '1.0-190da2a2aa9457edc771d888f7d225c4', + 'CGSnapshot': '1.0-78b91e76cb4c56e9cf5c9c41e208c05a', 'CGSnapshotList': '1.0-e8c3f4078cd0ee23487b34d173eec776', - 'ConsistencyGroup': '1.2-ed7f90a6871991a19af716ade7337fc9', + 'ConsistencyGroup': '1.2-3aeb6b25664057e8078bd6d45bf23e0a', 'ConsistencyGroupList': '1.1-73916823b697dfa0c7f02508d87e0f28', - 'Service': '1.3-e8f82835bd43722d8d84c55072466eba', + 'Service': '1.3-66c8e1683f58546c54551e9ff0a3b111', 'ServiceList': '1.1-cb758b200f0a3a90efabfc5aa2ffb627', - 'Snapshot': '1.0-a6c33eefeadefb324d79f72f66c54e9a', + 'Snapshot': '1.0-404c1a8b48a808aa0b7cc92cd3ec1e57', 'SnapshotList': '1.0-71661e7180ef6cc51501704a9bea4bf1', - 'Volume': '1.3-97c3977846dae6588381e7bd3e6e6558', - 'VolumeAttachment': '1.0-f14a7c03ffc5b93701d496251a5263aa', + 'Volume': '1.3-264388ec57bc4c3353c89f93bebf9482', + 'VolumeAttachment': '1.0-8fc9a9ac6f554fdf2a194d25dbf28a3b', 'VolumeAttachmentList': '1.0-307d2b6c8dd55ef854f6386898e9e98e', 'VolumeList': '1.1-03ba6cb8c546683e64e15c50042cb1a3', - 'VolumeType': '1.0-bf8abbbea2e852ed2e9bac5a9f5f70f2', + 'VolumeType': '1.0-dd980cfd1eef2dcce941a981eb469fc8', 'VolumeTypeList': '1.1-8a1016c03570dc13b9a33fe04a6acb2c', } @@ -68,3 +69,22 @@ class TestObjectVersions(test.TestCase): 'Some objects versions have changed; please make ' 'sure a new objects history version was added in ' 'cinder.objects.base.OBJ_VERSIONS.') + + def test_object_nullable_match_db(self): + # This test is to keep nullable of every field in corresponding + # db model and object match. + def _check_table_matched(db_model, cls): + for column in db_model.__table__.columns: + if column.name in cls.fields: + self.assertEqual( + column.nullable, + cls.fields[column.name].nullable, + 'Column %(c)s in table %(t)s not match.' + % {'c': column.name, + 't': name}) + + classes = base.CinderObjectRegistry.obj_classes() + for name, cls in classes.items(): + if not issubclass(cls[0], base.ObjectListBase): + db_model = db.get_model_for_versioned_object(cls[0]) + _check_table_matched(db_model, cls[0])