Skip to content

Commit

Permalink
Deprecate ExternalTestCluster (#101844)
Browse files Browse the repository at this point in the history
`ExternalTestCluster` doesn't really make sense now that the transport
client is removed. We only use it in the ML integ test suite and it'd be
good to avoid expanding its usage further, so this commit deprecates it
and removes the functionality in `ESIntegTestCase` that might quietly
switch to using it in a new test suite if running with certain system
properties.

Relates #49582
  • Loading branch information
DaveCTurner committed Nov 6, 2023
1 parent cef2b80 commit 4a37aef
Show file tree
Hide file tree
Showing 31 changed files with 41 additions and 178 deletions.
Expand Up @@ -17,7 +17,6 @@
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.InternalTestCluster;

import java.io.FileNotFoundException;
import java.io.IOException;
Expand All @@ -44,15 +43,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(CommonAnalysisPlugin.class);
}

/**
* This test needs to write to the config directory, this is difficult in an external cluster so we overwrite this to force running with
* {@link InternalTestCluster}
*/
@Override
protected boolean ignoreExternalCluster() {
return true;
}

public void testSynonymsUpdateable() throws FileNotFoundException, IOException, InterruptedException {
testSynonymsUpdate(false);
}
Expand Down
Expand Up @@ -36,10 +36,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return List.of(DataStreamsPlugin.class, MockTransportService.TestPlugin.class);
}

protected boolean ignoreExternalCluster() {
return true;
}

public void testGetLifecycle() throws Exception {
DataStreamLifecycle lifecycle = randomLifecycle();
putComposableIndexTemplate("id1", null, List.of("with-lifecycle*"), null, null, lifecycle);
Expand Down
Expand Up @@ -90,10 +90,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return List.of(DataStreamsPlugin.class, MockTransportService.TestPlugin.class);
}

protected boolean ignoreExternalCluster() {
return true;
}

@Override
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
Settings.Builder settings = Settings.builder().put(super.nodeSettings(nodeOrdinal, otherSettings));
Expand Down
Expand Up @@ -62,10 +62,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return List.of(DataStreamsPlugin.class, MockTransportService.TestPlugin.class);
}

protected boolean ignoreExternalCluster() {
return true;
}

@Override
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
Settings.Builder settings = Settings.builder().put(super.nodeSettings(nodeOrdinal, otherSettings));
Expand Down
Expand Up @@ -50,11 +50,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(IngestCommonPlugin.class, CustomScriptPlugin.class);
}

@Override
protected boolean ignoreExternalCluster() {
return true;
}

