From 16e163053ca39886f11fdb8a3af10a28619fc105 Mon Sep 17 00:00:00 2001 From: melanie witt Date: Thu, 12 Jul 2018 21:48:23 +0000 Subject: [PATCH] Don't generate service UUID for deleted services In Pike, we added a UUID field to services and during an upgrade from Ocata => Pike, when instances are accessed joined with their associated services, we generate a UUID for the services on-the-fly. This causes a problem in the scenario where an operator upgrades their cluster and has old, deleted services with hostnames matching existing services associated with instances. When we go to generate the service UUID for the old, deleted service, we hit a ServiceTooOld exception. This addresses the problem by not bothering to generate a UUID for a deleted service. One alternative would be to exclude deleted services when we join the 'instances' and 'services' tables, but I'm not sure whether that approach might cause unintended effects where service information that used to be viewable for instances becomes hidden. Closes-Bug: #1778305 Closes-Bug: #1764556 Change-Id: I347096a527c257075cefe7b81210622f6cd87daf --- nova/objects/service.py | 2 +- .../regressions/test_bug_1764556.py | 13 +++------- .../regressions/test_bug_1778305.py | 24 +++++++------------ 3 files changed, 13 insertions(+), 26 deletions(-) diff --git a/nova/objects/service.py b/nova/objects/service.py index b8b4d2f7ff7e..98d49669f809 100644 --- a/nova/objects/service.py +++ b/nova/objects/service.py @@ -279,7 +279,7 @@ class Service(base.NovaPersistentObject, base.NovaObject, service.obj_reset_changes() # TODO(dpeschman): Drop this once all services have uuids in database - if 'uuid' not in service: + 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)) diff --git a/nova/tests/functional/regressions/test_bug_1764556.py b/nova/tests/functional/regressions/test_bug_1764556.py index 18c1ca131a17..2a9cce0c50c2 100644 --- a/nova/tests/functional/regressions/test_bug_1764556.py +++ b/nova/tests/functional/regressions/test_bug_1764556.py @@ -10,13 +10,10 @@ # License for the specific language governing permissions and limitations # under the License. -import six - from nova import context as nova_context from nova.db import api as db from nova import test from nova.tests import fixtures as nova_fixtures -from nova.tests.functional.api import client as api_client from nova.tests.functional import fixtures as func_fixtures from nova.tests.functional import integrated_helpers from nova.tests.unit.image import fake as fake_image @@ -149,10 +146,6 @@ class InstanceListWithDeletedServicesTestCase( # Finally, list servers as an admin so it joins on services to get host # information. - # FIXME(mriedem): This is bug 1764556 where the join on the services - # table also pulls the deleted service that doesn't have a uuid and - # attempts to migrate that service to have a uuid, which fails because - # it's not using a read_deleted='yes' context. - ex = self.assertRaises(api_client.OpenStackApiException, - self.admin_api.get_servers, detail=True) - self.assertIn('ServiceNotFound', six.text_type(ex)) + servers = self.admin_api.get_servers(detail=True) + for server in servers: + self.assertEqual('UP', server['host_status']) diff --git a/nova/tests/functional/regressions/test_bug_1778305.py b/nova/tests/functional/regressions/test_bug_1778305.py index 07410e9153da..dc9e945f30f5 100644 --- a/nova/tests/functional/regressions/test_bug_1778305.py +++ b/nova/tests/functional/regressions/test_bug_1778305.py @@ -49,19 +49,13 @@ class InstanceListWithOldDeletedServiceTestCase(test.TestCase): host='fake-host') inst.create() - # TODO(melwitt): Remove this assert when the bug is fixed. - self.assertRaises(nova.exception.ServiceTooOld, - objects.InstanceList.get_by_filters, - self.context, {}, expected_attrs=['services']) - - # TODO(melwitt): Uncomment these asserts when the bug is fixed. - # insts = objects.InstanceList.get_by_filters( - # self.context, {}, expected_attrs=['services']) - # self.assertEqual(1, len(insts)) - # self.assertEqual(2, len(insts[0].services)) + insts = objects.InstanceList.get_by_filters( + self.context, {}, expected_attrs=['services']) + self.assertEqual(1, len(insts)) + self.assertEqual(2, len(insts[0].services)) # Deleted service should not have a UUID - # for service in insts[0].services: - # if service.deleted: - # self.assertNotIn('uuid', service) - # else: - # self.assertIsNotNone(service.uuid) + for service in insts[0].services: + if service.deleted: + self.assertNotIn('uuid', service) + else: + self.assertIsNotNone(service.uuid)