Cells: Don't allow active -> build

Adds support for passing 'expected_vm_state' in an instance_update dict
much like the current support for 'expected_task_state'.

For cells, when the API cell receives an instance update containing a
vm_state of BUILDING, raise an exception if the instance is in some
other state.  This addresses out-of-order messaging issues that can
cause an instance to appear to have a huge delay going ACTIVE.  (A
periodic task run can later heal the state, but it can take a long
while)

Fixes bug 1180283

Change-Id: I64252b30e2596812f3b84da64b1fc8681661d7f8

nova/cells/messaging.py

@@ -31,6 +31,7 @@ from nova.cells import state as cells_state
 from nova.cells import utils as cells_utils
 from nova import compute
 from nova.compute import rpcapi as compute_rpcapi
+from nova.compute import vm_states
 from nova.consoleauth import rpcapi as consoleauth_rpcapi
 from nova import context
 from nova.db import base
@@ -810,6 +811,21 @@ class _BroadcastMessageMethods(_BaseMessageMethods):
         LOG.debug(_("Got update for instance %(instance_uuid)s: "
                 "%(instance)s") % locals())
 
+        # To attempt to address out-of-order messages, do some sanity
+        # checking on the VM state.
+        expected_vm_state_map = {
+                # For updates containing 'vm_state' of 'building',
+                # only allow them to occur if the DB already says
+                # 'building' or if the vm_state is None.  None
+                # really shouldn't be possible as instances always
+                # start out in 'building' anyway.. but just in case.
+                vm_states.BUILDING: [vm_states.BUILDING, None]}
+
+        expected_vm_states = expected_vm_state_map.get(
+                instance.get('vm_state'))
+        if expected_vm_states:
+                instance['expected_vm_state'] = expected_vm_states
+
         # It's possible due to some weird condition that the instance
         # was already set as deleted... so we'll attempt to update
         # it with permissions that allows us to read deleted.

nova/db/sqlalchemy/api.py

@@ -2009,6 +2009,14 @@ def _instance_update(context, instance_uuid, values, copy_old_instance=False):
             if actual_state not in expected:
                 raise exception.UnexpectedTaskStateError(actual=actual_state,
                                                          expected=expected)
