Fix the race in confirm resize func test
New confirm resize functional tests were added in Ia69dace6d3b395fa113c1382e8eb52c2cc36119d. But they could be racy [1] because the migration allocation is deleted in placement as the last thing in the confirm_resize process. So it happens after the migration is set to confirmed and the instance state is set to ACTIVE. This patch changes the confirm resize tests to wait for the successful instance action event as that is sent _after_ the migration allocation is deleted. [1] see the bug report Closes-Bug: #1843433 Change-Id: Ibeb16ce16263c43bad9f148480bbebca413d8117
This commit is contained in:
Balazs Gibizer
Sylvain Bauza
parent
43afc0443c
commit
07171778e6
|
|
@ -27,6 +27,7 @@ import os_traits
|
|||
from oslo_log import log as logging
|
||||
from oslo_utils.fixture import uuidsentinel as uuids
|
||||
|
||||
from nova.compute import instance_actions
|
||||
from nova.compute import utils as compute_utils
|
||||
import nova.conf
|
||||
from nova import context
|
||||
|
|
@ -309,12 +310,21 @@ class InstanceHelperMixin(object):
|
|||
"""
|
||||
if api is None:
|
||||
api = self.api
|
||||
completion_event = None
|
||||
return self._wait_for_instance_action_event(
|
||||
api, server, expected_action, event_name, event_result='error')
|
||||
|
||||
def _wait_for_instance_action_event(
|
||||
self, api, server, action_name, event_name, event_result):
|
||||
"""Polls the instance action events for the given instance, action,
|
||||
event, and event result until it finds the event.
|
||||
"""
|
||||
actions = []
|
||||
events = []
|
||||
for attempt in range(10):
|
||||
actions = api.get_instance_actions(server['id'])
|
||||
# Look for the migrate action.
|
||||
# The API returns the newest event first
|
||||
for action in actions:
|
||||
if action['action'] == expected_action:
|
||||
if action['action'] == action_name:
|
||||
events = (
|
||||
api.api_get(
|
||||
'/servers/%s/os-instance-actions/%s' %
|
||||
|
|
@ -322,21 +332,18 @@ class InstanceHelperMixin(object):
|
|||
).body['instanceAction']['events'])
|
||||
# Look for the action event being in error state.
|
||||
for event in events:
|
||||
result = event['result']
|
||||
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
|
||||
result is not None and
|
||||
result.lower() == event_result.lower()):
|
||||
return event
|
||||
# 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))
|
||||
self.fail(
|
||||
'Timed out waiting for %s instance action event. Current instance '
|
||||
'actions: %s. Events in the last matching action: %s'
|
||||
% (event_name, actions, events))
|
||||
|
||||
def _wait_for_migration_status(self, server, expected_statuses):
|
||||
"""Waits for a migration record with the given statuses to be found
|
||||
|
|
@ -912,3 +919,11 @@ class ProviderUsageBaseTestCase(test.TestCase, InstanceHelperMixin):
|
|||
# Account for reserved_host_cpus.
|
||||
expected_vcpu_usage = CONF.reserved_host_cpus + flavor['vcpus']
|
||||
self.assertEqual(expected_vcpu_usage, hypervisor['vcpus_used'])
|
||||
|
||||
def _confirm_resize(self, server):
|
||||
self.api.post_server_action(server['id'], {'confirmResize': None})
|
||||
server = self._wait_for_state_change(self.api, server, 'ACTIVE')
|
||||
self._wait_for_instance_action_event(
|
||||
self.api, server, instance_actions.CONFIRM_RESIZE,
|
||||
'compute_confirm_resize', 'success')
|
||||
return server
|
||||
|
|
|
|||
|
|
@ -1811,10 +1811,7 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
|
|||
source_rp_uuid, dest_rp_uuid)
|
||||
|
||||
# Confirm the resize and check the usages
|
||||
post = {'confirmResize': None}
|
||||
self.api.post_server_action(
|
||||
server['id'], post, check_response_status=[204])
|
||||
self._wait_for_state_change(self.api, server, 'ACTIVE')
|
||||
self._confirm_resize(server)
|
||||
|
||||
# After confirming, we should have an allocation only on the
|
||||
# destination host
|
||||
|
|
@ -1889,10 +1886,7 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
|
|||
server, self.flavor2, self.flavor3, rp_uuid)
|
||||
|
||||
# Confirm the resize and check the usages
|
||||
post = {'confirmResize': None}
|
||||
self.api.post_server_action(
|
||||
server['id'], post, check_response_status=[204])
|
||||
self._wait_for_state_change(self.api, server, 'ACTIVE')
|
||||
self._confirm_resize(server)
|
||||
|
||||
self._run_periodics()
|
||||
|
||||
|
|
@ -2020,8 +2014,7 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
|
|||
dest_rp_uuid, self.flavor2, volume_backed=False)
|
||||
|
||||
# Now confirm the resize and check hypervisor usage again.
|
||||
self.api.post_server_action(server['id'], {'confirmResize': None})
|
||||
self._wait_for_state_change(self.api, server, 'ACTIVE')
|
||||
self._confirm_resize(server)
|
||||
|
||||
# There should no resource usage for flavor1 on the source host.
|
||||
self.assert_hypervisor_usage(
|
||||
|
|
@ -3125,11 +3118,7 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
|
|||
self._wait_for_state_change(self.api, created_server, 'VERIFY_RESIZE')
|
||||
|
||||
# Confirm the resize
|
||||
post = {'confirmResize': None}
|
||||
self.api.post_server_action(
|
||||
created_server['id'], post, check_response_status=[204])
|
||||
new_server = self._wait_for_state_change(self.api, created_server,
|
||||
'ACTIVE')
|
||||
new_server = self._confirm_resize(created_server)
|
||||
inst_dest_host = new_server["OS-EXT-SRV-ATTR:host"]
|
||||
|
||||
self.assertEqual(dest_hostname, inst_dest_host)
|
||||
|
|
@ -3300,10 +3289,7 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
|
|||
server, self.flavor1, source_rp_uuid, dest_rp_uuid)
|
||||
|
||||
# Confirm the move and check the usages
|
||||
post = {'confirmResize': None}
|
||||
self.api.post_server_action(
|
||||
server['id'], post, check_response_status=[204])
|
||||
self._wait_for_state_change(self.api, server, 'ACTIVE')
|
||||
self._confirm_resize(server)
|
||||
|
||||
def _check_allocation():
|
||||
# the target host usage should be according to the flavor
|
||||
|
|
@ -3951,8 +3937,7 @@ class VolumeBackedServerTest(integrated_helpers.ProviderUsageBaseTestCase):
|
|||
self.admin_api, server, 'VERIFY_RESIZE')
|
||||
self.assertEqual('host2', server['OS-EXT-SRV-ATTR:host'])
|
||||
# Confirm the cold migration and check usage and the request spec.
|
||||
self.api.post_server_action(server['id'], {'confirmResize': None})
|
||||
self._wait_for_state_change(self.api, server, 'ACTIVE')
|
||||
self._confirm_resize(server)
|
||||
reqspec = objects.RequestSpec.get_by_instance_uuid(ctxt, server['id'])
|
||||
# Make sure it's set.
|
||||
self.assertTrue(reqspec.is_bfv)
|
||||
|
|
@ -6552,9 +6537,7 @@ class ServerMoveWithPortResourceRequestTest(
|
|||
server, compute3_rp_uuid, non_qos_port, qos_port,
|
||||
migration_uuid, source_compute_rp_uuid=self.compute1_rp_uuid)
|
||||
|
||||
self.api.post_server_action(server['id'], {'confirmResize': None})
|
||||
self._wait_for_migration_status(server, ['confirmed'])
|
||||
|
||||
self._confirm_resize(server)
|
||||
# check that allocation is still OK
|
||||
self._check_allocation(
|
||||
server, compute3_rp_uuid, non_qos_port, qos_port)
|
||||
|
|
@ -6585,8 +6568,7 @@ class ServerMoveWithPortResourceRequestTest(
|
|||
server, self.compute2_rp_uuid, non_qos_port, qos_port,
|
||||
migration_uuid, source_compute_rp_uuid=self.compute1_rp_uuid)
|
||||
|
||||
self.api.post_server_action(server['id'], {'confirmResize': None})
|
||||
self._wait_for_migration_status(server, ['confirmed'])
|
||||
self._confirm_resize(server)
|
||||
|
||||
# check that allocation is still OK
|
||||
self._check_allocation(
|
||||
|
|
|
|||
Loading…
Reference in New Issue