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 <emma.l.foley@intel.com>

diff --git a/yardstick/benchmark/contexts/heat.py b/yardstick/benchmark/contexts/heat.py
index 8dd7d85..be88150 100644 (file)
--- 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 558b5d2..403c7e8 100644 (file)
--- 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 2f2d464..1e31abc 100644 (file)
--- 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)