diff options
3 files changed, 122 insertions, 4 deletions
diff --git a/nova/db/sqlalchemy/migrate_repo/versions/153_instance_type_in_system_metadata.py b/nova/db/sqlalchemy/migrate_repo/versions/153_instance_type_in_system_metadata.py index 20e75a6eb..36545b435 100644 --- a/nova/db/sqlalchemy/migrate_repo/versions/153_instance_type_in_system_metadata.py +++ b/nova/db/sqlalchemy/migrate_repo/versions/153_instance_type_in_system_metadata.py @@ -38,8 +38,9 @@ def upgrade(migrate_engine): i = sys_meta.insert() for values in q.execute(): for index in range(0, len(instance_type_props)): + value = values[index + 1] i.execute({"key": "instance_type_%s" % instance_type_props[index], - "value": str(values[index + 1]), + "value": None if value is None else str(value), "instance_uuid": values[0]}) diff --git a/nova/db/sqlalchemy/migrate_repo/versions/161_fix_system_metadata_none_strings.py b/nova/db/sqlalchemy/migrate_repo/versions/161_fix_system_metadata_none_strings.py new file mode 100644 index 000000000..bd8f22a97 --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/161_fix_system_metadata_none_strings.py @@ -0,0 +1,43 @@ +# Copyright 2013 IBM Corp. +# +# 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. + +from sqlalchemy import MetaData, Table +from nova.openstack.common import timeutils + + +def upgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + sys_meta = Table('instance_system_metadata', meta, autoload=True) + + sys_meta.update().\ + values(value=None).\ + where(sys_meta.c.key != 'instance_type_name').\ + where(sys_meta.c.key != 'instance_type_flavorid').\ + where(sys_meta.c.key.like('instance_type_%')).\ + where(sys_meta.c.value == 'None').\ + execute() + + now = timeutils.utcnow() + sys_meta.update().\ + values(created_at=now).\ + where(sys_meta.c.created_at == None).\ + where(sys_meta.c.key.like('instance_type_%')).\ + execute() + + +def downgrade(migration_engine): + # This migration only touches data, and only metadata at that. No need + # to go through and delete old metadata items. + pass diff --git a/nova/tests/test_migrations.py b/nova/tests/test_migrations.py index cf5c2f509..60975c68c 100644 --- a/nova/tests/test_migrations.py +++ b/nova/tests/test_migrations.py @@ -785,7 +785,7 @@ class TestNovaMigrations(BaseMigrationTestCase, CommonTestsMixIn): is_public=False), dict(id=13, name='type4', memory_mb=128, vcpus=1, root_gb=10, ephemeral_gb=0, flavorid="4", swap=0, - rxtx_factor=1.0, vcpu_weight=1, disabled=True, + rxtx_factor=1.0, vcpu_weight=None, disabled=True, is_public=True), dict(id=14, name='type5', memory_mb=128, vcpus=1, root_gb=10, ephemeral_gb=0, flavorid="5", swap=0, @@ -831,8 +831,14 @@ class TestNovaMigrations(BaseMigrationTestCase, CommonTestsMixIn): for prop in instance_type_props: prop_name = 'instance_type_%s' % prop self.assertIn(prop_name, inst_sys_meta) - self.assertEqual(str(inst_sys_meta[prop_name]), - str(inst_type[prop])) + if prop == "vcpu_weight": + # NOTE(danms) vcpu_weight can be NULL + self.assertEqual(inst_sys_meta[prop_name], + inst_type[prop] and str(inst_type[prop]) + or None) + else: + self.assertEqual(str(inst_sys_meta[prop_name]), + str(inst_type[prop])) # migration 154, add shadow tables for deleted data # There are 53 shadow tables but we only test one @@ -1032,6 +1038,74 @@ class TestNovaMigrations(BaseMigrationTestCase, CommonTestsMixIn): for key, value in data[the_id].items(): self.assertEqual(value, result[key]) + # migration 161, fix system_metadata "None" values should be NULL + def _pre_upgrade_161(self, engine): + fake_instances = [dict(uuid='m161-uuid1')] + sm_base = dict(instance_uuid='m161-uuid1', value=None) + now = timeutils.utcnow().replace(microsecond=0) + fake_sys_meta = [ + # Should be fixed + dict(sm_base, key='instance_type_foo', value='None'), + dict(sm_base, key='instance_type_bar', value='88 mph'), + + # Should be unaffected + dict(sm_base, key='instance_type_name', value='None'), + dict(sm_base, key='instance_type_flavorid', value='None'), + dict(sm_base, key='foo', value='None'), + dict(sm_base, key='instance_type_bat'), + dict(sm_base, key='instance_type_baz', created_at=now), + ] + + instances = get_table(engine, 'instances') + sys_meta = get_table(engine, 'instance_system_metadata') + engine.execute(instances.insert(), fake_instances) + + data = {} + for sm in fake_sys_meta: + result = sys_meta.insert().values(sm).execute() + sm['id'] = result.inserted_primary_key[0] + data[sm['id']] = sm + + return data + + def _check_161(self, engine, data): + our_ids = data.keys() + sys_meta = get_table(engine, 'instance_system_metadata') + results = sys_meta.select().where(sys_meta.c.id.in_(our_ids)).\ + execute() + results = list(results) + self.assertEqual(len(our_ids), len(results)) + for result in results: + the_id = result['id'] + key = result['key'] + value = result['value'] + original = data[the_id] + + if key == 'instance_type_baz': + # Neither value nor created_at should have been altered + self.assertEqual(result['value'], original['value']) + self.assertEqual(result['created_at'], original['created_at']) + elif key in ['instance_type_name', 'instance_type_flavorid']: + # These should not have their values changed, but should + # have corrected created_at stamps + self.assertEqual(result['value'], original['value']) + self.assertTrue(isinstance(result['created_at'], + datetime.datetime)) + elif key.startswith('instance_type'): + # Values like instance_type_% should be stamped and values + # converted from 'None' to None where appropriate + self.assertEqual(result['value'], + None if original['value'] == 'None' + else original['value']) + self.assertTrue(isinstance(result['created_at'], + datetime.datetime)) + else: + # None of the non-instance_type values should have + # been touched. Since we didn't set created_at on any + # of them, they should all still be None. + self.assertEqual(result['value'], original['value']) + self.assertEqual(result['created_at'], None) + class TestBaremetalMigrations(BaseMigrationTestCase, CommonTestsMixIn): """Test sqlalchemy-migrate migrations.""" |