openstack/rally
Code Issues Proposed changes

Refactor tempest config generator

Browse Source 
Use `requests` library instead of `urllib2` & `httplib`
Main profit from `requests` library:
- `requests` is HTTP for Humans;
- `requests` is well documented[1];
- `requests` can save downloaded files by parts, so usage of memory
is less(useful when downloading big data, for example - some images)
- `requests` has same name as in Python 2x and Python 3x

Use `inspect.getmembers`[2][3] for discovery of each function for section
generator. Profit : decrease lines of code

Add checks for existence of subnet in 'network' section.

Move function `_write_config`[4] from class `Tempest`[5] to `TempestConf`.
Reason: this function not related to verifier for Tempest.

Fix message for exception when user doesn't have admin role.

Move name of section into kwargs to each section function.
Profit: less string objects with section names

[1] - http://docs.python-requests.org/en/latest/
[2] - https://docs.python.org/2/library/inspect.html#types-and-members
[3] - https://docs.python.org/3.3/library/inspect.html#types-and-members
[4] - http://git.io/E1zPOQ
[5] - http://git.io/8Ru-Yw

Change-Id: I8307ec33dd93ef055450f58d7cd55bf6b200f249
This commit is contained in:
Andrey Kurilin 2014-06-17 18:06:46 +03:00
parent 3ed98ecec0
commit b9a1fab7a0
4 changed files with 117 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

137
rally/verification/verifiers/tempest/config.py
View File

