Merge "Add user_id field to InstanceMapping"

Author: Zuul

nova/objects/instance_mapping.py

@@ -10,6 +10,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_log import log as logging
 from oslo_utils import versionutils
 from sqlalchemy.orm import joinedload
 from sqlalchemy.sql import false
@@ -19,34 +20,51 @@
 from nova.db.sqlalchemy import api as db_api
 from nova.db.sqlalchemy import api_models
 from nova import exception
+from nova.i18n import _
 from nova import objects
 from nova.objects import base
 from nova.objects import cell_mapping
 from nova.objects import fields
 
 
+LOG = logging.getLogger(__name__)
+
+
 @base.NovaObjectRegistry.register
 class InstanceMapping(base.NovaTimestampObject, base.NovaObject):
     # Version 1.0: Initial version
     # Version 1.1: Add queued_for_delete
-    VERSION = '1.1'
+    # Version 1.2: Add user_id
+    VERSION = '1.2'
 
     fields = {
         'id': fields.IntegerField(read_only=True),
         'instance_uuid': fields.UUIDField(),
         'cell_mapping': fields.ObjectField('CellMapping', nullable=True),
         'project_id': fields.StringField(),
+        'user_id': fields.StringField(),
         'queued_for_delete': fields.BooleanField(default=False),
         }
 
     def obj_make_compatible(self, primitive, target_version):
         super(InstanceMapping, self).obj_make_compatible(primitive,
                                                          target_version)
         target_version = versionutils.convert_version_to_tuple(target_version)
+        if target_version < (1, 2) and 'user_id' in primitive:
+            del primitive['user_id']
         if target_version < (1, 1):
             if 'queued_for_delete' in primitive:
                 del primitive['queued_for_delete']
 
+    def obj_load_attr(self, attrname):
+        if attrname == 'user_id':
+            LOG.error('The unset user_id attribute of an unmigrated instance '
+                      'mapping should not be accessed.')
+            raise exception.ObjectActionError(
+                action='obj_load_attr',
+                reason=_('attribute user_id is not lazy-loadable'))
+        super(InstanceMapping, self).obj_load_attr(attrname)
+
     def _update_with_cell_id(self, updates):
         cell_mapping_obj = updates.pop("cell_mapping", None)
         if cell_mapping_obj:
@@ -63,6 +81,11 @@ def _from_db_object(context, instance_mapping, db_instance_mapping):
                 if db_value:
                     db_value = cell_mapping.CellMapping._from_db_object(
                         context, cell_mapping.CellMapping(), db_value)
+            if key == 'user_id' and db_value is None:
+                # NOTE(melwitt): If user_id is NULL, we can't set the field
+                # because it's non-nullable. We don't plan for any code to read
+                # the user_id field at this time, so skip setting it.
+                continue
             setattr(instance_mapping, key, db_value)
         instance_mapping.obj_reset_changes()
         instance_mapping._context = context

nova/tests/functional/db/test_instance_mapping.py

@@ -206,6 +206,16 @@ def test_populate_queued_for_delete(self):
         self.assertEqual(0, done)
         self.assertEqual(0, total)
 
+    def test_user_id_not_set_if_null_from_db(self):
+        # Create an instance mapping with user_id=None.
+        db_mapping = create_mapping()
+        self.assertIsNone(db_mapping['user_id'])
+        # Get the mapping to run convert from db object to versioned object.
+        im = instance_mapping.InstanceMapping.get_by_instance_uuid(
+            self.context, db_mapping['instance_uuid'])
+        # Verify the user_id is not set.
+        self.assertNotIn('user_id', im)
+
 
 class InstanceMappingListTestCase(test.NoDBTestCase):
     USES_DB_SELF = True

nova/tests/unit/objects/test_instance_mapping.py

@@ -13,6 +13,7 @@
 import mock
 from oslo_utils import uuidutils
 
+from nova import exception
 from nova import objects
 from nova.objects import instance_mapping
 from nova.tests.unit.objects import test_cell_mapping
@@ -25,6 +26,7 @@ def get_db_mapping(**updates):
             'instance_uuid': uuidutils.generate_uuid(),
             'cell_id': None,
             'project_id': 'fake-project',
+            'user_id': 'fake-user',
             'created_at': None,
             'updated_at': None,
             'queued_for_delete': False,
@@ -77,13 +79,15 @@ def test_create(self, create_in_db):
         mapping_obj.cell_mapping = objects.CellMapping(self.context,
                 id=db_mapping['cell_mapping']['id'])
         mapping_obj.project_id = db_mapping['project_id']
