From ddd0395bbbebfe84c50d769e921963762e6cba3c Mon Sep 17 00:00:00 2001 From: Anton Studenov Date: Fri, 17 Feb 2017 14:47:56 +0300 Subject: [PATCH] [validation] validate scenario default_context This patch adds validation of default contexts of scenarios added to task. * Added hidden field to plugin meta of all plugins. * `rally plugin list` now does not contain hidden plugins. Change-Id: I1becb93aea095e467779dbf4278b5b2ecfefdf56 --- rally/common/plugin/plugin.py | 37 +++++++++++++++---- .../plugins/openstack/context/api_versions.py | 5 +-- .../openstack/context/cleanup/admin.py | 4 +- .../plugins/openstack/context/cleanup/user.py | 4 +- rally/task/context.py | 20 ++++------ rally/task/engine.py | 14 +++++-- rally/verification/context.py | 23 +++++------- tests/unit/common/plugin/test_plugin.py | 19 ++++++++++ .../openstack/context/test_api_versions.py | 30 +++++++-------- tests/unit/task/test_context.py | 27 ++++++-------- tests/unit/task/test_engine.py | 24 +++++++++--- tests/unit/task/test_scenario.py | 3 +- tests/unit/verification/test_context.py | 13 +++---- 13 files changed, 135 insertions(+), 88 deletions(-) diff --git a/rally/common/plugin/plugin.py b/rally/common/plugin/plugin.py index ce12510c65..ef874dd78b 100644 --- a/rally/common/plugin/plugin.py +++ b/rally/common/plugin/plugin.py @@ -59,15 +59,18 @@ def base(): return wrapper -def configure(name, namespace="default"): +def configure(name, namespace="default", hidden=False): """Use this decorator to configure plugin's attributes. :param name: name of plugin that is used for searching purpose :param namespace: plugin namespace + :param hidden: if True the plugin will be marked as hidden and can be + loaded only explicitly """ def decorator(plugin): plugin._configure(name, namespace) + plugin._meta_set("hidden", hidden) return plugin return decorator @@ -204,7 +207,7 @@ class Plugin(meta.MetaMixin, info.InfoMixin): return cls @classmethod - def get(cls, name, namespace=None): + def get(cls, name, namespace=None, allow_hidden=False): """Return plugin by its name from specified namespace. This method iterates over all subclasses of cls and returns plugin @@ -215,15 +218,20 @@ class Plugin(meta.MetaMixin, info.InfoMixin): :param name: Plugin's name :param namespace: Namespace where to search for plugins + :param allow_hidden: if False and found plugin is hidden then + PluginNotFound will be raised """ potential_result = [] - for p in cls.get_all(namespace=namespace): + for p in cls.get_all(namespace=namespace, allow_hidden=True): if p.get_name() == name: potential_result.append(p) if len(potential_result) == 1: - return potential_result[0] + plugin = potential_result[0] + if allow_hidden or not plugin.is_hidden(): + return plugin + elif potential_result: hint = _LE("Try to choose the correct Plugin base or namespace to " "search in.") @@ -240,19 +248,27 @@ class Plugin(meta.MetaMixin, info.InfoMixin): name=name, namespace=namespace or "any of") @classmethod - def get_all(cls, namespace=None): + def get_all(cls, namespace=None, allow_hidden=False): """Return all subclass plugins of plugin. All plugins that are not configured will be ignored. :param namespace: return only plugins from specified namespace. + :param allow_hidden: if False return only non hidden plugins """ plugins = [] for p in discover.itersubclasses(cls): - if issubclass(p, Plugin) and p._meta_is_inited(raise_exc=False): - if not namespace or namespace == p.get_namespace(): - plugins.append(getattr(p, "func_ref", p)) + if not issubclass(p, Plugin): + continue + if not p._meta_is_inited(raise_exc=False): + continue + if namespace and namespace != p.get_namespace(): + continue + if not allow_hidden and p.is_hidden(): + continue + + plugins.append(getattr(p, "func_ref", p)) return plugins @@ -266,6 +282,11 @@ class Plugin(meta.MetaMixin, info.InfoMixin): """"Return namespace of plugin, e.g. default or openstack.""" return cls._meta_get("namespace") + @classmethod + def is_hidden(cls): + """Return True if plugin is hidden.""" + return cls._meta_get("hidden", False) + @classmethod def is_deprecated(cls): """Return deprecation details for deprecated plugins.""" diff --git a/rally/plugins/openstack/context/api_versions.py b/rally/plugins/openstack/context/api_versions.py index 2052afbf28..bebee5e00c 100644 --- a/rally/plugins/openstack/context/api_versions.py +++ b/rally/plugins/openstack/context/api_versions.py @@ -221,9 +221,8 @@ class OpenStackAPIVersions(context.Context): pass @classmethod - def validate(cls, config, non_hidden=False): - super(OpenStackAPIVersions, cls).validate(config, - non_hidden=non_hidden) + def validate(cls, config): + super(OpenStackAPIVersions, cls).validate(config) for client in config: client_cls = osclients.OSClient.get(client) if ("service_type" in config[client] and diff --git a/rally/plugins/openstack/context/cleanup/admin.py b/rally/plugins/openstack/context/cleanup/admin.py index b318e5467d..ad19a41e74 100644 --- a/rally/plugins/openstack/context/cleanup/admin.py +++ b/rally/plugins/openstack/context/cleanup/admin.py @@ -31,8 +31,8 @@ class AdminCleanup(base.CleanupMixin, context.Context): """Context class for admin resources cleanup.""" @classmethod - def validate(cls, config, non_hidden=False): - super(AdminCleanup, cls).validate(config, non_hidden) + def validate(cls, config): + super(AdminCleanup, cls).validate(config) missing = set(config) missing -= manager.list_resource_names(admin_required=True) diff --git a/rally/plugins/openstack/context/cleanup/user.py b/rally/plugins/openstack/context/cleanup/user.py index e8197adcdb..ef8a2269c8 100644 --- a/rally/plugins/openstack/context/cleanup/user.py +++ b/rally/plugins/openstack/context/cleanup/user.py @@ -31,8 +31,8 @@ class UserCleanup(base.CleanupMixin, context.Context): """Context class for user resources cleanup.""" @classmethod - def validate(cls, config, non_hidden=False): - super(UserCleanup, cls).validate(config, non_hidden) + def validate(cls, config): + super(UserCleanup, cls).validate(config) missing = set(config) missing -= manager.list_resource_names(admin_required=False) diff --git a/rally/task/context.py b/rally/task/context.py index 6cfeb185a2..490ddd31ba 100644 --- a/rally/task/context.py +++ b/rally/task/context.py @@ -21,7 +21,6 @@ import six from rally.common import logging from rally.common.plugin import plugin from rally.common import utils -from rally import exceptions from rally.task import functional LOG = logging.getLogger(__name__) @@ -42,9 +41,8 @@ def configure(name, order, hidden=False): task config """ def wrapper(cls): - cls = plugin.configure(name=name)(cls) + cls = plugin.configure(name=name, hidden=hidden)(cls) cls._meta_set("order", order) - cls._meta_set("hidden", hidden) return cls return wrapper @@ -102,12 +100,7 @@ class BaseContext(plugin.Plugin, functional.FunctionalMixin, return not self.__eq__(other) @classmethod - def validate(cls, config, non_hidden=False): - # TODO(boris-42): This is going to be replaced with common validation - # mechanism (generalization of scenario validation) - if non_hidden and cls._meta_get("hidden"): - raise exceptions.PluginNotFound(name=cls.get_name(), - namespace="context") + def validate(cls, config): jsonschema.validate(config, cls.CONFIG_SCHEMA) @classmethod @@ -161,13 +154,14 @@ class ContextManager(object): self.context_obj = context_obj @staticmethod - def validate(ctx, non_hidden=False): + def validate(ctx, allow_hidden=False): for name, config in ctx.items(): - Context.get(name).validate(config, non_hidden=non_hidden) + Context.get(name, allow_hidden=allow_hidden).validate(config) def _get_sorted_context_lst(self): - ctxlst = map(Context.get, self.context_obj["config"]) - return sorted(map(lambda ctx: ctx(self.context_obj), ctxlst)) + return sorted([ + Context.get(ctx_name, allow_hidden=True)(self.context_obj) + for ctx_name in self.context_obj["config"].keys()]) def setup(self): """Creates benchmark environment from config.""" diff --git a/rally/task/engine.py b/rally/task/engine.py index a487096a8d..99d584f9dc 100644 --- a/rally/task/engine.py +++ b/rally/task/engine.py @@ -284,10 +284,12 @@ class TaskEngine(object): def _validate_config_syntax(self, config): for subtask in config.subtasks: for pos, workload in enumerate(subtask.workloads): + scenario_context = self._get_defualt_context(workload.name) try: runner.ScenarioRunner.validate(workload.runner) - context.ContextManager.validate( - workload.context, non_hidden=True) + context.ContextManager.validate(workload.context) + context.ContextManager.validate(scenario_context, + allow_hidden=True) sla.SLA.validate(workload.sla) for hook_conf in workload.hooks: hook.Hook.validate(hook_conf) @@ -362,9 +364,13 @@ class TaskEngine(object): config = config or {"type": "serial"} return runner.ScenarioRunner.get(config["type"])(self.task, config) - def _prepare_context(self, ctx, name, credential): - scenario_context = copy.deepcopy( + @staticmethod + def _get_defualt_context(name): + return copy.deepcopy( scenario.Scenario.get(name)._meta_get("default_context")) + + def _prepare_context(self, ctx, name, credential): + scenario_context = self._get_defualt_context(name) if self.existing_users and "users" not in ctx: scenario_context.setdefault("existing_users", self.existing_users) elif "users" not in ctx: diff --git a/rally/verification/context.py b/rally/verification/context.py index f7b37d8461..a10dec948d 100644 --- a/rally/verification/context.py +++ b/rally/verification/context.py @@ -12,10 +12,13 @@ # License for the specific language governing permissions and limitations # under the License. +import functools + from rally.common.plugin import plugin from rally.task import context -configure = context.configure +# all VerifierContexts should be always hidden +configure = functools.partial(context.configure, hidden=True) @plugin.base() @@ -28,14 +31,7 @@ class VerifierContext(context.BaseContext): self.verifier = self.context["verifier"] @classmethod - def _meta_get(cls, key, default=None): - # It should be always hidden - if key == "hidden": - return True - return super(VerifierContext, cls)._meta_get(key, default) - - @classmethod - def validate(cls, config, non_hidden=False): + def validate(cls, config): # do not validate jsonschema. pass @@ -43,10 +39,11 @@ class VerifierContext(context.BaseContext): class ContextManager(context.ContextManager): @staticmethod - def validate(ctx, non_hidden=False): + def validate(ctx): for name, config in ctx.items(): - VerifierContext.get(name).validate(config, non_hidden=non_hidden) + VerifierContext.get(name, allow_hidden=True).validate(config) def _get_sorted_context_lst(self): - return sorted([VerifierContext.get(name)(self.context_obj) - for name in self.context_obj["config"].keys()]) + return sorted([ + VerifierContext.get(name, allow_hidden=True)(self.context_obj) + for name in self.context_obj["config"].keys()]) diff --git a/tests/unit/common/plugin/test_plugin.py b/tests/unit/common/plugin/test_plugin.py index 71f7455973..fc21264d8b 100644 --- a/tests/unit/common/plugin/test_plugin.py +++ b/tests/unit/common/plugin/test_plugin.py @@ -41,6 +41,7 @@ class PluginModuleTestCase(test.TestCase): return a self.assertEqual("configure_func_plugin_test", func.get_name()) + self.assertFalse(func.is_hidden()) self.assertEqual(42, func(42)) def test_deprecated_cls(self): @@ -148,6 +149,11 @@ class SomePlugin(BasePlugin): pass +@plugin.configure(name="test_hidden_plugin", hidden=True) +class HiddenPlugin(BasePlugin): + pass + + @plugin.deprecated("some_reason", "0.1.1") @plugin.configure(name="test_deprecated_plugin") class DeprecatedPlugin(BasePlugin): @@ -174,6 +180,15 @@ class PluginTestCase(test.TestCase): self.assertEqual(SomePlugin, BasePlugin.get("test_some_plugin")) + def test_get_hidden(self): + self.assertEqual(HiddenPlugin, + BasePlugin.get("test_hidden_plugin", + allow_hidden=True)) + + def test_get_hidden_not_found(self): + self.assertRaises(exceptions.PluginNotFound, + BasePlugin.get, "test_hidden_plugin") + def test_get_not_found(self): self.assertRaises(exceptions.PluginNotFound, BasePlugin.get, "non_existing") @@ -198,6 +213,10 @@ class PluginTestCase(test.TestCase): set(BasePlugin.get_all())) self.assertEqual([], SomePlugin.get_all()) + def test_get_all_hidden(self): + self.assertEqual(set([SomePlugin, DeprecatedPlugin, HiddenPlugin]), + set(BasePlugin.get_all(allow_hidden=True))) + def test_is_deprecated(self): self.assertFalse(SomePlugin.is_deprecated()) self.assertEqual(DeprecatedPlugin.is_deprecated(), diff --git a/tests/unit/plugins/openstack/context/test_api_versions.py b/tests/unit/plugins/openstack/context/test_api_versions.py index e935392d09..dcfa849f64 100644 --- a/tests/unit/plugins/openstack/context/test_api_versions.py +++ b/tests/unit/plugins/openstack/context/test_api_versions.py @@ -40,44 +40,44 @@ class OpenStackServicesTestCase(test.TestCase): }) def test_validate_wrong_configs(self): + # Non-existing clients should be caught self.assertRaises( exceptions.PluginNotFound, api_versions.OpenStackAPIVersions.validate, - {"invalid": {"service_type": "some_type"}}, - "Non-existing clients should be caught.") + {"invalid": {"service_type": "some_type"}}) + # Additional properties should be restricted self.assertRaises( jsonschema.ValidationError, api_versions.OpenStackAPIVersions.validate, - {"nova": {"some_key": "some_value"}}, - "Additional properties should be restricted.") + {"nova": {"some_key": "some_value"}}) + # Setting service_type is allowed only + # for those clients, which support it self.assertRaises( exceptions.ValidationError, api_versions.OpenStackAPIVersions.validate, - {"keystone": {"service_type": "identity"}}, - "Setting service_type is allowed only for those clients, which " - "support it.") + {"keystone": {"service_type": "identity"}}) + # Setting service_name is allowed only + # for those clients, which support it self.assertRaises( exceptions.ValidationError, api_versions.OpenStackAPIVersions.validate, - {"keystone": {"service_name": "keystone"}}, - "Setting service_name is allowed only for those clients, which " - "support it.") + {"keystone": {"service_name": "keystone"}}) + # Setting version is allowed only + # for those clients, which support it self.assertRaises( exceptions.ValidationError, api_versions.OpenStackAPIVersions.validate, - {"keystone": {"version": 1}}, - "Setting version is allowed only for those clients, which " - "support it.") + {"keystone": {"version": 1}}) + # Unsupported version should be caught self.assertRaises( exceptions.ValidationError, api_versions.OpenStackAPIVersions.validate, - {"nova": {"version": 666}}, - "Unsupported version should be caught.") + {"nova": {"version": 666}}) def test_setup_with_wrong_service_name(self): context = { diff --git a/tests/unit/task/test_context.py b/tests/unit/task/test_context.py index f15d49a6b1..e0d34ba234 100644 --- a/tests/unit/task/test_context.py +++ b/tests/unit/task/test_context.py @@ -69,12 +69,6 @@ class BaseContextTestCase(test.TestCase): self.assertRaises(jsonschema.ValidationError, fakes.FakeContext.validate, {"nonexisting": 2}) - def test_validate__hidden(self): - fakes.FakeHiddenContext.validate({"test": 2}) - self.assertRaises(exceptions.PluginNotFound, - fakes.FakeHiddenContext.validate, - {"test": 2}, non_hidden=True) - def test_setup_is_abstract(self): @context.configure("test_abstract_setup", 0) @@ -154,22 +148,22 @@ class ContextManagerTestCase(test.TestCase): context.ContextManager.validate(config) for ctx in ("ctx1", "ctx2"): mock_context_get.assert_has_calls([ - mock.call(ctx), - mock.call().validate(config[ctx], non_hidden=False), + mock.call(ctx, allow_hidden=False), + mock.call().validate(config[ctx]), ]) @mock.patch("rally.task.context.Context.get") - def test_validate_non_hidden(self, mock_context_get): + def test_validate_hidden(self, mock_context_get): config = { "ctx1": mock.MagicMock(), "ctx2": mock.MagicMock() } - context.ContextManager.validate(config, non_hidden=True) + context.ContextManager.validate(config, allow_hidden=True) for ctx in ("ctx1", "ctx2"): mock_context_get.assert_has_calls([ - mock.call(ctx), - mock.call().validate(config[ctx], non_hidden=True), + mock.call(ctx, allow_hidden=True), + mock.call().validate(config[ctx]), ]) def test_validate__non_existing_context(self): @@ -191,7 +185,8 @@ class ContextManagerTestCase(test.TestCase): self.assertEqual(result, ctx_object) mock_context_get.assert_has_calls( - [mock.call("a"), mock.call("b")], any_order=True) + [mock.call("a", allow_hidden=True), + mock.call("b", allow_hidden=True)], any_order=True) mock_context.assert_has_calls( [mock.call(ctx_object), mock.call(ctx_object)], any_order=True) self.assertEqual([mock_context(), mock_context()], manager._visited) @@ -208,7 +203,8 @@ class ContextManagerTestCase(test.TestCase): manager = context.ContextManager(ctx_object) manager.cleanup() mock_context_get.assert_has_calls( - [mock.call("a"), mock.call("b")], any_order=True) + [mock.call("a", allow_hidden=True), + mock.call("b", allow_hidden=True)], any_order=True) mock_context.assert_has_calls( [mock.call(ctx_object), mock.call(ctx_object)], any_order=True) mock_context.return_value.assert_has_calls( @@ -225,7 +221,8 @@ class ContextManagerTestCase(test.TestCase): manager.cleanup() mock_context_get.assert_has_calls( - [mock.call("a"), mock.call("b")], any_order=True) + [mock.call("a", allow_hidden=True), + mock.call("b", allow_hidden=True)], any_order=True) mock_context.assert_has_calls( [mock.call(ctx_object), mock.call(ctx_object)], any_order=True) mock_context.return_value.assert_has_calls( diff --git a/tests/unit/task/test_engine.py b/tests/unit/task/test_engine.py index 880df6b3d3..16aa27836f 100644 --- a/tests/unit/task/test_engine.py +++ b/tests/unit/task/test_engine.py @@ -157,6 +157,7 @@ class TaskEngineTestCase(test.TestCase): eng._validate_config_scenarios_name, mock_task_instance) + @mock.patch("rally.task.engine.scenario.Scenario.get") @mock.patch("rally.task.hook.Hook.validate") @mock.patch("rally.task.engine.TaskConfig") @mock.patch("rally.task.engine.runner.ScenarioRunner.validate") @@ -165,8 +166,11 @@ class TaskEngineTestCase(test.TestCase): self, mock_context_manager_validate, mock_scenario_runner_validate, mock_task_config, - mock_hook_validate + mock_hook_validate, + mock_scenario_get ): + default_context = {"foo": 1} + mock_scenario_get.return_value._meta_get.return_value = default_context mock_task_instance = mock.MagicMock() mock_subtask = mock.MagicMock() mock_subtask.workloads = [ @@ -180,16 +184,24 @@ class TaskEngineTestCase(test.TestCase): mock_scenario_runner_validate.assert_has_calls( [mock.call({}), mock.call("b")], any_order=True) mock_context_manager_validate.assert_has_calls( - [mock.call("a", non_hidden=True), mock.call({}, non_hidden=True)], - any_order=True) + [mock.call("a"), + mock.call(default_context, allow_hidden=True), + mock.call({}), + mock.call(default_context, allow_hidden=True), + mock.call({}), + mock.call(default_context, allow_hidden=True)], + any_order=True + ) mock_hook_validate.assert_called_once_with("c") + @mock.patch("rally.task.engine.scenario.Scenario.get") @mock.patch("rally.task.engine.TaskConfig") @mock.patch("rally.task.engine.runner.ScenarioRunner") @mock.patch("rally.task.engine.context.ContextManager.validate") def test__validate_config_syntax__wrong_runner( self, mock_context_manager_validate, - mock_scenario_runner, mock_task_config): + mock_scenario_runner, mock_task_config, mock_scenario_get): + mock_scenario_get.return_value._meta_get.return_value = {} mock_task_instance = mock.MagicMock() mock_subtask = mock.MagicMock() mock_subtask.workloads = [ @@ -204,12 +216,14 @@ class TaskEngineTestCase(test.TestCase): self.assertRaises(exceptions.InvalidTaskConfig, eng._validate_config_syntax, mock_task_instance) + @mock.patch("rally.task.engine.scenario.Scenario.get") @mock.patch("rally.task.engine.TaskConfig") @mock.patch("rally.task.engine.runner.ScenarioRunner.validate") @mock.patch("rally.task.engine.context.ContextManager") def test__validate_config_syntax__wrong_context( self, mock_context_manager, mock_scenario_runner_validate, - mock_task_config): + mock_task_config, mock_scenario_get): + mock_scenario_get.return_value._meta_get.return_value = {} mock_task_instance = mock.MagicMock() mock_subtask = mock.MagicMock() mock_subtask.workloads = [ diff --git a/tests/unit/task/test_scenario.py b/tests/unit/task/test_scenario.py index c00c4577cb..1a9b6f61c4 100644 --- a/tests/unit/task/test_scenario.py +++ b/tests/unit/task/test_scenario.py @@ -263,7 +263,8 @@ class ScenarioTestCase(test.TestCase): def test_scenario_context_are_valid(self): for s in scenario.Scenario.get_all(): try: - context.ContextManager.validate(s._meta_get("default_context")) + context.ContextManager.validate(s._meta_get("default_context"), + allow_hidden=True) except Exception: print(traceback.format_exc()) self.fail("Scenario `%s` has wrong context" % scenario) diff --git a/tests/unit/verification/test_context.py b/tests/unit/verification/test_context.py index 63d220093f..671c390774 100644 --- a/tests/unit/verification/test_context.py +++ b/tests/unit/verification/test_context.py @@ -28,17 +28,15 @@ class FakeContext(context.VerifierContext): class VerifierContextTestCase(test.TestCase): def test__meta_get(self): - data = {"key1": "value1", "key2": "value2", "hidden": False} + data = {"key1": "value1", "key2": "value2"} for k, v in data.items(): FakeContext._meta_set(k, v) for k, v in data.items(): - if k != "hidden": - self.assertEqual(v, FakeContext._meta_get(k)) + self.assertEqual(v, FakeContext._meta_get(k)) - self.assertTrue(FakeContext._meta_get("hidden")) - self.assertNotEqual(data["hidden"], FakeContext._meta_get("hidden")) + self.assertTrue(FakeContext.is_hidden()) class ContextManagerTestCase(test.TestCase): @@ -48,8 +46,9 @@ class ContextManagerTestCase(test.TestCase): context.ContextManager.validate(config) - self.assertEqual([mock.call(k) for k, v in config.items()], + self.assertEqual([mock.call(k, allow_hidden=True) + for k, v in config.items()], mock_verifier_context.get.call_args_list) self.assertEqual( - [mock.call(v, non_hidden=False) for k, v in config.items()], + [mock.call(v) for k, v in config.items()], mock_verifier_context.get.return_value.validate.call_args_list)