Use Instance Objects for Start/Stop

This patch makes the start and stop operations use the Instance
object instead of passing SQLA-derived dicts over RPC. Until
something more sophisticated is needed (and developed), it also
adds a nova.object.register_all() function which just triggers
imports of all the object models so that they are registered.
When adding a new object type, that register function should be
updated appropriately.

Related to bp/unified-object-model

Change-Id: I3c8d9cba07d34097a279502062906de802d19d1f

3 files changed, 153 insertions, 88 deletions

diff --git a/nova/tests/api/openstack/compute/contrib/test_server_start_stop.py b/nova/tests/api/openstack/compute/contrib/test_server_start_stop.py
index eb708a574..31b832084 100644
--- a/nova/tests/api/openstack/compute/contrib/test_server_start_stop.py
+++ b/nova/tests/api/openstack/compute/contrib/test_server_start_stop.py
@@ -17,13 +17,19 @@ import webob
 
 from nova.api.openstack.compute.contrib import server_start_stop
 from nova.compute import api as compute_api
+from nova import db
 from nova import exception
 from nova import test
 from nova.tests.api.openstack import fakes
 
 
-def fake_compute_api_get(self, context, instance_id):
-    return {'id': 1, 'uuid': instance_id}
+def fake_instance_get(self, context, instance_id):
+    result = fakes.stub_instance(id=1, uuid=instance_id)
+    result['created_at'] = None
+    result['deleted_at'] = None
+    result['updated_at'] = None
+    result['deleted'] = 0
+    return result
 
 
 def fake_start_stop_not_ready(self, context, instance):
@@ -37,7 +43,7 @@ class ServerStartStopTest(test.TestCase):
         self.controller = server_start_stop.ServerStartStopActionController()
 
     def test_start(self):
-        self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
+        self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get)
         self.mox.StubOutWithMock(compute_api.API, 'start')
         compute_api.API.start(mox.IgnoreArg(), mox.IgnoreArg())
         self.mox.ReplayAll()
@@ -47,7 +53,7 @@ class ServerStartStopTest(test.TestCase):
         self.controller._start_server(req, 'test_inst', body)
 
     def test_start_not_ready(self):
-        self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
+        self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get)
         self.stubs.Set(compute_api.API, 'start', fake_start_stop_not_ready)
         req = fakes.HTTPRequest.blank('/v2/fake/servers/test_inst/action')
         body = dict(start="")
@@ -55,7 +61,7 @@ class ServerStartStopTest(test.TestCase):
             self.controller._start_server, req, 'test_inst', body)
 
     def test_stop(self):
-        self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
+        self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get)
         self.mox.StubOutWithMock(compute_api.API, 'stop')
         compute_api.API.stop(mox.IgnoreArg(), mox.IgnoreArg())
         self.mox.ReplayAll()
@@ -65,7 +71,7 @@ class ServerStartStopTest(test.TestCase):
         self.controller._stop_server(req, 'test_inst', body)
 
     def test_stop_not_ready(self):
-        self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
+        self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get)
         self.stubs.Set(compute_api.API, 'stop', fake_start_stop_not_ready)
         req = fakes.HTTPRequest.blank('/v2/fake/servers/test_inst/action')
         body = dict(start="")
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index fc3e0126b..211838ea1 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -34,6 +34,7 @@ import nova
 from nova import compute
 from nova.compute import api as compute_api
 from nova.compute import flavors
+from nova.compute import instance_actions
 from nova.compute import manager as compute_manager
 from nova.compute import power_state
 from nova.compute import rpcapi as compute_rpcapi
@@ -48,6 +49,7 @@ from nova.image import glance
 from nova.network import api as network_api
 from nova.network import model as network_model
 from nova.network.security_group import openstack_driver
+from nova.objects import instance as instance_obj
 from nova.openstack.common import importutils
 from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
@@ -62,6 +64,7 @@ from nova import quota
 from nova import test
 from nova.tests.compute import fake_resource_tracker
 from nova.tests.db import fakes as db_fakes
+from nova.tests import fake_instance
 from nova.tests import fake_instance_actions
 from nova.tests import fake_network
 from nova.tests import fake_network_cache_model
@@ -236,6 +239,7 @@ class BaseTestCase(test.TestCase):
 
         inst = {}
         inst['vm_state'] = vm_states.ACTIVE
+        inst['task_state'] = None
         inst['image_ref'] = FAKE_IMAGE_REF
         inst['reservation_id'] = 'r-fakeres'
         inst['user_id'] = self.user_id
@@ -252,11 +256,27 @@ class BaseTestCase(test.TestCase):
         inst['architecture'] = 'x86_64'
         inst['os_type'] = 'Linux'
         inst['system_metadata'] = make_fake_sys_meta()
