Skip to content

Make NodeInfo#nodeVersion strongly-typed as Version #29515

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 13, 2018
Merged

Make NodeInfo#nodeVersion strongly-typed as Version #29515

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
edeef51
Make NodeInfo#nodeVersion strongly-typed as Version
jasontedor Apr 13, 2018
ee06d86
Make bwcVersion a Version too
jasontedor Apr 13, 2018
be69351
An improvement?
jasontedor Apr 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,19 +99,22 @@ class ClusterFormationTasks {

configureDistributionDependency(project, config.distribution, bwcDistro, config.bwcVersion)
for (Map.Entry<String, Project> entry : config.plugins.entrySet()) {
configureBwcPluginDependency("${prefix}_elasticsearchBwcPlugins", project, entry.getValue(), bwcPlugins, config.bwcVersion)
configureBwcPluginDependency("${prefix}_elasticsearchBwcPlugins", project, entry.getValue(), bwcPlugins, Version.fromString(config.bwcVersion))
}
bwcDistro.resolutionStrategy.cacheChangingModulesFor(0, TimeUnit.SECONDS)
bwcPlugins.resolutionStrategy.cacheChangingModulesFor(0, TimeUnit.SECONDS)
}
for (int i = 0; i < config.numNodes; i++) {
// we start N nodes and out of these N nodes there might be M bwc nodes.
// for each of those nodes we might have a different configuration
String elasticsearchVersion = VersionProperties.elasticsearch
Configuration distro = currentDistro
final Configuration distro
final Version elasticsearchVersion
if (i < config.numBwcNodes) {
elasticsearchVersion = config.bwcVersion
elasticsearchVersion = Version.fromString(config.bwcVersion)
Copy link
Member

@rjernst rjernst Apr 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ClusterConfiguration.bwcVersion can be changed to be Version as well, so we don't have to parse here? All the places I see it set (eg qa/full-cluster-restart) have an already constructed Version object.

Copy link
Member Author

@jasontedor jasontedor Apr 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed ee06d86.

distro = bwcDistro
} else {
elasticsearchVersion = Version.fromString(VersionProperties.elasticsearch)
Copy link
Member

@rjernst rjernst Apr 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be parsed in the static init of VersionProperties (changing that to Version)? The toString() would still work for all the places that use VersionProperties.elasticsearch eg to set maven version.

Copy link
Member Author

@jasontedor jasontedor Apr 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed ee06d86.

distro = currentDistro
}
NodeInfo node = new NodeInfo(config, i, project, prefix, elasticsearchVersion, sharedDir)
nodes.add(node)
Expand All @@ -137,7 +140,7 @@ class ClusterFormationTasks {
}

/** Adds a dependency on a different version of the given plugin, which will be retrieved using gradle's dependency resolution */
static void configureBwcPluginDependency(String name, Project project, Project pluginProject, Configuration configuration, String elasticsearchVersion) {
static void configureBwcPluginDependency(String name, Project project, Project pluginProject, Configuration configuration, Version elasticsearchVersion) {
verifyProjectHasBuildPlugin(name, elasticsearchVersion, project, pluginProject)
final String pluginName = findPluginName(pluginProject)
project.dependencies.add(configuration.name, "org.elasticsearch.plugin:${pluginName}:${elasticsearchVersion}@zip")
Expand Down Expand Up @@ -183,7 +186,7 @@ class ClusterFormationTasks {
setup = configureAddKeystoreFileTasks(prefix, project, setup, node)

if (node.config.plugins.isEmpty() == false) {
if (node.nodeVersion == VersionProperties.elasticsearch) {
if (node.nodeVersion == Version.fromString(VersionProperties.elasticsearch)) {
setup = configureCopyPluginsTask(taskName(prefix, node, 'copyPlugins'), project, setup, node, prefix)
} else {
setup = configureCopyBwcPluginsTask(taskName(prefix, node, 'copyBwcPlugins'), project, setup, node, prefix)
Expand Down Expand Up @@ -303,7 +306,7 @@ class ClusterFormationTasks {
// Default the watermarks to absurdly low to prevent the tests from failing on nodes without enough disk space
esConfig['cluster.routing.allocation.disk.watermark.low'] = '1b'
esConfig['cluster.routing.allocation.disk.watermark.high'] = '1b'
if (Version.fromString(node.nodeVersion).major >= 6) {
if (node.nodeVersion.major >= 6) {
esConfig['cluster.routing.allocation.disk.watermark.flood_stage'] = '1b'
}
// increase script compilation limit since tests can rapid-fire script compilations
Expand Down Expand Up @@ -514,7 +517,7 @@ class ClusterFormationTasks {

static Task configureInstallPluginTask(String name, Project project, Task setup, NodeInfo node, Project plugin, String prefix) {
final FileCollection pluginZip;
if (node.nodeVersion != VersionProperties.elasticsearch) {
if (node.nodeVersion != Version.fromString(VersionProperties.elasticsearch)) {
pluginZip = project.configurations.getByName(pluginBwcConfigurationName(prefix, plugin))
} else {
pluginZip = project.configurations.getByName(pluginConfigurationName(prefix, plugin))
Expand Down Expand Up @@ -803,7 +806,7 @@ class ClusterFormationTasks {
return retVal
}

static void verifyProjectHasBuildPlugin(String name, String version, Project project, Project pluginProject) {
static void verifyProjectHasBuildPlugin(String name, Version version, Project project, Project pluginProject) {
if (pluginProject.plugins.hasPlugin(PluginBuildPlugin) == false && pluginProject.plugins.hasPlugin(MetaPluginBuildPlugin) == false) {
throw new GradleException("Task [${name}] cannot add plugin [${pluginProject.path}] with version [${version}] to project's " +
"[${project.path}] dependencies: the plugin is not an esplugin or es_meta_plugin")
Expand Down
12 changes: 6 additions & 6 deletions buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,10 @@ class NodeInfo {
ByteArrayOutputStream buffer = new ByteArrayOutputStream()

/** the version of elasticsearch that this node runs */
String nodeVersion
Version nodeVersion

/** Holds node configuration for part of a test cluster. */
NodeInfo(ClusterConfiguration config, int nodeNum, Project project, String prefix, String nodeVersion, File sharedDir) {
NodeInfo(ClusterConfiguration config, int nodeNum, Project project, String prefix, Version nodeVersion, File sharedDir) {
this.config = config
this.nodeNum = nodeNum
this.sharedDir = sharedDir
Expand Down Expand Up @@ -166,13 +166,13 @@ class NodeInfo {

final String javaHome
final Map<Integer, JavaVersion> javaVersions = project.javaVersions
if (Version.fromString(nodeVersion).before("6.2.0")) {
if (nodeVersion.before("6.2.0")) {
final String java8Home = javaVersions.get(8)
if (java8Home == null) {
throw new GradleException("JAVA8_HOME must be set to run BWC tests against [" + nodeVersion + "]")
}
javaHome = java8Home
} else if (Version.fromString(nodeVersion).onOrAfter("6.2.0") && Version.fromString(nodeVersion).before("6.3.0")) {
} else if (nodeVersion.onOrAfter("6.2.0") && nodeVersion.before("6.3.0")) {
final String java9Home = javaVersions.get(9)
if (java9Home == null) {
throw new GradleException("JAVA9_HOME must be set to run BWC tests against [" + nodeVersion + "]")
Expand Down Expand Up @@ -304,7 +304,7 @@ class NodeInfo {
}

/** Returns the directory elasticsearch home is contained in for the given distribution */
static File homeDir(File baseDir, String distro, String nodeVersion) {
static File homeDir(File baseDir, String distro, Version nodeVersion) {
String path
switch (distro) {
case 'integ-test-zip':
Expand All @@ -322,7 +322,7 @@ class NodeInfo {
return new File(baseDir, path)
}

static File pathConf(File baseDir, String distro, String nodeVersion) {
static File pathConf(File baseDir, String distro, Version nodeVersion) {
switch (distro) {
case 'integ-test-zip':
case 'zip':
Expand Down