openstack
/
nova
Code Issues Proposed changes

Merge "Ensure source compute is up when confirming a resize"

This commit is contained in:
Zuul 2020-08-26 21:03:44 +00:00 committed by Gerrit Code Review
parent cf7288c53f e4601c77fb
commit 197dac3c78
7 changed files with 129 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
nova/api/openstack/compute/servers.py
View File

@ -885,7 +885,10 @@ class ServersController(wsgi.Controller):
except exception.MigrationNotFound:
msg = _("Instance has not been resized.")
raise exc.HTTPBadRequest(explanation=msg)
except exception.InstanceIsLocked as e:
except (
exception.InstanceIsLocked,
exception.ServiceUnavailable,
) as e:
raise exc.HTTPConflict(explanation=e.format_message())
except exception.InstanceInvalidState as state_error:
common.raise_http_conflict_for_instance_invalid_state(state_error,

39
nova/compute/api.py
View File

@ -3775,10 +3775,40 @@ class API(base.Base):
migration.dest_compute,
reqspec)
@staticmethod
def _get_source_compute_service(context, migration):
"""Find the source compute Service object given the Migration.
:param context: nova auth RequestContext target at the destination
compute cell
:param migration: Migration object for the move operation
:return: Service object representing the source host nova-compute
"""
if migration.cross_cell_move:
# The source compute could be in another cell so look up the
# HostMapping to determine the source cell.
hm = objects.HostMapping.get_by_host(
context, migration.source_compute)
with nova_context.target_cell(context, hm.cell_mapping) as cctxt:
return objects.Service.get_by_compute_host(
cctxt, migration.source_compute)
# Same-cell migration so just use the context we have.
return objects.Service.get_by_compute_host(
context, migration.source_compute)
@check_instance_lock
@check_instance_state(vm_state=[vm_states.RESIZED])
def confirm_resize(self, context, instance, migration=None):
"""Confirms a migration/resize and deletes the 'old' instance."""
"""Confirms a migration/resize and deletes the 'old' instance.
:param context: nova auth RequestContext
:param instance: Instance object to confirm the resize
:param migration: Migration object; provided if called from the
_poll_unconfirmed_resizes periodic task on the dest compute.
:raises: MigrationNotFound if migration is not provided and a migration
cannot be found for the instance with status "finished".
:raises: ServiceUnavailable if the source compute service is down.
"""
elevated = context.elevated()
# NOTE(melwitt): We're not checking quota here because there isn't a
# change in resource usage when confirming a resize. Resource
@ -3789,6 +3819,13 @@ class API(base.Base):
migration = objects.Migration.get_by_instance_and_status(
elevated, instance.uuid, 'finished')
# Check if the source compute service is up before modifying the
# migration record because once we do we cannot come back through this
# method since it will be looking for a "finished" status migration.
source_svc = self._get_source_compute_service(context, migration)
if not self.servicegroup_api.service_is_up(source_svc):
raise exception.ServiceUnavailable()
migration.status = 'confirming'
migration.save()

8
nova/tests/fixtures.py
View File

@ -324,8 +324,12 @@ class SingleCellSimple(fixtures.Fixture):
@contextmanager
def _fake_target_cell(self, context, target_cell):
# NOTE(danms): Just pass through the context without actually
# targeting anything.
# Just do something simple and set/unset the cell_uuid on the context.
if target_cell:
context.cell_uuid = getattr(target_cell, 'uuid',
uuidsentinel.cell1)
else:
context.cell_uuid = None
yield context
def _fake_set_target_cell(self, context, cell_mapping):

8
nova/tests/functional/test_cross_cell_migrate.py
View File

