-
Notifications
You must be signed in to change notification settings - Fork 314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for multiple clusters in custom runners #488
Merged
Merged
Changes from all commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
b79a7a5
WiP support for multiple clusters in --target-hosts
dliappis 91d261b
Initial working example
dliappis 6964a06
Support client-options multi params
dliappis ed785be
Allow parsing of client-options and target-hosts from json files too
dliappis 751b61e
Docstring and debugging print cleanups
dliappis ef3ec49
Bug fix
dliappis 2de6a98
Rename things for better readability
dliappis 23d037a
More cleanups and fix tests after renaming DelegatinRunner
dliappis b93a624
Allow no --client-options even with multi cluster target-hosts
dliappis bed9491
Fix wrapping of context-manager capable Runners
dliappis d6a9f00
Fix ExternalLauncherTests
dliappis 3963a4b
Add test for context-manager preservation
dliappis 2009f8a
Bug fixes for cluster aware launcher
dliappis b7658fe
Fix launcher tests after rebase with master
dliappis d29c5c2
Fix cluster telemetry stop
dliappis 67e67c3
Bug fix when no target-host is used
dliappis ded44a6
Remove debug print message
dliappis 9f60438
Check for the boolean value of multi_cluster in custom runner
dliappis ab8af4a
Revert concept of Clusters from launcher
dliappis 5979419
Do not commit yet any CCR telemetry device
dliappis 88fe218
WiP documentation for --target-hosts
dliappis 393a320
Fix tests after ab8af4a1d565028ddfe724a27ea4bd47a9749b41
dliappis 703a9c9
Remove debugging code
dliappis 185a7c2
Correct test case for inline json cli parameter of target-hosts
dliappis 0bca981
Add documentation for multi cluster support
dliappis 7112b1d
Remove left over code for stopping clusters
dliappis 286ad76
Remove unintentionally inserted newline
dliappis 8605d10
Re-introduce newline, as found in current master
dliappis d0d5071
Remove debug prints
dliappis bdc3f0e
Remove some more debugging prints
dliappis da1a168
Addressing PR comments
dliappis 9dde567
Addressing PR comment
dliappis f9fc471
Addressing PR comment
dliappis 484900b
Addressing PR comment
dliappis 916b278
Addressing PR comment
dliappis 89ca81e
Addressing PR comments
dliappis db17a6b
Addressing PR comments
dliappis 28af9c0
Addressing PR comments
dliappis ba5e8b3
Addressing PR docs comments.
dliappis 284e3fa
Addressing PR comment.
dliappis 0a17876
Addressing PR comments
dliappis e04df54
Addressing PR comments
dliappis File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,23 +62,31 @@ def start(self): | |
""" | ||
enabled_devices = self.cfg.opts("mechanic", "telemetry.devices") | ||
telemetry_params = self.cfg.opts("mechanic", "telemetry.params") | ||
hosts = self.cfg.opts("client", "hosts") | ||
client_options = self.cfg.opts("client", "options") | ||
es = self.client_factory(hosts, client_options).create() | ||
all_hosts = self.cfg.opts("client", "hosts").all_hosts | ||
default_hosts = self.cfg.opts("client", "hosts").default | ||
|
||
es = {} | ||
for cluster_name, cluster_hosts in all_hosts.items(): | ||
all_client_options = self.cfg.opts("client", "options").all_client_options | ||
cluster_client_options = all_client_options[cluster_name] | ||
es[cluster_name] = self.client_factory(cluster_hosts, cluster_client_options).create() | ||
|
||
es_default = es["default"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am unsure about using the "magic name" |
||
|
||
t = telemetry.Telemetry(enabled_devices, devices=[ | ||
telemetry.NodeStats(telemetry_params, es, self.metrics_store), | ||
telemetry.ClusterMetaDataInfo(es), | ||
telemetry.ClusterEnvironmentInfo(es, self.metrics_store), | ||
telemetry.GcTimesSummary(es, self.metrics_store), | ||
telemetry.IndexStats(es, self.metrics_store), | ||
telemetry.MlBucketProcessingTime(es, self.metrics_store) | ||
telemetry.NodeStats(telemetry_params, es_default, self.metrics_store), | ||
telemetry.ClusterMetaDataInfo(es_default), | ||
telemetry.ClusterEnvironmentInfo(es_default, self.metrics_store), | ||
telemetry.GcTimesSummary(es_default, self.metrics_store), | ||
telemetry.IndexStats(es_default, self.metrics_store), | ||
telemetry.MlBucketProcessingTime(es_default, self.metrics_store) | ||
]) | ||
|
||
# The list of nodes will be populated by ClusterMetaDataInfo, so no need to do it here | ||
c = cluster.Cluster(hosts, [], t) | ||
c = cluster.Cluster(default_hosts, [], t) | ||
|
||
logger.info("All cluster nodes have successfully started. Checking if REST API is available.") | ||
if wait_for_rest_layer(es, max_attempts=40): | ||
if wait_for_rest_layer(es_default, max_attempts=40): | ||
logger.info("REST API is available. Attaching telemetry devices to cluster.") | ||
t.attach_to_cluster(c) | ||
logger.info("Telemetry devices are now attached to the cluster.") | ||
|
@@ -88,7 +96,7 @@ def start(self): | |
self.stop(c) | ||
raise exceptions.LaunchError("Elasticsearch REST API layer is not available. Forcefully terminated cluster.") | ||
if self.on_post_launch: | ||
self.on_post_launch(es) | ||
self.on_post_launch(es_default) | ||
return c | ||
|
||
def stop(self, c): | ||
|
@@ -184,8 +192,8 @@ def __init__(self, cfg, metrics_store, client_factory_class=client.EsClientFacto | |
self.client_factory = client_factory_class | ||
|
||
def start(self, node_configurations=None): | ||
hosts = self.cfg.opts("client", "hosts") | ||
client_options = self.cfg.opts("client", "options") | ||
hosts = self.cfg.opts("client", "hosts").default | ||
client_options = self.cfg.opts("client", "options").default | ||
es = self.client_factory(hosts, client_options).create() | ||
|
||
# cannot enable custom telemetry devices here | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems
MultiClusterDelegatingRunner
does not support the context manager protocol.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right; I thought we don't really need it as it is a special case after all. Then again, if it doesn't support the context manager protocol, I should clarify this in the docs, which sounds to me like more effort than adding the support :)