+        inst['locked'] = False
         inst.update(params)
         _create_service_entries(self.context.elevated(),
                 {'fake_zone': [inst['host']]})
         return db.instance_create(self.context, inst)
 
+    def _create_instance_obj(self, params=None, type_name='m1.tiny'):
+        """Create a test instance object."""
+        instance = instance_obj.Instance()
+        instance.uuid = uuidutils.generate_uuid()
+        instance.cell_name = 'api!child'
+
+        def _fake_db_create(_ctxt, inst):
+            for k, v in inst.items():
+                setattr(instance, k, v)
+            return instance
+
+        self.stubs.Set(db, 'instance_create', _fake_db_create)
+        return self._create_fake_instance(params=params,
+                type_name=type_name)
+
     def _create_instance(self, params=None, type_name='m1.tiny'):
         """Create a test instance. Returns uuid."""
         return self._create_fake_instance(params, type_name=type_name)
@@ -691,6 +711,16 @@ class ComputeTestCase(BaseTestCase):
         self.assertTrue(called['finished'])
         self.assertEqual('An unknown exception occurred.', called['message'])
 
+    def test_object_compat(self):
+        db_inst = fake_instance.fake_db_instance()
+
+        @compute_manager.object_compat
+        def test_fn(_self, context, instance):
+            self.assertTrue(isinstance(instance, instance_obj.Instance))
+            self.assertEqual(instance.uuid, db_inst['uuid'])
+
+        test_fn(None, self.context, instance=db_inst)
+
     def test_create_instance_with_img_ref_associates_config_drive(self):
         # Make sure create associates a config drive.
 
@@ -1238,7 +1268,12 @@ class ComputeTestCase(BaseTestCase):
         self.compute.run_instance(self.context, instance=instance)
         db.instance_update(self.context, instance['uuid'],
                            {"task_state": task_states.POWERING_OFF})
-        self.compute.stop_instance(self.context, instance=instance)
+        inst_uuid = instance['uuid']
+        extra = ['system_metadata', 'metadata']
+        inst_obj = instance_obj.Instance.get_by_uuid(self.context,
+                                                     inst_uuid,
+                                                     expected_attrs=extra)
+        self.compute.stop_instance(self.context, instance=inst_obj)
         self.compute.terminate_instance(self.context, instance=instance)
 
     def test_start(self):
@@ -1247,10 +1282,15 @@ class ComputeTestCase(BaseTestCase):
         self.compute.run_instance(self.context, instance=instance)
         db.instance_update(self.context, instance['uuid'],
                            {"task_state": task_states.POWERING_OFF})
-        self.compute.stop_instance(self.context, instance=instance)
-        db.instance_update(self.context, instance['uuid'],
-                           {"task_state": task_states.POWERING_ON})
-        self.compute.start_instance(self.context, instance=instance)
+        extra = ['system_metadata', 'metadata']
+        inst_uuid = instance['uuid']
+        inst_obj = instance_obj.Instance.get_by_uuid(self.context,
+                                                     inst_uuid,
+                                                     expected_attrs=extra)
+        self.compute.stop_instance(self.context, instance=inst_obj)
+        inst_obj.task_state = task_states.POWERING_ON
+        inst_obj.save(self.context)
+        self.compute.start_instance(self.context, instance=inst_obj)
         self.compute.terminate_instance(self.context, instance=instance)
 
     def test_stop_start_no_image(self):
@@ -1259,10 +1299,15 @@ class ComputeTestCase(BaseTestCase):
         self.compute.run_instance(self.context, instance=instance)
         db.instance_update(self.context, instance['uuid'],
                            {"task_state": task_states.POWERING_OFF})
-        self.compute.stop_instance(self.context, instance=instance)
-        db.instance_update(self.context, instance['uuid'],
-                           {"task_state": task_states.POWERING_ON})
-        self.compute.start_instance(self.context, instance=instance)
+        extra = ['system_metadata', 'metadata']
+        inst_uuid = instance['uuid']
+        inst_obj = instance_obj.Instance.get_by_uuid(self.context,
+                                                     inst_uuid,
+                                                     expected_attrs=extra)
+        self.compute.stop_instance(self.context, instance=inst_obj)
+        inst_obj.task_state = task_states.POWERING_ON
+        inst_obj.save(self.context)
+        self.compute.start_instance(self.context, instance=inst_obj)
         self.compute.terminate_instance(self.context, instance=instance)
 
     def test_rescue(self):
@@ -1338,11 +1383,15 @@ class ComputeTestCase(BaseTestCase):
 
         instance = jsonutils.to_primitive(self._create_fake_instance())
         self.compute.run_instance(self.context, instance=instance)