@ -963,6 +963,10 @@ class TestMultiCellMigrate(integrated_helpers.ProviderUsageBaseTestCase):
server2['id'], {'migrate': {'host': 'host2'}})
self._wait_for_migration_status(server2, ['error'])
@mock.patch('nova.compute.api.API._get_source_compute_service',
new=mock.Mock())
@mock.patch('nova.servicegroup.api.API.service_is_up',
new=mock.Mock(return_value=True))
def test_poll_unconfirmed_resizes_with_upcall(self):
"""Tests the _poll_unconfirmed_resizes periodic task with a cross-cell
resize once the instance is in VERIFY_RESIZE status on the dest host.
@ -993,6 +997,10 @@ class TestMultiCellMigrate(integrated_helpers.ProviderUsageBaseTestCase):
self._assert_confirm(
server, source_rp_uuid, target_rp_uuid, new_flavor)
@mock.patch('nova.compute.api.API._get_source_compute_service',
new=mock.Mock())
@mock.patch('nova.servicegroup.api.API.service_is_up',
new=mock.Mock(return_value=True))
def test_poll_unconfirmed_resizes_with_no_upcall(self):
"""Tests the _poll_unconfirmed_resizes periodic task with a cross-cell
resize once the instance is in VERIFY_RESIZE status on the dest host.

42
nova/tests/functional/test_servers.py
View File

