Adding more ITs and ser/de for new parameter

Signed-off-by: Harsh Garg <[email protected]>
opensearch-project · Dec 2, 2024 · 10d0701 · 10d0701
1 parent 40b2641
commit 10d0701
Show file tree

Hide file tree

Showing 4 changed files with 185 additions and 12 deletions.
diff --git a/...usterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java b/...usterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java
@@ -8,6 +8,8 @@
 
 package org.opensearch.action.admin.cluster.shards;
 
+import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest;
+import org.opensearch.action.admin.indices.datastream.DataStreamTestCase;
 import org.opensearch.action.admin.indices.stats.IndicesStatsResponse;
 import org.opensearch.action.pagination.PageParams;
 import org.opensearch.client.Requests;
@@ -31,9 +33,10 @@
 import static org.opensearch.cluster.routing.UnassignedInfo.INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING;
 import static org.opensearch.common.unit.TimeValue.timeValueMillis;
 import static org.opensearch.search.SearchService.NO_TIMEOUT;
+import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 
 @OpenSearchIntegTestCase.ClusterScope(numDataNodes = 0, scope = OpenSearchIntegTestCase.Scope.TEST)
-public class TransportCatShardsActionIT extends OpenSearchIntegTestCase {
+public class TransportCatShardsActionIT extends DataStreamTestCase {
 
     public void testCatShardsWithSuccessResponse() throws InterruptedException {
         internalCluster().startClusterManagerOnlyNodes(1);
@@ -130,6 +133,71 @@ public void onFailure(Exception e) {
         latch.await();
     }
 
+    public void testListShardsWithHiddenIndex() throws Exception {
+        final int numShards = 1;
+        final int numReplicas = 1;
+        internalCluster().startClusterManagerOnlyNodes(1);
+        internalCluster().startDataOnlyNodes(2);
+        createIndex(
+            "test-hidden-idx",
+            Settings.builder()
+                .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards)
+                .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
+                .put(IndexMetadata.SETTING_INDEX_HIDDEN, true)
+                .build()
+        );
+        ensureGreen();
+
+        // Verify result for a default query: "_list/shards"
+        CatShardsRequest listShardsRequest = getListShardsTransportRequest(Strings.EMPTY_ARRAY, 100);
+        ActionFuture<CatShardsResponse> listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
+        assertSingleIndexShardsWithStats(listShardsResponse.get(), "test-hidden-idx", 2, true);
+
+        // Verify result when hidden index is explicitly queried: "_list/shards"
+        listShardsRequest = getListShardsTransportRequest(new String[] { "test-hidden-idx" }, 100);
+        listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
+        assertSingleIndexShardsWithStats(listShardsResponse.get(), "test-hidden-idx", 2, true);
+
+        // Verify result when hidden index is queried with wildcard: "_list/shards*"
+        listShardsRequest = getListShardsTransportRequest(new String[] { "test-hidden-idx*" }, 100);
+        listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
+        assertSingleIndexShardsWithStats(listShardsResponse.get(), "test-hidden-idx", 2, true);
+    }
+
+    public void testListShardsWithClosedIndex() throws Exception {
+        final int numShards = 1;
+        final int numReplicas = 1;
+        internalCluster().startClusterManagerOnlyNodes(1);
+        internalCluster().startDataOnlyNodes(2);
+        createIndex(
+            "test-closed-idx",
+            Settings.builder()
+                .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards)
+                .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
+                .build()
+        );
+        ensureGreen();
+
+        // close index "test-closed-idx"
+        client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx")).get();
+        ensureGreen();
+
+        // Verify result for a default query: "_list/shards"
+        CatShardsRequest listShardsRequest = getListShardsTransportRequest(Strings.EMPTY_ARRAY, 100);
+        ActionFuture<CatShardsResponse> listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
+        assertSingleIndexShardsWithStats(listShardsResponse.get(), "test-closed-idx", 2, false);
+
+        // Verify result when closed index is explicitly queried: "_list/shards"
+        listShardsRequest = getListShardsTransportRequest(new String[] { "test-closed-idx" }, 100);
+        listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
+        assertSingleIndexShardsWithStats(listShardsResponse.get(), "test-closed-idx", 2, false);
+
+        // Verify result when closed index is queried with wildcard: "_list/shards*"
+        listShardsRequest = getListShardsTransportRequest(new String[] { "test-closed-idx*" }, 100);
+        listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
+        assertSingleIndexShardsWithStats(listShardsResponse.get(), "test-closed-idx", 2, false);
+    }
+
     public void testListShardsWithClosedAndHiddenIndices() throws InterruptedException, ExecutionException {
         final int numIndices = 3;
         final int numShards = 1;
@@ -166,9 +234,9 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti
                 .put(IndexMetadata.SETTING_INDEX_HIDDEN, true)
                 .build()
         );
-        ensureGreen();
         // close index "test-closed-idx"
         client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx")).get();
+        ensureGreen();
 
         // Verifying response for default queries: /_list/shards
         // all the shards should be part of response, however stats should not be displayed for closed index
@@ -199,7 +267,7 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti
         );
 
         // Verifying responses when hidden indices are queried with wildcards: /_list/shards/test-hidden-idx*
