From 0ce4649e889d08157c51929518c9298bfe50eacd Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 14 Nov 2018 11:22:00 +0200 Subject: [PATCH] Manage dependencies for test clusters (#35304) * Manage dependencies for test clusters Create a configuration and add the distribution to it automatically. A task is created and added as a dependency to any task that uses a test cluster. The task extracts all the zip archives ( only zip support for now ) in the configuration. We do this only once because most tests mostly use the same distribution and thus we can avoid extracting it multiple times. With this we will be able to start the node from the same files which will most of the time live in OS caches or COW if the configuration requires it. --- build.gradle | 6 +- .../elasticsearch/GradleServicesAdapter.java | 7 +- .../elasticsearch/gradle/Distribution.java | 14 ++-- .../testclusters/ElasticsearchNode.java | 8 +- .../SyncTestClustersConfiguration.java | 77 +++++++++++++++++++ .../testclusters/TestClustersPlugin.java | 77 ++++++++++++++++++- .../testclusters/TestClustersPluginIT.java | 6 +- .../src/testKit/testclusters/build.gradle | 9 ++- .../alpha/build.gradle | 5 +- .../bravo/build.gradle | 5 +- .../testclusters_multiproject/build.gradle | 13 +++- 11 files changed, 201 insertions(+), 26 deletions(-) create mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/SyncTestClustersConfiguration.java diff --git a/build.gradle b/build.gradle index 9857d5d7a21..b21d0a48520 100644 --- a/build.gradle +++ b/build.gradle @@ -37,7 +37,7 @@ if (properties.get("org.elasticsearch.acceptScanTOS", "false") == "true") { } // common maven publishing configuration -subprojects { +allprojects { group = 'org.elasticsearch' version = VersionProperties.elasticsearch description = "Elasticsearch subproject ${project.path}" @@ -185,7 +185,7 @@ task branchConsistency { dependsOn verifyVersions, verifyBwcTestsEnabled } -subprojects { +allprojects { // ignore missing javadocs tasks.withType(Javadoc) { Javadoc javadoc -> // the -quiet here is because of a bug in gradle, in that adding a string option @@ -288,7 +288,7 @@ subprojects { if (dep.group == null || false == dep.group.startsWith('org.elasticsearch')) { return } - Project upstreamProject = dependencyToProject(dep) + Project upstreamProject = project.ext.dependencyToProject(dep) if (upstreamProject == null) { return } diff --git a/buildSrc/src/main/java/org/elasticsearch/GradleServicesAdapter.java b/buildSrc/src/main/java/org/elasticsearch/GradleServicesAdapter.java index 6d256ba0449..5027a440337 100644 --- a/buildSrc/src/main/java/org/elasticsearch/GradleServicesAdapter.java +++ b/buildSrc/src/main/java/org/elasticsearch/GradleServicesAdapter.java @@ -21,6 +21,7 @@ package org.elasticsearch; import org.gradle.api.Action; import org.gradle.api.Project; import org.gradle.api.file.CopySpec; +import org.gradle.api.file.FileCollection; import org.gradle.api.file.FileTree; import org.gradle.api.tasks.WorkResult; import org.gradle.process.ExecResult; @@ -29,7 +30,7 @@ import org.gradle.process.JavaExecSpec; import java.io.File; /** - * Facilitate access to Gradle services without a direct dependency on Project. + * Bridge a gap until Gradle offers service injection for plugins. * * In a future release Gradle will offer service injection, this adapter plays that role until that time. * It exposes the service methods that are part of the public API as the classes implementing them are not. @@ -65,4 +66,8 @@ public class GradleServicesAdapter { public FileTree zipTree(File zipPath) { return project.zipTree(zipPath); } + + public FileCollection fileTree(File dir) { + return project.fileTree(dir); + } } diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/Distribution.java b/buildSrc/src/main/java/org/elasticsearch/gradle/Distribution.java index c926e70b3f7..365a12c076c 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/Distribution.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/Distribution.java @@ -20,17 +20,17 @@ package org.elasticsearch.gradle; public enum Distribution { - INTEG_TEST("integ-test-zip"), - ZIP("zip"), - ZIP_OSS("zip-oss"); + INTEG_TEST("integ-test"), + ZIP("elasticsearch"), + ZIP_OSS("elasticsearch-oss"); - private final String name; + private final String fileName; Distribution(String name) { - this.name = name; + this.fileName = name; } - public String getName() { - return name; + public String getFileName() { + return fileName; } } diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java index 012e05f2f6c..4c7e84c423e 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java @@ -20,7 +20,6 @@ package org.elasticsearch.gradle.testclusters; import org.elasticsearch.GradleServicesAdapter; import org.elasticsearch.gradle.Distribution; -import org.elasticsearch.gradle.Version; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; @@ -35,7 +34,7 @@ public class ElasticsearchNode { private final Logger logger = Logging.getLogger(ElasticsearchNode.class); private Distribution distribution; - private Version version; + private String version; public ElasticsearchNode(String name, GradleServicesAdapter services) { this.name = name; @@ -46,11 +45,11 @@ public class ElasticsearchNode { return name; } - public Version getVersion() { + public String getVersion() { return version; } - public void setVersion(Version version) { + public void setVersion(String version) { checkFrozen(); this.version = version; } @@ -75,6 +74,7 @@ public class ElasticsearchNode { public void freeze() { logger.info("Locking configuration of `{}`", this); configurationFrozen.set(true); + Objects.requireNonNull(version, "Version of test cluster `" + this + "` can't be null"); } private void checkFrozen() { diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/SyncTestClustersConfiguration.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/SyncTestClustersConfiguration.java new file mode 100644 index 00000000000..d1a86a38c66 --- /dev/null +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/SyncTestClustersConfiguration.java @@ -0,0 +1,77 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.gradle.testclusters; + +import org.gradle.api.DefaultTask; +import org.gradle.api.Project; +import org.gradle.api.file.FileCollection; +import org.gradle.api.tasks.InputFiles; +import org.gradle.api.tasks.OutputDirectory; +import org.gradle.api.tasks.TaskAction; + +import java.io.File; +import java.util.Set; +import java.util.stream.Collectors; + +public class SyncTestClustersConfiguration extends DefaultTask { + + @InputFiles + public FileCollection getDependencies() { + Set nonZip = getProject().getConfigurations() + .getByName(TestClustersPlugin.HELPER_CONFIGURATION_NAME) + .getFiles() + .stream() + .filter(file -> file.getName().endsWith(".zip") == false) + .collect(Collectors.toSet()); + if(nonZip.isEmpty() == false) { + throw new IllegalStateException("Expected only zip files in configuration : " + + TestClustersPlugin.HELPER_CONFIGURATION_NAME + " but found " + + nonZip + ); + } + return getProject().files( + getProject().getConfigurations() + .getByName(TestClustersPlugin.HELPER_CONFIGURATION_NAME) + .getFiles() + ); + } + + @OutputDirectory + public File getOutputDir() { + return getTestClustersConfigurationExtractDir(getProject()); + } + + @TaskAction + public void doExtract() { + File outputDir = getOutputDir(); + getProject().delete(outputDir); + outputDir.mkdirs(); + getDependencies().forEach(dep -> + getProject().copy(spec -> { + spec.from(getProject().zipTree(dep)); + spec.into(new File(outputDir, "zip")); + }) + ); + } + + static File getTestClustersConfigurationExtractDir(Project project) { + return new File(TestClustersPlugin.getTestClustersBuildDir(project), "extract"); + } + +} diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java index 5191c7d4feb..2ea5e62306a 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java @@ -24,6 +24,7 @@ import org.gradle.api.NamedDomainObjectContainer; import org.gradle.api.Plugin; import org.gradle.api.Project; import org.gradle.api.Task; +import org.gradle.api.artifacts.Configuration; import org.gradle.api.execution.TaskActionListener; import org.gradle.api.execution.TaskExecutionListener; import org.gradle.api.logging.Logger; @@ -31,6 +32,7 @@ import org.gradle.api.logging.Logging; import org.gradle.api.plugins.ExtraPropertiesExtension; import org.gradle.api.tasks.TaskState; +import java.io.File; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -44,7 +46,8 @@ public class TestClustersPlugin implements Plugin { private static final String LIST_TASK_NAME = "listTestClusters"; private static final String NODE_EXTENSION_NAME = "testClusters"; - public static final String PROPERTY_TESTCLUSTERS_RUN_ONCE = "_testclusters_run_once"; + static final String HELPER_CONFIGURATION_NAME = "testclusters"; + private static final String SYNC_ARTIFACTS_TASK_NAME = "syncTestClustersArtifacts"; private final Logger logger = Logging.getLogger(TestClustersPlugin.class); @@ -56,6 +59,8 @@ public class TestClustersPlugin implements Plugin { @Override public void apply(Project project) { + Project rootProject = project.getRootProject(); + // enable the DSL to describe clusters NamedDomainObjectContainer container = createTestClustersContainerExtension(project); @@ -67,14 +72,28 @@ public class TestClustersPlugin implements Plugin { // There's a single Gradle instance for multi project builds, this means that some configuration needs to be // done only once even if the plugin is applied multiple times as a part of multi project build - ExtraPropertiesExtension rootProperties = project.getRootProject().getExtensions().getExtraProperties(); - if (rootProperties.has(PROPERTY_TESTCLUSTERS_RUN_ONCE) == false) { - rootProperties.set(PROPERTY_TESTCLUSTERS_RUN_ONCE, true); + if (rootProject.getConfigurations().findByName(HELPER_CONFIGURATION_NAME) == null) { + // We use a single configuration on the root project to resolve all testcluster dependencies ( like distros ) + // at once, only once without the need to repeat it for each project. This pays off assuming that most + // projects use the same dependencies. + Configuration helperConfiguration = project.getRootProject().getConfigurations().create(HELPER_CONFIGURATION_NAME); + helperConfiguration.setDescription( + "Internal helper configuration used by cluster configuration to download " + + "ES distributions and plugins." + ); + // When running in the Daemon it's possible for this to hold references to past usedClusters.clear(); claimsInventory.clear(); runningClusters.clear(); + + // We have a single task to sync the helper configuration to "artifacts dir" + // the clusters will look for artifacts there based on the naming conventions. + // Tasks that use a cluster will add this as a dependency automatically so it's guaranteed to run early in + // the build. + rootProject.getTasks().create(SYNC_ARTIFACTS_TASK_NAME, SyncTestClustersConfiguration.class); + // When we know what tasks will run, we claim the clusters of those task to differentiate between clusters // that are defined in the build script and the ones that will actually be used in this invocation of gradle // we use this information to determine when the last task that required the cluster executed so that we can @@ -86,6 +105,10 @@ public class TestClustersPlugin implements Plugin { // After each task we determine if there are clusters that are no longer needed. configureStopClustersHook(project); + + // Since we have everything modeled in the DSL, add all the required dependencies e.x. the distribution to the + // configuration so the user doesn't have to repeat this. + autoConfigureClusterDependencies(project, rootProject, container); } } @@ -123,7 +146,15 @@ public class TestClustersPlugin implements Plugin { "useCluster", new Closure(this, task) { public void doCall(ElasticsearchNode node) { + Object thisObject = this.getThisObject(); + if (thisObject instanceof Task == false) { + throw new AssertionError("Expected " + thisObject + " to be an instance of " + + "Task, but got: " + thisObject.getClass()); + } usedClusters.computeIfAbsent(task, k -> new ArrayList<>()).add(node); + ((Task) thisObject).dependsOn( + project.getRootProject().getTasks().getByName(SYNC_ARTIFACTS_TASK_NAME) + ); } }) ); @@ -205,4 +236,42 @@ public class TestClustersPlugin implements Plugin { ); } + static File getTestClustersBuildDir(Project project) { + return new File(project.getRootProject().getBuildDir(), "testclusters"); + } + + /** + * Boilerplate to get testClusters container extension + * + * Equivalent to project.testClusters in the DSL + */ + @SuppressWarnings("unchecked") + public static NamedDomainObjectContainer getNodeExtension(Project project) { + return (NamedDomainObjectContainer) + project.getExtensions().getByName(NODE_EXTENSION_NAME); + } + + private void autoConfigureClusterDependencies( + Project project, + Project rootProject, + NamedDomainObjectContainer container + ) { + // When the project evaluated we know of all tasks that use clusters. + // Each of these have to depend on the artifacts being synced. + // We need afterEvaluate here despite the fact that container is a domain object, we can't implement this with + // all because fields can change after the fact. + project.afterEvaluate(ip -> container.forEach(esNode -> { + // declare dependencies against artifacts needed by cluster formation. + String dependency = String.format( + "org.elasticsearch.distribution.zip:%s:%s@zip", + esNode.getDistribution().getFileName(), + esNode.getVersion() + ); + logger.info("Cluster {} depends on {}", esNode.getName(), dependency); + rootProject.getDependencies().add(HELPER_CONFIGURATION_NAME, dependency); + })); + } + + + } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/testclusters/TestClustersPluginIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/testclusters/TestClustersPluginIT.java index c6e3b2ca370..940eff47253 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/testclusters/TestClustersPluginIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/testclusters/TestClustersPluginIT.java @@ -81,7 +81,7 @@ public class TestClustersPluginIT extends GradleIntegrationTestCase { public void testMultiProject() { BuildResult result = GradleRunner.create() .withProjectDir(getProjectDir("testclusters_multiproject")) - .withArguments("user1", "user2", "-s", "-i", "--parallel") + .withArguments("user1", "user2", "-s", "-i", "--parallel", "-Dlocal.repo.path=" + getLocalTestRepoPath()) .withPluginClasspath() .build(); assertTaskSuccessful(result, ":user1", ":user2"); @@ -130,16 +130,16 @@ public class TestClustersPluginIT extends GradleIntegrationTestCase { } private GradleRunner getTestClustersRunner(String... tasks) { - String[] arguments = Arrays.copyOf(tasks, tasks.length + 2); + String[] arguments = Arrays.copyOf(tasks, tasks.length + 3); arguments[tasks.length] = "-s"; arguments[tasks.length + 1] = "-i"; + arguments[tasks.length + 2] = "-Dlocal.repo.path=" + getLocalTestRepoPath(); return GradleRunner.create() .withProjectDir(getProjectDir("testclusters")) .withArguments(arguments) .withPluginClasspath(); } - private void assertStartedAndStoppedOnce(BuildResult result) { assertOutputOnlyOnce( result.getOutput(), diff --git a/buildSrc/src/testKit/testclusters/build.gradle b/buildSrc/src/testKit/testclusters/build.gradle index bd1cfc143f4..a9b30d91026 100644 --- a/buildSrc/src/testKit/testclusters/build.gradle +++ b/buildSrc/src/testKit/testclusters/build.gradle @@ -4,7 +4,14 @@ plugins { testClusters { myTestCluster { - distribution = 'ZIP' + distribution = 'ZIP' + version = '7.0.0-alpha1-SNAPSHOT' + } +} + +repositories { + maven { + url System.getProperty("local.repo.path") } } diff --git a/buildSrc/src/testKit/testclusters_multiproject/alpha/build.gradle b/buildSrc/src/testKit/testclusters_multiproject/alpha/build.gradle index d9f18afd68b..3c4aebf8f23 100644 --- a/buildSrc/src/testKit/testclusters_multiproject/alpha/build.gradle +++ b/buildSrc/src/testKit/testclusters_multiproject/alpha/build.gradle @@ -2,7 +2,10 @@ plugins { id 'elasticsearch.testclusters' } testClusters { - myTestCluster + myTestCluster { + distribution = 'ZIP' + version = '7.0.0-alpha1-SNAPSHOT' + } } task user1 { useCluster testClusters.myTestCluster diff --git a/buildSrc/src/testKit/testclusters_multiproject/bravo/build.gradle b/buildSrc/src/testKit/testclusters_multiproject/bravo/build.gradle index 2e1461f0b0f..d479ceaa058 100644 --- a/buildSrc/src/testKit/testclusters_multiproject/bravo/build.gradle +++ b/buildSrc/src/testKit/testclusters_multiproject/bravo/build.gradle @@ -3,7 +3,10 @@ plugins { } testClusters { - myTestCluster + myTestCluster { + distribution = 'ZIP' + version = '7.0.0-alpha1-SNAPSHOT' + } } task user1 { diff --git a/buildSrc/src/testKit/testclusters_multiproject/build.gradle b/buildSrc/src/testKit/testclusters_multiproject/build.gradle index 3527d1821d2..acd3213d2b1 100644 --- a/buildSrc/src/testKit/testclusters_multiproject/build.gradle +++ b/buildSrc/src/testKit/testclusters_multiproject/build.gradle @@ -2,8 +2,19 @@ plugins { id 'elasticsearch.testclusters' } +allprojects { + repositories { + maven { + url System.getProperty("local.repo.path") + } + } +} + testClusters { - myTestCluster + myTestCluster { + distribution = 'ZIP' + version = '7.0.0-alpha1-SNAPSHOT' + } } task user1 {