Fail start on invalid index metadata (#37748)

Node started with node.data=false and node.master=false can no longer
start if they have index metadata. This avoids resurrecting old indexes
into the cluster and ensures metadata is cleaned out before
re-purposing a node that was previously master or data node.

Issue #27073

Author: henningandersen

server/src/main/java/org/elasticsearch/env/NodeEnvironment.java

@@ -312,6 +312,10 @@ public NodeEnvironment(Settings settings, Environment environment) throws IOExce
             }
 
             if (DiscoveryNode.isDataNode(settings) == false) {
+                if (DiscoveryNode.isMasterNode(settings) == false) {
+                    ensureNoIndexMetaData(nodePaths);
+                }
+
                 ensureNoShardData(nodePaths);
             }
 
@@ -1037,37 +1041,59 @@ private static void ensureAtomicMoveSupported(final NodePath[] nodePaths) throws
     }
 
     private void ensureNoShardData(final NodePath[] nodePaths) throws IOException {
-        List<Path> shardDataPaths = new ArrayList<>();
+        List<Path> shardDataPaths = collectIndexSubPaths(nodePaths, this::isShardPath);
+        if (shardDataPaths.isEmpty() == false) {
+            throw new IllegalStateException("Node is started with "
+                + Node.NODE_DATA_SETTING.getKey()
+                + "=false, but has shard data: "
+                + shardDataPaths);
+        }
+    }
+
+    private void ensureNoIndexMetaData(final NodePath[] nodePaths) throws IOException {
+        List<Path> indexMetaDataPaths = collectIndexSubPaths(nodePaths, this::isIndexMetaDataPath);
+        if (indexMetaDataPaths.isEmpty() == false) {
+            throw new IllegalStateException("Node is started with "
+                + Node.NODE_DATA_SETTING.getKey()
+                + "=false and "
+                + Node.NODE_MASTER_SETTING.getKey()
+                + "=false, but has index metadata: "
+                + indexMetaDataPaths);
+        }
+    }
+
+    private List<Path> collectIndexSubPaths(NodePath[] nodePaths, Predicate<Path> subPathPredicate) throws IOException {
+        List<Path> indexSubPaths = 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)
+                                shardStream.filter(subPathPredicate)
                                     .map(Path::toAbsolutePath)
-                                    .forEach(shardDataPaths::add);
+                                    .forEach(indexSubPaths::add);
                             }
                         }
                     }
                 }
             }
         }
 
-        if (shardDataPaths.isEmpty() == false) {
-            throw new IllegalStateException("Node is started with "
-                + Node.NODE_DATA_SETTING.getKey()
-                + "=false, but has shard data: "
-                + shardDataPaths);
-        }
+        return indexSubPaths;
     }
 
     private boolean isShardPath(Path path) {
         return Files.isDirectory(path)
             && path.getFileName().toString().chars().allMatch(Character::isDigit);
     }
 
