Skip to content

Commit 78e5fab

Browse files
breskebybreskeby
committed
Change finalization of elasticsearch distribution
- avoid to rely on afterEvaluate lifecycle hook - allows us to define test cluster only in the task creation which can be later than afterevaluate
1 parent 724b244 commit 78e5fab

File tree

9 files changed

+88
-37
lines changed

9 files changed

+88
-37
lines changed

‎buildSrc/src/integTest/groovy/org/elasticsearch/gradle/DistributionDownloadPluginFuncTest.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ class DistributionDownloadPluginFuncTest extends AbstractGradleFuncTest {
150150
private static String setupDistroTask() {
151151
return """
152152
tasks.register("setupDistro", Sync) {
153-
from(elasticsearch_distributions.test_distro.extracted)
153+
from(elasticsearch_distributions.test_distro)
154154
into("build/distro")
155155
}
156156
"""

‎buildSrc/src/integTest/groovy/org/elasticsearch/gradle/TestClustersPluginFuncTest.groovy

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,31 @@ class TestClustersPluginFuncTest extends AbstractGradleFuncTest {
7575
assertNoCustomDistro('myCluster')
7676
}
7777

78+
def "can declare test cluster in lazy evaluated task configuration block"() {
79+
given:
80+
buildFile << """
81+
tasks.register('myTask', SomeClusterAwareTask) {
82+
testClusters {
83+
myCluster {
84+
testDistribution = 'default'
85+
}
86+
}
87+
useCluster testClusters.myCluster
88+
}
89+
"""
90+
91+
when:
92+
def result = withMockedDistributionDownload(gradleRunner("myTask", '-i')) {
93+
build()
94+
}
95+
96+
then:
97+
result.output.contains("elasticsearch-keystore script executed!")
98+
assertEsStdoutContains("myCluster", "Starting Elasticsearch process")
99+
assertEsStdoutContains("myCluster", "Stopping node")
100+
assertNoCustomDistro('myCluster')
101+
}
102+
78103
def "custom distro folder created for tweaked cluster distribution"() {
79104
given:
80105
buildFile << """

‎buildSrc/src/integTest/groovy/org/elasticsearch/gradle/internal/InternalDistributionDownloadPluginFuncTest.groovy

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ class InternalDistributionDownloadPluginFuncTest extends AbstractGradleFuncTest
6363
}
6464
}
6565
tasks.register("setupDistro", Sync) {
66-
from(elasticsearch_distributions.test_distro.extracted)
66+
from(elasticsearch_distributions.test_distro)
6767
into("build/distro")
6868
}
6969
"""
@@ -93,7 +93,7 @@ class InternalDistributionDownloadPluginFuncTest extends AbstractGradleFuncTest
9393
}
9494
}
9595
tasks.register("setupDistro", Sync) {
96-
from(elasticsearch_distributions.test_distro.extracted)
96+
from(elasticsearch_distributions.test_distro)
9797
into("build/distro")
9898
}
9999
"""
@@ -124,7 +124,7 @@ class InternalDistributionDownloadPluginFuncTest extends AbstractGradleFuncTest
124124
}
125125
}
126126
tasks.register("createExtractedTestDistro") {
127-
dependsOn elasticsearch_distributions.test_distro.extracted
127+
dependsOn elasticsearch_distributions.test_distro
128128
}
129129
"""
130130
when:

‎buildSrc/src/main/java/org/elasticsearch/gradle/DistributionDownloadPlugin.java

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,21 @@ public void apply(Project project) {
8181
setupResolutionsContainer(project);
8282
setupDistributionContainer(project, dockerSupport);
8383
setupDownloadServiceRepo(project);
84-
project.afterEvaluate(this::setupDistributions);
8584
}
8685

8786
private void setupDistributionContainer(Project project, Provider<DockerSupportService> dockerSupport) {
8887
distributionsContainer = project.container(ElasticsearchDistribution.class, name -> {
8988
Configuration fileConfiguration = project.getConfigurations().create("es_distro_file_" + name);
9089
Configuration extractedConfiguration = project.getConfigurations().create(DISTRO_EXTRACTED_CONFIG_PREFIX + name);
9190
extractedConfiguration.getAttributes().attribute(ArtifactAttributes.ARTIFACT_FORMAT, ArtifactTypeDefinition.DIRECTORY_TYPE);
92-
return new ElasticsearchDistribution(name, project.getObjects(), dockerSupport, fileConfiguration, extractedConfiguration);
91+
return new ElasticsearchDistribution(
92+
name,
93+
project.getObjects(),
94+
dockerSupport,
95+
fileConfiguration,
96+
extractedConfiguration,
97+
(dist) -> finalizeDistributionDependencies(project, dist)
98+
);
9399
});
94100
project.getExtensions().add(CONTAINER_NAME, distributionsContainer);
95101
}
@@ -113,21 +119,18 @@ public static NamedDomainObjectContainer<DistributionResolution> getRegistration
113119
return (NamedDomainObjectContainer<DistributionResolution>) project.getExtensions().getByName(RESOLUTION_CONTAINER_NAME);
114120
}
115121