-        db.instance_update(self.context, instance['uuid'],
-                           {"task_state": task_states.POWERING_ON})
-        self.compute.start_instance(self.context, instance=instance)
+        extra = ['system_metadata', 'metadata']
+        inst_obj = instance_obj.Instance.get_by_uuid(self.context,
+                                                     instance['uuid'],
+                                                     expected_attrs=extra)
+        inst_obj.task_state = task_states.POWERING_ON
+        inst_obj.save(self.context)
+        self.compute.start_instance(self.context, instance=inst_obj)
         self.assertTrue(called['power_on'])
-        self.compute.terminate_instance(self.context, instance=instance)
+        self.compute.terminate_instance(self.context, instance=inst_obj)
 
     def test_power_off(self):
         # Ensure instance can be powered off.
@@ -1357,11 +1406,15 @@ class ComputeTestCase(BaseTestCase):
 
         instance = jsonutils.to_primitive(self._create_fake_instance())
         self.compute.run_instance(self.context, instance=instance)
-        db.instance_update(self.context, instance['uuid'],
-                           {"task_state": task_states.POWERING_OFF})
-        self.compute.stop_instance(self.context, instance=instance)
+        extra = ['system_metadata', 'metadata']
+        inst_obj = instance_obj.Instance.get_by_uuid(self.context,
+                                                     instance['uuid'],
+                                                     expected_attrs=extra)
+        inst_obj.task_state = task_states.POWERING_OFF
+        inst_obj.save(self.context)
+        self.compute.stop_instance(self.context, instance=inst_obj)
         self.assertTrue(called['power_off'])
-        self.compute.terminate_instance(self.context, instance=instance)
+        self.compute.terminate_instance(self.context, instance=inst_obj)
 
     def test_pause(self):
         # Ensure instance can be paused and unpaused.
@@ -5334,47 +5387,85 @@ class ComputeAPITestCase(BaseTestCase):
             db.instance_destroy(self.context, ref[0]['uuid'])
 
     def test_start(self):
-        instance = jsonutils.to_primitive(self._create_fake_instance())
-        instance_uuid = instance['uuid']
-        self.compute.run_instance(self.context, instance=instance)
-
-        db.instance_update(self.context, instance['uuid'],
-                           {"task_state": task_states.POWERING_OFF})
-        self.compute.stop_instance(self.context, instance=instance)
+        # Undo setUp() stubs (test_compute_cells)
+        self.stubs.UnsetAll()
+        instance = self._create_instance_obj()
+        instance.vm_state = vm_states.STOPPED
+
+        self.mox.StubOutWithMock(instance, 'save')
+        self.mox.StubOutWithMock(self.compute_api,
+                '_record_action_start')
+        self.mox.StubOutWithMock(
+                self.compute_api.compute_rpcapi,
+                'start_instance')
+
+        instance.save(expected_task_state=None)
+        self.compute_api._record_action_start(self.context,
+                instance, instance_actions.START)
+        self.compute_api.compute_rpcapi.start_instance(
+                self.context, instance)
 
-        instance = db.instance_get_by_uuid(self.context, instance_uuid)
-        self.assertEqual(instance['task_state'], None)
+        self.mox.ReplayAll()
 
         self.compute_api.start(self.context, instance)
+        self.assertEqual(task_states.POWERING_ON,
+                         instance.task_state)
 
-        instance = db.instance_get_by_uuid(self.context, instance_uuid)
-        self.assertEqual(instance['task_state'], task_states.POWERING_ON)
-
-        db.instance_destroy(self.context, instance['uuid'])
+    def test_start_invalid_state(self):
+        # Undo setUp() stubs (test_compute_cells)
+        self.stubs.UnsetAll()
+        instance = self._create_instance_obj()
+        instance.vm_state = vm_states.ACTIVE
+        self.assertRaises(exception.InstanceInvalidState,
+                          self.compute_api.start,
+                          self.context, instance)
 
     def test_start_no_host(self):
-        instance = self._create_fake_instance(params={'host': ''})
-
+        # Undo setUp() stubs (test_compute_cells)
+        self.stubs.UnsetAll()
+        instance = self._create_instance_obj()
+        instance.vm_state = vm_states.STOPPED
+        instance.host = ''
         self.assertRaises(exception.InstanceNotReady,
                           self.compute_api.start,
                           self.context, instance)
 
-        db.instance_destroy(self.context, instance['uuid'])
-
     def test_stop(self):
-        instance = jsonutils.to_primitive(self._create_fake_instance())
-        instance_uuid = instance['uuid']
-        self.compute.run_instance(self.context, instance=instance)
+        # Undo setUp() stubs (test_compute_cells)
+        self.stubs.UnsetAll()
+        instance = self._create_instance_obj()
+        instance.task_state = None
+        # Make sure this gets reset
+        instance.progress = 99
+
+        self.mox.StubOutWithMock(instance, 'save')
+        self.mox.StubOutWithMock(self.compute_api,
+                '_record_action_start')
+        self.mox.StubOutWithMock(
+                self.compute_api.compute_rpcapi,
+                'stop_instance')
+
+        instance.save(expected_task_state=None)
+        self.compute_api._record_action_start(self.context,
+                instance, instance_actions.STOP)
+        self.compute_api.compute_rpcapi.stop_instance(
+                self.context, instance, cast=True)
 
