From 5c40c19d05bf552f87acc69ded47ae1a13db03b6 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Thu, 26 Apr 2018 07:49:35 +0200 Subject: [PATCH 01/12] Remove plugin-specific names in bootstrap handler --- esrally/mechanic/mechanic.py | 15 +++++------ esrally/mechanic/provisioner.py | 4 +-- esrally/mechanic/team.py | 42 +++++++++++++++++++----------- tests/mechanic/provisioner_test.py | 2 +- tests/mechanic/team_test.py | 12 ++++----- 5 files changed, 43 insertions(+), 32 deletions(-) diff --git a/esrally/mechanic/mechanic.py b/esrally/mechanic/mechanic.py index 53fcf054e..916828309 100644 --- a/esrally/mechanic/mechanic.py +++ b/esrally/mechanic/mechanic.py @@ -360,8 +360,7 @@ def receiveMsg_NodesStopped(self, msg, sender): self.transition_when_all_children_responded(sender, msg, "cluster_stopping", "cluster_stopped", self.on_all_nodes_stopped) def on_all_nodes_started(self): - plugin_handler = PostLaunchPluginHandler(self.plugins) - self.cluster_launcher = launcher.ClusterLauncher(self.cfg, self.metrics_store, on_post_launch=plugin_handler) + self.cluster_launcher = launcher.ClusterLauncher(self.cfg, self.metrics_store, on_post_launch=PostLaunchHandler(self.plugins)) # Workaround because we could raise a LaunchError here and thespian will attempt to retry a failed message. # In that case, we will get a followup RallyAssertionError because on the second attempt, Rally will check # the status which is now "nodes_started" but we expected the status to be "nodes_starting" previously. @@ -409,19 +408,19 @@ def on_all_nodes_stopped(self): # do not self-terminate, let the parent actor handle this -class PostLaunchPluginHandler: - def __init__(self, plugins, hook_handler_class=team.PluginBootstrapHookHandler): +class PostLaunchHandler: + def __init__(self, components, hook_handler_class=team.BootstrapHookHandler): self.handlers = [] - if plugins: - for plugin in plugins: - handler = hook_handler_class(plugin) + if components: + for component in components: + handler = hook_handler_class(component) if handler.can_load(): handler.load() self.handlers.append(handler) def __call__(self, client): for handler in self.handlers: - handler.invoke(team.PluginBootstrapPhase.post_launch.name, client=client) + handler.invoke(team.BootstrapPhase.post_launch.name, client=client) @thespian.actors.requireCapability('coordinator') diff --git a/esrally/mechanic/provisioner.py b/esrally/mechanic/provisioner.py index 2942caae7..0ded69858 100644 --- a/esrally/mechanic/provisioner.py +++ b/esrally/mechanic/provisioner.py @@ -158,7 +158,7 @@ def prepare(self, binary): for installer in self.plugin_installers: # Never let install hooks modify our original provisioner variables and just provide a copy! - installer.invoke_install_hook(team.PluginBootstrapPhase.post_install, provisioner_vars.copy()) + installer.invoke_install_hook(team.BootstrapPhase.post_install, provisioner_vars.copy()) return NodeConfiguration(self.es_installer.car, self.es_installer.node_ip, self.es_installer.node_name, self.es_installer.node_root_dir, self.es_installer.es_home_path, self.es_installer.node_log_dir, @@ -283,7 +283,7 @@ def _data_paths(self): class PluginInstaller: - def __init__(self, plugin, hook_handler_class=team.PluginBootstrapHookHandler): + def __init__(self, plugin, hook_handler_class=team.BootstrapHookHandler): self.plugin = plugin self.hook_handler = hook_handler_class(self.plugin) if self.hook_handler.can_load(): diff --git a/esrally/mechanic/team.py b/esrally/mechanic/team.py index 37c4a518c..f375eab45 100644 --- a/esrally/mechanic/team.py +++ b/esrally/mechanic/team.py @@ -327,6 +327,8 @@ def __init__(self, name, core_plugin=False, config=None, root_path=None, config_ self.root_path = root_path self.config_paths = config_paths self.variables = variables + # name of the initial Python file to load for plugins. + self.entry_point = "plugin" def __str__(self): return "Plugin descriptor for [%s]" % self.name @@ -344,28 +346,37 @@ def __eq__(self, other): return isinstance(other, type(self)) and (self.name, self.config, self.core_plugin) == (other.name, other.config, other.core_plugin) -class PluginBootstrapPhase(Enum): +class BootstrapPhase(Enum): post_install = 10 post_launch = 20 @classmethod def valid(cls, name): - for n in PluginBootstrapPhase.names(): + for n in BootstrapPhase.names(): if n == name: return True return False @classmethod def names(cls): - return [p.name for p in list(PluginBootstrapPhase)] + return [p.name for p in list(BootstrapPhase)] -class PluginBootstrapHookHandler: - def __init__(self, plugin, loader_class=modules.ComponentLoader): - self.plugin = plugin +class BootstrapHookHandler: + """ + Responsible for loading and executing component-specific intitialization code. + """ + def __init__(self, component, loader_class=modules.ComponentLoader): + """ + Creates a new BootstrapHookHandler. + + :param component: The component that should be loaded. In practice, this is a PluginDescriptor instance. + :param loader_class: The implementation that loads the provided component's code. + """ + self.component = component # Don't allow the loader to recurse. The subdirectories may contain Elasticsearch specific files which we do not want to add to # Rally's Python load path. We may need to define a more advanced strategy in the future. - self.loader = loader_class(root_path=self.plugin.root_path, component_entry_point="plugin", recurse=False) + self.loader = loader_class(root_path=self.component.root_path, component_entry_point=self.component.entry_point, recurse=False) self.hooks = {} def can_load(self): @@ -380,24 +391,25 @@ def load(self): # just pass our own exceptions transparently. raise except BaseException: - msg = "Could not load plugin bootstrap hooks in [{}]".format(self.loader.root_path) + msg = "Could not load bootstrap hooks in [{}]".format(self.loader.root_path) logger.exception(msg) raise exceptions.SystemSetupError(msg) def register(self, phase, hook): - logger.info("Registering plugin bootstrap hook [%s] for phase [%s] in plugin [%s]", hook.__name__, phase, self.plugin.name) - if not PluginBootstrapPhase.valid(phase): - raise exceptions.SystemSetupError("Phase [{}] is unknown. Valid phases are: {}.".format(phase, PluginBootstrapPhase.names())) + logger.info("Registering bootstrap hook [%s] for phase [%s] in component [%s]", hook.__name__, phase, self.component.name) + if not BootstrapPhase.valid(phase): + raise exceptions.SystemSetupError("Unknown bootstrap phase [{}]. Valid phases are: {}.".format(phase, BootstrapPhase.names())) if phase not in self.hooks: self.hooks[phase] = [] self.hooks[phase].append(hook) def invoke(self, phase, **kwargs): if phase in self.hooks: - logger.info("Invoking phase [%s] for plugin [%s] in config [%s]", phase, self.plugin.name, self.plugin.config) + logger.info("Invoking phase [%s] for component [%s] in config [%s]", phase, self.component.name, self.component.config) for hook in self.hooks[phase]: - logger.info("Invoking hook [%s].", hook.__name__) + logger.info("Invoking bootstrap hook [%s].", hook.__name__) # hooks should only take keyword arguments to be forwards compatible with Rally! - hook(config_names=self.plugin.config, **kwargs) + hook(config_names=self.component.config, **kwargs) else: - logger.debug("Plugin [%s] in config [%s] has no hook registered for phase [%s].", self.plugin.name, self.plugin.config, phase) \ No newline at end of file + logger.debug("Component [%s] in config [%s] has no hook registered for phase [%s].", + self.component.name, self.component.config, phase) \ No newline at end of file diff --git a/tests/mechanic/provisioner_test.py b/tests/mechanic/provisioner_test.py index b06ead1f4..1705c8447 100644 --- a/tests/mechanic/provisioner_test.py +++ b/tests/mechanic/provisioner_test.py @@ -439,7 +439,7 @@ def test_invokes_hook(self): installer = provisioner.PluginInstaller(plugin, hook_handler_class=PluginInstallerTests.NoopHookHandler) self.assertEqual(0, len(installer.hook_handler.hook_calls)) - installer.invoke_install_hook(team.PluginBootstrapPhase.post_install, {"foo": "bar"}) + installer.invoke_install_hook(team.BootstrapPhase.post_install, {"foo": "bar"}) self.assertEqual(1, len(installer.hook_handler.hook_calls)) self.assertEqual({"foo": "bar"}, installer.hook_handler.hook_calls["post_install"]) diff --git a/tests/mechanic/team_test.py b/tests/mechanic/team_test.py index b8a94ecbb..28ce2d4fc 100644 --- a/tests/mechanic/team_test.py +++ b/tests/mechanic/team_test.py @@ -120,7 +120,7 @@ def test_loads_configured_plugin(self): }, plugin.variables) -class PluginBootstrapHookHandlerTests(TestCase): +class BootstrapHookHandlerTests(TestCase): class UnitTestComponentLoader: def __init__(self, root_path, component_entry_point, recurse): self.root_path = root_path @@ -146,8 +146,8 @@ def register(self, handler): def test_loads_module(self): plugin = team.PluginDescriptor("unittest-plugin") - hook = PluginBootstrapHookHandlerTests.UnitTestHook() - handler = team.PluginBootstrapHookHandler(plugin, loader_class=PluginBootstrapHookHandlerTests.UnitTestComponentLoader) + hook = BootstrapHookHandlerTests.UnitTestHook() + handler = team.BootstrapHookHandler(plugin, loader_class=BootstrapHookHandlerTests.UnitTestComponentLoader) handler.loader.registration_function = hook handler.load() @@ -159,11 +159,11 @@ def test_loads_module(self): def test_cannot_register_for_unknown_phase(self): plugin = team.PluginDescriptor("unittest-plugin") - hook = PluginBootstrapHookHandlerTests.UnitTestHook(phase="this_is_an_unknown_install_phase") - handler = team.PluginBootstrapHookHandler(plugin, loader_class=PluginBootstrapHookHandlerTests.UnitTestComponentLoader) + hook = BootstrapHookHandlerTests.UnitTestHook(phase="this_is_an_unknown_install_phase") + handler = team.BootstrapHookHandler(plugin, loader_class=BootstrapHookHandlerTests.UnitTestComponentLoader) handler.loader.registration_function = hook with self.assertRaises(exceptions.SystemSetupError) as ctx: handler.load() - self.assertEqual("Phase [this_is_an_unknown_install_phase] is unknown. Valid phases are: ['post_install', 'post_launch'].", + self.assertEqual("Unknown bootstrap phase [this_is_an_unknown_install_phase]. Valid phases are: ['post_install', 'post_launch'].", ctx.exception.args[0]) From 987a8253848c01c695b08f7dc29fe0dfe48a2391 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Thu, 26 Apr 2018 09:15:09 +0200 Subject: [PATCH 02/12] Introduce team versioning --- docs/car.rst | 44 ++++++------- docs/elasticsearch_plugins.rst | 4 +- docs/migrate.rst | 64 +++++++++++++++++++ esrally/mechanic/team.py | 33 ++++++---- tests/mechanic/data/cars/{ => v1}/32gheap.ini | 0 tests/mechanic/data/cars/{ => v1}/default.ini | 0 tests/mechanic/data/cars/{ => v1}/ea.ini | 0 .../data/cars/{ => v1}/empty_config_base.ini | 0 .../cars/{ => v1}/missing_config_base.ini | 0 tests/mechanic/data/cars/{ => v1}/verbose.ini | 0 .../{ => v1}/complex_plugin/config-a.ini | 0 .../{ => v1}/complex_plugin/config-b.ini | 0 .../data/plugins/{ => v1}/core-plugins.txt | 0 tests/mechanic/team_test.py | 14 ++-- 14 files changed, 115 insertions(+), 44 deletions(-) rename tests/mechanic/data/cars/{ => v1}/32gheap.ini (100%) rename tests/mechanic/data/cars/{ => v1}/default.ini (100%) rename tests/mechanic/data/cars/{ => v1}/ea.ini (100%) rename tests/mechanic/data/cars/{ => v1}/empty_config_base.ini (100%) rename tests/mechanic/data/cars/{ => v1}/missing_config_base.ini (100%) rename tests/mechanic/data/cars/{ => v1}/verbose.ini (100%) rename tests/mechanic/data/plugins/{ => v1}/complex_plugin/config-a.ini (100%) rename tests/mechanic/data/plugins/{ => v1}/complex_plugin/config-b.ini (100%) rename tests/mechanic/data/plugins/{ => v1}/core-plugins.txt (100%) diff --git a/docs/car.rst b/docs/car.rst index 855d38e83..c6f4f4895 100644 --- a/docs/car.rst +++ b/docs/car.rst @@ -37,28 +37,24 @@ The Anatomy of a car The default car definitions of Rally are stored in ``~/.rally/benchmarks/teams/default/cars``. There we find the following structure:: - ├── 16gheap.ini - ├── 1gheap.ini - ├── 2gheap.ini - ├── 4gheap.ini - ├── defaults.ini - ├── ea - │   └── config - │   └── jvm.options - ├── ea.ini - ├── vanilla - │   └── config - │   ├── elasticsearch.yml - │   ├── jvm.options - │   └── log4j2.properties - ├── verbose_iw - │   └── config - │   ├── elasticsearch.yml - │   ├── jvm.options - │   └── log4j2.properties - └── verbose_iw.ini - -Each ``.ini`` file in the top level directory defines a car. And each directory (``ea``, ``vanilla`` or ``verbose_iw``) contains templates for the config files. + . + └── v1 + ├── 1gheap.ini + ├── 2gheap.ini + ├── defaults.ini + ├── ea + │   └── templates + │   └── config + │   └── jvm.options + ├── ea.ini + └── vanilla + └── templates + └── config + ├── elasticsearch.yml + ├── jvm.options + └── log4j2.properties + +The top-level directory "v1" denotes the configuration format in version 1. Below that directory, each ``.ini`` file defines a car. Each directory (``ea`` or ``vanilla``) contains templates for the config files. Rally will ignore the contents of the top-level directory and only copy the files in the ``templates`` subdirectory. Let's have a look at the ``1gheap`` car by inspecting ``1gheap.ini``:: @@ -74,7 +70,7 @@ Let's have a look at the ``1gheap`` car by inspecting ``1gheap.ini``:: The name of the car is derived from the ini file name. In the ``meta`` section we can provide a ``description`` and the ``type``. Use ``car`` if a configuration can be used standalone and ``mixin`` if it needs to be combined with other configurations. In the ``config`` section we define that this definition is based on the ``vanilla`` configuration. We also define a variable ``heap_size`` and set it to ``1g``. -Let's open ``vanilla/config/jvm.options`` to see how this variable is used (we'll only show the relevant part here):: +Let's open ``vanilla/config/templates/jvm.options`` to see how this variable is used (we'll only show the relevant part here):: # Xms represents the initial size of total heap space # Xmx represents the maximum size of total heap space @@ -84,7 +80,7 @@ Let's open ``vanilla/config/jvm.options`` to see how this variable is used (we'l So Rally reads all variables and the template files and replaces the variables in the final configuration. Note that Rally does not know anything about ``jvm.options`` or ``elasticsearch.yml``. For Rally, these are just plain text templates that need to be copied to the Elasticsearch directory before running a benchmark. Under the hood, Rally uses `Jinja2 `_ as template language. This allows you to use Jinja2 expressions in your car configuration files. -If you open ``vanilla/config/elasticsearch.yml`` you will see a few variables that are not defined in the ``.ini`` file: +If you open ``vanilla/templates/config/elasticsearch.yml`` you will see a few variables that are not defined in the ``.ini`` file: * ``network_host`` * ``http_port`` diff --git a/docs/elasticsearch_plugins.rst b/docs/elasticsearch_plugins.rst index c105ab211..5ba73dbe1 100644 --- a/docs/elasticsearch_plugins.rst +++ b/docs/elasticsearch_plugins.rst @@ -169,12 +169,12 @@ We want to support two configurations for this plugin: ``simple`` which will set First, we need a template configuration. We will call this a "config base" in Rally. We will just need one config base for this example and will call it "default". -In ``$TEAM_REPO_ROOT`` create the directory structure for the plugin and its config base with `mkdir -p myplugin/default/config` and add the following ``elasticsearch.yml`` in the new directory:: +In ``$TEAM_REPO_ROOT`` create the directory structure for the plugin and its config base with `mkdir -p myplugin/default/templates/config` and add the following ``elasticsearch.yml`` in the new directory:: myplugin.active: true myplugin.mode={{my_plugin_mode}} -That's it. Later, Rally will just copy all files in ``myplugin/default`` to the home directory of the Elasticsearch node that it configures. First, Rally will always apply the car's configuration and then plugins can add their configuration on top. This also explains why we have created a ``config/elasticsearch.yml``. Rally will just copy this file and replace template variables on the way. +That's it. Later, Rally will just copy all files in ``myplugin/default/templates`` to the home directory of the Elasticsearch node that it configures. First, Rally will always apply the car's configuration and then plugins can add their configuration on top. This also explains why we have created a ``config/elasticsearch.yml``. Rally will just copy this file and replace template variables on the way. .. note:: If you create a new customization for a plugin, ensure that the plugin name in the team repository matches the core plugin name. Note that hyphens need to be replaced by underscores (e.g. "x-pack" becomes "x_pack"). The reason is that Rally allows to write custom install hooks and the plugin name will become the root package name of the install hook. However, hyphens are not supported in Python which is why we use underscores instead. diff --git a/docs/migrate.rst b/docs/migrate.rst index 067e15826..732daab0b 100644 --- a/docs/migrate.rst +++ b/docs/migrate.rst @@ -1,6 +1,70 @@ Migration Guide =============== +Migrating to Rally 0.11.0 +------------------------- + +Versioned teams +~~~~~~~~~~~~~~~ + +.. note:: + + You can skip this section if you do not create custom Rally teams. + +We have introduced versioned team specifications and consequently the directory structure changes. All cars and plugins need to reside in a version-specific subdirectory now. Up to now the structure of a team repository was as follows:: + + . + ├── cars + │   ├── 1gheap.ini + │   ├── 2gheap.ini + │   ├── defaults.ini + │   ├── ea + │   │   └── config + │   │   └── jvm.options + │   ├── ea.ini + │   └── vanilla + │   └── config + │   ├── elasticsearch.yml + │   ├── jvm.options + │   └── log4j2.properties + └── plugins + ├── core-plugins.txt + └── transport_nio + ├── default + │   └── config + │   └── elasticsearch.yml + └── transport.ini + +Starting with Rally 0.11.0, Rally will look for a directory "v1" within ``cars`` and ``plugins``. The files that should be copied to the Elasticsearch directory, need to be contained in a ``templates`` subdirectory. Therefore, the new structure is as follows:: + + . + ├── cars + │   └── v1 + │   ├── 1gheap.ini + │   ├── 2gheap.ini + │   ├── defaults.ini + │   ├── ea + │   │   └── templates + │   │   └── config + │   │   └── jvm.options + │   ├── ea.ini + │   └── vanilla + │   └── templates + │   └── config + │   ├── elasticsearch.yml + │   ├── jvm.options + │   └── log4j2.properties + └── plugins + └── v1 + ├── core-plugins.txt + └── transport_nio + ├── default + │   └── templates + │   └── config + │   └── elasticsearch.yml + └── transport.ini + + Migrating to Rally 0.10.0 ------------------------- diff --git a/esrally/mechanic/team.py b/esrally/mechanic/team.py index f375eab45..c4bd5afff 100644 --- a/esrally/mechanic/team.py +++ b/esrally/mechanic/team.py @@ -10,6 +10,15 @@ logger = logging.getLogger("rally.team") +TEAM_FORMAT_VERSION = 1 + + +def _path_for(team_root_path, team_member_type): + root_path = os.path.join(team_root_path, team_member_type, "v{}".format(TEAM_FORMAT_VERSION)) + if not os.path.exists(root_path): + raise exceptions.SystemSetupError("Path {} for {} does not exist.".format(root_path, team_member_type)) + return root_path + def list_cars(cfg): loader = CarLoader(team_path(cfg)) @@ -23,7 +32,7 @@ def list_cars(cfg): console.println(tabulate.tabulate([[c.name, c.type, c.description] for c in cars], headers=["Name", "Type", "Description"])) -def load_car(repo, name, car_params={}): +def load_car(repo, name, car_params=None): # preserve order as we append to existing config files later during provisioning. all_config_paths = [] all_variables = {} @@ -59,7 +68,7 @@ def list_plugins(cfg): console.println("No Elasticsearch plugins are available.\n") -def load_plugin(repo, name, config, plugin_params={}): +def load_plugin(repo, name, config, plugin_params=None): if config is not None: logger.info("Loading plugin [%s] with configuration(s) [%s]." % (name, config)) else: @@ -67,7 +76,7 @@ def load_plugin(repo, name, config, plugin_params={}): return PluginLoader(repo).load_plugin(name, config, plugin_params) -def load_plugins(repo, plugin_names, plugin_params={}): +def load_plugins(repo, plugin_names, plugin_params=None): def name_and_config(p): plugin_spec = p.split(":") if len(plugin_spec) == 1: @@ -104,7 +113,7 @@ def team_path(cfg): class CarLoader: def __init__(self, team_root_path): - self.cars_dir = os.path.join(team_root_path, "cars") + self.cars_dir = _path_for(team_root_path, "cars") def car_names(self): def __car_name(path): @@ -119,7 +128,7 @@ def __is_car(path): def _car_file(self, name): return os.path.join(self.cars_dir, "%s.ini" % name) - def load_car(self, name, car_params={}): + def load_car(self, name, car_params=None): car_config_file = self._car_file(name) if not io.exists(car_config_file): raise exceptions.SystemSetupError("Unknown car [%s]. List the available cars with %s list cars." % (name, PROGRAM_NAME)) @@ -138,7 +147,7 @@ def load_car(self, name, car_params={}): config_bases = config["config"]["base"].split(",") for base in config_bases: if base: - config_paths.append(os.path.join(self.cars_dir, base)) + config_paths.append(os.path.join(self.cars_dir, base, "templates")) # it's possible that some cars don't have a config base, e.g. mixins which only override variables if len(config_paths) == 0: @@ -149,7 +158,8 @@ def load_car(self, name, car_params={}): for k, v in config["variables"].items(): variables[k] = v # add all car params here to override any defaults - variables.update(car_params) + if car_params: + variables.update(car_params) env = {} if "env" in config.sections(): @@ -203,7 +213,7 @@ def __str__(self): class PluginLoader: def __init__(self, team_root_path): - self.plugins_root_path = os.path.join(team_root_path, "plugins") + self.plugins_root_path = _path_for(team_root_path, "plugins") def plugins(self): known_plugins = self._core_plugins() + self._configured_plugins() @@ -256,7 +266,7 @@ def _plugin_name_to_file(self, plugin_name): def _core_plugin(self, name): return next((p for p in self._core_plugins() if p.name == name and p.config is None), None) - def load_plugin(self, name, config_names, plugin_params={}): + def load_plugin(self, name, config_names, plugin_params=None): root_path = self._plugin_root_path(name) if not config_names: # maybe we only have a config folder but nothing else (e.g. if there is only an install hook) @@ -299,14 +309,15 @@ def load_plugin(self, name, config_names, plugin_params={}): config_bases = config["config"]["base"].split(",") for base in config_bases: if base and base not in known_config_bases: - config_paths.append(os.path.join(root_path, base)) + config_paths.append(os.path.join(root_path, base, "templates")) known_config_bases.add(base) if "variables" in config.sections(): for k, v in config["variables"].items(): variables[k] = v # add all plugin params here to override any defaults - variables.update(plugin_params) + if plugin_params: + variables.update(plugin_params) # maybe one of the configs is really just for providing variables. However, we still require one config base overall. if len(config_paths) == 0: diff --git a/tests/mechanic/data/cars/32gheap.ini b/tests/mechanic/data/cars/v1/32gheap.ini similarity index 100% rename from tests/mechanic/data/cars/32gheap.ini rename to tests/mechanic/data/cars/v1/32gheap.ini diff --git a/tests/mechanic/data/cars/default.ini b/tests/mechanic/data/cars/v1/default.ini similarity index 100% rename from tests/mechanic/data/cars/default.ini rename to tests/mechanic/data/cars/v1/default.ini diff --git a/tests/mechanic/data/cars/ea.ini b/tests/mechanic/data/cars/v1/ea.ini similarity index 100% rename from tests/mechanic/data/cars/ea.ini rename to tests/mechanic/data/cars/v1/ea.ini diff --git a/tests/mechanic/data/cars/empty_config_base.ini b/tests/mechanic/data/cars/v1/empty_config_base.ini similarity index 100% rename from tests/mechanic/data/cars/empty_config_base.ini rename to tests/mechanic/data/cars/v1/empty_config_base.ini diff --git a/tests/mechanic/data/cars/missing_config_base.ini b/tests/mechanic/data/cars/v1/missing_config_base.ini similarity index 100% rename from tests/mechanic/data/cars/missing_config_base.ini rename to tests/mechanic/data/cars/v1/missing_config_base.ini diff --git a/tests/mechanic/data/cars/verbose.ini b/tests/mechanic/data/cars/v1/verbose.ini similarity index 100% rename from tests/mechanic/data/cars/verbose.ini rename to tests/mechanic/data/cars/v1/verbose.ini diff --git a/tests/mechanic/data/plugins/complex_plugin/config-a.ini b/tests/mechanic/data/plugins/v1/complex_plugin/config-a.ini similarity index 100% rename from tests/mechanic/data/plugins/complex_plugin/config-a.ini rename to tests/mechanic/data/plugins/v1/complex_plugin/config-a.ini diff --git a/tests/mechanic/data/plugins/complex_plugin/config-b.ini b/tests/mechanic/data/plugins/v1/complex_plugin/config-b.ini similarity index 100% rename from tests/mechanic/data/plugins/complex_plugin/config-b.ini rename to tests/mechanic/data/plugins/v1/complex_plugin/config-b.ini diff --git a/tests/mechanic/data/plugins/core-plugins.txt b/tests/mechanic/data/plugins/v1/core-plugins.txt similarity index 100% rename from tests/mechanic/data/plugins/core-plugins.txt rename to tests/mechanic/data/plugins/v1/core-plugins.txt diff --git a/tests/mechanic/team_test.py b/tests/mechanic/team_test.py index 28ce2d4fc..68bf8d0b6 100644 --- a/tests/mechanic/team_test.py +++ b/tests/mechanic/team_test.py @@ -24,14 +24,14 @@ def test_lists_car_names(self): def test_load_known_car(self): car = team.load_car(self.team_dir, ["default"], car_params={"data_paths": ["/mnt/disk0", "/mnt/disk1"]}) self.assertEqual("default", car.name) - self.assertEqual([os.path.join(current_dir, "data", "cars", "vanilla")], car.config_paths) + self.assertEqual([os.path.join(current_dir, "data", "cars", "v1", "vanilla", "templates")], car.config_paths) self.assertDictEqual({"heap_size": "1g", "data_paths": ["/mnt/disk0", "/mnt/disk1"]}, car.variables) self.assertEqual({}, car.env) def test_load_car_with_mixin_single_config_base(self): car = team.load_car(self.team_dir, ["32gheap", "ea"]) self.assertEqual("32gheap+ea", car.name) - self.assertEqual([os.path.join(current_dir, "data", "cars", "vanilla")], car.config_paths) + self.assertEqual([os.path.join(current_dir, "data", "cars", "v1", "vanilla", "templates")], car.config_paths) self.assertEqual({"heap_size": "32g", "assertions": "true"}, car.variables) self.assertEqual({"JAVA_TOOL_OPTS": "A B C D E F"}, car.env) @@ -39,8 +39,8 @@ def test_load_car_with_mixin_multiple_config_bases(self): car = team.load_car(self.team_dir, ["32gheap", "ea", "verbose"]) self.assertEqual("32gheap+ea+verbose", car.name) self.assertEqual([ - os.path.join(current_dir, "data", "cars", "vanilla"), - os.path.join(current_dir, "data", "cars", "verbose_logging"), + os.path.join(current_dir, "data", "cars", "v1", "vanilla", "templates"), + os.path.join(current_dir, "data", "cars", "v1", "verbose_logging", "templates"), ], car.config_paths) self.assertEqual({"heap_size": "32g", "assertions": "true"}, car.variables) self.assertEqual({"JAVA_TOOL_OPTS": "A B C D E F G H I"}, car.env) @@ -103,13 +103,13 @@ def test_loads_configured_plugin(self): self.assertEqual("complex-plugin", plugin.name) self.assertCountEqual(["config-a", "config-b"], plugin.config) - expected_root_path = os.path.join(current_dir, "data", "plugins", "complex_plugin") + expected_root_path = os.path.join(current_dir, "data", "plugins", "v1", "complex_plugin") self.assertEqual(expected_root_path, plugin.root_path) # order does matter here! We should not swap it self.assertListEqual([ - os.path.join(expected_root_path, "default"), - os.path.join(expected_root_path, "special"), + os.path.join(expected_root_path, "default", "templates"), + os.path.join(expected_root_path, "special", "templates"), ], plugin.config_paths) self.assertEqual({ From d629acfa845e59d094ae48dbb734f357a70621e4 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Thu, 26 Apr 2018 12:35:15 +0200 Subject: [PATCH 03/12] Read variables also from car config bases --- docs/car.rst | 10 ++- esrally/mechanic/team.py | 85 ++++++++++++------- .../mechanic/data/cars/v1/vanilla/config.ini | 5 ++ .../data/cars/v1/verbose_logging/config.ini | 2 + tests/mechanic/team_test.py | 19 ++++- 5 files changed, 84 insertions(+), 37 deletions(-) create mode 100644 tests/mechanic/data/cars/v1/vanilla/config.ini create mode 100644 tests/mechanic/data/cars/v1/verbose_logging/config.ini diff --git a/docs/car.rst b/docs/car.rst index c6f4f4895..3f413c11a 100644 --- a/docs/car.rst +++ b/docs/car.rst @@ -54,7 +54,13 @@ The default car definitions of Rally are stored in ``~/.rally/benchmarks/teams/d ├── jvm.options └── log4j2.properties -The top-level directory "v1" denotes the configuration format in version 1. Below that directory, each ``.ini`` file defines a car. Each directory (``ea`` or ``vanilla``) contains templates for the config files. Rally will ignore the contents of the top-level directory and only copy the files in the ``templates`` subdirectory. +The top-level directory "v1" denotes the configuration format in version 1. Below that directory, each ``.ini`` file defines a car. Each directory (``ea`` or ``vanilla``) contains templates for the config files. Rally will only copy the files in the ``templates`` subdirectory. The top-level directory is reserved for a special file, ``config.ini`` which you can use to define default variables that apply to all cars that are based on this configuration. Below is an example ``config.ini`` file:: + + [variables] + clean_command=./gradlew clean + +This defines the variable ``clean_command`` for all cars that reference this configuration. + Let's have a look at the ``1gheap`` car by inspecting ``1gheap.ini``:: @@ -68,7 +74,7 @@ Let's have a look at the ``1gheap`` car by inspecting ``1gheap.ini``:: [variables] heap_size=1g -The name of the car is derived from the ini file name. In the ``meta`` section we can provide a ``description`` and the ``type``. Use ``car`` if a configuration can be used standalone and ``mixin`` if it needs to be combined with other configurations. In the ``config`` section we define that this definition is based on the ``vanilla`` configuration. We also define a variable ``heap_size`` and set it to ``1g``. +The name of the car is derived from the ``.ini`` file name. In the ``meta`` section we can provide a ``description`` and the ``type``. Use ``car`` if a configuration can be used standalone and ``mixin`` if it needs to be combined with other configurations. In the ``config`` section we define that this definition is based on the ``vanilla`` configuration. We also define a variable ``heap_size`` and set it to ``1g``. Note that variables defined here take precedence over variables defined in the ``config.ini`` file of any of the referenced configurations. Let's open ``vanilla/config/templates/jvm.options`` to see how this variable is used (we'll only show the relevant part here):: diff --git a/esrally/mechanic/team.py b/esrally/mechanic/team.py index c4bd5afff..e499cd788 100644 --- a/esrally/mechanic/team.py +++ b/esrally/mechanic/team.py @@ -35,7 +35,8 @@ def list_cars(cfg): def load_car(repo, name, car_params=None): # preserve order as we append to existing config files later during provisioning. all_config_paths = [] - all_variables = {} + all_config_base_vars = {} + all_car_vars = {} all_env = {} for n in name: @@ -43,7 +44,8 @@ def load_car(repo, name, car_params=None): for p in descriptor.config_paths: if p not in all_config_paths: all_config_paths.append(p) - all_variables.update(descriptor.variables) + all_config_base_vars.update(descriptor.config_base_variables) + all_car_vars.update(descriptor.variables) # env needs to be merged individually, consider ES_JAVA_OPTS="-Xms1G" and ES_JAVA_OPTS="-ea". # We want it to be ES_JAVA_OPTS="-Xms1G -ea" in the end. for k, v in descriptor.env.items(): @@ -56,7 +58,12 @@ def load_car(repo, name, car_params=None): if len(all_config_paths) == 0: raise exceptions.SystemSetupError("At least one config base is required for car %s" % name) - return Car("+".join(name), all_config_paths, all_variables, all_env) + vars = {} + # car variables *always* take precedence over config base variables + vars.update(all_config_base_vars) + vars.update(all_car_vars) + + return Car("+".join(name), all_config_paths, vars, all_env) def list_plugins(cfg): @@ -126,54 +133,68 @@ def __is_car(path): return map(__car_name, filter(__is_car, os.listdir(self.cars_dir))) def _car_file(self, name): - return os.path.join(self.cars_dir, "%s.ini" % name) + return os.path.join(self.cars_dir, "{}.ini".format(name)) def load_car(self, name, car_params=None): car_config_file = self._car_file(name) if not io.exists(car_config_file): - raise exceptions.SystemSetupError("Unknown car [%s]. List the available cars with %s list cars." % (name, PROGRAM_NAME)) - config = configparser.ConfigParser(interpolation=configparser.ExtendedInterpolation()) - # Do not modify the case of option keys but read them as is - config.optionxform = lambda option: option - config.read(car_config_file) + raise exceptions.SystemSetupError("Unknown car [{}]. List the available cars with {} list cars.".format(name, PROGRAM_NAME)) + config = self._config_loader(car_config_file) config_paths = [] - description = "" - car_type = "car" - if "meta" in config: - description = config["meta"].get("description", description) - car_type = config["meta"].get("type", car_type) - - if "config" in config and "base" in config["config"]: - config_bases = config["config"]["base"].split(",") - for base in config_bases: - if base: - config_paths.append(os.path.join(self.cars_dir, base, "templates")) + config_base_vars = {} + description = self._value(config, ["meta", "description"], default="") + car_type = self._value(config, ["meta", "type"], default="car") + config_bases = self._value(config, ["config", "base"], default="").split(",") + for base in config_bases: + if base: + config_paths.append(os.path.join(self.cars_dir, base, "templates")) + config_file = os.path.join(self.cars_dir, base, "config.ini") + if io.exists(config_file): + base_config = self._config_loader(config_file) + self._copy_section(base_config, "variables", config_base_vars) # it's possible that some cars don't have a config base, e.g. mixins which only override variables if len(config_paths) == 0: - logger.info("Car [%s] does not define any config paths. Assuming that it is used as a mixin." % name) - - variables = {} - if "variables" in config.sections(): - for k, v in config["variables"].items(): - variables[k] = v + logger.info("Car [%s] does not define any config paths. Assuming that it is used as a mixin.", name) + variables = self._copy_section(config, "variables", {}) # add all car params here to override any defaults if car_params: variables.update(car_params) - env = {} - if "env" in config.sections(): - for k, v in config["env"].items(): - env[k] = v - return CarDescriptor(name, description, car_type, config_paths, variables, env) + env = self._copy_section(config, "env", {}) + return CarDescriptor(name, description, car_type, config_paths, config_base_vars, variables, env) + + def _config_loader(self, file_name): + config = configparser.ConfigParser(interpolation=configparser.ExtendedInterpolation()) + # Do not modify the case of option keys but read them as is + config.optionxform = lambda option: option + config.read(file_name) + return config + + def _value(self, cfg, section_path, default=None): + path = [section_path] if (isinstance(section_path, str)) else section_path + current_cfg = cfg + for k in path: + if k in current_cfg: + current_cfg = current_cfg[k] + else: + return default + return current_cfg + + def _copy_section(self, cfg, section, target): + if section in cfg.sections(): + for k, v in cfg[section].items(): + target[k] = v + return target class CarDescriptor: - def __init__(self, name, description, type, config_paths, variables, env): + def __init__(self, name, description, type, config_paths, config_base_variables, variables, env): self.name = name self.description = description self.type = type self.config_paths = config_paths + self.config_base_variables = config_base_variables self.variables = variables self.env = env diff --git a/tests/mechanic/data/cars/v1/vanilla/config.ini b/tests/mechanic/data/cars/v1/vanilla/config.ini new file mode 100644 index 000000000..da1da8c18 --- /dev/null +++ b/tests/mechanic/data/cars/v1/vanilla/config.ini @@ -0,0 +1,5 @@ +[variables] +# this should *always* be overridden by the car definition (config base +# variables take least precedence) +heap_size=0 +clean_command=./gradlew clean \ No newline at end of file diff --git a/tests/mechanic/data/cars/v1/verbose_logging/config.ini b/tests/mechanic/data/cars/v1/verbose_logging/config.ini new file mode 100644 index 000000000..6064da6df --- /dev/null +++ b/tests/mechanic/data/cars/v1/verbose_logging/config.ini @@ -0,0 +1,2 @@ +[variables] +verbose_logging=true \ No newline at end of file diff --git a/tests/mechanic/team_test.py b/tests/mechanic/team_test.py index 68bf8d0b6..f066c6c34 100644 --- a/tests/mechanic/team_test.py +++ b/tests/mechanic/team_test.py @@ -25,14 +25,22 @@ def test_load_known_car(self): car = team.load_car(self.team_dir, ["default"], car_params={"data_paths": ["/mnt/disk0", "/mnt/disk1"]}) self.assertEqual("default", car.name) self.assertEqual([os.path.join(current_dir, "data", "cars", "v1", "vanilla", "templates")], car.config_paths) - self.assertDictEqual({"heap_size": "1g", "data_paths": ["/mnt/disk0", "/mnt/disk1"]}, car.variables) + self.assertDictEqual({ + "heap_size": "1g", + "clean_command": "./gradlew clean", + "data_paths": ["/mnt/disk0", "/mnt/disk1"] + }, car.variables) self.assertEqual({}, car.env) def test_load_car_with_mixin_single_config_base(self): car = team.load_car(self.team_dir, ["32gheap", "ea"]) self.assertEqual("32gheap+ea", car.name) self.assertEqual([os.path.join(current_dir, "data", "cars", "v1", "vanilla", "templates")], car.config_paths) - self.assertEqual({"heap_size": "32g", "assertions": "true"}, car.variables) + self.assertEqual({ + "heap_size": "32g", + "clean_command": "./gradlew clean", + "assertions": "true" + }, car.variables) self.assertEqual({"JAVA_TOOL_OPTS": "A B C D E F"}, car.env) def test_load_car_with_mixin_multiple_config_bases(self): @@ -42,7 +50,12 @@ def test_load_car_with_mixin_multiple_config_bases(self): os.path.join(current_dir, "data", "cars", "v1", "vanilla", "templates"), os.path.join(current_dir, "data", "cars", "v1", "verbose_logging", "templates"), ], car.config_paths) - self.assertEqual({"heap_size": "32g", "assertions": "true"}, car.variables) + self.assertEqual({ + "heap_size": "32g", + "clean_command": "./gradlew clean", + "verbose_logging": "true", + "assertions": "true" + }, car.variables) self.assertEqual({"JAVA_TOOL_OPTS": "A B C D E F G H I"}, car.env) def test_raises_error_on_unknown_car(self): From ca27b0104b79d8e749e816251cc54fd36c5c45be Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Thu, 26 Apr 2018 13:41:25 +0200 Subject: [PATCH 04/12] Externalize Elasticsearch build commands --- esrally/mechanic/mechanic.py | 2 +- esrally/mechanic/supplier.py | 36 ++++++++++--------- tests/mechanic/supplier_test.py | 64 +++++++++++++++++++++++++-------- 3 files changed, 71 insertions(+), 31 deletions(-) diff --git a/esrally/mechanic/mechanic.py b/esrally/mechanic/mechanic.py index 916828309..9ef5caa5d 100644 --- a/esrally/mechanic/mechanic.py +++ b/esrally/mechanic/mechanic.py @@ -639,7 +639,7 @@ def create(cfg, metrics_store, all_node_ips, cluster_settings=None, sources=Fals car, plugins = load_team(cfg, external) if sources or distribution: - s = supplier.create(cfg, sources, distribution, build, challenge_root_path, plugins) + s = supplier.create(cfg, sources, distribution, build, challenge_root_path, car, plugins) p = [] for node_id in node_ids: p.append(provisioner.local_provisioner(cfg, car, plugins, cluster_settings, all_node_ips, challenge_root_path, node_id)) diff --git a/esrally/mechanic/supplier.py b/esrally/mechanic/supplier.py index 3e65b2fd5..c06b5d5d8 100644 --- a/esrally/mechanic/supplier.py +++ b/esrally/mechanic/supplier.py @@ -10,15 +10,13 @@ logger = logging.getLogger("rally.supplier") -GRADLE_BINARY = "./gradlew" -CLEAN_COMMAND = "{} clean".format(GRADLE_BINARY) -ASSEMBLE_COMMAND = "{} :distribution:archives:tar:assemble".format(GRADLE_BINARY) - # e.g. my-plugin:current - we cannot simply use String#split(":") as this would not work for timestamp-based revisions REVISION_PATTERN = r"(\w.*?):(.*)" -def create(cfg, sources, distribution, build, challenge_root_path, plugins): +def create(cfg, sources, distribution, build, challenge_root_path, car, plugins=None): + if plugins is None: + plugins = [] revisions = _extract_revisions(cfg.opts("mechanic", "source.revision")) distribution_version = cfg.opts("mechanic", "distribution.version", mandatory=False) supply_requirements = _supply_requirements(sources, distribution, build, plugins, revisions, distribution_version) @@ -36,7 +34,8 @@ def create(cfg, sources, distribution, build, challenge_root_path, plugins): es_supplier_type, es_version, es_build = supply_requirements["elasticsearch"] if es_supplier_type == "source": es_src_dir = os.path.join(_src_dir(cfg), _config_value(src_config, "elasticsearch.src.subdir")) - suppliers.append(ElasticsearchSourceSupplier(es_version, es_src_dir, remote_url=cfg.opts("source", "remote.repo.url"), builder=builder)) + suppliers.append( + ElasticsearchSourceSupplier(es_version, es_src_dir, remote_url=cfg.opts("source", "remote.repo.url"), car=car, builder=builder)) repo = None else: es_src_dir = None @@ -173,10 +172,11 @@ def __call__(self, *args, **kwargs): class ElasticsearchSourceSupplier: - def __init__(self, revision, es_src_dir, remote_url, builder): + def __init__(self, revision, es_src_dir, remote_url, car, builder): self.revision = revision self.src_dir = es_src_dir self.remote_url = remote_url + self.car = car self.builder = builder def fetch(self): @@ -184,17 +184,23 @@ def fetch(self): def prepare(self): if self.builder: - self.builder.build([CLEAN_COMMAND, ASSEMBLE_COMMAND]) + self.builder.build([self.value_of("clean_command"), self.value_of("build_command")]) def add(self, binaries): binaries["elasticsearch"] = self.resolve_binary() def resolve_binary(self): try: - return glob.glob("%s/distribution/archives/tar/build/distributions/*.tar.gz" % self.src_dir)[0] + return glob.glob("{}/{}".format(self.src_dir, self.value_of("artifact_path_pattern")))[0] except IndexError: raise SystemSetupError("Couldn't find a tar.gz distribution. Please run Rally with the pipeline 'from-sources-complete'.") + def value_of(self, k): + try: + return self.car.variables[k] + except KeyError: + raise exceptions.SystemSetupError("Car '{}' misses config variable '{}' to build Elasticsearch.".format(self.car, k)) + class ExternalPluginSourceSupplier: def __init__(self, plugin, revision, src_dir, src_config, builder): @@ -262,8 +268,7 @@ def fetch(self): def prepare(self): if self.builder: - command = "{} :plugins:{}:assemble".format(GRADLE_BINARY, self.plugin.name) - self.builder.build([command]) + self.builder.build(["./gradlew :plugins:{}:assemble".format(self.plugin.name)]) def add(self, binaries): binaries[self.plugin.name] = self.resolve_binary() @@ -443,18 +448,17 @@ def build(self, commands, override_src_dir=None): self.run(command, override_src_dir) def run(self, command, override_src_dir=None): - from esrally.utils import jvm src_dir = self.src_dir if override_src_dir is None else override_src_dir - logger.info("Building from sources in [{}].".format(src_dir)) - logger.info("Executing {}...".format(command)) + logger.info("Building from sources in [%s].", src_dir) + logger.info("Executing %s...", command) io.ensure_dir(self.log_dir) - log_file = "{}/build.log".format(self.log_dir) + log_file = os.path.join(self.log_dir, "build.log") # we capture all output to a dedicated build log file build_cmd = "export JAVA_HOME={}; cd {}; {} >> {} 2>&1".format(self.java_home, src_dir, command, log_file) - logger.info("Running build command [{}]".format(build_cmd)) + logger.info("Running build command [%s]", build_cmd) if process.run_subprocess(build_cmd): msg = "Executing '{}' failed. The last 20 lines in the build log file are:\n".format(command) diff --git a/tests/mechanic/supplier_test.py b/tests/mechanic/supplier_test.py index 41d609fcc..79bf93433 100644 --- a/tests/mechanic/supplier_test.py +++ b/tests/mechanic/supplier_test.py @@ -121,13 +121,13 @@ def test_build_on_jdk_8(self, jvm_major_version, mock_run_subprocess): mock_run_subprocess.return_value = False b = supplier.Builder(src_dir="/src", java_home="/opt/jdk8", log_dir="logs") - b.build([supplier.CLEAN_COMMAND, supplier.ASSEMBLE_COMMAND]) + b.build(["./gradlew clean", "./gradlew assemble"]) calls = [ # Actual call mock.call("export JAVA_HOME=/opt/jdk8; cd /src; ./gradlew clean >> logs/build.log 2>&1"), # Return value check - mock.call("export JAVA_HOME=/opt/jdk8; cd /src; ./gradlew :distribution:archives:tar:assemble >> logs/build.log 2>&1"), + mock.call("export JAVA_HOME=/opt/jdk8; cd /src; ./gradlew assemble >> logs/build.log 2>&1"), ] mock_run_subprocess.assert_has_calls(calls) @@ -139,14 +139,13 @@ def test_build_on_jdk_10(self, jvm_major_version, mock_run_subprocess): mock_run_subprocess.return_value = False b = supplier.Builder(src_dir="/src", java_home="/opt/jdk10", log_dir="logs") - b.build([supplier.CLEAN_COMMAND, supplier.ASSEMBLE_COMMAND]) + b.build(["./gradlew clean", "./gradlew assemble"]) calls = [ # Actual call mock.call("export JAVA_HOME=/opt/jdk10; cd /src; ./gradlew clean >> logs/build.log 2>&1"), # Return value check - mock.call("export JAVA_HOME=/opt/jdk10; cd /src; ./gradlew " - ":distribution:archives:tar:assemble >> logs/build.log 2>&1"), + mock.call("export JAVA_HOME=/opt/jdk10; cd /src; ./gradlew assemble >> logs/build.log 2>&1"), ] mock_run_subprocess.assert_has_calls(calls) @@ -154,20 +153,47 @@ def test_build_on_jdk_10(self, jvm_major_version, mock_run_subprocess): class ElasticsearchSourceSupplierTests(TestCase): def test_no_build(self): - es = supplier.ElasticsearchSourceSupplier(revision="abc", es_src_dir="/src", remote_url="", builder=None) + car = team.Car("default", config_paths=[], variables={ + "clean_command": "./gradlew clean", + "build_command": "./gradlew assemble" + }) + es = supplier.ElasticsearchSourceSupplier(revision="abc", es_src_dir="/src", remote_url="", car=car, builder=None) es.prepare() # nothing has happened (intentionally) because there is no builder def test_build(self): + car = team.Car("default", config_paths=[], variables={ + "clean_command": "./gradlew clean", + "build_command": "./gradlew assemble" + }) builder = mock.create_autospec(supplier.Builder) - es = supplier.ElasticsearchSourceSupplier(revision="abc", es_src_dir="/src", remote_url="", builder=builder) + es = supplier.ElasticsearchSourceSupplier(revision="abc", es_src_dir="/src", remote_url="", car=car, builder=builder) es.prepare() - builder.build.assert_called_once_with([supplier.CLEAN_COMMAND, supplier.ASSEMBLE_COMMAND]) + builder.build.assert_called_once_with(["./gradlew clean", "./gradlew assemble"]) + + def test_raises_error_on_missing_car_variable(self): + car = team.Car("default", config_paths=[], variables={ + "clean_command": "./gradlew clean", + # build_command is not defined + }) + builder = mock.create_autospec(supplier.Builder) + es = supplier.ElasticsearchSourceSupplier(revision="abc", es_src_dir="/src", remote_url="", car=car, builder=builder) + with self.assertRaisesRegex(exceptions.SystemSetupError, + "Car 'default' misses config variable 'build_command' to build Elasticsearch."): + es.prepare() + + builder.build.assert_not_called() + @mock.patch("glob.glob", lambda p: ["elasticsearch.tar.gz"]) def test_add_elasticsearch_binary(self): - es = supplier.ElasticsearchSourceSupplier(revision="abc", es_src_dir="/src", remote_url="", builder=None) + car = team.Car("default", config_paths=[], variables={ + "clean_command": "./gradlew clean", + "build_command": "./gradlew assemble", + "artifact_path_pattern": "distribution/archives/tar/build/distributions/*.tar.gz" + }) + es = supplier.ElasticsearchSourceSupplier(revision="abc", es_src_dir="/src", remote_url="", car=car, builder=None) binaries = {} es.add(binaries=binaries) self.assertEqual(binaries, {"elasticsearch": "elasticsearch.tar.gz"}) @@ -337,7 +363,9 @@ def test_create_suppliers_for_es_only_config(self): cfg.add(config.Scope.application, "runtime", "java10.home", "/usr/local/bin/java10/") cfg.add(config.Scope.application, "node", "root.dir", "/opt/rally") - composite_supplier = supplier.create(cfg, sources=False, distribution=True, build=False, challenge_root_path="/", plugins=[]) + car = team.Car("default", config_paths=[]) + + composite_supplier = supplier.create(cfg, sources=False, distribution=True, build=False, challenge_root_path="/", car=car) self.assertEqual(1, len(composite_supplier.suppliers)) self.assertIsInstance(composite_supplier.suppliers[0], supplier.ElasticsearchDistributionSupplier) @@ -355,11 +383,12 @@ def test_create_suppliers_for_es_distribution_plugin_source_skip(self): cfg.add(config.Scope.application, "node", "root.dir", "/opt/rally") cfg.add(config.Scope.application, "source", "plugin.community-plugin.src.dir", "/home/user/Projects/community-plugin") + car = team.Car("default", config_paths=[]) core_plugin = team.PluginDescriptor("analysis-icu", core_plugin=True) external_plugin = team.PluginDescriptor("community-plugin", core_plugin=False) # --pipeline=from-sources-skip-build - composite_supplier = supplier.create(cfg, sources=True, distribution=False, build=False, challenge_root_path="/", plugins=[ + composite_supplier = supplier.create(cfg, sources=True, distribution=False, build=False, challenge_root_path="/", car=car, plugins=[ core_plugin, external_plugin ]) @@ -388,9 +417,11 @@ def test_create_suppliers_for_es_missing_distribution_plugin_source_skip(self): core_plugin = team.PluginDescriptor("analysis-icu", core_plugin=True) external_plugin = team.PluginDescriptor("community-plugin", core_plugin=False) + car = team.Car("default", config_paths=[]) + # --from-sources-skip-build --revision="community-plugin:current" (distribution version is missing!) with self.assertRaises(exceptions.SystemSetupError) as ctx: - supplier.create(cfg, sources=True, distribution=False, build=False, challenge_root_path="/", plugins=[ + supplier.create(cfg, sources=True, distribution=False, build=False, challenge_root_path="/", car=car, plugins=[ core_plugin, external_plugin ]) @@ -411,11 +442,12 @@ def test_create_suppliers_for_es_distribution_plugin_source_build(self): cfg.add(config.Scope.application, "source", "elasticsearch.src.subdir", "elasticsearch") cfg.add(config.Scope.application, "source", "plugin.community-plugin.src.dir", "/home/user/Projects/community-plugin") + car = team.Car("default", config_paths=[]) core_plugin = team.PluginDescriptor("analysis-icu", core_plugin=True) external_plugin = team.PluginDescriptor("community-plugin", core_plugin=False) # --revision="community-plugin:effab" --distribution-version="6.0.0" - composite_supplier = supplier.create(cfg, sources=False, distribution=True, build=False, challenge_root_path="/", plugins=[ + composite_supplier = supplier.create(cfg, sources=False, distribution=True, build=False, challenge_root_path="/", car=car, plugins=[ core_plugin, external_plugin ]) @@ -442,11 +474,15 @@ def test_create_suppliers_for_es_and_plugin_source_build(self): cfg.add(config.Scope.application, "source", "remote.repo.url", "https://github.com/elastic/elasticsearch.git") cfg.add(config.Scope.application, "source", "plugin.community-plugin.src.subdir", "elasticsearch-extra/community-plugin") + car = team.Car("default", config_paths=[], variables={ + "clean_command": "./gradlew clean", + "build_command": "./gradlew assemble" + }) core_plugin = team.PluginDescriptor("analysis-icu", core_plugin=True) external_plugin = team.PluginDescriptor("community-plugin", core_plugin=False) # --revision="elasticsearch:abc,community-plugin:effab" - composite_supplier = supplier.create(cfg, sources=True, distribution=False, build=True, challenge_root_path="/", plugins=[ + composite_supplier = supplier.create(cfg, sources=True, distribution=False, build=True, challenge_root_path="/", car=car, plugins=[ core_plugin, external_plugin ]) From 7b81007e909d45b9cf5cdd7505351f20cbcc29a7 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Thu, 26 Apr 2018 15:55:15 +0200 Subject: [PATCH 05/12] Invoke install hooks for cars --- esrally/mechanic/mechanic.py | 6 +- esrally/mechanic/provisioner.py | 11 ++- esrally/mechanic/team.py | 67 ++++++++++++----- tests/mechanic/data/cars/v1/hook2/config.py | 1 + .../mechanic/data/cars/v1/too_many_hooks.ini | 6 ++ tests/mechanic/data/cars/v1/with_hook.ini | 6 ++ .../mechanic/data/cars/v1/with_hook/config.py | 1 + tests/mechanic/provisioner_test.py | 71 ++++++++++++------- tests/mechanic/supplier_test.py | 19 +++-- tests/mechanic/team_test.py | 30 +++++++- tests/mechanic/telemetry_test.py | 2 +- 11 files changed, 163 insertions(+), 57 deletions(-) create mode 100644 tests/mechanic/data/cars/v1/hook2/config.py create mode 100644 tests/mechanic/data/cars/v1/too_many_hooks.ini create mode 100644 tests/mechanic/data/cars/v1/with_hook.ini create mode 100644 tests/mechanic/data/cars/v1/with_hook/config.py diff --git a/esrally/mechanic/mechanic.py b/esrally/mechanic/mechanic.py index 9ef5caa5d..166ca94fe 100644 --- a/esrally/mechanic/mechanic.py +++ b/esrally/mechanic/mechanic.py @@ -232,7 +232,7 @@ def __init__(self): self.race_control = None self.cluster_launcher = None self.cluster = None - self.plugins = None + self.car = None def receiveUnrecognizedMessage(self, msg, sender): logger.info("MechanicActor#receiveMessage unrecognized(msg = [%s] sender = [%s])" % (str(type(msg)), str(sender))) @@ -263,7 +263,7 @@ def receiveMsg_StartEngine(self, msg, sender): cls = metrics.metrics_store_class(self.cfg) self.metrics_store = cls(self.cfg) self.metrics_store.open(ctx=msg.open_metrics_context) - _, self.plugins = load_team(self.cfg, msg.external) + self.car, _ = load_team(self.cfg, msg.external) # In our startup procedure we first create all mechanics. Only if this succeeds we'll continue. hosts = self.cfg.opts("client", "hosts") @@ -360,7 +360,7 @@ def receiveMsg_NodesStopped(self, msg, sender): self.transition_when_all_children_responded(sender, msg, "cluster_stopping", "cluster_stopped", self.on_all_nodes_stopped) def on_all_nodes_started(self): - self.cluster_launcher = launcher.ClusterLauncher(self.cfg, self.metrics_store, on_post_launch=PostLaunchHandler(self.plugins)) + self.cluster_launcher = launcher.ClusterLauncher(self.cfg, self.metrics_store, on_post_launch=PostLaunchHandler([self.car])) # Workaround because we could raise a LaunchError here and thespian will attempt to retry a failed message. # In that case, we will get a followup RallyAssertionError because on the second attempt, Rally will check # the status which is now "nodes_started" but we expected the status to be "nodes_starting" previously. diff --git a/esrally/mechanic/provisioner.py b/esrally/mechanic/provisioner.py index 0ded69858..8d2e6e3cb 100644 --- a/esrally/mechanic/provisioner.py +++ b/esrally/mechanic/provisioner.py @@ -156,8 +156,9 @@ def prepare(self, binary): for plugin_config_path in installer.config_source_paths: self.apply_config(plugin_config_path, target_root_path, provisioner_vars) + # Never let install hooks modify our original provisioner variables and just provide a copy! + self.es_installer.invoke_install_hook(team.BootstrapPhase.post_install, provisioner_vars.copy()) for installer in self.plugin_installers: - # Never let install hooks modify our original provisioner variables and just provide a copy! installer.invoke_install_hook(team.BootstrapPhase.post_install, provisioner_vars.copy()) return NodeConfiguration(self.es_installer.car, self.es_installer.node_ip, self.es_installer.node_name, @@ -203,7 +204,7 @@ def _provisioner_variables(self): class ElasticsearchInstaller: - def __init__(self, car, node_name, node_root_dir, all_node_ips, ip, http_port): + def __init__(self, car, node_name, node_root_dir, all_node_ips, ip, http_port, hook_handler_class=team.BootstrapHookHandler): self.car = car self.node_name = node_name self.node_root_dir = node_root_dir @@ -213,6 +214,9 @@ def __init__(self, car, node_name, node_root_dir, all_node_ips, ip, http_port): self.all_node_ips = all_node_ips self.node_ip = ip self.http_port = http_port + self.hook_handler = hook_handler_class(self.car) + if self.hook_handler.can_load(): + self.hook_handler.load() self.es_home_path = None self.data_paths = None @@ -232,6 +236,9 @@ def delete_pre_bundled_configuration(self): logger.info("Deleting pre-bundled Elasticsearch configuration at [%s]" % config_path) shutil.rmtree(config_path) + def invoke_install_hook(self, phase, variables): + self.hook_handler.invoke(phase.name, variables=variables) + def cleanup(self, preserve): cleanup(preserve, self.install_dir, self.data_paths) diff --git a/esrally/mechanic/team.py b/esrally/mechanic/team.py index e499cd788..e8e9929f4 100644 --- a/esrally/mechanic/team.py +++ b/esrally/mechanic/team.py @@ -33,6 +33,12 @@ def list_cars(cfg): def load_car(repo, name, car_params=None): + class Component: + def __init__(self, root_path, entry_point): + self.root_path = root_path + self.entry_point = entry_point + + root_path = None # preserve order as we append to existing config files later during provisioning. all_config_paths = [] all_config_base_vars = {} @@ -44,6 +50,13 @@ def load_car(repo, name, car_params=None): for p in descriptor.config_paths: if p not in all_config_paths: all_config_paths.append(p) + for p in descriptor.root_paths: + # probe whether we have a root path + if BootstrapHookHandler(Component(root_path=p, entry_point=Car.entry_point)).can_load(): + if not root_path: + root_path = p + else: + raise exceptions.SystemSetupError("Invalid car: {}. Multiple bootstrap hooks are forbidden.".format(name)) all_config_base_vars.update(descriptor.config_base_variables) all_car_vars.update(descriptor.variables) # env needs to be merged individually, consider ES_JAVA_OPTS="-Xms1G" and ES_JAVA_OPTS="-ea". @@ -57,13 +70,13 @@ def load_car(repo, name, car_params=None): all_env[k] = all_env[k] + " " + v if len(all_config_paths) == 0: - raise exceptions.SystemSetupError("At least one config base is required for car %s" % name) - vars = {} + raise exceptions.SystemSetupError("At least one config base is required for car {}".format(name)) + variables = {} # car variables *always* take precedence over config base variables - vars.update(all_config_base_vars) - vars.update(all_car_vars) + variables.update(all_config_base_vars) + variables.update(all_car_vars) - return Car("+".join(name), all_config_paths, vars, all_env) + return Car(name, root_path, all_config_paths, variables, all_env) def list_plugins(cfg): @@ -140,6 +153,7 @@ def load_car(self, name, car_params=None): if not io.exists(car_config_file): raise exceptions.SystemSetupError("Unknown car [{}]. List the available cars with {} list cars.".format(name, PROGRAM_NAME)) config = self._config_loader(car_config_file) + root_paths = [] config_paths = [] config_base_vars = {} description = self._value(config, ["meta", "description"], default="") @@ -147,8 +161,10 @@ def load_car(self, name, car_params=None): config_bases = self._value(config, ["config", "base"], default="").split(",") for base in config_bases: if base: - config_paths.append(os.path.join(self.cars_dir, base, "templates")) - config_file = os.path.join(self.cars_dir, base, "config.ini") + root_path = os.path.join(self.cars_dir, base) + root_paths.append(root_path) + config_paths.append(os.path.join(root_path, "templates")) + config_file = os.path.join(root_path, "config.ini") if io.exists(config_file): base_config = self._config_loader(config_file) self._copy_section(base_config, "variables", config_base_vars) @@ -162,7 +178,7 @@ def load_car(self, name, car_params=None): variables.update(car_params) env = self._copy_section(config, "env", {}) - return CarDescriptor(name, description, car_type, config_paths, config_base_vars, variables, env) + return CarDescriptor(name, description, car_type, root_paths, config_paths, config_base_vars, variables, env) def _config_loader(self, file_name): config = configparser.ConfigParser(interpolation=configparser.ExtendedInterpolation()) @@ -189,10 +205,11 @@ def _copy_section(self, cfg, section, target): class CarDescriptor: - def __init__(self, name, description, type, config_paths, config_base_variables, variables, env): + def __init__(self, name, description, type, root_paths, config_paths, config_base_variables, variables, env): self.name = name self.description = description self.type = type + self.root_paths = root_paths self.config_paths = config_paths self.config_base_variables = config_base_variables self.variables = variables @@ -206,11 +223,15 @@ def __eq__(self, other): class Car: - def __init__(self, name, config_paths, variables=None, env=None): + # name of the initial Python file to load for cars. + entry_point = "config" + + def __init__(self, names, root_path, config_paths, variables=None, env=None): """ Creates new settings for a benchmark candidate. - :param name: A descriptive name for this car. + :param names: Descriptive name(s) for this car. + :param root_path: The root path from which bootstrap hooks should be loaded if any. May be ``None``. :param config_paths: A non-empty list of paths where the raw config can be found. :param variables: A dict containing variable definitions that need to be replaced. :param env: Environment variables that should be set when launching the benchmark candidate. @@ -219,14 +240,27 @@ def __init__(self, name, config_paths, variables=None, env=None): env = {} if variables is None: variables = {} - self.name = name + if isinstance(names, str): + self.names = [names] + else: + self.names = names + self.root_path = root_path self.config_paths = config_paths self.variables = variables self.env = env + @property + def name(self): + return "+".join(self.names) + + # Adapter method for BootstrapHookHandler + @property + def config(self): + return self.name + @property def safe_name(self): - return self.name.replace("+", "_") + return "_".join(self.names) def __str__(self): return self.name @@ -348,6 +382,9 @@ def load_plugin(self, name, config_names, plugin_params=None): class PluginDescriptor: + # name of the initial Python file to load for plugins. + entry_point = "plugin" + def __init__(self, name, core_plugin=False, config=None, root_path=None, config_paths=None, variables=None): if config_paths is None: config_paths = [] @@ -359,8 +396,6 @@ def __init__(self, name, core_plugin=False, config=None, root_path=None, config_ self.root_path = root_path self.config_paths = config_paths self.variables = variables - # name of the initial Python file to load for plugins. - self.entry_point = "plugin" def __str__(self): return "Plugin descriptor for [%s]" % self.name @@ -402,7 +437,7 @@ def __init__(self, component, loader_class=modules.ComponentLoader): """ Creates a new BootstrapHookHandler. - :param component: The component that should be loaded. In practice, this is a PluginDescriptor instance. + :param component: The component that should be loaded. In practice, this is a PluginDescriptor or a Car instance. :param loader_class: The implementation that loads the provided component's code. """ self.component = component diff --git a/tests/mechanic/data/cars/v1/hook2/config.py b/tests/mechanic/data/cars/v1/hook2/config.py new file mode 100644 index 000000000..e547190ba --- /dev/null +++ b/tests/mechanic/data/cars/v1/hook2/config.py @@ -0,0 +1 @@ +# Empty placeholder. We just need this file to verify that Rally will recognize it as an install hook entry point. diff --git a/tests/mechanic/data/cars/v1/too_many_hooks.ini b/tests/mechanic/data/cars/v1/too_many_hooks.ini new file mode 100644 index 000000000..2f1068133 --- /dev/null +++ b/tests/mechanic/data/cars/v1/too_many_hooks.ini @@ -0,0 +1,6 @@ +[meta] +type=mixin +description=A car with multiple hooks + +[config] +base=vanilla,with_hook,hook2 diff --git a/tests/mechanic/data/cars/v1/with_hook.ini b/tests/mechanic/data/cars/v1/with_hook.ini new file mode 100644 index 000000000..59350fbf6 --- /dev/null +++ b/tests/mechanic/data/cars/v1/with_hook.ini @@ -0,0 +1,6 @@ +[meta] +type=mixin +description=A car with an install hook + +[config] +base=vanilla,with_hook diff --git a/tests/mechanic/data/cars/v1/with_hook/config.py b/tests/mechanic/data/cars/v1/with_hook/config.py new file mode 100644 index 000000000..e547190ba --- /dev/null +++ b/tests/mechanic/data/cars/v1/with_hook/config.py @@ -0,0 +1 @@ +# Empty placeholder. We just need this file to verify that Rally will recognize it as an install hook entry point. diff --git a/tests/mechanic/provisioner_test.py b/tests/mechanic/provisioner_test.py index 1705c8447..022e403d3 100644 --- a/tests/mechanic/provisioner_test.py +++ b/tests/mechanic/provisioner_test.py @@ -21,7 +21,8 @@ def null_apply_config(source_root_path, target_root_path, config_vars): installer = provisioner.ElasticsearchInstaller(car= team.Car( - name="unit-test-car", + names="unit-test-car", + root_path=None, config_paths=["~/.rally/benchmarks/teams/default/my-car"], variables={"heap": "4g"}), node_name="rally-node-0", @@ -66,7 +67,6 @@ def null_apply_config(source_root_path, target_root_path, config_vars): "install_root_path": "/opt/elasticsearch-5.0.0" }, config_vars) - class NoopHookHandler: def __init__(self, plugin): self.hook_calls = {} @@ -123,7 +123,8 @@ def null_apply_config(source_root_path, target_root_path, config_vars): installer = provisioner.ElasticsearchInstaller(car= team.Car( - name="unit-test-car", + names="unit-test-car", + root_path=None, config_paths=["~/.rally/benchmarks/teams/default/my-car"], variables={"heap": "4g"}), node_name="rally-node-0", @@ -198,7 +199,8 @@ def null_apply_config(source_root_path, target_root_path, config_vars): installer = provisioner.ElasticsearchInstaller(car= team.Car( - name="unit-test-car", + names="unit-test-car", + root_path=None, config_paths=["~/.rally/benchmarks/teams/default/my-car"], variables={"heap": "4g"}), node_name="rally-node-0", @@ -255,13 +257,24 @@ def null_apply_config(source_root_path, target_root_path, config_vars): }, config_vars) +class NoopHookHandler: + def __init__(self, component): + self.hook_calls = {} + + def can_load(self): + return False + + def invoke(self, phase, variables): + self.hook_calls[phase] = variables + + class ElasticsearchInstallerTests(TestCase): @mock.patch("shutil.rmtree") @mock.patch("os.path.exists") def test_cleanup_nothing_on_preserve(self, mock_path_exists, mock_rm): mock_path_exists.return_value = False - installer = provisioner.ElasticsearchInstaller(car=team.Car("defaults", "/tmp"), + installer = provisioner.ElasticsearchInstaller(car=team.Car("defaults", None, "/tmp"), node_name="rally-node-0", all_node_ips={"127.0.0.1"}, ip="127.0.0.1", @@ -277,7 +290,8 @@ def test_cleanup_nothing_on_preserve(self, mock_path_exists, mock_rm): def test_cleanup(self, mock_path_exists, mock_rm): mock_path_exists.return_value = True - installer = provisioner.ElasticsearchInstaller(car=team.Car(name="defaults", + installer = provisioner.ElasticsearchInstaller(car=team.Car(names="defaults", + root_path=None, config_paths="/tmp", variables={"data_paths": "/tmp/some/data-path-dir"}), node_name="rally-node-0", @@ -296,7 +310,8 @@ def test_cleanup(self, mock_path_exists, mock_rm): @mock.patch("esrally.utils.io.ensure_dir") @mock.patch("shutil.rmtree") def test_prepare_default_data_paths(self, mock_rm, mock_ensure_dir, mock_decompress): - installer = provisioner.ElasticsearchInstaller(car=team.Car(name="defaults", + installer = provisioner.ElasticsearchInstaller(car=team.Car(names="defaults", + root_path=None, config_paths="/tmp"), node_name="rally-node-0", all_node_ips=["10.17.22.22", "10.17.22.23"], @@ -330,7 +345,8 @@ def test_prepare_default_data_paths(self, mock_rm, mock_ensure_dir, mock_decompr @mock.patch("esrally.utils.io.ensure_dir") @mock.patch("shutil.rmtree") def test_prepare_user_provided_data_path(self, mock_rm, mock_ensure_dir, mock_decompress): - installer = provisioner.ElasticsearchInstaller(car=team.Car(name="defaults", + installer = provisioner.ElasticsearchInstaller(car=team.Car(names="defaults", + root_path=None, config_paths="/tmp", variables={"data_paths": "/tmp/some/data-path-dir"}), node_name="rally-node-0", @@ -360,24 +376,31 @@ def test_prepare_user_provided_data_path(self, mock_rm, mock_ensure_dir, mock_de self.assertEqual(installer.data_paths, ["/tmp/some/data-path-dir"]) + def test_invokes_hook(self): + installer = provisioner.ElasticsearchInstaller(car=team.Car(names="defaults", + root_path="/tmp", + config_paths="/tmp/templates", + variables={"data_paths": "/tmp/some/data-path-dir"}), + node_name="rally-node-0", + all_node_ips=["10.17.22.22", "10.17.22.23"], + ip="10.17.22.23", + http_port=9200, + node_root_dir="~/.rally/benchmarks/races/unittest", + hook_handler_class=NoopHookHandler) -class PluginInstallerTests(TestCase): - class NoopHookHandler: - def __init__(self, plugin): - self.hook_calls = {} - - def can_load(self): - return False + self.assertEqual(0, len(installer.hook_handler.hook_calls)) + installer.invoke_install_hook(team.BootstrapPhase.post_install, {"foo": "bar"}) + self.assertEqual(1, len(installer.hook_handler.hook_calls)) + self.assertEqual({"foo": "bar"}, installer.hook_handler.hook_calls["post_install"]) - def invoke(self, phase, variables): - self.hook_calls[phase] = variables +class PluginInstallerTests(TestCase): @mock.patch("esrally.utils.process.run_subprocess_with_logging") def test_install_plugin_successfully(self, installer_subprocess): installer_subprocess.return_value = 0 plugin = team.PluginDescriptor(name="unit-test-plugin", config="default", variables={"active": True}) - installer = provisioner.PluginInstaller(plugin, hook_handler_class=PluginInstallerTests.NoopHookHandler) + installer = provisioner.PluginInstaller(plugin, hook_handler_class=NoopHookHandler) installer.install(es_home_path="/opt/elasticsearch") @@ -389,7 +412,7 @@ def test_install_unknown_plugin(self, installer_subprocess): installer_subprocess.return_value = 64 plugin = team.PluginDescriptor(name="unknown") - installer = provisioner.PluginInstaller(plugin, hook_handler_class=PluginInstallerTests.NoopHookHandler) + installer = provisioner.PluginInstaller(plugin, hook_handler_class=NoopHookHandler) with self.assertRaises(exceptions.SystemSetupError) as ctx: installer.install(es_home_path="/opt/elasticsearch") @@ -403,7 +426,7 @@ def test_install_plugin_with_io_error(self, installer_subprocess): installer_subprocess.return_value = 74 plugin = team.PluginDescriptor(name="simple") - installer = provisioner.PluginInstaller(plugin, hook_handler_class=PluginInstallerTests.NoopHookHandler) + installer = provisioner.PluginInstaller(plugin, hook_handler_class=NoopHookHandler) with self.assertRaises(exceptions.SupplyError) as ctx: installer.install(es_home_path="/opt/elasticsearch") @@ -417,7 +440,7 @@ def test_install_plugin_with_unknown_error(self, installer_subprocess): installer_subprocess.return_value = 12987 plugin = team.PluginDescriptor(name="simple") - installer = provisioner.PluginInstaller(plugin, hook_handler_class=PluginInstallerTests.NoopHookHandler) + installer = provisioner.PluginInstaller(plugin, hook_handler_class=NoopHookHandler) with self.assertRaises(exceptions.RallyError) as ctx: installer.install(es_home_path="/opt/elasticsearch") @@ -428,7 +451,7 @@ def test_install_plugin_with_unknown_error(self, installer_subprocess): def test_pass_plugin_properties(self): plugin = team.PluginDescriptor(name="unit-test-plugin", config="default", config_paths=["/etc/plugin"], variables={"active": True}) - installer = provisioner.PluginInstaller(plugin, hook_handler_class=PluginInstallerTests.NoopHookHandler) + installer = provisioner.PluginInstaller(plugin, hook_handler_class=NoopHookHandler) self.assertEqual("unit-test-plugin", installer.plugin_name) self.assertEqual({"active": True}, installer.variables) @@ -436,7 +459,7 @@ def test_pass_plugin_properties(self): def test_invokes_hook(self): plugin = team.PluginDescriptor(name="unit-test-plugin", config="default", config_paths=["/etc/plugin"], variables={"active": True}) - installer = provisioner.PluginInstaller(plugin, hook_handler_class=PluginInstallerTests.NoopHookHandler) + installer = provisioner.PluginInstaller(plugin, hook_handler_class=NoopHookHandler) self.assertEqual(0, len(installer.hook_handler.hook_calls)) installer.invoke_install_hook(team.BootstrapPhase.post_install, {"foo": "bar"}) @@ -456,7 +479,7 @@ def test_provisioning(self, uuid4, total_memory): rally_root = os.path.normpath(os.path.join(os.path.dirname(os.path.realpath(__file__)), "../../esrally")) - c = team.Car("unit-test-car", "/tmp", variables={ + c = team.Car("unit-test-car", None, "/tmp", variables={ "xpack.security.enabled": False }) diff --git a/tests/mechanic/supplier_test.py b/tests/mechanic/supplier_test.py index 79bf93433..6c866a8b4 100644 --- a/tests/mechanic/supplier_test.py +++ b/tests/mechanic/supplier_test.py @@ -153,7 +153,7 @@ def test_build_on_jdk_10(self, jvm_major_version, mock_run_subprocess): class ElasticsearchSourceSupplierTests(TestCase): def test_no_build(self): - car = team.Car("default", config_paths=[], variables={ + car = team.Car("default", root_path=None, config_paths=[], variables={ "clean_command": "./gradlew clean", "build_command": "./gradlew assemble" }) @@ -162,7 +162,7 @@ def test_no_build(self): # nothing has happened (intentionally) because there is no builder def test_build(self): - car = team.Car("default", config_paths=[], variables={ + car = team.Car("default", root_path=None, config_paths=[], variables={ "clean_command": "./gradlew clean", "build_command": "./gradlew assemble" }) @@ -173,7 +173,7 @@ def test_build(self): builder.build.assert_called_once_with(["./gradlew clean", "./gradlew assemble"]) def test_raises_error_on_missing_car_variable(self): - car = team.Car("default", config_paths=[], variables={ + car = team.Car("default", root_path=None, config_paths=[], variables={ "clean_command": "./gradlew clean", # build_command is not defined }) @@ -185,10 +185,9 @@ def test_raises_error_on_missing_car_variable(self): builder.build.assert_not_called() - @mock.patch("glob.glob", lambda p: ["elasticsearch.tar.gz"]) def test_add_elasticsearch_binary(self): - car = team.Car("default", config_paths=[], variables={ + car = team.Car("default", root_path=None, config_paths=[], variables={ "clean_command": "./gradlew clean", "build_command": "./gradlew assemble", "artifact_path_pattern": "distribution/archives/tar/build/distributions/*.tar.gz" @@ -363,7 +362,7 @@ def test_create_suppliers_for_es_only_config(self): cfg.add(config.Scope.application, "runtime", "java10.home", "/usr/local/bin/java10/") cfg.add(config.Scope.application, "node", "root.dir", "/opt/rally") - car = team.Car("default", config_paths=[]) + car = team.Car("default", root_path=None, config_paths=[]) composite_supplier = supplier.create(cfg, sources=False, distribution=True, build=False, challenge_root_path="/", car=car) @@ -383,7 +382,7 @@ def test_create_suppliers_for_es_distribution_plugin_source_skip(self): cfg.add(config.Scope.application, "node", "root.dir", "/opt/rally") cfg.add(config.Scope.application, "source", "plugin.community-plugin.src.dir", "/home/user/Projects/community-plugin") - car = team.Car("default", config_paths=[]) + car = team.Car("default", root_path=None, config_paths=[]) core_plugin = team.PluginDescriptor("analysis-icu", core_plugin=True) external_plugin = team.PluginDescriptor("community-plugin", core_plugin=False) @@ -417,7 +416,7 @@ def test_create_suppliers_for_es_missing_distribution_plugin_source_skip(self): core_plugin = team.PluginDescriptor("analysis-icu", core_plugin=True) external_plugin = team.PluginDescriptor("community-plugin", core_plugin=False) - car = team.Car("default", config_paths=[]) + car = team.Car("default", root_path=None, config_paths=[]) # --from-sources-skip-build --revision="community-plugin:current" (distribution version is missing!) with self.assertRaises(exceptions.SystemSetupError) as ctx: @@ -442,7 +441,7 @@ def test_create_suppliers_for_es_distribution_plugin_source_build(self): cfg.add(config.Scope.application, "source", "elasticsearch.src.subdir", "elasticsearch") cfg.add(config.Scope.application, "source", "plugin.community-plugin.src.dir", "/home/user/Projects/community-plugin") - car = team.Car("default", config_paths=[]) + car = team.Car("default", root_path=None, config_paths=[]) core_plugin = team.PluginDescriptor("analysis-icu", core_plugin=True) external_plugin = team.PluginDescriptor("community-plugin", core_plugin=False) @@ -474,7 +473,7 @@ def test_create_suppliers_for_es_and_plugin_source_build(self): cfg.add(config.Scope.application, "source", "remote.repo.url", "https://github.com/elastic/elasticsearch.git") cfg.add(config.Scope.application, "source", "plugin.community-plugin.src.subdir", "elasticsearch-extra/community-plugin") - car = team.Car("default", config_paths=[], variables={ + car = team.Car("default", root_path=None, config_paths=[], variables={ "clean_command": "./gradlew clean", "build_command": "./gradlew assemble" }) diff --git a/tests/mechanic/team_test.py b/tests/mechanic/team_test.py index f066c6c34..8f2118528 100644 --- a/tests/mechanic/team_test.py +++ b/tests/mechanic/team_test.py @@ -19,29 +19,36 @@ def setUp(self): def test_lists_car_names(self): # contrary to the name this assertion compares contents but does not care about order. - self.assertCountEqual(["default", "32gheap", "missing_config_base", "empty_config_base", "ea", "verbose"], self.loader.car_names()) + self.assertCountEqual( + ["default", "with_hook", "32gheap", "missing_config_base", "empty_config_base", "ea", "verbose", "too_many_hooks"], + self.loader.car_names() + ) def test_load_known_car(self): car = team.load_car(self.team_dir, ["default"], car_params={"data_paths": ["/mnt/disk0", "/mnt/disk1"]}) self.assertEqual("default", car.name) self.assertEqual([os.path.join(current_dir, "data", "cars", "v1", "vanilla", "templates")], car.config_paths) + self.assertIsNone(car.root_path) self.assertDictEqual({ "heap_size": "1g", "clean_command": "./gradlew clean", "data_paths": ["/mnt/disk0", "/mnt/disk1"] }, car.variables) self.assertEqual({}, car.env) + self.assertIsNone(car.root_path) def test_load_car_with_mixin_single_config_base(self): car = team.load_car(self.team_dir, ["32gheap", "ea"]) self.assertEqual("32gheap+ea", car.name) self.assertEqual([os.path.join(current_dir, "data", "cars", "v1", "vanilla", "templates")], car.config_paths) + self.assertIsNone(car.root_path) self.assertEqual({ "heap_size": "32g", "clean_command": "./gradlew clean", "assertions": "true" }, car.variables) self.assertEqual({"JAVA_TOOL_OPTS": "A B C D E F"}, car.env) + self.assertIsNone(car.root_path) def test_load_car_with_mixin_multiple_config_bases(self): car = team.load_car(self.team_dir, ["32gheap", "ea", "verbose"]) @@ -50,6 +57,7 @@ def test_load_car_with_mixin_multiple_config_bases(self): os.path.join(current_dir, "data", "cars", "v1", "vanilla", "templates"), os.path.join(current_dir, "data", "cars", "v1", "verbose_logging", "templates"), ], car.config_paths) + self.assertIsNone(car.root_path) self.assertEqual({ "heap_size": "32g", "clean_command": "./gradlew clean", @@ -58,6 +66,21 @@ def test_load_car_with_mixin_multiple_config_bases(self): }, car.variables) self.assertEqual({"JAVA_TOOL_OPTS": "A B C D E F G H I"}, car.env) + def test_load_car_with_install_hook(self): + car = team.load_car(self.team_dir, ["default", "with_hook"], car_params={"data_paths": ["/mnt/disk0", "/mnt/disk1"]}) + self.assertEqual("default+with_hook", car.name) + self.assertEqual([ + os.path.join(current_dir, "data", "cars", "v1", "vanilla", "templates"), + os.path.join(current_dir, "data", "cars", "v1", "with_hook", "templates"), + ], car.config_paths) + self.assertEqual(os.path.join(current_dir, "data", "cars", "v1", "with_hook"), car.root_path) + self.assertDictEqual({ + "heap_size": "1g", + "clean_command": "./gradlew clean", + "data_paths": ["/mnt/disk0", "/mnt/disk1"] + }, car.variables) + self.assertEqual({}, car.env) + def test_raises_error_on_unknown_car(self): with self.assertRaises(exceptions.SystemSetupError) as ctx: team.load_car(self.team_dir, ["don_t-know-you"]) @@ -73,6 +96,11 @@ def test_raises_error_on_missing_config_base(self): team.load_car(self.team_dir, ["missing_config_base"]) self.assertEqual("At least one config base is required for car ['missing_config_base']", ctx.exception.args[0]) + def test_raises_error_if_more_than_one_install_hook(self): + with self.assertRaises(exceptions.SystemSetupError) as ctx: + team.load_car(self.team_dir, ["too_many_hooks"]) + self.assertEqual("Invalid car: ['too_many_hooks']. Multiple bootstrap hooks are forbidden.", ctx.exception.args[0]) + class PluginLoaderTests(TestCase): def __init__(self, args): diff --git a/tests/mechanic/telemetry_test.py b/tests/mechanic/telemetry_test.py index 63d9b1d47..c72fc51c7 100644 --- a/tests/mechanic/telemetry_test.py +++ b/tests/mechanic/telemetry_test.py @@ -48,7 +48,7 @@ def test_merges_options_set_by_different_devices(self): t = telemetry.Telemetry(enabled_devices=None, devices=devices) - default_car = team.Car(name="default-car", config_paths=["/tmp/rally-config"]) + default_car = team.Car(names="default-car", root_path=None, config_paths=["/tmp/rally-config"]) opts = t.instrument_candidate_env(default_car, "default-node") self.assertTrue(opts) From dd98aefc473327ddf2ac077cdb90a63f80aba2cd Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Thu, 26 Apr 2018 16:15:35 +0200 Subject: [PATCH 06/12] Add usage examples to docs --- docs/car.rst | 33 +++++++++++++++++++++------------ docs/elasticsearch_plugins.rst | 10 ++-------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/docs/car.rst b/docs/car.rst index 3f413c11a..373751f71 100644 --- a/docs/car.rst +++ b/docs/car.rst @@ -17,18 +17,27 @@ A Rally "car" is a specific configuration of Elasticsearch. You can list the ava /_/ |_|\__,_/_/_/\__, / /____/ - Name Type Description - ---------- ------ ---------------------------------- - 16gheap car Sets the Java heap to 16GB - 1gheap car Sets the Java heap to 1GB - 2gheap car Sets the Java heap to 2GB - 4gheap car Sets the Java heap to 4GB - 8gheap car Sets the Java heap to 8GB - defaults car Sets the Java heap to 1GB - verbose_iw car Log more detailed merge time stats - ea mixin Enables Java assertions - -You can specify the car that Rally should use with e.g. ``--car="4gheap"``. It is also possible to specify one or more "mixins" to further customize the configuration. For example, you can specify ``--car="4gheap,ea"`` to run with a 4GB heap and enable Java assertions (they are disabled by default). + Name Type Description + ----------------------- ------ ---------------------------------- + 16gheap car Sets the Java heap to 16GB + 1gheap car Sets the Java heap to 1GB + 2gheap car Sets the Java heap to 2GB + 4gheap car Sets the Java heap to 4GB + 8gheap car Sets the Java heap to 8GB + defaults car Sets the Java heap to 1GB + verbose_iw car Log more detailed merge time stats + ea mixin Enables Java assertions + fp mixin Preserves frame pointers + x_pack_ml mixin X-Pack Machine Learning + x_pack_monitoring_http mixin X-Pack Monitoring (HTTP exporter) + x_pack_monitoring_local mixin X-Pack Monitoring (local exporter) + x_pack_security mixin X-Pack Security + +You can specify the car that Rally should use with e.g. ``--car="4gheap"``. It is also possible to specify one or more "mixins" to further customize the configuration. For example, you can specify ``--car="4gheap,ea"`` to run with a 4GB heap and enable Java assertions (they are disabled by default) or ``--car="4gheap,x_pack_security"`` to benchmark Elasticsearch with X-Pack Security enabled (requires Elasticsearch 6.3.0 or better). + +.. note:: + To benchmark ``x_pack_security`` you need to add the following command line options: ``--client-options="use_ssl:true,verify_certs:false,basic_auth_user:'rally',basic_auth_password:'rally-password'"`` + Similar to :doc:`custom tracks `, you can also define your own cars. diff --git a/docs/elasticsearch_plugins.rst b/docs/elasticsearch_plugins.rst index 5ba73dbe1..8c3c6e63f 100644 --- a/docs/elasticsearch_plugins.rst +++ b/docs/elasticsearch_plugins.rst @@ -38,9 +38,6 @@ To see which plugins are available, run ``esrally list elasticsearch-plugins``:: repository-hdfs repository-s3 store-smb - x-pack monitoring-local - x-pack monitoring-http - x-pack security Rally supports plugins only for Elasticsearch 5.0 or better. As the availability of plugins may change from release to release we recommend that you include the ``--distribution-version`` parameter when listing plugins. By default Rally assumes that you want to benchmark the latest master version of Elasticsearch. @@ -65,13 +62,13 @@ Rally will use several techniques to install and configure plugins: * First, Rally checks whether directory ``plugins/PLUGIN_NAME`` in the currently configured team repository exists. If this is the case, then plugin installation and configuration details will be read from this directory. * Next, Rally will use the provided plugin name when running the Elasticsearch plugin installer. With this approach we can avoid to create a plugin configuration directory in the team repository for very simple plugins that do not need any configuration. -As mentioned above, Rally also allows you to specify a plugin configuration and you can even combine them. Here are some examples: +As mentioned above, Rally also allows you to specify a plugin configuration and you can even combine them. Here are some examples (requires Elasticsearch < 6.3.0 because with 6.3.0 x-pack has turned into a module of Elasticsearch which is treated as a "car" in Rally): * Run a benchmark with the ``x-pack`` plugin in the ``security`` configuration: ``--elasticsearch-plugins=x-pack:security`` * Run a benchmark with the ``x-pack`` plugin in the ``security`` and the ``graph`` configuration: ``--elasticsearch-plugins=x-pack:security+graph`` .. note:: - To benchmark the ``security`` configuration of ``x-pack`` you need to add the following command line options: ``--client-options="use_ssl:true,verify_certs:false,basic_auth_user:'rally',basic_auth_password:'rally-password'" --cluster-health=yellow`` + To benchmark the ``security`` configuration of ``x-pack`` you need to add the following command line options: ``--client-options="use_ssl:true,verify_certs:false,basic_auth_user:'rally',basic_auth_password:'rally-password'"`` You can also override plugin variables with ``--plugin-params`` which is needed for example if you want to use the ``monitoring-http`` configuration in order to export monitoring data. You can export monitoring data e.g. with the following configuration:: @@ -229,9 +226,6 @@ Rally will now know about ``myplugin`` and its two configurations. Let's check t repository-hdfs repository-s3 store-smb - x-pack monitoring-local - x-pack monitoring-http - x-pack security As ``myplugin`` is not a core plugin, the Elasticsearch plugin manager does not know from where to install it, so we need to add the download URL to ``~/.rally/rally.ini`` as before:: From d67cc2b1e42e4eb15f76fcc77615db38541b1b45 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Fri, 27 Apr 2018 05:32:49 +0200 Subject: [PATCH 07/12] Hyphenate x-pack mixins --- docs/car.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/car.rst b/docs/car.rst index 373751f71..378aec44e 100644 --- a/docs/car.rst +++ b/docs/car.rst @@ -28,15 +28,15 @@ A Rally "car" is a specific configuration of Elasticsearch. You can list the ava verbose_iw car Log more detailed merge time stats ea mixin Enables Java assertions fp mixin Preserves frame pointers - x_pack_ml mixin X-Pack Machine Learning - x_pack_monitoring_http mixin X-Pack Monitoring (HTTP exporter) - x_pack_monitoring_local mixin X-Pack Monitoring (local exporter) - x_pack_security mixin X-Pack Security + x-pack-ml mixin X-Pack Machine Learning + x-pack-monitoring-http mixin X-Pack Monitoring (HTTP exporter) + x-pack-monitoring-local mixin X-Pack Monitoring (local exporter) + x-pack-security mixin X-Pack Security -You can specify the car that Rally should use with e.g. ``--car="4gheap"``. It is also possible to specify one or more "mixins" to further customize the configuration. For example, you can specify ``--car="4gheap,ea"`` to run with a 4GB heap and enable Java assertions (they are disabled by default) or ``--car="4gheap,x_pack_security"`` to benchmark Elasticsearch with X-Pack Security enabled (requires Elasticsearch 6.3.0 or better). +You can specify the car that Rally should use with e.g. ``--car="4gheap"``. It is also possible to specify one or more "mixins" to further customize the configuration. For example, you can specify ``--car="4gheap,ea"`` to run with a 4GB heap and enable Java assertions (they are disabled by default) or ``--car="4gheap,x-pack-security"`` to benchmark Elasticsearch with X-Pack Security enabled (requires Elasticsearch 6.3.0 or better). .. note:: - To benchmark ``x_pack_security`` you need to add the following command line options: ``--client-options="use_ssl:true,verify_certs:false,basic_auth_user:'rally',basic_auth_password:'rally-password'"`` + To benchmark ``x-pack-security`` you need to add the following command line options: ``--client-options="use_ssl:true,verify_certs:false,basic_auth_user:'rally',basic_auth_password:'rally-password'"`` Similar to :doc:`custom tracks `, you can also define your own cars. From d1e00791cb568125eeb82c4122698e8984e951a1 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Fri, 27 Apr 2018 05:43:36 +0200 Subject: [PATCH 08/12] Allow multiple cars to refer to the same hook --- esrally/mechanic/team.py | 3 +- .../data/cars/v1/another_with_hook.ini | 9 +++++ ...pty_config_base.ini => empty_cfg_base.ini} | 0 ...g_config_base.ini => missing_cfg_base.ini} | 0 .../v1/{too_many_hooks.ini => multi_hook.ini} | 0 tests/mechanic/team_test.py | 33 ++++++++++++++----- 6 files changed, 36 insertions(+), 9 deletions(-) create mode 100644 tests/mechanic/data/cars/v1/another_with_hook.ini rename tests/mechanic/data/cars/v1/{empty_config_base.ini => empty_cfg_base.ini} (100%) rename tests/mechanic/data/cars/v1/{missing_config_base.ini => missing_cfg_base.ini} (100%) rename tests/mechanic/data/cars/v1/{too_many_hooks.ini => multi_hook.ini} (100%) diff --git a/esrally/mechanic/team.py b/esrally/mechanic/team.py index e8e9929f4..6c6096a58 100644 --- a/esrally/mechanic/team.py +++ b/esrally/mechanic/team.py @@ -55,7 +55,8 @@ def __init__(self, root_path, entry_point): if BootstrapHookHandler(Component(root_path=p, entry_point=Car.entry_point)).can_load(): if not root_path: root_path = p - else: + # multiple cars are based on the same hook + elif root_path != p: raise exceptions.SystemSetupError("Invalid car: {}. Multiple bootstrap hooks are forbidden.".format(name)) all_config_base_vars.update(descriptor.config_base_variables) all_car_vars.update(descriptor.variables) diff --git a/tests/mechanic/data/cars/v1/another_with_hook.ini b/tests/mechanic/data/cars/v1/another_with_hook.ini new file mode 100644 index 000000000..bf1f9e19f --- /dev/null +++ b/tests/mechanic/data/cars/v1/another_with_hook.ini @@ -0,0 +1,9 @@ +[meta] +type = mixin +description = A verbose car with an install hook + +[config] +base = verbose_logging,with_hook + +[variables] +heap_size = 16g diff --git a/tests/mechanic/data/cars/v1/empty_config_base.ini b/tests/mechanic/data/cars/v1/empty_cfg_base.ini similarity index 100% rename from tests/mechanic/data/cars/v1/empty_config_base.ini rename to tests/mechanic/data/cars/v1/empty_cfg_base.ini diff --git a/tests/mechanic/data/cars/v1/missing_config_base.ini b/tests/mechanic/data/cars/v1/missing_cfg_base.ini similarity index 100% rename from tests/mechanic/data/cars/v1/missing_config_base.ini rename to tests/mechanic/data/cars/v1/missing_cfg_base.ini diff --git a/tests/mechanic/data/cars/v1/too_many_hooks.ini b/tests/mechanic/data/cars/v1/multi_hook.ini similarity index 100% rename from tests/mechanic/data/cars/v1/too_many_hooks.ini rename to tests/mechanic/data/cars/v1/multi_hook.ini diff --git a/tests/mechanic/team_test.py b/tests/mechanic/team_test.py index 8f2118528..71b932a96 100644 --- a/tests/mechanic/team_test.py +++ b/tests/mechanic/team_test.py @@ -20,7 +20,7 @@ def setUp(self): def test_lists_car_names(self): # contrary to the name this assertion compares contents but does not care about order. self.assertCountEqual( - ["default", "with_hook", "32gheap", "missing_config_base", "empty_config_base", "ea", "verbose", "too_many_hooks"], + ["default", "with_hook", "32gheap", "missing_cfg_base", "empty_cfg_base", "ea", "verbose", "multi_hook", "another_with_hook"], self.loader.car_names() ) @@ -81,6 +81,23 @@ def test_load_car_with_install_hook(self): }, car.variables) self.assertEqual({}, car.env) + def test_load_car_with_multiple_bases_referring_same_install_hook(self): + car = team.load_car(self.team_dir, ["with_hook", "another_with_hook"]) + self.assertEqual("with_hook+another_with_hook", car.name) + self.assertEqual([ + os.path.join(current_dir, "data", "cars", "v1", "vanilla", "templates"), + os.path.join(current_dir, "data", "cars", "v1", "with_hook", "templates"), + os.path.join(current_dir, "data", "cars", "v1", "verbose_logging", "templates") + ], car.config_paths) + self.assertEqual(os.path.join(current_dir, "data", "cars", "v1", "with_hook"), car.root_path) + self.assertDictEqual({ + "heap_size": "16g", + "clean_command": "./gradlew clean", + "verbose_logging": "true" + }, car.variables) + self.assertEqual({}, car.env) + + def test_raises_error_on_unknown_car(self): with self.assertRaises(exceptions.SystemSetupError) as ctx: team.load_car(self.team_dir, ["don_t-know-you"]) @@ -88,18 +105,18 @@ def test_raises_error_on_unknown_car(self): def test_raises_error_on_empty_config_base(self): with self.assertRaises(exceptions.SystemSetupError) as ctx: - team.load_car(self.team_dir, ["empty_config_base"]) - self.assertEqual("At least one config base is required for car ['empty_config_base']", ctx.exception.args[0]) + team.load_car(self.team_dir, ["empty_cfg_base"]) + self.assertEqual("At least one config base is required for car ['empty_cfg_base']", ctx.exception.args[0]) def test_raises_error_on_missing_config_base(self): with self.assertRaises(exceptions.SystemSetupError) as ctx: - team.load_car(self.team_dir, ["missing_config_base"]) - self.assertEqual("At least one config base is required for car ['missing_config_base']", ctx.exception.args[0]) + team.load_car(self.team_dir, ["missing_cfg_base"]) + self.assertEqual("At least one config base is required for car ['missing_cfg_base']", ctx.exception.args[0]) - def test_raises_error_if_more_than_one_install_hook(self): + def test_raises_error_if_more_than_one_different_install_hook(self): with self.assertRaises(exceptions.SystemSetupError) as ctx: - team.load_car(self.team_dir, ["too_many_hooks"]) - self.assertEqual("Invalid car: ['too_many_hooks']. Multiple bootstrap hooks are forbidden.", ctx.exception.args[0]) + team.load_car(self.team_dir, ["multi_hook"]) + self.assertEqual("Invalid car: ['multi_hook']. Multiple bootstrap hooks are forbidden.", ctx.exception.args[0]) class PluginLoaderTests(TestCase): From f923b9d2ba03bc07333d3502788ac77c12448003 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Fri, 27 Apr 2018 09:44:22 +0200 Subject: [PATCH 09/12] Use mock#call_count instead of assert_not_called With this commit we replace all usages of `assert_not_called` with asserts that assert for a zero call_count. The reason is that `assert_not_called` has only beeen introduced in Python 3.5 but we also support (and test) Python 3.4. --- tests/driver/driver_test.py | 6 +++--- tests/driver/runner_test.py | 14 +++++++------- tests/mechanic/launcher_test.py | 2 +- tests/mechanic/provisioner_test.py | 4 ++-- tests/mechanic/supplier_test.py | 10 +++++----- tests/mechanic/telemetry_test.py | 10 +++++----- tests/track/loader_test.py | 18 +++++++++--------- tests/utils/repo_test.py | 14 +++++++------- 8 files changed, 39 insertions(+), 39 deletions(-) diff --git a/tests/driver/driver_test.py b/tests/driver/driver_test.py index 981546e71..4e99bcd45 100644 --- a/tests/driver/driver_test.py +++ b/tests/driver/driver_test.py @@ -146,8 +146,8 @@ def test_client_reaches_join_point_others_still_executing(self): self.assertEqual(1, len(d.clients_completed_current_step)) - target.on_task_finished.assert_not_called() - target.drive_at.assert_not_called() + self.assertEqual(0, target.on_task_finished.call_count) + self.assertEqual(0, target.drive_at.call_count) def test_client_reaches_join_point_which_completes_parent(self): target = self.create_test_driver_target() @@ -738,7 +738,7 @@ def run(*args, **kwargs): with self.assertRaises(ExpectedUnitTestException): execute_schedule() - es.assert_not_called() + self.assertEqual(0, es.call_count) def test_execute_single_no_return_value(self): es = None diff --git a/tests/driver/runner_test.py b/tests/driver/runner_test.py index dc7462ae4..723a4b368 100644 --- a/tests/driver/runner_test.py +++ b/tests/driver/runner_test.py @@ -919,7 +919,7 @@ def test_param_body_mandatory(self, es): "Please add it to your parameter source."): r(es, params) - es.ingest.put_pipeline.assert_not_called() + self.assertEqual(0, es.ingest.put_pipeline.call_count) @mock.patch("elasticsearch.Elasticsearch") def test_param_id_mandatory(self, es): @@ -933,7 +933,7 @@ def test_param_id_mandatory(self, es): "Please add it to your parameter source."): r(es, params) - es.ingest.put_pipeline.assert_not_called() + self.assertEqual(0, es.ingest.put_pipeline.call_count) class ClusterHealthRunnerTests(TestCase): @@ -1079,7 +1079,7 @@ def test_param_indices_mandatory(self, es): "Please add it to your parameter source."): r(es, params) - es.indices.create.assert_not_called() + self.assertEqual(0, es.indices.create.call_count) class DeleteIndexRunnerTests(TestCase): @@ -1121,7 +1121,7 @@ def test_deletes_all_indices(self, es): mock.call(index="indexA", ignore_unavailable=True, expand_wildcards="none"), mock.call(index="indexB", ignore_unavailable=True, expand_wildcards="none") ]) - es.indices.exists.assert_not_called() + self.assertEqual(0, es.indices.exists.call_count) class CreateIndexTemplateRunnerTests(TestCase): @@ -1159,7 +1159,7 @@ def test_param_templates_mandatory(self, es): "'templates'. Please add it to your parameter source."): r(es, params) - es.indices.put_template.assert_not_called() + self.assertEqual(0, es.indices.put_template.call_count) class DeleteIndexTemplateRunnerTests(TestCase): @@ -1211,7 +1211,7 @@ def test_deletes_only_existing_index_templates(self, es): es.indices.delete_template.assert_called_once_with(name="templateB", timeout=60) # not called because the matching index is empty. - es.indices.delete.assert_not_called() + self.assertEqual(0, es.indices.delete.call_count) @mock.patch("elasticsearch.Elasticsearch") def test_param_templates_mandatory(self, es): @@ -1223,7 +1223,7 @@ def test_param_templates_mandatory(self, es): "'templates'. Please add it to your parameter source."): r(es, params) - es.indices.delete_template.assert_not_called() + self.assertEqual(0, es.indices.delete_template.call_count) class RawRequestRunnerTests(TestCase): diff --git a/tests/mechanic/launcher_test.py b/tests/mechanic/launcher_test.py index 8c54bfae9..0dfea3eb1 100644 --- a/tests/mechanic/launcher_test.py +++ b/tests/mechanic/launcher_test.py @@ -148,4 +148,4 @@ def test_error_on_cluster_launch(self, sleep): with self.assertRaisesRegex(exceptions.LaunchError, "Elasticsearch REST API layer is not available. Forcefully terminated cluster."): cluster_launcher.start() - on_post_launch.assert_not_called() \ No newline at end of file + self.assertEqual(0, on_post_launch.call_count) \ No newline at end of file diff --git a/tests/mechanic/provisioner_test.py b/tests/mechanic/provisioner_test.py index 022e403d3..40495560f 100644 --- a/tests/mechanic/provisioner_test.py +++ b/tests/mechanic/provisioner_test.py @@ -282,8 +282,8 @@ def test_cleanup_nothing_on_preserve(self, mock_path_exists, mock_rm): node_root_dir="~/.rally/benchmarks/races/unittest") installer.cleanup(preserve=True) - mock_path_exists.assert_not_called() - mock_rm.assert_not_called() + self.assertEqual(0, mock_path_exists.call_count) + self.assertEqual(0, mock_rm.call_count) @mock.patch("shutil.rmtree") @mock.patch("os.path.exists") diff --git a/tests/mechanic/supplier_test.py b/tests/mechanic/supplier_test.py index 6c866a8b4..eec42a78a 100644 --- a/tests/mechanic/supplier_test.py +++ b/tests/mechanic/supplier_test.py @@ -52,8 +52,8 @@ def test_checkout_current(self, mock_is_working_copy, mock_clone, mock_pull, moc s.fetch("current") mock_is_working_copy.assert_called_with("/src") - mock_clone.assert_not_called() - mock_pull.assert_not_called() + self.assertEqual(0, mock_clone.call_count) + self.assertEqual(0, mock_pull.call_count) mock_head_revision.assert_called_with("/src")\ @@ -71,8 +71,8 @@ def test_checkout_revision_for_local_only_repo(self, mock_is_working_copy, mock_ s.fetch("67c2f42") mock_is_working_copy.assert_called_with("/src") - mock_clone.assert_not_called() - mock_pull.assert_not_called() + self.assertEqual(0, mock_clone.call_count) + self.assertEqual(0, mock_pull.call_count) mock_checkout.assert_called_with("/src", "67c2f42") mock_head_revision.assert_called_with("/src") @@ -183,7 +183,7 @@ def test_raises_error_on_missing_car_variable(self): "Car 'default' misses config variable 'build_command' to build Elasticsearch."): es.prepare() - builder.build.assert_not_called() + self.assertEqual(0, builder.build.call_count) @mock.patch("glob.glob", lambda p: ["elasticsearch.tar.gz"]) def test_add_elasticsearch_binary(self): diff --git a/tests/mechanic/telemetry_test.py b/tests/mechanic/telemetry_test.py index c72fc51c7..4dadba61f 100644 --- a/tests/mechanic/telemetry_test.py +++ b/tests/mechanic/telemetry_test.py @@ -95,8 +95,8 @@ def test_store_nothing_if_no_metrics_present(self, listdir_mock, open_mock, metr merge_parts_device.attach_to_node(node) merge_parts_device.on_benchmark_stop() - metrics_store_put_value.assert_not_called() - metrics_store_put_count.assert_not_called() + self.assertEqual(0, metrics_store_put_value.call_count) + self.assertEqual(0, metrics_store_put_count.call_count) @mock.patch("esrally.metrics.EsMetricsStore.put_count_node_level") @mock.patch("esrally.metrics.EsMetricsStore.put_value_node_level") @@ -1297,6 +1297,6 @@ def test_stores_nothing_if_no_data_path(self, run_subprocess, metrics_store_clus t.detach_from_node(node, running=True) t.detach_from_node(node, running=False) - run_subprocess.assert_not_called() - metrics_store_cluster_count.assert_not_called() - get_size.assert_not_called() + self.assertEqual(0, run_subprocess.call_count) + self.assertEqual(0, metrics_store_cluster_count.call_count) + self.assertEqual(0, get_size.call_count) diff --git a/tests/track/loader_test.py b/tests/track/loader_test.py index 7afea9a79..179bdbdd3 100644 --- a/tests/track/loader_test.py +++ b/tests/track/loader_test.py @@ -306,8 +306,8 @@ def test_raise_download_error_if_offline(self, is_file, ensure_dir, download): self.assertEqual("Cannot find /tmp/docs.json. Please disable offline mode and retry again.", ctx.exception.args[0]) - ensure_dir.assert_not_called() - download.assert_not_called() + self.assertEqual(0, ensure_dir.call_count) + self.assertEqual(0, download.call_count) @mock.patch("esrally.utils.net.download") @mock.patch("esrally.utils.io.ensure_dir") @@ -330,8 +330,8 @@ def test_raise_download_error_if_no_url_provided_and_file_missing(self, is_file, self.assertEqual("/tmp/docs.json is missing and it cannot be downloaded because no base URL is provided.", ctx.exception.args[0]) - ensure_dir.assert_not_called() - download.assert_not_called() + self.assertEqual(0, ensure_dir.call_count) + self.assertEqual(0, download.call_count) @mock.patch("esrally.utils.net.download") @mock.patch("esrally.utils.io.ensure_dir") @@ -355,8 +355,8 @@ def test_raise_download_error_if_no_url_provided_and_wrong_file_size(self, is_fi self.assertEqual("/tmp/docs.json is present but does not have the expected size of 2000 bytes and it cannot be downloaded because " "no base URL is provided.", ctx.exception.args[0]) - ensure_dir.assert_not_called() - download.assert_not_called() + self.assertEqual(0, ensure_dir.call_count) + self.assertEqual(0, download.call_count) @mock.patch("esrally.utils.net.download") @mock.patch("esrally.utils.io.ensure_dir") @@ -456,8 +456,8 @@ def test_prepare_bundled_document_set_does_nothing_if_no_document_files(self, is uncompressed_size_in_bytes=2000), data_root=".")) - decompress.assert_not_called() - prepare_file_offset_table.assert_not_called() + self.assertEqual(0, decompress.call_count) + self.assertEqual(0, prepare_file_offset_table.call_count) @mock.patch("esrally.utils.io.prepare_file_offset_table") @mock.patch("esrally.utils.io.decompress") @@ -531,7 +531,7 @@ def test_prepare_bundled_document_set_uncompressed_docs_wrong_size(self, is_file data_root=".") self.assertEqual("./docs.json is present but does not have the expected size of 2000 bytes.", ctx.exception.args[0]) - prepare_file_offset_table.assert_not_called() + self.assertEqual(0, prepare_file_offset_table.call_count) class TemplateRenderTests(TestCase): diff --git a/tests/utils/repo_test.py b/tests/utils/repo_test.py index 21e0ea2ec..52865e28d 100644 --- a/tests/utils/repo_test.py +++ b/tests/utils/repo_test.py @@ -79,7 +79,7 @@ def test_does_not_fetch_if_suppressed(self, fetch, is_working_copy): self.assertTrue(r.remote) - fetch.assert_not_called() + self.assertEqual(0, fetch.call_count) @mock.patch("esrally.utils.git.is_working_copy", autospec=True) @mock.patch("esrally.utils.git.fetch") @@ -139,7 +139,7 @@ def test_updates_locally(self, rebase, checkout, branches, fetch, is_working_cop r.update(distribution_version="6.0.0") branches.assert_called_with("/rally-resources/unit-test", remote=False) - rebase.assert_not_called() + self.assertEqual(0, rebase.call_count) checkout.assert_called_with("/rally-resources/unit-test", branch="master") @mock.patch("esrally.utils.git.is_working_copy", autospec=True) @@ -173,8 +173,8 @@ def test_does_not_update_unknown_branch_remotely(self, rebase, checkout, branche ] branches.assert_has_calls(calls) - checkout.assert_not_called() - rebase.assert_not_called() + self.assertEqual(0, checkout.call_count) + self.assertEqual(0, rebase.call_count) @mock.patch("esrally.utils.git.is_working_copy", autospec=True) @mock.patch("esrally.utils.git.fetch", autospec=True) @@ -204,7 +204,7 @@ def test_does_not_update_unknown_branch_remotely_local_fallback(self, rebase, ch branches.assert_has_calls(calls) checkout.assert_called_with("/rally-resources/unit-test", branch="1") - rebase.assert_not_called() + self.assertEqual(0, rebase.call_count) @mock.patch("esrally.utils.git.is_working_copy", autospec=True) @mock.patch("esrally.utils.git.fetch", autospec=True) @@ -228,7 +228,7 @@ def test_does_not_update_unknown_branch_locally(self, rebase, checkout, branches self.assertEqual("Cannot find unittest-resources for distribution version 4.0.0", ctx.exception.args[0]) branches.assert_called_with("/rally-resources/unit-test", remote=False) - checkout.assert_not_called() - rebase.assert_not_called() + self.assertEqual(0, checkout.call_count) + self.assertEqual(0, rebase.call_count) From e582fab66f04b5dfb06c2aed89da9e0aac3998cd Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Fri, 27 Apr 2018 09:54:42 +0200 Subject: [PATCH 10/12] Skip PostLaunchHook for external clusters --- esrally/mechanic/mechanic.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/esrally/mechanic/mechanic.py b/esrally/mechanic/mechanic.py index 166ca94fe..df695a10a 100644 --- a/esrally/mechanic/mechanic.py +++ b/esrally/mechanic/mechanic.py @@ -360,7 +360,9 @@ def receiveMsg_NodesStopped(self, msg, sender): self.transition_when_all_children_responded(sender, msg, "cluster_stopping", "cluster_stopped", self.on_all_nodes_stopped) def on_all_nodes_started(self): - self.cluster_launcher = launcher.ClusterLauncher(self.cfg, self.metrics_store, on_post_launch=PostLaunchHandler([self.car])) + # we might not have a car if we benchmark external clusters + post_launch_handler = PostLaunchHandler([self.car]) if self.car else None + self.cluster_launcher = launcher.ClusterLauncher(self.cfg, self.metrics_store, on_post_launch=post_launch_handler) # Workaround because we could raise a LaunchError here and thespian will attempt to retry a failed message. # In that case, we will get a followup RallyAssertionError because on the second attempt, Rally will check # the status which is now "nodes_started" but we expected the status to be "nodes_starting" previously. From 9a768d8e4207c6b621c7970a7f0c7d8aa4672db1 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Fri, 27 Apr 2018 10:50:38 +0200 Subject: [PATCH 11/12] Reword error message --- esrally/mechanic/supplier.py | 2 +- tests/mechanic/supplier_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/esrally/mechanic/supplier.py b/esrally/mechanic/supplier.py index c06b5d5d8..47d6f82c5 100644 --- a/esrally/mechanic/supplier.py +++ b/esrally/mechanic/supplier.py @@ -199,7 +199,7 @@ def value_of(self, k): try: return self.car.variables[k] except KeyError: - raise exceptions.SystemSetupError("Car '{}' misses config variable '{}' to build Elasticsearch.".format(self.car, k)) + raise exceptions.SystemSetupError("Car '{}' is missing config variable '{}' to build Elasticsearch.".format(self.car, k)) class ExternalPluginSourceSupplier: diff --git a/tests/mechanic/supplier_test.py b/tests/mechanic/supplier_test.py index eec42a78a..6cba55b96 100644 --- a/tests/mechanic/supplier_test.py +++ b/tests/mechanic/supplier_test.py @@ -180,7 +180,7 @@ def test_raises_error_on_missing_car_variable(self): builder = mock.create_autospec(supplier.Builder) es = supplier.ElasticsearchSourceSupplier(revision="abc", es_src_dir="/src", remote_url="", car=car, builder=builder) with self.assertRaisesRegex(exceptions.SystemSetupError, - "Car 'default' misses config variable 'build_command' to build Elasticsearch."): + "Car 'default' is missing config variable 'build_command' to build Elasticsearch."): es.prepare() self.assertEqual(0, builder.build.call_count) From f50c57955456308e710d212556a68b75c6e61389 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Fri, 27 Apr 2018 10:50:53 +0200 Subject: [PATCH 12/12] Fix newlines --- esrally/mechanic/team.py | 2 +- tests/mechanic/data/cars/v1/vanilla/config.ini | 4 ++-- tests/mechanic/data/cars/v1/verbose_logging/config.ini | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/esrally/mechanic/team.py b/esrally/mechanic/team.py index 6c6096a58..941fe9327 100644 --- a/esrally/mechanic/team.py +++ b/esrally/mechanic/team.py @@ -480,4 +480,4 @@ def invoke(self, phase, **kwargs): hook(config_names=self.component.config, **kwargs) else: logger.debug("Component [%s] in config [%s] has no hook registered for phase [%s].", - self.component.name, self.component.config, phase) \ No newline at end of file + self.component.name, self.component.config, phase) diff --git a/tests/mechanic/data/cars/v1/vanilla/config.ini b/tests/mechanic/data/cars/v1/vanilla/config.ini index da1da8c18..27510a7c7 100644 --- a/tests/mechanic/data/cars/v1/vanilla/config.ini +++ b/tests/mechanic/data/cars/v1/vanilla/config.ini @@ -1,5 +1,5 @@ [variables] # this should *always* be overridden by the car definition (config base # variables take least precedence) -heap_size=0 -clean_command=./gradlew clean \ No newline at end of file +heap_size = 0 +clean_command = ./gradlew clean diff --git a/tests/mechanic/data/cars/v1/verbose_logging/config.ini b/tests/mechanic/data/cars/v1/verbose_logging/config.ini index 6064da6df..3e5af96fa 100644 --- a/tests/mechanic/data/cars/v1/verbose_logging/config.ini +++ b/tests/mechanic/data/cars/v1/verbose_logging/config.ini @@ -1,2 +1,2 @@ [variables] -verbose_logging=true \ No newline at end of file +verbose_logging = true