public static class CustomScriptPlugin extends MockScriptPlugin {
@Override
protected Map<String, Function<Map<String, Object>, Object>> pluginScripts() {
Expand Down
Expand Up @@ -29,11 +29,6 @@
@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE)
public abstract class ParentChildTestCase extends ESIntegTestCase {

@Override
protected boolean ignoreExternalCluster() {
return true;
}

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(InternalSettingsPlugin.class, ParentJoinPlugin.class);
Expand Down
Expand Up @@ -56,11 +56,6 @@ public class ReindexDocumentationIT extends ESIntegTestCase {
private static final Semaphore ALLOWED_OPERATIONS = new Semaphore(0);
private static final String INDEX_NAME = "source_index";

@Override
protected boolean ignoreExternalCluster() {
return true;
}

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(ReindexPlugin.class, ReindexCancellationPlugin.class);
Expand Down
Expand Up @@ -19,11 +19,6 @@

public abstract class ESNetty4IntegTestCase extends ESIntegTestCase {

@Override
protected boolean ignoreExternalCluster() {
return true;
}

@Override
protected boolean addMockTransportService() {
return false;
Expand Down
Expand Up @@ -41,11 +41,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return List.of(getTestTransportPlugin(), MainRestPlugin.class);
}

@Override
protected boolean ignoreExternalCluster() {
return true;
}

public static void assertOK(Response response) {
assertThat(response.getStatusLine().getStatusCode(), oneOf(200, 201));
}
Expand Down
Expand Up @@ -45,11 +45,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return List.of(CustomIngestTestPlugin.class, CustomScriptPlugin.class);
}

@Override
protected boolean ignoreExternalCluster() {
return true;
}

@SuppressWarnings("unchecked")
public void testIngestStatsNamesAndTypes() throws IOException {
String pipeline1 = org.elasticsearch.core.Strings.format("""
Expand Down
Expand Up @@ -34,10 +34,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return Collections.singletonList(TestPersistentTasksPlugin.class);
}

protected boolean ignoreExternalCluster() {
return true;
}

public void testFullClusterRestart() throws Exception {
PersistentTasksService service = internalCluster().getInstance(PersistentTasksService.class);
int numberOfTasks = randomIntBetween(1, 10);
Expand Down
Expand Up @@ -51,10 +51,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return Collections.singletonList(TestPersistentTasksPlugin.class);
}

protected boolean ignoreExternalCluster() {
return true;
}

@Before
public void resetNonClusterStateCondition() {
TestPersistentTasksExecutor.setNonClusterStateCondition(true);
Expand Down
Expand Up @@ -36,11 +36,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return singletonList(TestPersistentTasksPlugin.class);
}

@Override
protected boolean ignoreExternalCluster() {
return true;
}

/**
* Test that the {@link EnableAssignmentDecider#CLUSTER_TASKS_ALLOCATION_ENABLE_SETTING} setting correctly
* prevents persistent tasks to be assigned after a cluster restart.
Expand Down
Expand Up @@ -193,11 +193,6 @@ protected boolean addMockHttpTransport() {
return false; // enable http
}

@Override
protected boolean ignoreExternalCluster() {
return true;
}

public void testFieldAlias() {
FieldCapabilitiesResponse response = client().prepareFieldCaps().setFields("distance", "route_length_miles").get();

Expand Down
Expand Up @@ -158,9 +158,7 @@
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
Expand Down Expand Up @@ -1983,46 +1981,7 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return Collections.emptyList();
}

private ExternalTestCluster buildExternalCluster(String clusterAddresses, String clusterName) throws IOException {
String[] stringAddresses = clusterAddresses.split(",");
TransportAddress[] transportAddresses = new TransportAddress[stringAddresses.length];
int i = 0;
for (String stringAddress : stringAddresses) {
URL url = new URL("http://" + stringAddress);
InetAddress inetAddress = InetAddress.getByName(url.getHost());
transportAddresses[i++] = new TransportAddress(new InetSocketAddress(inetAddress, url.getPort()));
}
return new ExternalTestCluster(
createTempDir(),
externalClusterClientSettings(),
nodePlugins(),
getClientWrapper(),
clusterName,
transportAddresses
);
}

protected Settings externalClusterClientSettings() {
return Settings.EMPTY;
}

protected boolean ignoreExternalCluster() {
return false;
}

protected TestCluster buildTestCluster(Scope scope, long seed) throws IOException {
String clusterAddresses = System.getProperty(TESTS_CLUSTER);
if (Strings.hasLength(clusterAddresses) && ignoreExternalCluster() == false) {
if (scope == Scope.TEST) {
throw new IllegalArgumentException("Cannot run TEST scope test with " + TESTS_CLUSTER);
}
String clusterName = System.getProperty(TESTS_CLUSTER_NAME);
if (Strings.isNullOrEmpty(clusterName)) {
throw new IllegalArgumentException("External test cluster name must be provided");
}
return buildExternalCluster(clusterAddresses, clusterName);
}

final String nodePrefix = switch (scope) {
case TEST -> TEST_CLUSTER_NODE_PREFIX;
case SUITE -> SUITE_CLUSTER_NODE_PREFIX;
Expand Down
Expand Up @@ -51,7 +51,11 @@
* External cluster to run the tests against.
* It is a pure immutable test cluster that allows to send requests to a pre-existing cluster
* and supports by nature all the needed test operations like wipeIndices etc.
*
* @deprecated not a realistic test setup since the removal of the transport client, use {@link ESIntegTestCase} for internal-cluster tests
* or {@link org.elasticsearch.test.rest.ESRestTestCase} otherwise.
*/
@Deprecated(forRemoval = true)
public final class ExternalTestCluster extends TestCluster {

private static final Logger logger = LogManager.getLogger(ExternalTestCluster.class);
Expand Down
Expand Up @@ -60,11 +60,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return Collections.unmodifiableList(result);
}

@Override
protected boolean ignoreExternalCluster() {
return true;
}

public void testCapacityRestCancellationAndResponse() throws Exception {
internalCluster().startMasterOnlyNode();

Expand Down
Expand Up @@ -43,10 +43,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return List.of(DataStreamsPlugin.class, LocalStateCompositeXPackPlugin.class, Downsample.class, AggregateMetricMapperPlugin.class);
}

protected boolean ignoreExternalCluster() {
return true;
}

@Override
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
Settings.Builder settings = Settings.builder().put(super.nodeSettings(nodeOrdinal, otherSettings));
Expand Down
Expand Up @@ -139,8 +139,4 @@ public void testRestCancellation() throws Exception {
expectThrows(CancellationException.class, future::actionGet);
}

@Override
protected boolean ignoreExternalCluster() {
return true;
}
}
Expand Up @@ -62,11 +62,6 @@ public void refreshDataStreamAndPolicy() {
managedIndex = "index-" + randomAlphaOfLengthBetween(10, 15).toLowerCase(Locale.ROOT);
}

@Override
protected boolean ignoreExternalCluster() {
return true;
}

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(LocalStateCompositeXPackPlugin.class, IndexLifecycle.class, Ccr.class);
Expand Down
Expand Up @@ -80,10 +80,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return List.of(LocalStateCompositeXPackPlugin.class, IndexLifecycle.class, DataStreamsPlugin.class);
}

protected boolean ignoreExternalCluster() {
return true;
}

@Override
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
Settings.Builder settings = Settings.builder().put(super.nodeSettings(nodeOrdinal, otherSettings));
Expand Down
Expand Up @@ -55,11 +55,6 @@ public void refreshDataStreamAndPolicy() {
managedIndex = "index-" + randomAlphaOfLengthBetween(10, 15).toLowerCase(Locale.ROOT);
}

@Override
protected boolean ignoreExternalCluster() {
return true;
}

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(LocalStateCompositeXPackPlugin.class, IndexLifecycle.class);
Expand Down
Expand Up @@ -109,11 +109,6 @@ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
return nodeSettings.build();
}

@Override
protected boolean ignoreExternalCluster() {
return true;
}

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(LocalStateCompositeXPackPlugin.class, IndexLifecycle.class, TestILMPlugin.class);
Expand Down

0 comments on commit 4a37aef

Please sign in to comment.