Address old TODO in claim_resources_on_destination
The TODO at the end of the method was based on some old nova-compute behavior which was removed with change I39d93dbf8552605e34b9f146e3613e6af62a1774 in Rocky. Instead of logging a warning, ConsumerAllocationRetrievalFailed is now raised since without the instance consumer allocations on the source node during a forced evacuate we cannot proceed with making those allocations on the destination host. The method is refactored a bit for clarity while in here and to drop the big nesting. Remember that this method is *only* called in the case of a live migration or evacuate operation with a forced target host and evacuate is the only case where source_allocations are not provided. Change-Id: I988d1bd4d7eb1a01d443e3d93964bd09afcc4929
This commit is contained in:
parent
9b121e3c37
commit
a57b990c6b
|
@ -529,6 +529,9 @@ def claim_resources_on_destination(
|
||||||
consumer
|
consumer
|
||||||
"""
|
"""
|
||||||
# Get the current allocations for the source node and the instance.
|
# Get the current allocations for the source node and the instance.
|
||||||
|
# NOTE(gibi) For the live migrate case, the caller provided the
|
||||||
|
# allocation that needs to be used on the dest_node along with the
|
||||||
|
# expected consumer_generation of the consumer (which is the instance).
|
||||||
if not source_allocations:
|
if not source_allocations:
|
||||||
# NOTE(gibi): This is the forced evacuate case where the caller did not
|
# NOTE(gibi): This is the forced evacuate case where the caller did not
|
||||||
# provide any allocation request. So we ask placement here for the
|
# provide any allocation request. So we ask placement here for the
|
||||||
|
@ -544,78 +547,69 @@ def claim_resources_on_destination(
|
||||||
context, instance.uuid)
|
context, instance.uuid)
|
||||||
source_allocations = allocations.get('allocations', {})
|
source_allocations = allocations.get('allocations', {})
|
||||||
consumer_generation = allocations.get('consumer_generation')
|
consumer_generation = allocations.get('consumer_generation')
|
||||||
else:
|
|
||||||
# NOTE(gibi) This is the live migrate case. The caller provided the
|
|
||||||
# allocation that needs to be used on the dest_node along with the
|
|
||||||
# expected consumer_generation of the consumer (which is the instance).
|
|
||||||
pass
|
|
||||||
|
|
||||||
if source_allocations:
|
if not source_allocations:
|
||||||
# Generate an allocation request for the destination node.
|
# This shouldn't happen, so just raise an error since we cannot
|
||||||
# NOTE(gibi): if the source allocation allocates from more than one RP
|
# proceed.
|
||||||
# then we need to fail as the dest allocation might also need to be
|
raise exception.ConsumerAllocationRetrievalFailed(
|
||||||
# complex (e.g. nested) and we cannot calculate that allocation request
|
consumer_uuid=instance.uuid,
|
||||||
# properly without a placement allocation candidate call.
|
error=_(
|
||||||
# Alternatively we could sum up the source allocation and try to
|
'Expected to find allocations for source node resource '
|
||||||
# allocate that from the root RP of the dest host. It would only work
|
'provider %s. Retry the operation without forcing a '
|
||||||
# if the dest host would not require nested allocation for this server
|
'destination host.') % source_node.uuid)
|
||||||
# which is really a rare case.
|
|
||||||
if len(source_allocations) > 1:
|
# Generate an allocation request for the destination node.
|
||||||
reason = (_('Unable to move instance %(instance_uuid)s to '
|
# NOTE(gibi): if the source allocation allocates from more than one RP
|
||||||
'host %(host)s. The instance has complex allocations '
|
# then we need to fail as the dest allocation might also need to be
|
||||||
'on the source host so move cannot be forced.') %
|
# complex (e.g. nested) and we cannot calculate that allocation request
|
||||||
{'instance_uuid': instance.uuid,
|
# properly without a placement allocation candidate call.
|
||||||
'host': dest_node.host})
|
# Alternatively we could sum up the source allocation and try to
|
||||||
raise exception.NoValidHost(reason=reason)
|
# allocate that from the root RP of the dest host. It would only work
|
||||||
alloc_request = {
|
# if the dest host would not require nested allocation for this server
|
||||||
'allocations': {
|
# which is really a rare case.
|
||||||
dest_node.uuid: {
|
if len(source_allocations) > 1:
|
||||||
'resources':
|
reason = (_('Unable to move instance %(instance_uuid)s to '
|
||||||
source_allocations[source_node.uuid]['resources']}
|
'host %(host)s. The instance has complex allocations '
|
||||||
},
|
'on the source host so move cannot be forced.') %
|
||||||
}
|
{'instance_uuid': instance.uuid,
|
||||||
# import locally to avoid cyclic import
|
'host': dest_node.host})
|
||||||
from nova.scheduler.client import report
|
raise exception.NoValidHost(reason=reason)
|
||||||
# The claim_resources method will check for existing allocations
|
alloc_request = {
|
||||||
# for the instance and effectively "double up" the allocations for
|
'allocations': {
|
||||||
# both the source and destination node. That's why when requesting
|
dest_node.uuid: {
|
||||||
# allocations for resources on the destination node before we move,
|
'resources':
|
||||||
# we use the existing resource allocations from the source node.
|
source_allocations[source_node.uuid]['resources']}
|
||||||
if reportclient.claim_resources(
|
},
|
||||||
context, instance.uuid, alloc_request,
|
}
|
||||||
instance.project_id, instance.user_id,
|
# import locally to avoid cyclic import
|
||||||
allocation_request_version=report.CONSUMER_GENERATION_VERSION,
|
from nova.scheduler.client import report
|
||||||
consumer_generation=consumer_generation):
|
# The claim_resources method will check for existing allocations
|
||||||
LOG.debug('Instance allocations successfully created on '
|
# for the instance and effectively "double up" the allocations for
|
||||||
'destination node %(dest)s: %(alloc_request)s',
|
# both the source and destination node. That's why when requesting
|
||||||
{'dest': dest_node.uuid,
|
# allocations for resources on the destination node before we move,
|
||||||
'alloc_request': alloc_request},
|
# we use the existing resource allocations from the source node.
|
||||||
instance=instance)
|
if reportclient.claim_resources(
|
||||||
else:
|
context, instance.uuid, alloc_request,
|
||||||
# We have to fail even though the user requested that we force
|
instance.project_id, instance.user_id,
|
||||||
# the host. This is because we need Placement to have an
|
allocation_request_version=report.CONSUMER_GENERATION_VERSION,
|
||||||
# accurate reflection of what's allocated on all nodes so the
|
consumer_generation=consumer_generation):
|
||||||
# scheduler can make accurate decisions about which nodes have
|
LOG.debug('Instance allocations successfully created on '
|
||||||
# capacity for building an instance.
|
'destination node %(dest)s: %(alloc_request)s',
|
||||||
reason = (_('Unable to move instance %(instance_uuid)s to '
|
{'dest': dest_node.uuid,
|
||||||
'host %(host)s. There is not enough capacity on '
|
'alloc_request': alloc_request},
|
||||||
'the host for the instance.') %
|
instance=instance)
|
||||||
{'instance_uuid': instance.uuid,
|
|
||||||
'host': dest_node.host})
|
|
||||||
raise exception.NoValidHost(reason=reason)
|
|
||||||
else:
|
else:
|
||||||
# This shouldn't happen, but it could be a case where there are
|
# We have to fail even though the user requested that we force
|
||||||
# older (Ocata) computes still so the existing allocations are
|
# the host. This is because we need Placement to have an
|
||||||
# getting overwritten by the update_available_resource periodic
|
# accurate reflection of what's allocated on all nodes so the
|
||||||
# task in the compute service.
|
# scheduler can make accurate decisions about which nodes have
|
||||||
# TODO(mriedem): Make this an error when the auto-heal
|
# capacity for building an instance.
|
||||||
# compatibility code in the resource tracker is removed.
|
reason = (_('Unable to move instance %(instance_uuid)s to '
|
||||||
LOG.warning('No instance allocations found for source node '
|
'host %(host)s. There is not enough capacity on '
|
||||||
'%(source)s in Placement. Not creating allocations '
|
'the host for the instance.') %
|
||||||
'for destination node %(dest)s and assuming the '
|
{'instance_uuid': instance.uuid,
|
||||||
'compute service will heal the allocations.',
|
'host': dest_node.host})
|
||||||
{'source': source_node.uuid, 'dest': dest_node.uuid},
|
raise exception.NoValidHost(reason=reason)
|
||||||
instance=instance)
|
|
||||||
|
|
||||||
|
|
||||||
def set_vm_state_and_notify(context, instance_uuid, service, method, updates,
|
def set_vm_state_and_notify(context, instance_uuid, service, method, updates,
|
||||||
|
|
|
@ -12,6 +12,7 @@
|
||||||
|
|
||||||
import mock
|
import mock
|
||||||
from oslo_utils.fixture import uuidsentinel as uuids
|
from oslo_utils.fixture import uuidsentinel as uuids
|
||||||
|
import six
|
||||||
|
|
||||||
from nova import context as nova_context
|
from nova import context as nova_context
|
||||||
from nova import exception
|
from nova import exception
|
||||||
|
@ -759,10 +760,15 @@ class TestUtils(test.NoDBTestCase):
|
||||||
'claim_resources',
|
'claim_resources',
|
||||||
new_callable=mock.NonCallableMock)
|
new_callable=mock.NonCallableMock)
|
||||||
def test(mock_claim, mock_get_allocs):
|
def test(mock_claim, mock_get_allocs):
|
||||||
utils.claim_resources_on_destination(
|
ex = self.assertRaises(
|
||||||
|
exception.ConsumerAllocationRetrievalFailed,
|
||||||
|
utils.claim_resources_on_destination,
|
||||||
self.context, reportclient, instance, source_node, dest_node)
|
self.context, reportclient, instance, source_node, dest_node)
|
||||||
mock_get_allocs.assert_called_once_with(
|
mock_get_allocs.assert_called_once_with(
|
||||||
self.context, instance.uuid)
|
self.context, instance.uuid)
|
||||||
|
self.assertIn(
|
||||||
|
'Expected to find allocations for source node resource '
|
||||||
|
'provider %s' % source_node.uuid, six.text_type(ex))
|
||||||
|
|
||||||
test()
|
test()
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue