Skip to content

Commit

Permalink
Make Legacy test cluster infrastructure configuration cache compatible (
Browse files Browse the repository at this point in the history 
#101903)

* Move TestCluster plugin and module setup from Node to Cluster
* Do not materialize unused TestCluster
* Fix lazy test cluster evaluation
* Register artifact transforms ones per project
* Make task caching for TestClusterAware tasks CC compatible
* Move stateful logic out of taskgraph.whenready
  • Loading branch information
@breskeby
breskeby authored Nov 28, 2023
1 parent a48c1a0 commit 4a9b4bc
Show file tree
Hide file tree
Showing 19 changed files with 174 additions and 163 deletions.
3 changes: 2 additions & 1 deletion ...c/integTest/groovy/org/elasticsearch/gradle/fixtures/AbstractRestResourcesFuncTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ abstract class AbstractRestResourcesFuncTest extends AbstractGradleFuncTest {
}
"""

subProject(":distribution:archives:integ-test-zip") << "configurations { extracted }"
subProject(":distribution:archives:integ-test-zip") << "configurations.create('extracted')\n"
subProject(":distribution:archives:integ-test-zip") << "configurations.create('default')\n"
}

void setupRestResources(List<String> apis, List<String> tests = [], List<String> xpackTests = []) {
Expand Down
7 changes: 2 additions & 5 deletions ...roovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ class LegacyYamlRestTestPluginFuncTest extends AbstractRestResourcesFuncTest {

def "yamlRestTest does nothing when there are no tests"() {
given:
// RestIntegTestTask not cc compatible due to
configurationCacheCompatible = false
buildFile << """
plugins {
id 'elasticsearch.legacy-yaml-rest-test'
Expand All @@ -43,8 +41,6 @@ class LegacyYamlRestTestPluginFuncTest extends AbstractRestResourcesFuncTest {

def "yamlRestTest executes and copies api and tests to correct source set"() {
given:
// RestIntegTestTask not cc compatible due to
configurationCacheCompatible = false
internalBuild()
buildFile << """
apply plugin: 'elasticsearch.legacy-yaml-rest-test'
Expand All @@ -56,9 +52,10 @@ class LegacyYamlRestTestPluginFuncTest extends AbstractRestResourcesFuncTest {
// can't actually spin up test cluster from this test
tasks.withType(Test).configureEach{ enabled = false }
def clazzpath = sourceSets.yamlRestTest.runtimeClasspath
tasks.register("printYamlRestTestClasspath").configure {
doLast {
println sourceSets.yamlRestTest.runtimeClasspath.asPath
println clazzpath.asPath
}
}
"""
Expand Down
11 changes: 5 additions & 6 deletions ...ternal/src/main/java/org/elasticsearch/gradle/internal/BaseInternalPluginBuildPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,12 @@ public void doCall() {
}
});

boolean isModule = GradleUtils.isModuleProject(project.getPath());
boolean isXPackModule = isModule && project.getPath().startsWith(":x-pack");
if (isModule == false || isXPackModule) {
addNoticeGeneration(project, extension);
}
project.afterEvaluate(p -> {
boolean isModule = GradleUtils.isModuleProject(p.getPath());
boolean isXPackModule = isModule && p.getPath().startsWith(":x-pack");
if (isModule == false || isXPackModule) {
addNoticeGeneration(p, extension);
}

@SuppressWarnings("unchecked")
NamedDomainObjectContainer<ElasticsearchCluster> testClusters = (NamedDomainObjectContainer<ElasticsearchCluster>) project
.getExtensions()
Expand Down
18 changes: 8 additions & 10 deletions build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/NoticeTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
import org.gradle.api.file.ProjectLayout;
import org.gradle.api.file.SourceDirectorySet;
import org.gradle.api.internal.file.FileOperations;
import org.gradle.api.model.ObjectFactory;
import org.gradle.api.provider.ListProperty;
import org.gradle.api.tasks.CacheableTask;
import org.gradle.api.tasks.InputFile;
import org.gradle.api.tasks.InputFiles;
import org.gradle.api.tasks.Internal;
import org.gradle.api.tasks.Optional;
import org.gradle.api.tasks.OutputFile;
import org.gradle.api.tasks.PathSensitive;
Expand All @@ -43,7 +43,7 @@
* A task to create a notice file which includes dependencies' notices.
*/
@CacheableTask
public class NoticeTask extends DefaultTask {
public abstract class NoticeTask extends DefaultTask {

@InputFile
@PathSensitive(PathSensitivity.RELATIVE)
Expand All @@ -57,19 +57,17 @@ public class NoticeTask extends DefaultTask {
/**
* Directories to include notices from
*/
private final ListProperty<File> licensesDirs;
@Internal
abstract ListProperty<File> getLicenseDirs();

private final FileOperations fileOperations;
private ObjectFactory objectFactory;

@Inject
public NoticeTask(BuildLayout buildLayout, ProjectLayout projectLayout, FileOperations fileOperations, ObjectFactory objectFactory) {
this.objectFactory = objectFactory;
public NoticeTask(BuildLayout buildLayout, ProjectLayout projectLayout, FileOperations fileOperations) {
this.fileOperations = fileOperations;
setDescription("Create a notice file from dependencies");
// Default licenses directory is ${projectDir}/licenses (if it exists)
licensesDirs = objectFactory.listProperty(File.class);
licensesDirs.add(projectLayout.getProjectDirectory().dir("licenses").getAsFile());
getLicenseDirs().add(projectLayout.getProjectDirectory().dir("licenses").getAsFile());
inputFile = new File(buildLayout.getRootDirectory(), "NOTICE.txt");
outputFile = projectLayout.getBuildDirectory().dir("notices/" + getName()).get().file("NOTICE.txt").getAsFile();
}
Expand All @@ -78,7 +76,7 @@ public NoticeTask(BuildLayout buildLayout, ProjectLayout projectLayout, FileOper
* Add notices from the specified directory.
*/
public void licensesDir(File licensesDir) {
licensesDirs.add(licensesDir);
getLicenseDirs().add(licensesDir);
}

public void source(Object source) {
Expand Down Expand Up @@ -185,7 +183,7 @@ public FileCollection getNoticeFiles() {
}

private List<File> existingLicenseDirs() {
return licensesDirs.get().stream().filter(d -> d.exists()).collect(Collectors.toList());
return getLicenseDirs().get().stream().filter(d -> d.exists()).collect(Collectors.toList());
}

@InputFiles
Expand Down
9 changes: 1 addition & 8 deletions ...ternal/src/main/java/org/elasticsearch/gradle/internal/test/LegacyRestTestBasePlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,8 @@
import org.gradle.api.specs.NotSpec;
import org.gradle.api.specs.Spec;
import org.gradle.api.tasks.Sync;
import org.gradle.api.tasks.TaskContainer;
import org.gradle.api.tasks.bundling.Zip;

import java.util.Collections;

import javax.inject.Inject;

import static org.elasticsearch.gradle.internal.RestrictedBuildApiService.BUILD_API_RESTRICTIONS_SYS_PROPERTY;
Expand Down Expand Up @@ -131,11 +128,7 @@ public void apply(Project project) {
}

private void configureCacheability(StandaloneRestIntegTestTask testTask) {
TaskContainer tasks = project.getTasks();
Spec<Task> taskSpec = t -> tasks.withType(StandaloneRestIntegTestTask.class)
.stream()
.filter(task -> task != testTask)
.anyMatch(task -> Collections.disjoint(task.getClusters(), testTask.getClusters()) == false);
Spec<Task> taskSpec = task -> testTask.getClusters().stream().anyMatch(ElasticsearchCluster::isShared);
testTask.getOutputs()
.doNotCacheIf(
"Caching disabled for this task since it uses a cluster shared by other tasks",
Expand Down
2 changes: 1 addition & 1 deletion ...ools-internal/src/main/java/org/elasticsearch/gradle/internal/test/TestWithSslPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void apply(Project project) {
NamedDomainObjectContainer<ElasticsearchCluster> clusters = (NamedDomainObjectContainer<ElasticsearchCluster>) project
.getExtensions()
.getByName(TestClustersPlugin.EXTENSION_NAME);
clusters.all(c -> {
clusters.configureEach(c -> {
if (BuildParams.isInFipsJvm()) {
c.setting("xpack.security.transport.ssl.key", "test-node.key");
c.keystore("xpack.security.transport.ssl.secure_key_passphrase", "test-node-key-password");
Expand Down
2 changes: 1 addition & 1 deletion build-tools/src/integTest/groovy/org/elasticsearch/gradle/TestClustersPluginFuncTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class TestClustersPluginFuncTest extends AbstractGradleFuncTest {

then:
result.output.contains("Task ':myTask' is not up-to-date because:\n" +
" Input property 'clusters.myCluster\$0.nodes.\$0.$propertyName'")
" Input property 'clusters.myCluster\$0.$propertyName'")
result.output.contains("elasticsearch-keystore script executed!")
assertEsOutputContains("myCluster", "Starting Elasticsearch process")
assertEsOutputContains("myCluster", "Stopping node")
Expand Down
1 change: 1 addition & 0 deletions build-tools/src/main/java/org/elasticsearch/gradle/DistributionDownloadPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public void apply(Project project) {
}

private void setupDistributionContainer(Project project, Property<Boolean> dockerAvailable) {

distributionsContainer = project.container(ElasticsearchDistribution.class, name -> {
Configuration fileConfiguration = project.getConfigurations().create(DISTRO_CONFIG_PREFIX + name);
Configuration extractedConfiguration = project.getConfigurations().create(DISTRO_EXTRACTED_CONFIG_PREFIX + name);
Expand Down
2 changes: 1 addition & 1 deletion build-tools/src/main/java/org/elasticsearch/gradle/ElasticsearchDistribution.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public String toString() {
private final Property<Boolean> failIfUnavailable;
private final Property<Boolean> preferArchive;
private final ConfigurableFileCollection extracted;
private Action<ElasticsearchDistribution> distributionFinalizer;
private transient Action<ElasticsearchDistribution> distributionFinalizer;
private boolean frozen = false;

ElasticsearchDistribution(
Expand Down
99 changes: 89 additions & 10 deletions build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,27 @@
import org.gradle.api.Named;
import org.gradle.api.NamedDomainObjectContainer;
import org.gradle.api.Project;
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.artifacts.Dependency;
import org.gradle.api.artifacts.type.ArtifactTypeDefinition;
import org.gradle.api.file.ArchiveOperations;
import org.gradle.api.file.ConfigurableFileCollection;
import org.gradle.api.file.FileCollection;
import org.gradle.api.file.FileSystemOperations;
import org.gradle.api.file.RegularFile;
import org.gradle.api.internal.file.FileOperations;
import org.gradle.api.logging.Logger;
import org.gradle.api.logging.Logging;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.Classpath;
import org.gradle.api.tasks.InputFiles;
import org.gradle.api.tasks.Internal;
import org.gradle.api.tasks.Nested;
import org.gradle.api.tasks.PathSensitive;
import org.gradle.api.tasks.PathSensitivity;
import org.gradle.api.tasks.Sync;
import org.gradle.api.tasks.TaskProvider;
import org.gradle.api.tasks.bundling.AbstractArchiveTask;
import org.gradle.api.tasks.bundling.Zip;
import org.gradle.process.ExecOperations;

Expand All @@ -35,6 +44,8 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.security.GeneralSecurityException;
import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -46,6 +57,9 @@
import java.util.function.Supplier;
import java.util.stream.Collectors;

import static org.elasticsearch.gradle.plugin.BasePluginBuildPlugin.EXPLODED_BUNDLE_CONFIG;
import static org.elasticsearch.gradle.testclusters.TestClustersPlugin.BUNDLE_ATTRIBUTE;

public class ElasticsearchCluster implements TestClusterConfiguration, Named {

private static final Logger LOGGER = Logging.getLogger(ElasticsearchNode.class);
Expand All @@ -59,7 +73,7 @@ public class ElasticsearchCluster implements TestClusterConfiguration, Named {
private final FileOperations fileOperations;
private final File workingDirBase;
private final LinkedHashMap<String, Predicate<TestClusterConfiguration>> waitConditions = new LinkedHashMap<>();
private final Project project;
private final transient Project project;
private final Provider<ReaperService> reaper;
private final FileSystemOperations fileSystemOperations;
private final ArchiveOperations archiveOperations;
Expand All @@ -68,6 +82,10 @@ public class ElasticsearchCluster implements TestClusterConfiguration, Named {
private final Function<Version, Boolean> isReleasedVersion;
private int nodeIndex = 0;

private final ConfigurableFileCollection pluginAndModuleConfiguration;

private boolean shared = false;

public ElasticsearchCluster(
String path,
String clusterName,
Expand All @@ -93,6 +111,7 @@ public ElasticsearchCluster(
this.runtimeJava = runtimeJava;
this.isReleasedVersion = isReleasedVersion;
this.nodes = project.container(ElasticsearchNode.class);
this.pluginAndModuleConfiguration = project.getObjects().fileCollection();
this.nodes.add(
new ElasticsearchNode(
safeName(clusterName),
Expand All @@ -113,6 +132,29 @@ public ElasticsearchCluster(
addWaitForClusterHealth();
}

/**
* this cluster si marked as shared across TestClusterAware tasks
* */
@Internal
public boolean isShared() {
return shared;
}

protected void setShared(boolean shared) {
this.shared = shared;
}

@Classpath
public FileCollection getInstalledClasspath() {
return pluginAndModuleConfiguration.getAsFileTree().filter(f -> f.getName().endsWith(".jar"));
}

@InputFiles
@PathSensitive(PathSensitivity.RELATIVE)
public FileCollection getInstalledFiles() {
return pluginAndModuleConfiguration.getAsFileTree().filter(f -> f.getName().endsWith(".jar") == false);
}

public void setNumberOfNodes(int numberOfNodes) {
checkFrozen();

Expand Down Expand Up @@ -195,34 +237,70 @@ public void setTestDistribution(TestDistribution distribution) {
nodes.all(each -> each.setTestDistribution(distribution));
}

@Override
public void plugin(Provider<RegularFile> plugin) {
nodes.all(each -> each.plugin(plugin));
private void registerExtractedConfig(Provider<RegularFile> pluginProvider) {
Dependency pluginDependency = this.project.getDependencies().create(project.files(pluginProvider));
Configuration extractedConfig = project.getConfigurations().detachedConfiguration(pluginDependency);
extractedConfig.getAttributes().attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, ArtifactTypeDefinition.DIRECTORY_TYPE);
extractedConfig.getAttributes().attribute(BUNDLE_ATTRIBUTE, true);
pluginAndModuleConfiguration.from(extractedConfig);
}

@Override
public void plugin(String pluginProjectPath) {
plugin(maybeCreatePluginOrModuleDependency(pluginProjectPath, "zip"));
}

public void plugin(TaskProvider<Zip> plugin) {
nodes.all(each -> each.plugin(plugin));
plugin(plugin.flatMap(AbstractArchiveTask::getArchiveFile));
}

@Override
public void plugin(String pluginProjectPath) {
nodes.all(each -> each.plugin(pluginProjectPath));
public void plugin(Provider<RegularFile> plugin) {
registerExtractedConfig(plugin);
nodes.all(each -> each.plugin(plugin));
}

@Override
public void module(Provider<RegularFile> module) {
registerExtractedConfig(module);
nodes.all(each -> each.module(module));
}

@Override
public void module(TaskProvider<Sync> module) {
nodes.all(each -> each.module(module));
module(project.getLayout().file(module.map(Sync::getDestinationDir)));
}

@Override
public void module(String moduleProjectPath) {
nodes.all(each -> each.module(moduleProjectPath));
module(maybeCreatePluginOrModuleDependency(moduleProjectPath, EXPLODED_BUNDLE_CONFIG));
}

private final Map<String, Configuration> pluginAndModuleConfigurations = new HashMap<>();

// package protected so only TestClustersAware can access
@Internal
Collection<Configuration> getPluginAndModuleConfigurations() {
return pluginAndModuleConfigurations.values();
}

// creates a configuration to depend on the given plugin project, then wraps that configuration
// to grab the zip as a file provider
private Provider<RegularFile> maybeCreatePluginOrModuleDependency(String path, String consumingConfiguration) {
var configuration = pluginAndModuleConfigurations.computeIfAbsent(path, key -> {
var bundleDependency = this.project.getDependencies().project(Map.of("path", path, "configuration", consumingConfiguration));
return project.getConfigurations().detachedConfiguration(bundleDependency);
});

Provider<File> fileProvider = configuration.getElements()
.map(
s -> s.stream()
.findFirst()
.orElseThrow(
() -> new IllegalStateException(consumingConfiguration + " configuration of project " + path + " had no files")
)
.getAsFile()
);
return project.getLayout().file(fileProvider);
}

@Override
Expand Down Expand Up @@ -579,4 +657,5 @@ public int hashCode() {
public String toString() {
return "cluster{" + path + ":" + clusterName + "}";
}

}
Loading

0 comments on commit 4a9b4bc

Please sign in to comment.