@ -14,14 +14,15 @@
# under the License.
import datetime
import inspect
import logging
import os
import time
import urllib2
import urlparse
from oslo.config import cfg
import requests
from six.moves import configparser
from six.moves import http_client as httplib
from rally import db
from rally import exceptions
@ -30,6 +31,9 @@ from rally.openstack.common.gettextutils import _
from rally import osclients
LOG = logging.getLogger(__name__)
image_opts = [
cfg.StrOpt('cirros_version',
default='0.3.2',
@ -50,8 +54,9 @@ class TempestConf(object):
try:
self.keystoneclient = self.clients.verified_keystone()
except exceptions.InvalidAdminException:
msg = _('Admin permission is required to run tempest. User %s '
'doesn\'t have admin role') % self.endpoint['username']
msg = (_("Admin permission is required to generate tempest "
"configuration file. User %s doesn't have admin role.") %
self.endpoint['username'])
raise exceptions.TempestConfigCreationFailure(message=msg)
self.available_services = [service['name'] for service in
self.keystoneclient.
@ -73,16 +78,20 @@ class TempestConf(object):
(CONF.image.cirros_version,
CONF.image.cirros_image))
try:
response = urllib2.urlopen(cirros_url)
except urllib2.URLError as err:
response = requests.get(cirros_url, stream=True)
except requests.ConnectionError as err:
msg = _('Error on downloading cirros image, possibly'
' no connection to Internet with message %s') % str(err)
raise exceptions.TempestConfigCreationFailure(message=msg)
if response.getcode() == httplib.OK:
with open(self.img_path, 'wb') as img_file:
img_file.write(response.read())
if response.http_status == 200:
with open(self.img_path + '.tmp', 'wb') as img_file:
for chunk in response.iter_content(chunk_size=1024):
if chunk: # filter out keep-alive new chunks
img_file.write(chunk)
img_file.flush()
os.rename(self.img_path + '.tmp', self.img_path)
else:
if response.getcode() == httplib.NOT_FOUND:
if response.http_status == 404:
msg = _('Error on downloading cirros image, possibly'
'invalid cirros_version or cirros_image in rally.conf')
else:
@ -102,14 +111,14 @@ class TempestConf(object):
os.makedirs(lock_path)
self.conf.set('DEFAULT', 'lock_path', lock_path)
def _set_boto(self):
self.conf.set('boto', 'ec2_url', self._get_url('ec2'))
self.conf.set('boto', 's3_url', self._get_url('s3'))
def _set_boto(self, section_name='boto'):
self.conf.set(section_name, 'ec2_url', self._get_url('ec2'))
self.conf.set(section_name, 's3_url', self._get_url('s3'))
matherials_path = os.path.join(self.data_path, 's3matherials')
self.conf.set('boto', 's3_materials_path', matherials_path)
self.conf.set(section_name, 's3_materials_path', matherials_path)
# TODO(olkonami): find out how can we get ami, ari, aki manifest files
def _set_compute_images(self):
def _set_compute_images(self, section_name='compute'):
glanceclient = self.clients.glance()
image_list = [img for img in glanceclient.images.list()
if img.status.lower() == 'active' and
@ -130,10 +139,10 @@ class TempestConf(object):
'new image could not be created.\n'
'Reason: %s') % e.message
raise exceptions.TempestConfigCreationFailure(message=msg)
self.conf.set('compute', 'image_ref', image_list[0].id)
self.conf.set('compute', 'image_ref_alt', image_list[1].id)
self.conf.set(section_name, 'image_ref', image_list[0].id)
self.conf.set(section_name, 'image_ref_alt', image_list[1].id)
def _set_compute_flavors(self):
def _set_compute_flavors(self, section_name='compute'):
novaclient = self.clients.nova()
flavor_list = sorted(novaclient.flavors.list(),
key=lambda flv: flv.ram)
@ -149,34 +158,37 @@ class TempestConf(object):
'new flavor could not be created.\n'
'Reason: %s') % e.message
raise exceptions.TempestConfigCreationFailure(message=msg)
self.conf.set('compute', 'flavor_ref', flavor_list[0].id)
self.conf.set('compute', 'flavor_ref_alt', flavor_list[1].id)
self.conf.set(section_name, 'flavor_ref', flavor_list[0].id)
self.conf.set(section_name, 'flavor_ref_alt', flavor_list[1].id)
def _set_compute_ssh_connect_method(self):
def _set_compute_ssh_connect_method(self, section_name='compute'):
if 'neutron' in self.available_services:
self.conf.set('compute', 'ssh_connect_method', 'floating')
self.conf.set(section_name, 'ssh_connect_method', 'floating')
else:
self.conf.set('compute', 'ssh_connect_method', 'fixed')
self.conf.set(section_name, 'ssh_connect_method', 'fixed')
def _set_compute_admin(self):
self.conf.set('compute-admin', 'username', self.endpoint['username'])
self.conf.set('compute-admin', 'password', self.endpoint['password'])
self.conf.set('compute-admin', 'tenant_name',
def _set_compute_admin(self, section_name='compute-admin'):
self.conf.set(section_name, 'username', self.endpoint['username'])
self.conf.set(section_name, 'password', self.endpoint['password'])
self.conf.set(section_name, 'tenant_name',
self.endpoint['tenant_name'])
def _set_identity(self):
self.conf.set('identity', 'username', self.endpoint['username'])
self.conf.set('identity', 'password', self.endpoint['password'])
self.conf.set('identity', 'tenant_name', self.endpoint['tenant_name'])
self.conf.set('identity', 'admin_username', self.endpoint['username'])
self.conf.set('identity', 'admin_password', self.endpoint['password'])
self.conf.set('identity', 'admin_tenant_name',
def _set_identity(self, section_name='identity'):
self.conf.set(section_name, 'username', self.endpoint['username'])
self.conf.set(section_name, 'password', self.endpoint['password'])
self.conf.set(section_name, 'tenant_name',
self.endpoint['tenant_name'])
self.conf.set('identity', 'uri', self.endpoint['auth_url'])
self.conf.set('identity', 'uri_v3',
self.conf.set(section_name, 'admin_username',
self.endpoint['username'])
self.conf.set(section_name, 'admin_password',
self.endpoint['password'])
self.conf.set(section_name, 'admin_tenant_name',
self.endpoint['tenant_name'])
self.conf.set(section_name, 'uri', self.endpoint['auth_url'])
self.conf.set(section_name, 'uri_v3',
self.endpoint['auth_url'].replace('/v2.0', '/v3'))
def _set_network(self):
def _set_network(self, section_name='network'):
if 'neutron' in self.available_services:
neutron = self.clients.neutron()
public_net = [net for net in neutron.list_networks()['networks'] if
@ -184,40 +196,51 @@ class TempestConf(object):
net['router:external'] is True]
if public_net:
net_id = public_net[0]['id']
self.conf.set('network', 'public_network_id', net_id)
self.conf.set(section_name, 'public_network_id', net_id)
public_router = neutron.list_routers(
network_id=net_id)['routers'][0]
self.conf.set('network', 'public_router_id',
self.conf.set(section_name, 'public_router_id',
public_router['id'])
subnet = neutron.list_subnets(network_id=net_id)['subnets'][0]
subnets = neutron.list_subnets(network_id=net_id)['subnets']
if subnets:
subnet = subnets[0]
else:
# TODO(akurilin): create public subnet
LOG.warn('No public subnet is found.')
else:
subnet = neutron.list_subnets()[0]
self.conf.set('network', 'default_network', subnet['cidr'])
subnets = neutron.list_subnets()
if subnets:
subnet = subnets[0]
else:
# TODO(akurilin): create subnet
LOG.warn('No subnet is found.')
self.conf.set(section_name, 'default_network', subnet['cidr'])
else:
network = self.clients.nova().networks.list()[0]
self.conf.set('network', 'default_network', network.cidr)
self.conf.set(section_name, 'default_network', network.cidr)
def _set_service_available(self):
def _set_service_available(self, section_name='service_available'):
services = ['neutron', 'heat', 'ceilometer', 'swift',
'cinder', 'nova', 'glance']
for service in services:
self.conf.set('service_available', service,
self.conf.set(section_name, service,
str(service in self.available_services))
horizon_url = ('http://' +
urlparse.urlparse(self.endpoint['auth_url']).hostname)
answer_code = urllib2.urlopen(horizon_url).getcode()
horizon_availability = (requests.get(horizon_url).http_status == 200)
# convert boolean to string because ConfigParser fails
# on attempt to get option with boolean value
self.conf.set('service_available', 'horizon',
str(answer_code == httplib.OK))
self.conf.set(section_name, 'horizon', str(horizon_availability))
def write_config(self, file_name):
with open(file_name, "w+") as f:
self.conf.write(f)
def generate(self, file_name=None):
for name, func in inspect.getmembers(self, predicate=inspect.ismethod):
if name.startswith('_set_'):
func(self)
if file_name:
self.write_config(file_name)
def generate(self):
self._set_default()
self._set_boto()
self._set_compute_images()
self._set_compute_flavors()
self._set_compute_admin()
self._set_identity()
self._set_network()
self._set_service_available()
return self.conf

7
rally/verification/verifiers/tempest/tempest.py
View File

@ -46,10 +46,6 @@ class Tempest(object):
self.verification = verification
self._env = None
def _write_config(self, conf):
with open(self.config_file, "w+") as f:
conf.write(f)
def _generate_env(self):
env = os.environ.copy()
env["TEMPEST_CONFIG_DIR"] = self.tempest_path
@ -87,8 +83,7 @@ class Tempest(object):
msg = _("Creation of configuration file for tempest.")
LOG.info(_("Starting: ") + msg)
conf = config.TempestConf(self.deploy_id).generate()
self._write_config(conf)
config.TempestConf(self.deploy_id).generate(self.config_file)
LOG.info(_("Completed: ") + msg)
else:
LOG.info("Tempest is already configured.")

57
tests/verification/verifiers/test_config.py
View File

@ -17,7 +17,6 @@ import os
import mock
from oslo.config import cfg
from six.moves import http_client as httplib
from rally import exceptions
from rally.verification.verifiers.tempest import config
@ -51,27 +50,26 @@ class ConfigTestCase(test.TestCase):
("use_stderr", "False"))
return [item for item in items if item not in defaults]
@mock.patch("rally.verification.verifiers.tempest.config.urllib2.urlopen")
@mock.patch("rally.verification.verifiers.tempest.config.requests")
@mock.patch("rally.verification.verifiers.tempest.config.os.rename")
@mock.patch("six.moves.builtins.open")
def test__load_img_success(self, mock_open, mock_urlopen):
def test__load_img_success(self, mock_open, mock_rename, mock_requests):
mock_result = mock.MagicMock()
mock_result.getcode.return_value = httplib.OK
mock_urlopen.return_value = mock_result
mock_result.http_status = 200
mock_requests.get.return_value = mock_result
mock_file = mock.MagicMock()
mock_open.return_value = mock_file
self.conf_generator._load_img()
cirros_url = ("http://download.cirros-cloud.net/%s/%s" %
(CONF.image.cirros_version,
CONF.image.cirros_image))
mock_urlopen.assert_called_once_with(cirros_url)
mock_file.__enter__().write.assert_called_once_with(mock_result.read())
mock_file.__exit__.assert_called_once_with(None, None, None)
mock_requests.get.assert_called_once_with(cirros_url, stream=True)
@mock.patch("rally.verification.verifiers.tempest.config.urllib2.urlopen")
def test__load_img_notfound(self, mock_urlopen):
@mock.patch("rally.verification.verifiers.tempest.config.requests")
def test__load_img_notfound(self, mock_requests):
mock_result = mock.MagicMock()
mock_result.getcode.return_value = httplib.NOT_FOUND
mock_urlopen.return_value = mock_result
mock_result.http_status = 404
mock_requests.get.return_value = mock_result
self.assertRaises(exceptions.TempestConfigCreationFailure,
self.conf_generator._load_img)
@ -271,11 +269,11 @@ class ConfigTestCase(test.TestCase):
self.conf_generator.conf.get("network",
"default_network"))
@mock.patch("rally.verification.verifiers.tempest.config.urllib2.urlopen")
def test__set_service_available(self, mock_urlopen):
@mock.patch("rally.verification.verifiers.tempest.config.requests")
def test__set_service_available(self, mock_requests):
mock_result = mock.MagicMock()
mock_result.getcode.return_value = httplib.NOT_FOUND
mock_urlopen.return_value = mock_result
mock_result.http_status = 404
mock_requests.get.return_value = mock_result
available_services = ("nova", "cinder", "glance")
self.conf_generator.available_services = available_services
self.conf_generator._set_service_available()
@ -287,12 +285,25 @@ class ConfigTestCase(test.TestCase):
self.conf_generator.conf.items("service_available"))
self.assertEqual(sorted(expected), sorted(options))
@mock.patch("rally.verification.verifiers.tempest.config.urllib2.urlopen")
def test__set_service_available_horizon(self, mock_urlopen):
@mock.patch("rally.verification.verifiers.tempest.config.requests")
def test__set_service_available_horizon(self, mock_requests):
mock_result = mock.MagicMock()
mock_result.getcode.return_value = httplib.OK
mock_urlopen.return_value = mock_result
mock_result.http_status = 200
mock_requests.get.return_value = mock_result
self.conf_generator._set_service_available()
self.assertEqual(self.conf_generator.conf.get("service_available",
"horizon"),
"True")
self.assertEqual(self.conf_generator.conf.get(
"service_available", "horizon"), "True")
@mock.patch('six.moves.builtins.open')
def test_write_config(self, mock_open):
self.conf_generator.conf = mock.Mock()
mock_file = mock.MagicMock()
mock_open.return_value = mock_file
file_name = '/path/to/fake/conf'
self.conf_generator.write_config(file_name)
mock_open.assert_called_once_with(file_name, 'w+')
self.conf_generator.conf.write.assert_called_once_with(
mock_file.__enter__())
mock_file.__exit__.assert_called_once_with(None, None, None)

20
tests/verification/verifiers/test_tempest.py
View File

@ -38,16 +38,6 @@ class TempestTestCase(test.TestCase):
self.verifier.log_file_raw = '/tmp/subunit.stream'
self.regex = None
@mock.patch('six.moves.builtins.open')
def test__write_config(self, mock_open):
conf = mock.Mock()
mock_file = mock.MagicMock()
mock_open.return_value = mock_file
self.verifier._write_config(conf)
mock_open.assert_called_once_with(self.verifier.config_file, 'w+')
conf.write.assert_called_once_with(mock_file.__enter__())
mock_file.__exit__.assert_called_once_with(None, None, None)
@mock.patch('os.path.exists')
def test_is_installed(self, mock_exists):
mock_exists.return_value = True
@ -110,20 +100,14 @@ class TempestTestCase(test.TestCase):
@mock.patch(TEMPEST_PATH + '.tempest.Tempest.discover_tests')
@mock.patch(TEMPEST_PATH + '.tempest.Tempest._initialize_testr')
@mock.patch(TEMPEST_PATH + '.tempest.Tempest.run')
@mock.patch(TEMPEST_PATH + '.tempest.Tempest._write_config')
@mock.patch(TEMPEST_PATH + '.config.TempestConf')
@mock.patch('rally.db.deployment_get')
@mock.patch('rally.osclients.Clients')
@mock.patch('rally.objects.endpoint.Endpoint')
def test_verify(self, mock_endpoint, mock_osclients, mock_get, mock_conf,
mock_write, mock_run, mock_testr_init, mock_discover,
mock_os):
fake_conf = mock.MagicMock()
mock_conf().generate.return_value = fake_conf
mock_run, mock_testr_init, mock_discover, mock_os):
self.verifier.verify("smoke", None)
mock_conf().generate.assert_called_once_with()
mock_write.assert_called_once_with(fake_conf)
mock_conf().generate.assert_called_once_with(self.verifier.config_file)
mock_run.assert_called_once_with("smoke")
@mock.patch('os.environ')