Merge "Make compute manager use nova-conductor for instance_update"

12 files changed, 81 insertions, 30 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index dae87de92..9a17e808c 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -51,6 +51,7 @@ from nova.compute import rpcapi as compute_rpcapi
 from nova.compute import task_states
 from nova.compute import utils as compute_utils
 from nova.compute import vm_states
+from nova import conductor
 import nova.context
 from nova import exception
 from nova.image import glance
@@ -258,9 +259,9 @@ class ComputeVirtAPI(virtapi.VirtAPI):
         self._compute = compute
 
     def instance_update(self, context, instance_uuid, updates):
-        return self._compute.db.instance_update_and_get_original(context,
-                                                                 instance_uuid,
-                                                                 updates)
+        return self._compute._instance_update(context,
+                                              instance_uuid,
+                                              **updates)
 
     def instance_get_by_uuid(self, context, instance_uuid):
         return self._compute.db.instance_get_by_uuid(context, instance_uuid)
@@ -322,6 +323,7 @@ class ComputeManager(manager.SchedulerDependentManager):
         self.compute_api = compute.API()
         self.compute_rpcapi = compute_rpcapi.ComputeAPI()
         self.scheduler_rpcapi = scheduler_rpcapi.SchedulerAPI()
+        self.conductor_api = conductor.API()
 
         super(ComputeManager, self).__init__(service_name="compute",
                                              *args, **kwargs)
@@ -340,11 +342,11 @@ class ComputeManager(manager.SchedulerDependentManager):
     def _instance_update(self, context, instance_uuid, **kwargs):
         """Update an instance in the database using kwargs as value."""
 
-        (old_ref, instance_ref) = self.db.instance_update_and_get_original(
-                context, instance_uuid, kwargs)
+        instance_ref = self.conductor_api.instance_update(context,
+                                                          instance_uuid,
+                                                          **kwargs)
         rt = self._get_resource_tracker(instance_ref.get('node'))
         rt.update_usage(context, instance_ref)
-        notifications.send_update(context, old_ref, instance_ref)
 
         return instance_ref
 
@@ -747,8 +749,7 @@ class ComputeManager(manager.SchedulerDependentManager):
                     if ip['version'] == 6:
                         update_info['access_ip_v6'] = ip['address']
         if update_info:
-            self.db.instance_update(context, instance['uuid'], update_info)
-            notifications.send_update(context, instance, instance)
+            self._instance_update(context, instance['uuid'], **update_info)
 
     def _check_instance_not_already_created(self, context, instance):
         """Ensure an instance with the same name is not already present."""
diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py
index 3ffe82645..18fc1149b 100644
--- a/nova/conductor/manager.py
+++ b/nova/conductor/manager.py
@@ -26,7 +26,9 @@ allowed_updates = ['task_state', 'vm_state', 'expected_task_state',
                    'power_state', 'access_ip_v4', 'access_ip_v6',
                    'launched_at', 'terminated_at', 'host',
                    'memory_mb', 'vcpus', 'root_gb', 'ephemeral_gb',
-                   'instance_type_id',
+                   'instance_type_id', 'root_device_name', 'host',
+                   'progress', 'vm_mode', 'default_ephemeral_device',
+                   'default_swap_device', 'root_device_name',
                    ]
 
 
diff --git a/nova/tests/api/ec2/test_cinder_cloud.py b/nova/tests/api/ec2/test_cinder_cloud.py
index 5c04321c7..1469ba64c 100644
--- a/nova/tests/api/ec2/test_cinder_cloud.py
+++ b/nova/tests/api/ec2/test_cinder_cloud.py
@@ -119,6 +119,9 @@ class CinderCloudTestCase(test.TestCase):
         self.cloud = cloud.CloudController()
         self.flags(scheduler_driver='nova.scheduler.chance.ChanceScheduler')
 
+        # Short-circuit the conductor service
+        self.flags(use_local=True, group='conductor')
+
         # set up services
         self.compute = self.start_service('compute')
         self.scheduler = self.start_service('scheduler')
diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py
index 98606b194..34142b0ec 100644
--- a/nova/tests/api/ec2/test_cloud.py
+++ b/nova/tests/api/ec2/test_cloud.py
@@ -131,6 +131,9 @@ class CloudTestCase(test.TestCase):
         self.cloud = cloud.CloudController()
         self.flags(scheduler_driver='nova.scheduler.chance.ChanceScheduler')
 
+        # Short-circuit the conductor service
+        self.flags(use_local=True, group='conductor')
+
         # set up services
         self.compute = self.start_service('compute')
         self.scheduler = self.start_service('scheduler')
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index a7e06cf92..32cd3ca3e 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -108,6 +108,7 @@ class BaseTestCase(test.TestCase):
                    notification_driver=[test_notifier.__name__],
                    network_manager='nova.network.manager.FlatManager')
         fake.set_nodes([NODENAME])
+        self.flags(use_local=True, group='conductor')
         self.compute = importutils.import_object(CONF.compute_manager)
 
         # override tracker with a version that doesn't need the database:
@@ -1416,7 +1417,7 @@ class ComputeTestCase(BaseTestCase):
         self.assertTrue('created_at' in payload)
         self.assertTrue('launched_at' in payload)
         self.assertTrue('deleted_at' in payload)
-        self.assertEqual(payload['deleted_at'], str(cur_time))
+        self.assertEqual(payload['deleted_at'], timeutils.strtime(cur_time))
         image_ref_url = utils.generate_image_url(FAKE_IMAGE_REF)
         self.assertEquals(payload['image_ref_url'], image_ref_url)
 
@@ -1764,7 +1765,7 @@ class ComputeTestCase(BaseTestCase):
         self.assertTrue('display_name' in payload)
         self.assertTrue('created_at' in payload)
         self.assertTrue('launched_at' in payload)
-        self.assertEqual(payload['launched_at'], str(cur_time))
+        self.assertEqual(payload['launched_at'], timeutils.strtime(cur_time))
         self.assertEquals(payload['image_ref_url'], new_image_ref_url)
         self.compute.terminate_instance(self.context,
                 instance=jsonutils.to_primitive(inst_ref))
@@ -1815,7 +1816,7 @@ class ComputeTestCase(BaseTestCase):
         self.assertTrue('display_name' in payload)
         self.assertTrue('created_at' in payload)
         self.assertTrue('launched_at' in payload)
-        self.assertEqual(payload['launched_at'], str(cur_time))
+        self.assertEqual(payload['launched_at'], timeutils.strtime(cur_time))
         image_ref_url = utils.generate_image_url(FAKE_IMAGE_REF)
         self.assertEquals(payload['image_ref_url'], image_ref_url)
         self.compute.terminate_instance(self.context,
@@ -2287,6 +2288,12 @@ class ComputeTestCase(BaseTestCase):
 
         self.stubs.Set(cinder.API, 'get', fake_volume_get)
 
+        def fake_instance_update(context, instance_uuid, **updates):
+            return db.instance_update_and_get_original(context, instance_uuid,
+                                                       updates)
+        self.stubs.Set(self.compute, '_instance_update',
+                       fake_instance_update)
+
         # creating mocks
         self.mox.StubOutWithMock(rpc, 'call')
 
diff --git a/nova/tests/compute/test_compute_utils.py b/nova/tests/compute/test_compute_utils.py
index 8a732be60..3c725e4d1 100644
--- a/nova/tests/compute/test_compute_utils.py
+++ b/nova/tests/compute/test_compute_utils.py
@@ -215,6 +215,7 @@ class UsageInfoTestCase(test.TestCase):
         self.stubs.Set(network_api.API, 'get_instance_nw_info',
                        fake_get_nw_info)
 
+        self.flags(use_local=True, group='conductor')
         self.flags(compute_driver='nova.virt.fake.FakeDriver',
                    notification_driver=[test_notifier.__name__],
                    network_manager='nova.network.manager.FlatManager')
diff --git a/nova/tests/conductor/test_conductor.py b/nova/tests/conductor/test_conductor.py
index fbf7d0325..e3138fd34 100644
--- a/nova/tests/conductor/test_conductor.py
+++ b/nova/tests/conductor/test_conductor.py
@@ -22,6 +22,7 @@ from nova.conductor import manager as conductor_manager
 from nova.conductor import rpcapi as conductor_rpcapi
 from nova import context
 from nova import db
+from nova.db.sqlalchemy import models
 from nova import notifications
 from nova import test
 
@@ -131,3 +132,30 @@ class ConductorImportTest(test.TestCase):
         self.flags(use_local=False, group='conductor')
         self.assertTrue(isinstance(conductor.API(),
                                    conductor_api.API))
+
+
+class ConductorPolicyTest(test.TestCase):
+    def test_all_allowed_keys(self):
+
+        def fake_db_instance_update(self, *args, **kwargs):
+            return None, None
+        self.stubs.Set(db, 'instance_update_and_get_original',
+                       fake_db_instance_update)
+
+        ctxt = context.RequestContext('fake-user', 'fake-project')
+        conductor = conductor_api.LocalAPI()
+        updates = {}
+        for key in conductor_manager.allowed_updates:
+            updates[key] = 'foo'
+        conductor.instance_update(ctxt, 'fake-instance', **updates)
+
+    def test_allowed_keys_are_real(self):
+        instance = models.Instance()
+        keys = list(conductor_manager.allowed_updates)
+
+        # NOTE(danms): expected_task_state is a parameter that gets
+        # passed to the db layer, but is not actually an instance attribute
+        del keys[keys.index('expected_task_state')]
+
+        for key in keys:
+            self.assertTrue(hasattr(instance, key))
diff --git a/nova/tests/integrated/api_samples/os-hosts/hosts-list-resp.json.tpl b/nova/tests/integrated/api_samples/os-hosts/hosts-list-resp.json.tpl
index 30444099d..555901b2e 100644
--- a/nova/tests/integrated/api_samples/os-hosts/hosts-list-resp.json.tpl
+++ b/nova/tests/integrated/api_samples/os-hosts/hosts-list-resp.json.tpl
@@ -19,6 +19,11 @@
             "host_name": "%(host_name)s",
             "service": "scheduler",
             "zone": "nova"
+        },
+        {
+	    "host_name": "%(host_name)s",
+	    "service": "conductor",
+	    "zone": "nova"
         }
     ]
 }
