From 61bf7ae5795a3dfac7afc57bed9b4195cfdb6042 Mon Sep 17 00:00:00 2001 From: Mohammed Naser Date: Fri, 2 Nov 2018 12:21:26 +0100 Subject: [PATCH] Default embedded instance.flavor.is_public attribute It is possible that really old instances don't actually have this attribute defined which can lead to raising exceptions when loading their embedded flavors from the database. This patch fixes this by defaulting these values to true if they are not set. Change-Id: If04cd802ce7184dc94f94804c743faebe0d4bd8c Closes-Bug: #1789423 (cherry picked from commit c4f6b0bf6cc903cf52c4b238c3771604dda174b8) (cherry picked from commit c689c09996c4a3da9e05ccd5178a4b5060949889) --- nova/objects/instance.py | 20 +++++++++++--------- nova/tests/unit/objects/test_instance.py | 10 ++++++++-- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/nova/objects/instance.py b/nova/objects/instance.py index 4b79fe397a41..08b60593fd25 100644 --- a/nova/objects/instance.py +++ b/nova/objects/instance.py @@ -321,29 +321,31 @@ class Instance(base.NovaPersistentObject, base.NovaObject, # Before we stored flavors in instance_extra, certain fields, defined # in nova.compute.flavors.system_metadata_flavor_props, were stored # in the instance.system_metadata for the embedded instance.flavor. - # The "disabled" field wasn't one of those keys, however, so really - # old instances that had their embedded flavor converted to the - # serialized instance_extra form won't have the disabled attribute - # set and we need to default those here so callers don't explode trying - # to load instance.flavor.disabled. - def _default_disabled(flavor): + # The "disabled" and "is_public" fields weren't one of those keys, + # however, so really old instances that had their embedded flavor + # converted to the serialized instance_extra form won't have the + # disabled attribute set and we need to default those here so callers + # don't explode trying to load instance.flavor.disabled. + def _default_flavor_values(flavor): if 'disabled' not in flavor: flavor.disabled = False + if 'is_public' not in flavor: + flavor.is_public = True flavor_info = jsonutils.loads(db_flavor) self.flavor = objects.Flavor.obj_from_primitive(flavor_info['cur']) - _default_disabled(self.flavor) + _default_flavor_values(self.flavor) if flavor_info['old']: self.old_flavor = objects.Flavor.obj_from_primitive( flavor_info['old']) - _default_disabled(self.old_flavor) + _default_flavor_values(self.old_flavor) else: self.old_flavor = None if flavor_info['new']: self.new_flavor = objects.Flavor.obj_from_primitive( flavor_info['new']) - _default_disabled(self.new_flavor) + _default_flavor_values(self.new_flavor) else: self.new_flavor = None self.obj_reset_changes(['flavor', 'old_flavor', 'new_flavor']) diff --git a/nova/tests/unit/objects/test_instance.py b/nova/tests/unit/objects/test_instance.py index 2d70c403578e..14798e36decb 100644 --- a/nova/tests/unit/objects/test_instance.py +++ b/nova/tests/unit/objects/test_instance.py @@ -336,8 +336,10 @@ class _TestInstanceObject(object): # make sure we default the "new" flavor's disabled value to False on # load from the database. fake_flavor = jsonutils.dumps( - {'cur': objects.Flavor(disabled=False).obj_to_primitive(), - 'old': objects.Flavor(disabled=True).obj_to_primitive(), + {'cur': objects.Flavor(disabled=False, + is_public=True).obj_to_primitive(), + 'old': objects.Flavor(disabled=True, + is_public=False).obj_to_primitive(), 'new': objects.Flavor().obj_to_primitive()}) fake_inst = dict(self.fake_instance, extra={'flavor': fake_flavor}) mock_get.return_value = fake_inst @@ -345,6 +347,10 @@ class _TestInstanceObject(object): self.assertFalse(inst.flavor.disabled) self.assertTrue(inst.old_flavor.disabled) self.assertFalse(inst.new_flavor.disabled) + # Assert the is_public values on the flavors + self.assertTrue(inst.flavor.is_public) + self.assertFalse(inst.old_flavor.is_public) + self.assertTrue(inst.new_flavor.is_public) @mock.patch.object(db, 'instance_get_by_uuid') def test_get_remote(self, mock_get):