-        // Shards for hidden index should appear in response without stats
+        // Shards for hidden index should appear in response with stats
         listShardsRequest.setIndices(List.of("test-hidden-idx*").toArray(new String[0]));
         listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
         assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx")));
@@ -227,4 +295,102 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti
         assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length);
     }
 
+    public void testListShardsWithDataStream() throws Exception {
+        final int numDataNodes = 3;
+        String dataStreamName = "logs-test";
+        internalCluster().startClusterManagerOnlyNodes(1);
+        internalCluster().startDataOnlyNodes(numDataNodes);
+        // Create an index template for data streams.
+        createDataStreamIndexTemplate("data-stream-template", List.of("logs-*"));
+        // Create data streams matching the "logs-*" index pattern.
+        createDataStream(dataStreamName);
+        ensureGreen();
+        // Verifying default query's result. Data stream should have created a hidden backing index in the
+        // background and all the corresponding shards should appear in the response along with stats.
+        CatShardsRequest listShardsRequest = getListShardsTransportRequest(Strings.EMPTY_ARRAY, numDataNodes * numDataNodes);
+        ActionFuture<CatShardsResponse> listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
+        assertSingleIndexShardsWithStats(listShardsResponse.get(), dataStreamName, numDataNodes + 1, true);
+        // Verifying result when data stream is directly queried. Again, all the shards with stats should appear
+        listShardsRequest = getListShardsTransportRequest(new String[] { dataStreamName }, numDataNodes * numDataNodes);
+        listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
+        assertSingleIndexShardsWithStats(listShardsResponse.get(), dataStreamName, numDataNodes + 1, true);
+    }
+
+    public void testListShardsWithAliases() throws Exception {
+        final int numShards = 1;
+        final int numReplicas = 1;
+        final String aliasName = "test-alias";
+        internalCluster().startClusterManagerOnlyNodes(1);
+        internalCluster().startDataOnlyNodes(3);
+        createIndex(
+            "test-closed-idx",
+            Settings.builder()
+                .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards)
+                .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
+                .build()
+        );
+        createIndex(
+            "test-hidden-idx",
+            Settings.builder()
+                .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards)
+                .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas)
+                .put(IndexMetadata.SETTING_INDEX_HIDDEN, true)
+                .build()
+        );
+        ensureGreen();
+
+        // Point test alias to both the indices (one being hidden while the other is closed)
+        final IndicesAliasesRequest request = new IndicesAliasesRequest().origin("allowed");
+        request.addAliasAction(IndicesAliasesRequest.AliasActions.add().index("test-closed-idx").alias(aliasName));
+        assertAcked(client().admin().indices().aliases(request).actionGet());
+
+        request.addAliasAction(IndicesAliasesRequest.AliasActions.add().index("test-hidden-idx").alias(aliasName));
+        assertAcked(client().admin().indices().aliases(request).actionGet());
+
+        // close index "test-closed-idx"
+        client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx")).get();
+        ensureGreen();
+
+        // Verifying result when an alias is explicitly queried.
+        CatShardsRequest listShardsRequest = getListShardsTransportRequest(new String[] { aliasName }, 100);
+        ActionFuture<CatShardsResponse> listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest);
+        assertTrue(
+            listShardsResponse.get()
+                .getResponseShards()
+                .stream()
+                .allMatch(shard -> shard.getIndexName().equals("test-hidden-idx") || shard.getIndexName().equals("test-closed-idx"))
+        );
+        assertTrue(
+            Arrays.stream(listShardsResponse.get().getIndicesStatsResponse().getShards())
+                .allMatch(shardStats -> shardStats.getShardRouting().getIndexName().equals("test-hidden-idx"))
+        );
+        assertEquals(4, listShardsResponse.get().getResponseShards().size());
+        assertEquals(2, listShardsResponse.get().getIndicesStatsResponse().getShards().length);
+    }
+
+    private void assertSingleIndexShardsWithStats(
+        CatShardsResponse catShardsResponse,
+        String indexNamePattern,
+        final int totalNumShards,
+        boolean shardStatsExist
+    ) {
+        assertTrue(catShardsResponse.getResponseShards().stream().allMatch(shard -> shard.getIndexName().contains(indexNamePattern)));
+        assertEquals(totalNumShards, catShardsResponse.getResponseShards().size());
+        if (shardStatsExist) {
+            assertTrue(
+                Arrays.stream(catShardsResponse.getIndicesStatsResponse().getShards())
+                    .allMatch(shardStats -> shardStats.getShardRouting().getIndexName().contains(indexNamePattern))
+            );
+        }
+        assertEquals(shardStatsExist ? totalNumShards : 0, catShardsResponse.getIndicesStatsResponse().getShards().length);
+    }
+
+    private CatShardsRequest getListShardsTransportRequest(String[] indices, final int pageSize) {
+        CatShardsRequest listShardsRequest = new CatShardsRequest();
+        listShardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT);
+        listShardsRequest.setIndices(indices);
+        listShardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize));
+        return listShardsRequest;
+    }
+
 }