diff --git a/nova/tests/integrated/api_samples/os-hosts/hosts-list-resp.xml.tpl b/nova/tests/integrated/api_samples/os-hosts/hosts-list-resp.xml.tpl
index 4fc35a54b..25ef5a299 100644
--- a/nova/tests/integrated/api_samples/os-hosts/hosts-list-resp.xml.tpl
+++ b/nova/tests/integrated/api_samples/os-hosts/hosts-list-resp.xml.tpl
@@ -4,4 +4,5 @@
   <host host_name="%(host_name)s" service="cert"/>
   <host host_name="%(host_name)s" service="network"/>
   <host host_name="%(host_name)s" service="scheduler"/>
+  <host host_name="%(host_name)s" service="conductor"/>
 </hosts>
diff --git a/nova/tests/integrated/integrated_helpers.py b/nova/tests/integrated/integrated_helpers.py
index 825881137..7c316aaa8 100644
--- a/nova/tests/integrated/integrated_helpers.py
+++ b/nova/tests/integrated/integrated_helpers.py
@@ -77,6 +77,8 @@ class _IntegratedTestBase(test.TestCase):
         self.scheduler = self.start_service('cert')
         self.network = self.start_service('network')
         self.scheduler = self.start_service('scheduler')
+        self.conductor = self.start_service(
+            'conductor', manager='nova.conductor.manager.ConductorManager')
 
         self._start_api_service()
 
diff --git a/nova/tests/integrated/test_servers.py b/nova/tests/integrated/test_servers.py
index d971af4f4..728fc3386 100644
--- a/nova/tests/integrated/test_servers.py
+++ b/nova/tests/integrated/test_servers.py
@@ -29,6 +29,10 @@ LOG = logging.getLogger(__name__)
 
 
 class ServersTest(integrated_helpers._IntegratedTestBase):
+    def setUp(self):
+        super(ServersTest, self).setUp()
+        self.conductor = self.start_service(
+            'conductor', manager='nova.conductor.manager.ConductorManager')
 
     def _wait_for_state_change(self, server, from_status):
         for i in xrange(0, 50):
diff --git a/nova/virt/xenapi/host.py b/nova/virt/xenapi/host.py
index 8057f0542..e893abf4f 100644
--- a/nova/virt/xenapi/host.py
+++ b/nova/virt/xenapi/host.py
@@ -85,33 +85,27 @@ class Host(object):
 
                     dest = _host_find(ctxt, self._session, aggregate[0],
                                       host_ref)
-                    (old_ref, new_ref) = self._virtapi.instance_update(
-                                    ctxt,
-                                    instance['uuid'],
-                                    {'host': dest,
-                                     'task_state': task_states.MIGRATING})
-                    notifications.send_update(ctxt, old_ref, new_ref)
+                    self._virtapi.instance_update(
+                        ctxt, instance['uuid'],
+                        {'host': dest,
+                         'task_state': task_states.MIGRATING})
 
                     self._session.call_xenapi('VM.pool_migrate',
                                               vm_ref, host_ref, {})
                     migrations_counter = migrations_counter + 1
 
-                    (old_ref, new_ref) = self._virtapi.instance_update(
-                                ctxt,
-                                instance['uuid'],
-                                {'vm_state': vm_states.ACTIVE})
-                    notifications.send_update(ctxt, old_ref, new_ref)
+                    self._virtapi.instance_update(
+                        ctxt, instance['uuid'],
+                        {'vm_state': vm_states.ACTIVE})
 
                     break
                 except self._session.XenAPI.Failure:
                     LOG.exception(_('Unable to migrate VM %(vm_ref)s'
-                                  'from %(host)s') % locals())
-                    (old_ref, new_ref) = self._virtapi.instance_update(
-                                ctxt,
-                                instance['uuid'],
-                                {'host': host,
-                                 'vm_state': vm_states.ACTIVE})
-                    notifications.send_update(ctxt, old_ref, new_ref)
+                                    'from %(host)s') % locals())
+                    self._virtapi.instance_update(
+                        ctxt, instance['uuid'],
+                        {'host': host,
+                         'vm_state': vm_states.ACTIVE})
 
         if vm_counter == migrations_counter:
             return 'on_maintenance'