+        mapping_obj.user_id = db_mapping['user_id']
 
         mapping_obj.create()
         create_in_db.assert_called_once_with(self.context,
                 {'instance_uuid': uuid,
                  'queued_for_delete': False,
                  'cell_id': db_mapping['cell_mapping']['id'],
-                 'project_id': db_mapping['project_id']})
+                 'project_id': db_mapping['project_id'],
+                 'user_id': db_mapping['user_id']})
         self.compare_obj(mapping_obj, db_mapping,
                          subs={'cell_mapping': 'cell_id'},
                          comparators={
@@ -98,12 +102,14 @@ def test_create_cell_mapping_none(self, create_in_db):
         mapping_obj.instance_uuid = uuid
         mapping_obj.cell_mapping = None
         mapping_obj.project_id = db_mapping['project_id']
+        mapping_obj.user_id = db_mapping['user_id']
 
         mapping_obj.create()
         create_in_db.assert_called_once_with(self.context,
                 {'instance_uuid': uuid,
                  'queued_for_delete': False,
-                 'project_id': db_mapping['project_id']})
+                 'project_id': db_mapping['project_id'],
+                 'user_id': db_mapping['user_id']})
         self.compare_obj(mapping_obj, db_mapping,
                          subs={'cell_mapping': 'cell_id'})
         self.assertIsNone(mapping_obj.cell_mapping)
@@ -116,13 +122,15 @@ def test_create_cell_mapping_with_qfd_true(self, create_in_db):
         mapping_obj.instance_uuid = db_mapping['instance_uuid']
         mapping_obj.cell_mapping = None
         mapping_obj.project_id = db_mapping['project_id']
+        mapping_obj.user_id = db_mapping['user_id']
         mapping_obj.queued_for_delete = True
 
         mapping_obj.create()
         create_in_db.assert_called_once_with(self.context,
                 {'instance_uuid': db_mapping['instance_uuid'],
                  'queued_for_delete': True,
-                 'project_id': db_mapping['project_id']})
+                 'project_id': db_mapping['project_id'],
+                 'user_id': db_mapping['user_id']})
 
     @mock.patch.object(instance_mapping.InstanceMapping, '_save_in_db')
     def test_save(self, save_in_db):
@@ -162,13 +170,32 @@ def test_obj_make_compatible(self):
         im_obj = instance_mapping.InstanceMapping(context=self.context)
         fake_im_obj = instance_mapping.InstanceMapping(context=self.context,
                                                        instance_uuid=uuid,
-                                                       queued_for_delete=False)
+                                                       queued_for_delete=False,
+                                                       user_id='fake-user')
+        obj_primitive = fake_im_obj.obj_to_primitive('1.1')
+        obj = im_obj.obj_from_primitive(obj_primitive)
+        self.assertIn('queued_for_delete', obj)
+        self.assertNotIn('user_id', obj)
+
         obj_primitive = fake_im_obj.obj_to_primitive('1.0')
         obj = im_obj.obj_from_primitive(obj_primitive)
         self.assertIn('instance_uuid', obj)
         self.assertEqual(uuid, obj.instance_uuid)
         self.assertNotIn('queued_for_delete', obj)
 
+    @mock.patch('nova.objects.instance_mapping.LOG.error')
+    def test_obj_load_attr(self, mock_log):
+        im_obj = instance_mapping.InstanceMapping()
+        # Access of unset user_id should have special handling
+        self.assertRaises(exception.ObjectActionError, im_obj.obj_load_attr,
+                          'user_id')
+        msg = ('The unset user_id attribute of an unmigrated instance mapping '
+               'should not be accessed.')
+        mock_log.assert_called_once_with(msg)
+        # Access of any other unset attribute should fall back to base class
+        self.assertRaises(NotImplementedError, im_obj.obj_load_attr,
+                          'project_id')
+
 
 class TestInstanceMappingObject(test_objects._LocalTest,
                                 _TestInstanceMappingObject):

nova/tests/unit/objects/test_objects.py

@@ -1107,7 +1107,7 @@ def obj_name(cls):
     'InstanceGroupList': '1.8-90f8f1a445552bb3bbc9fa1ae7da27d4',
     'InstanceInfoCache': '1.5-cd8b96fefe0fc8d4d337243ba0bf0e1e',
     'InstanceList': '2.4-d2c5723da8c1d08e07cb00160edfd292',
-    'InstanceMapping': '1.1-808df83f25987578ed3b187e16b47405',
+    'InstanceMapping': '1.2-3bd375e65c8eb9c45498d2f87b882e03',
     'InstanceMappingList': '1.2-ee638619aa3d8a82a59c0c83bfa64d78',
     'InstanceNUMACell': '1.4-7c1eb9a198dee076b4de0840e45f4f55',
     'InstanceNUMATopology': '1.3-ec0030cb0402a49c96da7051c037082a',