openstack
/
nova
Code Issues Proposed changes

Use _evacuate_server helper in func test 

We have a nice helper for calling evacuate and waiting for the result so
let's replace the direct post api calls in func test with the helper.

Change-Id: I87a10e6ad65ae493cd34c6c0e62d60c5d6f9d1d8
This commit is contained in:
Balazs Gibizer 2020-10-14 18:35:42 +02:00
parent edd8fefe3f
commit f419f1a4b1
13 changed files with 95 additions and 133 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
nova/tests/functional/integrated_helpers.py
View File

@ -94,6 +94,12 @@ class StubComputeRPCAPI(compute_rpcapi.ComputeAPI):
return rpc.ClientRouter(default_client)
# placeholder used as a default parameter value to distinguish between the case
# when the parameter is specified by the caller with None from the case when it
# was not specified
NOT_SPECIFIED = object()
class InstanceHelperMixin:
def _wait_for_server_parameter(
@ -485,11 +491,32 @@ class InstanceHelperMixin:
self.api.post_server_action(server['id'], {'unshelve': {}})
return self._wait_for_state_change(server, expected_state)
def _evacuate_server(self, server, host, expected_state='ACTIVE'):
def _evacuate_server(
self, server, extra_post_args=None, expected_host=None,
expected_state='ACTIVE', expected_task_state=NOT_SPECIFIED,
expected_migration_status='done'):
"""Evacuate a server."""
self.api.post_server_action(server['id'], {'evacuate': {}})
self._wait_for_server_parameter(
server, {'OS-EXT-SRV-ATTR:host': host, 'status': expected_state})
api = getattr(self, 'admin_api', self.api)
post = {'evacuate': {}}
if extra_post_args:
post['evacuate'].update(extra_post_args)
expected_result = {'status': expected_state}
if expected_host:
expected_result['OS-EXT-SRV-ATTR:host'] = expected_host
if expected_task_state is not NOT_SPECIFIED:
expected_result['OS-EXT-STS:task_state'] = expected_task_state
api.post_server_action(server['id'], post)
# NOTE(gibi): The order of waiting for the migration and returning
# a fresh server from _wait_for_server_parameter is important as
# the compute manager sets status of the instance before sets the
# host and finally sets the migration status. So waiting for the
# migration first makes the returned server object more consistent.
self._wait_for_migration_status(server, [expected_migration_status])
return self._wait_for_server_parameter(server, expected_result)
class PlacementHelperMixin:

9
nova/tests/functional/libvirt/test_evacuate.py
View File

@ -514,20 +514,17 @@ class _LibvirtEvacuateTest(integrated_helpers.InstanceHelperMixin):
def _evacuate_with_failure(self, server, compute1):
# Perform an evacuation during which we experience a failure on the
# destination host
instance_uuid = server['id']
with mock.patch.object(compute1.driver, 'plug_vifs') as plug_vifs:
plug_vifs.side_effect = test.TestingException
self.api.post_server_action(instance_uuid,
{'evacuate': {'host': 'compute1'}})
server = self._evacuate_server(
server, {'host': 'compute1'}, expected_state='ERROR',
expected_task_state=None, expected_migration_status='failed')
# Wait for the rebuild to start, then complete
fake_notifier.wait_for_versioned_notifications(
'instance.rebuild.start')
self._wait_for_migration_status(server, ['failed'])
server = self._wait_for_server_parameter(
server, {'OS-EXT-STS:task_state': None})
# Meta-test
plug_vifs.assert_called()

4
nova/tests/functional/notification_sample_tests/test_compute_task.py
View File

@ -69,9 +69,9 @@ class TestComputeTaskNotificationSample(
# NOTE(takashin): The rebuild action and the evacuate action shares
# same code path. So the 'evacuate' action is used for this test.
post = {'evacuate': {}}
self._evacuate_server(
server, expected_state='ERROR', expected_migration_status='error')
self.admin_api.post_server_action(server['id'], post)
self._wait_for_notification('compute_task.rebuild_server.error')
# 0. instance.evacuate
# 1. scheduler.select_destinations.start

11
nova/tests/functional/regressions/test_bug_1669054.py
View File

@ -57,14 +57,9 @@ class ResizeEvacuateTestCase(integrated_helpers._IntegratedTestBase):
host2.stop()
self.api.force_down_service('host2', 'nova-compute', forced_down=True)
# Now try to evacuate the server back to the original source compute.
req = {'evacuate': {'onSharedStorage': False}}
self.api.post_server_action(server['id'], req)
server = self._wait_for_state_change(server, 'ACTIVE')
# The evacuate flow in the compute manager is annoying in that it
# sets the instance status to ACTIVE before updating the host, so we
# have to wait for the migration record to be 'done' to avoid a race.
self._wait_for_migration_status(server, ['done'])
self.assertEqual(self.compute.host, server['OS-EXT-SRV-ATTR:host'])
server = self._evacuate_server(
server, {'onSharedStorage': 'False'},
expected_host=self.compute.host, expected_migration_status='done')
# Assert the RequestSpec.ignore_hosts field is not populated.
reqspec = objects.RequestSpec.get_by_instance_uuid(

9
nova/tests/functional/regressions/test_bug_1713783.py
View File

@ -96,14 +96,11 @@ class FailedEvacuateStateTests(test.TestCase,
fake_notifier.reset()
# Initiate evacuation
post = {'evacuate': {}}
self.api.post_server_action(server['id'], post)
self._evacuate_server(
server, expected_state='ERROR', expected_host=self.hostname,
expected_migration_status='error')
self._wait_for_notification_event_type('compute_task.rebuild_server')
server = self._wait_for_state_change(server, 'ERROR')
self.assertEqual(self.hostname, server['OS-EXT-SRV-ATTR:host'])
# Check migrations
migrations = self.api.get_migrations()
self.assertEqual(1, len(migrations))

9
nova/tests/functional/regressions/test_bug_1764883.py
View File

@ -97,12 +97,9 @@ class TestEvacuationWithSourceReturningDuringRebuild(
self.computes.get(self.source_compute).stop()
self.api.force_down_service(self.source_compute, 'nova-compute', True)
# Start evacuating the instance from the source_host
self.api.post_server_action(server['id'], {'evacuate': {}})
# Wait for the instance to go into an ACTIVE state
self._wait_for_state_change(server, 'ACTIVE')
server = self.api.get_server(server['id'])
# Evacuate the instance from the source_host
server = self._evacuate_server(
server, expected_migration_status='done')
host = server['OS-EXT-SRV-ATTR:host']
migrations = self.api.get_migrations()

7
nova/tests/functional/regressions/test_bug_1794996.py
View File

@ -52,12 +52,7 @@ class TestEvacuateDeleteServerRestartOriginalCompute(
source_compute_id, {'forced_down': 'true'})
# evacuate the server
post = {'evacuate': {}}
self.api.post_server_action(
server['id'], post)
expected_params = {'OS-EXT-SRV-ATTR:host': dest_hostname,
'status': 'ACTIVE'}
server = self._wait_for_server_parameter(server, expected_params)
server = self._evacuate_server(server, expected_host=dest_hostname)
# Expect to have allocation and usages on both computes as the
# source compute is still down

8
nova/tests/functional/regressions/test_bug_1815153.py
View File

@ -142,11 +142,9 @@ class NonPersistentFieldNotResetTest(
# Its status becomes 'ACTIVE'.
# If requested_destination is not reset, a status of the server
# becomes 'ERROR' because the target host is down.
self.api.post_server_action(
server['id'], {'evacuate': {'host': target_host}})
expected_params = {'OS-EXT-SRV-ATTR:host': original_host,
'status': 'ERROR'}
server = self._wait_for_server_parameter(server, expected_params)
server = self._evacuate_server(
server, {'host': target_host}, expected_host=original_host,
expected_state='ERROR', expected_migration_status='error')
# Make sure 'is_bfv' is set.
reqspec = objects.RequestSpec.get_by_instance_uuid(self.ctxt,

8
nova/tests/functional/regressions/test_bug_1823370.py
View File

@ -64,8 +64,6 @@ class MultiCellEvacuateTestCase(integrated_helpers._IntegratedTestBase):
# Now evacuate the server which should send it to host3 since it is
# in the same cell as host1, even though host2 in cell2 is weighed
# higher than host3.
req = {'evacuate': {'onSharedStorage': False}}
self.api.post_server_action(server['id'], req)
self._wait_for_migration_status(server, ['done'])
server = self._wait_for_state_change(server, 'ACTIVE')
self.assertEqual('host3', server['OS-EXT-SRV-ATTR:host'])
self._evacuate_server(
server, {'onSharedStorage': 'False'}, expected_host='host3',
expected_migration_status='done')

7
nova/tests/functional/test_nova_manage.py
View File

@ -1713,11 +1713,8 @@ class TestNovaManagePlacementAudit(
self.admin_api.put_service(source_service_id, {'forced_down': 'true'})
# evacuate the instance to the target
post = {'evacuate': {"host": dest_hostname}}
self.admin_api.post_server_action(server['id'], post)
self._wait_for_server_parameter(server,
{'OS-EXT-SRV-ATTR:host': dest_hostname,
'status': 'ACTIVE'})
self._evacuate_server(
server, {'host': dest_hostname}, expected_host=dest_hostname)
# Now the instance is gone, we can delete the compute service
self.admin_api.api_delete('/os-services/%s' % source_service_id)

51
nova/tests/functional/test_server_group.py
View File

@ -446,10 +446,9 @@ class ServerGroupTestV21(ServerGroupTestBase):
# Start additional host to test evacuation
self.start_service('compute', host='host3')
post = {'evacuate': {'onSharedStorage': False}}
self.admin_api.post_server_action(servers[1]['id'], post)
self._wait_for_migration_status(servers[1], ['done'])
evacuated_server = self._wait_for_state_change(servers[1], 'ACTIVE')
evacuated_server = self._evacuate_server(
servers[1], {'onSharedStorage': 'False'},
expected_migration_status='done')
# check that the server is evacuated to another host
self.assertNotEqual(evacuated_server['OS-EXT-SRV-ATTR:host'],
@ -467,11 +466,9 @@ class ServerGroupTestV21(ServerGroupTestBase):
# Set forced_down on the host to ensure nova considers the host down.
self._set_forced_down(host, True)
post = {'evacuate': {'onSharedStorage': False}}
self.admin_api.post_server_action(servers[1]['id'], post)
self._wait_for_migration_status(servers[1], ['error'])
server_after_failed_evac = self._wait_for_state_change(
servers[1], 'ERROR')
server_after_failed_evac = self._evacuate_server(
servers[1], {'onSharedStorage': 'False'}, expected_state='ERROR',
expected_migration_status='error')
# assert that after a failed evac the server active on the same host
# as before
@ -487,11 +484,9 @@ class ServerGroupTestV21(ServerGroupTestBase):
# Set forced_down on the host to ensure nova considers the host down.
self._set_forced_down(host, True)
post = {'evacuate': {'onSharedStorage': False}}
self.admin_api.post_server_action(servers[1]['id'], post)
self._wait_for_migration_status(servers[1], ['error'])
server_after_failed_evac = self._wait_for_state_change(
servers[1], 'ERROR')
server_after_failed_evac = self._evacuate_server(
servers[1], {'onSharedStorage': 'False'}, expected_state='ERROR',
expected_migration_status='error')
# assert that after a failed evac the server active on the same host
# as before
@ -629,10 +624,8 @@ class ServerGroupTestV215(ServerGroupTestV21):
# Start additional host to test evacuation
compute3 = self.start_service('compute', host='host3')
post = {'evacuate': {}}
self.admin_api.post_server_action(servers[1]['id'], post)
self._wait_for_migration_status(servers[1], ['done'])
evacuated_server = self._wait_for_state_change(servers[1], 'ACTIVE')
evacuated_server = self._evacuate_server(
servers[1], expected_migration_status='done')
# check that the server is evacuated
self.assertNotEqual(evacuated_server['OS-EXT-SRV-ATTR:host'],
@ -652,11 +645,9 @@ class ServerGroupTestV215(ServerGroupTestV21):
# Set forced_down on the host to ensure nova considers the host down.
self._set_forced_down(host, True)
post = {'evacuate': {}}
self.admin_api.post_server_action(servers[1]['id'], post)
self._wait_for_migration_status(servers[1], ['error'])
server_after_failed_evac = self._wait_for_state_change(
servers[1], 'ERROR')
server_after_failed_evac = self._evacuate_server(
servers[1], expected_state='ERROR',
expected_migration_status='error')
# assert that after a failed evac the server active on the same host
# as before
@ -672,11 +663,9 @@ class ServerGroupTestV215(ServerGroupTestV21):
# Set forced_down on the host to ensure nova considers the host down.
self._set_forced_down(host, True)
post = {'evacuate': {}}
self.admin_api.post_server_action(servers[1]['id'], post)
self._wait_for_migration_status(servers[1], ['error'])
server_after_failed_evac = self._wait_for_state_change(
servers[1], 'ERROR')
server_after_failed_evac = self._evacuate_server(
servers[1], expected_state='ERROR',
expected_migration_status='error')
# assert that after a failed evac the server active on the same host
# as before
@ -814,10 +803,8 @@ class ServerGroupTestV215(ServerGroupTestV21):
# Set forced_down on the host to ensure nova considers the host down.
self._set_forced_down(host, True)
post = {'evacuate': {}}
self.admin_api.post_server_action(servers[1]['id'], post)
self._wait_for_migration_status(servers[1], ['done'])
evacuated_server = self._wait_for_state_change(servers[1], 'ACTIVE')
evacuated_server = self._evacuate_server(
servers[1], expected_migration_status='done')
# Note(gibi): need to get the server again as the state of the instance
# goes to ACTIVE first then the host of the instance changes to the

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

@ -2045,8 +2045,11 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
fake_notifier.reset()
# Initiate evacuation
post = {'evacuate': {}}
self.api.post_server_action(server['id'], post)
# There is no other host to evacuate to so the rebuild should put the
# VM to ERROR state, but it should remain on source compute
server = self._evacuate_server(
server, expected_state='ERROR', expected_host=source_hostname,
expected_migration_status='error')
# NOTE(elod.illes): Should be changed to non-polling solution when
# patch https://review.opendev.org/#/c/482629/ gets merged:
@ -2056,13 +2059,6 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
self._run_periodics()
# There is no other host to evacuate to so the rebuild should put the
# VM to ERROR state, but it should remain on source compute
expected_params = {'OS-EXT-SRV-ATTR:host': source_hostname,
'status': 'ERROR'}
server = self._wait_for_server_parameter(server,
expected_params)
# Check migrations
migrations = self.api.get_migrations()
self.assertEqual(1, len(migrations))
@ -2140,13 +2136,7 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
source_compute_id, {'forced_down': 'true'})
# evacuate the server
post = {'evacuate': {}}
self.api.post_server_action(
server['id'], post)
expected_params = {'OS-EXT-SRV-ATTR:host': dest_hostname,
'status': 'ACTIVE'}
server = self._wait_for_server_parameter(server,
expected_params)
server = self._evacuate_server(server, expected_host=dest_hostname)
# Expect to have allocation and usages on both computes as the
# source compute is still down
@ -2398,11 +2388,11 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
with mock.patch('nova.compute.claims.MoveClaim', fake_move_claim):
# evacuate the server
self.api.post_server_action(server['id'], {'evacuate': {}})
# the migration will fail on the dest node and the instance will
# stay ACTIVE and task_state will be set to None.
server = self._wait_for_server_parameter(
server, {'status': 'ACTIVE', 'OS-EXT-STS:task_state': None})
server = self._evacuate_server(
server, expected_task_state=None,
expected_migration_status='failed')
# Run the periodics to show those don't modify allocations.
self._run_periodics()
@ -2474,10 +2464,11 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
with mock.patch.object(
self.compute2.driver, 'rebuild', fake_rebuild):
# evacuate the server
self.api.post_server_action(server['id'], {'evacuate': {}})
# the migration will fail on the dest node and the instance will
# go into error state
server = self._wait_for_state_change(server, 'ERROR')
server = self._evacuate_server(
server, expected_state='ERROR',
expected_migration_status='failed')
# Run the periodics to show those don't modify allocations.
self._run_periodics()
@ -7162,14 +7153,8 @@ class ServerMoveWithPortResourceRequestTest(
self.admin_api.put_service(
self.compute1_service_id, {'forced_down': 'true'})
req = {'evacuate': {}}
if host:
req['evacuate']['host'] = host
self.api.post_server_action(server['id'], req)
self._wait_for_server_parameter(server,
{'OS-EXT-SRV-ATTR:host': 'host2',
'status': 'ACTIVE'})
self._evacuate_server(
server, {'host': host} if host else {}, expected_host='host2')
self._check_allocation_during_evacuate(
server, self.flavor_with_group_policy, self.compute1_rp_uuid,
@ -7212,16 +7197,11 @@ class ServerMoveWithPortResourceRequestTest(
'nova.compute.resource_tracker.ResourceTracker.rebuild_claim',
side_effect=exception.ComputeResourcesUnavailable(
reason='test evacuate failure')):
req = {'evacuate': {}}
self.api.post_server_action(server['id'], req)
# Evacuate does not have reschedule loop so evacuate expected to
# simply fail and the server remains on the source host
server = self._wait_for_server_parameter(server,
{'OS-EXT-SRV-ATTR:host': 'host1',
'status': 'ACTIVE',
'OS-EXT-STS:task_state': None})
self._wait_for_migration_status(server, ['failed'])
server = self._evacuate_server(
server, expected_host='host1', expected_task_state=None,
expected_migration_status='failed')
# As evacuation failed the resource allocation should be untouched
self._check_allocation(
@ -7267,12 +7247,9 @@ class ServerMoveWithPortResourceRequestTest(
# The compute manager on host2 will raise from
# update_pci_request_spec_with_allocated_interface_name
self.api.post_server_action(server['id'], {'evacuate': {}})
server = self._wait_for_server_parameter(server,
{'OS-EXT-SRV-ATTR:host': 'host1',
'OS-EXT-STS:task_state': None,
'status': 'ERROR'})
self._wait_for_migration_status(server, ['failed'])
server = self._evacuate_server(
server, expected_host='host1', expected_state='ERROR',
expected_task_state=None, expected_migration_status='failed')
self.assertIn(
'does not have a properly formatted name',
@ -8179,7 +8156,7 @@ class AcceleratorServerOpsTest(AcceleratorServerBase):
arqs = self.cyborg.fake_get_arqs_for_instance(self.server['id'])
compute_to_stop, compute_to_evacuate = self._test_evacuate(
self.server, self.NUM_HOSTS)
self._evacuate_server(self.server, compute_to_evacuate.host)
self._evacuate_server(self.server, {'host': compute_to_evacuate.host})
compute_to_stop.start()
self.server = self.api.get_server(self.server['id'])
arqs_new = self.cyborg.fake_get_arqs_for_instance(self.server['id'])

5
nova/tests/functional/wsgi/test_services.py
View File

@ -143,12 +143,9 @@ class TestServicesAPI(integrated_helpers.ProviderUsageBaseTestCase):
host1.stop()
# Start another host and trigger the server evacuate to that host.
self._start_compute('host2')
self.admin_api.post_server_action(server['id'], {'evacuate': {}})
# The host does not change until after the status is changed to ACTIVE
# so wait for both parameters.
self._wait_for_server_parameter(server, {
'status': 'ACTIVE',
'OS-EXT-SRV-ATTR:host': 'host2'})
self._evacuate_server(server, expected_host='host2')
# Delete the compute service for host1 and check the related
# placement resources for that host.
self.admin_api.api_delete('/os-services/%s' % service['id'])