diff --git a/...er/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/...er/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java
@@ -26,14 +26,12 @@
 import org.opensearch.common.inject.Inject;
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.action.NotifyOnceListener;
-import org.opensearch.core.common.Strings;
 import org.opensearch.tasks.CancellableTask;
 import org.opensearch.tasks.Task;
 import org.opensearch.transport.TransportService;
 
 import java.util.List;
 import java.util.Objects;
-import java.util.stream.Collectors;
 
 import static org.opensearch.common.breaker.ResponseLimitSettings.LimitEntity.SHARDS;
 
@@ -115,9 +113,7 @@ public void onResponse(ClusterStateResponse clusterStateResponse) {
 
                         String[] indices = Objects.isNull(paginationStrategy)
                             ? shardsRequest.getIndices()
-                            : filterPaginationResponse(clusterStateResponse.getState(), paginationStrategy.getRequestedIndices()).toArray(
-                                Strings.EMPTY_ARRAY
-                            );
+                            : filterClosedIndices(clusterStateResponse.getState(), paginationStrategy.getRequestedIndices());
                         // For paginated queries, if strategy outputs no shards to be returned, avoid fetching IndicesStats.
                         if (shouldSkipIndicesStatsRequest(paginationStrategy, indices)) {
                             catShardsResponse.setIndicesStatsResponse(IndicesStatsResponse.getEmptyResponse());
@@ -190,10 +186,10 @@ private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy pagination
      * IndicesStats. Since pagination strategy always passes concrete indices to TransportIndicesStatsAction,
      * the default behaviour of StrictExpandOpenAndForbidClosed leads to errors if closed indices are encountered.
      */
-    private List<String> filterPaginationResponse(ClusterState clusterState, List<String> strategyIndices) {
+    private String[] filterClosedIndices(ClusterState clusterState, List<String> strategyIndices) {
         return strategyIndices.stream().filter(index -> {
             IndexMetadata metadata = clusterState.metadata().indices().get(index);
-            return metadata != null && metadata.getState().equals(IndexMetadata.State.OPEN);
-        }).collect(Collectors.toList());
+            return metadata != null && metadata.getState().equals(IndexMetadata.State.CLOSE) == false;
+        }).toArray(String[]::new);
     }
 }
diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java
@@ -32,6 +32,7 @@
 
 package org.opensearch.action.admin.indices.stats;
 
+import org.opensearch.Version;
 import org.opensearch.action.support.broadcast.BroadcastRequest;
 import org.opensearch.common.annotation.PublicApi;
 import org.opensearch.core.common.io.stream.StreamInput;
@@ -66,6 +67,9 @@ public IndicesStatsRequest() {
     public IndicesStatsRequest(StreamInput in) throws IOException {
         super(in);
         flags = new CommonStatsFlags(in);
+        if (in.getVersion().onOrAfter(Version.V_3_0_0)) {
+            skipIndexNameResolver = in.readBoolean();
+        }
     }
 
     /**
@@ -302,6 +306,9 @@ public IndicesStatsRequest includeUnloadedSegments(boolean includeUnloadedSegmen
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
         flags.writeTo(out);
+        if (out.getVersion().onOrAfter(Version.V_3_0_0)) {
+            out.writeOptionalBoolean(skipIndexNameResolver);
+        }
     }
 
     @Override

diff --git a/.../src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java b/.../src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java
@@ -56,6 +56,7 @@
 import org.opensearch.transport.TransportService;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -157,7 +158,10 @@ protected ShardStats shardOperation(IndicesStatsRequest request, ShardRouting sh
     @Override
     protected String[] resolveConcreteIndexNames(ClusterState clusterState, IndicesStatsRequest request) {
         if (request.skipIndexNameResolver()) {
-            return request.indices();
+            // filter out all the indices which might not be present in the local clusterState
+            return Arrays.stream(request.indices())
+                .filter(index -> clusterState.metadata().indices().containsKey(index))
+                .toArray(String[]::new);
         }
         return super.resolveConcreteIndexNames(clusterState, request);
     }