diff --git a/nova/objects/instance.py b/nova/objects/instance.py index f6d212b420..36172fbbe7 100644 --- a/nova/objects/instance.py +++ b/nova/objects/instance.py @@ -429,7 +429,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject, if 'security_groups' in expected_attrs: sec_groups = base.obj_make_list( context, objects.SecurityGroupList(context), - objects.SecurityGroup, db_inst.get('security_groups', [])) + objects.SecurityGroup, []) instance['security_groups'] = sec_groups if 'tags' in expected_attrs: @@ -525,7 +525,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject, @base.remotable_classmethod def get_by_uuid(cls, context, uuid, expected_attrs=None, use_slave=False): if expected_attrs is None: - expected_attrs = ['info_cache', 'security_groups'] + expected_attrs = ['info_cache'] columns_to_join = _expected_cols(expected_attrs) db_inst = cls._db_instance_get_by_uuid(context, uuid, columns_to_join, use_slave=use_slave) @@ -535,7 +535,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject, @base.remotable_classmethod def get_by_id(cls, context, inst_id, expected_attrs=None): if expected_attrs is None: - expected_attrs = ['info_cache', 'security_groups'] + expected_attrs = ['info_cache'] columns_to_join = _expected_cols(expected_attrs) db_inst = db.instance_get(context, inst_id, columns_to_join=columns_to_join) @@ -576,10 +576,6 @@ class Instance(base.NovaPersistentObject, base.NovaObject, expected_attrs = [attr for attr in INSTANCE_DEFAULT_FIELDS if attr in updates] - # TODO(stephenfin): Remove this as it's related to nova-network - if 'security_groups' in updates: - updates['security_groups'] = [x.name for x in - updates['security_groups']] if 'info_cache' in updates: updates['info_cache'] = { 'network_info': updates['info_cache'].network_info.json() @@ -699,11 +695,9 @@ class Instance(base.NovaPersistentObject, base.NovaObject, # TODO(stephenfin): Remove this as it's related to nova-network def _save_security_groups(self, context): - security_groups = self.security_groups or [] - for secgroup in security_groups: - with secgroup.obj_alternate_context(context): - secgroup.save() - self.security_groups.obj_reset_changes() + # NOTE(stephenfin): We no longer bother saving these since they + # shouldn't be created in the first place + pass def _save_fault(self, context): # NOTE(danms): I don't think we need to worry about this, do we? @@ -1007,11 +1001,6 @@ class Instance(base.NovaPersistentObject, base.NovaObject, def _load_ec2_ids(self): self.ec2_ids = objects.EC2Ids.get_by_instance(self._context, self) - # TODO(stephenfin): Remove this as it's related to nova-network - def _load_security_groups(self): - self.security_groups = objects.SecurityGroupList.get_by_instance( - self._context, self) - def _load_pci_devices(self): self.pci_devices = objects.PciDeviceList.get_by_instance_uuid( self._context, self.uuid) @@ -1198,7 +1187,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject, elif attrname == 'resources': return self._load_resources() elif attrname == 'security_groups': - self._load_security_groups() + self.security_groups = objects.SecurityGroupList() elif attrname == 'pci_devices': self._load_pci_devices() elif 'flavor' in attrname: @@ -1560,17 +1549,12 @@ class InstanceList(base.ObjectListBase, base.NovaObject): # TODO(stephenfin): Remove this as it's related to nova-network @base.remotable_classmethod def get_by_security_group_id(cls, context, security_group_id): - db_secgroup = db.security_group_get( - context, security_group_id, - columns_to_join=['instances.info_cache', - 'instances.system_metadata']) - return _make_instance_list(context, cls(), db_secgroup['instances'], - ['info_cache', 'system_metadata']) + raise NotImplementedError() # TODO(stephenfin): Remove this as it's related to nova-network @classmethod def get_by_security_group(cls, context, security_group): - return cls.get_by_security_group_id(context, security_group.id) + raise NotImplementedError() # TODO(stephenfin): Remove this as it's related to nova-network @base.remotable_classmethod diff --git a/nova/tests/unit/objects/test_instance.py b/nova/tests/unit/objects/test_instance.py index 6341da6cce..998f6d224d 100644 --- a/nova/tests/unit/objects/test_instance.py +++ b/nova/tests/unit/objects/test_instance.py @@ -36,7 +36,6 @@ from nova.objects import fields from nova.objects import instance from nova.objects import instance_info_cache from nova.objects import pci_device -from nova.objects import security_group from nova import test from nova.tests.unit import fake_instance from nova.tests.unit.objects import test_instance_device_metadata @@ -46,7 +45,6 @@ from nova.tests.unit.objects import test_instance_numa from nova.tests.unit.objects import test_instance_pci_requests from nova.tests.unit.objects import test_migration_context as test_mig_ctxt from nova.tests.unit.objects import test_objects -from nova.tests.unit.objects import test_security_group from nova.tests.unit.objects import test_vcpu_model from nova import utils @@ -66,7 +64,6 @@ class _TestInstanceObject(object): db_inst['launched_at'] = datetime.datetime(1955, 11, 12, 22, 4, 0) db_inst['deleted'] = False - db_inst['security_groups'] = [] db_inst['pci_devices'] = [] db_inst['user_id'] = self.context.user_id db_inst['project_id'] = self.context.project_id @@ -380,7 +377,7 @@ class _TestInstanceObject(object): inst = objects.Instance.get_by_id(self.context, 'instid') self.assertEqual(self.fake_instance['uuid'], inst.uuid) mock_get.assert_called_once_with(self.context, 'instid', - columns_to_join=['info_cache', 'security_groups']) + columns_to_join=['info_cache']) @mock.patch.object(db, 'instance_get_by_uuid') def test_load(self, mock_get): @@ -399,8 +396,7 @@ class _TestInstanceObject(object): self.assertEqual({'foo': 'bar'}, meta2) call_list = [mock.call(self.context, fake_uuid, - columns_to_join=['info_cache', - 'security_groups']), + columns_to_join=['info_cache']), mock.call(self.context, fake_uuid, columns_to_join=['metadata']), ] @@ -475,7 +471,7 @@ class _TestInstanceObject(object): str(inst.access_ip_v6)) mock_get.assert_called_once_with(self.context, uuids.instance, - columns_to_join=['info_cache', 'security_groups']) + columns_to_join=['info_cache']) @mock.patch.object(instance_info_cache.InstanceInfoCache, 'refresh') @mock.patch.object(db, 'instance_get_by_uuid') @@ -492,11 +488,9 @@ class _TestInstanceObject(object): self.assertEqual(set([]), inst.obj_what_changed()) get_call_list = [mock.call(self.context, fake_uuid, - columns_to_join=['info_cache', - 'security_groups']), + columns_to_join=['info_cache']), mock.call(self.context, fake_uuid, - columns_to_join=['info_cache', - 'security_groups']), + columns_to_join=['info_cache']), ] mock_get.assert_has_calls(get_call_list, any_order=False) mock_refresh.assert_called_once_with() @@ -566,12 +560,10 @@ class _TestInstanceObject(object): # NOTE(danms): Ignore flavor migrations for the moment self.assertEqual(set([]), inst.obj_what_changed() - set(['flavor'])) mock_db_instance_get_by_uuid.assert_called_once_with( - self.context, fake_uuid, columns_to_join=['info_cache', - 'security_groups']) + self.context, fake_uuid, columns_to_join=['info_cache']) mock_db_instance_update_and_get_original.assert_called_once_with( self.context, fake_uuid, expected_updates, - columns_to_join=['info_cache', 'security_groups', - 'system_metadata'] + columns_to_join=['info_cache', 'system_metadata'] ) mock_notifications_send_update.assert_called_with(self.context, mock.ANY, @@ -609,10 +601,10 @@ class _TestInstanceObject(object): self.assertEqual(set([]), inst.obj_what_changed() - set(['flavor'])) mock_get.assert_called_once_with(self.context, fake_uuid, - columns_to_join=['info_cache', 'security_groups']) + columns_to_join=['info_cache']) mock_update_and_get.assert_called_once_with(self.context, fake_uuid, - expected_updates, columns_to_join=['info_cache', 'security_groups', - 'system_metadata']) + expected_updates, columns_to_join=['info_cache', 'system_metadata'] + ) mock_send.assert_called_once_with(self.context, mock.ANY, mock.ANY) @mock.patch('nova.db.main.api.instance_extra_update_by_uuid') @@ -794,7 +786,7 @@ class _TestInstanceObject(object): # NOTE(danms): Make sure it's actually a bool self.assertTrue(inst.deleted) mock_get.assert_called_once_with(self.context, fake_uuid, - columns_to_join=['info_cache', 'security_groups']) + columns_to_join=['info_cache']) @mock.patch.object(db, 'instance_get_by_uuid') def test_get_not_cleaned(self, mock_get): @@ -806,7 +798,7 @@ class _TestInstanceObject(object): # NOTE(mikal): Make sure it's actually a bool self.assertFalse(inst.cleaned) mock_get.assert_called_once_with(self.context, fake_uuid, - columns_to_join=['info_cache', 'security_groups']) + columns_to_join=['info_cache']) @mock.patch.object(db, 'instance_get_by_uuid') def test_get_cleaned(self, mock_get): @@ -818,7 +810,7 @@ class _TestInstanceObject(object): # NOTE(mikal): Make sure it's actually a bool self.assertTrue(inst.cleaned) mock_get.assert_called_once_with(self.context, fake_uuid, - columns_to_join=['info_cache', 'security_groups']) + columns_to_join=['info_cache']) @mock.patch.object(db, 'instance_update_and_get_original') @mock.patch.object(db, 'instance_info_cache_update') @@ -846,7 +838,7 @@ class _TestInstanceObject(object): inst.save() mock_get.assert_called_once_with(self.context, fake_uuid, - columns_to_join=['info_cache', 'security_groups']) + columns_to_join=['info_cache']) mock_upd_cache.assert_called_once_with(self.context, fake_uuid, {'network_info': nwinfo2_json}) self.assertFalse(mock_upd_and_get.called) @@ -900,22 +892,15 @@ class _TestInstanceObject(object): mock_upd_secgrp.return_value = fake_inst['security_groups'][0] inst = objects.Instance.get_by_uuid(self.context, fake_uuid) - self.assertEqual(2, len(inst.security_groups)) - for index, group in enumerate(fake_inst['security_groups']): - for key in group: - self.assertEqual(group[key], - getattr(inst.security_groups[index], key)) - self.assertIsInstance(inst.security_groups[index], - security_group.SecurityGroup) + # we no longer actually save these, so this should return 0 + self.assertEqual(0, len(inst.security_groups)) self.assertEqual(set(), inst.security_groups.obj_what_changed()) - inst.security_groups[0].description = 'changed' inst.save() self.assertEqual(set(), inst.security_groups.obj_what_changed()) mock_get.assert_called_once_with(self.context, fake_uuid, - columns_to_join=['info_cache', 'security_groups']) - mock_upd_secgrp.assert_called_once_with(self.context, 1, - {'description': 'changed'}) + columns_to_join=['info_cache']) + mock_upd_secgrp.assert_not_called() self.assertFalse(mock_upd_and_get.called) @mock.patch.object(db, 'instance_get_by_uuid') @@ -927,7 +912,7 @@ class _TestInstanceObject(object): inst = objects.Instance.get_by_uuid(self.context, fake_uuid) self.assertEqual(0, len(inst.security_groups)) mock_get.assert_called_once_with(self.context, fake_uuid, - columns_to_join=['info_cache', 'security_groups']) + columns_to_join=['info_cache']) @mock.patch.object(db, 'instance_get_by_uuid') def test_with_empty_pci_devices(self, mock_get): @@ -1201,23 +1186,16 @@ class _TestInstanceObject(object): fake_inst = fake_instance.fake_db_instance() mock_create.return_value = fake_inst - secgroups = security_group.SecurityGroupList() - secgroups.objects = [] - for name in ('foo', 'bar'): - secgroup = security_group.SecurityGroup() - secgroup.name = name - secgroups.objects.append(secgroup) info_cache = instance_info_cache.InstanceInfoCache() info_cache.network_info = network_model.NetworkInfo() inst = objects.Instance(context=self.context, - host='foo-host', security_groups=secgroups, + host='foo-host', info_cache=info_cache) inst.create() mock_create.assert_called_once_with(self.context, {'host': 'foo-host', 'deleted': 0, - 'security_groups': ['foo', 'bar'], 'info_cache': {'network_info': '[]'}, 'extra': { 'vcpu_model': None, @@ -1340,6 +1318,7 @@ class _TestInstanceObject(object): self.assertIsNone(inst.info_cache) def test_from_db_object_security_groups_net_set(self): + # this is the default now since we no longer set these things inst = instance.Instance(context=self.context, info_cache=None) db_inst = fake_instance.fake_db_instance() @@ -1601,20 +1580,6 @@ class _TestInstanceObject(object): mock_get.assert_called_once_with(self.context, inst) self.assertEqual(fake_ec2_ids, ec2_ids) - @mock.patch('nova.objects.SecurityGroupList.get_by_instance') - def test_load_security_groups(self, mock_get): - secgroups = [] - for name in ('foo', 'bar'): - secgroup = security_group.SecurityGroup() - secgroup.name = name - secgroups.append(secgroup) - fake_secgroups = security_group.SecurityGroupList(objects=secgroups) - mock_get.return_value = fake_secgroups - inst = objects.Instance(context=self.context, uuid=uuids.instance) - secgroups = inst.security_groups - mock_get.assert_called_once_with(self.context, inst) - self.assertEqual(fake_secgroups, secgroups) - @mock.patch('nova.objects.PciDeviceList.get_by_instance_uuid') def test_load_pci_devices(self, mock_get): fake_pci_devices = pci_device.PciDeviceList() @@ -1744,7 +1709,6 @@ class _TestInstanceListObject(object): db_inst['updated_at'] = None db_inst['launched_at'] = datetime.datetime(1955, 11, 12, 22, 4, 0) - db_inst['security_groups'] = [] db_inst['deleted'] = 0 db_inst['info_cache'] = dict(test_instance_info_cache.fake_info_cache, @@ -2010,49 +1974,6 @@ class _TestInstanceListObject(object): [x.uuid for x in insts], latest=True) - @mock.patch('nova.objects.instance.Instance.obj_make_compatible') - def test_get_by_security_group(self, mock_compat): - fake_secgroup = dict(test_security_group.fake_secgroup) - fake_secgroup['instances'] = [ - fake_instance.fake_db_instance(id=1, - system_metadata={'foo': 'bar'}), - fake_instance.fake_db_instance(id=2), - ] - - with mock.patch.object(db, 'security_group_get') as sgg: - sgg.return_value = fake_secgroup - secgroup = security_group.SecurityGroup() - secgroup.id = fake_secgroup['id'] - instances = instance.InstanceList.get_by_security_group( - self.context, secgroup) - - self.assertEqual(2, len(instances)) - self.assertEqual([1, 2], [x.id for x in instances]) - self.assertTrue(instances[0].obj_attr_is_set('system_metadata')) - self.assertEqual({'foo': 'bar'}, instances[0].system_metadata) - - def test_get_by_security_group_after_destroy(self): - db_sg = db.security_group_create( - self.context, - {'name': 'foo', - 'description': 'test group', - 'user_id': self.context.user_id, - 'project_id': self.context.project_id}) - self.assertFalse(db.security_group_in_use(self.context, db_sg.id)) - inst = objects.Instance( - context=self.context, - user_id=self.context.user_id, - project_id=self.context.project_id) - inst.create() - - db.instance_add_security_group(self.context, - inst.uuid, - db_sg.id) - - self.assertTrue(db.security_group_in_use(self.context, db_sg.id)) - inst.destroy() - self.assertFalse(db.security_group_in_use(self.context, db_sg.id)) - @mock.patch('nova.db.main.api.instance_get_all_uuids_by_hosts') def test_get_uuids_by_host_no_match(self, mock_get_all): mock_get_all.return_value = collections.defaultdict(list) @@ -2159,14 +2080,14 @@ class TestInstanceObjectMisc(test.NoDBTestCase): def test_expected_cols_no_duplicates(self): expected_attr = ['metadata', 'system_metadata', 'info_cache', - 'security_groups', 'info_cache', 'metadata', + 'info_cache', 'metadata', 'pci_devices', 'tags', 'extra', 'flavor'] result_list = instance._expected_cols(expected_attr) self.assertEqual(len(result_list), len(set(expected_attr))) self.assertEqual(['metadata', 'system_metadata', 'info_cache', - 'security_groups', 'pci_devices', 'tags', 'extra', + 'pci_devices', 'tags', 'extra', 'extra.flavor'], result_list)