-        instance = db.instance_get_by_uuid(self.context, instance_uuid)
-        self.assertEqual(instance['task_state'], None)
+        self.mox.ReplayAll()
 
         self.compute_api.stop(self.context, instance)
+        self.assertEqual(task_states.POWERING_OFF,
+                         instance.task_state)
+        self.assertEqual(0, instance.progress)
 
-        instance = db.instance_get_by_uuid(self.context, instance_uuid)
-        self.assertEqual(instance['task_state'], task_states.POWERING_OFF)
-
-        db.instance_destroy(self.context, instance['uuid'])
+    def test_stop_invalid_state(self):
+        # Undo setUp() stubs (test_compute_cells)
+        self.stubs.UnsetAll()
+        instance = self._create_instance_obj()
+        instance.vm_state = vm_states.PAUSED
+        self.assertRaises(exception.InstanceInvalidState,
+                          self.compute_api.stop,
+                          self.context, instance)
 
     def test_stop_a_stopped_inst(self):
         instance = jsonutils.to_primitive(self._create_fake_instance(
@@ -5387,46 +5478,14 @@ class ComputeAPITestCase(BaseTestCase):
         db.instance_destroy(self.context, instance['uuid'])
 
     def test_stop_no_host(self):
-        instance = self._create_fake_instance(params={'host': ''})
-
+        # Undo setUp() stubs (test_compute_cells)
+        self.stubs.UnsetAll()
+        instance = self._create_instance_obj()
+        instance.host = ''
         self.assertRaises(exception.InstanceNotReady,
                           self.compute_api.stop,
                           self.context, instance)
 
-        db.instance_destroy(self.context, instance['uuid'])
-
-    def test_start_shutdown(self):
-        def check_state(instance_uuid, power_state_, vm_state_, task_state_):
-            instance = db.instance_get_by_uuid(self.context, instance_uuid)
-            self.assertEqual(instance['power_state'], power_state_)
-            self.assertEqual(instance['vm_state'], vm_state_)
-            self.assertEqual(instance['task_state'], task_state_)
-
-        def start_check_state(instance_uuid,
-                              power_state_, vm_state_, task_state_):
-            instance = db.instance_get_by_uuid(self.context, instance_uuid)
-            self.compute_api.start(self.context, instance)
-            check_state(instance_uuid, power_state_, vm_state_, task_state_)
-
-        instance = jsonutils.to_primitive(self._create_fake_instance())
-        self.compute.run_instance(self.context, instance=instance)
-
-        check_state(instance['uuid'], power_state.RUNNING, vm_states.ACTIVE,
-                    None)
-
-        # NOTE(yamahata): emulate compute.manager._sync_power_state() that
-        # the instance is shutdown by itself
-        db.instance_update(self.context, instance['uuid'],
-                           {'power_state': power_state.NOSTATE,
-                            'vm_state': vm_states.STOPPED})
-        check_state(instance['uuid'], power_state.NOSTATE, vm_states.STOPPED,
-                    None)
-
-        start_check_state(instance['uuid'], power_state.NOSTATE,
-                          vm_states.STOPPED, task_states.POWERING_ON)
-
-        db.instance_destroy(self.context, instance['uuid'])
-
     def test_delete(self):
         instance, instance_uuid = self._run_instance(params={
                 'host': CONF.host})
diff --git a/nova/tests/compute/test_rpcapi.py b/nova/tests/compute/test_rpcapi.py
index 2c04f8060..aaf90288a 100644
--- a/nova/tests/compute/test_rpcapi.py
+++ b/nova/tests/compute/test_rpcapi.py
@@ -354,15 +354,15 @@ class ComputeRpcAPITestCase(test.TestCase):
 
     def test_start_instance(self):
         self._test_compute_api('start_instance', 'cast',
-                instance=self.fake_instance)
+                instance=self.fake_instance, version='2.29')
 
     def test_stop_instance_cast(self):
         self._test_compute_api('stop_instance', 'cast',
-                instance=self.fake_instance)
+                instance=self.fake_instance, version='2.29')
 
     def test_stop_instance_call(self):
         self._test_compute_api('stop_instance', 'call',
-                instance=self.fake_instance)
+                instance=self.fake_instance, version='2.29')
 
     def test_suspend_instance(self):
         self._test_compute_api('suspend_instance', 'cast',