-
Notifications
You must be signed in to change notification settings - Fork 988
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
Update the QA project to use test cluster api #1430
Conversation
Also update the project to run fixtures on the java 8 runtime. Java home can now be set on cluster configs and is used by fixtures. A cluster can be configured with a dependent ElasticsearchCluster. This cluster is depended on for any configuration tasks in case its cluster address is used. Settings are now stored as objects in cluster configurations instead of Strings since they may be GStrings with closures inside of them that must be resolved ONLY at configuration writing time. This is needed to resolve test cluster http addresses which are only discoverable once the cluster is started. In order to make this resolution process simpler, removed most of the Map<String, String> types in the code and replaced them with the FileSettings type which can resolve closures as needed when writing the configuration to a file. Upgraded Spark QA runtime to 2.3.4 as the older version has been removed from apache's mirrors. AbstractClusterTask now extends DefaultTestClustersTask, as it is the only way to ensure that Elasticsearch is still running when starting the task. In order to configure JAVA_HOME consistently, each AbstractClusterTask now has its environment variables resolved in its super class instead of with duplicated code in each task.
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.
Looking good ... a couple comments so far...
...ain/groovy/org/elasticsearch/hadoop/gradle/fixture/hadoop/HadoopClusterFormationTasks.groovy
Outdated
Show resolved
Hide resolved
...ain/groovy/org/elasticsearch/hadoop/gradle/fixture/hadoop/HadoopClusterFormationTasks.groovy
Outdated
Show resolved
Hide resolved
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.
LGTM
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.
Couple of comments. I'm not super familiar with this stuff but there doesn't seem to be any glaring issues standing out for me. 👍
@@ -69,7 +72,7 @@ class HadoopClusterFormationTasks { | |||
* <p> | |||
* Returns a list of NodeInfo objects for each node in the cluster. | |||
* | |||
* Based on {@link org.elasticsearch.gradle.test.ClusterFormationTasks} | |||
* Based on (now removed) org.elasticsearch.gradle.test.ClusterFormationTasks |
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.
Is this really necessary then?
static String unapplyString(Object value) { | ||
if (value == null) { | ||
return null | ||
} else if (value instanceof Closure) { |
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.
You can make this even more generic and just use Callable
.
Also update the project to run fixtures on the java 8 runtime. Java home can now be set on cluster configs and is used by fixtures. A cluster can be configured with a dependent ElasticsearchCluster. This cluster is depended on for any configuration tasks in case its cluster address is used. Settings are now stored as objects in cluster configurations instead of Strings since they may be GStrings with closures inside of them that must be resolved ONLY at configuration writing time. This is needed to resolve test cluster http addresses which are only discoverable once the cluster is started. In order to make this resolution process simpler, removed most of the Map<String, String> types in the code and replaced them with the FileSettings type which can resolve closures as needed when writing the configuration to a file. Upgraded Spark QA runtime to 2.3.4 as the older version has been removed from apache's mirrors. AbstractClusterTask now extends DefaultTestClustersTask, as it is the only way to ensure that Elasticsearch is still running when starting the task. In order to configure JAVA_HOME consistently, each AbstractClusterTask now has its environment variables resolved in its super class instead of with duplicated code in each task.
Relates #1423 |
This PR includes changes necessary to run the Kerberos QA tests using the new test clusters functionality in build-tools.
This also updates the project to run fixtures on the Java 8 runtime. JAVA_HOME can now be set on cluster configs and is used by fixtures.
A cluster can be configured with a dependent ElasticsearchCluster. This cluster is depended on for any configuration tasks in case its cluster address is used.
Settings are now stored as objects in cluster configurations instead of Strings since they may be GStrings with closures inside of them that must be resolved ONLY at configuration writing time. This is needed to resolve test cluster http addresses which are only discoverable once the cluster is started.
In order to make this resolution process simpler, removed most of the Map<String, String> types in the code and replaced them with the FileSettings type which can resolve closures as needed when writing the configuration to a file.
Upgraded Spark QA runtime to 2.3.4 as the older version has been removed from apache's mirrors.
AbstractClusterTask now extends DefaultTestClustersTask, as it is the only way to ensure that Elasticsearch is still running when starting the task.
In order to configure JAVA_HOME consistently, each AbstractClusterTask now has its environment variables resolved in its super class instead of with duplicated code in each task.