+    private boolean isIndexMetaDataPath(Path path) {
+        return Files.isDirectory(path)
+            && path.getFileName().toString().equals(MetaDataStateFormat.STATE_DIR_NAME);
+    }
+
     /**
      * Resolve the custom path for a index's shard.
      * Uses the {@code IndexMetaData.SETTING_DATA_PATH} setting to determine

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

@@ -42,15 +42,37 @@ public void testStartFailureOnDataForNonDataNode() throws Exception {
         ).get();
         final String indexUUID = resolveIndex(indexName).getUUID();
 
+        logger.info("--> restarting the node with node.data=false and node.master=false");
+        IllegalStateException ex = expectThrows(IllegalStateException.class,
+            "Node started with node.data=false and node.master=false while having existing index metadata must fail",
+            () ->
+                internalCluster().restartRandomDataNode(new InternalTestCluster.RestartCallback() {
+                    @Override
+                    public Settings onNodeStopped(String nodeName) {
+                        return Settings.builder()
+                            .put(Node.NODE_DATA_SETTING.getKey(), false)
+                            .put(Node.NODE_MASTER_SETTING.getKey(), false)
+                            .build();
+                    }
+                }));
+        assertThat(ex.getMessage(), containsString(indexUUID));
+        assertThat(ex.getMessage(),
+            startsWith("Node is started with "
+                + Node.NODE_DATA_SETTING.getKey()
+                + "=false and "
+                + Node.NODE_MASTER_SETTING.getKey()
+                + "=false, but has index metadata"));
+
+        // client() also starts the node
         logger.info("--> indexing a simple document");
         client().prepareIndex(indexName, "type1", "1").setSource("field1", "value1").get();
 
-        logger.info("--> restarting the node with node.data=true");
+        logger.info("--> restarting the node with node.data=true and node.master=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",
+        ex = expectThrows(IllegalStateException.class,
+            "Node started with node.data=false while having existing shard data must fail",
             () ->
                 internalCluster().restartRandomDataNode(new InternalTestCluster.RestartCallback() {
                     @Override

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

@@ -472,45 +472,96 @@ public void testExistingTempFiles() throws IOException {
         }
     }
 
-    public void testEnsureNoShardData() throws IOException {
+    public void testEnsureNoShardDataOrIndexMetaData() throws IOException {
         Settings settings = buildEnvSettings(Settings.EMPTY);
         Index index = new Index("test", "testUUID");
 
+        // build settings using same path.data as original but with node.data=false and node.master=false
+        Settings noDataNoMasterSettings = Settings.builder()
+            .put(settings)
+            .put(Node.NODE_DATA_SETTING.getKey(), false)
+            .put(Node.NODE_MASTER_SETTING.getKey(), false)
+            .build();
+
+        // test that we can create data=false and master=false with no meta information
+        newNodeEnvironment(noDataNoMasterSettings).close();
+
+        Path indexPath;
         try (NodeEnvironment env = newNodeEnvironment(settings)) {
             for (Path path : env.indexPaths(index)) {
                 Files.createDirectories(path.resolve(MetaDataStateFormat.STATE_DIR_NAME));
             }
+            indexPath = env.indexPaths(index)[0];
         }
 
+        verifyFailsOnMetaData(noDataNoMasterSettings, indexPath);
+
         // 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
+        // test that we can create data=false env with only meta information. Also create shard data for following asserts
         try (NodeEnvironment env = newNodeEnvironment(noDataSettings)) {
             for (Path path : env.indexPaths(index)) {
                 Files.createDirectories(path.resolve(shardDataDirName));
             }
-            shardPath = env.indexPaths(index)[0];
         }
 
+        verifyFailsOnShardData(noDataSettings, indexPath, shardDataDirName);
+
+        // assert that we get the stricter message on meta-data when both conditions fail
+        verifyFailsOnMetaData(noDataNoMasterSettings, indexPath);
+
+        // build settings using same path.data as original but with node.master=false
+        Settings noMasterSettings = Settings.builder()
+            .put(settings)
+            .put(Node.NODE_MASTER_SETTING.getKey(), false)
+            .build();
+
+        // test that we can create master=false env regardless of data.
+        newNodeEnvironment(noMasterSettings).close();
+
+        // test that we can create data=true, master=true env. Also remove state dir to leave only shard data for following asserts
+        try (NodeEnvironment env = newNodeEnvironment(settings)) {
+            for (Path path : env.indexPaths(index)) {
+                Files.delete(path.resolve(MetaDataStateFormat.STATE_DIR_NAME));
+            }
+        }
+
+        // assert that we fail on shard data even without the metadata dir.
+        verifyFailsOnShardData(noDataSettings, indexPath, shardDataDirName);
+        verifyFailsOnShardData(noDataNoMasterSettings, indexPath, shardDataDirName);
+    }
+
+    private void verifyFailsOnShardData(Settings settings, Path indexPath, String shardDataDirName) {
         IllegalStateException ex = expectThrows(IllegalStateException.class,
             "Must fail creating NodeEnvironment on a data path that has shard data if node.data=false",
-            () -> newNodeEnvironment(noDataSettings).close());
+            () -> newNodeEnvironment(settings).close());
 
         assertThat(ex.getMessage(),
-            containsString(shardPath.resolve(shardDataDirName).toAbsolutePath().toString()));
+            containsString(indexPath.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();
+    private void verifyFailsOnMetaData(Settings settings, Path indexPath) {
+        IllegalStateException ex = expectThrows(IllegalStateException.class,
+            "Must fail creating NodeEnvironment on a data path that has index meta-data if node.data=false and node.master=false",
+            () -> newNodeEnvironment(settings).close());
+
+        assertThat(ex.getMessage(),
+            containsString(indexPath.resolve(MetaDataStateFormat.STATE_DIR_NAME).toAbsolutePath().toString()));
+        assertThat(ex.getMessage(),
+            startsWith("Node is started with "
+                + Node.NODE_DATA_SETTING.getKey()
+                + "=false and "
+                + Node.NODE_MASTER_SETTING.getKey()
+                + "=false, but has index metadata"));
     }
 
     /** Converts an array of Strings to an array of Paths, adding an additional child if specified */