diff --git a/rally/benchmark/context/base.py b/rally/benchmark/context/base.py index 33bc48feb5..5e7f13c701 100644 --- a/rally/benchmark/context/base.py +++ b/rally/benchmark/context/base.py @@ -85,10 +85,6 @@ class Context(object): raise exceptions.NoSuchContext(name=cls.get_name()) jsonschema.validate(config, cls.CONFIG_SCHEMA) - @classmethod - def validate_semantic(cls, config, admin=None, users=None, task=None): - """Context semantic validation towards the deployment.""" - @classmethod def get_name(cls): return cls._ctx_name @@ -132,12 +128,6 @@ class ContextManager(object): for name, config in six.iteritems(context): Context.get_by_name(name).validate(config, non_hidden=non_hidden) - @staticmethod - def validate_semantic(context, admin=None, users=None, task=None): - for name, config in six.iteritems(context): - Context.get_by_name(name).validate_semantic(config, admin=admin, - users=users, task=task) - def _get_sorted_context_lst(self): ctxlst = map(Context.get_by_name, self.context_obj["config"]) return sorted(map(lambda ctx: ctx(self.context_obj), ctxlst)) diff --git a/rally/benchmark/context/images.py b/rally/benchmark/context/images.py index 9a62bc694b..c33d499f8f 100644 --- a/rally/benchmark/context/images.py +++ b/rally/benchmark/context/images.py @@ -12,14 +12,11 @@ # License for the specific language governing permissions and limitations # under the License. -import six - from rally.benchmark.context import base from rally.benchmark.context.cleanup import manager as resource_manager from rally.benchmark.scenarios import base as scenario_base from rally.benchmark.scenarios.glance import utils as glance_utils from rally.common.i18n import _ -from rally import exceptions from rally import log as logging from rally import osclients from rally import utils as rutils @@ -88,15 +85,3 @@ class ImageGenerator(base.Context): # TODO(boris-42): Delete only resources created by this context resource_manager.cleanup(names=["glance.images"], users=self.context.get("users", [])) - - @classmethod - def validate_semantic(cls, config, admin, users, task): - """Check if the image service is available.""" - try: - glance = users[0].glance() - list(glance.images.list(limit=0)) - except Exception as e: - message = _( - "The image service is unavailable, Reason: %(reason)s") % { - "reason": six.text_type(e)} - raise exceptions.InvalidScenarioArgument(message) diff --git a/rally/benchmark/context/users.py b/rally/benchmark/context/users.py index a0c2374718..80ad28c3cd 100644 --- a/rally/benchmark/context/users.py +++ b/rally/benchmark/context/users.py @@ -26,7 +26,7 @@ from rally.common.i18n import _ from rally import consts from rally import exceptions from rally import log as logging -from rally.objects import endpoint +from rally import objects from rally import osclients from rally import utils as rutils @@ -182,7 +182,7 @@ class UserGenerator(base.Context): user = client.create_user(username, password, "%s@email.me" % username, tenant_id, user_dom) - user_endpoint = endpoint.Endpoint( + user_endpoint = objects.Endpoint( client.auth_url, user.name, password, self.context["tenants"][tenant_id]["name"], consts.EndpointPermission.USER, client.region_name, diff --git a/rally/benchmark/engine.py b/rally/benchmark/engine.py index 0dc86c853d..3f32b5d9dc 100644 --- a/rally/benchmark/engine.py +++ b/rally/benchmark/engine.py @@ -30,7 +30,7 @@ from rally.common.i18n import _ from rally import consts from rally import exceptions from rally import log as logging -from rally.objects import endpoint +from rally import objects from rally import osclients from rally import utils as rutils @@ -103,8 +103,8 @@ class BenchmarkEngine(object): """ self.config = config self.task = task - self.admin = admin and endpoint.Endpoint(**admin) or None - self.users = map(lambda u: endpoint.Endpoint(**u), users or []) + self.admin = admin and objects.Endpoint(**admin) or None + self.users = map(lambda u: objects.Endpoint(**u), users or []) @rutils.log_task_wrapper(LOG.info, _("Task validation check cloud.")) def _check_cloud(self): @@ -139,14 +139,11 @@ class BenchmarkEngine(object): ) def _validate_config_semantic_helper(self, admin, user, name, pos, - task, kwargs): - context = {} if not kwargs else kwargs.get("context", {}) - + deployment, kwargs): try: base_scenario.Scenario.validate(name, kwargs, admin=admin, - users=[user], task=task) - base_ctx.ContextManager.validate_semantic(context, admin=admin, - users=[user], task=task) + users=[user], + deployment=deployment) except exceptions.InvalidScenarioArgument as e: kw = {"name": name, "pos": pos, "config": kwargs, "reason": six.text_type(e)} @@ -159,6 +156,8 @@ class BenchmarkEngine(object): # NOTE(boris-42): In future we will have more complex context, because # we will have pre-created users mode as well. context = {"task": self.task, "admin": {"endpoint": self.admin}} + deployment = objects.Deployment.get(self.task["deployment_uuid"]) + with users_ctx.UserGenerator(context) as ctx: ctx.setup() admin = osclients.Clients(self.admin) @@ -167,7 +166,7 @@ class BenchmarkEngine(object): for name, values in six.iteritems(config): for pos, kwargs in enumerate(values): self._validate_config_semantic_helper(admin, user, name, - pos, self.task, + pos, deployment, kwargs) @rutils.log_task_wrapper(LOG.info, _("Task validation.")) diff --git a/rally/benchmark/scenarios/base.py b/rally/benchmark/scenarios/base.py index 015b65e8fb..59974e0c7c 100644 --- a/rally/benchmark/scenarios/base.py +++ b/rally/benchmark/scenarios/base.py @@ -22,9 +22,13 @@ import time from rally import consts from rally import exceptions +from rally import log as logging from rally import utils +LOG = logging.getLogger(__name__) + + def scenario(context=None): """Make from plain python method benchmark. @@ -120,18 +124,20 @@ class Scenario(object): return benchmark_scenarios_flattened @staticmethod - def _validate_helper(validators, clients, config, task): + def _validate_helper(validators, clients, config, deployment): for validator in validators: try: - result = validator(config, clients=clients, task=task) + result = validator(config, clients=clients, + deployment=deployment) except Exception as e: + LOG.exception(e) raise exceptions.InvalidScenarioArgument(e) else: if not result.is_valid: raise exceptions.InvalidScenarioArgument(result.msg) @classmethod - def validate(cls, name, config, admin=None, users=None, task=None): + def validate(cls, name, config, admin=None, users=None, deployment=None): """Semantic check of benchmark arguments.""" validators = cls.meta(name, "validators", default=[]) @@ -146,10 +152,10 @@ class Scenario(object): # NOTE(boris-42): Potential bug, what if we don't have "admin" client # and scenario have "admin" validators. if admin: - cls._validate_helper(admin_validators, admin, config, task) + cls._validate_helper(admin_validators, admin, config, deployment) if users: for user in users: - cls._validate_helper(user_validators, user, config, task) + cls._validate_helper(user_validators, user, config, deployment) @staticmethod def meta(cls, attr_name, method_name=None, default=None): diff --git a/rally/benchmark/validation.py b/rally/benchmark/validation.py index 430bbeba21..a0565d3437 100644 --- a/rally/benchmark/validation.py +++ b/rally/benchmark/validation.py @@ -14,6 +14,7 @@ # License for the specific language governing permissions and limitations # under the License. +import functools import os from glanceclient import exc as glance_exc @@ -23,7 +24,6 @@ from rally.benchmark import types as types from rally.common.i18n import _ from rally import consts from rally import exceptions -from rally import objects from rally.verification.verifiers.tempest import tempest @@ -50,9 +50,11 @@ def validator(fn): :param kwargs: the keyword arguments of the decorator of the scenario ex. @my_decorator(kwarg1="kwarg1"), then kwargs = {"kwarg1": "kwarg1"} """ - def wrap_validator(config, clients, task): + + @functools.wraps(fn) + def wrap_validator(config, clients, deployment): # NOTE(amaretskiy): validator is successful by default - return (fn(config, clients, task, *args, **kwargs) or + return (fn(config, clients, deployment, *args, **kwargs) or ValidationResult()) def wrap_scenario(scenario): @@ -70,7 +72,7 @@ def validator(fn): @validator -def number(config, clients, task, param_name, minval=None, maxval=None, +def number(config, clients, deployment, param_name, minval=None, maxval=None, nullable=False, integer_only=False): """Checks that parameter is number that pass specified condition. @@ -123,7 +125,7 @@ def number(config, clients, task, param_name, minval=None, maxval=None, @validator -def file_exists(config, clients, task, param_name, mode=os.R_OK): +def file_exists(config, clients, deployment, param_name, mode=os.R_OK): """Validator checks parameter is proper path to file with proper mode. Ensure a file exists and can be accessed with the specified mode. @@ -180,7 +182,7 @@ def _get_validated_flavor(config, clients, param_name): @validator -def image_exists(config, clients, task, param_name): +def image_exists(config, clients, deployment, param_name): """Returns validator for image_id :param param_name: defines which variable should be used @@ -190,7 +192,7 @@ def image_exists(config, clients, task, param_name): @validator -def flavor_exists(config, clients, task, param_name): +def flavor_exists(config, clients, deployment, param_name): """Returns validator for flavor :param param_name: defines which variable should be used @@ -200,7 +202,8 @@ def flavor_exists(config, clients, task, param_name): @validator -def image_valid_on_flavor(config, clients, task, flavor_name, image_name): +def image_valid_on_flavor(config, clients, deployment, flavor_name, + image_name): """Returns validator for image could be used for current flavor :param flavor_name: defines which variable should be used @@ -236,7 +239,7 @@ def image_valid_on_flavor(config, clients, task, flavor_name, image_name): @validator -def network_exists(config, clients, task, network_name): +def network_exists(config, clients, deployment, network_name): """Validator checks that network with network_name exist.""" network = config.get("args", {}).get(network_name, "private") @@ -255,7 +258,7 @@ def network_exists(config, clients, task, network_name): @validator -def external_network_exists(config, clients, task, network_name, +def external_network_exists(config, clients, deployment, network_name, use_external_network): """Validator checks that externatl network with network_name exist.""" @@ -281,7 +284,7 @@ def external_network_exists(config, clients, task, network_name, @validator -def tempest_tests_exists(config, clients, task): +def tempest_tests_exists(config, clients, deployment): """Validator checks that specified test exists.""" args = config.get("args", {}) @@ -294,7 +297,7 @@ def tempest_tests_exists(config, clients, task): return ValidationResult(False, _("Parameter 'test_name' or 'test_names' " "should be specified.")) - verifier = tempest.Tempest(task["deployment_uuid"]) + verifier = tempest.Tempest(deployment["uuid"]) if not verifier.is_installed(): try: verifier.install() @@ -320,7 +323,7 @@ def tempest_tests_exists(config, clients, task): @validator -def tempest_set_exists(config, clients, task): +def tempest_set_exists(config, clients, deployment): """Validator that check that tempest set_name is valid.""" set_name = config.get("args", {}).get("set_name") @@ -335,7 +338,7 @@ def tempest_set_exists(config, clients, task): @validator -def required_parameters(config, clients, task, *required_params): +def required_parameters(config, clients, deployment, *required_params): """Validtor for checking required parameters are specified. :param *required_params: list of required parameters @@ -349,7 +352,7 @@ def required_parameters(config, clients, task, *required_params): @validator -def required_services(config, clients, task, *required_services): +def required_services(config, clients, deployment, *required_services): """Validator checks if specified OpenStack services are available. :param *required_services: list of services names @@ -366,7 +369,7 @@ def required_services(config, clients, task, *required_services): @validator -def required_contexts(config, clients, task, *context_names): +def required_contexts(config, clients, deployment, *context_names): """Validator hecks if required benchmark contexts are specified. :param *context_names: list of context names that should be specified @@ -382,7 +385,7 @@ def required_contexts(config, clients, task, *context_names): @validator -def required_openstack(config, clients, task, admin=False, users=False): +def required_openstack(config, clients, deployment, admin=False, users=False): """Validator that requires OpenStack admin or (and) users. This allows us to create 4 kind of benchmarks: @@ -399,8 +402,6 @@ def required_openstack(config, clients, task, admin=False, users=False): return ValidationResult( False, _("You should specify admin=True or users=True or both.")) - deployment = objects.Deployment.get(task["deployment_uuid"]) - if deployment["admin"] and deployment["users"]: return ValidationResult() @@ -417,7 +418,7 @@ def required_openstack(config, clients, task, admin=False, users=False): @validator -def volume_type_exists(config, clients, task, param_name): +def volume_type_exists(config, clients, deployment, param_name): """Returns validator for volume types. check_types: defines variable to be used as the flag to determine if diff --git a/rally/cmd/commands/deployment.py b/rally/cmd/commands/deployment.py index c72c1e12c7..93c77ba4ed 100644 --- a/rally/cmd/commands/deployment.py +++ b/rally/cmd/commands/deployment.py @@ -31,7 +31,7 @@ from rally.cmd import envutils from rally.common.i18n import _ from rally import db from rally import exceptions -from rally.objects import endpoint +from rally import objects from rally.openstack.common import cliutils as common_cliutils from rally import osclients from rally import utils @@ -241,7 +241,7 @@ class DeploymentCommands(object): admin = db.deployment_get(deployment)['admin'] # TODO(boris-42): make this work for users in future for endpoint_dict in [admin]: - clients = osclients.Clients(endpoint.Endpoint(**endpoint_dict)) + clients = osclients.Clients(objects.Endpoint(**endpoint_dict)) client = clients.verified_keystone() print("keystone endpoints are valid and following " "services are available:") diff --git a/rally/cmd/commands/show.py b/rally/cmd/commands/show.py index 3d3ea1b29b..b1e8d2517d 100644 --- a/rally/cmd/commands/show.py +++ b/rally/cmd/commands/show.py @@ -22,7 +22,7 @@ from rally.cmd import envutils from rally.common.i18n import _ from rally import db from rally import exceptions -from rally.objects import endpoint +from rally import objects from rally.openstack.common import cliutils as common_cliutils from rally import osclients from rally import utils @@ -64,7 +64,7 @@ class ShowCommands(object): try: for endpoint_dict in self._get_endpoints(deployment): - clients = osclients.Clients(endpoint.Endpoint(**endpoint_dict)) + clients = osclients.Clients(objects.Endpoint(**endpoint_dict)) glance_client = clients.glance() for image in glance_client.images.list(): data = [image.id, image.name, image.size] @@ -100,7 +100,7 @@ class ShowCommands(object): table_rows = [] try: for endpoint_dict in self._get_endpoints(deployment): - clients = osclients.Clients(endpoint.Endpoint(**endpoint_dict)) + clients = osclients.Clients(objects.Endpoint(**endpoint_dict)) nova_client = clients.nova() for flavor in nova_client.flavors.list(): data = [flavor.id, flavor.name, flavor.vcpus, @@ -130,7 +130,7 @@ class ShowCommands(object): table_rows = [] try: for endpoint_dict in self._get_endpoints(deployment): - clients = osclients.Clients(endpoint.Endpoint(**endpoint_dict)) + clients = osclients.Clients(objects.Endpoint(**endpoint_dict)) nova_client = clients.nova() for network in nova_client.networks.list(): data = [network.id, network.label, network.cidr] @@ -157,7 +157,7 @@ class ShowCommands(object): table_rows = [] try: for endpoint_dict in self._get_endpoints(deployment): - clients = osclients.Clients(endpoint.Endpoint(**endpoint_dict)) + clients = osclients.Clients(objects.Endpoint(**endpoint_dict)) nova_client = clients.nova() for secgroup in nova_client.security_groups.list(): data = [secgroup.id, secgroup.name, @@ -187,7 +187,7 @@ class ShowCommands(object): table_rows = [] try: for endpoint_dict in self._get_endpoints(deployment): - clients = osclients.Clients(endpoint.Endpoint(**endpoint_dict)) + clients = osclients.Clients(objects.Endpoint(**endpoint_dict)) nova_client = clients.nova() for keypair in nova_client.keypairs.list(): data = [keypair.name, keypair.fingerprint] diff --git a/rally/deploy/serverprovider/providers/openstack.py b/rally/deploy/serverprovider/providers/openstack.py index c3b671a8c9..d46c44724b 100644 --- a/rally/deploy/serverprovider/providers/openstack.py +++ b/rally/deploy/serverprovider/providers/openstack.py @@ -24,7 +24,7 @@ from rally.common.i18n import _ from rally.deploy.serverprovider import provider from rally import exceptions from rally import log as logging -from rally.objects import endpoint +from rally import objects from rally import osclients @@ -104,8 +104,8 @@ class OpenStackProvider(provider.ProviderFactory): def __init__(self, deployment, config): super(OpenStackProvider, self).__init__(deployment, config) - user_endpoint = endpoint.Endpoint(config['auth_url'], config['user'], - config['password'], config['tenant']) + user_endpoint = objects.Endpoint(config['auth_url'], config['user'], + config['password'], config['tenant']) clients = osclients.Clients(user_endpoint) self.nova = clients.nova() try: diff --git a/rally/verification/verifiers/tempest/config.py b/rally/verification/verifiers/tempest/config.py index c901d9516d..26cadc1e56 100644 --- a/rally/verification/verifiers/tempest/config.py +++ b/rally/verification/verifiers/tempest/config.py @@ -27,7 +27,7 @@ from rally.common.i18n import _ from rally import db from rally import exceptions from rally import log as logging -from rally.objects import endpoint +from rally import objects from rally import osclients @@ -54,7 +54,7 @@ class TempestConf(object): def __init__(self, deployment): self.endpoint = db.deployment_get(deployment)['admin'] - self.clients = osclients.Clients(endpoint.Endpoint(**self.endpoint)) + self.clients = osclients.Clients(objects.Endpoint(**self.endpoint)) try: self.keystoneclient = self.clients.verified_keystone() except exceptions.InvalidAdminException: diff --git a/tests/hacking/README.rst b/tests/hacking/README.rst index 1b83407561..69a9ad44e6 100644 --- a/tests/hacking/README.rst +++ b/tests/hacking/README.rst @@ -25,3 +25,4 @@ Rally Specific Commandments * [N330] - Ensure that ``dict.iteritems()`` is not used * [N331] - Ensure that ``basestring`` is not used * [N332] - Ensure that ``StringIO.StringIO`` is not used +* [N340] - Ensure that we are importing always ``from rally import objects`` \ No newline at end of file diff --git a/tests/hacking/checks.py b/tests/hacking/checks.py index 4a6009f9ca..8020a35986 100644 --- a/tests/hacking/checks.py +++ b/tests/hacking/checks.py @@ -267,6 +267,23 @@ def check_StringIO_method(logical_line): "rather than StringIO.StringIO.") +def check_no_direct_rally_objects_import(logical_line, filename): + """Check if rally.objects are properly imported. + + If you import "from rally import objects" you are able to use objects + directly like objects.Task. + + N340 + """ + if filename == "./rally/objects/__init__.py": + return + + if (logical_line.startswith("from rally.objects") + or logical_line.startswith("import rally.objects.")): + yield (0, "N340: Import objects module: `from rally import objects`. " + "After that you can use directly objects e.g. objects.Task") + + def factory(register): register(check_assert_methods_from_mock) register(check_import_of_logging) @@ -280,3 +297,4 @@ def factory(register): register(check_iteritems_method) register(check_basestring_method) register(check_StringIO_method) + register(check_no_direct_rally_objects_import) diff --git a/tests/unit/benchmark/context/test_base.py b/tests/unit/benchmark/context/test_base.py index 664d21b126..6e981a59bb 100644 --- a/tests/unit/benchmark/context/test_base.py +++ b/tests/unit/benchmark/context/test_base.py @@ -160,21 +160,6 @@ class ContextManagerTestCase(test.TestCase): mock.call().validate(config[ctx], non_hidden=False), ]) - @mock.patch("rally.benchmark.context.base.Context.get_by_name") - def test_validate_semantic(self, mock_get): - config = { - "ctx1": mock.MagicMock(), - "ctx2": mock.MagicMock() - } - - base.ContextManager.validate_semantic(config) - for ctx in ("ctx1", "ctx2"): - mock_get.assert_has_calls([ - mock.call(ctx), - mock.call().validate_semantic(config[ctx], admin=None, - users=None, task=None), - ]) - @mock.patch("rally.benchmark.context.base.Context.get_by_name") def test_validate_non_hidden(self, mock_get): config = { diff --git a/tests/unit/benchmark/context/test_images.py b/tests/unit/benchmark/context/test_images.py index ba6266dd5d..ab638733e4 100644 --- a/tests/unit/benchmark/context/test_images.py +++ b/tests/unit/benchmark/context/test_images.py @@ -19,7 +19,6 @@ import jsonschema import mock from rally.benchmark.context import images -from rally import exceptions from tests.unit import fakes from tests.unit import test @@ -158,14 +157,3 @@ class ImageGeneratorTestCase(test.TestCase): images_ctx.cleanup() mock_cleanup.assert_called_once_with(names=["glance.images"], users=context["users"]) - - def test_validate_semantic(self): - users = [fakes.FakeClients()] - images.ImageGenerator.validate_semantic(None, None, users, None) - - @mock.patch("%s.images.osclients.Clients.glance" % CTX) - def test_validate_semantic_unavailabe(self, mock_glance): - mock_glance.side_effect = Exception("list error") - self.assertRaises(exceptions.InvalidScenarioArgument, - images.ImageGenerator.validate_semantic, None, None, - None, None) diff --git a/tests/unit/benchmark/scenarios/glance/test_images.py b/tests/unit/benchmark/scenarios/glance/test_images.py index c967436126..aa139eac44 100644 --- a/tests/unit/benchmark/scenarios/glance/test_images.py +++ b/tests/unit/benchmark/scenarios/glance/test_images.py @@ -17,7 +17,7 @@ import mock from rally.benchmark.scenarios.glance import images from rally.benchmark.scenarios.nova import servers -from rally.objects import endpoint +from rally import objects from rally import osclients from tests.unit import fakes from tests.unit import test @@ -79,7 +79,7 @@ class GlanceImagesTestCase(test.TestCase): fc.glance = lambda: fake_glance fake_nova = fakes.FakeNovaClient() fc.nova = lambda: fake_nova - user_endpoint = endpoint.Endpoint("url", "user", "password", "tenant") + user_endpoint = objects.Endpoint("url", "user", "password", "tenant") nova_scenario._clients = osclients.Clients(user_endpoint) fake_image = fakes.FakeImage() fake_servers = [object() for i in range(5)] diff --git a/tests/unit/benchmark/scenarios/nova/test_servers.py b/tests/unit/benchmark/scenarios/nova/test_servers.py index 223df5b083..0770656500 100644 --- a/tests/unit/benchmark/scenarios/nova/test_servers.py +++ b/tests/unit/benchmark/scenarios/nova/test_servers.py @@ -17,7 +17,7 @@ import mock from rally.benchmark.scenarios.nova import servers from rally import exceptions as rally_exceptions -from rally.objects import endpoint +from rally import objects from rally import osclients from tests.unit import fakes from tests.unit import test @@ -231,7 +231,7 @@ class NovaServersTestCase(test.TestCase): nova = fakes.FakeNovaClient() fc.nova = lambda: nova - user_endpoint = endpoint.Endpoint("url", "user", "password", "tenant") + user_endpoint = objects.Endpoint("url", "user", "password", "tenant") clients = osclients.Clients(user_endpoint) scenario = servers.NovaServers(clients=clients) diff --git a/tests/unit/benchmark/scenarios/test_base.py b/tests/unit/benchmark/scenarios/test_base.py index b9eb406709..b308b75acf 100644 --- a/tests/unit/benchmark/scenarios/test_base.py +++ b/tests/unit/benchmark/scenarios/test_base.py @@ -68,10 +68,11 @@ class ScenarioTestCase(test.TestCase): ] clients = mock.MagicMock() config = {"a": 1, "b": 2} - task = mock.MagicMock() - base.Scenario._validate_helper(validators, clients, config, task) + deployment = mock.MagicMock() + base.Scenario._validate_helper(validators, clients, config, deployment) for validator in validators: - validator.assert_called_with(config, clients=clients, task=task) + validator.assert_called_with(config, clients=clients, + deployment=deployment) def test__validate_helper_somethingwent_wrong(self): validator = mock.MagicMock() @@ -79,8 +80,9 @@ class ScenarioTestCase(test.TestCase): self.assertRaises(exceptions.InvalidScenarioArgument, base.Scenario._validate_helper, - [validator], "cl", "config", "task") - validator.assert_called_once_with("config", clients="cl", task="task") + [validator], "cl", "config", "deployment") + validator.assert_called_once_with("config", clients="cl", + deployment="deployment") def test__validate_helper__no_valid(self): validators = [ @@ -125,12 +127,12 @@ class ScenarioTestCase(test.TestCase): validator.permission = consts.EndpointPermission.ADMIN FakeScenario.do_it.validators = validators - task = mock.MagicMock() + deployment = mock.MagicMock() args = {"a": 1, "b": 2} base.Scenario.validate( - "FakeScenario.do_it", args, admin="admin", task=task) + "FakeScenario.do_it", args, admin="admin", deployment=deployment) mock_validate_helper.assert_called_once_with(validators, "admin", args, - task) + deployment) @mock.patch("rally.benchmark.scenarios.base.Scenario._validate_helper") @mock.patch("rally.benchmark.scenarios.base.Scenario.get_by_name") diff --git a/tests/unit/benchmark/test_engine.py b/tests/unit/benchmark/test_engine.py index 5ffeeebd93..e3998f2f34 100644 --- a/tests/unit/benchmark/test_engine.py +++ b/tests/unit/benchmark/test_engine.py @@ -161,12 +161,13 @@ class BenchmarkEngineTestCase(test.TestCase): @mock.patch("rally.benchmark.engine.base_scenario.Scenario.validate") def test__validate_config_semantic_helper(self, mock_validate): - task = mock.MagicMock() + deployment = mock.MagicMock() eng = engine.BenchmarkEngine(mock.MagicMock(), mock.MagicMock()) eng._validate_config_semantic_helper("admin", "user", "name", "pos", - task, {"args": "args"}) - mock_validate.assert_called_once_with( - "name", {"args": "args"}, admin="admin", users=["user"], task=task) + deployment, {"args": "args"}) + mock_validate.assert_called_once_with("name", {"args": "args"}, + admin="admin", users=["user"], + deployment=deployment) @mock.patch("rally.benchmark.engine.base_scenario.Scenario.validate") def test__validate_config_semanitc_helper_invalid_arg(self, mock_validate): @@ -177,24 +178,14 @@ class BenchmarkEngineTestCase(test.TestCase): eng._validate_config_semantic_helper, "a", "u", "n", "p", mock.MagicMock(), {}) - @mock.patch("rally.benchmark.engine.base_scenario.Scenario") - @mock.patch( - "rally.benchmark.engine.base_ctx.ContextManager.validate_semantic") - def test__validate_config_semanitc_helper_invalid_context(self, - mock_validate_sm, - mock_scenario): - mock_validate_sm.side_effect = exceptions.InvalidScenarioArgument() - eng = engine.BenchmarkEngine(mock.MagicMock(), mock.MagicMock()) - - self.assertRaises(exceptions.InvalidBenchmarkConfig, - eng._validate_config_semantic_helper, "a", "u", "n", - "p", mock.MagicMock(), {}) - @mock.patch("rally.benchmark.engine.osclients.Clients") @mock.patch("rally.benchmark.engine.users_ctx") @mock.patch("rally.benchmark.engine.BenchmarkEngine" "._validate_config_semantic_helper") - def test__validate_config_semantic(self, mock_helper, mock_userctx, + @mock.patch("rally.benchmark.engine.objects.Deployment.get", + return_value="FakeDeployment") + def test__validate_config_semantic(self, mock_deployment_get, + mock_helper, mock_userctx, mock_osclients): mock_userctx.UserGenerator = fakes.FakeUserContext mock_osclients.return_value = mock.MagicMock() @@ -216,11 +207,14 @@ class BenchmarkEngineTestCase(test.TestCase): ] mock_osclients.assert_has_calls(expected_calls) + mock_deployment_get.assert_called_once_with(fake_task["uuid"]) + admin = user = mock_osclients.return_value + fake_deployment = mock_deployment_get.return_value expected_calls = [ - mock.call(admin, user, "a", 0, fake_task, config["a"][0]), - mock.call(admin, user, "a", 1, fake_task, config["a"][1]), - mock.call(admin, user, "b", 0, fake_task, config["b"][0]) + mock.call(admin, user, "a", 0, fake_deployment, config["a"][0]), + mock.call(admin, user, "a", 1, fake_deployment, config["a"][1]), + mock.call(admin, user, "b", 0, fake_deployment, config["b"][0]) ] mock_helper.assert_has_calls(expected_calls, any_order=True) diff --git a/tests/unit/benchmark/test_validation.py b/tests/unit/benchmark/test_validation.py index 0c1a92341b..e2985b7d64 100644 --- a/tests/unit/benchmark/test_validation.py +++ b/tests/unit/benchmark/test_validation.py @@ -20,7 +20,6 @@ from novaclient import exceptions as nova_exc from rally.benchmark import validation from rally import consts from rally import exceptions -from rally import objects from rally.verification.verifiers.tempest import tempest from tests.unit import test @@ -188,12 +187,12 @@ class ValidatorsTestCase(test.TestCase): def test_image_exists(self): validator = self._unwrap_validator(validation.image_exists, "param") - result = validator({}, "clients", "task") + result = validator({}, "clients", "deployment") self.assertFalse(result.is_valid, result.msg) def test_flavor_exists(self): validator = self._unwrap_validator(validation.flavor_exists, "param") - result = validator({}, "clients", "task") + result = validator({}, "clients", "deployment") self.assertFalse(result.is_valid, result.msg) def test_image_valid_on_flavor_flavor_or_image_not_specified(self): @@ -293,68 +292,49 @@ class ValidatorsTestCase(test.TestCase): self.assertFalse(result.is_valid, result.msg) @mock.patch("rally.benchmark.validation.tempest.Tempest") - @mock.patch("rally.objects.task.db.task_create") - def test_tempest_tests_exists(self, mock_create, mock_tempest): - mock_create.return_value = { - 'status': 'init', - 'deployment_uuid': 'deployment-uuid', - 'verification_log': '', - 'uuid': 'task-uuid', - 'created_at': '', - 'failed': False, - 'tag': '', - 'id': 42} + def test_tempest_tests_exists(self, mock_tempest): mock_tempest().is_installed.return_value = False mock_tempest().is_configured.return_value = False mock_tempest().discover_tests.return_value = set([ "tempest.api.a", "tempest.api.b", "tempest.api.c"]) - task = objects.Task(deployment_uuid='deployment-uuid') + + deployment = {"uuid": "someuuid"} validator = self._unwrap_validator(validation.tempest_tests_exists) - result = validator({"args": {"test_name": "a"}}, None, task) + result = validator({"args": {"test_name": "a"}}, None, deployment) self.assertTrue(result.is_valid, result.msg) mock_tempest().is_installed.assert_called_once_with() mock_tempest().is_configured.assert_called_once_with() mock_tempest().discover_tests.assert_called_once_with() - result = validator({"args": {"test_name": "d"}}, None, task) + result = validator({"args": {"test_name": "d"}}, None, deployment) self.assertFalse(result.is_valid, result.msg) result = validator({"args": {"test_name": "tempest.api.a"}}, None, - task) + deployment) self.assertTrue(result.is_valid, result.msg) result = validator({"args": {"test_name": "tempest.api.d"}}, None, - task) + deployment) self.assertFalse(result.is_valid, result.msg) result = validator({"args": {"test_names": ["tempest.api.a", "b"]}}, - None, task) + None, deployment) self.assertTrue(result.is_valid, result.msg) result = validator({"args": {"test_names": ["tempest.api.j", "e"]}}, - None, task) + None, deployment) self.assertFalse(result.is_valid, result.msg) @mock.patch("rally.benchmark.validation.tempest.Tempest") - @mock.patch("rally.objects.task.db.task_create") - def test_tempest_tests_exists_tempest_installation_failed( - self, mock_create, mock_tempest): - mock_create.return_value = { - 'status': 'init', - 'deployment_uuid': 'deployment-uuid', - 'verification_log': '', - 'uuid': 'task-uuid', - 'created_at': '', - 'failed': False, - 'tag': '', - 'id': 42} + def test_tempest_tests_exists_tempest_installation_failed(self, + mock_tempest): mock_tempest().is_installed.return_value = False mock_tempest().install.side_effect = tempest.TempestSetupFailure - task = objects.Task(deployment_uuid='deployment-uuid') + deployment = {"uuid": "someuuid"} validator = self._unwrap_validator(validation.tempest_tests_exists) - result = validator({"args": {"test_name": "a"}}, None, task) + result = validator({"args": {"test_name": "a"}}, None, deployment) self.assertFalse(result.is_valid, result.msg) mock_tempest().is_installed.assert_called_once_with() @@ -418,81 +398,53 @@ class ValidatorsTestCase(test.TestCase): None, None) self.assertTrue(result.is_valid, result.msg) - @mock.patch("rally.benchmark.validation.objects.Deployment.get") - def test_required_openstack_with_admin(self, mock_deploy_get): + def test_required_openstack_with_admin(self): validator = self._unwrap_validator(validation.required_openstack, admin=True) # admin presented in deployment - task = {"deployment_uuid": mock.MagicMock()} - mock_deploy_get.return_value = {"admin": "admin_endpoint", "users": []} - self.assertTrue(validator(None, None, task).is_valid) - mock_deploy_get.assert_called_once_with(task["deployment_uuid"]) - - mock_deploy_get.reset_mock() + fake_deployment = {"admin": "admin_endpoint", "users": []} + self.assertTrue(validator(None, None, fake_deployment).is_valid) # admin not presented in deployment - mock_deploy_get.return_value = {"admin": None, "users": ["u1", "h2"]} - self.assertFalse(validator(None, None, task).is_valid) - mock_deploy_get.assert_called_once_with(task["deployment_uuid"]) + fake_deployment = {"admin": None, "users": ["u1", "h2"]} + self.assertFalse(validator(None, None, fake_deployment).is_valid) - @mock.patch("rally.benchmark.validation.objects.Deployment.get") - def test_required_openstack_with_users(self, mock_deploy_get): + def test_required_openstack_with_users(self): validator = self._unwrap_validator(validation.required_openstack, users=True) # users presented in deployment - task = {"deployment_uuid": mock.MagicMock()} - mock_deploy_get.return_value = {"admin": None, "users": ["u_endpoint"]} - self.assertTrue(validator({}, None, task).is_valid) - mock_deploy_get.assert_called_once_with(task["deployment_uuid"]) - - mock_deploy_get.reset_mock() + fake_deployment = {"admin": None, "users": ["u_endpoint"]} + self.assertTrue(validator({}, None, fake_deployment).is_valid) # admin and users presented in deployment - mock_deploy_get.return_value = {"admin": "a", "users": ["u1", "h2"]} - self.assertTrue(validator({}, None, task).is_valid) - mock_deploy_get.assert_called_once_with(task["deployment_uuid"]) - - mock_deploy_get.reset_mock() + fake_deployment = {"admin": "a", "users": ["u1", "h2"]} + self.assertTrue(validator({}, None, fake_deployment).is_valid) # admin and user context - mock_deploy_get.return_value = {"admin": "a", "users": []} + fake_deployment = {"admin": "a", "users": []} context = {"context": {"users": True}} - self.assertTrue(validator(context, None, task).is_valid) - mock_deploy_get.assert_called_once_with(task["deployment_uuid"]) - - mock_deploy_get.reset_mock() + self.assertTrue(validator(context, None, fake_deployment).is_valid) # just admin presented - mock_deploy_get.return_value = {"admin": "a", "users": []} - self.assertFalse(validator({}, None, task).is_valid) - mock_deploy_get.assert_called_once_with(task["deployment_uuid"]) + fake_deployment = {"admin": "a", "users": []} + self.assertFalse(validator({}, None, fake_deployment).is_valid) - @mock.patch("rally.benchmark.validation.objects.Deployment.get") - def test_required_openstack_with_admin_and_users(self, mock_deploy_get): + def test_required_openstack_with_admin_and_users(self): validator = self._unwrap_validator(validation.required_openstack, admin=True, users=True) - task = {"deployment_uuid": mock.MagicMock()} + fake_deployment = {"admin": "a", "users": []} + self.assertFalse(validator({}, None, fake_deployment).is_valid) - mock_deploy_get.return_value = {"admin": "a", "users": []} - self.assertFalse(validator({}, None, task).is_valid) - mock_deploy_get.assert_called_once_with(task["deployment_uuid"]) - - mock_deploy_get.reset_mock() - - mock_deploy_get.return_value = {"admin": "a", "users": ["u"]} - self.assertTrue(validator({}, None, task).is_valid) - mock_deploy_get.assert_called_once_with(task["deployment_uuid"]) - - mock_deploy_get.reset_mock() + fake_deployment = {"admin": "a", "users": ["u"]} + self.assertTrue(validator({}, None, fake_deployment).is_valid) # admin and user context - mock_deploy_get.return_value = {"admin": "a", "users": []} + fake_deployment = {"admin": "a", "users": []} context = {"context": {"users": True}} - self.assertTrue(validator(context, None, task).is_valid) - mock_deploy_get.assert_called_once_with(task["deployment_uuid"]) + self.assertTrue(validator(context, None, fake_deployment).is_valid) def test_required_openstack_invalid(self): validator = self._unwrap_validator(validation.required_openstack) @@ -502,39 +454,34 @@ class ValidatorsTestCase(test.TestCase): validator = self._unwrap_validator(validation.volume_type_exists, param_name="volume_type") - task = {"deployment_uuid": mock.MagicMock()} - clients = mock.MagicMock() clients.cinder().volume_type.list.return_value = [] context = {"args": {"volume_type": False}} - result = validator(context, clients, task) + result = validator(context, clients, mock.MagicMock()) self.assertTrue(result.is_valid, result.msg) def test_volume_type_exists_check_types(self): validator = self._unwrap_validator(validation.volume_type_exists, param_name="volume_type") - task = {"deployment_uuid": mock.MagicMock()} clients = mock.MagicMock() clients.cinder().volume_types.list.return_value = ["type"] context = {"args": {"volume_type": True}} - result = validator(context, clients, task) + result = validator(context, clients, mock.MagicMock()) self.assertTrue(result.is_valid, result.msg) def test_volume_type_exists_check_types_no_types_exist(self): validator = self._unwrap_validator(validation.volume_type_exists, param_name="volume_type") - task = {"deployment_uuid": mock.MagicMock()} - clients = mock.MagicMock() clients().cinder().volume_type.list.return_value = [] context = {"args": {"volume_type": True}} - result = validator(context, clients, task) + result = validator(context, clients, mock.MagicMock()) self.assertFalse(result.is_valid, result.msg) \ No newline at end of file diff --git a/tests/unit/fakes.py b/tests/unit/fakes.py index 5c62de8b6c..200b73e711 100644 --- a/tests/unit/fakes.py +++ b/tests/unit/fakes.py @@ -28,7 +28,7 @@ import six from rally.benchmark.context import base as base_ctx from rally.benchmark.scenarios import base -from rally.objects import endpoint +from rally import objects from rally import utils as rally_utils @@ -1199,7 +1199,7 @@ class FakeClients(object): self._ceilometer = None self._zaqar = None self._trove = None - self._endpoint = endpoint_ or endpoint.Endpoint( + self._endpoint = endpoint_ or objects.Endpoint( "http://fake.example.org:5000/v2.0/", "fake_username", "fake_password", @@ -1339,11 +1339,11 @@ class FakeUserContext(FakeContext): admin = { "id": "adminuuid", - "endpoint": endpoint.Endpoint("aurl", "aname", "apwd", "atenant") + "endpoint": objects.Endpoint("aurl", "aname", "apwd", "atenant") } user = { "id": "uuid", - "endpoint": endpoint.Endpoint("url", "name", "pwd", "tenant"), + "endpoint": objects.Endpoint("url", "name", "pwd", "tenant"), "tenant_id": "uuid" } tenants = {"uuid": {"name": "tenant"}} diff --git a/tests/unit/test_hacking.py b/tests/unit/test_hacking.py index 75b51e3083..5324b0a276 100644 --- a/tests/unit/test_hacking.py +++ b/tests/unit/test_hacking.py @@ -62,24 +62,24 @@ class HackingTestCase(test.TestCase): self.assertTrue(actual_msg.startswith('N303')) def test_check_wrong_logging_import(self): - fake_imports = ["from rally.openstack.common import log", - "import rally.openstack.common.log" - "import logging"] + bad_imports = ["from rally.openstack.common import log", + "import rally.openstack.common.log", + "import logging"] good_imports = ["from rally import log", "from rally.log", "import rally.log"] - for fake_import in fake_imports: - checkres = checks.check_import_of_logging(fake_import, "fakefile") + for bad_import in bad_imports: + checkres = checks.check_import_of_logging(bad_import, "fakefile") self.assertIsNotNone(next(checkres)) - for fake_import in fake_imports: - checkres = checks.check_import_of_logging(fake_import, + for bad_import in bad_imports: + checkres = checks.check_import_of_logging(bad_import, "./rally/log.py") self.assertEqual([], list(checkres)) - for fake_import in good_imports: - checkres = checks.check_import_of_logging(fake_import, + for good_import in good_imports: + checkres = checks.check_import_of_logging(good_import, "fakefile") self.assertEqual([], list(checkres)) @@ -258,3 +258,24 @@ class HackingTestCase(test.TestCase): self.assertEqual(len(list(checks.assert_equal_in( "self.assertEqual(False, any(a==1 for a in b))"))), 0) + + def test_check_no_direct_rally_objects_import(self): + + bad_imports = ["from rally.objects import task", + "import rally.objects.task"] + + good_import = "from rally import objects" + + for bad_import in bad_imports: + checkres = checks.check_no_direct_rally_objects_import(bad_import, + "fakefile") + self.assertIsNotNone(next(checkres)) + + for bad_import in bad_imports: + checkres = checks.check_no_direct_rally_objects_import( + bad_import, "./rally/objects/__init__.py") + self.assertEqual([], list(checkres)) + + checkres = checks.check_no_direct_rally_objects_import(good_import, + "fakefile") + self.assertEqual([], list(checkres)) diff --git a/tests/unit/test_osclients.py b/tests/unit/test_osclients.py index d30d4e863e..e262529321 100644 --- a/tests/unit/test_osclients.py +++ b/tests/unit/test_osclients.py @@ -21,7 +21,7 @@ from oslo.config import cfg from rally import consts from rally import exceptions -from rally.objects import endpoint +from rally import objects from rally import osclients from tests.unit import fakes from tests.unit import test @@ -31,10 +31,10 @@ class OSClientsTestCase(test.TestCase): def setUp(self): super(OSClientsTestCase, self).setUp() - self.endpoint = endpoint.Endpoint("http://auth_url", "use", "pass", - "tenant") - self.endpoint_https = endpoint.Endpoint("https://auth_url/v2.0/admin", - "use", "pass", "tenant") + self.endpoint = objects.Endpoint("http://auth_url", "use", "pass", + "tenant") + self.endpoint_https = objects.Endpoint("https://auth_url/v2.0/admin", + "use", "pass", "tenant") self.clients = osclients.Clients(self.endpoint) self.clients_https = osclients.Clients(self.endpoint_https)