From ee05cd8b9e73f8a9a43dbe9ce028e6a6beaec2bd Mon Sep 17 00:00:00 2001 From: Artom Lifshitz Date: Mon, 3 Feb 2020 14:36:35 -0500 Subject: [PATCH] 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. Change-Id: I36dd64dc272ea1743995b3b696323a9431666489 safdasdf Change-Id: I33d8ac0a1cae0b2d275a21287d5e44c008a68122 --- nova/test.py | 40 +++++++++++++++++-- .../functional/compute/test_cache_image.py | 6 +-- .../functional/db/test_virtual_interface.py | 6 +-- nova/tests/functional/integrated_helpers.py | 34 +--------------- .../regressions/test_bug_1764883.py | 7 +--- .../regressions/test_bug_1823370.py | 6 +-- .../regressions/test_bug_1830747.py | 6 +-- nova/tests/functional/test_aggregates.py | 16 +------- nova/tests/functional/test_nova_manage.py | 4 +- nova/tests/functional/test_scheduler.py | 10 ++--- nova/tests/functional/test_server_group.py | 4 +- nova/tests/functional/test_servers.py | 23 +---------- nova/tests/unit/conductor/test_conductor.py | 4 +- 13 files changed, 62 insertions(+), 104 deletions(-) diff --git a/nova/test.py b/nova/test.py index 11335d3fad9f..6f5a8eebe3c0 100644 --- a/nova/test.py +++ b/nova/test.py @@ -76,6 +76,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' @@ -233,6 +234,7 @@ class TestCase(base.BaseTestCase): 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 @@ -376,7 +378,7 @@ class TestCase(base.BaseTestCase): 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) @@ -393,7 +395,7 @@ class TestCase(base.BaseTestCase): # 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 @@ -421,6 +423,36 @@ class TestCase(base.BaseTestCase): 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 @@ -464,10 +496,10 @@ class TestCase(base.BaseTestCase): '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/compute/test_cache_image.py b/nova/tests/functional/compute/test_cache_image.py index ab70db2cf3ae..86a3e9e26e79 100644 --- a/nova/tests/functional/compute/test_cache_image.py +++ b/nova/tests/functional/compute/test_cache_image.py @@ -34,11 +34,11 @@ class ImageCacheTest(test.TestCase): self.compute1 = self.start_service('compute', host='compute1') self.compute2 = self.start_service('compute', host='compute2') self.compute3 = self.start_service('compute', host='compute3', - cell='cell2') + cell_name='cell2') self.compute4 = self.start_service('compute', host='compute4', - cell='cell2') + cell_name='cell2') self.compute5 = self.start_service('compute', host='compute5', - cell='cell2') + cell_name='cell2') cell2 = self.cell_mappings['cell2'] with context.target_cell(self.context, cell2) as cctxt: diff --git a/nova/tests/functional/db/test_virtual_interface.py b/nova/tests/functional/db/test_virtual_interface.py index 9ab2dd144577..a175231cd653 100644 --- a/nova/tests/functional/db/test_virtual_interface.py +++ b/nova/tests/functional/db/test_virtual_interface.py @@ -70,9 +70,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, @@ -93,7 +91,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 cb56e9e2690e..cef3306ab802 100644 --- a/nova/tests/functional/integrated_helpers.py +++ b/nova/tests/functional/integrated_helpers.py @@ -377,7 +377,7 @@ class _IntegratedTestBase(test.TestCase, InstanceHelperMixin): 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') @@ -504,21 +504,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 @@ -856,23 +841,6 @@ class ProviderUsageBaseTestCase(test.TestCase, InstanceHelperMixin): self.assertEqual(0, len(allocations)) return migration_uuid - 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 3b79f9872315..3d82bd267144 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 9e40298c5d03..67762800014b 100644 --- a/nova/tests/functional/regressions/test_bug_1823370.py +++ b/nova/tests/functional/regressions/test_bug_1823370.py @@ -49,9 +49,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 @@ -62,7 +60,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 269d2eae4ec0..849fe81c51a2 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/test_aggregates.py b/nova/tests/functional/test_aggregates.py index dbe396216365..0bf8fcfa09e9 100644 --- a/nova/tests/functional/test_aggregates.py +++ b/nova/tests/functional/test_aggregates.py @@ -178,17 +178,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 @@ -840,9 +829,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. @@ -864,7 +850,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 2634bde14eb5..4ffb5e90cd1f 100644 --- a/nova/tests/functional/test_nova_manage.py +++ b/nova/tests/functional/test_nova_manage.py @@ -1467,9 +1467,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_scheduler.py b/nova/tests/functional/test_scheduler.py index 1351aabee573..47667d250108 100644 --- a/nova/tests/functional/test_scheduler.py +++ b/nova/tests/functional/test_scheduler.py @@ -64,8 +64,8 @@ class MultiCellSchedulerTestCase(test.TestCase, in different cells and make sure that migration fails with NoValidHost. """ # Hosts in different cells - self.start_service('compute', host='compute1', cell=CELL1_NAME) - self.start_service('compute', host='compute2', cell=CELL2_NAME) + self.start_service('compute', host='compute1', cell_name=CELL1_NAME) + self.start_service('compute', host='compute2', cell_name=CELL2_NAME) _, server = self._test_create_and_migrate(expected_status=202) # The instance action should have failed with details. @@ -79,11 +79,11 @@ class MultiCellSchedulerTestCase(test.TestCase, migration is allowed. """ # Hosts in the same cell - self.start_service('compute', host='compute1', cell=CELL1_NAME) - self.start_service('compute', host='compute2', cell=CELL1_NAME) + self.start_service('compute', host='compute1', cell_name=CELL1_NAME) + self.start_service('compute', host='compute2', cell_name=CELL1_NAME) # Create another host just so it looks like we have hosts in # both cells - self.start_service('compute', host='compute3', cell=CELL2_NAME) + self.start_service('compute', host='compute3', cell_name=CELL2_NAME) # Force the server onto compute1 in cell1 so we do not accidentally # land on compute3 in cell2 and fail to migrate. diff --git a/nova/tests/functional/test_server_group.py b/nova/tests/functional/test_server_group.py index a01a945a083e..e6b593d293eb 100644 --- a/nova/tests/functional/test_server_group.py +++ b/nova/tests/functional/test_server_group.py @@ -867,9 +867,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 021f9f0f82d9..8a552f6dc484 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -74,7 +74,6 @@ class ServersTestBase(integrated_helpers._IntegratedTestBase): _min_count_parameter = 'min_count' def setUp(self): - self.computes = {} super(ServersTestBase, self).setUp() def _get_access_ips_params(self): @@ -102,23 +101,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. @@ -170,8 +152,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 = [] @@ -4580,7 +4561,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 39ff24c83472..be52db4c78bf 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -2051,8 +2051,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()