From 504594afb807e5bdf407fd15911f98f97c1051d8 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 30 Dec 2016 12:56:20 -0500 Subject: [PATCH] Handle unicode when dealing with duplicate aggregate errors during migration The AggregateNameExists message can be translated and have unicode in it so we can't cast it to a str in python 2.7 else we'll get a UnicodeEncodeError. That would break the online data migration for aggregates along with any other online data migrations that run after that one which impacts upgrades. This uses six.text_type rather than str for logging the error and adds a test to recreate the bug and show that it's fixed. Change-Id: I040db22ecbb9fabe5bbda8a3d9600cc9f76cb170 Closes-Bug: #1653261 --- nova/objects/aggregate.py | 3 ++- nova/tests/functional/db/test_aggregate.py | 23 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/nova/objects/aggregate.py b/nova/objects/aggregate.py index a16455afc..654c3092d 100644 --- a/nova/objects/aggregate.py +++ b/nova/objects/aggregate.py @@ -16,6 +16,7 @@ from oslo_db import exception as db_exc from oslo_log import log as logging from oslo_utils import excutils from oslo_utils import uuidutils +import six from sqlalchemy.orm import contains_eager from sqlalchemy.orm import joinedload from sqlalchemy.sql import func @@ -593,7 +594,7 @@ def migrate_aggregates(ctxt, count): _LW('Aggregate id %(id)i disappeared during migration'), {'id': aggregate_id}) except (exception.AggregateNameExists) as e: - LOG.error(str(e)) + LOG.error(six.text_type(e)) return count_all, count_hit diff --git a/nova/tests/functional/db/test_aggregate.py b/nova/tests/functional/db/test_aggregate.py index e995df343..ab05e5367 100644 --- a/nova/tests/functional/db/test_aggregate.py +++ b/nova/tests/functional/db/test_aggregate.py @@ -635,3 +635,26 @@ class AggregateMigrationTestCase(test.TestCase): self.context, 0) self.assertEqual(0, match) self.assertEqual(0, done) + + @mock.patch('nova.objects.aggregate.LOG.error') + def test_migrate_aggregates_duplicate_unicode(self, mock_log_error): + """Tests that we handle a duplicate aggregate when migrating and that + we handle when the exception message is in unicode. + """ + # First create an aggregate that will be migrated from main to API DB. + create_aggregate(self.context, 1, in_api=False) + # Now create that same aggregate in the API DB. + create_aggregate(self.context, 1, in_api=True) + # Now let's run the online data migration which will fail to create + # a duplicate aggregate in the API database and will raise + # AggregateNameExists which we want to modify to have a unicode + # message. + with mock.patch.object(exception.AggregateNameExists, 'msg_fmt', + u'\xF0\x9F\x92\xA9'): + match, done = aggregate_obj.migrate_aggregates(self.context, 50) + # we found one + self.assertEqual(1, match) + # but we didn't migrate it + self.assertEqual(0, done) + # and we logged an error for the duplicate aggregate + mock_log_error.assert_called()