From 4f86f726ab473dcd8346c3d1c9bcb0588cda5708 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Mon, 22 Jul 2013 14:43:31 -0700 Subject: [PATCH] Make compute_api use Instance.create() This makes compute_api use Instance.create() instead of the db.instance_create() API. Related to blueprint compute-api-objects Change-Id: I2ca1232fd9e12ca2fb57cd2acaa27ebd8aaa6dbe --- nova/cells/scheduler.py | 11 ++- nova/compute/api.py | 69 +++++++++++-------- nova/network/security_group/neutron_driver.py | 4 +- nova/objects/instance.py | 2 +- nova/objects/security_group.py | 16 +++++ .../compute/contrib/test_disk_config.py | 30 ++++---- .../plugins/v3/test_availability_zone.py | 5 +- .../compute/plugins/v3/test_config_drive.py | 5 +- .../compute/plugins/v3/test_disk_config.py | 30 ++++---- .../plugins/v3/test_scheduler_hints.py | 5 +- .../compute/plugins/v3/test_servers.py | 6 +- .../compute/plugins/v3/test_user_data.py | 5 +- .../api/openstack/compute/test_servers.py | 8 ++- nova/tests/compute/test_compute.py | 6 +- 14 files changed, 119 insertions(+), 83 deletions(-) diff --git a/nova/cells/scheduler.py b/nova/cells/scheduler.py index 6f3f3bf7de57..54c978e60fbd 100644 --- a/nova/cells/scheduler.py +++ b/nova/cells/scheduler.py @@ -31,6 +31,8 @@ from nova.compute import vm_states from nova import conductor from nova.db import base from nova import exception +from nova.objects import instance as instance_obj +from nova.objects import security_group from nova.openstack.common.gettextutils import _ from nova.openstack.common import log as logging from nova.scheduler import rpcapi as scheduler_rpcapi @@ -92,15 +94,20 @@ class CellsScheduler(base.Base): sys_metadata = flavors.save_flavor_info(sys_metadata, instance_type) instance_values['system_metadata'] = sys_metadata instance_values.pop('name') + if 'security_groups' in instance_values: + instance_values = security_group.make_secgroup_list( + instance_values['security_groups']) num_instances = len(instance_uuids) for i, instance_uuid in enumerate(instance_uuids): - instance_values['uuid'] = instance_uuid + instance = instance_obj.Instance() + instance.update(instance_values) + instance.uuid = instance_uuid instance = self.compute_api.create_db_entry_for_new_instance( ctxt, instance_type, image, - instance_values, + instance, security_groups, block_device_mapping, num_instances, i) diff --git a/nova/compute/api.py b/nova/compute/api.py index b9919203c408..dc73b9fd4ec0 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -47,11 +47,14 @@ from nova import exception from nova import hooks from nova.image import glance from nova import network +from nova.network import model as network_model from nova.network.security_group import openstack_driver from nova.network.security_group import security_group_base from nova import notifications from nova.objects import base as obj_base from nova.objects import instance as instance_obj +from nova.objects import instance_info_cache +from nova.objects import security_group from nova.openstack.common import excutils from nova.openstack.common.gettextutils import _ from nova.openstack.common import jsonutils @@ -531,11 +534,10 @@ class API(base.Base): LOG.exception(_('Failed to set instance name using ' 'multi_instance_display_name_template.')) new_name = instance['display_name'] - updates = {'display_name': new_name} - if not instance.get('hostname'): - updates['hostname'] = utils.sanitize_hostname(new_name) - instance = self.db.instance_update(context, - instance['uuid'], updates) + instance.display_name = new_name + if not instance.get('hostname', None): + instance.hostname = utils.sanitize_hostname(new_name) + instance.save() return instance def _check_config_drive(self, config_drive): @@ -680,7 +682,7 @@ class API(base.Base): 'key_name': key_name, 'key_data': key_data, 'locked': False, - 'metadata': metadata, + 'metadata': metadata or {}, 'access_ip_v4': access_ip_v4, 'access_ip_v6': access_ip_v6, 'availability_zone': availability_zone, @@ -717,9 +719,10 @@ class API(base.Base): instances = [] try: for i in xrange(num_instances): - options = base_options.copy() + instance = instance_obj.Instance() + instance.update(base_options) instance = self.create_db_entry_for_new_instance( - context, instance_type, boot_meta, options, + context, instance_type, boot_meta, instance, security_groups, block_device_mapping, num_instances, i) @@ -976,16 +979,19 @@ class API(base.Base): if (block_device_mapping or image_properties.get('mappings') or image_properties.get('block_device_mapping')): - instance['shutdown_terminate'] = False + instance.shutdown_terminate = False def _populate_instance_names(self, instance, num_instances): """Populate instance display_name and hostname.""" display_name = instance.get('display_name') - hostname = instance.get('hostname') + if instance.obj_attr_is_set('hostname'): + hostname = instance.get('hostname') + else: + hostname = None if display_name is None: display_name = self._default_display_name(instance['uuid']) - instance['display_name'] = display_name + instance.display_name = display_name if hostname is None and num_instances == 1: # NOTE(russellb) In the multi-instance case, we're going to @@ -996,32 +1002,32 @@ class API(base.Base): # Otherwise, it will be built after the template based # display_name. hostname = display_name - instance['hostname'] = utils.sanitize_hostname(hostname) + instance.hostname = utils.sanitize_hostname(hostname) def _default_display_name(self, instance_uuid): return "Server %s" % instance_uuid - def _populate_instance_for_create(self, base_options, image, - security_groups): + def _populate_instance_for_create(self, instance, image, security_groups): """Build the beginning of a new instance.""" image_properties = image.get('properties', {}) - instance = base_options - if not instance.get('uuid'): + if not instance.obj_attr_is_set('uuid'): # Generate the instance_uuid here so we can use it # for additional setup before creating the DB entry. instance['uuid'] = str(uuid.uuid4()) - instance['launch_index'] = 0 - instance['vm_state'] = vm_states.BUILDING - instance['task_state'] = task_states.SCHEDULING - instance['info_cache'] = {'network_info': '[]'} + instance.launch_index = 0 + instance.vm_state = vm_states.BUILDING + instance.task_state = task_states.SCHEDULING + info_cache = instance_info_cache.InstanceInfoCache() + info_cache.instance_uuid = instance.uuid + info_cache.network_info = network_model.NetworkInfo() + instance.info_cache = info_cache # Store image properties so we can use them later # (for notifications, etc). Only store what we can. - instance.setdefault('system_metadata', {}) - # Make sure we have the dict form that we need for instance_update. - instance['system_metadata'] = utils.instance_sys_meta(instance) + if not instance.obj_attr_is_set('system_metadata'): + instance.system_metadata = {} prefix_format = SM_IMAGE_PROP_PREFIX + '%s' for key, value in image_properties.iteritems(): new_value = unicode(value)[:255] @@ -1033,9 +1039,9 @@ class API(base.Base): if not base_image_ref: # base image ref property not previously set through a snapshot. # default to using the image ref as the base: - base_image_ref = base_options['image_ref'] + base_image_ref = instance.image_ref - instance['system_metadata']['image_base_image_ref'] = base_image_ref + instance.system_metadata['image_base_image_ref'] = base_image_ref self.security_group_api.populate_security_groups(instance, security_groups) return instance @@ -1043,7 +1049,7 @@ class API(base.Base): #NOTE(bcwaldon): No policy check since this is only used by scheduler and # the compute api. That should probably be cleaned up, though. def create_db_entry_for_new_instance(self, context, instance_type, image, - base_options, security_group, block_device_mapping, num_instances, + instance, security_group, block_device_mapping, num_instances, index): """Create an entry in the DB for this new instance, including any related table updates (such as security group, @@ -1052,8 +1058,7 @@ class API(base.Base): This is called by the scheduler after a location for the instance has been determined. """ - instance = self._populate_instance_for_create(base_options, - image, security_group) + self._populate_instance_for_create(instance, image, security_group) self._populate_instance_names(instance, num_instances) @@ -1061,7 +1066,7 @@ class API(base.Base): block_device_mapping) self.security_group_api.ensure_default(context) - instance = self.db.instance_create(context, instance) + instance.create(context) if num_instances > 1: # NOTE(russellb) We wait until this spot to handle @@ -3567,4 +3572,8 @@ class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase): return [{'name': group['name']} for group in groups] def populate_security_groups(self, instance, security_groups): - instance['security_groups'] = security_groups + if not security_groups: + instance.security_groups = None + return + instance.security_groups = security_group.make_secgroup_list( + security_groups) diff --git a/nova/network/security_group/neutron_driver.py b/nova/network/security_group/neutron_driver.py index f9b2db47c2a6..a4a62a8cb447 100644 --- a/nova/network/security_group/neutron_driver.py +++ b/nova/network/security_group/neutron_driver.py @@ -28,6 +28,7 @@ from nova.compute import api as compute_api from nova import exception from nova.network import neutronv2 from nova.network.security_group import security_group_base +from nova.objects import security_group from nova.openstack.common import excutils from nova.openstack.common.gettextutils import _ from nova.openstack.common import log as logging @@ -460,4 +461,5 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase): def populate_security_groups(self, instance, security_groups): # Setting to emply list since we do not want to populate this field # in the nova database if using the neutron driver - instance['security_groups'] = [] + instance['security_groups'] = security_group.SecurityGroupList() + instance['security_groups'].objects = [] diff --git a/nova/objects/instance.py b/nova/objects/instance.py index 61345a5437af..74052483a9a2 100644 --- a/nova/objects/instance.py +++ b/nova/objects/instance.py @@ -290,7 +290,7 @@ class Instance(base.NovaObject): @base.remotable def create(self, context): - if hasattr(self, base.get_attrname('id')): + if self.obj_attr_is_set('id'): raise exception.ObjectActionError(action='create', reason='already created') updates = {} diff --git a/nova/objects/security_group.py b/nova/objects/security_group.py index ccf4f027ec4f..223bd511dceb 100644 --- a/nova/objects/security_group.py +++ b/nova/objects/security_group.py @@ -93,3 +93,19 @@ class SecurityGroupList(base.ObjectListBase, base.NovaObject): return _make_secgroup_list(context, cls(), db.security_group_get_by_instance( context, instance.uuid)) + + +def make_secgroup_list(security_groups): + """A helper to make security group objects from a list of names. + + Note that this does not make them save-able or have the rest of the + attributes they would normally have, but provides a quick way to fill, + for example, an instance object during create. + """ + secgroups = SecurityGroupList() + secgroups.objects = [] + for name in security_groups: + secgroup = SecurityGroup() + secgroup.name = name + secgroups.objects.append(secgroup) + return secgroups diff --git a/nova/tests/api/openstack/compute/contrib/test_disk_config.py b/nova/tests/api/openstack/compute/contrib/test_disk_config.py index b89efbaa9d57..d8f32b2fc6cd 100644 --- a/nova/tests/api/openstack/compute/contrib/test_disk_config.py +++ b/nova/tests/api/openstack/compute/contrib/test_disk_config.py @@ -23,6 +23,7 @@ from nova.openstack.common import jsonutils import nova.openstack.common.rpc from nova import test from nova.tests.api.openstack import fakes +from nova.tests import fake_instance import nova.tests.image.fake @@ -82,23 +83,18 @@ class DiskConfigTestCase(test.TestCase): fake_instance_get_all) def fake_instance_create(context, inst_, session=None): - class FakeModel(dict): - def save(self, session=None): - pass - - inst = FakeModel(**inst_) - inst['id'] = 1 - inst['uuid'] = AUTO_INSTANCE_UUID - inst['created_at'] = datetime.datetime(2010, 10, 10, 12, 0, 0) - inst['updated_at'] = datetime.datetime(2010, 10, 10, 12, 0, 0) - inst['progress'] = 0 - inst['name'] = 'instance-1' # this is a property - inst['task_state'] = '' - inst['vm_state'] = '' - # NOTE(vish): db create translates security groups into model - # objects. Translate here so tests pass - inst['security_groups'] = [{'name': group} - for group in inst['security_groups']] + inst = fake_instance.fake_db_instance(**{ + 'id': 1, + 'uuid': AUTO_INSTANCE_UUID, + 'created_at': datetime.datetime(2010, 10, 10, 12, 0, 0), + 'updated_at': datetime.datetime(2010, 10, 10, 12, 0, 0), + 'progress': 0, + 'name': 'instance-1', # this is a property + 'task_state': '', + 'vm_state': '', + 'auto_disk_config': inst_['auto_disk_config'], + 'security_groups': inst_['security_groups'], + }) def fake_instance_get_for_create(context, id_, *args, **kwargs): return (inst, inst) diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_availability_zone.py b/nova/tests/api/openstack/compute/plugins/v3/test_availability_zone.py index 2c789290e5cc..f19713180abc 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_availability_zone.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_availability_zone.py @@ -32,6 +32,7 @@ from nova.openstack.common import rpc from nova import servicegroup from nova import test from nova.tests.api.openstack import fakes +from nova.tests import fake_instance from nova.tests.image import fake CONF = cfg.CONF @@ -307,7 +308,7 @@ class ServersControllerCreateTest(test.TestCase): image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' def_image_ref = 'http://localhost/images/%s' % image_uuid self.instance_cache_num += 1 - instance = { + instance = fake_instance.fake_db_instance(**{ 'id': self.instance_cache_num, 'display_name': inst['display_name'] or 'test', 'uuid': FAKE_UUID, @@ -325,7 +326,7 @@ class ServersControllerCreateTest(test.TestCase): "fixed_ips": [], "task_state": "", "vm_state": "", - } + }) self.instance_cache_by_id[instance['id']] = instance self.instance_cache_by_uuid[instance['uuid']] = instance diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_config_drive.py b/nova/tests/api/openstack/compute/plugins/v3/test_config_drive.py index 6aeee4043e5c..f5160d89bfaf 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_config_drive.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_config_drive.py @@ -30,6 +30,7 @@ from nova.openstack.common import jsonutils from nova.openstack.common import rpc from nova import test from nova.tests.api.openstack import fakes +from nova.tests import fake_instance from nova.tests.image import fake @@ -107,7 +108,7 @@ class ServersControllerCreateTest(test.TestCase): image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' def_image_ref = 'http://localhost/images/%s' % image_uuid self.instance_cache_num += 1 - instance = { + instance = fake_instance.fake_db_instance(**{ 'id': self.instance_cache_num, 'display_name': inst['display_name'] or 'test', 'uuid': FAKE_UUID, @@ -125,7 +126,7 @@ class ServersControllerCreateTest(test.TestCase): "fixed_ips": [], "task_state": "", "vm_state": "", - } + }) self.instance_cache_by_id[instance['id']] = instance self.instance_cache_by_uuid[instance['uuid']] = instance diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_disk_config.py b/nova/tests/api/openstack/compute/plugins/v3/test_disk_config.py index b89efbaa9d57..d8f32b2fc6cd 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_disk_config.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_disk_config.py @@ -23,6 +23,7 @@ from nova.openstack.common import jsonutils import nova.openstack.common.rpc from nova import test from nova.tests.api.openstack import fakes +from nova.tests import fake_instance import nova.tests.image.fake @@ -82,23 +83,18 @@ class DiskConfigTestCase(test.TestCase): fake_instance_get_all) def fake_instance_create(context, inst_, session=None): - class FakeModel(dict): - def save(self, session=None): - pass - - inst = FakeModel(**inst_) - inst['id'] = 1 - inst['uuid'] = AUTO_INSTANCE_UUID - inst['created_at'] = datetime.datetime(2010, 10, 10, 12, 0, 0) - inst['updated_at'] = datetime.datetime(2010, 10, 10, 12, 0, 0) - inst['progress'] = 0 - inst['name'] = 'instance-1' # this is a property - inst['task_state'] = '' - inst['vm_state'] = '' - # NOTE(vish): db create translates security groups into model - # objects. Translate here so tests pass - inst['security_groups'] = [{'name': group} - for group in inst['security_groups']] + inst = fake_instance.fake_db_instance(**{ + 'id': 1, + 'uuid': AUTO_INSTANCE_UUID, + 'created_at': datetime.datetime(2010, 10, 10, 12, 0, 0), + 'updated_at': datetime.datetime(2010, 10, 10, 12, 0, 0), + 'progress': 0, + 'name': 'instance-1', # this is a property + 'task_state': '', + 'vm_state': '', + 'auto_disk_config': inst_['auto_disk_config'], + 'security_groups': inst_['security_groups'], + }) def fake_instance_get_for_create(context, id_, *args, **kwargs): return (inst, inst) diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_scheduler_hints.py b/nova/tests/api/openstack/compute/plugins/v3/test_scheduler_hints.py index 83e8e4547f62..ce4f955689ff 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_scheduler_hints.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_scheduler_hints.py @@ -32,6 +32,7 @@ from nova.openstack.common import jsonutils from nova.openstack.common import rpc from nova import test from nova.tests.api.openstack import fakes +from nova.tests import fake_instance from nova.tests.image import fake @@ -142,7 +143,7 @@ class ServersControllerCreateTest(test.TestCase): image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' def_image_ref = 'http://localhost/images/%s' % image_uuid self.instance_cache_num += 1 - instance = { + instance = fake_instance.fake_db_instance(**{ 'id': self.instance_cache_num, 'display_name': inst['display_name'] or 'test', 'uuid': FAKE_UUID, @@ -160,7 +161,7 @@ class ServersControllerCreateTest(test.TestCase): "fixed_ips": [], "task_state": "", "vm_state": "", - } + }) self.instance_cache_by_id[instance['id']] = instance self.instance_cache_by_uuid[instance['uuid']] = instance diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_servers.py b/nova/tests/api/openstack/compute/plugins/v3/test_servers.py index f548754d2971..330b83825c6c 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_servers.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_servers.py @@ -55,6 +55,7 @@ from nova.openstack.common import rpc from nova import policy from nova import test from nova.tests.api.openstack import fakes +from nova.tests import fake_instance from nova.tests import fake_network from nova.tests.image import fake from nova.tests import matchers @@ -1711,7 +1712,7 @@ class ServersControllerCreateTest(test.TestCase): image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' def_image_ref = 'http://localhost/images/%s' % image_uuid self.instance_cache_num += 1 - instance = { + instance = fake_instance.fake_db_instance(**{ 'id': self.instance_cache_num, 'display_name': inst['display_name'] or 'test', 'uuid': FAKE_UUID, @@ -1729,7 +1730,8 @@ class ServersControllerCreateTest(test.TestCase): "fixed_ips": [], "task_state": "", "vm_state": "", - } + "security_groups": inst['security_groups'], + }) self.instance_cache_by_id[instance['id']] = instance self.instance_cache_by_uuid[instance['uuid']] = instance diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_user_data.py b/nova/tests/api/openstack/compute/plugins/v3/test_user_data.py index 0143316f6dc9..f32c85293d0c 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_user_data.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_user_data.py @@ -31,6 +31,7 @@ from nova.openstack.common import jsonutils from nova.openstack.common import rpc from nova import test from nova.tests.api.openstack import fakes +from nova.tests import fake_instance from nova.tests.image import fake @@ -70,7 +71,7 @@ class ServersControllerCreateTest(test.TestCase): image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' def_image_ref = 'http://localhost/images/%s' % image_uuid self.instance_cache_num += 1 - instance = { + instance = fake_instance.fake_db_instance(**{ 'id': self.instance_cache_num, 'display_name': inst['display_name'] or 'test', 'uuid': FAKE_UUID, @@ -88,7 +89,7 @@ class ServersControllerCreateTest(test.TestCase): "fixed_ips": [], "task_state": "", "vm_state": "", - } + }) self.instance_cache_by_id[instance['id']] = instance self.instance_cache_by_uuid[instance['uuid']] = instance diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py index cb6db81a1642..a7a3ef71b78d 100644 --- a/nova/tests/api/openstack/compute/test_servers.py +++ b/nova/tests/api/openstack/compute/test_servers.py @@ -54,6 +54,7 @@ from nova.openstack.common import rpc from nova import policy from nova import test from nova.tests.api.openstack import fakes +from nova.tests import fake_instance from nova.tests import fake_network from nova.tests.image import fake from nova.tests import matchers @@ -1614,7 +1615,7 @@ class ServersControllerCreateTest(test.TestCase): image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' def_image_ref = 'http://localhost/images/%s' % image_uuid self.instance_cache_num += 1 - instance = { + instance = fake_instance.fake_db_instance(**{ 'id': self.instance_cache_num, 'display_name': inst['display_name'] or 'test', 'uuid': FAKE_UUID, @@ -1633,7 +1634,8 @@ class ServersControllerCreateTest(test.TestCase): "task_state": "", "vm_state": "", "root_device_name": inst.get('root_device_name', 'vda'), - } + "security_groups": inst['security_groups'], + }) self.instance_cache_by_id[instance['id']] = instance self.instance_cache_by_uuid[instance['uuid']] = instance @@ -1662,7 +1664,7 @@ class ServersControllerCreateTest(test.TestCase): request_spec['instance_properties'])) return instances - def server_update(context, instance_uuid, params): + def server_update(context, instance_uuid, params, update_cells=False): inst = self.instance_cache_by_uuid[instance_uuid] inst.update(params) return (inst, inst) diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 6d099eb652cc..10532b9541a2 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -5479,11 +5479,13 @@ class ComputeAPITestCase(BaseTestCase): base_options = {'image_ref': self.fake_image['id'], 'system_metadata': utils.dict_to_metadata( {'fake': 'value'})} + instance = instance_obj.Instance() + instance.update(base_options) instance = self.compute_api._populate_instance_for_create( - base_options, + instance, self.fake_image, security_groups=None) - self.assertEquals(base_options['image_ref'], + self.assertEquals(str(base_options['image_ref']), instance['system_metadata']['image_base_image_ref']) self.assertEquals(vm_states.BUILDING, instance['vm_state']) self.assertEquals(task_states.SCHEDULING, instance['task_state'])