Merge "Remove 'instance_update_from_api'"
This commit is contained in:
Zuul
Gerrit Code Review
commit
38cb0a353c
|
|
@ -426,14 +426,6 @@ class CellsManager(manager.Manager):
|
|||
migrations.extend(response.value_or_raise().objects)
|
||||
return objects.MigrationList(objects=migrations)
|
||||
|
||||
def instance_update_from_api(self, ctxt, instance, expected_vm_state,
|
||||
expected_task_state, admin_state_reset):
|
||||
"""Update an instance in its cell."""
|
||||
self.msg_runner.instance_update_from_api(ctxt, instance,
|
||||
expected_vm_state,
|
||||
expected_task_state,
|
||||
admin_state_reset)
|
||||
|
||||
def start_instance(self, ctxt, instance):
|
||||
"""Start an instance in its cell."""
|
||||
self.msg_runner.start_instance(ctxt, instance)
|
||||
|
|
|
|||
|
|
@ -768,23 +768,6 @@ class _TargetedMessageMethods(_BaseMessageMethods):
|
|||
def get_migrations(self, message, filters):
|
||||
return self.compute_api.get_migrations(message.ctxt, filters)
|
||||
|
||||
def instance_update_from_api(self, message, instance,
|
||||
expected_vm_state,
|
||||
expected_task_state,
|
||||
admin_state_reset):
|
||||
"""Update an instance in this cell."""
|
||||
if not admin_state_reset:
|
||||
# NOTE(comstud): We don't want to nuke this cell's view
|
||||
# of vm_state and task_state unless it's a forced reset
|
||||
# via admin API.
|
||||
instance.obj_reset_changes(['vm_state', 'task_state'])
|
||||
# NOTE(alaski): A cell should be authoritative for its system_metadata
|
||||
# and metadata so we don't want to sync it down from the api.
|
||||
instance.obj_reset_changes(['metadata', 'system_metadata'])
|
||||
with instance.skip_cells_sync():
|
||||
instance.save(expected_vm_state=expected_vm_state,
|
||||
expected_task_state=expected_task_state)
|
||||
|
||||
def _call_compute_api_with_obj(self, ctxt, instance, method, *args,
|
||||
**kwargs):
|
||||
try:
|
||||
|
|
@ -1475,24 +1458,6 @@ class MessageRunner(object):
|
|||
need_response=need_response)
|
||||
return message.process()
|
||||
|
||||
def instance_update_from_api(self, ctxt, instance,
|
||||
expected_vm_state, expected_task_state,
|
||||
admin_state_reset):
|
||||
"""Update an instance object in its cell."""
|
||||
cell_name = instance.cell_name
|
||||
if not cell_name:
|
||||
LOG.warning("No cell_name for instance update from API",
|
||||
instance=instance)
|
||||
return
|
||||
method_kwargs = {'instance': instance,
|
||||
'expected_vm_state': expected_vm_state,
|
||||
'expected_task_state': expected_task_state,
|
||||
'admin_state_reset': admin_state_reset}
|
||||
message = _TargetedMessage(self, ctxt, 'instance_update_from_api',
|
||||
method_kwargs, 'down',
|
||||
cell_name)
|
||||
message.process()
|
||||
|
||||
def start_instance(self, ctxt, instance):
|
||||
"""Start an instance in its cell."""
|
||||
self._instance_action(ctxt, instance, 'start_instance')
|
||||
|
|
|
|||
|
|
@ -382,19 +382,6 @@ class CellsAPI(object):
|
|||
cctxt = self.client.prepare(version='1.11')
|
||||
return cctxt.call(ctxt, 'get_migrations', filters=filters)
|
||||
|
||||
def instance_update_from_api(self, ctxt, instance, expected_vm_state,
|
||||
expected_task_state, admin_state_reset):
|
||||
"""Update an instance in its cell.
|
||||
|
||||
This method takes a new-world instance object.
|
||||
"""
|
||||
cctxt = self.client.prepare(version='1.16')
|
||||
cctxt.cast(ctxt, 'instance_update_from_api',
|
||||
instance=instance,
|
||||
expected_vm_state=expected_vm_state,
|
||||
expected_task_state=expected_task_state,
|
||||
admin_state_reset=admin_state_reset)
|
||||
|
||||
def start_instance(self, ctxt, instance):
|
||||
"""Start an instance in its cell.
|
||||
|
||||
|
|
|
|||
|
|
@ -29,10 +29,6 @@ from nova.objects import base as obj_base
|
|||
# Separator used between cell names for the 'full cell name' and routing
|
||||
# path
|
||||
PATH_CELL_SEP = '!'
|
||||
# Flag prepended to a cell name to indicate data shouldn't be synced during
|
||||
# an instance save. There are no illegal chars in a cell name so using the
|
||||
# meaningful PATH_CELL_SEP in an invalid way will need to suffice.
|
||||
BLOCK_SYNC_FLAG = '!!'
|
||||
# Separator used between cell name and item
|
||||
CELL_ITEM_SEP = '@'
|
||||
|
||||
|
|
|
|||
|
|
@ -25,9 +25,6 @@ from sqlalchemy.sql import func
|
|||
from sqlalchemy.sql import null
|
||||
|
||||
from nova import availability_zones as avail_zone
|
||||
from nova.cells import opts as cells_opts
|
||||
from nova.cells import rpcapi as cells_rpcapi
|
||||
from nova.cells import utils as cells_utils
|
||||
from nova.compute import task_states
|
||||
from nova.compute import vm_states
|
||||
from nova.db import api as db
|
||||
|
|
@ -185,6 +182,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
|
|||
'shutdown_terminate': fields.BooleanField(default=False),
|
||||
'disable_terminate': fields.BooleanField(default=False),
|
||||
|
||||
# TODO(stephenfin): Remove this in version 3.0 of the object
|
||||
'cell_name': fields.StringField(nullable=True),
|
||||
|
||||
'metadata': fields.DictOfStringsField(),
|
||||
|
|
@ -720,6 +718,8 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
|
|||
value = jsonutils.dumps(obj.obj_to_primitive())
|
||||
self._extra_values_to_save[field] = value
|
||||
|
||||
# TODO(stephenfin): Remove the 'admin_state_reset' field in version 3.0 of
|
||||
# the object
|
||||
@base.remotable
|
||||
def save(self, expected_vm_state=None,
|
||||
expected_task_state=None, admin_state_reset=False):
|
||||
|
|
@ -737,35 +737,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
|
|||
:param admin_state_reset: True if admin API is forcing setting
|
||||
of task_state/vm_state
|
||||
"""
|
||||
# Store this on the class because _cell_name_blocks_sync is useless
|
||||
# after the db update call below.
|
||||
self._sync_cells = not self._cell_name_blocks_sync()
|
||||
|
||||
context = self._context
|
||||
cell_type = cells_opts.get_cell_type()
|
||||
|
||||
if cell_type is not None:
|
||||
# NOTE(comstud): We need to stash a copy of ourselves
|
||||
# before any updates are applied. When we call the save
|
||||
# methods on nested objects, we will lose any changes to
|
||||
# them. But we need to make sure child cells can tell
|
||||
# what is changed.
|
||||
#
|
||||
# We also need to nuke any updates to vm_state and task_state
|
||||
# unless admin_state_reset is True. compute cells are
|
||||
# authoritative for their view of vm_state and task_state.
|
||||
stale_instance = self.obj_clone()
|
||||
|
||||
cells_update_from_api = (cell_type == 'api' and self.cell_name and
|
||||
self._sync_cells)
|
||||
|
||||
if cells_update_from_api:
|
||||
def _handle_cell_update_from_api():
|
||||
cells_api = cells_rpcapi.CellsAPI()
|
||||
cells_api.instance_update_from_api(context, stale_instance,
|
||||
expected_vm_state,
|
||||
expected_task_state,
|
||||
admin_state_reset)
|
||||
|
||||
self._extra_values_to_save = {}
|
||||
updates = {}
|
||||
|
|
@ -794,20 +766,13 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
|
|||
# as a result of bug.
|
||||
raise exception.InstanceNotFound(instance_id=self.uuid)
|
||||
elif field in changes:
|
||||
if (field == 'cell_name' and self[field] is not None and
|
||||
self[field].startswith(cells_utils.BLOCK_SYNC_FLAG)):
|
||||
updates[field] = self[field].replace(
|
||||
cells_utils.BLOCK_SYNC_FLAG, '', 1)
|
||||
else:
|
||||
updates[field] = self[field]
|
||||
updates[field] = self[field]
|
||||
|
||||
if self._extra_values_to_save:
|
||||
db.instance_extra_update_by_uuid(context, self.uuid,
|
||||
self._extra_values_to_save)
|
||||
|
||||
if not updates:
|
||||
if cells_update_from_api:
|
||||
_handle_cell_update_from_api()
|
||||
return
|
||||
|
||||
# Cleaned needs to be turned back into an int here
|
||||
|
|
@ -839,24 +804,11 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
|
|||
self._from_db_object(context, self, inst_ref,
|
||||
expected_attrs=expected_attrs)
|
||||
|
||||
if cells_update_from_api:
|
||||
_handle_cell_update_from_api()
|
||||
|
||||
def _notify():
|
||||
# NOTE(danms): We have to be super careful here not to trigger
|
||||
# any lazy-loads that will unmigrate or unbackport something. So,
|
||||
# make a copy of the instance for notifications first.
|
||||
new_ref = self.obj_clone()
|
||||
|
||||
notifications.send_update(context, old_ref, new_ref)
|
||||
|
||||
# NOTE(alaski): If cell synchronization is blocked it means we have
|
||||
# already run this block of code in either the parent or child of this
|
||||
# cell. Therefore this notification has already been sent.
|
||||
if not self._sync_cells:
|
||||
_notify = lambda: None # noqa: F811
|
||||
|
||||
_notify()
|
||||
# NOTE(danms): We have to be super careful here not to trigger
|
||||
# any lazy-loads that will unmigrate or unbackport something. So,
|
||||
# make a copy of the instance for notifications first.
|
||||
new_ref = self.obj_clone()
|
||||
notifications.send_update(context, old_ref, new_ref)
|
||||
|
||||
self.obj_reset_changes()
|
||||
|
||||
|
|
@ -1206,49 +1158,6 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
|
|||
if not md_was_changed:
|
||||
self.obj_reset_changes(['metadata'])
|
||||
|
||||
def _cell_name_blocks_sync(self):
|
||||
if (self.obj_attr_is_set('cell_name') and
|
||||
self.cell_name is not None and
|
||||
self.cell_name.startswith(cells_utils.BLOCK_SYNC_FLAG)):
|
||||
return True
|
||||
return False
|
||||
|
||||
def _normalize_cell_name(self):
|
||||
"""Undo skip_cell_sync()'s cell_name modification if applied"""
|
||||
|
||||
if not self.obj_attr_is_set('cell_name') or self.cell_name is None:
|
||||
return
|
||||
cn_changed = 'cell_name' in self.obj_what_changed()
|
||||
if self.cell_name.startswith(cells_utils.BLOCK_SYNC_FLAG):
|
||||
self.cell_name = self.cell_name.replace(
|
||||
cells_utils.BLOCK_SYNC_FLAG, '', 1)
|
||||
# cell_name is not normally an empty string, this means it was None
|
||||
# or unset before cells_utils.BLOCK_SYNC_FLAG was applied.
|
||||
if len(self.cell_name) == 0:
|
||||
self.cell_name = None
|
||||
if not cn_changed:
|
||||
self.obj_reset_changes(['cell_name'])
|
||||
|
||||
@contextlib.contextmanager
|
||||
def skip_cells_sync(self):
|
||||
"""Context manager to save an instance without syncing cells.
|
||||
|
||||
Temporarily disables the cells syncing logic, if enabled. This should
|
||||
only be used when saving an instance that has been passed down/up from
|
||||
another cell in order to avoid passing it back to the originator to be
|
||||
re-saved.
|
||||
"""
|
||||
cn_changed = 'cell_name' in self.obj_what_changed()
|
||||
if not self.obj_attr_is_set('cell_name') or self.cell_name is None:
|
||||
self.cell_name = ''
|
||||
self.cell_name = '%s%s' % (cells_utils.BLOCK_SYNC_FLAG, self.cell_name)
|
||||
if not cn_changed:
|
||||
self.obj_reset_changes(['cell_name'])
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
self._normalize_cell_name()
|
||||
|
||||
def get_network_info(self):
|
||||
if self.info_cache is None:
|
||||
return network_model.NetworkInfo.hydrate([])
|
||||
|
|
|
|||
|
|
@ -637,20 +637,6 @@ class CellsManagerClassTestCase(test.NoDBTestCase):
|
|||
response = self.cells_manager.get_migrations(self.ctxt, filters)
|
||||
self.assertEqual(migrations.objects, response.objects)
|
||||
|
||||
def test_instance_update_from_api(self):
|
||||
self.mox.StubOutWithMock(self.msg_runner,
|
||||
'instance_update_from_api')
|
||||
self.msg_runner.instance_update_from_api(self.ctxt,
|
||||
'fake-instance',
|
||||
'exp_vm', 'exp_task',
|
||||
'admin_reset')
|
||||
self.mox.ReplayAll()
|
||||
self.cells_manager.instance_update_from_api(
|
||||
self.ctxt, instance='fake-instance',
|
||||
expected_vm_state='exp_vm',
|
||||
expected_task_state='exp_task',
|
||||
admin_state_reset='admin_reset')
|
||||
|
||||
def test_start_instance(self):
|
||||
self.mox.StubOutWithMock(self.msg_runner, 'start_instance')
|
||||
self.msg_runner.start_instance(self.ctxt, 'fake-instance')
|
||||
|
|
|
|||
|
|
@ -1069,78 +1069,6 @@ class CellsTargetedMethodsTestCase(test.NoDBTestCase):
|
|||
'delete')
|
||||
delete.assert_called_once_with(self.ctxt, instance)
|
||||
|
||||
def _instance_update_helper(self, admin_state_reset):
|
||||
class FakeMessage(object):
|
||||
pass
|
||||
|
||||
message = FakeMessage()
|
||||
message.ctxt = self.ctxt
|
||||
|
||||
instance = objects.Instance()
|
||||
instance.cell_name = self.tgt_cell_name
|
||||
instance.obj_reset_changes()
|
||||
instance.task_state = 'meow'
|
||||
instance.vm_state = 'wuff'
|
||||
instance.user_data = 'foo'
|
||||
instance.metadata = {'meta': 'data'}
|
||||
instance.system_metadata = {'system': 'metadata'}
|
||||
self.assertEqual(set(['user_data', 'vm_state', 'task_state',
|
||||
'metadata', 'system_metadata']),
|
||||
instance.obj_what_changed())
|
||||
|
||||
self.mox.StubOutWithMock(instance, 'save')
|
||||
|
||||
def _check_object(*args, **kwargs):
|
||||
# task_state and vm_state changes should have been cleared
|
||||
# before calling save()
|
||||
if admin_state_reset:
|
||||
self.assertEqual(
|
||||
set(['user_data', 'vm_state', 'task_state']),
|
||||
instance.obj_what_changed())
|
||||
else:
|
||||
self.assertEqual(set(['user_data']),
|
||||
instance.obj_what_changed())
|
||||
|
||||
instance.save(expected_task_state='exp_task',
|
||||
expected_vm_state='exp_vm').WithSideEffects(
|
||||
_check_object)
|
||||
|
||||
self.mox.ReplayAll()
|
||||
|
||||
self.tgt_methods_cls.instance_update_from_api(
|
||||
message,
|
||||
instance,
|
||||
expected_vm_state='exp_vm',
|
||||
expected_task_state='exp_task',
|
||||
admin_state_reset=admin_state_reset)
|
||||
|
||||
def test_instance_update_from_api(self):
|
||||
self._instance_update_helper(False)
|
||||
|
||||
def test_instance_update_from_api_admin_state_reset(self):
|
||||
self._instance_update_helper(True)
|
||||
|
||||
def test_instance_update_from_api_calls_skip_cells_sync(self):
|
||||
self.flags(enable=True, cell_type='compute', group='cells')
|
||||
instance = fake_instance.fake_instance_obj(self.ctxt)
|
||||
instance.cell_name = self.tgt_cell_name
|
||||
instance.task_state = 'meow'
|
||||
instance.vm_state = 'wuff'
|
||||
instance.user_data = 'foo'
|
||||
message = ''
|
||||
|
||||
@mock.patch.object(instance, 'save', side_effect=test.TestingException)
|
||||
@mock.patch.object(instance, 'skip_cells_sync')
|
||||
def _ensure_skip_cells_sync_called(mock_sync, mock_save):
|
||||
self.assertRaises(test.TestingException,
|
||||
self.tgt_methods_cls.instance_update_from_api,
|
||||
message, instance, expected_vm_state='exp_vm',
|
||||
expected_task_state='exp_task', admin_state_reset=False)
|
||||
mock_sync.assert_has_calls([mock.call()])
|
||||
|
||||
_ensure_skip_cells_sync_called()
|
||||
self.assertEqual(self.tgt_cell_name, instance.cell_name)
|
||||
|
||||
def _test_instance_action_method(self, method, args, kwargs,
|
||||
expected_args, expected_kwargs,
|
||||
expect_result):
|
||||
|
|
|
|||
|
|
@ -433,22 +433,6 @@ class CellsAPITestCase(test.NoDBTestCase):
|
|||
self._check_result(call_info, 'get_migrations', expected_args,
|
||||
version="1.11")
|
||||
|
||||
def test_instance_update_from_api(self):
|
||||
call_info = self._stub_rpc_method('cast', None)
|
||||
|
||||
self.cells_rpcapi.instance_update_from_api(
|
||||
self.fake_context, 'fake-instance',
|
||||
expected_vm_state='exp_vm',
|
||||
expected_task_state='exp_task',
|
||||
admin_state_reset='admin_reset')
|
||||
|
||||
expected_args = {'instance': 'fake-instance',
|
||||
'expected_vm_state': 'exp_vm',
|
||||
'expected_task_state': 'exp_task',
|
||||
'admin_state_reset': 'admin_reset'}
|
||||
self._check_result(call_info, 'instance_update_from_api',
|
||||
expected_args, version='1.16')
|
||||
|
||||
def test_start_instance(self):
|
||||
call_info = self._stub_rpc_method('cast', None)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue