Adding feature to exclude indexes starting with dot from shard validator

CHANGELOG.md

@@ -137,6 +137,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
   - Addition of Doc values on the GeoShape Field
   - Addition of GeoShape ValueSource level code interfaces for accessing the DocValues.
   - Addition of Missing Value feature in the GeoShape Aggregations.
+- Added feature to ignore indexes starting with dot during shard limit validation.([#4695](https://github.com/opensearch-project/OpenSearch/pull/4695))
 
 ### Changed
 

server/src/internalClusterTest/java/org/opensearch/cluster/shards/ClusterShardLimitIT.java

@@ -63,12 +63,18 @@
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST)
 public class ClusterShardLimitIT extends OpenSearchIntegTestCase {
     private static final String shardsPerNodeKey = ShardLimitValidator.SETTING_CLUSTER_MAX_SHARDS_PER_NODE.getKey();
+    private static final String ignoreHiddenIndexKey = ShardLimitValidator.SETTING_CLUSTER_IGNORE_HIDDEN_INDEXES.getKey();
 
     public void testSettingClusterMaxShards() {
         int shardsPerNode = between(1, 500_000);
         setShardsPerNode(shardsPerNode);
     }
 
+    public void testSettingIgnoreHiddenIndexes() {
+        boolean ignoreHiddenIndexes = randomBoolean();
+        setIgnoreHiddenIndex(ignoreHiddenIndexes);
+    }
+
     public void testMinimumPerNode() {
         int negativeShardsPerNode = between(-50_000, 0);
         try {
@@ -127,6 +133,108 @@ public void testIndexCreationOverLimit() {
         assertFalse(clusterState.getMetadata().hasIndex("should-fail"));
     }
 
+    /**
+     * The test checks if the index starting with the dot can be created if the node has
+     * number of shards equivalent to the cluster.max_shards_per_node and the cluster.ignore_hidden_indexes
+     * setting is set to true. If the cluster.ignore_hidden_indexes is set to true index creation of
+     * indexes starting with dot would succeed.
+     */
+    public void testIndexCreationOverLimitForHiddenIndexesSucceeds() {
+        int dataNodes = client().admin().cluster().prepareState().get().getState().getNodes().getDataNodes().size();
+
+        // Setting the cluster.max_shards_per_node setting according to the data node count.
+        setShardsPerNode(dataNodes);
+
+        /*
+            Create an index that will bring us up to the limit. It would create index with primary equal to the
+            dataNodes * dataNodes so that cluster.max_shards_per_node setting is reached.
+         */
+        createIndex(
+            "test",
+            Settings.builder()
+                .put(indexSettings())
+                .put(SETTING_NUMBER_OF_SHARDS, dataNodes * dataNodes)
+                .put(SETTING_NUMBER_OF_REPLICAS, 0)
+                .build()
+        );
+
+        // Getting total active shards in the cluster.
+        int currentActiveShards = client().admin().cluster().prepareHealth().get().getActiveShards();
+
+        // Getting cluster.max_shards_per_node setting
+        ClusterState clusterState = client().admin().cluster().prepareState().get().getState();
+        String maxShardsPerNode = clusterState.getMetadata()
+            .settings()
+            .get(ShardLimitValidator.SETTING_CLUSTER_MAX_SHARDS_PER_NODE.getKey());
+
+        // Checking if the total shards created are equivalent to dataNodes * cluster.max_shards_per_node
+        assertEquals(dataNodes * Integer.parseInt(maxShardsPerNode), currentActiveShards);
+
+        createIndex(
+            ".test-index",
+            Settings.builder().put(indexSettings()).put(SETTING_NUMBER_OF_SHARDS, 1).put(SETTING_NUMBER_OF_REPLICAS, 0).build()
+        );
+
+        clusterState = client().admin().cluster().prepareState().get().getState();
+        assertTrue(clusterState.getMetadata().hasIndex(".test-index"));
+    }
+
+    /**
+     * The test checks if the index starting with the dot should not be created if the node has
+     * number of shards equivalent to the cluster.max_shards_per_node and the cluster.ignore_hidden_indexes
+     * setting is set to false. If the cluster.ignore_hidden_indexes is set to false index creation of
+     * indexes starting with dot would fail as well.
+     */
+    public void testIndexCreationOverLimitForHiddenIndexesFail() {
+        int dataNodes = client().admin().cluster().prepareState().get().getState().getNodes().getDataNodes().size();
+        int maxAllowedShards = dataNodes * dataNodes;
+
+        // Setting the cluster.max_shards_per_node setting according to the data node count.
+        setShardsPerNode(dataNodes);
+        setIgnoreHiddenIndex(false);
+
+        /*
+            Create an index that will bring us up to the limit. It would create index with primary equal to the
+            dataNodes * dataNodes so that cluster.max_shards_per_node setting is reached.
+         */
+        createIndex(
+            "test",
+            Settings.builder()
+                .put(indexSettings())
+                .put(SETTING_NUMBER_OF_SHARDS, maxAllowedShards)
+                .put(SETTING_NUMBER_OF_REPLICAS, 0)
+                .build()
+        );
+
+        // Getting total active shards in the cluster.
+        int currentActiveShards = client().admin().cluster().prepareHealth().get().getActiveShards();
+
+        // Getting cluster.max_shards_per_node setting
+        ClusterState clusterState = client().admin().cluster().prepareState().get().getState();
+        String maxShardsPerNode = clusterState.getMetadata()
+            .settings()
+            .get(ShardLimitValidator.SETTING_CLUSTER_MAX_SHARDS_PER_NODE.getKey());
+
+        // Checking if the total shards created are equivalent to dataNodes * cluster.max_shards_per_node
+        assertEquals(dataNodes * Integer.parseInt(maxShardsPerNode), currentActiveShards);
+
+        int extraShardCount = 1;
+        try {
+            createIndex(
+                ".test-index",
+                Settings.builder()
+                    .put(indexSettings())
+                    .put(SETTING_NUMBER_OF_SHARDS, extraShardCount)
+                    .put(SETTING_NUMBER_OF_REPLICAS, 0)
+                    .build()
+            );
+        } catch (IllegalArgumentException e) {
+            verifyException(maxAllowedShards, currentActiveShards, extraShardCount, e);
+        }
+        clusterState = client().admin().cluster().prepareState().get().getState();
+        assertFalse(clusterState.getMetadata().hasIndex(".test-index"));
+    }
+
     public void testIndexCreationOverLimitFromTemplate() {
         int dataNodes = client().admin().cluster().prepareState().get().getState().getNodes().getDataNodes().size();
 
@@ -457,6 +565,29 @@ private void setShardsPerNode(int shardsPerNode) {
         }
     }
 
+    private void setIgnoreHiddenIndex(boolean ignoreHiddenIndex) {
+        try {
+            ClusterUpdateSettingsResponse response;
+            if (frequently()) {
+                response = client().admin()
+                    .cluster()
+                    .prepareUpdateSettings()
+                    .setPersistentSettings(Settings.builder().put(ignoreHiddenIndexKey, ignoreHiddenIndex).build())
+                    .get();
+                assertEquals(ignoreHiddenIndex, response.getPersistentSettings().getAsBoolean(ignoreHiddenIndexKey, true));
+            } else {
+                response = client().admin()
+                    .cluster()
+                    .prepareUpdateSettings()
+                    .setTransientSettings(Settings.builder().put(ignoreHiddenIndexKey, ignoreHiddenIndex).build())
+                    .get();
+                assertEquals(ignoreHiddenIndex, response.getTransientSettings().getAsBoolean(ignoreHiddenIndexKey, true));
+            }
+        } catch (IllegalArgumentException ex) {
+            fail(ex.getMessage());
+        }
+    }
+
     private void verifyException(int dataNodes, ShardCounts counts, IllegalArgumentException e) {
         int totalShards = counts.getFailingIndexShards() * (1 + counts.getFailingIndexReplicas());
         int currentShards = counts.getFirstIndexShards() * (1 + counts.getFirstIndexReplicas());
@@ -471,4 +602,15 @@ private void verifyException(int dataNodes, ShardCounts counts, IllegalArgumentE
         assertEquals(expectedError, e.getMessage());
     }
 
+    private void verifyException(int maxShards, int currentShards, int extraShards, IllegalArgumentException e) {
+        String expectedError = "Validation Failed: 1: this action would add ["
+            + extraShards
+            + "] total shards, but this cluster currently has ["
+            + currentShards
+            + "]/["
+            + maxShards
+            + "] maximum shards open;";
+        assertEquals(expectedError, e.getMessage());
+    }
+
 }

server/src/main/java/org/opensearch/common/settings/ClusterSettings.java

@@ -252,6 +252,7 @@ public void apply(Settings value, Settings current, Settings previous) {
                 Metadata.SETTING_READ_ONLY_SETTING,
                 Metadata.SETTING_READ_ONLY_ALLOW_DELETE_SETTING,
                 ShardLimitValidator.SETTING_CLUSTER_MAX_SHARDS_PER_NODE,
+                ShardLimitValidator.SETTING_CLUSTER_IGNORE_HIDDEN_INDEXES,
                 RecoverySettings.INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING,
                 RecoverySettings.INDICES_RECOVERY_RETRY_DELAY_STATE_SYNC_SETTING,
                 RecoverySettings.INDICES_RECOVERY_RETRY_DELAY_NETWORK_SETTING,

server/src/main/java/org/opensearch/indices/ShardLimitValidator.java

@@ -64,12 +64,23 @@ public class ShardLimitValidator {
         Setting.Property.Dynamic,
         Setting.Property.NodeScope
     );
+
+    public static final Setting<Boolean> SETTING_CLUSTER_IGNORE_HIDDEN_INDEXES = Setting.boolSetting(
+        "cluster.ignore_hidden_indexes",
+        true,
+        Setting.Property.Dynamic,
+        Setting.Property.NodeScope
+    );
+
     protected final AtomicInteger shardLimitPerNode = new AtomicInteger();
     private final SystemIndices systemIndices;
+    private volatile boolean ignoreHiddenIndexes;
 
     public ShardLimitValidator(final Settings settings, ClusterService clusterService, SystemIndices systemIndices) {
         this.shardLimitPerNode.set(SETTING_CLUSTER_MAX_SHARDS_PER_NODE.get(settings));
+        this.ignoreHiddenIndexes = SETTING_CLUSTER_IGNORE_HIDDEN_INDEXES.get(settings);
         clusterService.getClusterSettings().addSettingsUpdateConsumer(SETTING_CLUSTER_MAX_SHARDS_PER_NODE, this::setShardLimitPerNode);
+        clusterService.getClusterSettings().addSettingsUpdateConsumer(SETTING_CLUSTER_IGNORE_HIDDEN_INDEXES, this::setIgnoreHiddenIndexes);
         this.systemIndices = systemIndices;
     }
 
@@ -85,17 +96,28 @@ public int getShardLimitPerNode() {
         return shardLimitPerNode.get();
     }
 
+    private void setIgnoreHiddenIndexes(boolean newValue) {
+        this.ignoreHiddenIndexes = newValue;
+    }
+
     /**
      * Checks whether an index can be created without going over the cluster shard limit.
+     * Validate shard limit only for non system indices as it is not hard limit anyways.
+     * Further also validates if the cluster.ignore_hidden_indexes is set to true.
+     * If so then it does not validate any index which starts with '.'
      *
      * @param indexName      the name of the index being created
      * @param settings       the settings of the index to be created
      * @param state          the current cluster state
      * @throws ValidationException if creating this index would put the cluster over the cluster shard limit
      */
     public void validateShardLimit(final String indexName, final Settings settings, final ClusterState state) {
-        // Validate shard limit only for non system indices as it is not hard limit anyways
-        if (systemIndices.validateSystemIndex(indexName)) {
+        /*
+        Validate shard limit only for non system indices as it is not hard limit anyways.
+        Further also validates if the cluster.ignore_hidden_indexes is set to true.
+        If so then it does not validate any index which starts with '.'.
+        */
+        if (systemIndices.validateSystemIndex(indexName) || validateHiddenIndex(indexName)) {
             return;
         }
 
@@ -113,16 +135,23 @@ public void validateShardLimit(final String indexName, final Settings settings,
 
     /**
      * Validates whether a list of indices can be opened without going over the cluster shard limit.  Only counts indices which are
-     * currently closed and will be opened, ignores indices which are already open.
+     * currently closed and will be opened, ignores indices which are already open. Adding to this it validates the
+     * shard limit only for non system indices and if the cluster.ignore_hidden_indexes property is set to true
+     * then the indexes starting with '.' are also ignored.
      *
      * @param currentState The current cluster state.
      * @param indicesToOpen The indices which are to be opened.
      * @throws ValidationException If this operation would take the cluster over the limit and enforcement is enabled.
      */
     public void validateShardLimit(ClusterState currentState, Index[] indicesToOpen) {
         int shardsToOpen = Arrays.stream(indicesToOpen)
-            // Validate shard limit only for non system indices as it is not hard limit anyways
+            /*
+            Validate shard limit only for non system indices as it is not hard limit anyways.
+            Further also validates if the cluster.ignore_hidden_indexes is set to true.
+            If so then it does not validate any index which starts with '.'.
+            */
             .filter(index -> !systemIndices.validateSystemIndex(index.getName()))
+            .filter(index -> !(validateHiddenIndex(index.getName())))
             .filter(index -> currentState.metadata().index(index).getState().equals(IndexMetadata.State.CLOSE))
             .mapToInt(index -> getTotalShardCount(currentState, index))
             .sum();
@@ -140,6 +169,16 @@ private static int getTotalShardCount(ClusterState state, Index index) {
         return indexMetadata.getNumberOfShards() * (1 + indexMetadata.getNumberOfReplicas());
     }
 
+    /**
+     * Returns true if the cluster.ignore_hidden_indexes property is set to true and the index name
+     * starts with '.' else false.
+     *
+     * @param indexName  The index which needs to be validated.
+     */
+    private boolean validateHiddenIndex(String indexName) {
+        return this.ignoreHiddenIndexes && indexName.charAt(0) == '.';
+    }
+
     /**
      * Checks to see if an operation can be performed without taking the cluster over the cluster-wide shard limit.
      * Returns an error message if appropriate, or an empty {@link Optional} otherwise.

server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java

@@ -592,7 +592,7 @@ public void testValidateIndexName() throws Exception {
                 null,
                 null,
                 null,
-                createTestShardLimitService(randomIntBetween(1, 1000), clusterService),
+                createTestShardLimitService(randomIntBetween(1, 1000), false, clusterService),
                 null,
                 null,
                 threadPool,
@@ -674,7 +674,7 @@ public void testValidateDotIndex() {
                 null,
                 null,
                 null,
-                createTestShardLimitService(randomIntBetween(1, 1000), clusterService),
+                createTestShardLimitService(randomIntBetween(1, 1000), false, clusterService),
                 null,
                 null,
                 threadPool,
@@ -1090,7 +1090,7 @@ public void testvalidateIndexSettings() {
             null,
             null,
             null,
-            createTestShardLimitService(randomIntBetween(1, 1000), clusterService),
+            createTestShardLimitService(randomIntBetween(1, 1000), false, clusterService),
             new Environment(Settings.builder().put("path.home", "dummy").build(), null),
             IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
             threadPool,
@@ -1234,7 +1234,7 @@ public void testIndexLifecycleNameSetting() {
                 null,
                 null,
                 null,
-                createTestShardLimitService(randomIntBetween(1, 1000), clusterService),
+                createTestShardLimitService(randomIntBetween(1, 1000), false, clusterService),
                 new Environment(Settings.builder().put("path.home", "dummy").build(), null),
                 new IndexScopedSettings(ilnSetting, Collections.emptySet()),
                 threadPool,
@@ -1311,7 +1311,7 @@ private static Map<String, CompressedXContent> convertMappings(ImmutableOpenMap<
     }
 
     private ShardLimitValidator randomShardLimitService() {
-        return createTestShardLimitService(randomIntBetween(10, 10000));
+        return createTestShardLimitService(randomIntBetween(10, 10000), false);
     }
 
     private void withTemporaryClusterService(BiConsumer<ClusterService, ThreadPool> consumer) {

server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java

@@ -2100,7 +2100,7 @@ private static List<Throwable> putTemplate(NamedXContentRegistry xContentRegistr
             null,
             null,
             null,
-            createTestShardLimitService(randomIntBetween(1, 1000)),
+            createTestShardLimitService(randomIntBetween(1, 1000), false),
             new Environment(builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).build(), null),
             IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
             null,
@@ -2163,7 +2163,7 @@ private MetadataIndexTemplateService getMetadataIndexTemplateService() {
             indicesService,
             null,
             null,
-            createTestShardLimitService(randomIntBetween(1, 1000)),
+            createTestShardLimitService(randomIntBetween(1, 1000), false),
             new Environment(builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).build(), null),
             IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
             null,