+        if "expected_vm_state" in values:
+            expected = values.pop("expected_vm_state")
+            if not isinstance(expected, (tuple, list, set)):
+                expected = (expected,)
+            actual_state = instance_ref["vm_state"]
+            if actual_state not in expected:
+                raise exception.UnexpectedVMStateError(actual=actual_state,
+                                                       expected=expected)
 
         instance_hostname = instance_ref['hostname'] or ''
         if ("hostname" in values and

nova/exception.py

@@ -1153,6 +1153,11 @@ class InstanceActionEventNotFound(NovaException):
     message = _("Event %(event)s not found for action id %(action_id)s")
 
 
+class UnexpectedVMStateError(NovaException):
+    message = _("unexpected VM state: expecting %(expected)s but "
+                "the actual state is %(actual)s")
+
+
 class CryptoCAFileNotFound(FileNotFound):
     message = _("The CA file for %(project)s could not be found")
 

nova/tests/cells/test_cells_messaging.py

@@ -19,6 +19,7 @@ from oslo.config import cfg
 
 from nova.cells import messaging
 from nova.cells import utils as cells_utils
+from nova.compute import vm_states
 from nova import context
 from nova import db
 from nova import exception
@@ -1038,6 +1039,59 @@ class CellsBroadcastMethodsTestCase(test.TestCase):
 
         self.src_msg_runner.instance_update_at_top(self.ctxt, fake_instance)
 
+    def test_instance_update_at_top_with_building_state(self):
+        fake_info_cache = {'id': 1,
+                           'instance': 'fake_instance',
+                           'other': 'moo'}
+        fake_sys_metadata = [{'id': 1,
+                              'key': 'key1',
+                              'value': 'value1'},
+                             {'id': 2,
+                              'key': 'key2',
+                              'value': 'value2'}]
+        fake_instance = {'id': 2,
+                         'uuid': 'fake_uuid',
+                         'security_groups': 'fake',
+                         'volumes': 'fake',
+                         'cell_name': 'fake',
+                         'name': 'fake',
+                         'metadata': 'fake',
+                         'info_cache': fake_info_cache,
+                         'system_metadata': fake_sys_metadata,
+                         'vm_state': vm_states.BUILDING,
+                         'other': 'meow'}
+        expected_sys_metadata = {'key1': 'value1',
+                                 'key2': 'value2'}
+        expected_info_cache = {'other': 'moo'}
+        expected_cell_name = 'api-cell!child-cell2!grandchild-cell1'
+        expected_instance = {'system_metadata': expected_sys_metadata,
+                             'cell_name': expected_cell_name,
+                             'other': 'meow',
+                             'vm_state': vm_states.BUILDING,
+                             'expected_vm_state': [vm_states.BUILDING, None],
+                             'uuid': 'fake_uuid'}
+
+        # To show these should not be called in src/mid-level cell
+        self.mox.StubOutWithMock(self.src_db_inst, 'instance_update')
+        self.mox.StubOutWithMock(self.src_db_inst,
+                                 'instance_info_cache_update')
+        self.mox.StubOutWithMock(self.mid_db_inst, 'instance_update')
+        self.mox.StubOutWithMock(self.mid_db_inst,
+                                 'instance_info_cache_update')
+
+        self.mox.StubOutWithMock(self.tgt_db_inst, 'instance_update')
+        self.mox.StubOutWithMock(self.tgt_db_inst,
+                                 'instance_info_cache_update')
+        self.tgt_db_inst.instance_update(self.ctxt, 'fake_uuid',
+                                         expected_instance,
+                                         update_cells=False)
+        self.tgt_db_inst.instance_info_cache_update(self.ctxt, 'fake_uuid',
+                                                    expected_info_cache,
+                                                    update_cells=False)
+        self.mox.ReplayAll()
+
+        self.src_msg_runner.instance_update_at_top(self.ctxt, fake_instance)
+
     def test_instance_destroy_at_top(self):
         fake_instance = {'uuid': 'fake_uuid'}
 

nova/tests/test_db_api.py

@@ -24,6 +24,7 @@ import datetime
 import types
 import uuid as stdlib_uuid
 
+import mox
 from oslo.config import cfg
 from sqlalchemy.dialects import sqlite
 from sqlalchemy import MetaData
@@ -36,6 +37,7 @@ from nova.db.sqlalchemy import api as sqlalchemy_api
 from nova import exception
 from nova.openstack.common.db.sqlalchemy import session as db_session
 from nova.openstack.common import timeutils
+from nova.openstack.common import uuidutils
 from nova import test
 from nova.tests import matchers
 from nova import utils
@@ -336,6 +338,52 @@ class DbApiTestCase(DbTestCase):
         self.assertEqual(0, len(results))
         db.instance_update(ctxt, instance['uuid'], {"task_state": None})
 
+    def test_instance_update_with_expected_vm_state(self):
+        ctxt = context.get_admin_context()
+        uuid = uuidutils.generate_uuid()
+        updates = {'expected_vm_state': 'meow',
+                   'moo': 'cow'}
+
+        class FakeInstance(dict):
+            def save(self, session=None):
+                pass
+
+        fake_instance_values = {'vm_state': 'meow',
+                                'hostname': '',
+                                'metadata': None,
+                                'system_metadata': None}
+        fake_instance = FakeInstance(fake_instance_values)
+
+        self.mox.StubOutWithMock(sqlalchemy_api, '_instance_get_by_uuid')
+        self.mox.StubOutWithMock(fake_instance, 'save')
+
+        sqlalchemy_api._instance_get_by_uuid(ctxt, uuid,
+                session=mox.IgnoreArg()).AndReturn(fake_instance)
+        fake_instance.save(session=mox.IgnoreArg())
+
+        self.mox.ReplayAll()
+
+        result = db.instance_update(ctxt, uuid, updates)
+        expected_instance = dict(fake_instance_values)
+        expected_instance['moo'] = 'cow'
+        self.assertEqual(expected_instance, result)
+
+    def test_instance_update_with_unexpected_vm_state(self):
+        ctxt = context.get_admin_context()
+        uuid = uuidutils.generate_uuid()
+        updates = {'expected_vm_state': 'meow'}
+        fake_instance = {'vm_state': 'nomatch'}
+
+        self.mox.StubOutWithMock(sqlalchemy_api, '_instance_get_by_uuid')
+
+        sqlalchemy_api._instance_get_by_uuid(ctxt, uuid,
+                session=mox.IgnoreArg()).AndReturn(fake_instance)
+
+        self.mox.ReplayAll()
+
+        self.assertRaises(exception.UnexpectedVMStateError,
+                          db.instance_update, ctxt, uuid, updates)
+
     def test_network_create_safe(self):
         ctxt = context.get_admin_context()
         values = {'host': 'localhost', 'project_id': 'project1'}