116-
// pkg private for tests
117-
void setupDistributions(Project project) {
118-
for (ElasticsearchDistribution distribution : distributionsContainer) {
119-
distribution.finalizeValues();
120-
DependencyHandler dependencies = project.getDependencies();
121-
// for the distribution as a file, just depend on the artifact directly
122-
DistributionDependency distributionDependency = resolveDependencyNotation(project, distribution);
123-
dependencies.add(distribution.configuration.getName(), distributionDependency.getDefaultNotation());
124-
// no extraction allowed for rpm, deb or docker
125-
if (distribution.getType().shouldExtract()) {
126-
// The extracted configuration depends on the artifact directly but has
127-
// an artifact transform registered to resolve it as an unpacked folder.
128-
dependencies.add(distribution.getExtracted().getName(), distributionDependency.getExtractedNotation());
129-
}
122+
private ElasticsearchDistribution finalizeDistributionDependencies(Project project, ElasticsearchDistribution distribution) {
123+
DependencyHandler dependencies = project.getDependencies();
124+
// for the distribution as a file, just depend on the artifact directly
125+
DistributionDependency distributionDependency = resolveDependencyNotation(project, distribution);
126+
dependencies.add(distribution.configuration.getName(), distributionDependency.getDefaultNotation());
127+
// no extraction needed for rpm, deb or docker
128+
if (distribution.getType().shouldExtract()) {
129+
// The extracted configuration depends on the artifact directly but has
130+
// an artifact transform registered to resolve it as an unpacked folder.
131+
dependencies.add(distribution.getExtracted().getName(), distributionDependency.getExtractedNotation());
130132
}
133+
return distribution;
131134
}
132135

133136
private DistributionDependency resolveDependencyNotation(Project p, ElasticsearchDistribution distribution) {

‎buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchDistribution.java

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.Collections;
3232
import java.util.Iterator;
3333
import java.util.Locale;
34+
import java.util.function.Function;
3435

3536
public class ElasticsearchDistribution implements Buildable, Iterable<File> {
3637

@@ -103,13 +104,16 @@ public String toString() {
103104
private final Property<Boolean> bundledJdk;
104105
private final Property<Boolean> failIfUnavailable;
105106
private final Configuration extracted;
107+
private Function<ElasticsearchDistribution, ElasticsearchDistribution> distributionFinalizer;
108+
private boolean froozen = false;
106109

107110
ElasticsearchDistribution(
108111
String name,
109112
ObjectFactory objectFactory,
110113
Provider<DockerSupportService> dockerSupport,
111114
Configuration fileConfiguration,
112-
Configuration extractedConfiguration
115+
Configuration extractedConfiguration,
116+
Function<ElasticsearchDistribution, ElasticsearchDistribution> distributionFinalizer
113117
) {
114118
this.name = name;
115119
this.dockerSupport = dockerSupport;
@@ -123,6 +127,7 @@ public String toString() {
123127
this.bundledJdk = objectFactory.property(Boolean.class);
124128
this.failIfUnavailable = objectFactory.property(Boolean.class).convention(true);
125129
this.extracted = extractedConfiguration;
130+
this.distributionFinalizer = distributionFinalizer;
126131
}
127132

128133
public String getName() {
@@ -196,7 +201,22 @@ public String toString() {
196201
return getName() + "_" + getType() + "_" + getVersion();
197202
}
198203

204+
/**
205+
* if not executed before, this
206+
* freezes the distribution configuration and
207+
* runs distribution finalizer logic.
208+
* */
209+
public ElasticsearchDistribution maybeFreeze() {
210+
if (!froozen) {
211+
finalizeValues();
212+
distributionFinalizer.apply(this);
213+
froozen = true;
214+
}
215+
return this;
216+
}
217+
199218
public String getFilepath() {
219+
maybeFreeze();
200220
return configuration.getSingleFile().toString();
201221
}
202222

@@ -221,18 +241,26 @@ public TaskDependency getBuildDependencies() {
221241
if (isDocker() && getFailIfUnavailable() == false && dockerSupport.get().getDockerAvailability().isAvailable == false) {
222242
return task -> Collections.emptySet();
223243
}
244+
maybeFreeze();
245+
return getType().shouldExtract() ? extracted.getBuildDependencies() : configuration.getBuildDependencies();
246+
}
224247

248+
public TaskDependency getArchiveBuildDependencies() {
249+
// For non-required Docker distributions, skip building the distribution is Docker is unavailable
250+
if (isDocker() && getFailIfUnavailable() == false && dockerSupport.get().getDockerAvailability().isAvailable == false) {
251+
return task -> Collections.emptySet();
252+
}
253+
maybeFreeze();
225254
return configuration.getBuildDependencies();
226255
}
227256

228257
@Override
229258
public Iterator<File> iterator() {
230-
return configuration.iterator();
259+
return getType().shouldExtract() ? extracted.iterator() : configuration.iterator();
231260
}
232261

233262
// internal, make this distribution's configuration unmodifiable
234263
void finalizeValues() {
235-
236264
if (getType() == Type.INTEG_TEST_ZIP) {
237265
if (platform.getOrNull() != null) {
238266
throw new IllegalArgumentException(

‎buildSrc/src/main/java/org/elasticsearch/gradle/test/DistroTestPlugin.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,8 @@ public void apply(Project project) {
113113
for (ElasticsearchDistribution distribution : testDistributions) {
114114
String taskname = destructiveDistroTestTaskName(distribution);
115115
TaskProvider<?> depsTask = project.getTasks().register(taskname + "#deps");
116-
depsTask.configure(t -> t.dependsOn(distribution, examplePlugin, quotaAwareFsPlugin));
116+
// explicitly depend on the archive not on the implicit extracted distribution
117+
depsTask.configure(t -> t.dependsOn(distribution.getArchiveBuildDependencies(), examplePlugin, quotaAwareFsPlugin));
117118
depsTasks.put(taskname, depsTask);
118119
TaskProvider<Test> destructiveTask = configureTestTask(project, taskname, distribution, t -> {
119120
t.onlyIf(t2 -> distribution.isDocker() == false || dockerSupport.get().getDockerAvailability().isAvailable);

‎buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ public void setVersions(List<String> versions) {
231231
}
232232

233233
private void doSetVersion(String version) {
234-
String distroName = "testclusters" + path.replace(":", "-") + "-" + this.name + "-" + version + "-";
234+
String distroName = "testclusters" + path.replace(":", "-") + "-" + this.name + "-" + version;
235235
NamedDomainObjectContainer<ElasticsearchDistribution> container = DistributionDownloadPlugin.getContainer(project);
236236
if (container.findByName(distroName) == null) {
237237
container.create(distroName);
@@ -425,6 +425,7 @@ public Path getConfigDir() {
425425
public void freeze() {
426426
requireNonNull(testDistribution, "null testDistribution passed when configuring test cluster `" + this + "`");
427427
LOGGER.info("Locking configuration of `{}`", this);
428+
distributions.stream().forEach(d -> d.maybeFreeze());
428429
configurationFrozen.set(true);
429430
}
430431

‎buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersAware.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ default void useCluster(ElasticsearchCluster cluster) {
3535
throw new TestClustersException("Task " + getPath() + " can't use test cluster from" + " another project " + cluster);
3636
}
3737

38-
cluster.getNodes().stream().flatMap(node -> node.getDistributions().stream()).forEach(distro -> dependsOn(distro.getExtracted()));
38+
cluster.getNodes()
39+
.stream()
40+
.flatMap(node -> node.getDistributions().stream())
41+
.forEach(distro -> dependsOn(getProject().provider(() -> distro.maybeFreeze())));
3942
cluster.getNodes().forEach(node -> dependsOn((Callable<Collection<Configuration>>) node::getPluginAndModuleConfigurations));
4043
getClusters().add(cluster);
4144
}

‎buildSrc/src/test/java/org/elasticsearch/gradle/DistributionDownloadPluginTests.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,6 @@ public void testLocalCurrentVersionIntegTestZip() {
185185
archiveProject.getConfigurations().create("default");
186186
archiveProject.getArtifacts().add("default", new File("doesnotmatter"));
187187
createDistro(project, "distro", VersionProperties.getElasticsearch(), Type.INTEG_TEST_ZIP, null, null, null);
188-
checkPlugin(project);
189188
}
190189

191190
public void testLocalCurrentVersionArchives() {
@@ -200,7 +199,6 @@ public void testLocalCurrentVersionArchives() {
200199
archiveProject.getConfigurations().create("default");
201200
archiveProject.getArtifacts().add("default", new File("doesnotmatter"));
202201
createDistro(project, "distro", VersionProperties.getElasticsearch(), Type.ARCHIVE, platform, flavor, bundledJdk);
203-
checkPlugin(project);
204202
}
205203
}
206204
}
@@ -216,7 +214,6 @@ public void testLocalCurrentVersionPackages() {
216214
packageProject.getConfigurations().create("default");
217215
packageProject.getArtifacts().add("default", new File("doesnotmatter"));
218216
createDistro(project, "distro", VersionProperties.getElasticsearch(), packageType, null, flavor, bundledJdk);
219-
checkPlugin(project);
220217
}
221218
}
222219
}
@@ -294,7 +291,7 @@ private ElasticsearchDistribution createDistro(
294291
if (bundledJdk != null) {
295292
distro.setBundledJdk(bundledJdk);
296293
}
297-
});
294+
}).maybeFreeze();
298295
}
299296

300297
// create a distro and finalize its configuration
@@ -312,12 +309,6 @@ private ElasticsearchDistribution checkDistro(
312309
return distribution;
313310
}
314311

315-
// check the download plugin can be fully configured
316-
private void checkPlugin(Project project) {
317-
DistributionDownloadPlugin plugin = project.getPlugins().getPlugin(DistributionDownloadPlugin.class);
318-
plugin.setupDistributions(project);
319-
}
320-
321312
private void checkBwc(
322313
String projectName,
323314
String config,
@@ -333,7 +324,6 @@ private void checkBwc(
333324
archiveProject.getConfigurations().create(config);
334325
archiveProject.getArtifacts().add(config, new File("doesnotmatter"));
335326
createDistro(project, "distro", version.toString(), type, platform, flavor, true);
336-
checkPlugin(project);
337327
}
338328

339329
private Project createProject(BwcVersions bwcVersions, boolean isInternal) {

0 commit comments

Comments
 (0)