From 371be3f4a9683429e63cc2c37d3e73208da2847e Mon Sep 17 00:00:00 2001 From: "Chung Chih, Hung" Date: Wed, 1 Jul 2015 10:54:00 +0000 Subject: [PATCH] Creating instance fail when inject ssh key in cells mode The conductor service of compute host will try to find ssh key in child cells when creating instance. But the ssh key was only stored at parent api cells. Therefore the conductor service will throw ssh key not found exception. There are three solutions: 1. add keypair_type into the instance object, along side keypair_name, etc 2. Sync ssh keypair to every child cell database 3. consider sending a message to the parent cell to fetch the keypair This commit prefer to use third solution. Because of the data ssh keypair should be global. And cells v2 split key_pairs table into global API tables. Change-Id: I156a1c3cf3e31f34cea5e240b14a9575e9e45239 Closes-Bug: #1443816 --- devstack/tempest-dsvm-cells-rc | 1 - nova/api/metadata/base.py | 14 +++- nova/cells/manager.py | 18 ++++- nova/cells/messaging.py | 21 ++++++ nova/cells/rpcapi.py | 13 ++++ nova/tests/unit/cells/test_cells_manager.py | 24 +++++++ nova/tests/unit/cells/test_cells_messaging.py | 42 +++++++++++ nova/tests/unit/cells/test_cells_rpcapi.py | 22 ++++++ nova/tests/unit/test_metadata.py | 72 +++++++++++++++++++ 9 files changed, 222 insertions(+), 5 deletions(-) diff --git a/devstack/tempest-dsvm-cells-rc b/devstack/tempest-dsvm-cells-rc index ac4e43a155..26e6757cb8 100644 --- a/devstack/tempest-dsvm-cells-rc +++ b/devstack/tempest-dsvm-cells-rc @@ -58,7 +58,6 @@ r="$r|(?:tempest\.api\.compute\.servers\.test_disk_config\.ServerDiskConfigTestJ r="$r|(?:tempest\.api\.compute\.servers\.test_create_server\.ServersTestJSON\.test_create_server_with_scheduler_hint_group*)" r="$r|(?:tempest\.api\.compute\.servers\.test_servers_negative\.ServersNegativeTestJSON\.test_shelve_shelved_server*)" r="$r|(?:tempest\.api\.compute\.servers\.test_create_server\.ServersTestManualDisk\.test_create_server_with_scheduler_hint_group*)" -r="$r|(?:tempest\.api\.compute\.servers\.test_servers\.ServersTestJSON\.test_create_specify_keypair*)" r="$r|(?:tempest\.api\.compute\.servers\.test_virtual_interfaces\.VirtualInterfacesTestJSON\.test_list_virtual_interfaces*)" r="$r|(?:tempest\.api\.compute\.test_networks\.NetworksTestJSON\.test_list_networks*)" r="$r|(?:tempest\.scenario\.test_minimum_basic\.TestMinimumBasicScenario\.test_minimum_basic_scenario*)" diff --git a/nova/api/metadata/base.py b/nova/api/metadata/base.py index e7e37788e5..c50cc5da40 100644 --- a/nova/api/metadata/base.py +++ b/nova/api/metadata/base.py @@ -31,6 +31,8 @@ from nova.api.ec2 import ec2utils from nova.api.metadata import password from nova import availability_zones as az from nova import block_device +from nova.cells import opts as cells_opts +from nova.cells import rpcapi as cells_rpcapi from nova import context from nova import network from nova import objects @@ -313,9 +315,15 @@ class InstanceMetadata(object): self.instance.key_name: self.instance.key_data } - keypair = keypair_obj.KeyPair.get_by_name( - context.get_admin_context(), self.instance.user_id, - self.instance.key_name) + if cells_opts.get_cell_type() == 'compute': + cells_api = cells_rpcapi.CellsAPI() + keypair = cells_api.get_keypair_at_top( + context.get_admin_context(), self.instance.user_id, + self.instance.key_name) + else: + keypair = keypair_obj.KeyPair.get_by_name( + context.get_admin_context(), self.instance.user_id, + self.instance.key_name) metadata['keys'] = [ {'name': keypair.name, 'type': keypair.type, diff --git a/nova/cells/manager.py b/nova/cells/manager.py index e2cbd0c69b..a257e1c5b2 100644 --- a/nova/cells/manager.py +++ b/nova/cells/manager.py @@ -76,7 +76,7 @@ class CellsManager(manager.Manager): Scheduling requests get passed to the scheduler class. """ - target = oslo_messaging.Target(version='1.36') + target = oslo_messaging.Target(version='1.37') def __init__(self, *args, **kwargs): LOG.warning(_LW('The cells feature of Nova is considered experimental ' @@ -583,3 +583,19 @@ class CellsManager(manager.Manager): def set_admin_password(self, ctxt, instance, new_pass): self.msg_runner.set_admin_password(ctxt, instance, new_pass) + + def get_keypair_at_top(self, ctxt, user_id, name): + responses = self.msg_runner.get_keypair_at_top(ctxt, user_id, name) + keypairs = [resp.value for resp in responses if resp.value is not None] + + if len(keypairs) == 0: + return None + elif len(keypairs) > 1: + cell_names = ', '.join([resp.cell_name for resp in responses + if resp.value is not None]) + LOG.warning(_LW("The same keypair name '%(name)s' exists in the " + "following cells: %(cell_names)s. The keypair " + "value from the first cell is returned."), + {'name': name, 'cell_names': cell_names}) + + return keypairs[0] diff --git a/nova/cells/messaging.py b/nova/cells/messaging.py index 62d3678b7f..14e9cfff45 100644 --- a/nova/cells/messaging.py +++ b/nova/cells/messaging.py @@ -1211,6 +1211,18 @@ class _BroadcastMessageMethods(_BaseMessageMethods): context = message.ctxt return self.compute_api.get_migrations(context, filters) + def get_keypair_at_top(self, message, user_id, name): + """Get keypair in API cells by name. Just return None if there is + no match keypair. + """ + if not self._at_the_top(): + return + + try: + return objects.KeyPair.get_by_name(message.ctxt, user_id, name) + except exception.KeypairNotFound: + pass + _CELL_MESSAGE_TYPE_TO_MESSAGE_CLS = {'targeted': _TargetedMessage, 'broadcast': _BroadcastMessage, @@ -1804,6 +1816,15 @@ class MessageRunner(object): self._instance_action(ctxt, instance, 'set_admin_password', extra_kwargs={'new_pass': new_pass}) + def get_keypair_at_top(self, ctxt, user_id, name): + """Get Key Pair by name at top level cell.""" + message = _BroadcastMessage(self, ctxt, + 'get_keypair_at_top', + dict(user_id=user_id, name=name), + 'up', + need_response=True, run_locally=False) + return message.process() + @staticmethod def get_message_types(): return _CELL_MESSAGE_TYPE_TO_MESSAGE_CLS.keys() diff --git a/nova/cells/rpcapi.py b/nova/cells/rpcapi.py index 16db040e89..9ef400960f 100644 --- a/nova/cells/rpcapi.py +++ b/nova/cells/rpcapi.py @@ -117,6 +117,7 @@ class CellsAPI(object): * 1.35 - Make instance_update_at_top, instance_destroy_at_top and instance_info_cache_update_at_top use instance objects * 1.36 - Added 'delete_type' parameter to terminate_instance() + * 1.37 - Add get_keypair_at_top to fetch keypair from api cell ''' VERSION_ALIASES = { @@ -691,3 +692,15 @@ class CellsAPI(object): cctxt = self.client.prepare(version='1.29') cctxt.cast(ctxt, 'set_admin_password', instance=instance, new_pass=new_pass) + + def get_keypair_at_top(self, ctxt, user_id, name): + if not CONF.cells.enable: + return + + cctxt = self.client.prepare(version='1.37') + keypair = cctxt.call(ctxt, 'get_keypair_at_top', user_id=user_id, + name=name) + if keypair is None: + raise exception.KeypairNotFound(user_id=user_id, + name=name) + return keypair diff --git a/nova/tests/unit/cells/test_cells_manager.py b/nova/tests/unit/cells/test_cells_manager.py index 17c2e6b8af..8f6f1a824a 100644 --- a/nova/tests/unit/cells/test_cells_manager.py +++ b/nova/tests/unit/cells/test_cells_manager.py @@ -880,3 +880,27 @@ class CellsManagerClassTestCase(test.NoDBTestCase): instance='fake-instance', new_pass='fake-password') set_admin_password.assert_called_once_with(self.ctxt, 'fake-instance', 'fake-password') + + def test_get_keypair_at_top(self): + keypairs = [self._get_fake_response('fake_keypair'), + self._get_fake_response('fake_keypair2')] + with mock.patch.object(self.msg_runner, + 'get_keypair_at_top', + return_value=keypairs) as fake_get_keypair: + response = self.cells_manager.get_keypair_at_top(self.ctxt, + 'fake_user_id', + 'fake_name') + fake_get_keypair.assert_called_once_with(self.ctxt, 'fake_user_id', + 'fake_name') + self.assertEqual('fake_keypair', response) + + def test_get_keypair_at_top_with_empty_responses(self): + with mock.patch.object(self.msg_runner, + 'get_keypair_at_top', + return_value=[]) as fake_get_keypair: + self.assertIsNone( + self.cells_manager.get_keypair_at_top(self.ctxt, + 'fake_user_id', + 'fake_name')) + fake_get_keypair.assert_called_once_with(self.ctxt, 'fake_user_id', + 'fake_name') diff --git a/nova/tests/unit/cells/test_cells_messaging.py b/nova/tests/unit/cells/test_cells_messaging.py index f2eb0ea29a..2ff5f21b35 100644 --- a/nova/tests/unit/cells/test_cells_messaging.py +++ b/nova/tests/unit/cells/test_cells_messaging.py @@ -2079,6 +2079,48 @@ class CellsBroadcastMethodsTestCase(test.TestCase): self.assertIn(response.value_or_raise(), [migrations_from_cell1, migrations_from_cell2]) + @mock.patch.object(objects.KeyPair, 'get_by_name', + return_value='fake_keypair') + def test_get_keypair_at_top(self, fake_get_by_name): + user_id = 'fake_user_id' + name = 'fake_keypair_name' + responses = self.src_msg_runner.get_keypair_at_top(self.ctxt, + user_id, name) + fake_get_by_name.assert_called_once_with(self.ctxt, user_id, name) + + for response in responses: + if response.value is not None: + self.assertEqual('fake_keypair', response.value) + + @mock.patch.object(objects.KeyPair, 'get_by_name') + def test_get_keypair_at_top_with_objects_exception(self, fake_get_by_name): + user_id = 'fake_user_id' + name = 'fake_keypair_name' + keypair_exception = exception.KeypairNotFound(user_id=user_id, + name=name) + fake_get_by_name.side_effect = keypair_exception + responses = self.src_msg_runner.get_keypair_at_top(self.ctxt, + user_id, + name) + fake_get_by_name.assert_called_once_with(self.ctxt, user_id, name) + + for response in responses: + self.assertIsNone(response.value) + + @mock.patch.object(messaging._BroadcastMessage, 'process') + def test_get_keypair_at_top_with_process_response(self, fake_process): + user_id = 'fake_user_id' + name = 'fake_keypair_name' + response = messaging.Response(self.ctxt, 'cell', 'keypair', False) + other_response = messaging.Response(self.ctxt, 'cell', + 'fake_other_keypair', False) + fake_process.return_value = [response, other_response] + + responses = self.src_msg_runner.get_keypair_at_top(self.ctxt, + user_id, name) + fake_process.assert_called_once_with() + self.assertEqual(fake_process.return_value, responses) + class CellsPublicInterfacesTestCase(test.TestCase): """Test case for the public interfaces into cells messaging.""" diff --git a/nova/tests/unit/cells/test_cells_rpcapi.py b/nova/tests/unit/cells/test_cells_rpcapi.py index cb503fd893..c2e1a5c6b5 100644 --- a/nova/tests/unit/cells/test_cells_rpcapi.py +++ b/nova/tests/unit/cells/test_cells_rpcapi.py @@ -760,3 +760,25 @@ class CellsAPITestCase(test.NoDBTestCase): 'new_pass': 'fake-password'} self._check_result(call_info, 'set_admin_password', expected_args, version='1.29') + + def test_get_keypair_at_top(self): + call_info = self._stub_rpc_method('call', 'fake_response') + result = self.cells_rpcapi.get_keypair_at_top(self.fake_context, + 'fake_user_id', 'fake_name') + + expected_args = {'user_id': 'fake_user_id', + 'name': 'fake_name'} + self._check_result(call_info, 'get_keypair_at_top', + expected_args, version='1.37') + self.assertEqual(result, 'fake_response') + + def test_get_keypair_at_top_with_not_found(self): + call_info = self._stub_rpc_method('call', None) + self.assertRaises(exception.KeypairNotFound, + self.cells_rpcapi.get_keypair_at_top, + self.fake_context, 'fake_user_id', 'fake_name') + + expected_args = {'user_id': 'fake_user_id', + 'name': 'fake_name'} + self._check_result(call_info, 'get_keypair_at_top', + expected_args, version='1.37') diff --git a/nova/tests/unit/test_metadata.py b/nova/tests/unit/test_metadata.py index 2dd1472322..69220963f9 100644 --- a/nova/tests/unit/test_metadata.py +++ b/nova/tests/unit/test_metadata.py @@ -57,6 +57,7 @@ CONF = cfg.CONF USER_DATA_STRING = (b"This is an encoded string") ENCODE_USER_DATA_STRING = base64.b64encode(USER_DATA_STRING) +FAKE_SEED = '7qtD24mpMR2' def fake_inst_obj(context): @@ -93,6 +94,12 @@ def fake_inst_obj(context): return inst +def fake_keypair_obj(name, data): + return objects.KeyPair(name=name, + type='fake_type', + public_key=data) + + def return_non_existing_address(*args, **kwarg): raise exception.NotFound() @@ -153,6 +160,8 @@ class MetadataTestCase(test.TestCase): self.context = context.RequestContext('fake', 'fake') self.instance = fake_inst_obj(self.context) self.flags(use_local=True, group='conductor') + self.keypair = fake_keypair_obj(self.instance.key_name, + self.instance.key_data) fake_network.stub_out_nw_api_get_instance_nw_info(self.stubs) def test_can_pickle_metadata(self): @@ -360,6 +369,69 @@ class MetadataTestCase(test.TestCase): data = md.get_ec2_metadata(version='2009-04-04') self.assertEqual(data['meta-data']['local-ipv4'], expected_local) + @mock.patch.object(base64, 'b64encode', lambda data: FAKE_SEED) + @mock.patch('nova.cells.rpcapi.CellsAPI.get_keypair_at_top') + @mock.patch.object(objects.KeyPair, 'get_by_name') + @mock.patch.object(jsonutils, 'dumps') + def _test_as_json_with_options(self, mock_json_dumps, + mock_keypair, mock_cells_keypair, + is_cells=False, os_version=base.GRIZZLY): + if is_cells: + self.flags(enable=True, group='cells') + self.flags(cell_type='compute', group='cells') + mock_keypair = mock_cells_keypair + + instance = self.instance + keypair = self.keypair + md = fake_InstanceMetadata(self.stubs, instance) + + expected_metadata = { + 'uuid': md.uuid, + 'hostname': md._get_hostname(), + 'name': instance.display_name, + 'launch_index': instance.launch_index, + 'availability_zone': md.availability_zone, + } + if md.launch_metadata: + expected_metadata['meta'] = md.launch_metadata + if md.files: + expected_metadata['files'] = md.files + if md.extra_md: + expected_metadata['extra_md'] = md.extra_md + if md.network_config: + expected_metadata['network_config'] = md.network_config + if instance.key_name: + expected_metadata['public_keys'] = { + keypair.name: keypair.public_key + } + expected_metadata['keys'] = [{'type': keypair.type, + 'data': keypair.public_key, + 'name': keypair.name}] + if md._check_os_version(base.GRIZZLY, os_version): + expected_metadata['random_seed'] = FAKE_SEED + if md._check_os_version(base.LIBERTY, os_version): + expected_metadata['project_id'] = instance.project_id + + mock_keypair.return_value = keypair + md._metadata_as_json(os_version, 'non useless path parameter') + if instance.key_name: + mock_keypair.assert_called_once_with(mock.ANY, + instance.user_id, + instance.key_name) + self.assertIsInstance(mock_keypair.call_args[0][0], + context.RequestContext) + self.assertEqual(md.md_mimetype, base.MIME_TYPE_APPLICATION_JSON) + mock_json_dumps.assert_called_once_with(expected_metadata) + + def test_as_json(self): + for os_version in base.OPENSTACK_VERSIONS: + self._test_as_json_with_options(os_version=os_version) + + def test_as_json_with_cells_mode(self): + for os_version in base.OPENSTACK_VERSIONS: + self._test_as_json_with_options(is_cells=True, + os_version=os_version) + class OpenStackMetadataTestCase(test.TestCase): def setUp(self):