From 1160921c2d053ce33279ca4ec1f00572271e7c95 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Tue, 26 Jun 2018 16:06:20 -0400 Subject: [PATCH] Remove compatibility code for instance groups The /345_require_online_migration_completion cell database migration added in Ocata prevents anyone from upgrading the cell database schema until they have performed the online instance group data migrations via the "nova-manage db online_data_migrations" command. This means we can remove the compatibility code in the InstanceGroup object which performs a lookup routine in the API database and if not found it falls back to the cell database, which at this point should be empty, therefore making that dead code now. The test_get_by_name and test_get_by_hint tests are moved from _TestInstanceGroupListObject to _TestInstanceGroupObject since they don't have anything to do with InstanceGroupList. A follow up change will remove the now unused main DB API methods for instance groups. Change-Id: Ifbd53b13fa0fef62e0329283b73d587f367e46c2 --- nova/cmd/manage.py | 3 - nova/objects/instance_group.py | 123 ++-------------- .../functional/db/test_instance_group.py | 133 +----------------- .../db/test_instance_group_model.py | 2 + .../compute/test_server_group_quotas.py | 17 +-- .../openstack/compute/test_server_groups.py | 37 +++-- .../tests/unit/objects/test_instance_group.py | 130 ++++++++--------- 7 files changed, 100 insertions(+), 345 deletions(-) diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index 0ee52571837..4e736a667a0 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -60,7 +60,6 @@ from nova.objects import build_request as build_request_obj from nova.objects import host_mapping as host_mapping_obj from nova.objects import instance as instance_obj -from nova.objects import instance_group as instance_group_obj from nova.objects import keypair as keypair_obj from nova.objects import quotas as quotas_obj from nova.objects import request_spec @@ -394,8 +393,6 @@ class DbCommands(object): request_spec.migrate_instances_add_request_spec, # Added in Newton keypair_obj.migrate_keypairs_to_api_db, - # Added in Newton - instance_group_obj.migrate_instance_groups_to_api_db, # Added in Ocata # NOTE(mriedem): This online migration is going to be backported to # Newton also since it's an upgrade issue when upgrading from Mitaka. diff --git a/nova/objects/instance_group.py b/nova/objects/instance_group.py index 28d1545615b..708788db49e 100644 --- a/nova/objects/instance_group.py +++ b/nova/objects/instance_group.py @@ -21,10 +21,8 @@ from sqlalchemy.orm import joinedload from nova.compute import utils as compute_utils -from nova import db from nova.db.sqlalchemy import api as db_api from nova.db.sqlalchemy import api_models -from nova.db.sqlalchemy import models as main_models from nova import exception from nova import objects from nova.objects import base @@ -108,6 +106,7 @@ def _instance_group_members_add_by_uuid(context, group_uuid, members): # TODO(berrange): Remove NovaObjectDictCompat +# TODO(mriedem): Replace NovaPersistentObject with TimestampedObject in v2.0. @base.NovaObjectRegistry.register class InstanceGroup(base.NovaPersistentObject, base.NovaObject, base.NovaObjectDictCompat): @@ -160,6 +159,7 @@ def _from_db_object(context, instance_group, db_inst): # the api database, we have removed soft-delete, so # the object fields for delete must be filled in with # default values for db models from the api database. + # TODO(mriedem): Remove this when NovaPersistentObject is removed. ignore = {'deleted': False, 'deleted_at': None} if field in ignore and not hasattr(db_inst, field): @@ -212,12 +212,7 @@ def _get_from_db_by_instance(context, instance_uuid): @staticmethod @db_api.api_context_manager.writer def _save_in_db(context, group_uuid, values): - grp = _instance_group_get_query(context, - id_field=api_models.InstanceGroup.uuid, - id=group_uuid).first() - if not grp: - raise exception.InstanceGroupNotFound(group_uuid=group_uuid) - + grp = InstanceGroup._get_from_db_by_uuid(context, group_uuid) values_copy = copy.copy(values) policies = values_copy.pop('policies', None) members = values_copy.pop('members', None) @@ -302,38 +297,17 @@ def obj_load_attr(self, attrname): @base.remotable_classmethod def get_by_uuid(cls, context, uuid): - db_group = None - try: - db_group = cls._get_from_db_by_uuid(context, uuid) - except exception.InstanceGroupNotFound: - pass - if db_group is None: - db_group = db.instance_group_get(context, uuid) + db_group = cls._get_from_db_by_uuid(context, uuid) return cls._from_db_object(context, cls(), db_group) @base.remotable_classmethod def get_by_name(cls, context, name): - try: - db_group = cls._get_from_db_by_name(context, name) - except exception.InstanceGroupNotFound: - igs = InstanceGroupList._get_main_by_project_id(context, - context.project_id) - for ig in igs: - if ig.name == name: - return ig - raise exception.InstanceGroupNotFound(group_uuid=name) + db_group = cls._get_from_db_by_name(context, name) return cls._from_db_object(context, cls(), db_group) @base.remotable_classmethod def get_by_instance_uuid(cls, context, instance_uuid): - db_group = None - try: - db_group = cls._get_from_db_by_instance(context, instance_uuid) - except exception.InstanceGroupNotFound: - pass - if db_group is None: - db_group = db.instance_group_get_by_instance(context, - instance_uuid) + db_group = cls._get_from_db_by_instance(context, instance_uuid) return cls._from_db_object(context, cls(), db_group) @classmethod @@ -367,11 +341,7 @@ def save(self): payload = dict(updates) payload['server_group_id'] = self.uuid - try: - db_group = self._save_in_db(self._context, self.uuid, updates) - except exception.InstanceGroupNotFound: - db.instance_group_update(self._context, self.uuid, updates) - db_group = db.instance_group_get(self._context, self.uuid) + db_group = self._save_in_db(self._context, self.uuid, updates) self._from_db_object(self._context, self, db_group) compute_utils.notify_about_server_group_update(self._context, "update", payload) @@ -385,10 +355,8 @@ def refresh(self): self[field] = current[field] self.obj_reset_changes() - def _create(self, skipcheck=False): - # NOTE(danms): This is just for the migration routine, and - # can be removed once we're no longer supporting the migration - # of instance groups from the main to api database. + @base.remotable + def create(self): if self.obj_attr_is_set('id'): raise exception.ObjectActionError(action='create', reason='already created') @@ -402,14 +370,6 @@ def _create(self, skipcheck=False): self.uuid = uuidutils.generate_uuid() updates['uuid'] = self.uuid - if not skipcheck: - try: - db.instance_group_get(self._context, self.uuid) - raise exception.ObjectActionError( - action='create', - reason='already created in main') - except exception.InstanceGroupNotFound: - pass db_group = self._create_in_db(self._context, updates, policies=policies, members=members) @@ -422,17 +382,10 @@ def _create(self, skipcheck=False): group=self, action=fields.NotificationAction.CREATE) - @base.remotable - def create(self): - self._create() - @base.remotable def destroy(self): payload = {'server_group_id': self.uuid} - try: - self._destroy_in_db(self._context, self.uuid) - except exception.InstanceGroupNotFound: - db.instance_group_delete(self._context, self.uuid) + self._destroy_in_db(self._context, self.uuid) self.obj_reset_changes() compute_utils.notify_about_server_group_update(self._context, "delete", payload) @@ -445,13 +398,9 @@ def destroy(self): def add_members(cls, context, group_uuid, instance_uuids): payload = {'server_group_id': group_uuid, 'instance_uuids': instance_uuids} - try: - members = cls._add_members_in_db(context, group_uuid, - instance_uuids) - members = [member['instance_uuid'] for member in members] - except exception.InstanceGroupNotFound: - members = db.instance_group_members_add(context, group_uuid, - instance_uuids) + members = cls._add_members_in_db(context, group_uuid, + instance_uuids) + members = [member['instance_uuid'] for member in members] compute_utils.notify_about_server_group_update(context, "addmember", payload) compute_utils.notify_about_server_group_add_member(context, group_uuid) @@ -511,13 +460,6 @@ def _get_from_db(context, project_id=None): query = query.filter_by(project_id=project_id) return query.all() - @classmethod - def _get_main_by_project_id(cls, context, project_id): - main_db_groups = db.instance_group_get_all_by_project_id(context, - project_id) - return base.obj_make_list(context, cls(context), objects.InstanceGroup, - main_db_groups) - @staticmethod @db_api.api_context_manager.reader def _get_counts_from_db(context, project_id, user_id=None): @@ -533,17 +475,14 @@ def _get_counts_from_db(context, project_id, user_id=None): @base.remotable_classmethod def get_by_project_id(cls, context, project_id): api_db_groups = cls._get_from_db(context, project_id=project_id) - main_db_groups = db.instance_group_get_all_by_project_id(context, - project_id) return base.obj_make_list(context, cls(context), objects.InstanceGroup, - api_db_groups + main_db_groups) + api_db_groups) @base.remotable_classmethod def get_all(cls, context): api_db_groups = cls._get_from_db(context) - main_db_groups = db.instance_group_get_all(context) return base.obj_make_list(context, cls(context), objects.InstanceGroup, - api_db_groups + main_db_groups) + api_db_groups) @base.remotable_classmethod def get_counts(cls, context, project_id, user_id=None): @@ -559,35 +498,3 @@ def get_counts(cls, context, project_id, user_id=None): 'user': {'server_groups': }} """ return cls._get_counts_from_db(context, project_id, user_id=user_id) - - -@db_api.pick_context_manager_reader -def _get_main_instance_groups(context, limit): - return context.session.query(main_models.InstanceGroup).\ - options(joinedload('_policies')).\ - options(joinedload('_members')).\ - filter_by(deleted=0).\ - limit(limit).\ - all() - - -def migrate_instance_groups_to_api_db(context, count): - main_groups = _get_main_instance_groups(context, count) - done = 0 - for db_group in main_groups: - group = objects.InstanceGroup(context=context, - user_id=db_group.user_id, - project_id=db_group.project_id, - uuid=db_group.uuid, - name=db_group.name, - policies=db_group.policies, - members=db_group.members) - try: - group._create(skipcheck=True) - except exception.InstanceGroupIdExists: - # NOTE(melwitt): This might happen if there's a failure right after - # the InstanceGroup was created and the migration is re-run. - pass - db_api.instance_group_delete(context, db_group.uuid) - done += 1 - return len(main_groups), done diff --git a/nova/tests/functional/db/test_instance_group.py b/nova/tests/functional/db/test_instance_group.py index 57a8d7d519d..db809d7e313 100644 --- a/nova/tests/functional/db/test_instance_group.py +++ b/nova/tests/functional/db/test_instance_group.py @@ -14,11 +14,9 @@ from oslo_versionedobjects import fixture as ovo_fixture from nova import context -from nova.db.sqlalchemy import api as db_api from nova import exception from nova import objects from nova.objects import base -from nova.objects import instance_group from nova import test from nova.tests import uuidsentinel as uuids @@ -39,19 +37,6 @@ def _api_group(self, **values): group.create() return group - def _main_group(self, policies=None, members=None, **values): - vals = { - 'user_id': self.context.user_id, - 'project_id': self.context.project_id, - 'name': 'foogroup', - } - vals.update(values) - policies = policies or ['foo1', 'foo2'] - members = members or ['memberfoo'] - return db_api.instance_group_create(self.context, vals, - policies=policies, - members=members) - def test_create(self): create_group = self._api_group() db_group = create_group._get_from_db_by_uuid(self.context, @@ -59,11 +44,6 @@ def test_create(self): ovo_fixture.compare_obj(self, create_group, db_group, allow_missing=('deleted', 'deleted_at')) - def test_create_duplicate_in_main(self): - self._main_group(uuid=uuids.main) - self.assertRaises(exception.ObjectActionError, - self._api_group, uuid=uuids.main) - def test_destroy(self): create_group = self._api_group() create_group.destroy() @@ -71,15 +51,6 @@ def test_destroy(self): create_group._get_from_db_by_uuid, self.context, create_group.uuid) - def test_destroy_main(self): - db_group = self._main_group() - create_group = objects.InstanceGroup._from_db_object( - self.context, objects.InstanceGroup(), db_group) - create_group.destroy() - self.assertRaises(exception.InstanceGroupNotFound, - db_api.instance_group_get, self.context, - create_group.uuid) - @mock.patch('nova.compute.utils.notify_about_server_group_update') def test_save(self, _mock_notify): create_group = self._api_group() @@ -92,18 +63,6 @@ def test_save(self, _mock_notify): ovo_fixture.compare_obj(self, create_group, db_group, allow_missing=('deleted', 'deleted_at')) - @mock.patch('nova.compute.utils.notify_about_server_group_update') - def test_save_main(self, _mock_notify): - db_group = self._main_group() - create_group = objects.InstanceGroup._from_db_object( - self.context, objects.InstanceGroup(), db_group) - create_group.policies = ['bar1', 'bar2'] - create_group.members = ['memberbar1', 'memberbar2'] - create_group.name = 'anewname' - create_group.save() - db_group = db_api.instance_group_get(self.context, create_group.uuid) - ovo_fixture.compare_obj(self, create_group, db_group) - def test_add_members(self): create_group = self._api_group() new_member = ['memberbar'] @@ -113,16 +72,6 @@ def test_add_members(self): create_group.uuid) self.assertEqual(create_group.members + new_member, db_group.members) - def test_add_members_main(self): - db_group = self._main_group() - create_group = objects.InstanceGroup._from_db_object( - self.context, objects.InstanceGroup(), db_group) - new_member = ['memberbar'] - objects.InstanceGroup.add_members(self.context, create_group.uuid, - new_member) - db_group = db_api.instance_group_get(self.context, create_group.uuid) - self.assertEqual(create_group.members + new_member, db_group.members) - def test_add_members_to_group_with_no_members(self): create_group = self._api_group(members=[]) new_member = ['memberbar'] @@ -157,52 +106,32 @@ def test_get_by_uuid(self): create_group.uuid) self.assertTrue(base.obj_equal_prims(create_group, get_group)) - def test_get_by_uuid_main(self): - db_group = self._main_group() - get_group = objects.InstanceGroup.get_by_uuid(self.context, - db_group.uuid) - ovo_fixture.compare_obj(self, get_group, db_group) - def test_get_by_name(self): create_group = self._api_group() get_group = objects.InstanceGroup.get_by_name(self.context, create_group.name) self.assertTrue(base.obj_equal_prims(create_group, get_group)) - def test_get_by_name_main(self): - db_group = self._main_group() - get_group = objects.InstanceGroup.get_by_name(self.context, - db_group.name) - ovo_fixture.compare_obj(self, get_group, db_group) - def test_get_by_instance_uuid(self): create_group = self._api_group(members=[uuids.instance]) get_group = objects.InstanceGroup.get_by_instance_uuid(self.context, uuids.instance) self.assertTrue(base.obj_equal_prims(create_group, get_group)) - def test_get_by_instance_uuid_main(self): - db_group = self._main_group(members=[uuids.instance]) - get_group = objects.InstanceGroup.get_by_instance_uuid(self.context, - uuids.instance) - ovo_fixture.compare_obj(self, get_group, db_group) - def test_get_by_project_id(self): create_group = self._api_group() - db_group = self._main_group() get_groups = objects.InstanceGroupList.get_by_project_id( self.context, self.context.project_id) - self.assertEqual(2, len(get_groups)) + self.assertEqual(1, len(get_groups)) self.assertTrue(base.obj_equal_prims(create_group, get_groups[0])) - ovo_fixture.compare_obj(self, get_groups[1], db_group) + ovo_fixture.compare_obj(self, get_groups[0], create_group) def test_get_all(self): create_group = self._api_group() - db_group = self._main_group() get_groups = objects.InstanceGroupList.get_all(self.context) - self.assertEqual(2, len(get_groups)) + self.assertEqual(1, len(get_groups)) self.assertTrue(base.obj_equal_prims(create_group, get_groups[0])) - ovo_fixture.compare_obj(self, get_groups[1], db_group) + ovo_fixture.compare_obj(self, get_groups[0], create_group) def test_get_counts(self): # _api_group() creates a group with project_id and user_id from @@ -223,57 +152,3 @@ def test_get_counts(self): self.assertEqual(2, counts['project']['server_groups']) self.assertEqual(1, counts['user']['server_groups']) - - def test_migrate_instance_groups(self): - self._api_group(name='apigroup') - orig_main_models = [] - orig_main_models.append(self._main_group(name='maingroup1')) - orig_main_models.append(self._main_group(name='maingroup2')) - orig_main_models.append(self._main_group(name='maingroup3')) - - total, done = instance_group.migrate_instance_groups_to_api_db( - self.context, 2) - self.assertEqual(2, total) - self.assertEqual(2, done) - - # This only fetches from the api db - api_groups = objects.InstanceGroupList._get_from_db(self.context) - self.assertEqual(3, len(api_groups)) - - # This only fetches from the main db - main_groups = db_api.instance_group_get_all(self.context) - self.assertEqual(1, len(main_groups)) - - self.assertEqual((1, 1), - instance_group.migrate_instance_groups_to_api_db( - self.context, 100)) - self.assertEqual((0, 0), - instance_group.migrate_instance_groups_to_api_db( - self.context, 100)) - - # Verify the api_models have all their attributes set properly - api_models = objects.InstanceGroupList._get_from_db(self.context) - # Filter out the group that was created in the api db originally - api_models = [x for x in api_models if x.name != 'apigroup'] - key_func = lambda model: model.uuid - api_models = sorted(api_models, key=key_func) - orig_main_models = sorted(orig_main_models, key=key_func) - ignore_fields = ('id', 'hosts', 'deleted', 'deleted_at', 'created_at', - 'updated_at') - for i in range(len(api_models)): - for field in instance_group.InstanceGroup.fields: - if field not in ignore_fields: - self.assertEqual(orig_main_models[i][field], - api_models[i][field]) - - def test_migrate_instance_groups_skips_existing(self): - self._api_group(uuid=uuids.group) - self._main_group(uuid=uuids.group) - total, done = instance_group.migrate_instance_groups_to_api_db( - self.context, 100) - self.assertEqual(1, total) - self.assertEqual(1, done) - total, done = instance_group.migrate_instance_groups_to_api_db( - self.context, 100) - self.assertEqual(0, total) - self.assertEqual(0, done) diff --git a/nova/tests/functional/db/test_instance_group_model.py b/nova/tests/functional/db/test_instance_group_model.py index fa45844c1ac..a2d42ceb80f 100644 --- a/nova/tests/functional/db/test_instance_group_model.py +++ b/nova/tests/functional/db/test_instance_group_model.py @@ -15,6 +15,8 @@ from nova import test +# TODO(mriedem): Drop this test when the "main" DB instance group API +# methods are all removed. class InstanceGroupTablesCompareTestCase(test.NoDBTestCase): def _get_column_list(self, model): column_list = [m.key for m in model.__table__.columns] diff --git a/nova/tests/unit/api/openstack/compute/test_server_group_quotas.py b/nova/tests/unit/api/openstack/compute/test_server_group_quotas.py index c212484e597..8608a478e0c 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_group_quotas.py +++ b/nova/tests/unit/api/openstack/compute/test_server_group_quotas.py @@ -147,25 +147,20 @@ def test_delete_server_group_by_admin(self): self._assert_server_groups_in_use(context.project_id, context.user_id, 0) - def test_delete_server_group_by_id(self): + @mock.patch('nova.objects.InstanceGroup.destroy') + def test_delete_server_group_by_id(self, mock_destroy): self._setup_quotas() sg = server_group_template(id=uuids.sg1_id) - self.called = False - def server_group_delete(context, id): - self.called = True - - def return_server_group(context, group_id): + def return_server_group(_cls, context, group_id): self.assertEqual(sg['id'], group_id) - return server_group_db(sg) + return objects.InstanceGroup(**server_group_db(sg)) - self.stub_out('nova.db.instance_group_delete', - server_group_delete) - self.stub_out('nova.db.instance_group_get', + self.stub_out('nova.objects.InstanceGroup.get_by_uuid', return_server_group) resp = self.controller.delete(self.req, uuids.sg1_id) - self.assertTrue(self.called) + mock_destroy.assert_called_once_with() # NOTE: on v2.1, http status code is set as wsgi_code of API # method instead of status_int in a response object. diff --git a/nova/tests/unit/api/openstack/compute/test_server_groups.py b/nova/tests/unit/api/openstack/compute/test_server_groups.py index 371f372aaa6..c2aaa521e70 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_groups.py +++ b/nova/tests/unit/api/openstack/compute/test_server_groups.py @@ -19,7 +19,6 @@ from nova.api.openstack.compute import server_groups as sg_v21 from nova import context -import nova.db from nova import exception from nova import objects from nova.policies import server_groups as sg_policies @@ -200,8 +199,8 @@ def _test_list_server_group_offset_and_limit(self, api_version='2.1'): limited='&offset=1&limit=1', path='/os-server-groups?all_projects=True') - @mock.patch.object(nova.db, 'instance_group_get_all_by_project_id') - @mock.patch.object(nova.db, 'instance_group_get_all') + @mock.patch('nova.objects.InstanceGroupList.get_by_project_id') + @mock.patch('nova.objects.InstanceGroupList.get_all') def _test_list_server_group(self, mock_get_all, mock_get_by_project, path, api_version='2.1', limited=None): policies = ['anti-affinity'] @@ -247,12 +246,16 @@ def _test_list_server_group(self, mock_get_all, mock_get_by_project, tenant_specific = {'server_groups': tenant_groups} def return_all_server_groups(): - return [server_group_db(sg) for sg in all_groups] + return objects.InstanceGroupList( + objects=[objects.InstanceGroup( + **server_group_db(sg)) for sg in all_groups]) mock_get_all.return_value = return_all_server_groups() def return_tenant_server_groups(): - return [server_group_db(sg) for sg in tenant_groups] + return objects.InstanceGroupList( + objects=[objects.InstanceGroup( + **server_group_db(sg)) for sg in tenant_groups]) mock_get_by_project.return_value = return_tenant_server_groups() @@ -271,7 +274,7 @@ def return_tenant_server_groups(): res_dict = self.controller.index(req) self.assertEqual(tenant_specific, res_dict) - @mock.patch.object(nova.db, 'instance_group_get_all_by_project_id') + @mock.patch('nova.objects.InstanceGroupList.get_by_project_id') def _test_list_server_group_by_tenant(self, mock_get_by_project, api_version='2.1'): policies = ['anti-affinity'] @@ -310,7 +313,9 @@ def _test_list_server_group_by_tenant(self, mock_get_by_project, expected = {'server_groups': groups} def return_server_groups(): - return [server_group_db(sg) for sg in groups] + return objects.InstanceGroupList( + objects=[objects.InstanceGroup( + **server_group_db(sg)) for sg in groups]) return_get_by_project = return_server_groups() mock_get_by_project.return_value = return_get_by_project @@ -587,25 +592,19 @@ def test_list_server_groups_rbac_admin_only(self): "Policy doesn't allow %s to be performed." % rule_name, exc.format_message()) - def test_delete_server_group_by_id(self): + @mock.patch('nova.objects.InstanceGroup.destroy') + def test_delete_server_group_by_id(self, mock_destroy): sg = server_group_template(id=uuidsentinel.sg1_id) - self.called = False - - def server_group_delete(context, id): - self.called = True - - def return_server_group(context, group_id): + def return_server_group(_cls, context, group_id): self.assertEqual(sg['id'], group_id) - return server_group_db(sg) + return objects.InstanceGroup(**server_group_db(sg)) - self.stub_out('nova.db.instance_group_delete', - server_group_delete) - self.stub_out('nova.db.instance_group_get', + self.stub_out('nova.objects.InstanceGroup.get_by_uuid', return_server_group) resp = self.controller.delete(self.req, uuidsentinel.sg1_id) - self.assertTrue(self.called) + mock_destroy.assert_called_once_with() # NOTE: on v2.1, http status code is set as wsgi_code of API # method instead of status_int in a response object. diff --git a/nova/tests/unit/objects/test_instance_group.py b/nova/tests/unit/objects/test_instance_group.py index 48aa992a4e1..da42ee02d15 100644 --- a/nova/tests/unit/objects/test_instance_group.py +++ b/nova/tests/unit/objects/test_instance_group.py @@ -37,22 +37,19 @@ 'name': 'fake_name', 'policies': ['policy1', 'policy2'], 'members': ['instance_id1', 'instance_id2'], - 'deleted': False, 'created_at': _TS_NOW, - 'updated_at': _TS_NOW, - 'deleted_at': None, + 'updated_at': _TS_NOW } class _TestInstanceGroupObject(object): - @mock.patch('nova.db.instance_group_get', return_value=_INST_GROUP_DB) @mock.patch('nova.objects.InstanceGroup._get_from_db_by_uuid', - side_effect=exception.InstanceGroupNotFound(group_uuid=_DB_UUID)) - def test_get_by_uuid_main(self, mock_api_get, mock_db_get): + return_value=_INST_GROUP_DB) + def test_get_by_uuid(self, mock_api_get): obj = objects.InstanceGroup.get_by_uuid(self.context, _DB_UUID) - mock_db_get.assert_called_once_with(self.context, _DB_UUID) + mock_api_get.assert_called_once_with(self.context, _DB_UUID) self.assertEqual(_INST_GROUP_DB['members'], obj.members) self.assertEqual(_INST_GROUP_DB['policies'], obj.policies) self.assertEqual(_DB_UUID, obj.uuid) @@ -60,18 +57,15 @@ def test_get_by_uuid_main(self, mock_api_get, mock_db_get): self.assertEqual(_INST_GROUP_DB['user_id'], obj.user_id) self.assertEqual(_INST_GROUP_DB['name'], obj.name) - @mock.patch('nova.db.instance_group_get_by_instance', + @mock.patch('nova.objects.InstanceGroup._get_from_db_by_instance', return_value=_INST_GROUP_DB) - @mock.patch('nova.objects.InstanceGroup._get_from_db_by_instance') - def test_get_by_instance_uuid_main(self, mock_api_get, mock_db_get): - error = exception.InstanceGroupNotFound(group_uuid='') - mock_api_get.side_effect = error + def test_get_by_instance_uuid(self, mock_api_get): objects.InstanceGroup.get_by_instance_uuid( self.context, mock.sentinel.instance_uuid) - mock_db_get.assert_called_once_with( + mock_api_get.assert_called_once_with( self.context, mock.sentinel.instance_uuid) - @mock.patch('nova.db.instance_group_get') + @mock.patch('nova.objects.InstanceGroup._get_from_db_by_uuid') def test_refresh(self, mock_db_get): changed_group = copy.deepcopy(_INST_GROUP_DB) changed_group['name'] = 'new_name' @@ -84,23 +78,27 @@ def test_refresh(self, mock_db_get): self.assertEqual(set([]), obj.obj_what_changed()) @mock.patch('nova.compute.utils.notify_about_server_group_update') - @mock.patch('nova.db.instance_group_update') - @mock.patch('nova.db.instance_group_get') - def test_save_main(self, mock_db_get, mock_db_update, mock_notify): + @mock.patch('nova.objects.InstanceGroup._get_from_db_by_uuid') + @mock.patch('nova.objects.instance_group._instance_group_policies_add') + @mock.patch('nova.objects.instance_group._instance_group_members_add') + def test_save(self, mock_members_add, mock_policies_add, mock_db_get, + mock_notify): changed_group = copy.deepcopy(_INST_GROUP_DB) changed_group['name'] = 'new_name' - mock_db_get.side_effect = [_INST_GROUP_DB, changed_group] - obj = objects.InstanceGroup.get_by_uuid(self.context, - _DB_UUID) + db_group = copy.deepcopy(_INST_GROUP_DB) + mock_db_get.return_value = db_group + obj = objects.InstanceGroup(self.context, **_INST_GROUP_DB) self.assertEqual(obj.name, 'fake_name') + obj.obj_reset_changes() obj.name = 'new_name' obj.policies = ['policy1'] # Remove policy 2 obj.members = ['instance_id1'] # Remove member 2 obj.save() - mock_db_update.assert_called_once_with(self.context, _DB_UUID, - {'name': 'new_name', - 'members': ['instance_id1'], - 'policies': ['policy1']}) + self.assertEqual(set([]), obj.obj_what_changed()) + mock_policies_add.assert_called_once_with( + self.context, mock_db_get.return_value, ['policy1']) + mock_members_add.assert_called_once_with( + self.context, mock_db_get.return_value, ['instance_id1']) mock_notify.assert_called_once_with(self.context, "update", {'name': 'new_name', 'members': ['instance_id1'], @@ -108,12 +106,11 @@ def test_save_main(self, mock_db_get, mock_db_update, mock_notify): 'server_group_id': _DB_UUID}) @mock.patch('nova.compute.utils.notify_about_server_group_update') - @mock.patch('nova.db.instance_group_update') - @mock.patch('nova.db.instance_group_get') - def test_save_without_hosts_main(self, mock_db_get, mock_db_update, - mock_notify): - mock_db_get.side_effect = [_INST_GROUP_DB, _INST_GROUP_DB] - obj = objects.InstanceGroup.get_by_uuid(self.context, _DB_UUID) + @mock.patch('nova.objects.InstanceGroup._get_from_db_by_uuid') + def test_save_without_hosts(self, mock_db_get, mock_notify): + mock_db_get.return_value = _INST_GROUP_DB + obj = objects.InstanceGroup(self.context, **_INST_GROUP_DB) + obj.obj_reset_changes() obj.hosts = ['fake-host1'] self.assertRaises(exception.InstanceGroupSaveException, obj.save) @@ -121,7 +118,6 @@ def test_save_without_hosts_main(self, mock_db_get, mock_db_update, obj.obj_reset_changes(['hosts']) obj.save() # since hosts was the only update, there is no actual call - self.assertFalse(mock_db_update.called) self.assertFalse(mock_notify.called) @mock.patch('nova.compute.utils.notify_about_server_group_action') @@ -138,8 +134,6 @@ def test_create(self, mock_db_create, mock_notify, mock_notify_action): obj.policies = _INST_GROUP_DB['policies'] obj.updated_at = _TS_NOW obj.created_at = _TS_NOW - obj.deleted_at = None - obj.deleted = False obj.create() mock_db_create.assert_called_once_with( self.context, @@ -149,8 +143,6 @@ def test_create(self, mock_db_create, mock_notify, mock_notify_action): 'project_id': _INST_GROUP_DB['project_id'], 'created_at': _TS_NOW, 'updated_at': _TS_NOW, - 'deleted_at': None, - 'deleted': False, }, members=_INST_GROUP_DB['members'], policies=_INST_GROUP_DB['policies']) @@ -162,8 +154,6 @@ def test_create(self, mock_db_create, mock_notify, mock_notify_action): 'project_id': _INST_GROUP_DB['project_id'], 'created_at': _TS_NOW, 'updated_at': _TS_NOW, - 'deleted_at': None, - 'deleted': False, 'members': _INST_GROUP_DB['members'], 'policies': _INST_GROUP_DB['policies'], 'server_group_id': _DB_UUID}) @@ -176,8 +166,6 @@ def _group_matcher(group): group.project_id == _INST_GROUP_DB['project_id'] and group.created_at == _TS_NOW and group.updated_at == _TS_NOW and - group.deleted_at is None and - group.deleted is False and group.members == _INST_GROUP_DB['members'] and group.policies == _INST_GROUP_DB['policies'] and group.id == 1) @@ -296,6 +284,26 @@ def test_load_anything_else_but_hosts(self): obj = objects.InstanceGroup(self.context) self.assertRaises(exception.ObjectActionError, getattr, obj, 'members') + @mock.patch('nova.objects.InstanceGroup._get_from_db_by_name') + def test_get_by_name(self, mock_api_get): + db_group = copy.deepcopy(_INST_GROUP_DB) + mock_api_get.side_effect = [ + db_group, exception.InstanceGroupNotFound(group_uuid='unknown')] + ig = objects.InstanceGroup.get_by_name(self.context, 'fake_name') + mock_api_get.assert_called_once_with(self.context, 'fake_name') + self.assertEqual('fake_name', ig.name) + self.assertRaises(exception.InstanceGroupNotFound, + objects.InstanceGroup.get_by_name, + self.context, 'unknown') + + @mock.patch('nova.objects.InstanceGroup.get_by_uuid') + @mock.patch('nova.objects.InstanceGroup.get_by_name') + def test_get_by_hint(self, mock_name, mock_uuid): + objects.InstanceGroup.get_by_hint(self.context, _DB_UUID) + mock_uuid.assert_called_once_with(self.context, _DB_UUID) + objects.InstanceGroup.get_by_hint(self.context, 'name') + mock_name.assert_called_once_with(self.context, 'name') + class TestInstanceGroupObject(test_objects._LocalTest, _TestInstanceGroupObject): @@ -307,7 +315,7 @@ class TestRemoteInstanceGroupObject(test_objects._RemoteTest, pass -def _mock_db_list_get(*args): +def _mock_db_list_get(*args, **kwargs): instances = [(uuids.f1, 'f1', 'p1'), (uuids.f2, 'f2', 'p1'), (uuids.f3, 'f3', 'p2'), @@ -324,48 +332,20 @@ def _mock_db_list_get(*args): class _TestInstanceGroupListObject(object): - @mock.patch('nova.db.instance_group_get_all') @mock.patch('nova.objects.InstanceGroupList._get_from_db') - def test_list_all_main(self, mock_api_get, mock_db_get): - mock_api_get.return_value = [] - mock_db_get.side_effect = _mock_db_list_get + def test_list_all(self, mock_api_get): + mock_api_get.side_effect = _mock_db_list_get inst_list = objects.InstanceGroupList.get_all(self.context) self.assertEqual(4, len(inst_list.objects)) - mock_db_get.assert_called_once_with(self.context) + mock_api_get.assert_called_once_with(self.context) - @mock.patch('nova.db.instance_group_get_all_by_project_id') @mock.patch('nova.objects.InstanceGroupList._get_from_db') - def test_list_by_project_id_main(self, mock_api_get, mock_db_get): - mock_api_get.return_value = [] - mock_db_get.side_effect = _mock_db_list_get + def test_list_by_project_id(self, mock_api_get): + mock_api_get.side_effect = _mock_db_list_get objects.InstanceGroupList.get_by_project_id( self.context, mock.sentinel.project_id) - mock_db_get.assert_called_once_with( - self.context, mock.sentinel.project_id) - - @mock.patch('nova.db.instance_group_get_all_by_project_id') - @mock.patch('nova.objects.InstanceGroup._get_from_db_by_name') - def test_get_by_name_main(self, mock_api_get, mock_db_get): - error = exception.InstanceGroupNotFound(group_uuid='f1') - mock_api_get.side_effect = error - mock_db_get.side_effect = _mock_db_list_get - # Need the project_id value set, otherwise we'd use mock.sentinel - mock_ctx = mock.MagicMock() - mock_ctx.project_id = 'fake_project' - ig = objects.InstanceGroup.get_by_name(mock_ctx, 'f1') - mock_db_get.assert_called_once_with(mock_ctx, 'fake_project') - self.assertEqual('f1', ig.name) - self.assertRaises(exception.InstanceGroupNotFound, - objects.InstanceGroup.get_by_name, - mock_ctx, 'unknown') - - @mock.patch('nova.objects.InstanceGroup.get_by_uuid') - @mock.patch('nova.objects.InstanceGroup.get_by_name') - def test_get_by_hint(self, mock_name, mock_uuid): - objects.InstanceGroup.get_by_hint(self.context, _DB_UUID) - mock_uuid.assert_called_once_with(self.context, _DB_UUID) - objects.InstanceGroup.get_by_hint(self.context, 'name') - mock_name.assert_called_once_with(self.context, 'name') + mock_api_get.assert_called_once_with( + self.context, project_id=mock.sentinel.project_id) class TestInstanceGroupListObject(test_objects._LocalTest,