diff --git a/nova/db/sqlalchemy/api_migrations/migrate_repo/versions/050_flavors_add_description.py b/nova/db/sqlalchemy/api_migrations/migrate_repo/versions/050_flavors_add_description.py new file mode 100644 index 0000000000..0529140200 --- /dev/null +++ b/nova/db/sqlalchemy/api_migrations/migrate_repo/versions/050_flavors_add_description.py @@ -0,0 +1,26 @@ +# 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 Column +from sqlalchemy import MetaData +from sqlalchemy import Table +from sqlalchemy import Text + + +def upgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + + flavors = Table('flavors', meta, autoload=True) + + if not hasattr(flavors.c, 'description'): + flavors.create_column(Column('description', Text())) diff --git a/nova/db/sqlalchemy/api_models.py b/nova/db/sqlalchemy/api_models.py index 2d42036928..3982606996 100644 --- a/nova/db/sqlalchemy/api_models.py +++ b/nova/db/sqlalchemy/api_models.py @@ -190,6 +190,7 @@ class Flavors(API_BASE): vcpu_weight = Column(Integer) disabled = Column(Boolean, default=False) is_public = Column(Boolean, default=True) + description = Column(Text) class FlavorExtraSpecs(API_BASE): diff --git a/nova/objects/flavor.py b/nova/objects/flavor.py index f9bb32e252..8c4d8e2f7e 100644 --- a/nova/objects/flavor.py +++ b/nova/objects/flavor.py @@ -15,6 +15,7 @@ from oslo_db import exception as db_exc from oslo_db.sqlalchemy import utils as sqlalchemyutils from oslo_log import log as logging +from oslo_utils import versionutils from sqlalchemy import or_ from sqlalchemy.orm import joinedload from sqlalchemy.sql.expression import asc @@ -37,6 +38,9 @@ OPTIONAL_FIELDS = ['extra_specs', 'projects'] # Remove these fields in version 2.0 of the object. DEPRECATED_FIELDS = ['deleted', 'deleted_at'] +# Non-joined fields which can be updated. +MUTABLE_FIELDS = set(['description']) + CONF = nova.conf.CONF @@ -197,7 +201,9 @@ class Flavor(base.NovaPersistentObject, base.NovaObject, # Version 1.0: Initial version # Version 1.1: Added save_projects(), save_extra_specs(), removed # remotable from save() - VERSION = '1.1' + # Version 1.2: Added description field. Note: this field should not be + # persisted with the embedded instance.flavor. + VERSION = '1.2' fields = { 'id': fields.IntegerField(), @@ -214,6 +220,7 @@ class Flavor(base.NovaPersistentObject, base.NovaObject, 'is_public': fields.BooleanField(), 'extra_specs': fields.DictOfStringsField(), 'projects': fields.ListOfStringsField(), + 'description': fields.StringField(nullable=True) } def __init__(self, *args, **kwargs): @@ -221,6 +228,12 @@ class Flavor(base.NovaPersistentObject, base.NovaObject, self._orig_extra_specs = {} self._orig_projects = [] + def obj_make_compatible(self, primitive, target_version): + super(Flavor, self).obj_make_compatible(primitive, target_version) + target_version = versionutils.convert_version_to_tuple(target_version) + if target_version < (1, 2) and 'description' in primitive: + del primitive['description'] + @staticmethod def _from_db_object(context, flavor, db_flavor, expected_attrs=None): if expected_attrs is None: @@ -473,13 +486,30 @@ class Flavor(base.NovaPersistentObject, base.NovaObject, self._flavor_extra_specs_del(self._context, self.id, key) self.obj_reset_changes(['extra_specs']) + # NOTE(mriedem): This method is not remotable since we only expect the API + # to be able to make updates to a flavor. + @db_api.api_context_manager.writer + def _save(self, context, values): + db_flavor = context.session.query(api_models.Flavors).\ + filter_by(id=self.id).first() + if not db_flavor: + raise exception.FlavorNotFound(flavor_id=self.id) + db_flavor.update(values) + db_flavor.save(context.session) + # Refresh ourselves from the DB object so we get the new updated_at. + self._from_db_object(context, self, db_flavor) + self.obj_reset_changes() + def save(self): updates = self.obj_get_changes() projects = updates.pop('projects', None) extra_specs = updates.pop('extra_specs', None) if updates: - raise exception.ObjectActionError( - action='save', reason='read-only fields were changed') + # Only allowed to update from the whitelist of mutable fields. + if set(updates.keys()) - MUTABLE_FIELDS: + raise exception.ObjectActionError( + action='save', reason='read-only fields were changed') + self._save(self._context, updates) if extra_specs is not None: deleted_keys = (set(self._orig_extra_specs.keys()) - @@ -505,7 +535,8 @@ class Flavor(base.NovaPersistentObject, base.NovaObject, if added_projects or deleted_projects: self.save_projects(added_projects, deleted_projects) - if added_keys or deleted_keys or added_projects or deleted_projects: + if (added_keys or deleted_keys or added_projects or deleted_projects or + updates): self._send_notification(fields.NotificationAction.UPDATE) @staticmethod diff --git a/nova/objects/instance.py b/nova/objects/instance.py index 3c166addd2..aca128c14a 100644 --- a/nova/objects/instance.py +++ b/nova/objects/instance.py @@ -539,6 +539,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject, 'old': old, 'new': new, } + self._nullify_flavor_description(flavor_info) updates['extra']['flavor'] = jsonutils.dumps(flavor_info) keypairs = updates.pop('keypairs', None) if keypairs is not None: @@ -625,6 +626,22 @@ class Instance(base.NovaPersistentObject, base.NovaObject, # NOTE(gibi): tags are not saved through the instance pass + @staticmethod + def _nullify_flavor_description(flavor_info): + """Helper method to nullify descriptions from a set of primitive + flavors. + + Note that we don't remove the flavor description since that would + make the versioned notification FlavorPayload have to handle the field + not being set on the embedded instance.flavor. + + :param dict: dict of primitive flavor objects where the values are the + flavors which get persisted in the instance_extra.flavor table. + """ + for flavor in flavor_info.values(): + if flavor and 'description' in flavor['nova_object.data']: + flavor['nova_object.data']['description'] = None + def _save_flavor(self, context): if not any([x in self.obj_what_changed() for x in ('flavor', 'old_flavor', 'new_flavor')]): @@ -636,6 +653,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject, 'new': (self.new_flavor and self.new_flavor.obj_to_primitive() or None), } + self._nullify_flavor_description(flavor_info) self._extra_values_to_save['flavor'] = jsonutils.dumps(flavor_info) self.obj_reset_changes(['flavor', 'old_flavor', 'new_flavor']) diff --git a/nova/tests/functional/db/api/test_migrations.py b/nova/tests/functional/db/api/test_migrations.py index 171edbe530..4bb1f52b23 100644 --- a/nova/tests/functional/db/api/test_migrations.py +++ b/nova/tests/functional/db/api/test_migrations.py @@ -643,6 +643,9 @@ class NovaAPIMigrationsWalk(test_migrations.WalkVersionsMixin): 'consumers_project_id_user_id_uuid_idx', ) + def _check_050(self, engine, data): + self.assertColumnExists(engine, 'flavors', 'description') + class TestNovaAPIMigrationsWalkSQLite(NovaAPIMigrationsWalk, test_base.DbTestCase, diff --git a/nova/tests/functional/db/test_flavor.py b/nova/tests/functional/db/test_flavor.py index 7709bb9142..71c1a2233e 100644 --- a/nova/tests/functional/db/test_flavor.py +++ b/nova/tests/functional/db/test_flavor.py @@ -35,6 +35,7 @@ fake_api_flavor = { 'is_public': True, 'extra_specs': {'foo': 'bar'}, 'projects': ['project1', 'project2'], + 'description': None } diff --git a/nova/tests/functional/db/test_flavor_model.py b/nova/tests/functional/db/test_flavor_model.py index 5cf484c054..447c2268e6 100644 --- a/nova/tests/functional/db/test_flavor_model.py +++ b/nova/tests/functional/db/test_flavor_model.py @@ -32,6 +32,9 @@ class FlavorTablesCompareTestCase(test.NoDBTestCase): flavors = api_models.Flavors() instance_types = models.InstanceTypes() columns_flavors = self._get_columns_list(flavors) + # The description column is only in the API database so we have to + # exclude it from this check. + columns_flavors.remove('description') columns_instance_types = self._get_columns_list(instance_types) self.assertTrue(self._check_column_list(columns_flavors, columns_instance_types)) diff --git a/nova/tests/functional/db/test_instance.py b/nova/tests/functional/db/test_instance.py index 75fcce8577..9e800c755d 100644 --- a/nova/tests/functional/db/test_instance.py +++ b/nova/tests/functional/db/test_instance.py @@ -10,6 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +from oslo_utils import uuidutils + from nova.compute import vm_states from nova import context from nova import objects @@ -40,3 +42,34 @@ class InstanceObjectTestCase(test.TestCase): self.context, self.context.project_id, self.context.user_id, vm_states.ACTIVE) self.assertEqual(1, count) + + def test_embedded_instance_flavor_description_is_not_persisted(self): + """The instance.flavor.description field will not be exposed out + of the REST API when showing server details, so we want to make + sure the embedded instance.flavor.description is not persisted with + the instance_extra.flavor information. + """ + # Create a flavor with a description. + flavorid = uuidutils.generate_uuid() + flavor = objects.Flavor(context.get_admin_context(), + name=flavorid, flavorid=flavorid, + memory_mb=2048, vcpus=2, + description='do not persist me in an instance') + flavor.create() + + # Now create the instance with that flavor. + instance = self._create_instance(flavor=flavor) + + # Make sure the embedded flavor.description is nulled out. + self.assertIsNone(instance.flavor.description) + + # Now set the flavor on the instance again to make sure save() does + # not persist the flavor.description value. + instance.flavor = flavor + self.assertIn('flavor', list(instance.obj_what_changed())) + instance.save() + + # Get the instance from the database since our old version is dirty. + instance = objects.Instance.get_by_uuid( + self.context, instance.uuid, expected_attrs=['flavor']) + self.assertIsNone(instance.flavor.description) diff --git a/nova/tests/unit/api/openstack/compute/test_flavor_access.py b/nova/tests/unit/api/openstack/compute/test_flavor_access.py index ef2f8048c3..20a73ef996 100644 --- a/nova/tests/unit/api/openstack/compute/test_flavor_access.py +++ b/nova/tests/unit/api/openstack/compute/test_flavor_access.py @@ -44,7 +44,8 @@ def generate_flavor(flavorid, ispublic): 'disabled': False, 'extra_specs': {}, 'vcpu_weight': None, - 'is_public': bool(ispublic) + 'is_public': bool(ispublic), + 'description': None } diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 8a773e7680..e8a66186e3 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -105,6 +105,7 @@ class _ComputeAPIUnitTestMixIn(object): 'created_at': datetime.datetime(2012, 1, 19, 18, 49, 30, 877329), 'updated_at': None, + 'description': None } if updates: flavor.update(updates) diff --git a/nova/tests/unit/db/fakes.py b/nova/tests/unit/db/fakes.py index c0ce7e36cc..b033fecada 100644 --- a/nova/tests/unit/db/fakes.py +++ b/nova/tests/unit/db/fakes.py @@ -362,6 +362,7 @@ def stub_out_db_instance_api(test, injected=True): 'disabled': False, 'is_public': True, 'extra_specs': {}, + 'description': None } if updates: instance_type.update(updates) diff --git a/nova/tests/unit/db/test_sqlalchemy_migration.py b/nova/tests/unit/db/test_sqlalchemy_migration.py index 8f4f7cdedf..c12d838583 100644 --- a/nova/tests/unit/db/test_sqlalchemy_migration.py +++ b/nova/tests/unit/db/test_sqlalchemy_migration.py @@ -382,11 +382,14 @@ class TestNewtonCellsCheck(test.NoDBTestCase): self.engine = db_api.get_api_engine() def _flavor_me(self): - flavor = objects.Flavor(context=self.context, - name='foo', memory_mb=123, - vcpus=1, root_gb=1, - flavorid='m1.foo') - flavor.create() + # We can't use the Flavor object or model to create the flavor because + # the model and object have the description field now but at this point + # we have not run the migration schema to add the description column. + flavors = db_utils.get_table(self.engine, 'flavors') + values = dict(name='foo', memory_mb=123, + vcpus=1, root_gb=1, + flavorid='m1.foo', swap=0) + flavors.insert().execute(values) def test_upgrade_with_no_cell_mappings(self): self._flavor_me() diff --git a/nova/tests/unit/fake_flavor.py b/nova/tests/unit/fake_flavor.py index dd1b0b15a8..178835944b 100644 --- a/nova/tests/unit/fake_flavor.py +++ b/nova/tests/unit/fake_flavor.py @@ -29,7 +29,8 @@ def fake_db_flavor(**updates): 'disabled': False, 'is_public': True, 'extra_specs': {}, - 'projects': [] + 'projects': [], + 'description': None } for name, field in objects.Flavor.fields.items(): diff --git a/nova/tests/unit/objects/test_flavor.py b/nova/tests/unit/objects/test_flavor.py index e6291cf29c..1162f3be0f 100644 --- a/nova/tests/unit/objects/test_flavor.py +++ b/nova/tests/unit/objects/test_flavor.py @@ -16,7 +16,9 @@ import datetime import mock from oslo_db import exception as db_exc +from oslo_utils import uuidutils +from nova import context as nova_context from nova.db.sqlalchemy import api as db_api from nova.db.sqlalchemy import api_models from nova import exception @@ -41,6 +43,7 @@ fake_flavor = { 'disabled': False, 'is_public': True, 'extra_specs': {'foo': 'bar'}, + 'description': None } @@ -321,6 +324,47 @@ class TestFlavor(test_objects._LocalTest, _TestFlavor): flavor = objects.Flavor.get_by_id(self.context, db_flavor['id']) self.assertEqual({'marty': 'mcfly'}, flavor.extra_specs) + # NOTE(mriedem): There is no remotable method for updating the description + # in a flavor so we test this local-only. + @mock.patch('nova.objects.Flavor._send_notification') + def test_description(self, mock_notify): + # Create a flavor with a description. + ctxt = nova_context.get_admin_context() + flavorid = uuidutils.generate_uuid() + dict_flavor = dict(fake_flavor, name=flavorid, flavorid=flavorid) + del dict_flavor['id'] + flavor = flavor_obj.Flavor(ctxt, **dict_flavor) + flavor.description = 'rainbows and unicorns' + flavor.create() + mock_notify.assert_called_once_with('create') + # Lookup the flavor to make sure the description is set. + flavor = flavor_obj.Flavor.get_by_flavor_id(ctxt, flavorid) + self.assertEqual('rainbows and unicorns', flavor.description) + + # Now reset the flavor.description since it's nullable=True. + mock_notify.reset_mock() + self.assertEqual(0, len(flavor.obj_what_changed()), + flavor.obj_what_changed()) + flavor.description = None + self.assertEqual(['description'], list(flavor.obj_what_changed()), + flavor.obj_what_changed()) + old_updated_at = flavor.updated_at + flavor.save() + # Make sure we reloaded the flavor from the database. + self.assertNotEqual(old_updated_at, flavor.updated_at) + mock_notify.assert_called_once_with('update') + self.assertEqual(0, len(flavor.obj_what_changed()), + flavor.obj_what_changed()) + # Lookup the flavor to make sure the description is gone. + flavor = flavor_obj.Flavor.get_by_flavor_id(ctxt, flavorid) + self.assertIsNone(flavor.description) + + # Test compatibility. + flavor.description = 'flavor descriptions are not backward compatible' + flavor_primitive = flavor.obj_to_primitive() + flavor.obj_make_compatible(flavor_primitive, '1.1') + self.assertNotIn('description', flavor_primitive) + class TestFlavorRemote(test_objects._RemoteTest, _TestFlavor): pass diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index 360c0c6326..a9d8a22e53 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1087,7 +1087,7 @@ object_data = { 'EC2VolumeMapping': '1.0-5b713751d6f97bad620f3378a521020d', 'FixedIP': '1.14-53e1c10b539f1a82fe83b1af4720efae', 'FixedIPList': '1.15-07b6261cef836cb09d2d8673f68ece15', - 'Flavor': '1.1-b6bb7a730a79d720344accefafacf7ee', + 'Flavor': '1.2-4ce99b41327bb230262e5a8f45ff0ce3', 'FlavorList': '1.1-52b5928600e7ca973aa4fc1e46f3934c', 'FloatingIP': '1.10-52a67d52d85eb8b3f324a5b7935a335b', 'FloatingIPList': '1.12-e4debd21fddb12cf40d36f737225fa9d',