From d7ef985a79ee18a5a567bfd50cc788ee6e0da457 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 15 Oct 2018 14:37:25 +0200 Subject: [PATCH 1/7] Disc: Move AbstractDisruptionTC to filebased D. * Relates #33675 --- .../discovery/AbstractDisruptionTestCase.java | 50 +++++++++---------- .../discovery/ClusterDisruptionIT.java | 6 +-- .../discovery/DiscoveryDisruptionIT.java | 4 +- .../discovery/SnapshotDisruptionIT.java | 2 +- .../test/InternalTestCluster.java | 46 ++++++++++++++--- .../ClusterDiscoveryConfiguration.java | 31 +----------- 6 files changed, 71 insertions(+), 68 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java b/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java index fa023882df55f..db31913ac72a7 100644 --- a/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java +++ b/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java @@ -19,6 +19,7 @@ package org.elasticsearch.discovery; +import java.nio.file.Path; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlock; import org.elasticsearch.cluster.block.ClusterBlockLevel; @@ -33,7 +34,7 @@ import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESIntegTestCase; -import org.elasticsearch.test.discovery.ClusterDiscoveryConfiguration; +import org.elasticsearch.test.NodeConfigurationSource; import org.elasticsearch.test.discovery.TestZenDiscovery; import org.elasticsearch.test.disruption.NetworkDisruption; import org.elasticsearch.test.disruption.NetworkDisruption.Bridge; @@ -52,9 +53,9 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import static org.elasticsearch.discovery.DiscoveryModule.DISCOVERY_HOSTS_PROVIDER_SETTING; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; @@ -62,7 +63,7 @@ public abstract class AbstractDisruptionTestCase extends ESIntegTestCase { static final TimeValue DISRUPTION_HEALING_OVERHEAD = TimeValue.timeValueSeconds(40); // we use 30s as timeout in many places. - private ClusterDiscoveryConfiguration discoveryConfig; + private NodeConfigurationSource discoveryConfig; @Override protected Settings nodeSettings(int nodeOrdinal) { @@ -116,18 +117,17 @@ protected void beforeIndexDeletion() throws Exception { } } - List startCluster(int numberOfNodes) throws ExecutionException, InterruptedException { + List startCluster(int numberOfNodes) { return startCluster(numberOfNodes, -1); } - List startCluster(int numberOfNodes, int minimumMasterNode) throws ExecutionException, InterruptedException { + List startCluster(int numberOfNodes, int minimumMasterNode) { return startCluster(numberOfNodes, minimumMasterNode, null); } - List startCluster(int numberOfNodes, int minimumMasterNode, @Nullable int[] unicastHostsOrdinals) throws - ExecutionException, InterruptedException { - configureCluster(numberOfNodes, unicastHostsOrdinals, minimumMasterNode); - List nodes = internalCluster().startNodes(numberOfNodes); + List startCluster(int numberOfNodes, int minimumMasterNode, @Nullable int[] unicastHostsOrdinals) { + configureCluster(numberOfNodes, minimumMasterNode); + List nodes = internalCluster().startNodes(numberOfNodes, unicastHostsOrdinals); ensureStableCluster(numberOfNodes); // TODO: this is a temporary solution so that nodes will not base their reaction to a partition based on previous successful results @@ -154,20 +154,11 @@ protected Collection> nodePlugins() { return Arrays.asList(MockTransportService.TestPlugin.class); } - void configureCluster( - int numberOfNodes, - @Nullable int[] unicastHostsOrdinals, - int minimumMasterNode - ) throws ExecutionException, InterruptedException { - configureCluster(DEFAULT_SETTINGS, numberOfNodes, unicastHostsOrdinals, minimumMasterNode); + void configureCluster(int numberOfNodes, int minimumMasterNode) { + configureCluster(DEFAULT_SETTINGS, numberOfNodes, minimumMasterNode); } - void configureCluster( - Settings settings, - int numberOfNodes, - @Nullable int[] unicastHostsOrdinals, - int minimumMasterNode - ) throws ExecutionException, InterruptedException { + void configureCluster(Settings settings, int numberOfNodes, int minimumMasterNode) { if (minimumMasterNode < 0) { minimumMasterNode = numberOfNodes / 2 + 1; } @@ -177,14 +168,21 @@ void configureCluster( .put(settings) .put(NodeEnvironment.MAX_LOCAL_STORAGE_NODES_SETTING.getKey(), numberOfNodes) .put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), minimumMasterNode) + .putList(DISCOVERY_HOSTS_PROVIDER_SETTING.getKey(), "file") .build(); if (discoveryConfig == null) { - if (unicastHostsOrdinals == null) { - discoveryConfig = new ClusterDiscoveryConfiguration.UnicastZen(numberOfNodes, nodeSettings); - } else { - discoveryConfig = new ClusterDiscoveryConfiguration.UnicastZen(numberOfNodes, nodeSettings, unicastHostsOrdinals); - } + discoveryConfig = new NodeConfigurationSource() { + @Override + public Settings nodeSettings(final int nodeOrdinal) { + return nodeSettings; + } + + @Override + public Path nodeConfigPath(final int nodeOrdinal) { + return null; + } + }; } } diff --git a/server/src/test/java/org/elasticsearch/discovery/ClusterDisruptionIT.java b/server/src/test/java/org/elasticsearch/discovery/ClusterDisruptionIT.java index 3b08eb6870e70..b35bf8444e95e 100644 --- a/server/src/test/java/org/elasticsearch/discovery/ClusterDisruptionIT.java +++ b/server/src/test/java/org/elasticsearch/discovery/ClusterDisruptionIT.java @@ -363,7 +363,7 @@ public void onFailure(Exception e) { */ public void testSearchWithRelocationAndSlowClusterStateProcessing() throws Exception { // don't use DEFAULT settings (which can cause node disconnects on a slow CI machine) - configureCluster(Settings.EMPTY, 3, null, 1); + configureCluster(Settings.EMPTY, 3, 1); internalCluster().startMasterOnlyNode(); final String node_1 = internalCluster().startDataOnlyNode(); @@ -390,7 +390,7 @@ public void testSearchWithRelocationAndSlowClusterStateProcessing() throws Excep public void testIndexImportedFromDataOnlyNodesIfMasterLostDataFolder() throws Exception { // test for https://github.com/elastic/elasticsearch/issues/8823 - configureCluster(2, null, 1); + configureCluster(2, 1); String masterNode = internalCluster().startMasterOnlyNode(Settings.EMPTY); internalCluster().startDataOnlyNode(Settings.EMPTY); @@ -421,7 +421,7 @@ public void testIndicesDeleted() throws Exception { .put(DiscoverySettings.COMMIT_TIMEOUT_SETTING.getKey(), "30s") // wait till cluster state is committed .build(); final String idxName = "test"; - configureCluster(settings, 3, null, 2); + configureCluster(settings, 3, 2); final List allMasterEligibleNodes = internalCluster().startMasterOnlyNodes(2); final String dataNode = internalCluster().startDataOnlyNode(); ensureStableCluster(3); diff --git a/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java b/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java index 4d190921f2da9..b92038e9ae695 100644 --- a/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java +++ b/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java @@ -194,7 +194,7 @@ public void testClusterJoinDespiteOfPublishingIssues() throws Exception { } public void testClusterFormingWithASlowNode() throws Exception { - configureCluster(3, null, 2); + configureCluster(3, 2); SlowClusterStateProcessing disruption = new SlowClusterStateProcessing(random(), 0, 0, 1000, 2000); @@ -210,7 +210,7 @@ public void testClusterFormingWithASlowNode() throws Exception { } public void testElectMasterWithLatestVersion() throws Exception { - configureCluster(3, null, 2); + configureCluster(3, 2); final Set nodes = new HashSet<>(internalCluster().startNodes(3)); ensureStableCluster(3); ServiceDisruptionScheme isolateAllNodes = diff --git a/server/src/test/java/org/elasticsearch/discovery/SnapshotDisruptionIT.java b/server/src/test/java/org/elasticsearch/discovery/SnapshotDisruptionIT.java index 3458cca0cf78e..4c9edf6e17eb1 100644 --- a/server/src/test/java/org/elasticsearch/discovery/SnapshotDisruptionIT.java +++ b/server/src/test/java/org/elasticsearch/discovery/SnapshotDisruptionIT.java @@ -59,7 +59,7 @@ public void testDisruptionOnSnapshotInitialization() throws Exception { .put(DiscoverySettings.COMMIT_TIMEOUT_SETTING.getKey(), "30s") // wait till cluster state is committed .build(); final String idxName = "test"; - configureCluster(settings, 4, null, 2); + configureCluster(settings, 4, 2); final List allMasterEligibleNodes = internalCluster().startMasterOnlyNodes(3); final String dataNode = internalCluster().startDataOnlyNode(); ensureStableCluster(4); diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index 794c7fef783ce..7463c65df1733 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -929,7 +929,7 @@ void restart(RestartCallback callback, boolean clearDataIfNeeded, int minMasterN if (!node.isClosed()) { closeNode(); } - recreateNodeOnRestart(callback, clearDataIfNeeded, minMasterNodes, () -> rebuildUnicastHostFiles(emptyList())); + recreateNodeOnRestart(callback, clearDataIfNeeded, minMasterNodes, () -> rebuildUnicastHostFiles(null, emptyList())); startNode(); } @@ -1091,7 +1091,7 @@ private synchronized void reset(boolean wipeData) throws IOException { final int numberOfMasterNodes = numSharedDedicatedMasterNodes > 0 ? numSharedDedicatedMasterNodes : numSharedDataNodes; final int defaultMinMasterNodes = (numberOfMasterNodes / 2) + 1; final List toStartAndPublish = new ArrayList<>(); // we want to start nodes in one go due to min master nodes - final Runnable onTransportServiceStarted = () -> rebuildUnicastHostFiles(toStartAndPublish); + final Runnable onTransportServiceStarted = () -> rebuildUnicastHostFiles(null, toStartAndPublish); for (int i = 0; i < numSharedDedicatedMasterNodes; i++) { final Settings.Builder settings = Settings.builder(); settings.put(Node.NODE_MASTER_SETTING.getKey(), true); @@ -1493,12 +1493,21 @@ private synchronized void startAndPublishNodesAndClients(List nod private final Object discoveryFileMutex = new Object(); - private void rebuildUnicastHostFiles(Collection newNodes) { + private void rebuildUnicastHostFiles(@Nullable int[] unicastHostOrdinals, List newNodes) { // cannot be a synchronized method since it's called on other threads from within synchronized startAndPublishNodesAndClients() synchronized (discoveryFileMutex) { try { - List discoveryFileContents = Stream.concat(nodes.values().stream(), newNodes.stream()) - .map(nac -> nac.node.injector().getInstance(TransportService.class)).filter(Objects::nonNull) + Stream unicastHosts = Stream.concat(nodes.values().stream(), newNodes.stream()); + if (unicastHostOrdinals != null) { + Set uniCastIds = new HashSet<>(); + for (int unicastHostOrdinal : unicastHostOrdinals) { + uniCastIds.add(unicastHostOrdinal); + } + unicastHosts = unicastHosts.filter(n -> uniCastIds.contains(n.nodeAndClientId())); + } + List discoveryFileContents = unicastHosts.map( + nac -> nac.node.injector().getInstance(TransportService.class) + ).filter(Objects::nonNull) .map(TransportService::getLocalNode).filter(Objects::nonNull).filter(DiscoveryNode::isMasterNode) .map(n -> n.getAddress().toString()) .distinct().collect(Collectors.toList()); @@ -1688,7 +1697,7 @@ public synchronized void fullRestart(RestartCallback callback) throws Exception logger.info("resetting node [{}] ", nodeAndClient.name); // we already cleared data folders, before starting nodes up nodeAndClient.recreateNodeOnRestart(callback, false, autoManageMinMasterNodes ? getMinMasterNodes(getMasterNodesCount()) : -1, - () -> rebuildUnicastHostFiles(startUpOrder)); + () -> rebuildUnicastHostFiles(null, startUpOrder)); } startAndPublishNodesAndClients(startUpOrder); @@ -1789,6 +1798,13 @@ public synchronized List startNodes(int numOfNodes) { return startNodes(numOfNodes, Settings.EMPTY); } + /** + * Starts multiple nodes with default settings and returns their names + */ + public synchronized List startNodes(int numOfNodes, int[] unicastHostOrdinals) { + return startNodes(numOfNodes, Settings.EMPTY, unicastHostOrdinals); + } + /** * Starts multiple nodes with the given settings and returns their names */ @@ -1796,10 +1812,26 @@ public synchronized List startNodes(int numOfNodes, Settings settings) { return startNodes(Collections.nCopies(numOfNodes, settings).stream().toArray(Settings[]::new)); } + /** + * Starts multiple nodes with the given settings and returns their names + */ + public synchronized List startNodes(int numOfNodes, Settings settings, int[] unicastHostOrdinals) { + return startNodes( + unicastHostOrdinals, Collections.nCopies(numOfNodes, settings).stream().toArray(Settings[]::new) + ); + } + /** * Starts multiple nodes with the given settings and returns their names */ public synchronized List startNodes(Settings... settings) { + return startNodes(null, settings); + } + + /** + * Starts multiple nodes with the given settings and returns their names + */ + public synchronized List startNodes( int[] unicastHostOrdinals, Settings... settings) { final int defaultMinMasterNodes; if (autoManageMinMasterNodes) { int mastersDelta = (int) Stream.of(settings).filter(Node.NODE_MASTER_SETTING::get).count(); @@ -1809,7 +1841,7 @@ public synchronized List startNodes(Settings... settings) { } final List nodes = new ArrayList<>(); for (Settings nodeSettings : settings) { - nodes.add(buildNode(nodeSettings, defaultMinMasterNodes, () -> rebuildUnicastHostFiles(nodes))); + nodes.add(buildNode(nodeSettings, defaultMinMasterNodes, () -> rebuildUnicastHostFiles(unicastHostOrdinals, nodes))); } startAndPublishNodesAndClients(nodes); if (autoManageMinMasterNodes) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java b/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java index a63ba22bb51f6..b1d619b85eba1 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java +++ b/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java @@ -18,13 +18,11 @@ */ package org.elasticsearch.test.discovery; -import com.carrotsearch.randomizedtesting.RandomizedTest; import com.carrotsearch.randomizedtesting.SysGlobals; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.network.NetworkUtils; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.mocksocket.MockServerSocket; import org.elasticsearch.test.NodeConfigurationSource; @@ -34,8 +32,6 @@ import java.net.InetSocketAddress; import java.net.ServerSocket; import java.nio.file.Path; -import java.util.HashSet; -import java.util.Set; public class ClusterDiscoveryConfiguration extends NodeConfigurationSource { @@ -89,38 +85,15 @@ public static class UnicastZen extends ClusterDiscoveryConfiguration { private final int[] unicastHostPorts; public UnicastZen(int numOfNodes, Settings extraSettings) { - this(numOfNodes, numOfNodes, extraSettings); - } - - public UnicastZen(int numOfNodes, int numOfUnicastHosts, Settings extraSettings) { super(numOfNodes, extraSettings); - if (numOfUnicastHosts == numOfNodes) { - unicastHostOrdinals = new int[numOfNodes]; - for (int i = 0; i < numOfNodes; i++) { + unicastHostOrdinals = new int[numOfNodes]; + for (int i = 0; i < numOfNodes; i++) { unicastHostOrdinals[i] = i; - } - } else { - Set ordinals = new HashSet<>(numOfUnicastHosts); - while (ordinals.size() != numOfUnicastHosts) { - ordinals.add(RandomizedTest.randomInt(numOfNodes - 1)); - } - unicastHostOrdinals = CollectionUtils.toArray(ordinals); } this.unicastHostPorts = unicastHostPorts(numOfNodes); assert unicastHostOrdinals.length <= unicastHostPorts.length; } - public UnicastZen(int numOfNodes, int[] unicastHostOrdinals) { - this(numOfNodes, Settings.EMPTY, unicastHostOrdinals); - } - - public UnicastZen(int numOfNodes, Settings extraSettings, int[] unicastHostOrdinals) { - super(numOfNodes, extraSettings); - this.unicastHostOrdinals = unicastHostOrdinals; - this.unicastHostPorts = unicastHostPorts(numOfNodes); - assert unicastHostOrdinals.length <= unicastHostPorts.length; - } - private static int calcBasePort() { return 30000 + JVM_BASE_PORT_OFFSET; } From 427b7c3768175b8c59e38a1fd73ffe66295f009a Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 15 Oct 2018 17:36:14 +0200 Subject: [PATCH 2/7] CR: Pass boolean instead of ordinal list --- .../discovery/AbstractDisruptionTestCase.java | 6 ++-- .../discovery/DiscoveryDisruptionIT.java | 4 +-- .../test/InternalTestCluster.java | 30 ++++++++----------- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java b/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java index db31913ac72a7..f78b2ec495183 100644 --- a/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java +++ b/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java @@ -122,12 +122,12 @@ List startCluster(int numberOfNodes) { } List startCluster(int numberOfNodes, int minimumMasterNode) { - return startCluster(numberOfNodes, minimumMasterNode, null); + return startCluster(numberOfNodes, minimumMasterNode, false); } - List startCluster(int numberOfNodes, int minimumMasterNode, @Nullable int[] unicastHostsOrdinals) { + List startCluster(int numberOfNodes, int minimumMasterNode, boolean hostsListContainsOnlyFirstNode) { configureCluster(numberOfNodes, minimumMasterNode); - List nodes = internalCluster().startNodes(numberOfNodes, unicastHostsOrdinals); + List nodes = internalCluster().startNodes(numberOfNodes, hostsListContainsOnlyFirstNode); ensureStableCluster(numberOfNodes); // TODO: this is a temporary solution so that nodes will not base their reaction to a partition based on previous successful results diff --git a/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java b/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java index b92038e9ae695..716a4f600c24b 100644 --- a/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java +++ b/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java @@ -59,7 +59,7 @@ public class DiscoveryDisruptionIT extends AbstractDisruptionTestCase { public void testIsolatedUnicastNodes() throws Exception { - List nodes = startCluster(4, -1, new int[]{0}); + List nodes = startCluster(4, -1, true); // Figure out what is the elected master node final String unicastTarget = nodes.get(0); @@ -98,7 +98,7 @@ public void testIsolatedUnicastNodes() throws Exception { * The rejoining node should take this master node and connect. */ public void testUnicastSinglePingResponseContainsMaster() throws Exception { - List nodes = startCluster(4, -1, new int[]{0}); + List nodes = startCluster(4, -1, true); // Figure out what is the elected master node final String masterNode = internalCluster().getMasterName(); logger.info("---> legit elected master node={}", masterNode); diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index 7463c65df1733..a8d4f57d09819 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -929,7 +929,7 @@ void restart(RestartCallback callback, boolean clearDataIfNeeded, int minMasterN if (!node.isClosed()) { closeNode(); } - recreateNodeOnRestart(callback, clearDataIfNeeded, minMasterNodes, () -> rebuildUnicastHostFiles(null, emptyList())); + recreateNodeOnRestart(callback, clearDataIfNeeded, minMasterNodes, () -> rebuildUnicastHostFiles(false, emptyList())); startNode(); } @@ -1091,7 +1091,7 @@ private synchronized void reset(boolean wipeData) throws IOException { final int numberOfMasterNodes = numSharedDedicatedMasterNodes > 0 ? numSharedDedicatedMasterNodes : numSharedDataNodes; final int defaultMinMasterNodes = (numberOfMasterNodes / 2) + 1; final List toStartAndPublish = new ArrayList<>(); // we want to start nodes in one go due to min master nodes - final Runnable onTransportServiceStarted = () -> rebuildUnicastHostFiles(null, toStartAndPublish); + final Runnable onTransportServiceStarted = () -> rebuildUnicastHostFiles(false, toStartAndPublish); for (int i = 0; i < numSharedDedicatedMasterNodes; i++) { final Settings.Builder settings = Settings.builder(); settings.put(Node.NODE_MASTER_SETTING.getKey(), true); @@ -1493,17 +1493,13 @@ private synchronized void startAndPublishNodesAndClients(List nod private final Object discoveryFileMutex = new Object(); - private void rebuildUnicastHostFiles(@Nullable int[] unicastHostOrdinals, List newNodes) { + private void rebuildUnicastHostFiles(boolean hostsListContainsOnlyFirstNode, List newNodes) { // cannot be a synchronized method since it's called on other threads from within synchronized startAndPublishNodesAndClients() synchronized (discoveryFileMutex) { try { Stream unicastHosts = Stream.concat(nodes.values().stream(), newNodes.stream()); - if (unicastHostOrdinals != null) { - Set uniCastIds = new HashSet<>(); - for (int unicastHostOrdinal : unicastHostOrdinals) { - uniCastIds.add(unicastHostOrdinal); - } - unicastHosts = unicastHosts.filter(n -> uniCastIds.contains(n.nodeAndClientId())); + if (hostsListContainsOnlyFirstNode) { + unicastHosts = unicastHosts.limit(1L); } List discoveryFileContents = unicastHosts.map( nac -> nac.node.injector().getInstance(TransportService.class) @@ -1697,7 +1693,7 @@ public synchronized void fullRestart(RestartCallback callback) throws Exception logger.info("resetting node [{}] ", nodeAndClient.name); // we already cleared data folders, before starting nodes up nodeAndClient.recreateNodeOnRestart(callback, false, autoManageMinMasterNodes ? getMinMasterNodes(getMasterNodesCount()) : -1, - () -> rebuildUnicastHostFiles(null, startUpOrder)); + () -> rebuildUnicastHostFiles(false, startUpOrder)); } startAndPublishNodesAndClients(startUpOrder); @@ -1801,8 +1797,8 @@ public synchronized List startNodes(int numOfNodes) { /** * Starts multiple nodes with default settings and returns their names */ - public synchronized List startNodes(int numOfNodes, int[] unicastHostOrdinals) { - return startNodes(numOfNodes, Settings.EMPTY, unicastHostOrdinals); + public synchronized List startNodes(int numOfNodes, boolean hostsListContainsOnlyFirstNode) { + return startNodes(numOfNodes, Settings.EMPTY, hostsListContainsOnlyFirstNode); } /** @@ -1815,9 +1811,9 @@ public synchronized List startNodes(int numOfNodes, Settings settings) { /** * Starts multiple nodes with the given settings and returns their names */ - public synchronized List startNodes(int numOfNodes, Settings settings, int[] unicastHostOrdinals) { + public synchronized List startNodes(int numOfNodes, Settings settings, boolean hostsListContainsOnlyFirstNode) { return startNodes( - unicastHostOrdinals, Collections.nCopies(numOfNodes, settings).stream().toArray(Settings[]::new) + hostsListContainsOnlyFirstNode, Collections.nCopies(numOfNodes, settings).stream().toArray(Settings[]::new) ); } @@ -1825,13 +1821,13 @@ public synchronized List startNodes(int numOfNodes, Settings settings, i * Starts multiple nodes with the given settings and returns their names */ public synchronized List startNodes(Settings... settings) { - return startNodes(null, settings); + return startNodes(false, settings); } /** * Starts multiple nodes with the given settings and returns their names */ - public synchronized List startNodes( int[] unicastHostOrdinals, Settings... settings) { + public synchronized List startNodes(boolean hostsListContainsOnlyFirstNode, Settings... settings) { final int defaultMinMasterNodes; if (autoManageMinMasterNodes) { int mastersDelta = (int) Stream.of(settings).filter(Node.NODE_MASTER_SETTING::get).count(); @@ -1841,7 +1837,7 @@ public synchronized List startNodes( int[] unicastHostOrdinals, Settings } final List nodes = new ArrayList<>(); for (Settings nodeSettings : settings) { - nodes.add(buildNode(nodeSettings, defaultMinMasterNodes, () -> rebuildUnicastHostFiles(unicastHostOrdinals, nodes))); + nodes.add(buildNode(nodeSettings, defaultMinMasterNodes, () -> rebuildUnicastHostFiles(hostsListContainsOnlyFirstNode, nodes))); } startAndPublishNodesAndClients(nodes); if (autoManageMinMasterNodes) { From 72f34e58710539f14f1db7e2fa018151d66a6fd7 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 15 Oct 2018 18:35:46 +0200 Subject: [PATCH 3/7] CR: hostsListContainsOnlyFirstNode is a field in InternalTestCluster now, ClusterDiscoveryConfiguration doesn't do port assignments anymore --- .../discovery/AbstractDisruptionTestCase.java | 5 ++- .../test/InternalTestCluster.java | 39 ++++++------------- .../ClusterDiscoveryConfiguration.java | 15 ------- 3 files changed, 15 insertions(+), 44 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java b/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java index f78b2ec495183..ae22efa9a2951 100644 --- a/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java +++ b/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java @@ -34,6 +34,7 @@ import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.InternalTestCluster; import org.elasticsearch.test.NodeConfigurationSource; import org.elasticsearch.test.discovery.TestZenDiscovery; import org.elasticsearch.test.disruption.NetworkDisruption; @@ -127,7 +128,9 @@ List startCluster(int numberOfNodes, int minimumMasterNode) { List startCluster(int numberOfNodes, int minimumMasterNode, boolean hostsListContainsOnlyFirstNode) { configureCluster(numberOfNodes, minimumMasterNode); - List nodes = internalCluster().startNodes(numberOfNodes, hostsListContainsOnlyFirstNode); + InternalTestCluster internalCluster = internalCluster(); + internalCluster.setHostsListContainsOnlyFirstNode(hostsListContainsOnlyFirstNode); + List nodes = internalCluster.startNodes(numberOfNodes); ensureStableCluster(numberOfNodes); // TODO: this is a temporary solution so that nodes will not base their reaction to a partition based on previous successful results diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index a8d4f57d09819..dabd4e7479de8 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -221,6 +221,9 @@ public final class InternalTestCluster extends TestCluster { private ServiceDisruptionScheme activeDisruptionScheme; private Function clientWrapper; + // If set to tru only the first node in the cluster will be made a unicast node + private boolean hostsListContainsOnlyFirstNode; + public InternalTestCluster( final long clusterSeed, final Path baseDir, @@ -929,7 +932,7 @@ void restart(RestartCallback callback, boolean clearDataIfNeeded, int minMasterN if (!node.isClosed()) { closeNode(); } - recreateNodeOnRestart(callback, clearDataIfNeeded, minMasterNodes, () -> rebuildUnicastHostFiles(false, emptyList())); + recreateNodeOnRestart(callback, clearDataIfNeeded, minMasterNodes, () -> rebuildUnicastHostFiles(emptyList())); startNode(); } @@ -1091,7 +1094,7 @@ private synchronized void reset(boolean wipeData) throws IOException { final int numberOfMasterNodes = numSharedDedicatedMasterNodes > 0 ? numSharedDedicatedMasterNodes : numSharedDataNodes; final int defaultMinMasterNodes = (numberOfMasterNodes / 2) + 1; final List toStartAndPublish = new ArrayList<>(); // we want to start nodes in one go due to min master nodes - final Runnable onTransportServiceStarted = () -> rebuildUnicastHostFiles(false, toStartAndPublish); + final Runnable onTransportServiceStarted = () -> rebuildUnicastHostFiles(toStartAndPublish); for (int i = 0; i < numSharedDedicatedMasterNodes; i++) { final Settings.Builder settings = Settings.builder(); settings.put(Node.NODE_MASTER_SETTING.getKey(), true); @@ -1493,7 +1496,7 @@ private synchronized void startAndPublishNodesAndClients(List nod private final Object discoveryFileMutex = new Object(); - private void rebuildUnicastHostFiles(boolean hostsListContainsOnlyFirstNode, List newNodes) { + private void rebuildUnicastHostFiles(List newNodes) { // cannot be a synchronized method since it's called on other threads from within synchronized startAndPublishNodesAndClients() synchronized (discoveryFileMutex) { try { @@ -1693,7 +1696,7 @@ public synchronized void fullRestart(RestartCallback callback) throws Exception logger.info("resetting node [{}] ", nodeAndClient.name); // we already cleared data folders, before starting nodes up nodeAndClient.recreateNodeOnRestart(callback, false, autoManageMinMasterNodes ? getMinMasterNodes(getMasterNodesCount()) : -1, - () -> rebuildUnicastHostFiles(false, startUpOrder)); + () -> rebuildUnicastHostFiles(startUpOrder)); } startAndPublishNodesAndClients(startUpOrder); @@ -1794,13 +1797,6 @@ public synchronized List startNodes(int numOfNodes) { return startNodes(numOfNodes, Settings.EMPTY); } - /** - * Starts multiple nodes with default settings and returns their names - */ - public synchronized List startNodes(int numOfNodes, boolean hostsListContainsOnlyFirstNode) { - return startNodes(numOfNodes, Settings.EMPTY, hostsListContainsOnlyFirstNode); - } - /** * Starts multiple nodes with the given settings and returns their names */ @@ -1808,26 +1804,10 @@ public synchronized List startNodes(int numOfNodes, Settings settings) { return startNodes(Collections.nCopies(numOfNodes, settings).stream().toArray(Settings[]::new)); } - /** - * Starts multiple nodes with the given settings and returns their names - */ - public synchronized List startNodes(int numOfNodes, Settings settings, boolean hostsListContainsOnlyFirstNode) { - return startNodes( - hostsListContainsOnlyFirstNode, Collections.nCopies(numOfNodes, settings).stream().toArray(Settings[]::new) - ); - } - /** * Starts multiple nodes with the given settings and returns their names */ public synchronized List startNodes(Settings... settings) { - return startNodes(false, settings); - } - - /** - * Starts multiple nodes with the given settings and returns their names - */ - public synchronized List startNodes(boolean hostsListContainsOnlyFirstNode, Settings... settings) { final int defaultMinMasterNodes; if (autoManageMinMasterNodes) { int mastersDelta = (int) Stream.of(settings).filter(Node.NODE_MASTER_SETTING::get).count(); @@ -1837,7 +1817,7 @@ public synchronized List startNodes(boolean hostsListContainsOnlyFirstNo } final List nodes = new ArrayList<>(); for (Settings nodeSettings : settings) { - nodes.add(buildNode(nodeSettings, defaultMinMasterNodes, () -> rebuildUnicastHostFiles(hostsListContainsOnlyFirstNode, nodes))); + nodes.add(buildNode(nodeSettings, defaultMinMasterNodes, () -> rebuildUnicastHostFiles(nodes))); } startAndPublishNodesAndClients(nodes); if (autoManageMinMasterNodes) { @@ -1954,6 +1934,9 @@ public synchronized int numMasterNodes() { return filterNodes(nodes, NodeAndClient::isMasterEligible).size(); } + public void setHostsListContainsOnlyFirstNode(boolean hostsListContainsOnlyFirstNode) { + this.hostsListContainsOnlyFirstNode = hostsListContainsOnlyFirstNode; + } public void setDisruptionScheme(ServiceDisruptionScheme scheme) { assert activeDisruptionScheme == null : diff --git a/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java b/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java index b1d619b85eba1..5955873ac4d3a 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java +++ b/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java @@ -26,7 +26,6 @@ import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.mocksocket.MockServerSocket; import org.elasticsearch.test.NodeConfigurationSource; -import org.elasticsearch.transport.TcpTransport; import java.io.IOException; import java.net.InetSocketAddress; @@ -101,20 +100,6 @@ private static int calcBasePort() { @Override public Settings nodeSettings(int nodeOrdinal) { Settings.Builder builder = Settings.builder().put(NodeEnvironment.MAX_LOCAL_STORAGE_NODES_SETTING.getKey(), numOfNodes); - - String[] unicastHosts = new String[unicastHostOrdinals.length]; - if (nodeOrdinal >= unicastHostPorts.length) { - throw new ElasticsearchException("nodeOrdinal [" + nodeOrdinal + "] is greater than the number unicast ports [" - + unicastHostPorts.length + "]"); - } else { - // we need to pin the node port & host so we'd know where to point things - builder.put(TcpTransport.PORT.getKey(), unicastHostPorts[nodeOrdinal]); - builder.put(TcpTransport.HOST.getKey(), IP_ADDR); // only bind on one IF we use v4 here by default - for (int i = 0; i < unicastHostOrdinals.length; i++) { - unicastHosts[i] = IP_ADDR + ":" + (unicastHostPorts[unicastHostOrdinals[i]]); - } - } - builder.putList("discovery.zen.ping.unicast.hosts", unicastHosts); return builder.put(super.nodeSettings(nodeOrdinal)).build(); } From 199c41eacea4c16c649f5c5f821b4f4d371d81f2 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 16 Oct 2018 08:21:23 +0200 Subject: [PATCH 4/7] CR: Remove now dead code --- .../test/InternalTestCluster.java | 2 +- .../ClusterDiscoveryConfiguration.java | 74 ------------------- 2 files changed, 1 insertion(+), 75 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index dabd4e7479de8..7de0e7d97c25b 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -221,7 +221,7 @@ public final class InternalTestCluster extends TestCluster { private ServiceDisruptionScheme activeDisruptionScheme; private Function clientWrapper; - // If set to tru only the first node in the cluster will be made a unicast node + // If set to true only the first node in the cluster will be made a unicast node private boolean hostsListContainsOnlyFirstNode; public InternalTestCluster( diff --git a/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java b/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java index 5955873ac4d3a..1ff3e0015dbec 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java +++ b/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java @@ -18,37 +18,15 @@ */ package org.elasticsearch.test.discovery; -import com.carrotsearch.randomizedtesting.SysGlobals; -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.network.NetworkUtils; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.NodeEnvironment; -import org.elasticsearch.mocksocket.MockServerSocket; import org.elasticsearch.test.NodeConfigurationSource; -import java.io.IOException; -import java.net.InetSocketAddress; -import java.net.ServerSocket; import java.nio.file.Path; public class ClusterDiscoveryConfiguration extends NodeConfigurationSource { - /** - * The number of ports in the range used for this JVM - */ - private static final int PORTS_PER_JVM = 100; - - private static final int JVM_ORDINAL = Integer.parseInt(System.getProperty(SysGlobals.CHILDVM_SYSPROP_JVM_ID, "0")); - - /** - * a per-JVM unique offset to be used for calculating unique port ranges. - */ - private static final int JVM_BASE_PORT_OFFSET = PORTS_PER_JVM * (JVM_ORDINAL + 1); - - static Settings DEFAULT_NODE_SETTINGS = Settings.EMPTY; - private static final String IP_ADDR = "127.0.0.1"; final int numOfNodes; final Settings nodeSettings; @@ -77,24 +55,8 @@ public Settings transportClientSettings() { public static class UnicastZen extends ClusterDiscoveryConfiguration { - // this variable is incremented on each bind attempt and will maintain the next port that should be tried - private static int nextPort = calcBasePort(); - - private final int[] unicastHostOrdinals; - private final int[] unicastHostPorts; - public UnicastZen(int numOfNodes, Settings extraSettings) { super(numOfNodes, extraSettings); - unicastHostOrdinals = new int[numOfNodes]; - for (int i = 0; i < numOfNodes; i++) { - unicastHostOrdinals[i] = i; - } - this.unicastHostPorts = unicastHostPorts(numOfNodes); - assert unicastHostOrdinals.length <= unicastHostPorts.length; - } - - private static int calcBasePort() { - return 30000 + JVM_BASE_PORT_OFFSET; } @Override @@ -103,41 +65,5 @@ public Settings nodeSettings(int nodeOrdinal) { return builder.put(super.nodeSettings(nodeOrdinal)).build(); } - @SuppressForbidden(reason = "we know we pass a IP address") - protected static synchronized int[] unicastHostPorts(int numHosts) { - int[] unicastHostPorts = new int[numHosts]; - - final int basePort = calcBasePort(); - final int maxPort = basePort + PORTS_PER_JVM; - int tries = 0; - for (int i = 0; i < unicastHostPorts.length; i++) { - boolean foundPortInRange = false; - while (tries < PORTS_PER_JVM && !foundPortInRange) { - try (ServerSocket serverSocket = new MockServerSocket()) { - // Set SO_REUSEADDR as we may bind here and not be able to reuse the address immediately without it. - serverSocket.setReuseAddress(NetworkUtils.defaultReuseAddress()); - serverSocket.bind(new InetSocketAddress(IP_ADDR, nextPort)); - // bind was a success - foundPortInRange = true; - unicastHostPorts[i] = nextPort; - } catch (IOException e) { - // Do nothing - } - - nextPort++; - if (nextPort >= maxPort) { - // Roll back to the beginning of the range and do not go into another JVM's port range - nextPort = basePort; - } - tries++; - } - - if (!foundPortInRange) { - throw new ElasticsearchException("could not find enough open ports in range [" + basePort + "-" + maxPort - + "]. required [" + unicastHostPorts.length + "] ports"); - } - } - return unicastHostPorts; - } } } From 8fe964de36e2f1dfa573257789b1e71c91aa8b7a Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 16 Oct 2018 08:49:25 +0200 Subject: [PATCH 5/7] Simplify away ClusterDiscoveryConfiguration --- .../discovery/AbstractDisruptionTestCase.java | 5 -- .../discovery/DiscoveryDisruptionIT.java | 6 +- .../ClusterDiscoveryConfiguration.java | 69 ------------------- .../test/SecuritySettingsSource.java | 16 +++-- 4 files changed, 13 insertions(+), 83 deletions(-) delete mode 100644 test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java diff --git a/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java b/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java index ae22efa9a2951..8dfc6525e9057 100644 --- a/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java +++ b/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java @@ -123,13 +123,8 @@ List startCluster(int numberOfNodes) { } List startCluster(int numberOfNodes, int minimumMasterNode) { - return startCluster(numberOfNodes, minimumMasterNode, false); - } - - List startCluster(int numberOfNodes, int minimumMasterNode, boolean hostsListContainsOnlyFirstNode) { configureCluster(numberOfNodes, minimumMasterNode); InternalTestCluster internalCluster = internalCluster(); - internalCluster.setHostsListContainsOnlyFirstNode(hostsListContainsOnlyFirstNode); List nodes = internalCluster.startNodes(numberOfNodes); ensureStableCluster(numberOfNodes); diff --git a/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java b/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java index 716a4f600c24b..1a20f7594460e 100644 --- a/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java +++ b/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java @@ -59,7 +59,8 @@ public class DiscoveryDisruptionIT extends AbstractDisruptionTestCase { public void testIsolatedUnicastNodes() throws Exception { - List nodes = startCluster(4, -1, true); + List nodes = startCluster(4, -1); + internalCluster().setHostsListContainsOnlyFirstNode(true); // Figure out what is the elected master node final String unicastTarget = nodes.get(0); @@ -98,7 +99,8 @@ public void testIsolatedUnicastNodes() throws Exception { * The rejoining node should take this master node and connect. */ public void testUnicastSinglePingResponseContainsMaster() throws Exception { - List nodes = startCluster(4, -1, true); + List nodes = startCluster(4, -1); + internalCluster().setHostsListContainsOnlyFirstNode(true); // Figure out what is the elected master node final String masterNode = internalCluster().getMasterName(); logger.info("---> legit elected master node={}", masterNode); diff --git a/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java b/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java deleted file mode 100644 index 1ff3e0015dbec..0000000000000 --- a/test/framework/src/main/java/org/elasticsearch/test/discovery/ClusterDiscoveryConfiguration.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * 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.test.discovery; - -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.env.NodeEnvironment; -import org.elasticsearch.test.NodeConfigurationSource; - -import java.nio.file.Path; - -public class ClusterDiscoveryConfiguration extends NodeConfigurationSource { - - static Settings DEFAULT_NODE_SETTINGS = Settings.EMPTY; - - final int numOfNodes; - final Settings nodeSettings; - final Settings transportClientSettings; - - public ClusterDiscoveryConfiguration(int numOfNodes, Settings extraSettings) { - this.numOfNodes = numOfNodes; - this.nodeSettings = Settings.builder().put(DEFAULT_NODE_SETTINGS).put(extraSettings).build(); - this.transportClientSettings = Settings.builder().put(extraSettings).build(); - } - - @Override - public Settings nodeSettings(int nodeOrdinal) { - return nodeSettings; - } - - @Override - public Path nodeConfigPath(int nodeOrdinal) { - return null; - } - - @Override - public Settings transportClientSettings() { - return transportClientSettings; - } - - public static class UnicastZen extends ClusterDiscoveryConfiguration { - - public UnicastZen(int numOfNodes, Settings extraSettings) { - super(numOfNodes, extraSettings); - } - - @Override - public Settings nodeSettings(int nodeOrdinal) { - Settings.Builder builder = Settings.builder().put(NodeEnvironment.MAX_LOCAL_STORAGE_NODES_SETTING.getKey(), numOfNodes); - return builder.put(super.nodeSettings(nodeOrdinal)).build(); - } - - } -} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java index 76482c5fa92a7..1e4e7d529297f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java @@ -17,7 +17,6 @@ import org.elasticsearch.index.reindex.ReindexPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESIntegTestCase.Scope; -import org.elasticsearch.test.discovery.ClusterDiscoveryConfiguration; import org.elasticsearch.transport.Netty4Plugin; import org.elasticsearch.xpack.core.XPackClientPlugin; import org.elasticsearch.xpack.core.XPackSettings; @@ -49,11 +48,10 @@ /** * {@link org.elasticsearch.test.NodeConfigurationSource} subclass that allows to set all needed settings for x-pack security. - * Unicast discovery is configured through {@link org.elasticsearch.test.discovery.ClusterDiscoveryConfiguration.UnicastZen}, - * also x-pack is installed with all the needed configuration and files. + * X-pack is installed with all the needed configuration and files. * To avoid conflicts, every cluster should have its own instance of this class as some configuration files need to be created. */ -public class SecuritySettingsSource extends ClusterDiscoveryConfiguration.UnicastZen { +public class SecuritySettingsSource extends NodeConfigurationSource { public static final Settings DEFAULT_SETTINGS = Settings.EMPTY; @@ -84,6 +82,9 @@ public class SecuritySettingsSource extends ClusterDiscoveryConfiguration.Unicas " - names: '*'\n" + " privileges: [ ALL ]\n"; + final Settings nodeSettings; + final Settings transportClientSettings; + private final Path parentFolder; private final String subfolderPrefix; private final boolean sslEnabled; @@ -99,7 +100,8 @@ public class SecuritySettingsSource extends ClusterDiscoveryConfiguration.Unicas * @param scope the scope of the test that is requiring an instance of SecuritySettingsSource */ public SecuritySettingsSource(int numOfNodes, boolean sslEnabled, Path parentFolder, Scope scope) { - super(numOfNodes, DEFAULT_SETTINGS); + this.nodeSettings = Settings.builder().put(Settings.EMPTY).put(DEFAULT_SETTINGS).build(); + this.transportClientSettings = Settings.builder().put(DEFAULT_SETTINGS).build(); this.parentFolder = parentFolder; this.subfolderPrefix = scope.name(); this.sslEnabled = sslEnabled; @@ -129,7 +131,7 @@ public Settings nodeSettings(int nodeOrdinal) { writeFile(xpackConf, "users", configUsers()); writeFile(xpackConf, "users_roles", configUsersRoles()); - Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)) + Settings.Builder builder = Settings.builder().put(nodeSettings) .put(XPackSettings.SECURITY_ENABLED.getKey(), true) .put(NetworkModule.TRANSPORT_TYPE_KEY, randomBoolean() ? SecurityField.NAME4 : SecurityField.NIO) .put(NetworkModule.HTTP_TYPE_KEY, randomBoolean() ? SecurityField.NAME4 : SecurityField.NIO) @@ -156,7 +158,7 @@ public Path nodeConfigPath(int nodeOrdinal) { @Override public Settings transportClientSettings() { - Settings superSettings = super.transportClientSettings(); + Settings superSettings = transportClientSettings; Settings.Builder builder = Settings.builder().put(superSettings); addClientSSLSettings(builder, ""); addDefaultSecurityTransportType(builder, superSettings); From 7b4072d1040299aaf1188b1f46cf7286013d4f6f Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 16 Oct 2018 10:56:09 +0200 Subject: [PATCH 6/7] CR: Remove noop settings actions in tests --- .../elasticsearch/test/SecurityIntegTestCase.java | 4 ++-- .../elasticsearch/test/SecuritySettingsSource.java | 14 +++----------- .../test/SecuritySingleNodeTestCase.java | 4 ++-- .../security/audit/index/IndexAuditTrailTests.java | 2 +- .../index/RemoteIndexAuditTrailStartingTests.java | 2 +- 5 files changed, 9 insertions(+), 17 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java index 7143182c1621a..c3e3bddf10e97 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java @@ -152,7 +152,7 @@ private static Scope getCurrentClusterScope(Class clazz) { public static void initDefaultSettings() { if (SECURITY_DEFAULT_SETTINGS == null) { SECURITY_DEFAULT_SETTINGS = - new SecuritySettingsSource(defaultMaxNumberOfNodes(), randomBoolean(), createTempDir(), Scope.SUITE); + new SecuritySettingsSource(randomBoolean(), createTempDir(), Scope.SUITE); } } @@ -367,7 +367,7 @@ protected int maxNumberOfNodes() { private class CustomSecuritySettingsSource extends SecuritySettingsSource { private CustomSecuritySettingsSource(boolean sslEnabled, Path configDir, Scope scope) { - super(maxNumberOfNodes(), sslEnabled, configDir, scope); + super(sslEnabled, configDir, scope); } @Override diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java index 1e4e7d529297f..7ed793031ee85 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java @@ -53,8 +53,6 @@ */ public class SecuritySettingsSource extends NodeConfigurationSource { - public static final Settings DEFAULT_SETTINGS = Settings.EMPTY; - public static final String TEST_USER_NAME = "test_user"; public static final String TEST_PASSWORD_HASHED = new String(Hasher.resolve(randomFrom("pbkdf2", "pbkdf2_1000", "bcrypt9", "bcrypt8", "bcrypt")). @@ -82,9 +80,6 @@ public class SecuritySettingsSource extends NodeConfigurationSource { " - names: '*'\n" + " privileges: [ ALL ]\n"; - final Settings nodeSettings; - final Settings transportClientSettings; - private final Path parentFolder; private final String subfolderPrefix; private final boolean sslEnabled; @@ -94,14 +89,11 @@ public class SecuritySettingsSource extends NodeConfigurationSource { /** * Creates a new {@link org.elasticsearch.test.NodeConfigurationSource} for the security configuration. * - * @param numOfNodes the number of nodes for proper unicast configuration (can be more than actually available) * @param sslEnabled whether ssl is enabled * @param parentFolder the parent folder that will contain all of the configuration files that need to be created * @param scope the scope of the test that is requiring an instance of SecuritySettingsSource */ - public SecuritySettingsSource(int numOfNodes, boolean sslEnabled, Path parentFolder, Scope scope) { - this.nodeSettings = Settings.builder().put(Settings.EMPTY).put(DEFAULT_SETTINGS).build(); - this.transportClientSettings = Settings.builder().put(DEFAULT_SETTINGS).build(); + public SecuritySettingsSource(boolean sslEnabled, Path parentFolder, Scope scope) { this.parentFolder = parentFolder; this.subfolderPrefix = scope.name(); this.sslEnabled = sslEnabled; @@ -131,7 +123,7 @@ public Settings nodeSettings(int nodeOrdinal) { writeFile(xpackConf, "users", configUsers()); writeFile(xpackConf, "users_roles", configUsersRoles()); - Settings.Builder builder = Settings.builder().put(nodeSettings) + Settings.Builder builder = Settings.builder() .put(XPackSettings.SECURITY_ENABLED.getKey(), true) .put(NetworkModule.TRANSPORT_TYPE_KEY, randomBoolean() ? SecurityField.NAME4 : SecurityField.NIO) .put(NetworkModule.HTTP_TYPE_KEY, randomBoolean() ? SecurityField.NAME4 : SecurityField.NIO) @@ -158,7 +150,7 @@ public Path nodeConfigPath(int nodeOrdinal) { @Override public Settings transportClientSettings() { - Settings superSettings = transportClientSettings; + Settings superSettings = Settings.EMPTY; Settings.Builder builder = Settings.builder().put(superSettings); addClientSSLSettings(builder, ""); addDefaultSecurityTransportType(builder, superSettings); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySingleNodeTestCase.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySingleNodeTestCase.java index cda627806e7b5..e555bfdb3d335 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySingleNodeTestCase.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySingleNodeTestCase.java @@ -65,7 +65,7 @@ public static void generateBootstrapPassword() { public static void initDefaultSettings() { if (SECURITY_DEFAULT_SETTINGS == null) { SECURITY_DEFAULT_SETTINGS = - new SecuritySettingsSource(1, randomBoolean(), createTempDir(), ESIntegTestCase.Scope.SUITE); + new SecuritySettingsSource(randomBoolean(), createTempDir(), ESIntegTestCase.Scope.SUITE); } } @@ -235,7 +235,7 @@ protected boolean transportSSLEnabled() { private class CustomSecuritySettingsSource extends SecuritySettingsSource { private CustomSecuritySettingsSource(boolean sslEnabled, Path configDir, ESIntegTestCase.Scope scope) { - super(1, sslEnabled, configDir, scope); + super(sslEnabled, configDir, scope); } @Override diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailTests.java index cb1b69708bdf2..cf19af9c5ec07 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailTests.java @@ -178,7 +178,7 @@ public void initializeRemoteClusterIfNecessary() throws Exception { logger.info("--> remote indexing enabled. security enabled: [{}], SSL enabled: [{}], nodes: [{}]", useSecurity, useSSL, numNodes); SecuritySettingsSource cluster2SettingsSource = - new SecuritySettingsSource(numNodes, useSSL, createTempDir(), Scope.SUITE) { + new SecuritySettingsSource(useSSL, createTempDir(), Scope.SUITE) { @Override public Settings nodeSettings(int nodeOrdinal) { Settings.Builder builder = Settings.builder() diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/index/RemoteIndexAuditTrailStartingTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/index/RemoteIndexAuditTrailStartingTests.java index 96bba962237fe..ba62e5b52c40e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/index/RemoteIndexAuditTrailStartingTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/index/RemoteIndexAuditTrailStartingTests.java @@ -95,7 +95,7 @@ public void startRemoteCluster() throws IOException, InterruptedException { // Setup a second test cluster with a single node, security enabled, and SSL final int numNodes = 1; SecuritySettingsSource cluster2SettingsSource = - new SecuritySettingsSource(numNodes, sslEnabled, createTempDir(), Scope.TEST) { + new SecuritySettingsSource(sslEnabled, createTempDir(), Scope.TEST) { @Override public Settings nodeSettings(int nodeOrdinal) { Settings.Builder builder = Settings.builder() From 5503164a5dfe1a41768339860898443bb21d7c17 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 16 Oct 2018 13:22:27 +0200 Subject: [PATCH 7/7] CR: Fix call order + remove Settings.EMPTY noop --- .../org/elasticsearch/discovery/DiscoveryDisruptionIT.java | 4 ++-- .../java/org/elasticsearch/test/SecuritySettingsSource.java | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java b/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java index 1a20f7594460e..2c7f17468ac3a 100644 --- a/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java +++ b/server/src/test/java/org/elasticsearch/discovery/DiscoveryDisruptionIT.java @@ -59,8 +59,8 @@ public class DiscoveryDisruptionIT extends AbstractDisruptionTestCase { public void testIsolatedUnicastNodes() throws Exception { - List nodes = startCluster(4, -1); internalCluster().setHostsListContainsOnlyFirstNode(true); + List nodes = startCluster(4, -1); // Figure out what is the elected master node final String unicastTarget = nodes.get(0); @@ -99,8 +99,8 @@ public void testIsolatedUnicastNodes() throws Exception { * The rejoining node should take this master node and connect. */ public void testUnicastSinglePingResponseContainsMaster() throws Exception { - List nodes = startCluster(4, -1); internalCluster().setHostsListContainsOnlyFirstNode(true); + List nodes = startCluster(4, -1); // Figure out what is the elected master node final String masterNode = internalCluster().getMasterName(); logger.info("---> legit elected master node={}", masterNode); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java index 7ed793031ee85..e4329fddb1f24 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java @@ -150,10 +150,9 @@ public Path nodeConfigPath(int nodeOrdinal) { @Override public Settings transportClientSettings() { - Settings superSettings = Settings.EMPTY; - Settings.Builder builder = Settings.builder().put(superSettings); + Settings.Builder builder = Settings.builder(); addClientSSLSettings(builder, ""); - addDefaultSecurityTransportType(builder, superSettings); + addDefaultSecurityTransportType(builder, Settings.EMPTY); if (randomBoolean()) { builder.put(SecurityField.USER_SETTING.getKey(),