From c5a5a7f322e2b2f76dab2647af2f15f88c2abac3 Mon Sep 17 00:00:00 2001
From: Dan Smith <dansmith@redhat.com>
Date: Fri, 11 Mar 2016 12:31:01 -0800
Subject: [PATCH] Avoid lazy-loads of ec2_ids on Instance

Right now, we only create the ec2_ids for an Instance when they are first
accessed, not at instance creation time. However, we always touch them
later for building metadata, configdrive, etc, which means we end up with
an expensive lazy-load later.

This makes us always create ec2_ids at instance create time. Since it's in
instance.create(), it runs on conductor, straight to the database, avoiding
a round-trip later (assuming this instance is passed to the compute node
for create and/or that the field is specified in a later call under
expected_attrs).

Partial-Bug: #1540526
Change-Id: I29bc317f990bfa0f01b4f9615699debcc3a3c2c6
---
 nova/cells/scheduler.py                       |  3 ++
 nova/objects/instance.py                      |  7 ++++
 nova/tests/unit/cells/test_cells_scheduler.py | 33 +++++++++++++++++++
 nova/tests/unit/objects/test_instance.py      |  1 +
 4 files changed, 44 insertions(+)

diff --git a/nova/cells/scheduler.py b/nova/cells/scheduler.py
index b45b39ce99..8e27498c30 100644
--- a/nova/cells/scheduler.py
+++ b/nova/cells/scheduler.py
@@ -81,6 +81,9 @@ class CellsScheduler(base.Base):
         # is sending proper instance objects.
         instance_values.pop('pci_requests', None)
 
+        # FIXME(danms): Same for ec2_ids
+        instance_values.pop('ec2_ids', None)
+
         instances = []
         num_instances = len(instance_uuids)
         security_groups = (
diff --git a/nova/objects/instance.py b/nova/objects/instance.py
index 819108dffa..e759fb582b 100644
--- a/nova/objects/instance.py
+++ b/nova/objects/instance.py
@@ -464,6 +464,13 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
         db_inst = db.instance_create(self._context, updates)
         self._from_db_object(self._context, self, db_inst, expected_attrs)
 
+        # NOTE(danms): The EC2 ids are created on their first load. In order
+        # to avoid them being missing and having to be loaded later, we
+        # load them once here on create now that the instance record is
+        # created.
+        self._load_ec2_ids()
+        self.obj_reset_changes(['ec2_ids'])
+
     @base.remotable
     def destroy(self):
         if not self.obj_attr_is_set('id'):
diff --git a/nova/tests/unit/cells/test_cells_scheduler.py b/nova/tests/unit/cells/test_cells_scheduler.py
index b534f70177..2f900300d5 100644
--- a/nova/tests/unit/cells/test_cells_scheduler.py
+++ b/nova/tests/unit/cells/test_cells_scheduler.py
@@ -18,6 +18,7 @@ Tests For CellsScheduler
 import copy
 import time
 
+import mock
 from oslo_utils import uuidutils
 
 from nova import block_device
@@ -33,6 +34,7 @@ from nova.scheduler import utils as scheduler_utils
 from nova import test
 from nova.tests.unit.cells import fakes
 from nova.tests.unit import fake_block_device
+from nova.tests import uuidsentinel
 from nova import utils
 
 CONF = nova.conf.CONF
@@ -139,6 +141,37 @@ class CellsSchedulerTestCase(test.TestCase):
                              instance['display_name'])
             self.assertEqual('fake_image_ref', instance['image_ref'])
 
+    @mock.patch('nova.objects.Instance.update')
+    def test_create_instances_here_pops_problematic_properties(self,
+                                                               mock_update):
+        values = {
+            'uuid': uuidsentinel.instance,
+            'metadata': [],
+            'id': 1,
+            'name': 'foo',
+            'info_cache': 'bar',
+            'security_groups': 'not secure',
+            'flavor': 'chocolate',
+            'pci_requests': 'no thanks',
+            'ec2_ids': 'prime',
+        }
+
+        @mock.patch.object(self.scheduler.compute_api,
+                           'create_db_entry_for_new_instance')
+        def test(mock_create_db):
+            self.scheduler._create_instances_here(
+                self.ctxt, [uuidsentinel.instance], values,
+                objects.Flavor(), 'foo', [], [])
+
+        test()
+
+        # NOTE(danms): Make sure that only the expected properties
+        # are applied to the instance object. The complex ones that
+        # would have been mangled over RPC should be removed.
+        mock_update.assert_called_once_with(
+            {'uuid': uuidsentinel.instance,
+             'metadata': {}})
+
     def test_build_instances_selects_child_cell(self):
         # Make sure there's no capacity info so we're sure to
         # select a child cell
diff --git a/nova/tests/unit/objects/test_instance.py b/nova/tests/unit/objects/test_instance.py
index 530040eb10..a0ffd9deb1 100644
--- a/nova/tests/unit/objects/test_instance.py
+++ b/nova/tests/unit/objects/test_instance.py
@@ -920,6 +920,7 @@ class _TestInstanceObject(object):
         inst = objects.Instance(context=self.context)
         inst.create()
         self.assertEqual(self.fake_instance['id'], inst.id)
+        self.assertIsNotNone(inst.ec2_ids)
 
     def test_create_with_values(self):
         inst1 = objects.Instance(context=self.context,