From 9880a164d438d1ec1c1b514fca5aa1a1da2d855e Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Wed, 4 May 2016 14:13:36 -0700 Subject: [PATCH] Create Instances with keypairs This makes compute_api create Instance objects with keypairs set, so that they are persisted in the instance_extra table and thus available without hitting the API DB once we move keypairs up there. Related to blueprint cells-keypairs-api-db Change-Id: I8d055265c8900b7fff8a209bb48884c9a734ceaf --- nova/cells/scheduler.py | 3 ++ nova/compute/api.py | 28 +++++++----- nova/objects/instance.py | 2 +- nova/tests/unit/compute/test_compute_api.py | 44 +++++++++++++++++-- nova/tests/unit/compute/test_compute_cells.py | 2 +- 5 files changed, 63 insertions(+), 16 deletions(-) diff --git a/nova/cells/scheduler.py b/nova/cells/scheduler.py index 8e27498c30a4..61c24f55f4fc 100644 --- a/nova/cells/scheduler.py +++ b/nova/cells/scheduler.py @@ -84,6 +84,9 @@ class CellsScheduler(base.Base): # FIXME(danms): Same for ec2_ids instance_values.pop('ec2_ids', None) + # FIXME(danms): Same for keypairs + instance_values.pop('keypairs', None) + instances = [] num_instances = len(instance_uuids) security_groups = ( diff --git a/nova/compute/api.py b/nova/compute/api.py index b87f9e2d94c5..3e5493b99edd 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -842,6 +842,8 @@ class API(base.Base): context.user_id, key_name) key_data = key_pair.public_key + else: + key_pair = None root_device_name = block_device.prepend_dev( block_device.properties_root_device_name( @@ -908,7 +910,7 @@ class API(base.Base): # return the validated options and maximum number of instances allowed # by the network quotas - return base_options, max_network_count + return base_options, max_network_count, key_pair def _create_build_request(self, context, instance, base_options, request_spec, security_groups, num_instances, index): @@ -947,7 +949,8 @@ class API(base.Base): def _provision_instances(self, context, instance_type, min_count, max_count, base_options, boot_meta, security_groups, block_device_mapping, shutdown_terminate, - instance_group, check_server_group_quota, filter_properties): + instance_group, check_server_group_quota, filter_properties, + key_pair): # Reserve quotas num_instances, quotas = self._check_num_instances_quota( context, instance_type, min_count, max_count) @@ -973,6 +976,9 @@ class API(base.Base): instance = objects.Instance(context=context) instance.uuid = instance_uuid instance.update(base_options) + instance.keypairs = objects.KeyPairList(objects=[]) + if key_pair: + instance.keypairs.objects.append(key_pair) instance = self.create_db_entry_for_new_instance(context, instance_type, boot_meta, instance, security_groups, block_device_mapping, num_instances, i, @@ -1151,13 +1157,14 @@ class API(base.Base): self._check_auto_disk_config(image=boot_meta, auto_disk_config=auto_disk_config) - base_options, max_net_count = self._validate_and_build_base_options( - context, instance_type, boot_meta, image_href, image_id, - kernel_id, ramdisk_id, display_name, display_description, - key_name, key_data, security_groups, availability_zone, - user_data, metadata, access_ip_v4, access_ip_v6, - requested_networks, config_drive, auto_disk_config, - reservation_id, max_count) + base_options, max_net_count, key_pair = \ + self._validate_and_build_base_options( + context, instance_type, boot_meta, image_href, image_id, + kernel_id, ramdisk_id, display_name, display_description, + key_name, key_data, security_groups, availability_zone, + user_data, metadata, access_ip_v4, access_ip_v6, + requested_networks, config_drive, auto_disk_config, + reservation_id, max_count) # max_net_count is the maximum number of instances requested by the # user adjusted for any network quota constraints, including @@ -1187,7 +1194,8 @@ class API(base.Base): instances = self._provision_instances(context, instance_type, min_count, max_count, base_options, boot_meta, security_groups, block_device_mapping, shutdown_terminate, - instance_group, check_server_group_quota, filter_properties) + instance_group, check_server_group_quota, filter_properties, + key_pair) for instance in instances: self._record_action_start(context, instance, diff --git a/nova/objects/instance.py b/nova/objects/instance.py index 04cff186e1c8..0fa6f91b510b 100644 --- a/nova/objects/instance.py +++ b/nova/objects/instance.py @@ -466,7 +466,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject, } updates['extra']['flavor'] = jsonutils.dumps(flavor_info) keypairs = updates.pop('keypairs', None) - if keypairs: + if keypairs is not None: expected_attrs.append('keypairs') updates['extra']['keypairs'] = jsonutils.dumps( keypairs.obj_to_primitive()) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index cf14d0a58856..5fad5b418dbf 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -211,7 +211,7 @@ class _ComputeAPIUnitTestMixIn(object): mock.patch.object(self.compute_api, '_check_auto_disk_config'), mock.patch.object(self.compute_api, '_validate_and_build_base_options', - return_value=({}, max_net_count)) + return_value=({}, max_net_count, None)) ) as ( get_image, check_auto_disk_config, @@ -3088,7 +3088,7 @@ class _ComputeAPIUnitTestMixIn(object): min_count, max_count, base_options, boot_meta, security_groups, block_device_mapping, shutdown_terminate, instance_group, check_server_group_quota, - filter_properties) + filter_properties, None) self.assertTrue(uuidutils.is_uuid_like(instances[0].uuid)) mock_req_spec_from_components.assert_called_once_with(ctxt, @@ -3099,6 +3099,42 @@ class _ComputeAPIUnitTestMixIn(object): do_test() + @mock.patch('nova.objects.RequestSpec.from_components') + @mock.patch('nova.objects.Instance') + @mock.patch('nova.objects.InstanceMapping.create') + def test_provision_instances_with_keypair(self, mock_im, mock_instance, + mock_rs): + fake_keypair = objects.KeyPair(name='test') + + @mock.patch.object(self.compute_api, '_check_num_instances_quota') + @mock.patch.object(self.compute_api, 'security_group_api') + @mock.patch.object(self.compute_api, + 'create_db_entry_for_new_instance') + @mock.patch.object(self.compute_api, '_create_build_request') + @mock.patch.object(self.compute_api, '_create_block_device_mapping') + def do_test(mock_cbdm, mock_cbr, mock_cdb, mock_sg, mock_cniq): + mock_cniq.return_value = 1, mock.MagicMock() + self.compute_api._provision_instances(self.context, + mock.sentinel.flavor, + 1, 1, mock.MagicMock(), + {}, None, + None, None, None, {}, None, + fake_keypair) + self.assertEqual( + 'test', + mock_instance.return_value.keypairs.objects[0].name) + self.compute_api._provision_instances(self.context, + mock.sentinel.flavor, + 1, 1, mock.MagicMock(), + {}, None, + None, None, None, {}, None, + None) + self.assertEqual( + 0, + len(mock_instance.return_value.keypairs.objects)) + + do_test() + def test_provision_instances_creates_destroys_build_request(self): @mock.patch.object(self.compute_api, '_check_num_instances_quota') @mock.patch.object(objects, 'Instance') @@ -3169,7 +3205,7 @@ class _ComputeAPIUnitTestMixIn(object): min_count, max_count, base_options, boot_meta, security_groups, block_device_mapping, shutdown_terminate, instance_group, check_server_group_quota, - filter_properties) + filter_properties, None) for instance in instances: self.assertTrue(uuidutils.is_uuid_like(instance.uuid)) @@ -3288,7 +3324,7 @@ class _ComputeAPIUnitTestMixIn(object): min_count, max_count, base_options, boot_meta, security_groups, block_device_mapping, shutdown_terminate, instance_group, check_server_group_quota, - filter_properties) + filter_properties, None) self.assertTrue(uuidutils.is_uuid_like(instances[0].uuid)) self.assertEqual(instances[0].uuid, diff --git a/nova/tests/unit/compute/test_compute_cells.py b/nova/tests/unit/compute/test_compute_cells.py index 678cc1307568..d6591732c1a1 100644 --- a/nova/tests/unit/compute/test_compute_cells.py +++ b/nova/tests/unit/compute/test_compute_cells.py @@ -342,7 +342,7 @@ class CellsConductorAPIRPCRedirect(test.NoDBTestCase): _validate, _get_image, _check_bdm, _provision, _record_action_start): _get_image.return_value = (None, 'fake-image') - _validate.return_value = ({}, 1) + _validate.return_value = ({}, 1, None) _check_bdm.return_value = objects.BlockDeviceMappingList() _provision.return_value = 'instances'