diff --git a/doc/source/cli/nova-manage.rst b/doc/source/cli/nova-manage.rst index 8f86b37db19c..68b9eb51139a 100644 --- a/doc/source/cli/nova-manage.rst +++ b/doc/source/cli/nova-manage.rst @@ -279,7 +279,6 @@ Nova Database | populate_missing_availability_zones | 0 | 0 | | populate_queued_for_delete | 2 | 2 | | populate_uuids | 0 | 0 | - | service_uuids_online_data_migration | 0 | 0 | +---------------------------------------------+--------------+-----------+ In the above example, the ``migrate_instances_add_request_spec`` migration diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index f23b52cff584..fc0947e8b4ea 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -386,9 +386,6 @@ class DbCommands(object): # complete have finished. # NOTE(stephenfin): These names must be unique online_migrations = ( - # Added in Pike - # TODO(mriedem): Remove this in the U release. - db.service_uuids_online_data_migration, # Added in Pike quotas_obj.migrate_quota_limits_to_api_db, # Added in Pike diff --git a/nova/db/api.py b/nova/db/api.py index c65803bed439..cc273a0ba6a0 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -1803,11 +1803,6 @@ def pcidevice_online_data_migration(context, max_count): return IMPL.pcidevice_online_data_migration(context, max_count) -# TODO(mriedem): Remove this in the U release. -def service_uuids_online_data_migration(context, max_count): - return IMPL.service_uuids_online_data_migration(context, max_count) - - #################### diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 9f43eebd0673..efda6754b092 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -5692,25 +5692,6 @@ def purge_shadow_tables(context, before_date, status_fn=None): return total_deleted -# TODO(mriedem): Remove this in the U release. -@pick_context_manager_writer -def service_uuids_online_data_migration(context, max_count): - from nova.objects import service - - count_all = 0 - count_hit = 0 - - db_services = model_query(context, models.Service).filter_by( - uuid=None).limit(max_count) - for db_service in db_services: - count_all += 1 - service_obj = service.Service._from_db_object( - context, service.Service(), db_service) - if 'uuid' in service_obj: - count_hit += 1 - return count_all, count_hit - - #################### diff --git a/nova/objects/service.py b/nova/objects/service.py index 61cdc6e88fda..9ea0fc2a2c2e 100644 --- a/nova/objects/service.py +++ b/nova/objects/service.py @@ -294,13 +294,6 @@ class Service(base.NovaPersistentObject, base.NovaObject, service._context = context service.obj_reset_changes() - # TODO(dpeschman): Drop this once all services have uuids in database - if 'uuid' not in service and not service.deleted: - service.uuid = uuidutils.generate_uuid() - LOG.debug('Generated UUID %(uuid)s for service %(id)i', - dict(uuid=service.uuid, id=service.id)) - service.save() - return service def obj_load_attr(self, attrname): diff --git a/nova/tests/functional/regressions/test_bug_1746509.py b/nova/tests/functional/regressions/test_bug_1746509.py deleted file mode 100644 index 176b64e5ae1b..000000000000 --- a/nova/tests/functional/regressions/test_bug_1746509.py +++ /dev/null @@ -1,62 +0,0 @@ -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -import nova.context -from nova.db import api as db -from nova import objects -from nova import test - - -class InstanceListWithServicesTestCase(test.TestCase): - """Test the scenario listing instances whose services may not have a UUID. - - We added a UUID field to the 'services' table in Pike, so after an upgrade, - we generate a UUID and save it to the service record upon access if it's an - older record that does not already have a UUID. - - Now, when we list instances through the API, the instances query is joined - with the 'services' table in order to provide service-related information. - The bug in Pike was that we were already inside of a 'reader' database - transaction context when we tried to compile the instance list, which - eventually led to an error: - - TypeError: Can't upgrade a READER transaction to a WRITER mid-transaction - - because we also tried to write the newly generated service UUID to the - service record while still nested under the 'reader' context. - - This test verifies that we are able to list instances joined with services - even if the associated service record does not yet have a UUID. - """ - def setUp(self): - super(InstanceListWithServicesTestCase, self).setUp() - self.context = nova.context.RequestContext('fake-user', 'fake-project') - - def test_instance_list_service_with_no_uuid(self): - # Create a nova-compute service record with a host that will match the - # instance's host, with no uuid. We can't do this through the - # Service object because it will automatically generate a uuid. - service = db.service_create(self.context, {'host': 'fake-host', - 'binary': 'nova-compute'}) - self.assertIsNone(service['uuid']) - - # Create an instance whose host will match the service with no uuid - inst = objects.Instance(context=self.context, - project_id=self.context.project_id, - host='fake-host') - inst.create() - - insts = objects.InstanceList.get_by_filters( - self.context, {}, expected_attrs=['services']) - self.assertEqual(1, len(insts)) - self.assertEqual(1, len(insts[0].services)) - self.assertIsNotNone(insts[0].services[0].uuid) diff --git a/nova/tests/unit/api/openstack/compute/test_services.py b/nova/tests/unit/api/openstack/compute/test_services.py index d21d17f58466..8bcad222bd44 100644 --- a/nova/tests/unit/api/openstack/compute/test_services.py +++ b/nova/tests/unit/api/openstack/compute/test_services.py @@ -684,18 +684,22 @@ class ServicesTestV21(test.TestCase): self.controller.update, self.req, "disable-log-reason", body=body) - def test_services_delete(self): - - compute = self.host_api.db.service_create(self.ctxt, - {'host': 'fake-compute-host', - 'binary': 'nova-compute', - 'topic': 'compute', - 'report_count': 0}) + @mock.patch('nova.objects.ComputeNodeList.get_all_by_host', + return_value=objects.ComputeNodeList(objects=[])) + def test_services_delete(self, mock_get_compute_nodes): + compute = objects.Service(self.ctxt, + **{'host': 'fake-compute-host', + 'binary': 'nova-compute', + 'topic': 'compute', + 'report_count': 0}) + compute.create() with mock.patch('nova.objects.Service.destroy') as service_delete: self.controller.delete(self.req, compute.id) service_delete.assert_called_once_with() self.assertEqual(self.controller.delete.wsgi_code, 204) + mock_get_compute_nodes.assert_called_once_with( + self.req.environ['nova.context'], compute.host) def test_services_delete_not_found(self): diff --git a/nova/tests/unit/compute/test_host_api.py b/nova/tests/unit/compute/test_host_api.py index 43d4aad3b4e2..4f6eca542cc5 100644 --- a/nova/tests/unit/compute/test_host_api.py +++ b/nova/tests/unit/compute/test_host_api.py @@ -471,11 +471,12 @@ class ComputeHostAPITestCase(test.TestCase): @mock.patch.object(objects.HostMapping, 'get_by_host') def test_service_delete_compute_in_aggregate( self, mock_hm, mock_get_cn, mock_add_host, mock_remove_host): - compute = self.host_api.db.service_create(self.ctxt, - {'host': 'fake-compute-host', - 'binary': 'nova-compute', - 'topic': 'compute', - 'report_count': 0}) + compute = objects.Service(self.ctxt, + **{'host': 'fake-compute-host', + 'binary': 'nova-compute', + 'topic': 'compute', + 'report_count': 0}) + compute.create() # This is needed because of lazy-loading service.compute_node cn = objects.ComputeNode(uuid=uuids.cn, host="fake-compute-host", hypervisor_hostname="fake-compute-host") diff --git a/nova/tests/unit/db/test_db_api.py b/nova/tests/unit/db/test_db_api.py index 78ef7e923b4d..586a186e7905 100644 --- a/nova/tests/unit/db/test_db_api.py +++ b/nova/tests/unit/db/test_db_api.py @@ -3717,43 +3717,6 @@ class ServiceTestCase(test.TestCase, ModelsObjectComparatorMixin): self.assertRaises(exception.ServiceTopicExists, db.service_create, self.ctxt, values) - def test_migrate_service_uuids(self): - # Start with nothing. - total, done = db.service_uuids_online_data_migration(self.ctxt, 10) - self.assertEqual(0, total) - self.assertEqual(0, done) - - # Create two services, one with a uuid and one without. - db.service_create(self.ctxt, - dict(host='host1', binary='nova-compute', - topic='compute', report_count=1, - disabled=False)) - db.service_create(self.ctxt, - dict(host='host2', binary='nova-compute', - topic='compute', report_count=1, - disabled=False, uuid=uuidsentinel.host2)) - - # Now migrate them, we should find one and update one. - total, done = db.service_uuids_online_data_migration( - self.ctxt, 10) - self.assertEqual(1, total) - self.assertEqual(1, done) - - # Get the services back to make sure the original uuid didn't change. - services = db.service_get_all_by_binary(self.ctxt, 'nova-compute') - self.assertEqual(2, len(services)) - for service in services: - if service['host'] == 'host2': - self.assertEqual(uuidsentinel.host2, service['uuid']) - else: - self.assertIsNotNone(service['uuid']) - - # Run the online migration again to see nothing was processed. - total, done = db.service_uuids_online_data_migration( - self.ctxt, 10) - self.assertEqual(0, total) - self.assertEqual(0, done) - def test_migration_migrate_to_uuid(self): total, done = sqlalchemy_api.migration_migrate_to_uuid(self.ctxt, 10) self.assertEqual(0, total) diff --git a/nova/tests/unit/objects/test_service.py b/nova/tests/unit/objects/test_service.py index 6cef4afefce1..de4d96d53f7a 100644 --- a/nova/tests/unit/objects/test_service.py +++ b/nova/tests/unit/objects/test_service.py @@ -426,20 +426,6 @@ class _TestServiceObject(object): s.obj_make_compatible_from_manifest(primitive, '1.20', mock.Mock()) self.assertNotIn('uuid', primitive) - @mock.patch('nova.objects.service.uuidutils.generate_uuid', - return_value=uuidsentinel.service4) - def test_from_db_object_without_uuid_generates_one(self, generate_uuid): - values = _fake_service(uuid=None, id=None) - db_service = db.service_create(self.context, values) - - s = service.Service() - service.Service._from_db_object(self.context, s, db_service) - self.assertEqual(uuidsentinel.service4, s.uuid) - - # Check the DB too - db_service2 = db.service_get(self.context, s.id) - self.assertEqual(s.uuid, db_service2['uuid']) - class TestServiceObject(test_objects._LocalTest, _TestServiceObject): diff --git a/nova/tests/unit/virt/xenapi/test_xenapi.py b/nova/tests/unit/virt/xenapi/test_xenapi.py index 3dc2a0a3c0a8..0985fe332077 100644 --- a/nova/tests/unit/virt/xenapi/test_xenapi.py +++ b/nova/tests/unit/virt/xenapi/test_xenapi.py @@ -3152,11 +3152,11 @@ def _create_service_entries(context, values={'avail_zone1': ['fake_host1', 'avail_zone2': ['fake_host3'], }): for hosts in values.values(): for service_host in hosts: - db.service_create(context, - {'host': service_host, + objects.Service(context, + **{'host': service_host, 'binary': 'nova-compute', 'topic': 'compute', - 'report_count': 0}) + 'report_count': 0}).create() return values