Skip to content

[Test] Allow configuring configDir for the Java test cluster #125094

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 12 commits into from
Mar 20, 2025
Merged

[Test] Allow configuring configDir for the Java test cluster #125094

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
280d89d
[Test] Allow add and remove files on Java test cluster
ywangd Mar 18, 2025
7fe0911
typo
ywangd Mar 18, 2025
46b2849
add spec option for config dir function
ywangd Mar 19, 2025
a3bd7a2
revert changes for add/remove config files
ywangd Mar 19, 2025
942695e
more removal
ywangd Mar 19, 2025
df7c6e7
[CI] Auto commit changes from spotless
Mar 19, 2025
9592e91
Merge remote-tracking branch 'origin/main' into add-and-remove-file-o…
ywangd Mar 19, 2025
0851c4d
change to use raw path
ywangd Mar 19, 2025
d8bb5cd
replace with supplier
ywangd Mar 19, 2025
0651da0
reduce laziness
ywangd Mar 19, 2025
b69d60e
Merge branch 'main' into add-and-remove-file-on-java-test-cluster
elasticmachine Mar 19, 2025
d6b7838
tweak
ywangd Mar 19, 2025
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
18 changes: 13 additions & 5 deletions ...sters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,11 @@ public Node(
this.repoDir = baseWorkingDir.resolve("repo");
this.dataDir = workingDir.resolve("data");
this.logsDir = workingDir.resolve("logs");
this.configDir = workingDir.resolve("config");
Path configDir = null;
if (spec.getConfigDirSupplier() != null) {
configDir = spec.getConfigDirSupplier().get();
}
this.configDir = configDir == null ? workingDir.resolve("config") : configDir;
this.tempDir = workingDir.resolve("tmp"); // elasticsearch temporary directory
this.debugPort = DefaultLocalClusterHandle.NEXT_DEBUG_PORT.getAndIncrement();
}
Expand Down Expand Up @@ -294,6 +298,10 @@ Path getWorkingDir() {
return workingDir;
}

Path getConfigDir() {
return configDir;
}