@ -46,7 +46,6 @@ from nova.tests import fixtures as nova_fixtures
from nova.tests.functional.api import client
from nova.tests.functional import integrated_helpers
from nova.tests.unit.api.openstack import fakes
from nova.tests.unit import cast_as_call
from nova.tests.unit import fake_block_device
from nova.tests.unit import fake_notifier
from nova.tests.unit import fake_requests
@ -3359,34 +3358,37 @@ class PollUnconfirmedResizesTest(integrated_helpers.ProviderUsageBaseTestCase):
self.api.put_service(
source_service['id'], {'status': 'disabled', 'forced_down': True})
# Now configure auto-confirm and call the method on the target compute
# so we do not have to wait for the periodic to run. Also configure
# the RPC call from the API to the source compute to timeout after 1
# second.
self.flags(resize_confirm_window=1, rpc_response_timeout=1)
# so we do not have to wait for the periodic to run.
self.flags(resize_confirm_window=1)
# Stub timeutils so the DB API query finds the unconfirmed migration.
future = timeutils.utcnow() + datetime.timedelta(hours=1)
ctxt = context.get_admin_context()
with osloutils_fixture.TimeFixture(future):
# Use the CastAsCall fixture since the fake rpc is not going to
# simulate a failure from the service being down.
with cast_as_call.CastAsCall(self):
self.computes['host2'].manager._poll_unconfirmed_resizes(ctxt)
self.computes['host2'].manager._poll_unconfirmed_resizes(ctxt)
self.assertIn('Error auto-confirming resize',
self.stdlog.logger.output)
self.assertIn('No reply on topic compute', self.stdlog.logger.output)
# FIXME(mriedem): This is bug 1855927 where the migration status is
# left in "confirming" so the _poll_unconfirmed_resizes task will not
# process it again nor can the user confirm the resize in the API since
# the migration status is not "finished".
self._wait_for_migration_status(server, ['confirming'])
self.assertIn('Service is unavailable at this time',
self.stdlog.logger.output)
# The source compute service check should have been done before the
# migration status was updated so it should still be "finished".
self._wait_for_migration_status(server, ['finished'])
# Try to confirm in the API while the source compute service is still
# down to assert the 409 (rather than a 500) error.
ex = self.assertRaises(client.OpenStackApiException,
self.api.post_server_action,
server['id'], {'confirmResize': None})
self.assertEqual(400, ex.response.status_code)
self.assertIn('Instance has not been resized', six.text_type(ex))
# That error message is bogus because the server is resized.
server = self.api.get_server(server['id'])
self.assertEqual('VERIFY_RESIZE', server['status'])
self.assertEqual(409, ex.response.status_code)
self.assertIn('Service is unavailable at this time', six.text_type(ex))
# Bring the source compute back up and try to confirm the resize which
# should work since the migration status is still "finished".
self.restart_compute_service(self.computes['host1'])
self.api.put_service(
source_service['id'], {'status': 'enabled', 'forced_down': False})
# Use the API to confirm the resize because _poll_unconfirmed_resizes
# requires mucking with the current time which causes problems with
# the service_is_up check in the API.
self.api.post_server_action(server['id'], {'confirmResize': None})
self._wait_for_state_change(server, 'ACTIVE')
class ServerLiveMigrateForceAndAbort(

42
nova/tests/unit/compute/test_api.py
View File

@ -1644,10 +1644,13 @@ class _ComputeAPIUnitTestMixIn(object):
test()
@mock.patch('nova.compute.api.API._get_source_compute_service')
@mock.patch('nova.servicegroup.api.API.service_is_up', return_value=True)
@mock.patch.object(objects.Migration, 'save')
@mock.patch.object(objects.Migration, 'get_by_instance_and_status')
@mock.patch.object(context.RequestContext, 'elevated')
def _test_confirm_resize(self, mock_elevated, mock_get, mock_save,
mock_service_is_up, mock_get_service,
mig_ref_passed=False):
params = dict(vm_state=vm_states.RESIZED)
fake_inst = self._create_instance_obj(params=params)
@ -1678,6 +1681,8 @@ class _ComputeAPIUnitTestMixIn(object):
self.compute_api.confirm_resize(self.context, fake_inst)
mock_elevated.assert_called_once_with()
mock_service_is_up.assert_called_once_with(
mock_get_service.return_value)
mock_save.assert_called_once_with()
mock_record.assert_called_once_with(self.context, fake_inst,
'confirmResize')
@ -1694,6 +1699,34 @@ class _ComputeAPIUnitTestMixIn(object):
def test_confirm_resize_with_migration_ref(self):
self._test_confirm_resize(mig_ref_passed=True)
@mock.patch('nova.objects.HostMapping.get_by_host',
return_value=objects.HostMapping(
cell_mapping=objects.CellMapping(
database_connection='fake://', transport_url='none://',
uuid=uuids.cell_uuid)))
@mock.patch('nova.objects.Service.get_by_compute_host')
def test_get_source_compute_service(self, mock_service_get, mock_hm_get):
# First start with a same-cell migration.
migration = objects.Migration(source_compute='source.host',
cross_cell_move=False)
self.compute_api._get_source_compute_service(self.context, migration)
mock_hm_get.assert_not_called()
mock_service_get.assert_called_once_with(self.context, 'source.host')
# Make sure the context was not targeted.
ctxt = mock_service_get.call_args[0][0]
self.assertIsNone(ctxt.cell_uuid)
# Now test with a cross-cell migration.
mock_service_get.reset_mock()
migration.cross_cell_move = True
self.compute_api._get_source_compute_service(self.context, migration)
mock_hm_get.assert_called_once_with(self.context, 'source.host')
mock_service_get.assert_called_once_with(
test.MatchType(context.RequestContext), 'source.host')
# Make sure the context was targeted.
ctxt = mock_service_get.call_args[0][0]
self.assertEqual(uuids.cell_uuid, ctxt.cell_uuid)
@mock.patch('nova.virt.hardware.numa_get_constraints')
@mock.patch('nova.network.neutron.API.get_requested_resource_for_instance',
return_value=[])
@ -7460,8 +7493,10 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
self._test_get_migrations_sorted_filter_duplicates(
[older, newer], newer)
@mock.patch('nova.servicegroup.api.API.service_is_up', return_value=True)
@mock.patch('nova.objects.Migration.get_by_instance_and_status')
def test_confirm_resize_cross_cell_move_true(self, mock_migration_get):
def test_confirm_resize_cross_cell_move_true(self, mock_migration_get,
mock_service_is_up):
"""Tests confirm_resize where Migration.cross_cell_move is True"""
instance = fake_instance.fake_instance_obj(
self.context, vm_state=vm_states.RESIZED, task_state=None,
@ -7475,12 +7510,15 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
mock.patch.object(self.compute_api, '_record_action_start'),
mock.patch.object(self.compute_api.compute_task_api,
'confirm_snapshot_based_resize'),
mock.patch.object(self.compute_api, '_get_source_compute_service'),
) as (
mock_elevated, mock_migration_save, mock_record_action,
mock_conductor_confirm
mock_conductor_confirm, mock_get_service
):
self.compute_api.confirm_resize(self.context, instance)
mock_elevated.assert_called_once_with()
mock_service_is_up.assert_called_once_with(
mock_get_service.return_value)
mock_migration_save.assert_called_once_with()
self.assertEqual('confirming', migration.status)
mock_record_action.assert_called_once_with(

20
nova/tests/unit/scheduler/test_host_manager.py
View File

@ -219,7 +219,8 @@ class HostManagerTestCase(test.NoDBTestCase):
inst2 = objects.Instance(host='host1', uuid=uuids.instance_2)
inst3 = objects.Instance(host='host2', uuid=uuids.instance_3)
cell = objects.CellMapping(database_connection='',
target_url='')
target_url='',
uuid=uuids.cell_uuid)
mock_get_by_filters.return_value = objects.InstanceList(
objects=[inst1, inst2, inst3])
hm = self.host_manager
@ -589,7 +590,7 @@ class HostManagerTestCase(test.NoDBTestCase):
mock_get_by_host.return_value = []
mock_get_all.return_value = fakes.COMPUTE_NODES
mock_get_by_binary.return_value = fakes.SERVICES
context = 'fake_context'
context = nova_context.get_admin_context()
compute_nodes, services = self.host_manager._get_computes_for_cells(
context, self.host_manager.enabled_cells)
@ -786,7 +787,7 @@ class HostManagerTestCase(test.NoDBTestCase):
@mock.patch('nova.objects.InstanceList.get_uuids_by_host')
def test_host_state_not_updated(self, mock_get_by_host):
context = 'fake_context'
context = nova_context.get_admin_context()
hm = self.host_manager
inst1 = objects.Instance(uuid=uuids.instance)
cn1 = objects.ComputeNode(host='host1')
@ -806,10 +807,11 @@ class HostManagerTestCase(test.NoDBTestCase):
@mock.patch('nova.objects.InstanceList.get_uuids_by_host')
def test_recreate_instance_info(self, mock_get_by_host):
context = nova_context.get_admin_context()
host_name = 'fake_host'
inst1 = fake_instance.fake_instance_obj('fake_context',
inst1 = fake_instance.fake_instance_obj(context,
uuid=uuids.instance_1)
inst2 = fake_instance.fake_instance_obj('fake_context',
inst2 = fake_instance.fake_instance_obj(context,
uuid=uuids.instance_2)
orig_inst_dict = {inst1.uuid: inst1, inst2.uuid: inst2}
mock_get_by_host.return_value = [uuids.instance_1, uuids.instance_2]
@ -818,7 +820,7 @@ class HostManagerTestCase(test.NoDBTestCase):
'instances': orig_inst_dict,
'updated': True,
}}
self.host_manager._recreate_instance_info('fake_context', host_name)
self.host_manager._recreate_instance_info(context, host_name)
new_info = self.host_manager._instance_info[host_name]
self.assertEqual(len(new_info['instances']),
len(mock_get_by_host.return_value))
@ -1231,7 +1233,7 @@ class HostManagerChangedNodesTestCase(test.NoDBTestCase):
mock_get_by_host.return_value = []
mock_get_all.return_value = fakes.COMPUTE_NODES
mock_get_by_binary.return_value = fakes.SERVICES
context = 'fake_context'
context = nova_context.get_admin_context()
compute_nodes, services = self.host_manager._get_computes_for_cells(
context, self.host_manager.enabled_cells)
@ -1256,7 +1258,7 @@ class HostManagerChangedNodesTestCase(test.NoDBTestCase):
mock_get_by_host.return_value = []
mock_get_all.side_effect = [fakes.COMPUTE_NODES, running_nodes]
mock_get_by_binary.side_effect = [fakes.SERVICES, fakes.SERVICES]
context = 'fake_context'
context = nova_context.get_admin_context()
# first call: all nodes
compute_nodes, services = self.host_manager._get_computes_for_cells(
@ -1286,7 +1288,7 @@ class HostManagerChangedNodesTestCase(test.NoDBTestCase):
mock_get_by_host.return_value = []
mock_get_all.side_effect = [fakes.COMPUTE_NODES, []]
mock_get_by_binary.side_effect = [fakes.SERVICES, fakes.SERVICES]
context = 'fake_context'
context = nova_context.get_admin_context()
# first call: all nodes
compute_nodes, services = self.host_manager._get_computes_for_cells(