Fail start of non-data node if node has data (#37347)

* 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

Author: henningandersen

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) == false) {
+                ensureNoShardData(nodePaths);
+            }
+
             success = true;
         } finally {
             if (success == false) {
@@ -1030,6 +1036,38 @@ private static void ensureAtomicMoveSupported(final NodePath[] nodePaths) throws
         }
     }
 
+    private void ensureNoShardData(final NodePath[] nodePaths) throws IOException {
+        List<Path> shardDataPaths = new ArrayList<>();
+        for (NodePath nodePath : nodePaths) {
+            Path indicesPath = nodePath.indicesPath;
+            if (Files.isDirectory(indicesPath)) {
+                try (DirectoryStream<Path> indexStream = Files.newDirectoryStream(indicesPath)) {
+                    for (Path indexPath : indexStream) {
+                        if (Files.isDirectory(indexPath)) {
+                            try (Stream<Path> shardStream = Files.list(indexPath)) {
+                                shardStream.filter(this::isShardPath)
+                                    .map(Path::toAbsolutePath)
+                                    .forEach(shardDataPaths::add);
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
+        if (shardDataPaths.isEmpty() == false) {
+            throw new IllegalStateException("Node is started with "
+                + Node.NODE_DATA_SETTING.getKey()
+                + "=false, but has shard data: "
+                + shardDataPaths);
+        }
+    }
+
+    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
@@ -1140,3 +1178,4 @@ private static void tryWriteTempFile(Path path) throws IOException {
         }
     }
 }
+

server/src/test/java/org/elasticsearch/env/NodeEnvironmentIT.java

@@ -0,0 +1,67 @@
+/*
+ * 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.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.startsWith;
+
+@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0)
+public class NodeEnvironmentIT extends ESIntegTestCase {
+    public void testStartFailureOnDataForNonDataNode() throws Exception {
+        final String indexName = "test-fail-on-data";
+
+        logger.info("--> starting one node");
+        internalCluster().startNode();
+
+        logger.info("--> creating index");
+        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(indexName, "type1", "1").setSource("field1", "value1").get();
+
+        logger.info("--> restarting the node with node.data=true");
+        internalCluster().restartRandomDataNode();
+
+        logger.info("--> restarting the node with node.data=false");
+        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));
+        assertThat(ex.getMessage(),
+            startsWith("Node is started with "
+                + Node.NODE_DATA_SETTING.getKey()
+                + "=false, but has shard data"));
+    }
+}

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;
 
@@ -52,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 {
@@ -470,6 +472,47 @@ public void testExistingTempFiles() throws IOException {
         }
     }
 
+    public void testEnsureNoShardData() throws IOException {
+        Settings settings = buildEnvSettings(Settings.EMPTY);
+        Index index = new Index("test", "testUUID");
+
+        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();
+
+        String shardDataDirName = Integer.toString(randomInt(10));
+        Path shardPath;
+
+        // 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];
+        }
+
+        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(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();
+    }
+
     /** 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];

test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java

@@ -1727,8 +1727,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 == false)
+                nodes.remove(nodeAndClient.name);
+        }
+
         if (activeDisruptionScheme != null) {
             activeDisruptionScheme.applyToNode(nodeAndClient.name, this);
         }