From 54dbf08b86730adafb5f3f7c0ce148f5556419d9 Mon Sep 17 00:00:00 2001 From: Tzanetos Balitsaris Date: Fri, 8 Aug 2014 20:40:36 +0300 Subject: [PATCH] Modify config semantic validation of benchmark engine Pass to validation the full benchmark configuration instead of extracting parts of it (ex. passing only args). Change-Id: I8f4cf3e2e8a0392612a0d0828a9c3bd6ab3da90f --- rally/benchmark/engine.py | 5 +- rally/benchmark/scenarios/base.py | 10 +- rally/benchmark/validation.py | 63 ++++++----- rally/exceptions.py | 3 +- tests/benchmark/scenarios/test_base.py | 6 +- tests/benchmark/test_engine.py | 3 +- tests/benchmark/test_validation.py | 146 ++++++++++++++----------- 7 files changed, 128 insertions(+), 108 deletions(-) diff --git a/rally/benchmark/engine.py b/rally/benchmark/engine.py index 751d87798f..c42dff221b 100644 --- a/rally/benchmark/engine.py +++ b/rally/benchmark/engine.py @@ -125,17 +125,16 @@ class BenchmarkEngine(object): def _validate_config_semantic_helper(self, admin, user, name, pos, task, kwargs): - args = {} if not kwargs else kwargs.get("args", {}) context = {} if not kwargs else kwargs.get("context", {}) try: - base_scenario.Scenario.validate(name, args, admin=admin, + base_scenario.Scenario.validate(name, kwargs, admin=admin, users=[user], task=task) base_ctx.ContextManager.validate_semantic(context, admin=admin, users=[user], task=task) except exceptions.InvalidScenarioArgument as e: kw = {"name": name, "pos": pos, - "args": args, "reason": six.text_type(e)} + "config": kwargs, "reason": six.text_type(e)} raise exceptions.InvalidBenchmarkConfig(**kw) @rutils.log_task_wrapper(LOG.info, _("Task validation of semantic.")) diff --git a/rally/benchmark/scenarios/base.py b/rally/benchmark/scenarios/base.py index 4ec8662117..e9051b3826 100644 --- a/rally/benchmark/scenarios/base.py +++ b/rally/benchmark/scenarios/base.py @@ -99,14 +99,14 @@ class Scenario(object): return benchmark_scenarios_flattened @staticmethod - def _validate_helper(validators, clients, args, task): + def _validate_helper(validators, clients, config, task): for validator in validators: - result = validator(clients=clients, task=task, **args) + result = validator(config, clients=clients, task=task) if not result.is_valid: raise exceptions.InvalidScenarioArgument(message=result.msg) @classmethod - def validate(cls, name, args, admin=None, users=None, task=None): + def validate(cls, name, config, admin=None, users=None, task=None): """Semantic check of benchmark arguments.""" validators = cls.meta(name, "validators", default=[]) @@ -121,10 +121,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, args, task) + cls._validate_helper(admin_validators, admin, config, task) if users: for user in users: - cls._validate_helper(user_validators, user, args, task) + cls._validate_helper(user_validators, user, config, task) @staticmethod def meta(cls, attr_name, method_name=None, default=None): diff --git a/rally/benchmark/validation.py b/rally/benchmark/validation.py index 03f9d2abc4..3d25f13644 100644 --- a/rally/benchmark/validation.py +++ b/rally/benchmark/validation.py @@ -41,10 +41,17 @@ def validator(fn): :returns: rally scenario validator """ def wrap_given(*args, **kwargs): - def wrap_validator(**options): - options.update({"args": args, "kwargs": kwargs}) + """Dynamic validation decorator for scenario. + + :param args: the arguments of the decorator of the benchmark scenario + ex. @my_decorator("arg1"), then args = ('arg1',) + :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): # NOTE(amaretskiy): validator is successful by default - return fn(*args, **options) or ValidationResult() + return (fn(config, clients, task, *args, **kwargs) or + ValidationResult()) def wrap_scenario(scenario): # NOTE(amaretskiy): user permission by default @@ -95,13 +102,13 @@ def number(param_name=None, minval=None, maxval=None, nullable=False, :param integer_only: Only accept integers """ - def number_validator(**kwargs): + def number_validator(config, clients, task): num_func = float if integer_only: num_func = int - val = kwargs.get(param_name, None) + val = config.get("args", {}).get(param_name) # None may be valid if the scenario sets a sensible default. if nullable and val is None: @@ -153,8 +160,8 @@ def file_exists(param_name, mode=os.R_OK): mode=os.R_OK+os.W_OK """ - def file_exists_validator(**kwargs): - file_name = kwargs.get(param_name) + def file_exists_validator(config, clients, task): + file_name = config.get("args", {}).get(param_name) if os.access(file_name, mode): return ValidationResult() else: @@ -173,11 +180,10 @@ def image_exists(param_name): :param param_name: defines which variable should be used to get image id value. """ - def image_exists_validator(**kwargs): - clients = kwargs.get('clients') + def image_exists_validator(config, clients, task): image_id = types.ImageResourceType.transform( clients=clients, - resource_config=kwargs.get(param_name)) + resource_config=config.get("args", {}).get(param_name)) try: clients.glance().images.get(image=image_id) return ValidationResult() @@ -193,11 +199,10 @@ def flavor_exists(param_name): :param param_name: defines which variable should be used to get flavor id value. """ - def flavor_exists_validator(**kwargs): - clients = kwargs.get('clients') + def flavor_exists_validator(config, clients, task): flavor_id = types.FlavorResourceType.transform( clients=clients, - resource_config=kwargs.get(param_name)) + resource_config=config.get("args", {}).get(param_name)) try: clients.nova().flavors.get(flavor=flavor_id) return ValidationResult() @@ -216,12 +221,10 @@ def image_valid_on_flavor(flavor_name, image_name): to get image id value. """ - def image_valid_on_flavor_validator(**kwargs): - clients = kwargs.get('clients') - + def image_valid_on_flavor_validator(config, clients, task): flavor_id = types.FlavorResourceType.transform( clients=clients, - resource_config=kwargs.get(flavor_name)) + resource_config=config.get("args", {}).get(flavor_name)) try: flavor = clients.nova().flavors.get(flavor=flavor_id) except nova_exc.NotFound: @@ -230,7 +233,7 @@ def image_valid_on_flavor(flavor_name, image_name): image_id = types.ImageResourceType.transform( clients=clients, - resource_config=kwargs.get(image_name)) + resource_config=config.get("args", {}).get(image_name)) try: image = clients.glance().images.get(image=image_id) except glance_exc.HTTPNotFound: @@ -257,11 +260,11 @@ def image_valid_on_flavor(flavor_name, image_name): def network_exists(network_name): - def network_exists_validator(**kwargs): - network = kwargs.get(network_name, "private") + def network_exists_validator(config, clients, task): + network = config.get("args", {}).get(network_name, "private") networks = [net.label for net in - kwargs["clients"].nova().networks.list()] + clients.nova().networks.list()] if network not in networks: message = _("Network with name %(network)s not found. " "Available networks: %(networks)s") % { @@ -275,14 +278,14 @@ def network_exists(network_name): def external_network_exists(network_name, use_external_network): - def external_network_exists_validator(**kwargs): - if not kwargs.get(use_external_network, True): + def external_network_exists_validator(config, clients, task): + if not config.get("args", {}).get(use_external_network, True): return ValidationResult() - ext_network = kwargs.get(network_name, "public") + ext_network = config.get("args", {}).get(network_name, "public") networks = [net.name for net in - kwargs["clients"].nova().floating_ip_pools.list()] + clients.nova().floating_ip_pools.list()] if ext_network not in networks: message = _("External (floating) network with name %(network)s " "not found. " @@ -345,8 +348,8 @@ def required_parameters(params): :param params: list of required parameters """ - def required_parameters_validator(**kwargs): - missing = set(params) - set(kwargs) + def required_parameters_validator(config, clients, task): + missing = set(params) - set(config.get("args", {})) if missing: message = _("%s parameters are not defined in " "the benchmark config file") % ", ".join(missing) @@ -356,13 +359,13 @@ def required_parameters(params): @validator -def required_services(*args, **kwargs): +def required_services(config, clients, task, *required_services): """Check if specified services are available. :param args: list of servives names """ - available_services = kwargs.get("clients").services().values() - for service in args: + available_services = clients.services().values() + for service in required_services: if service not in consts.Service: return ValidationResult(False, _("Unknown service: %s") % service) if service not in available_services: diff --git a/rally/exceptions.py b/rally/exceptions.py index 2c71f12617..0e68ea6934 100644 --- a/rally/exceptions.py +++ b/rally/exceptions.py @@ -110,7 +110,8 @@ class InvalidBenchmarkConfig(InvalidTaskException): msg_fmt = _("Task config is invalid.\n" "\tBenchmark %(name)s has wrong configuration at" " position %(pos)s" - "\n\tReason: %(reason)s") + "\n\tReason: %(reason)s" + "\n\tBenchmark configuration: %(config)s") class TestException(RallyException): diff --git a/tests/benchmark/scenarios/test_base.py b/tests/benchmark/scenarios/test_base.py index 74cdb8286b..43b32aaa67 100644 --- a/tests/benchmark/scenarios/test_base.py +++ b/tests/benchmark/scenarios/test_base.py @@ -50,12 +50,12 @@ class ScenarioTestCase(test.TestCase): mock.MagicMock(return_value=validation.ValidationResult()) ] clients = mock.MagicMock() - args = {"a": 1, "b": 2} + config = {"a": 1, "b": 2} task = mock.MagicMock() scenario_base.Scenario._validate_helper(validators, clients, - args, task) + config, task) for validator in validators: - validator.assert_called_with(clients=clients, task=task, **args) + validator.assert_called_with(config, clients=clients, task=task) def test__validate_helper__no_valid(self): validators = [ diff --git a/tests/benchmark/test_engine.py b/tests/benchmark/test_engine.py index a59fc8efd5..6b5d9d486b 100644 --- a/tests/benchmark/test_engine.py +++ b/tests/benchmark/test_engine.py @@ -163,8 +163,7 @@ class BenchmarkEngineTestCase(test.TestCase): eng._validate_config_semantic_helper("admin", "user", "name", "pos", task, {"args": "args"}) mock_validate.assert_called_once_with( - "name", "args", admin="admin", users=["user"], - task=task) + "name", {"args": "args"}, admin="admin", users=["user"], task=task) @mock.patch("rally.benchmark.engine.base_scenario.Scenario.validate") def test__validate_config_semanitc_helper_invalid_arg(self, mock_validate): diff --git a/tests/benchmark/test_validation.py b/tests/benchmark/test_validation.py index 7d24e4d3d2..668143bead 100644 --- a/tests/benchmark/test_validation.py +++ b/tests/benchmark/test_validation.py @@ -56,8 +56,7 @@ class ValidationUtilsTestCase(test.TestCase): validators = wrap_validator.validators self.assertEqual(1, len(validators)) validator, = validators - self.assertEqual({"args": tuple(wrap_args), "kwargs": wrap_kwargs}, - validator()) + self.assertEqual(wrap_kwargs, validator(None, None, None)) self.assertEqual(wrap_validator, scenario) # Default permission @@ -74,12 +73,12 @@ class ValidationUtilsTestCase(test.TestCase): # Default result func_success = lambda *a, **kv: None validator, = self._get_scenario_validators(func_success, scenario) - self.assertTrue(validator().is_valid) + self.assertTrue(validator(None, None, None).is_valid) # Failure result func_failure = lambda *a, **kv: failure validator, = self._get_scenario_validators(func_failure, scenario) - self.assertFalse(validator().is_valid) + self.assertFalse(validator(None, None, None).is_valid) def test_required_services(self): available_services = { @@ -94,7 +93,7 @@ class ValidationUtilsTestCase(test.TestCase): required_services = (lambda *services: validation.required_services(*services) (lambda: None).validators.pop() - (clients=clients)) + (None, clients, None)) # Services are available result = required_services(consts.Service.KEYSTONE) @@ -138,76 +137,94 @@ class ValidationUtilsTestCase(test.TestCase): def test_number_invalid(self): validator = validation.number('param', 0, 10, nullable=False) - result = validator(param=-1) + config = {"args": {"param": -1}} + result = validator(config, None, None) self.assertFalse(result.is_valid) - result = validator(param=11) + config = {"args": {"param": 11}} + result = validator(config, None, None) self.assertFalse(result.is_valid) - result = validator(param="not an int") + config = {"args": {"param": "not an int"}} + result = validator(config, None, None) self.assertFalse(result.is_valid) - result = validator(param=None) + config = {"args": {"param": None}} + result = validator(config, None, None) self.assertFalse(result.is_valid) - result = validator() + config = {"args": {}} + result = validator(config, None, None) self.assertFalse(result.is_valid) - result = validator(param=-0.1) + config = {"args": {"param": -0.1}} + result = validator(config, None, None) self.assertFalse(result.is_valid) def test_number_integer_only(self): validator = validation.number('param', 0, 10, nullable=False, integer_only=True) - result = validator(param="5.0") + config = {"args": {"param": "5.0"}} + result = validator(config, None, None) self.assertFalse(result.is_valid) validator = validation.number('param', 0, 10, nullable=False, integer_only=False) - result = validator(param="5.0") + config = {"args": {"param": "5.0"}} + result = validator(config, None, None) self.assertTrue(result.is_valid) def test_number_valid(self): validator = validation.number('param', 0, 10, nullable=False) - result = validator(param=0) + config = {"args": {"param": 0}} + result = validator(config, None, None) self.assertTrue(result.is_valid) - result = validator(param=10) + config = {"args": {"param": 10}} + result = validator(config, None, None) self.assertTrue(result.is_valid) - result = validator(param=10.0) + config = {"args": {"param": 10.0}} + result = validator(config, None, None) self.assertTrue(result.is_valid) - result = validator(param=5.6) + config = {"args": {"param": 5.6}} + result = validator(config, None, None) self.assertTrue(result.is_valid) def test_number_nullable(self): validator = validation.number('param', 0, 10, nullable=True) - result = validator(param=None) + config = {"args": {"param": None}} + result = validator(config, None, None) self.assertTrue(result.is_valid) - result = validator() + config = {"args": {}} + result = validator(config, None, None) self.assertTrue(result.is_valid) - result = validator(param=-1) + config = {"args": {"param": -1}} + result = validator(config, None, None) self.assertFalse(result.is_valid) - result = validator(param=0) + config = {"args": {"param": 0}} + result = validator(config, None, None) self.assertTrue(result.is_valid) @mock.patch("os.access") def test_file_exists(self, mock_access): validator = validation.file_exists('param') - result = validator(param='/tmp/foo') + config = {"args": {"param": "/tmp/foo"}} + result = validator(config, None, None) mock_access.assert_called_once_with('/tmp/foo', os.R_OK) self.assertTrue(result.is_valid) @mock.patch("os.access", return_value=False) def test_file_exists_negative(self, mock_access): validator = validation.file_exists('param') - result = validator(param='/tmp/bah') + config = {"args": {"param": "/tmp/bah"}} + result = validator(config, None, None) mock_access.assert_called_with('/tmp/bah', os.R_OK) self.assertFalse(result.is_valid) @@ -219,8 +236,8 @@ class ValidationUtilsTestCase(test.TestCase): validator = validation.image_exists("image") test_img_id = "test_image_id" resource = {"id": test_img_id} - result = validator(clients=mock_osclients, - image=resource) + config = {"args": {"image": resource}} + result = validator(config, clients=mock_osclients, task=None) fakegclient.images.get.assert_called_once_with(image=test_img_id) self.assertTrue(result.is_valid) self.assertIsNone(result.msg) @@ -234,8 +251,8 @@ class ValidationUtilsTestCase(test.TestCase): validator = validation.image_exists("image") test_img_id = "test_image_id" resource = {"id": test_img_id} - result = validator(clients=mock_osclients, - image=resource) + config = {"args": {"image": resource}} + result = validator(config, clients=mock_osclients, task=None) fakegclient.images.get.assert_called_once_with(image=test_img_id) self.assertFalse(result.is_valid) self.assertIsNotNone(result.msg) @@ -248,8 +265,8 @@ class ValidationUtilsTestCase(test.TestCase): validator = validation.flavor_exists("flavor") test_flavor_id = 1 resource = {"id": test_flavor_id} - result = validator(clients=mock_osclients, - flavor=resource) + config = {"args": {"flavor": resource}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.flavors.get.assert_called_once_with(flavor=test_flavor_id) self.assertTrue(result.is_valid) self.assertIsNone(result.msg) @@ -263,8 +280,8 @@ class ValidationUtilsTestCase(test.TestCase): validator = validation.flavor_exists("flavor") test_flavor_id = 101 resource = {"id": test_flavor_id} - result = validator(clients=mock_osclients, - flavor=resource) + config = {"args": {"flavor": resource}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.flavors.get.assert_called_once_with(flavor=test_flavor_id) self.assertFalse(result.is_valid) self.assertIsNotNone(result.msg) @@ -288,9 +305,9 @@ class ValidationUtilsTestCase(test.TestCase): validator = validation.image_valid_on_flavor("flavor", "image") - result = validator(clients=mock_osclients, - flavor={"id": flavor.id}, - image={"id": image.id}) + config = {"args": {"image": {"id": image.id}, + "flavor": {"id": flavor.id}}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.flavors.get.assert_called_once_with(flavor=flavor.id) fakegclient.images.get.assert_called_once_with(image=image.id) @@ -317,9 +334,9 @@ class ValidationUtilsTestCase(test.TestCase): validator = validation.image_valid_on_flavor("flavor", "image") - result = validator(clients=mock_osclients, - flavor={"id": flavor.id}, - image={"id": image.id}) + config = {"args": {"image": {"id": image.id}, + "flavor": {"id": flavor.id}}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.flavors.get.assert_called_once_with(flavor=flavor.id) fakegclient.images.get.assert_called_once_with(image=image.id) @@ -346,9 +363,9 @@ class ValidationUtilsTestCase(test.TestCase): validator = validation.image_valid_on_flavor("flavor", "image") - result = validator(clients=mock_osclients, - flavor={"id": flavor.id}, - image={"id": image.id}) + config = {"args": {"image": {"id": image.id}, + "flavor": {"id": flavor.id}}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.flavors.get.assert_called_once_with(flavor=flavor.id) fakegclient.images.get.assert_called_once_with(image=image.id) @@ -376,9 +393,9 @@ class ValidationUtilsTestCase(test.TestCase): validator = validation.image_valid_on_flavor("flavor", "image") - result = validator(clients=mock_osclients, - flavor={"id": flavor.id}, - image={"id": image.id}) + config = {"args": {"image": {"id": image.id}, + "flavor": {"id": flavor.id}}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.flavors.get.assert_called_once_with(flavor=flavor.id) fakegclient.images.get.assert_called_once_with(image=image.id) @@ -405,9 +422,9 @@ class ValidationUtilsTestCase(test.TestCase): validator = validation.image_valid_on_flavor("flavor", "image") - result = validator(clients=mock_osclients, - flavor={"id": flavor.id}, - image={"id": image.id}) + config = {"args": {"image": {"id": image.id}, + "flavor": {"id": flavor.id}}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.flavors.get.assert_called_once_with(flavor=flavor.id) fakegclient.images.get.assert_called_once_with(image=image.id) @@ -430,9 +447,9 @@ class ValidationUtilsTestCase(test.TestCase): test_img_id = "test_image_id" - result = validator(clients=mock_osclients, - flavor={"id": flavor.id}, - image={"id": test_img_id}) + config = {"args": {"flavor": {"id": flavor.id}, + "image": {"id": test_img_id}}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.flavors.get.assert_called_once_with(flavor=flavor.id) fakegclient.images.get.assert_called_once_with(image=test_img_id) @@ -454,8 +471,8 @@ class ValidationUtilsTestCase(test.TestCase): network_name = "private" - result = validator(clients=mock_osclients, - fixed_network=network_name) + config = {"args": {"fixed_network": network_name}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.networks.list.assert_called_once_with() self.assertTrue(result.is_valid) @@ -476,8 +493,8 @@ class ValidationUtilsTestCase(test.TestCase): network_name = "foo" - result = validator(clients=mock_osclients, - fixed_network=network_name) + config = {"args": {"fixed_network": network_name}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.networks.list.assert_called_once_with() self.assertFalse(result.is_valid) @@ -499,8 +516,8 @@ class ValidationUtilsTestCase(test.TestCase): network_name = "floating" - result = validator(clients=mock_osclients, - floating_network=network_name) + config = {"args": {"floating_network": network_name}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.floating_ip_pools.list.assert_called_once_with() self.assertTrue(result.is_valid) @@ -520,9 +537,9 @@ class ValidationUtilsTestCase(test.TestCase): network_name = "not_used" - result = validator(clients=mock_osclients, - floating_network=network_name, - use_floatingip=False) + config = {"args": {"floating_network": network_name, + "use_floatingip": False}} + result = validator(config, clients=mock_osclients, task=None) self.assertFalse(fakenclient.floating_ip_pools.list.called) self.assertTrue(result.is_valid) @@ -542,8 +559,8 @@ class ValidationUtilsTestCase(test.TestCase): network_name = "foo" - result = validator(clients=mock_osclients, - floating_network=network_name) + config = {"args": {"floating_network": network_name}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.floating_ip_pools.list.assert_called_once_with() self.assertFalse(result.is_valid) @@ -566,9 +583,9 @@ class ValidationUtilsTestCase(test.TestCase): test_img_id = "test_image_id" test_flavor_id = 101 - result = validator(clients=mock_osclients, - flavor={"id": test_flavor_id}, - image={"id": test_img_id}) + config = {"args": {"flavor": {"id": test_flavor_id}, + "image": {"id": test_img_id}}} + result = validator(config, clients=mock_osclients, task=None) fakenclient.flavors.get.assert_called_once_with(flavor=test_flavor_id) @@ -700,5 +717,6 @@ class ValidationUtilsTestCase(test.TestCase): def test_required_parameters(self): validator = validation.required_parameters("test1") - result = validator(task=mock.MagicMock()) + config = {"args": {}} + result = validator(config, clients=None, task=mock.MagicMock()) self.assertFalse(result.is_valid)