From 0668bc1a86427f848c024d7eab9fb2167a38ccc3 Mon Sep 17 00:00:00 2001 From: Artom Lifshitz Date: Mon, 3 Feb 2020 14:36:35 -0500 Subject: func tests: move _run_periodics() into base class There are two almost identical implementations of the _run_periodics() helper - and a third one would have joined them in a subsequent patch, if not for this patch. This patch moves the _run_periodics() to the base test class. In addition, _run_periodics() depends on the self.computes dict used for compute service tracking. The method that populates that dict, _start_compute(), is therefore also moved to the base class. This enables some light refactoring of existing tests that need either the _run_periodics() helper, or the compute service tracking. In addition, a needless override of _start_compute() in test_aggregates that provided no added value is removed. This is done to avoid any potential confusion around _start_compute()'s role. Conflicts: nova/tests/functional/compute/test_cache_image.py nova/tests/functional/integrated_helpers.py nova/tests/functional/test_scheduler.py NOTE(stephenfin): Conflicts are due to the two test files not existing yet and the large number of helpers missing from the 'integrated_helpers' class. Changes: nova/tests/functional/regressions/test_bug_1889108.py NOTE(stephenfin): Changes are due to a backport that was modified to remove use of the functions we're adding here. Change-Id: I33d8ac0a1cae0b2d275a21287d5e44c008a68122 (cherry picked from commit ee05cd8b9e73f8a9a43dbe9ce028e6a6beaec2bd) --- nova/test.py | 40 +++++++++++++++++++--- nova/tests/functional/db/test_virtual_interface.py | 6 ++-- nova/tests/functional/integrated_helpers.py | 34 +----------------- .../functional/regressions/test_bug_1764883.py | 7 ++-- .../functional/regressions/test_bug_1823370.py | 6 ++-- .../functional/regressions/test_bug_1830747.py | 6 ++-- .../functional/regressions/test_bug_1889108.py | 4 +-- nova/tests/functional/test_aggregates.py | 16 +-------- nova/tests/functional/test_nova_manage.py | 4 +-- nova/tests/functional/test_server_group.py | 4 +-- nova/tests/functional/test_servers.py | 23 ++----------- nova/tests/unit/conductor/test_conductor.py | 4 +-- 12 files changed, 56 insertions(+), 98 deletions(-) diff --git a/nova/test.py b/nova/test.py index a10f18043d..c44c9b6ae4 100644 --- a/nova/test.py +++ b/nova/test.py @@ -79,6 +79,7 @@ logging.register_options(CONF) CONF.set_override('use_stderr', False) logging.setup(CONF, 'nova') cache.configure(CONF) +LOG = logging.getLogger(__name__) _TRUE_VALUES = ('True', 'true', '1', 'yes') CELL1_NAME = 'cell1' @@ -273,6 +274,7 @@ class TestCase(testtools.TestCase): context.CELL_CACHE = {} context.CELLS = [] + self.computes = {} self.cell_mappings = {} self.host_mappings = {} # NOTE(danms): If the test claims to want to set up the database @@ -429,7 +431,7 @@ class TestCase(testtools.TestCase): for k, v in kw.items(): CONF.set_override(k, v, group) - def start_service(self, name, host=None, **kwargs): + def start_service(self, name, host=None, cell_name=None, **kwargs): # Disallow starting multiple scheduler services if name == 'scheduler' and self._service_fixture_count[name]: raise TestingException("Duplicate start_service(%s)!" % name) @@ -446,7 +448,7 @@ class TestCase(testtools.TestCase): # otherwise we'll fail to update the scheduler while running # the compute node startup routines below. ctxt = context.get_context() - cell_name = kwargs.pop('cell', CELL1_NAME) or CELL1_NAME + cell_name = cell_name or CELL1_NAME cell = self.cell_mappings[cell_name] if (host or name) not in self.host_mappings: # NOTE(gibi): If the HostMapping does not exists then this is @@ -474,6 +476,36 @@ class TestCase(testtools.TestCase): return svc.service + def _start_compute(self, host, cell_name=None): + """Start a nova compute service on the given host + + :param host: the name of the host that will be associated to the + compute service. + :param cell_name: optional name of the cell in which to start the + compute service + :return: the nova compute service object + """ + compute = self.start_service('compute', host=host, cell_name=cell_name) + self.computes[host] = compute + return compute + + def _run_periodics(self): + """Run the update_available_resource task on every compute manager + + This runs periodics on the computes in an undefined order; some child + class redefine this function to force a specific order. + """ + + ctx = context.get_admin_context() + for host, compute in self.computes.items(): + LOG.info('Running periodic for compute (%s)', host) + # Make sure the context is targeted to the proper cell database + # for multi-cell tests. + with context.target_cell( + ctx, self.host_mappings[host].cell_mapping) as cctxt: + compute.manager.update_available_resource(cctxt) + LOG.info('Finished with periodics') + def restart_compute_service(self, compute, keep_hypervisor_state=True): """Stops the service and starts a new one to have realistic restart @@ -517,10 +549,10 @@ class TestCase(testtools.TestCase): 'nova.virt.driver.load_compute_driver') as load_driver: load_driver.return_value = old_driver new_compute = self.start_service( - 'compute', host=compute.host, cell=cell_name) + 'compute', host=compute.host, cell_name=cell_name) else: new_compute = self.start_service( - 'compute', host=compute.host, cell=cell_name) + 'compute', host=compute.host, cell_name=cell_name) return new_compute diff --git a/nova/tests/functional/db/test_virtual_interface.py b/nova/tests/functional/db/test_virtual_interface.py index 6b9172e4a8..12a62f83ae 100644 --- a/nova/tests/functional/db/test_virtual_interface.py +++ b/nova/tests/functional/db/test_virtual_interface.py @@ -72,9 +72,7 @@ class VirtualInterfaceListMigrationTestCase( fake_network.set_stub_network_methods(self) self.cells = objects.CellMappingList.get_all(self.context) - compute_cell0 = self.start_service( - 'compute', host='compute2', cell='cell0') - self.computes = [compute_cell0, self.compute] + self._start_compute('compute2') self.instances = [] def _create_instances(self, pre_newton=2, deleted=0, total=5, @@ -95,7 +93,7 @@ class VirtualInterfaceListMigrationTestCase( flavor=flavor, created_at=datetime.datetime(1985, 10, 25, 1, 21, 0), launched_at=datetime.datetime(1985, 10, 25, 1, 22, 0), - host=self.computes[0].host, + host=self.computes['compute2'].host, hostname='%s-inst%i' % (target_cell.name, i)) inst.create() diff --git a/nova/tests/functional/integrated_helpers.py b/nova/tests/functional/integrated_helpers.py index ed74dd2de0..47980d8703 100644 --- a/nova/tests/functional/integrated_helpers.py +++ b/nova/tests/functional/integrated_helpers.py @@ -105,7 +105,7 @@ class _IntegratedTestBase(test.TestCase): self.addCleanup(nova.tests.unit.image.fake.FakeImageService_reset) def _setup_compute_service(self): - return self.start_service('compute') + return self._start_compute('compute') def _setup_scheduler_service(self): return self.start_service('scheduler') @@ -451,21 +451,6 @@ class ProviderUsageBaseTestCase(test.TestCase, InstanceHelperMixin): self.addCleanup(nova.tests.unit.image.fake.FakeImageService_reset) - self.computes = {} - - def _start_compute(self, host, cell_name=None): - """Start a nova compute service on the given host - - :param host: the name of the host that will be associated to the - compute service. - :param cell_name: optional name of the cell in which to start the - compute service (defaults to cell1) - :return: the nova compute service object - """ - compute = self.start_service('compute', host=host, cell=cell_name) - self.computes[host] = compute - return compute - def _get_provider_uuid_by_host(self, host): # NOTE(gibi): the compute node id is the same as the compute node # provider uuid on that compute @@ -785,23 +770,6 @@ class ProviderUsageBaseTestCase(test.TestCase, InstanceHelperMixin): allocations = self._get_allocations_by_server_uuid(migration_uuid) self.assertEqual(0, len(allocations)) - def _run_periodics(self): - """Run the update_available_resource task on every compute manager - - This runs periodics on the computes in an undefined order; some child - class redefined this function to force a specific order. - """ - - ctx = context.get_admin_context() - for host, compute in self.computes.items(): - LOG.info('Running periodic for compute (%s)', host) - # Make sure the context is targeted to the proper cell database - # for multi-cell tests. - with context.target_cell( - ctx, self.host_mappings[host].cell_mapping) as cctxt: - compute.manager.update_available_resource(cctxt) - LOG.info('Finished with periodics') - def _move_and_check_allocations(self, server, request, old_flavor, new_flavor, source_rp_uuid, dest_rp_uuid): self.api.post_server_action(server['id'], request) diff --git a/nova/tests/functional/regressions/test_bug_1764883.py b/nova/tests/functional/regressions/test_bug_1764883.py index 2c77a72e56..81b75b841b 100644 --- a/nova/tests/functional/regressions/test_bug_1764883.py +++ b/nova/tests/functional/regressions/test_bug_1764883.py @@ -60,11 +60,8 @@ class TestEvacuationWithSourceReturningDuringRebuild( self.start_service('scheduler') # Start two computes - self.computes = {} - - self.computes['host1'] = self.start_service('compute', host='host1') - - self.computes['host2'] = self.start_service('compute', host='host2') + self._start_compute('host1') + self._start_compute('host2') self.image_id = self.api.get_images()[0]['id'] self.flavor_id = self.api.get_flavors()[0]['id'] diff --git a/nova/tests/functional/regressions/test_bug_1823370.py b/nova/tests/functional/regressions/test_bug_1823370.py index 60a8e0053b..fe1f23b756 100644 --- a/nova/tests/functional/regressions/test_bug_1823370.py +++ b/nova/tests/functional/regressions/test_bug_1823370.py @@ -51,9 +51,7 @@ class MultiCellEvacuateTestCase(integrated_helpers._IntegratedTestBase, """ host_to_cell = {'host1': 'cell1', 'host2': 'cell2', 'host3': 'cell1'} for host, cell in host_to_cell.items(): - svc = self.start_service('compute', host=host, cell=cell) - # Set an attribute so we can access this service later. - setattr(self, host, svc) + self._start_compute(host, cell_name=cell) def test_evacuate_multi_cell(self): # Create a server which should land on host1 since it has the highest @@ -64,7 +62,7 @@ class MultiCellEvacuateTestCase(integrated_helpers._IntegratedTestBase, self.assertEqual('host1', server['OS-EXT-SRV-ATTR:host']) # Disable the host on which the server is now running. - self.host1.stop() + self.computes['host1'].stop() self.api.force_down_service('host1', 'nova-compute', forced_down=True) # Now evacuate the server which should send it to host3 since it is diff --git a/nova/tests/functional/regressions/test_bug_1830747.py b/nova/tests/functional/regressions/test_bug_1830747.py index 177048396e..0e7a2b6456 100644 --- a/nova/tests/functional/regressions/test_bug_1830747.py +++ b/nova/tests/functional/regressions/test_bug_1830747.py @@ -68,10 +68,8 @@ class MissingReqSpecInstanceGroupUUIDTestCase( self.start_service('scheduler') # Start two computes, one where the server will be created and another # where we'll cold migrate it. - self.computes = {} # keep track of the compute services per host name - for host in ('host1', 'host2'): - compute_service = self.start_service('compute', host=host) - self.computes[host] = compute_service + self._start_compute('host1') + self._start_compute('host2') def test_cold_migrate_reschedule(self): # Create an anti-affinity group for the server. diff --git a/nova/tests/functional/regressions/test_bug_1889108.py b/nova/tests/functional/regressions/test_bug_1889108.py index b69a79ae93..3e52ce5672 100644 --- a/nova/tests/functional/regressions/test_bug_1889108.py +++ b/nova/tests/functional/regressions/test_bug_1889108.py @@ -41,8 +41,8 @@ class TestVolAttachmentsDuringPreLiveMigration( self.addCleanup(fake_notifier.reset) def _setup_compute_service(self): - self.start_service('compute', host='src', cell=None) - self.start_service('compute', host='dest', cell=None) + self._start_compute('src') + self._start_compute('dest') @mock.patch('nova.virt.fake.FakeDriver.pre_live_migration', side_effect=test.TestingException) diff --git a/nova/tests/functional/test_aggregates.py b/nova/tests/functional/test_aggregates.py index 02115ffe90..8f8e9c8d2c 100644 --- a/nova/tests/functional/test_aggregates.py +++ b/nova/tests/functional/test_aggregates.py @@ -82,17 +82,6 @@ class AggregateRequestFiltersTest( # Aggregate with neither host self._create_aggregate('no-hosts') - def _start_compute(self, host): - """Start a nova compute service on the given host - - :param host: the name of the host that will be associated to the - compute service. - :return: the nova compute service object - """ - compute = self.start_service('compute', host=host) - self.computes[host] = compute - return compute - def _create_aggregate(self, name): agg = self.admin_api.post_aggregate({'aggregate': {'name': name}}) self.aggregates[name] = agg @@ -766,9 +755,6 @@ class TestAggregateFiltersTogether(AggregateRequestFiltersTest): class TestAggregateMultiTenancyIsolationFilter( test.TestCase, integrated_helpers.InstanceHelperMixin): - def _start_compute(self, host): - self.start_service('compute', host=host) - def setUp(self): super(TestAggregateMultiTenancyIsolationFilter, self).setUp() # Stub out glance, placement and neutron. @@ -787,7 +773,7 @@ class TestAggregateMultiTenancyIsolationFilter( self.flags(enabled_filters=enabled_filters, group='filter_scheduler') self.start_service('scheduler') for host in ('host1', 'host2'): - self._start_compute(host) + self.start_service('compute', host=host) def test_aggregate_multitenancy_isolation_filter(self): """Tests common scenarios with the AggregateMultiTenancyIsolation diff --git a/nova/tests/functional/test_nova_manage.py b/nova/tests/functional/test_nova_manage.py index d913c180ce..4307636a39 100644 --- a/nova/tests/functional/test_nova_manage.py +++ b/nova/tests/functional/test_nova_manage.py @@ -1455,9 +1455,9 @@ class TestDBArchiveDeletedRowsMultiCell(integrated_helpers.InstanceHelperMixin, # Start two compute services, one per cell self.compute1 = self.start_service('compute', host='host1', - cell='cell1') + cell_name='cell1') self.compute2 = self.start_service('compute', host='host2', - cell='cell2') + cell_name='cell2') def test_archive_deleted_rows(self): admin_context = context.get_admin_context(read_deleted='yes') diff --git a/nova/tests/functional/test_server_group.py b/nova/tests/functional/test_server_group.py index 60cf89d764..8ad73b865e 100644 --- a/nova/tests/functional/test_server_group.py +++ b/nova/tests/functional/test_server_group.py @@ -906,9 +906,9 @@ class ServerGroupTestMultiCell(ServerGroupTestBase): super(ServerGroupTestMultiCell, self).setUp() # Start two compute services, one per cell self.compute1 = self.start_service('compute', host='host1', - cell='cell1') + cell_name='cell1') self.compute2 = self.start_service('compute', host='host2', - cell='cell2') + cell_name='cell2') # This is needed to find a server that is still booting with multiple # cells, while waiting for the state change to ACTIVE. See the # _get_instance method in the compute/api for details. diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index 7cc393ee18..7ec7a2f4d5 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -72,7 +72,6 @@ class ServersTestBase(integrated_helpers._IntegratedTestBase): USE_NEUTRON = True def setUp(self): - self.computes = {} super(ServersTestBase, self).setUp() def _wait_for_state_change(self, server, from_status): @@ -134,23 +133,6 @@ class ServersTest(ServersTestBase): node.hypervisor_hostname: int(node.stats.get('failed_builds', 0)) for node in computes} - def _run_periodics(self): - """Run the update_available_resource task on every compute manager - - This runs periodics on the computes in an undefined order; some child - class redefined this function to force a specific order. - """ - - if self.compute.host not in self.computes: - self.computes[self.compute.host] = self.compute - - ctx = context.get_admin_context() - for compute in self.computes.values(): - LOG.info('Running periodic for compute (%s)', - compute.manager.host) - compute.manager.update_available_resource(ctx) - LOG.info('Finished with periodics') - def test_create_server_with_error(self): # Create a server which will enter error state. @@ -203,8 +185,7 @@ class ServersTest(ServersTestBase): def _test_create_server_with_error_with_retries(self): # Create a server which will enter error state. - self.compute2 = self.start_service('compute', host='host2') - self.computes['compute2'] = self.compute2 + self._start_compute('host2') fails = [] @@ -4481,7 +4462,7 @@ class ServerTestV256MultiCellTestCase(ServerTestV256Common): 'host2': 'cell2'} for host in sorted(host_to_cell_mappings): self.start_service('compute', host=host, - cell=host_to_cell_mappings[host]) + cell_name=host_to_cell_mappings[host]) def test_migrate_server_to_host_in_different_cell(self): # We target host1 specifically so that we have a predictable target for diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 525d673f11..8e04c9df37 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -1922,8 +1922,8 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): params = self.params # The cells are created in the base TestCase setup. - self.start_service('compute', host='host1', cell='cell1') - self.start_service('compute', host='host2', cell='cell2') + self.start_service('compute', host='host1', cell_name='cell1') + self.start_service('compute', host='host2', cell_name='cell2') get_hostmapping.side_effect = self.host_mappings.values() -- cgit v1.2.1