From 5c2824d8e184a3ff63a52e7c7cca7b4e6f0c0222 Mon Sep 17 00:00:00 2001 From: Emma Foley Date: Fri, 16 Feb 2018 15:22:12 +0000 Subject: Use context name instead of key_uuid context.key_uuid is only used as a suffix for the key files. There is already a unique ID associated with a the context; the context name includes a task ID, which is a UUID. This patch sets context.key_uuid to to context.name instead of generating a separate UUID As a result, get_short_key_uuid() is not needed. JIRA: YARDSTICK-1028 Change-Id: Id175061d6cfe23a068bb3d12ce176c1f176e8236 Signed-off-by: Emma Foley --- yardstick/benchmark/contexts/heat.py | 25 ++++++++------ yardstick/orchestrator/heat.py | 10 ++---- .../tests/unit/benchmark/contexts/test_heat.py | 40 +++++++++++----------- 3 files changed, 36 insertions(+), 39 deletions(-) diff --git a/yardstick/benchmark/contexts/heat.py b/yardstick/benchmark/contexts/heat.py index 8dd7d85e6..be8815060 100644 --- a/yardstick/benchmark/contexts/heat.py +++ b/yardstick/benchmark/contexts/heat.py @@ -13,7 +13,6 @@ from __future__ import print_function import collections import logging import os -import uuid import errno from collections import OrderedDict @@ -27,7 +26,7 @@ from yardstick.benchmark.contexts.model import Server from yardstick.benchmark.contexts.model import update_scheduler_hints from yardstick.common import exceptions as y_exc from yardstick.common.openstack_utils import get_neutron_client -from yardstick.orchestrator.heat import HeatTemplate, get_short_key_uuid +from yardstick.orchestrator.heat import HeatTemplate from yardstick.common import constants as consts from yardstick.common.utils import source_env from yardstick.ssh import SSH @@ -68,13 +67,8 @@ class HeatContext(Context): self.template_file = None self.heat_parameters = None self.neutron_client = None - # generate an uuid to identify yardstick_key - # the first 8 digits of the uuid will be used - self.key_uuid = uuid.uuid4() self.heat_timeout = None - self.key_filename = ''.join( - [consts.YARDSTICK_ROOT_PATH, 'yardstick/resources/files/yardstick_key-', - get_short_key_uuid(self.key_uuid)]) + self.key_filename = None super(HeatContext, self).__init__() @staticmethod @@ -137,7 +131,16 @@ class HeatContext(Context): self._server_map[server.dn] = server self.attrs = attrs - SSH.gen_keys(self.key_filename) + + self.key_filename = ''.join( + [consts.YARDSTICK_ROOT_PATH, + 'yardstick/resources/files/yardstick_key-', + self.name]) + # Permissions may have changed since creation; this can be fixed. If we + # overwrite the file, we lose future access to VMs using this key. + # As long as the file exists, even if it is unreadable, keep it intact + if not os.path.exists(self.key_filename): + SSH.gen_keys(self.key_filename) def check_environment(self): try: @@ -176,7 +179,7 @@ class HeatContext(Context): template.add_flavor(**self.flavor) self.flavors.add(flavor) - template.add_keypair(self.keypair_name, self.key_uuid) + template.add_keypair(self.keypair_name, self.name) template.add_security_group(self.secgroup_name) for network in self.networks.values(): @@ -438,7 +441,7 @@ class HeatContext(Context): pkey = pkg_resources.resource_string( 'yardstick.resources', - h_join('files/yardstick_key', get_short_key_uuid(self.key_uuid))).decode('utf-8') + h_join('files/yardstick_key', self.name)).decode('utf-8') result = { "user": server.context.user, diff --git a/yardstick/orchestrator/heat.py b/yardstick/orchestrator/heat.py index 558b5d2a6..403c7e8d9 100644 --- a/yardstick/orchestrator/heat.py +++ b/yardstick/orchestrator/heat.py @@ -30,17 +30,11 @@ from yardstick.common import template_format log = logging.getLogger(__name__) -HEAT_KEY_UUID_LENGTH = 8 - PROVIDER_SRIOV = "sriov" _DEPLOYED_STACKS = {} -def get_short_key_uuid(uuid): - return str(uuid)[:HEAT_KEY_UUID_LENGTH] - - class HeatStack(object): """Represents a Heat stack (deployed template) """ @@ -413,7 +407,7 @@ name (i.e. %s). } } - def add_keypair(self, name, key_uuid): + def add_keypair(self, name, key_id): """add to the template a Nova KeyPair""" log.debug("adding Nova::KeyPair '%s'", name) self.resources[name] = { @@ -425,7 +419,7 @@ name (i.e. %s). pkg_resources.resource_string( 'yardstick.resources', 'files/yardstick_key-' + - get_short_key_uuid(key_uuid) + '.pub'), + key_id + '.pub'), 'utf-8') } } diff --git a/yardstick/tests/unit/benchmark/contexts/test_heat.py b/yardstick/tests/unit/benchmark/contexts/test_heat.py index 2f2d4643e..1e31abce8 100644 --- a/yardstick/tests/unit/benchmark/contexts/test_heat.py +++ b/yardstick/tests/unit/benchmark/contexts/test_heat.py @@ -12,8 +12,6 @@ from collections import OrderedDict from itertools import count import logging -import os -import uuid import mock import unittest @@ -23,6 +21,7 @@ from yardstick.benchmark.contexts import heat from yardstick.benchmark.contexts import model from yardstick.common import exceptions as y_exc from yardstick.orchestrator import heat as orch_heat +from yardstick import ssh LOG = logging.getLogger(__name__) @@ -61,14 +60,14 @@ class HeatContextTestCase(unittest.TestCase): self.assertIsNone(self.test_context._user) self.assertIsNone(self.test_context.template_file) self.assertIsNone(self.test_context.heat_parameters) - self.assertIsNotNone(self.test_context.key_uuid) - self.assertIsNotNone(self.test_context.key_filename) + self.assertIsNone(self.test_context.key_filename) + @mock.patch.object(ssh.SSH, 'gen_keys') @mock.patch('yardstick.benchmark.contexts.heat.PlacementGroup') @mock.patch('yardstick.benchmark.contexts.heat.ServerGroup') @mock.patch('yardstick.benchmark.contexts.heat.Network') @mock.patch('yardstick.benchmark.contexts.heat.Server') - def test_init(self, mock_server, mock_network, mock_sg, mock_pg): + def test_init(self, mock_server, mock_network, mock_sg, mock_pg, mock_ssh_gen_keys): pgs = {'pgrp1': {'policy': 'availability'}} sgs = {'servergroup1': {'policy': 'affinity'}} @@ -106,13 +105,7 @@ class HeatContextTestCase(unittest.TestCase): servers['baz']) self.assertEqual(len(self.test_context.servers), 1) - if os.path.exists(self.test_context.key_filename): - try: - os.remove(self.test_context.key_filename) - os.remove(self.test_context.key_filename + ".pub") - except OSError: - LOG.exception("key_filename: %s", - self.test_context.key_filename) + mock_ssh_gen_keys.assert_called() def test_init_no_name_or_task_id(self): attrs = {} @@ -145,6 +138,7 @@ class HeatContextTestCase(unittest.TestCase): self.test_context.key_uuid = "2f2e4997-0a8e-4eb7-9fa4-f3f8fbbc393b" netattrs = {'cidr': '10.0.0.0/24', 'provider': None, 'external_network': 'ext_net'} + self.test_context.networks = OrderedDict( {"mynet": model.Network("mynet", self.test_context, netattrs)}) @@ -152,7 +146,7 @@ class HeatContextTestCase(unittest.TestCase): self.test_context._add_resources_to_template(mock_template) mock_template.add_keypair.assert_called_with( "ctx-key", - "2f2e4997-0a8e-4eb7-9fa4-f3f8fbbc393b") + "ctx-12345678") mock_template.add_security_group.assert_called_with("ctx-secgroup") mock_template.add_network.assert_called_with( "ctx-12345678-mynet", 'physnet1', None, None, None, None) @@ -294,13 +288,16 @@ class HeatContextTestCase(unittest.TestCase): self.assertEqual(len(server.interfaces), 3) self.assertDictEqual(server.interfaces['port_a'], expected) + @mock.patch('yardstick.benchmark.contexts.heat.os') @mock.patch('yardstick.benchmark.contexts.heat.HeatTemplate') - def test_undeploy(self, mock_template): + def test_undeploy(self, mock_template, *args): self.test_context.stack = mock_template self.test_context._name = 'foo' self.test_context._task_id = '1234567890' self.test_context._name_task_id = '{}-{}'.format( self.test_context._name, self.test_context._task_id[:8]) + # mock_os.path.exists.return_value = True + self.test_context.key_filename = 'foo/bar/foobar' self.test_context.undeploy() self.assertTrue(mock_template.delete.called) @@ -313,6 +310,7 @@ class HeatContextTestCase(unittest.TestCase): self.test_context._name_task_id = '{}-{}'.format( self.test_context._name, self.test_context._task_id) mock_os.path.exists.return_value = True + self.test_context.key_filename = 'foo/bar/foobar' self.assertIsNone(self.test_context.undeploy()) @mock.patch("yardstick.benchmark.contexts.heat.pkg_resources") @@ -343,7 +341,6 @@ class HeatContextTestCase(unittest.TestCase): 'private_ip': '10.0.0.1', 'public_ip': '127.0.0.1', } - self.test_context.key_uuid = uuid.uuid4() self.test_context._server_map = { 'baz3': baz3_server, 'foo2': foo2_server, @@ -354,6 +351,7 @@ class HeatContextTestCase(unittest.TestCase): 'private_ip_attr': 'private_ip', 'public_ip_attr': 'public_ip', } + self.test_context.key_uuid = 'foo-42' result = self.test_context._get_server(attr_name) self.assertEqual(result['user'], 'bot') self.assertEqual(result['ip'], '127.0.0.1') @@ -385,7 +383,6 @@ class HeatContextTestCase(unittest.TestCase): 'private_ip': '10.0.0.1', 'public_ip': '127.0.0.1', } - self.test_context.key_uuid = uuid.uuid4() self.test_context._server_map = { 'baz3': baz3_server, 'foo2': foo2_server, @@ -394,6 +391,8 @@ class HeatContextTestCase(unittest.TestCase): attr_name = { 'name': 'foo.bar-12345678', } + + self.test_context.key_uuid = 'foo-42' result = self.test_context._get_server(attr_name) self.assertEqual(result['user'], 'bot') # no private ip attr mapping in the map results in None value in the result @@ -418,12 +417,13 @@ class HeatContextTestCase(unittest.TestCase): baz3_server.context.user = 'zab' self.test_context._name = 'bar1' + self.test_context._task_id = '1234567890' + self.test_context._name_task_id = 'bar1-12345678' self.test_context.stack = mock.Mock() self.test_context.stack.outputs = { 'private_ip': '10.0.0.1', 'public_ip': '127.0.0.1', } - self.test_context.key_uuid = uuid.uuid4() self.test_context.generate_routing_table = mock.MagicMock(return_value=[]) self.test_context._server_map = { @@ -461,13 +461,13 @@ class HeatContextTestCase(unittest.TestCase): 'private_ip': '10.0.0.1', 'public_ip': '127.0.0.1', } - self.test_context.key_uuid = uuid.uuid4() self.test_context._server_map = { 'baz3': baz3_server, 'foo2': foo2_server, 'wow4': None, } + self.test_context.key_uuid = 'foo-42' attr_name = 'wow4' result = self.test_context._get_server(attr_name) self.assertIsNone(result) @@ -497,12 +497,12 @@ class HeatContextTestCase(unittest.TestCase): 'private_ip': '10.0.0.1', 'public_ip': '127.0.0.1', } - self.test_context.key_uuid = uuid.uuid4() self.test_context._server_map = { 'baz3': baz3_server, 'foo2': foo2_server, } + self.test_context.key_uuid = 'foo-42' attr_name = { 'name': 'foo.wow4', 'private_ip_attr': 'private_ip', @@ -533,12 +533,12 @@ class HeatContextTestCase(unittest.TestCase): 'private_ip': '10.0.0.1', 'public_ip': '127.0.0.1', } - self.mock_context.key_uuid = uuid.uuid4() self.mock_context._server_map = { 'baz3': baz3_server, 'foo2': foo2_server, } + self.test_context.key_uuid = 'foo-42' attr_name = 'foo.wow4' result = self.test_context._get_server(attr_name) self.assertIsNone(result) -- cgit 1.2.3-korg