From 3d698040a17f39954fe095502dafb2b193120243 Mon Sep 17 00:00:00 2001 From: Alexey Stupnikov Date: Sat, 19 Feb 2022 21:38:44 +0100 Subject: Add functional tests to reproduce bug #1960412 Instance would be affected by problems described in bug #1949808 and bug #1960412 when queued live migration is aborted. This change adds functional test to reproduce problems with placement allocations (record for aborted live migration is not removed when queued live migration is aborted) and with Neutron port bindings (INACTIVE port binding records for destination host are not removed when queued live migration is aborted). It looks like there are no other modifications introduced by Nova control plane which should be reverted when queued live migration is aborted. This patch also changes neutron fixture: - neutron fixture was changed to improve active port binding's tracking during live migration: without this change port's binding:host_id is not updated when activate_port_binding() is called. As a result, list_ports() function returns empty list when constants.BINDING_HOST_ID is used in search_opts, which is the case for setup_networks_on_host() called with teardown=True. Conflicts: - nova/tests/fixtures/libvirt.py - nova/tests/fixtures/neutron.py NOTE. There is no need to change libvirt fixture because original problem with lack of address element is no longer there (I also removed related note from commit message itself). NeutronFixture class is defined in different place instable/wallaby, but code staus the same. Related-bug: #1960412 Related-bug: #1949808 Change-Id: I152581deb6e659c551f78eed66e4b0b958b20c53 (cherry picked from commit 1ad287bf9a8f65ce68c14f4634775f58abda15c2) (cherry picked from commit 479b8db3ab07dd1f50c029904cca17f3a5708685) --- nova/tests/fixtures.py | 7 +- .../functional/libvirt/test_live_migration.py | 116 ++++++++++++++++++++- 2 files changed, 116 insertions(+), 7 deletions(-) diff --git a/nova/tests/fixtures.py b/nova/tests/fixtures.py index 4d1a06f616..360f820a6f 100644 --- a/nova/tests/fixtures.py +++ b/nova/tests/fixtures.py @@ -1844,13 +1844,16 @@ class NeutronFixture(fixtures.Fixture): return fake_requests.FakeResponse(204) - def _activate_port_binding(self, port_id, host): + def _activate_port_binding(self, port_id, host, modify_port=False): # It makes sure that only one binding is active for a port for h, binding in self._port_bindings[port_id].items(): if h == host: # NOTE(gibi): neutron returns 409 if this binding is already # active but nova does not depend on this behaviour yet. binding['status'] = 'ACTIVE' + if modify_port: + # We need to ensure that port's binding:host_id is valid + self._merge_in_active_binding(self._ports[port_id]) else: binding['status'] = 'INACTIVE' @@ -1860,7 +1863,7 @@ class NeutronFixture(fixtures.Fixture): if failure is not None: return failure - self._activate_port_binding(port_id, host) + self._activate_port_binding(port_id, host, modify_port=True) return fake_requests.FakeResponse(200) diff --git a/nova/tests/functional/libvirt/test_live_migration.py b/nova/tests/functional/libvirt/test_live_migration.py index f714a5f043..720f2bcb1d 100644 --- a/nova/tests/functional/libvirt/test_live_migration.py +++ b/nova/tests/functional/libvirt/test_live_migration.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +import copy import threading from lxml import etree @@ -19,15 +20,18 @@ from nova.tests.functional import integrated_helpers from nova.tests.functional.libvirt import base as libvirt_base -class LiveMigrationQueuedAbortTest( +class LiveMigrationWithLockBase( libvirt_base.LibvirtMigrationMixin, libvirt_base.ServersTestBase, integrated_helpers.InstanceHelperMixin ): - """Functional test for bug 1949808. + """Base for live migration tests which require live migration to be + locked for certain period of time and then unlocked afterwards. - This test is used to confirm that VM's state is reverted properly - when queued Live migration is aborted. + Separate base class is needed because locking mechanism could work + in an unpredicted way if two tests for the same class would try to + use it simultaneously. Every test using this mechanism should use + separate class instance. """ api_major_version = 'v2.1' @@ -69,7 +73,15 @@ class LiveMigrationQueuedAbortTest( dom = conn.lookupByUUIDString(server) dom.complete_job() - def test_queued_live_migration_abort(self): + +class LiveMigrationQueuedAbortTestVmStatus(LiveMigrationWithLockBase): + """Functional test for bug #1949808. + + This test is used to confirm that VM's state is reverted properly + when queued Live migration is aborted. + """ + + def test_queued_live_migration_abort_vm_status(self): # Lock live migrations self.lock_live_migration.acquire() @@ -115,3 +127,97 @@ class LiveMigrationQueuedAbortTest( AssertionError, self._wait_for_state_change, self.server_b, 'ACTIVE') self._wait_for_state_change(self.server_b, 'MIGRATING') + + +class LiveMigrationQueuedAbortTestLeftoversRemoved(LiveMigrationWithLockBase): + """Functional test for bug #1960412. + + Placement allocations for live migration and inactive Neutron port + bindings on destination host created by Nova control plane when live + migration is initiated should be removed when queued live migration + is aborted using Nova API. + """ + + def test_queued_live_migration_abort_leftovers_removed(self): + # Lock live migrations + self.lock_live_migration.acquire() + + # Start instances: first one would be used to occupy + # executor's live migration queue, second one would be used + # to actually confirm that queued live migrations are + # aborted properly. + # port_1 is created automatically when neutron fixture is + # initialized, port_2 is created manually + self.server_a = self._create_server( + host=self.src_hostname, + networks=[{'port': self.neutron.port_1['id']}]) + self.neutron.create_port({'port': self.neutron.port_2}) + self.server_b = self._create_server( + host=self.src_hostname, + networks=[{'port': self.neutron.port_2['id']}]) + # Issue live migration requests for both servers. We expect that + # server_a live migration would be running, but locked by + # self.lock_live_migration and server_b live migration would be + # queued. + self._live_migrate( + self.server_a, + migration_expected_state='running', + server_expected_state='MIGRATING' + ) + self._live_migrate( + self.server_b, + migration_expected_state='queued', + server_expected_state='MIGRATING' + ) + + # Abort live migration for server_b + migration_server_a = self.api.api_get( + '/os-migrations?instance_uuid=%s' % self.server_a['id'] + ).body['migrations'].pop() + migration_server_b = self.api.api_get( + '/os-migrations?instance_uuid=%s' % self.server_b['id'] + ).body['migrations'].pop() + + self.api.api_delete( + '/servers/%s/migrations/%s' % (self.server_b['id'], + migration_server_b['id'])) + self._wait_for_migration_status(self.server_b, ['cancelled']) + # Unlock live migrations and confirm that server_a becomes + # active again after successful live migration + self.lock_live_migration.release() + self._wait_for_state_change(self.server_a, 'ACTIVE') + self._wait_for_migration_status(self.server_a, ['completed']) + # FIXME(astupnikov) Assert the server_b never comes out of 'MIGRATING' + # This should be fixed after bug #1949808 is addressed + self._wait_for_state_change(self.server_b, 'MIGRATING') + + # FIXME(astupnikov) Because of bug #1960412 allocations for aborted + # queued live migration (server_b) would not be removed. Allocations + # for completed live migration (server_a) should be empty. + allocations_server_a_migration = self.placement.get( + '/allocations/%s' % migration_server_a['uuid'] + ).body['allocations'] + self.assertEqual({}, allocations_server_a_migration) + allocations_server_b_migration = self.placement.get( + '/allocations/%s' % migration_server_b['uuid'] + ).body['allocations'] + src_uuid = self.api.api_get( + 'os-hypervisors?hypervisor_hostname_pattern=%s' % + self.src_hostname).body['hypervisors'][0]['id'] + self.assertIn(src_uuid, allocations_server_b_migration) + + # FIXME(astupnikov) Because of bug #1960412 INACTIVE port binding + # on destination host would not be removed when queued live migration + # is aborted, so 2 port bindings would exist for server_b port from + # Neutron's perspective. + # server_a should be migrated to dest compute, server_b should still + # be hosted by src compute. + port_binding_server_a = copy.deepcopy( + self.neutron._port_bindings[self.neutron.port_1['id']] + ) + self.assertEqual(1, len(port_binding_server_a)) + self.assertNotIn('src', port_binding_server_a) + port_binding_server_b = copy.deepcopy( + self.neutron._port_bindings[self.neutron.port_2['id']] + ) + self.assertEqual(2, len(port_binding_server_b)) -- cgit v1.2.1