public void waitUntilReady() {
try {
Retry.retryUntilTrue(NODE_UP_TIMEOUT, Duration.ofMillis(500), () -> {
Expand Down Expand Up @@ -426,7 +434,7 @@ private void writeConfiguration() {
try (Stream<Path> configFiles = Files.walk(distributionDir.resolve("config"))) {
for (Path file : configFiles.toList()) {
Path relativePath = distributionDir.resolve("config").relativize(file);
Path dest = configDir.resolve(relativePath);
Path dest = configDir.resolve(relativePath.toFile().getPath());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to do this path -> file -> path conversion?

Copy link
Member Author

Choose a reason for hiding this comment

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

The last conversion gives a String. The reason is that temporary directory created by Lucene temporariy directory, e.g. LuceneTestCase#createTempDir returns a Lucene FilterDirectory which always works for resolve(String) but throws for resolve(Path) if the given Path argument is not also a FilterDirectory. Since I changed the other PR to use Junit's TemporaryDirectory, this is no longer an issue for now. But I think it might be worthwhile to keep the change so that it does not just break if someone decides to use Lucene temporary in future?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have super strong feelings about working around idiosyncrasies with lucene types. Can this be simplified to relativePath.toString()?

if (Files.exists(dest) == false) {
Files.createDirectories(dest.getParent());
Files.copy(file, dest);
Expand Down Expand Up @@ -640,7 +648,7 @@ private void configureSecurity() {
if (operators.isEmpty() == false) {
// TODO: Support service accounts here
final String operatorUsersFileName = "operator_users.yml";
final Path destination = workingDir.resolve("config").resolve(operatorUsersFileName);
final Path destination = configDir.resolve(operatorUsersFileName);
if (Files.exists(destination)) {
throw new IllegalStateException(
"Operator users file ["
Expand All @@ -667,7 +675,7 @@ private void configureSecurity() {
}

private void writeRolesFile() {
Path destination = workingDir.resolve("config").resolve("roles.yml");
Path destination = configDir.resolve("roles.yml");
spec.getRolesFiles().forEach(rolesFile -> {
try (
Writer writer = Files.newBufferedWriter(destination, StandardOpenOption.APPEND);
Expand Down Expand Up @@ -857,7 +865,7 @@ private void startElasticsearch() {

private Map<String, String> getEnvironmentVariables() {
Map<String, String> environment = new HashMap<>(spec.resolveEnvironment());
environment.put("ES_PATH_CONF", workingDir.resolve("config").toString());
environment.put("ES_PATH_CONF", configDir.toString());
environment.put("ES_TMPDIR", workingDir.resolve("tmp").toString());
// Windows requires this as it defaults to `c:\windows` despite ES_TMPDIR
environment.put("TMP", workingDir.resolve("tmp").toString());
Expand Down
3 changes: 2 additions & 1 deletion ...s/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterSpecBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ private LocalNodeSpec build(LocalClusterSpec cluster, int nodeIndex) {
getExtraConfigFiles(),
getSystemPropertyProviders(),
getSystemProperties(),
getJvmArgs()
getJvmArgs(),
getConfigDirSupplier()
);
}
}
Expand Down
12 changes: 12 additions & 0 deletions ...clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalSpecBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.test.cluster.util.Version;
import org.elasticsearch.test.cluster.util.resource.Resource;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.HashMap;
Expand Down Expand Up @@ -47,6 +48,7 @@ public abstract class AbstractLocalSpecBuilder<T extends LocalSpecBuilder<?>> im
private DistributionType distributionType;
private Version version;
private String keystorePassword;
private Supplier<Path> configDirSupplier;

protected AbstractLocalSpecBuilder(AbstractLocalSpecBuilder<?> parent) {
this.parent = parent;
Expand Down Expand Up @@ -270,6 +272,16 @@ public String getKeystorePassword() {
return inherit(() -> parent.getKeystorePassword(), keystorePassword);
}

@Override
public T withConfigDir(Supplier<Path> configDirSupplier) {
this.configDirSupplier = configDirSupplier;
return cast(this);
}

public Supplier<Path> getConfigDirSupplier() {
return inherit(() -> parent.getConfigDirSupplier(), configDirSupplier);
}

@Override
public T version(Version version) {
this.version = version;
Expand Down
10 changes: 5 additions & 5 deletions ...lusters/src/main/java/org/elasticsearch/test/cluster/local/DefaultLocalClusterHandle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,15 +237,15 @@ private WaitForHttpResource configureWaitForReady() throws MalformedURLException
private void configureWaitSecurity(WaitForHttpResource wait, Node node) {
String caFile = node.getSpec().getSetting("xpack.security.http.ssl.certificate_authorities", null);
if (caFile != null) {
wait.setCertificateAuthorities(node.getWorkingDir().resolve("config").resolve(caFile).toFile());
wait.setCertificateAuthorities(node.getConfigDir().resolve(caFile).toFile());
}
String sslCertFile = node.getSpec().getSetting("xpack.security.http.ssl.certificate", null);
if (sslCertFile != null) {
wait.setCertificateAuthorities(node.getWorkingDir().resolve("config").resolve(sslCertFile).toFile());
wait.setCertificateAuthorities(node.getConfigDir().resolve(sslCertFile).toFile());
}
String sslKeystoreFile = node.getSpec().getSetting("xpack.security.http.ssl.keystore.path", null);
if (sslKeystoreFile != null && caFile == null) { // Can not set both trust stores and CA
wait.setTrustStoreFile(node.getWorkingDir().resolve("config").resolve(sslKeystoreFile).toFile());
wait.setTrustStoreFile(node.getConfigDir().resolve(sslKeystoreFile).toFile());
}
String keystorePassword = node.getSpec().getSetting("xpack.security.http.ssl.keystore.secure_password", null);
if (keystorePassword != null) {
Expand All @@ -254,7 +254,7 @@ private void configureWaitSecurity(WaitForHttpResource wait, Node node) {
}

private boolean isSecurityAutoConfigured(Node node) {
Path configFile = node.getWorkingDir().resolve("config").resolve("elasticsearch.yml");
Path configFile = node.getConfigDir().resolve("elasticsearch.yml");
try (Stream<String> lines = Files.lines(configFile)) {
return lines.anyMatch(l -> l.contains("BEGIN SECURITY AUTO CONFIGURATION"));
} catch (IOException e) {
Expand All @@ -273,7 +273,7 @@ private void writeUnicastHostsFile() {
LOGGER.info("Skipping writing unicast hosts file for node {}", node.getName());
return;
}
Path hostsFile = node.getWorkingDir().resolve("config").resolve("unicast_hosts.txt");
Path hostsFile = node.getConfigDir().resolve("unicast_hosts.txt");
LOGGER.info("Writing unicast hosts file {} for node {}", hostsFile, node.getName());
Files.writeString(hostsFile, transportUris);
} catch (IOException e) {
Expand Down
14 changes: 12 additions & 2 deletions test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/LocalClusterSpec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@
import org.elasticsearch.test.cluster.util.Version;
import org.elasticsearch.test.cluster.util.resource.Resource;

import java.nio.file.Path;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;

public class LocalClusterSpec implements ClusterSpec {
Expand Down Expand Up @@ -103,6 +105,7 @@ public static class LocalNodeSpec {
private final List<SystemPropertyProvider> systemPropertyProviders;
private final Map<String, String> systemProperties;
private final List<String> jvmArgs;
private final Supplier<Path> configDirSupplier;
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, I don't think we need to plumb laziness all the way through to the spec here. We only build the spec once we attempt to start the server, at which point everything we depend on needs to be initialized anyway. Let's just make this a Path to make downstream usages simpler.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. Pushed 0651da0

private Version version;

public LocalNodeSpec(
Expand All @@ -124,7 +127,8 @@ public LocalNodeSpec(
Map<String, Resource> extraConfigFiles,
List<SystemPropertyProvider> systemPropertyProviders,
Map<String, String> systemProperties,
List<String> jvmArgs
List<String> jvmArgs,
Supplier<Path> configDirSupplier
) {
this.cluster = cluster;
this.name = name;
Expand All @@ -145,6 +149,7 @@ public LocalNodeSpec(
this.systemPropertyProviders = systemPropertyProviders;
this.systemProperties = systemProperties;
this.jvmArgs = jvmArgs;
this.configDirSupplier = configDirSupplier;
}

void setVersion(Version version) {
Expand Down Expand Up @@ -203,6 +208,10 @@ public List<String> getJvmArgs() {
return jvmArgs;
}

public Supplier<Path> getConfigDirSupplier() {
return configDirSupplier;
}

public boolean isSecurityEnabled() {
return Boolean.parseBoolean(getSetting("xpack.security.enabled", getVersion().onOrAfter("8.0.0") ? "true" : "false"));
}
Expand Down Expand Up @@ -339,7 +348,8 @@ private LocalNodeSpec getFilteredSpec(SettingsProvider filteredProvider, Setting
n.extraConfigFiles,
n.systemPropertyProviders,
n.systemProperties,
n.jvmArgs
n.jvmArgs,
n.configDirSupplier
)
)
.toList();
Expand Down
7 changes: 7 additions & 0 deletions test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/LocalSpecBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.elasticsearch.test.cluster.util.Version;
import org.elasticsearch.test.cluster.util.resource.Resource;

import java.nio.file.Path;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.function.Supplier;
Expand Down Expand Up @@ -155,4 +156,10 @@ interface LocalSpecBuilder<T extends LocalSpecBuilder<?>> {
* Adds an additional command line argument to node JVM arguments.
*/
T jvmArg(String arg);

/**
* Register a supplier to provide the config directory. The default config directory
* is used when the supplier is null or the return value of the supplier is null.
*/
T withConfigDir(Supplier<Path> configDirSupplier);
}