Merge "Validate new image via scheduler during rebuild" into stable/newton
This commit is contained in:
commit
b85139255a
|
@ -2813,9 +2813,24 @@ class API(base.Base):
|
|||
# sure that all our instances are currently migrated to have an
|
||||
# attached RequestSpec object but let's consider that the operator only
|
||||
# half migrated all their instances in the meantime.
|
||||
host = instance.host
|
||||
try:
|
||||
request_spec = objects.RequestSpec.get_by_instance_uuid(
|
||||
context, instance.uuid)
|
||||
# If a new image is provided on rebuild, we will need to run
|
||||
# through the scheduler again, but we want the instance to be
|
||||
# rebuilt on the same host it's already on.
|
||||
if orig_image_ref != image_href:
|
||||
request_spec.requested_destination = objects.Destination(
|
||||
host=instance.host,
|
||||
node=instance.node)
|
||||
# We have to modify the request spec that goes to the scheduler
|
||||
# to contain the new image. We persist this since we've already
|
||||
# changed the instance.image_ref above so we're being
|
||||
# consistent.
|
||||
request_spec.image = objects.ImageMeta.from_dict(image)
|
||||
request_spec.save()
|
||||
host = None # This tells conductor to call the scheduler.
|
||||
except exception.RequestSpecNotFound:
|
||||
# Some old instances can still have no RequestSpec object attached
|
||||
# to them, we need to support the old way
|
||||
|
@ -2825,7 +2840,7 @@ class API(base.Base):
|
|||
new_pass=admin_password, injected_files=files_to_inject,
|
||||
image_ref=image_href, orig_image_ref=orig_image_ref,
|
||||
orig_sys_metadata=orig_sys_metadata, bdms=bdms,
|
||||
preserve_ephemeral=preserve_ephemeral, host=instance.host,
|
||||
preserve_ephemeral=preserve_ephemeral, host=host,
|
||||
request_spec=request_spec,
|
||||
kwargs=kwargs)
|
||||
|
||||
|
|
|
@ -673,7 +673,7 @@ class ComputeTaskManager(base.Base):
|
|||
filter_properties = {'ignore_hosts': [instance.host]}
|
||||
request_spec = scheduler_utils.build_request_spec(
|
||||
context, image_ref, [instance])
|
||||
else:
|
||||
elif recreate:
|
||||
# NOTE(sbauza): Augment the RequestSpec object by excluding
|
||||
# the source host for avoiding the scheduler to pick it
|
||||
request_spec.ignore_hosts = request_spec.ignore_hosts or []
|
||||
|
@ -688,6 +688,10 @@ class ComputeTaskManager(base.Base):
|
|||
filter_properties = request_spec.\
|
||||
to_legacy_filter_properties_dict()
|
||||
request_spec = request_spec.to_legacy_request_spec_dict()
|
||||
else:
|
||||
filter_properties = request_spec. \
|
||||
to_legacy_filter_properties_dict()
|
||||
request_spec = request_spec.to_legacy_request_spec_dict()
|
||||
try:
|
||||
hosts = self._schedule_instances(
|
||||
context, request_spec, filter_properties)
|
||||
|
|
|
@ -300,6 +300,16 @@ class TestOpenStackClient(object):
|
|||
def delete_image(self, image_id):
|
||||
return self.api_delete('/images/%s' % image_id)
|
||||
|
||||
def put_image_meta_key(self, image_id, key, value):
|
||||
"""Creates or updates a given image metadata key/value pair."""
|
||||
req_body = {
|
||||
'meta': {
|
||||
key: value
|
||||
}
|
||||
}
|
||||
return self.api_put('/images/%s/metadata/%s' % (image_id, key),
|
||||
req_body)
|
||||
|
||||
def get_flavor(self, flavor_id):
|
||||
return self.api_get('/flavors/%s' % flavor_id).body['flavor']
|
||||
|
||||
|
|
|
@ -70,7 +70,6 @@ class _IntegratedTestBase(test.TestCase):
|
|||
self.flags(verbose=True)
|
||||
|
||||
nova.tests.unit.image.fake.stub_out_image_service(self)
|
||||
self._setup_services()
|
||||
|
||||
self.api_fixture = self.useFixture(
|
||||
nova_fixtures.OSAPIFixture(self.api_major_version))
|
||||
|
@ -82,8 +81,13 @@ class _IntegratedTestBase(test.TestCase):
|
|||
else:
|
||||
self.api = self.api_fixture.api
|
||||
|
||||
if hasattr(self, 'microversion'):
|
||||
self.api.microversion = self.microversion
|
||||
|
||||
self.useFixture(cast_as_call.CastAsCall(self.stubs))
|
||||
|
||||
self._setup_services()
|
||||
|
||||
self.addCleanup(nova.tests.unit.image.fake.FakeImageService_reset)
|
||||
|
||||
def _setup_compute_service(self):
|
||||
|
@ -96,12 +100,13 @@ class _IntegratedTestBase(test.TestCase):
|
|||
def _setup_services(self):
|
||||
self.conductor = self.start_service('conductor',
|
||||
manager=CONF.conductor.manager)
|
||||
self.compute = self._setup_compute_service()
|
||||
self.consoleauth = self.start_service('consoleauth')
|
||||
|
||||
self.network = self.start_service('network')
|
||||
self.scheduler = self._setup_scheduler_service()
|
||||
|
||||
self.compute = self._setup_compute_service()
|
||||
|
||||
def get_unused_server_name(self):
|
||||
servers = self.api.get_servers()
|
||||
server_names = [server['name'] for server in servers]
|
||||
|
@ -231,3 +236,40 @@ class InstanceHelperMixin(object):
|
|||
server['flavorRef'] = ('http://fake.server/%s' % flavor_id)
|
||||
server['name'] = name
|
||||
return server
|
||||
|
||||
def _wait_for_action_fail_completion(
|
||||
self, server, expected_action, event_name, api=None):
|
||||
"""Polls instance action events for the given instance, action and
|
||||
action event name until it finds the action event with an error
|
||||
result.
|
||||
"""
|
||||
if api is None:
|
||||
api = self.api
|
||||
completion_event = None
|
||||
for attempt in range(10):
|
||||
actions = api.get_instance_actions(server['id'])
|
||||
# Look for the migrate action.
|
||||
for action in actions:
|
||||
if action['action'] == expected_action:
|
||||
events = (
|
||||
api.api_get(
|
||||
'/servers/%s/os-instance-actions/%s' %
|
||||
(server['id'], action['request_id'])
|
||||
).body['instanceAction']['events'])
|
||||
# Look for the action event being in error state.
|
||||
for event in events:
|
||||
if (event['event'] == event_name and
|
||||
event['result'] is not None and
|
||||
event['result'].lower() == 'error'):
|
||||
completion_event = event
|
||||
# Break out of the events loop.
|
||||
break
|
||||
if completion_event:
|
||||
# Break out of the actions loop.
|
||||
break
|
||||
# We didn't find the completion event yet, so wait a bit.
|
||||
time.sleep(0.5)
|
||||
|
||||
if completion_event is None:
|
||||
self.fail('Timed out waiting for %s failure event. Current '
|
||||
'instance actions: %s' % (event_name, actions))
|
||||
|
|
|
@ -22,6 +22,7 @@ from oslo_log import log as logging
|
|||
from oslo_utils import timeutils
|
||||
|
||||
from nova.compute import api as compute_api
|
||||
from nova.compute import instance_actions
|
||||
from nova.compute import rpcapi
|
||||
from nova import context
|
||||
from nova import exception
|
||||
|
@ -33,6 +34,7 @@ from nova.tests.functional import integrated_helpers
|
|||
from nova.tests.unit.api.openstack import fakes
|
||||
from nova.tests.unit import fake_block_device
|
||||
from nova.tests.unit import fake_network
|
||||
import nova.tests.unit.image.fake
|
||||
from nova import volume
|
||||
|
||||
|
||||
|
@ -823,3 +825,71 @@ class ServerTestV220(ServersTestBase):
|
|||
self.assertTrue(mock_clean_vols.called)
|
||||
|
||||
self._delete_server(server_id)
|
||||
|
||||
|
||||
class ServerRebuildTestCase(integrated_helpers._IntegratedTestBase,
|
||||
integrated_helpers.InstanceHelperMixin):
|
||||
api_major_version = 'v2.1'
|
||||
# We have to cap the microversion at 2.38 because that's the max we
|
||||
# can use to update image metadata via our compute images proxy API.
|
||||
microversion = '2.38'
|
||||
|
||||
# We need the ImagePropertiesFilter so override the base class setup
|
||||
# which configures to use the chance_scheduler.
|
||||
def _setup_scheduler_service(self):
|
||||
self.flags(scheduler_default_filters=['ImagePropertiesFilter'])
|
||||
return self.start_service('scheduler')
|
||||
|
||||
def test_rebuild_with_image_novalidhost(self):
|
||||
"""Creates a server with an image that is valid for the single compute
|
||||
that we have. Then rebuilds the server, passing in an image with
|
||||
metadata that does not fit the single compute which should result in
|
||||
a NoValidHost error. The ImagePropertiesFilter filter is enabled by
|
||||
default so that should filter out the host based on the image meta.
|
||||
"""
|
||||
server_req_body = {
|
||||
'server': {
|
||||
# We hard-code from a fake image since we can't get images
|
||||
# via the compute /images proxy API with microversion > 2.35.
|
||||
'imageRef': '155d900f-4e14-4e4c-a73d-069cbf4541e6',
|
||||
'flavorRef': '1', # m1.tiny from DefaultFlavorsFixture,
|
||||
'name': 'test_rebuild_with_image_novalidhost',
|
||||
# We don't care about networking for this test. This requires
|
||||
# microversion >= 2.37.
|
||||
'networks': 'none'
|
||||
}
|
||||
}
|
||||
server = self.api.post_server(server_req_body)
|
||||
self._wait_for_state_change(self.api, server, 'ACTIVE')
|
||||
# Now update the image metadata to be something that won't work with
|
||||
# the fake compute driver we're using since the fake driver has an
|
||||
# "x86_64" architecture.
|
||||
rebuild_image_ref = (
|
||||
nova.tests.unit.image.fake.AUTO_DISK_CONFIG_ENABLED_IMAGE_UUID)
|
||||
self.api.put_image_meta_key(
|
||||
rebuild_image_ref, 'hw_architecture', 'unicore32')
|
||||
# Now rebuild the server with that updated image and it should result
|
||||
# in a NoValidHost failure from the scheduler.
|
||||
rebuild_req_body = {
|
||||
'rebuild': {
|
||||
'imageRef': rebuild_image_ref
|
||||
}
|
||||
}
|
||||
# Since we're using the CastAsCall fixture, the NoValidHost error
|
||||
# should actually come back to the API and result in a 500 error.
|
||||
# Normally the user would get a 202 response because nova-api RPC casts
|
||||
# to nova-conductor which RPC calls the scheduler which raises the
|
||||
# NoValidHost. We can mimic the end user way to figure out the failure
|
||||
# by looking for the failed 'rebuild' instance action event.
|
||||
self.api.api_post('/servers/%s/action' % server['id'],
|
||||
rebuild_req_body, check_response_status=[500])
|
||||
# Look for the failed rebuild action.
|
||||
self._wait_for_action_fail_completion(
|
||||
server, instance_actions.REBUILD, 'rebuild_server',
|
||||
# Before microversion 2.51 events are only returned for instance
|
||||
# actions if you're an admin.
|
||||
self.api_fixture.admin_api)
|
||||
# Unfortunately the server's image_ref is updated to be the new image
|
||||
# even though the rebuild should not work.
|
||||
server = self.api.get_server(server['id'])
|
||||
self.assertEqual(rebuild_image_ref, server['image']['id'])
|
||||
|
|
|
@ -2979,6 +2979,7 @@ class _ComputeAPIUnitTestMixIn(object):
|
|||
None, image, flavor, {}, [], None)
|
||||
self.assertNotEqual(orig_system_metadata, instance.system_metadata)
|
||||
|
||||
@mock.patch.object(objects.RequestSpec, 'save')
|
||||
@mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid')
|
||||
@mock.patch.object(objects.Instance, 'save')
|
||||
@mock.patch.object(objects.Instance, 'get_flavor')
|
||||
|
@ -2990,7 +2991,7 @@ class _ComputeAPIUnitTestMixIn(object):
|
|||
def test_rebuild_change_image(self, _record_action_start,
|
||||
_checks_for_create_and_rebuild, _check_auto_disk_config,
|
||||
_get_image, bdm_get_by_instance_uuid, get_flavor, instance_save,
|
||||
req_spec_get_by_inst_uuid):
|
||||
req_spec_get_by_inst_uuid, req_spec_save):
|
||||
orig_system_metadata = {}
|
||||
get_flavor.return_value = test_flavor.fake_flavor
|
||||
orig_image_href = 'orig_image'
|
||||
|
@ -3035,8 +3036,15 @@ class _ComputeAPIUnitTestMixIn(object):
|
|||
injected_files=files_to_inject, image_ref=new_image_href,
|
||||
orig_image_ref=orig_image_href,
|
||||
orig_sys_metadata=orig_system_metadata, bdms=bdms,
|
||||
preserve_ephemeral=False, host=instance.host,
|
||||
preserve_ephemeral=False, host=None,
|
||||
request_spec=fake_spec, kwargs={})
|
||||
# assert the request spec was modified so the scheduler picks
|
||||
# the existing instance host/node
|
||||
req_spec_save.assert_called_once_with()
|
||||
self.assertIn('requested_destination', fake_spec)
|
||||
requested_destination = fake_spec.requested_destination
|
||||
self.assertEqual(instance.host, requested_destination.host)
|
||||
self.assertEqual(instance.node, requested_destination.node)
|
||||
|
||||
_check_auto_disk_config.assert_called_once_with(image=new_image)
|
||||
_checks_for_create_and_rebuild.assert_called_once_with(self.context,
|
||||
|
|
|
@ -1422,7 +1422,10 @@ class _BaseTaskTestCase(object):
|
|||
self.conductor_manager.rebuild_instance(context=self.context,
|
||||
instance=inst_obj,
|
||||
**rebuild_args)
|
||||
reset_fd.assert_called_once_with()
|
||||
if rebuild_args['recreate']:
|
||||
reset_fd.assert_called_once_with()
|
||||
else:
|
||||
reset_fd.assert_not_called()
|
||||
to_reqspec.assert_called_once_with()
|
||||
to_filtprops.assert_called_once_with()
|
||||
fp_mock.assert_called_once_with(self.context, request_spec,
|
||||
|
|
Loading…
Reference in New Issue