Aggregate object fixups

In the original removal of DictCompat from the Aggregate object, the
aggregate.get(foo, default) calls were changed to getattr(aggregate,
foo, default). This is an incorrect change, because the default of
getattr() is only used if AttributeError is raised. The original get()
call would return the default if the variable was not set, but in order
to get the default with getattr() 'in' has to be used. So now, the
getattr() calls are changed to:

foo = aggregate.foo if 'foo' in aggregate else default

Also, a comment was added for using getattr() on all aggregate fields,
explaining we can do the getattr() and not worry about metadata being
unset, because the compute API always sets it. nova.objects.base also
has a helper method to compare the primitives of two objects to check if
they're equal, so a helper method in the tests were changed over to use
that helper method.

Change-Id: Iee651704c90fcdda0938f907924a4565399601d7
Closes-Bug: #1554617

nova/api/openstack/compute/aggregates.py

@@ -208,6 +208,11 @@ class AggregateController(wsgi.Controller):
 
     def _build_aggregate_items(self, aggregate):
         keys = aggregate.obj_fields
+        # NOTE(rlrossit): Within the compute API, metadata will always be
+        # set on the aggregate object (at a minimum to {}). Because of this,
+        # we can freely use getattr() on keys in obj_extra_fields (in this
+        # case it is only ['availability_zone']) without worrying about
+        # lazy-loading an unset variable
         for key in keys:
             # NOTE(danms): Skip the uuid field because we have no microversion
             # to expose it

nova/api/openstack/compute/legacy_v2/contrib/aggregates.py

@@ -276,6 +276,11 @@ class AggregateController(object):
         return {"aggregate": _aggregate}
 
     def _build_aggregate_items(self, aggregate):
+        # NOTE(rlrossit): Within the compute API, metadata will always be
+        # set on the aggregate object (at a minimum to {}). Because of this,
+        # we can freely use getattr() on keys in obj_extra_fields (in this
+        # case it is only ['availability_zone']) without worrying about
+        # lazy-loading an unset variable
         keys = aggregate.obj_fields
         for key in keys:
             # NOTE(danms): Skip the uuid field because we have no microversion

nova/tests/unit/api/openstack/compute/test_aggregates.py

@@ -25,6 +25,7 @@ from nova.compute import api as compute_api
 from nova import context
 from nova import exception
 from nova import objects
+from nova.objects import base as obj_base
 from nova import test
 from nova.tests.unit.api.openstack import fakes
 from nova.tests.unit import matchers
@@ -429,7 +430,7 @@ class AggregateTestCaseV21(test.NoDBTestCase):
                                                            "host1"}})
 
         aggregate = _transform_aggregate_az(aggregate['aggregate'])
-        self._assert_agg_data(AGGREGATE, aggregate)
+        self._assert_agg_data(AGGREGATE, _make_agg_obj(aggregate))
 
     def test_add_host_no_admin(self):
         self.assertRaises(exception.PolicyNotAuthorized,
@@ -713,9 +714,8 @@ class AggregateTestCaseV21(test.NoDBTestCase):
         self.assertEqual(agg, marshalled_agg['aggregate'])
 
     def _assert_agg_data(self, expected, actual):
-        expected_data = expected.obj_to_primitive()['nova_object.data']
-        actual_data = expected.obj_to_primitive()['nova_object.data']
-        self.assertEqual(expected_data, actual_data)
+        self.assertTrue(obj_base.obj_equal_prims(expected, actual),
+                        "The aggregate objects were not equal")
 
 
 class AggregateTestCaseV2(AggregateTestCaseV21):