From 115c7f98a533713f0b9cd3d5b61d20075a968522 Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Thu, 10 Jan 2019 15:56:54 +0100 Subject: [PATCH 1/3] Fail start of non-data node if node has data Check that nodes started with node.data=false cannot start if they have shard data to avoid (old) indexes being resurrected into the cluster in red status. Issue #27073 --- .../elasticsearch/env/NodeEnvironment.java | 52 +++++++++++-- .../elasticsearch/env/NodeEnvironmentIT.java | 76 +++++++++++++++++++ .../env/NodeEnvironmentTests.java | 67 ++++++++++++++++ .../test/InternalTestCluster.java | 12 ++- 4 files changed, 198 insertions(+), 9 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index 2d236b08f79de..5dc751fb32119 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -59,6 +59,7 @@ import org.elasticsearch.monitor.fs.FsInfo; import org.elasticsearch.monitor.fs.FsProbe; import org.elasticsearch.monitor.jvm.JvmInfo; +import org.elasticsearch.node.Node; import java.io.Closeable; import java.io.IOException; @@ -309,6 +310,11 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce if (DiscoveryNode.isMasterNode(settings) || DiscoveryNode.isDataNode(settings)) { ensureAtomicMoveSupported(nodePaths); } + + if (!DiscoveryNode.isDataNode(settings)) { + ensureNoIndexData(nodePaths); + } + success = true; } finally { if (success == false) { @@ -1030,13 +1036,44 @@ private static void ensureAtomicMoveSupported(final NodePath[] nodePaths) throws } } - /** - * Resolve the custom path for a index's shard. - * Uses the {@code IndexMetaData.SETTING_DATA_PATH} setting to determine - * the root path for the index. - * - * @param indexSettings settings for the index - */ + private void ensureNoIndexData(final NodePath[] nodePaths) throws IOException { + List badDataPaths = new ArrayList<>(); + for (NodePath nodePath : nodePaths) { + Path indicesPath = nodePath.indicesPath; + if (Files.isDirectory(indicesPath)) { + try (DirectoryStream indexStream = Files.newDirectoryStream(indicesPath)) { + for (Path indexPath : indexStream) { + if (Files.isDirectory(indexPath)) { + try (Stream shardStream = Files.list(indexPath)) { + shardStream.filter(this::isShardPath) + .forEach(badDataPaths::add); + } + } + } + } + } + } + + if (!badDataPaths.isEmpty()) { + throw new IllegalArgumentException("Node is started with " + + Node.NODE_DATA_SETTING.getKey() + + "=false, but found shard data: " + + badDataPaths); + } + } + + private boolean isShardPath(Path path) { + return Files.isDirectory(path) + && path.getFileName().toString().chars().allMatch(Character::isDigit); + } + + /** + * Resolve the custom path for a index's shard. + * Uses the {@code IndexMetaData.SETTING_DATA_PATH} setting to determine + * the root path for the index. + * + * @param indexSettings settings for the index + */ public static Path resolveBaseCustomLocation(IndexSettings indexSettings, Path sharedDataPath, int nodeLockId) { String customDataDir = indexSettings.customDataPath(); if (customDataDir != null) { @@ -1140,3 +1177,4 @@ private static void tryWriteTempFile(Path path) throws IOException { } } } + diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java new file mode 100644 index 0000000000000..1df7acc5c5f73 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java @@ -0,0 +1,76 @@ +/* + * 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.env; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.node.Node; +import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.InternalTestCluster; + +import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; +import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; +import static org.hamcrest.Matchers.equalTo; + +@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0) +public class NodeEnvironmentIT extends ESIntegTestCase { + public void testStartFailureOnDataForNonDataNode() throws Exception { + logger.info("--> starting two nodes"); + + final String node_1 = internalCluster().startNodes(1).get(0); + + logger.info("--> creating index"); + prepareCreate("test", Settings.builder() + .put("index.number_of_shards", 1) + .put("index.number_of_replicas", 0) + ).get(); + + logger.info("--> indexing a simple document"); + client().prepareIndex("test", "type1", "1").setSource("field1", "value1").setRefreshPolicy(IMMEDIATE).get(); + + logger.info("--> waiting for green status"); + ensureGreen(); + + logger.info("--> verify 1 doc in the index"); + for (int i = 0; i < 10; i++) { + assertHitCount(client().prepareSearch().setQuery(matchAllQuery()).get(), 1L); + } + assertThat(client().prepareGet("test", "type1", "1").execute().actionGet().isExists(), equalTo(true)); + + logger.info("--> restarting the node with node.data=true"); + internalCluster().restartNode(node_1, new InternalTestCluster.RestartCallback()); + + logger.info("--> waiting for green status"); + ensureGreen(); + + logger.info("--> restarting the node with node.data=false"); + try { + internalCluster().restartNode(node_1, new InternalTestCluster.RestartCallback() { + @Override + public Settings onNodeStopped(String nodeName) throws Exception { + return Settings.builder().put(Node.NODE_DATA_SETTING.getKey(), false).build(); + } + }); + fail("Starting a node with node.data=false that has index/shard data must fail"); + } catch (IllegalArgumentException e) { + // EXPECTED + } + } +} diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index 55ab02c1dcfb9..3ad4eee2f875b 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -31,6 +31,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.node.Node; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.IndexSettingsModule; @@ -470,6 +471,72 @@ public void testExistingTempFiles() throws IOException { } } + public void testFailOnDataForNonDataNode() throws IOException { + Settings settings = buildEnvSettings(Settings.EMPTY); + final NodeEnvironment env = newNodeEnvironment(settings); + + Index badIndex = new Index("bad", "badUUID"); + for (Path path : env.indexPaths(badIndex)) { + Files.createDirectories(path.resolve("0")); + } + + env.close(); + + try { + Settings noDataSettings = Settings.builder() + .put(settings) + .put(Node.NODE_DATA_SETTING.getKey(), false).build(); + newNodeEnvironment(noDataSettings).close(); + fail("Must fail creating NodeEnvironment on a data path that has shard data if node.data=false"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString(env.indexPaths(badIndex)[0].resolve("0").getFileName().toString())); + } + } + + public void testSucceedOnMetaOnlyForNonDataNode() throws IOException { + Settings settings = buildEnvSettings(Settings.EMPTY); + final NodeEnvironment env = newNodeEnvironment(settings); + + Index goodIndex = new Index("good", "goodUUID"); + + for (Path path : env.indexPaths(goodIndex)) { + Files.createDirectories(path.resolve(MetaDataStateFormat.STATE_DIR_NAME)); + } + + env.close(); + + Settings noDataSettings = Settings.builder() + .put(settings) + .put(Node.NODE_DATA_SETTING.getKey(), false).build(); + + // test that this succeeds + newNodeEnvironment(noDataSettings).close(); + } + + public void testSucceedOnDataForDataNode() throws IOException { + Settings settings = buildEnvSettings(Settings.EMPTY); + final NodeEnvironment env = newNodeEnvironment(settings); + + Index goodIndex = new Index("good", "goodUUID"); + + for (Path path : env.indexPaths(goodIndex)) { + Files.createDirectories(path.resolve("0")); + } + + env.close(); + + // test that this succeeds + newNodeEnvironment(settings).close(); + } + + private String[] pathsToStrings(Path[] paths) { + String[] result = new String[paths.length]; + for (int i = 0; i < paths.length; i++) { + result[i] = paths[i].toString(); + } + return result; + } + /** Converts an array of Strings to an array of Paths, adding an additional child if specified */ private Path[] stringsToPaths(String[] strings, String additional) { Path[] locations = new Path[strings.length]; 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 e6e11dacb749f..aa80d6a8741cc 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -1736,8 +1736,16 @@ private void restartNode(NodeAndClient nodeAndClient, RestartCallback callback) removeExclusions(excludedNodeIds); - nodeAndClient.recreateNode(newSettings, () -> rebuildUnicastHostFiles(emptyList())); - nodeAndClient.startNode(); + boolean success = false; + try { + nodeAndClient.recreateNode(newSettings, () -> rebuildUnicastHostFiles(emptyList())); + nodeAndClient.startNode(); + success = true; + } finally { + if (!success) + nodes.remove(nodeAndClient.name); + } + if (activeDisruptionScheme != null) { activeDisruptionScheme.applyToNode(nodeAndClient.name, this); } From 7d6d8247423afde5ce44a7dab5ff7d5a7b3211e2 Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Fri, 11 Jan 2019 17:38:07 +0100 Subject: [PATCH 2/3] Fail start of non-data node if node has data Improved message and exception type plus cleaner tests. --- .../elasticsearch/env/NodeEnvironment.java | 33 +++++----- .../elasticsearch/env/NodeEnvironmentIT.java | 49 ++++++-------- .../env/NodeEnvironmentTests.java | 64 +++++-------------- 3 files changed, 52 insertions(+), 94 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index 5dc751fb32119..2f676eb846770 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -311,8 +311,8 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce ensureAtomicMoveSupported(nodePaths); } - if (!DiscoveryNode.isDataNode(settings)) { - ensureNoIndexData(nodePaths); + if (DiscoveryNode.isDataNode(settings) == false) { + ensureNoShardData(nodePaths); } success = true; @@ -1036,8 +1036,8 @@ private static void ensureAtomicMoveSupported(final NodePath[] nodePaths) throws } } - private void ensureNoIndexData(final NodePath[] nodePaths) throws IOException { - List badDataPaths = new ArrayList<>(); + private void ensureNoShardData(final NodePath[] nodePaths) throws IOException { + List shardDataPaths = new ArrayList<>(); for (NodePath nodePath : nodePaths) { Path indicesPath = nodePath.indicesPath; if (Files.isDirectory(indicesPath)) { @@ -1046,7 +1046,8 @@ private void ensureNoIndexData(final NodePath[] nodePaths) throws IOException { if (Files.isDirectory(indexPath)) { try (Stream shardStream = Files.list(indexPath)) { shardStream.filter(this::isShardPath) - .forEach(badDataPaths::add); + .map(Path::toAbsolutePath) + .forEach(shardDataPaths::add); } } } @@ -1054,11 +1055,11 @@ private void ensureNoIndexData(final NodePath[] nodePaths) throws IOException { } } - if (!badDataPaths.isEmpty()) { - throw new IllegalArgumentException("Node is started with " + if (shardDataPaths.isEmpty() == false) { + throw new IllegalStateException("Node is started with " + Node.NODE_DATA_SETTING.getKey() - + "=false, but found shard data: " - + badDataPaths); + + "=false, but has shard data: " + + shardDataPaths); } } @@ -1067,13 +1068,13 @@ private boolean isShardPath(Path path) { && path.getFileName().toString().chars().allMatch(Character::isDigit); } - /** - * Resolve the custom path for a index's shard. - * Uses the {@code IndexMetaData.SETTING_DATA_PATH} setting to determine - * the root path for the index. - * - * @param indexSettings settings for the index - */ + /** + * Resolve the custom path for a index's shard. + * Uses the {@code IndexMetaData.SETTING_DATA_PATH} setting to determine + * the root path for the index. + * + * @param indexSettings settings for the index + */ public static Path resolveBaseCustomLocation(IndexSettings indexSettings, Path sharedDataPath, int nodeLockId) { String customDataDir = indexSettings.customDataPath(); if (customDataDir != null) { diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java index 1df7acc5c5f73..6ce53b18455c4 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java @@ -25,52 +25,39 @@ import org.elasticsearch.test.InternalTestCluster; import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; -import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; -import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.containsString; @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0) public class NodeEnvironmentIT extends ESIntegTestCase { public void testStartFailureOnDataForNonDataNode() throws Exception { - logger.info("--> starting two nodes"); + final String indexName = "test-fail-on-data"; - final String node_1 = internalCluster().startNodes(1).get(0); + logger.info("--> starting one node"); + internalCluster().startNodes(1); logger.info("--> creating index"); - prepareCreate("test", Settings.builder() + prepareCreate(indexName, Settings.builder() .put("index.number_of_shards", 1) .put("index.number_of_replicas", 0) ).get(); + final String indexUUID = resolveIndex(indexName).getUUID(); logger.info("--> indexing a simple document"); - client().prepareIndex("test", "type1", "1").setSource("field1", "value1").setRefreshPolicy(IMMEDIATE).get(); - - logger.info("--> waiting for green status"); - ensureGreen(); - - logger.info("--> verify 1 doc in the index"); - for (int i = 0; i < 10; i++) { - assertHitCount(client().prepareSearch().setQuery(matchAllQuery()).get(), 1L); - } - assertThat(client().prepareGet("test", "type1", "1").execute().actionGet().isExists(), equalTo(true)); + client().prepareIndex(indexName, "type1", "1").setSource("field1", "value1").setRefreshPolicy(IMMEDIATE).get(); logger.info("--> restarting the node with node.data=true"); - internalCluster().restartNode(node_1, new InternalTestCluster.RestartCallback()); - - logger.info("--> waiting for green status"); - ensureGreen(); + internalCluster().restartRandomDataNode(); logger.info("--> restarting the node with node.data=false"); - try { - internalCluster().restartNode(node_1, new InternalTestCluster.RestartCallback() { - @Override - public Settings onNodeStopped(String nodeName) throws Exception { - return Settings.builder().put(Node.NODE_DATA_SETTING.getKey(), false).build(); - } - }); - fail("Starting a node with node.data=false that has index/shard data must fail"); - } catch (IllegalArgumentException e) { - // EXPECTED - } + IllegalStateException ex = expectThrows(IllegalStateException.class, + "Node started with node.data=false and existing shard data must fail", + () -> + internalCluster().restartRandomDataNode(new InternalTestCluster.RestartCallback() { + @Override + public Settings onNodeStopped(String nodeName) { + return Settings.builder().put(Node.NODE_DATA_SETTING.getKey(), false).build(); + } + })); + assertThat(ex.getMessage(), containsString(indexUUID)); } } diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index 3ad4eee2f875b..28288203683af 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -471,35 +471,12 @@ public void testExistingTempFiles() throws IOException { } } - public void testFailOnDataForNonDataNode() throws IOException { + public void testEnsureNoShardData() throws IOException { Settings settings = buildEnvSettings(Settings.EMPTY); - final NodeEnvironment env = newNodeEnvironment(settings); - - Index badIndex = new Index("bad", "badUUID"); - for (Path path : env.indexPaths(badIndex)) { - Files.createDirectories(path.resolve("0")); - } - - env.close(); - - try { - Settings noDataSettings = Settings.builder() - .put(settings) - .put(Node.NODE_DATA_SETTING.getKey(), false).build(); - newNodeEnvironment(noDataSettings).close(); - fail("Must fail creating NodeEnvironment on a data path that has shard data if node.data=false"); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), containsString(env.indexPaths(badIndex)[0].resolve("0").getFileName().toString())); - } - } - - public void testSucceedOnMetaOnlyForNonDataNode() throws IOException { - Settings settings = buildEnvSettings(Settings.EMPTY); - final NodeEnvironment env = newNodeEnvironment(settings); - - Index goodIndex = new Index("good", "goodUUID"); + NodeEnvironment env = newNodeEnvironment(settings); - for (Path path : env.indexPaths(goodIndex)) { + Index index = new Index("test", "testUUID"); + for (Path path : env.indexPaths(index)) { Files.createDirectories(path.resolve(MetaDataStateFormat.STATE_DIR_NAME)); } @@ -509,32 +486,25 @@ public void testSucceedOnMetaOnlyForNonDataNode() throws IOException { .put(settings) .put(Node.NODE_DATA_SETTING.getKey(), false).build(); - // test that this succeeds - newNodeEnvironment(noDataSettings).close(); - } + // test that we can create data=false env with only meta information + env = newNodeEnvironment(noDataSettings); - public void testSucceedOnDataForDataNode() throws IOException { - Settings settings = buildEnvSettings(Settings.EMPTY); - final NodeEnvironment env = newNodeEnvironment(settings); - - Index goodIndex = new Index("good", "goodUUID"); - - for (Path path : env.indexPaths(goodIndex)) { - Files.createDirectories(path.resolve("0")); + String shardDir = Integer.toString(randomInt(10)); + for (Path path : env.indexPaths(index)) { + Files.createDirectories(path.resolve(shardDir)); } env.close(); - // test that this succeeds - newNodeEnvironment(settings).close(); - } + IllegalStateException ex = expectThrows(IllegalStateException.class, + "Must fail creating NodeEnvironment on a data path that has shard data if node.data=false", + () -> newNodeEnvironment(noDataSettings).close()); - private String[] pathsToStrings(Path[] paths) { - String[] result = new String[paths.length]; - for (int i = 0; i < paths.length; i++) { - result[i] = paths[i].toString(); - } - return result; + assertThat(ex.getMessage(), + containsString(env.indexPaths(index)[0].resolve(shardDir).toAbsolutePath().toString())); + + // test that we can create data=true env + newNodeEnvironment(settings).close(); } /** Converts an array of Strings to an array of Paths, adding an additional child if specified */ From 288860b06bc9660a776d3ea201dc774e77b5adac Mon Sep 17 00:00:00 2001 From: Henning Andersen Date: Fri, 18 Jan 2019 01:38:51 +0100 Subject: [PATCH 3/3] Fail start of non-data node if node has data Avoid impacting subsequent tests on failures and added assert for more of the message. --- .../elasticsearch/env/NodeEnvironmentIT.java | 10 ++++-- .../env/NodeEnvironmentTests.java | 34 +++++++++++-------- .../test/InternalTestCluster.java | 2 +- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java index 6ce53b18455c4..bdca86858701f 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java @@ -24,8 +24,8 @@ import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.InternalTestCluster; -import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.startsWith; @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0) public class NodeEnvironmentIT extends ESIntegTestCase { @@ -33,7 +33,7 @@ public void testStartFailureOnDataForNonDataNode() throws Exception { final String indexName = "test-fail-on-data"; logger.info("--> starting one node"); - internalCluster().startNodes(1); + internalCluster().startNode(); logger.info("--> creating index"); prepareCreate(indexName, Settings.builder() @@ -43,7 +43,7 @@ public void testStartFailureOnDataForNonDataNode() throws Exception { final String indexUUID = resolveIndex(indexName).getUUID(); logger.info("--> indexing a simple document"); - client().prepareIndex(indexName, "type1", "1").setSource("field1", "value1").setRefreshPolicy(IMMEDIATE).get(); + client().prepareIndex(indexName, "type1", "1").setSource("field1", "value1").get(); logger.info("--> restarting the node with node.data=true"); internalCluster().restartRandomDataNode(); @@ -59,5 +59,9 @@ public Settings onNodeStopped(String nodeName) { } })); assertThat(ex.getMessage(), containsString(indexUUID)); + assertThat(ex.getMessage(), + startsWith("Node is started with " + + Node.NODE_DATA_SETTING.getKey() + + "=false, but has shard data")); } } diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index 28288203683af..2771bc9f243ac 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -53,6 +53,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; @LuceneTestCase.SuppressFileSystems("ExtrasFS") // TODO: fix test to allow extras public class NodeEnvironmentTests extends ESTestCase { @@ -473,35 +474,40 @@ public void testExistingTempFiles() throws IOException { public void testEnsureNoShardData() throws IOException { Settings settings = buildEnvSettings(Settings.EMPTY); - NodeEnvironment env = newNodeEnvironment(settings); - Index index = new Index("test", "testUUID"); - for (Path path : env.indexPaths(index)) { - Files.createDirectories(path.resolve(MetaDataStateFormat.STATE_DIR_NAME)); - } - env.close(); + try (NodeEnvironment env = newNodeEnvironment(settings)) { + for (Path path : env.indexPaths(index)) { + Files.createDirectories(path.resolve(MetaDataStateFormat.STATE_DIR_NAME)); + } + } + // build settings using same path.data as original but with node.data=false Settings noDataSettings = Settings.builder() .put(settings) .put(Node.NODE_DATA_SETTING.getKey(), false).build(); - // test that we can create data=false env with only meta information - env = newNodeEnvironment(noDataSettings); + String shardDataDirName = Integer.toString(randomInt(10)); + Path shardPath; - String shardDir = Integer.toString(randomInt(10)); - for (Path path : env.indexPaths(index)) { - Files.createDirectories(path.resolve(shardDir)); + // test that we can create data=false env with only meta information + try (NodeEnvironment env = newNodeEnvironment(noDataSettings)) { + for (Path path : env.indexPaths(index)) { + Files.createDirectories(path.resolve(shardDataDirName)); + } + shardPath = env.indexPaths(index)[0]; } - env.close(); - IllegalStateException ex = expectThrows(IllegalStateException.class, "Must fail creating NodeEnvironment on a data path that has shard data if node.data=false", () -> newNodeEnvironment(noDataSettings).close()); assertThat(ex.getMessage(), - containsString(env.indexPaths(index)[0].resolve(shardDir).toAbsolutePath().toString())); + containsString(shardPath.resolve(shardDataDirName).toAbsolutePath().toString())); + assertThat(ex.getMessage(), + startsWith("Node is started with " + + Node.NODE_DATA_SETTING.getKey() + + "=false, but has shard data")); // test that we can create data=true env newNodeEnvironment(settings).close(); 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 aa80d6a8741cc..ef097a881bab6 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -1742,7 +1742,7 @@ private void restartNode(NodeAndClient nodeAndClient, RestartCallback callback) nodeAndClient.startNode(); success = true; } finally { - if (!success) + if (success == false) nodes.remove(nodeAndClient.name); }