Remove assumption of http error if consumer not exists

The heal_allocations_for_instance assumes that placement GET
/allocations/<instance_uuid> query returns an error code if the consumer
does not exists in placement. However placement returns an empty
allocation instead.

This patch removes such assumption and treates the negative response
from placement as a fatal error.

Change-Id: I7e2df32029e4cff57a0dddcd905b6c1aac207546
Closes-Bug: #1835419
This commit is contained in:
Balazs Gibizer
2019-07-04 16:15:33 +02:00
parent 1fdbe88ce7
commit e6f0119262
2 changed files with 8 additions and 20 deletions

View File

@@ -1718,20 +1718,17 @@ class PlacementCommands(object):
try: try:
allocations = placement.get_allocs_for_consumer( allocations = placement.get_allocs_for_consumer(
ctxt, instance.uuid) ctxt, instance.uuid)
except ks_exc.ClientException as e: except (ks_exc.ClientException,
exception.ConsumerAllocationRetrievalFailed) as e:
raise exception.AllocationUpdateFailed( raise exception.AllocationUpdateFailed(
consumer_uuid=instance.uuid, consumer_uuid=instance.uuid,
error=_("Allocation retrieval failed: %s") % e) error=_("Allocation retrieval failed: %s") % e)
except exception.ConsumerAllocationRetrievalFailed as e:
output(_("Allocation retrieval failed: %s") % e)
allocations = None
need_healing = False need_healing = False
# get_allocations_for_consumer uses safe_connect which will
# return None if we can't communicate with Placement, and the # Placement response can have an empty {'allocations': {}} in it if
# response can have an empty {'allocations': {}} response if # there are no allocations for the instance
# there are no allocations for the instance so handle both if not allocations.get('allocations'):
if not allocations or not allocations.get('allocations'):
# This instance doesn't have allocations # This instance doesn't have allocations
need_healing = 'Create' need_healing = 'Create'
allocations = self._heal_missing_alloc(ctxt, instance, node_cache) allocations = self._heal_missing_alloc(ctxt, instance, node_cache)

View File

@@ -2531,20 +2531,11 @@ class TestNovaManagePlacement(test.NoDBTestCase):
new=mock.Mock( new=mock.Mock(
side_effect=exception.ConsumerAllocationRetrievalFailed( side_effect=exception.ConsumerAllocationRetrievalFailed(
consumer_uuid='CONSUMER', error='ERROR'))) consumer_uuid='CONSUMER', error='ERROR')))
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename',
new=mock.Mock(
return_value=objects.ComputeNode(uuid=uuidsentinel.node)))
@mock.patch('nova.scheduler.utils.resources_from_flavor',
new=mock.Mock(return_value=mock.sentinel.resources))
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.put', @mock.patch('nova.scheduler.client.report.SchedulerReportClient.put',
return_value=fake_requests.FakeResponse(204)) new_callable=mock.NonCallableMagicMock)
def test_heal_allocations_get_allocs_retrieval_fails(self, mock_put, def test_heal_allocations_get_allocs_retrieval_fails(self, mock_put,
mock_getinst): mock_getinst):
# This "succeeds" self.assertEqual(3, self.cli.heal_allocations())
self.assertEqual(0, self.cli.heal_allocations())
# We're really just verifying that we got to the end
mock_put.assert_called_once()
self.assertEqual(2, mock_getinst.call_count)
@mock.patch('nova.objects.CellMappingList.get_all', @mock.patch('nova.objects.CellMappingList.get_all',
return_value=objects.CellMappingList(objects=[ return_value=objects.CellMappingList(objects=[