From 05e02d204b802a3dcc6ea91eb75e821a073561f3 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Fri, 29 Nov 2024 10:02:14 +0800 Subject: [PATCH 01/21] Use _list/indices API instead of _cat/index API in CatIndexTool Signed-off-by: zane-neo --- .../ml/engine/tools/CatIndexTool.java | 287 ++++++++++++------ .../ml/engine/tools/CatIndexToolTests.java | 4 +- .../opensearch/ml/tools/CatIndexToolIT.java | 98 ++++++ .../ml/tools/CatIndexAgentRegistration.json | 19 ++ 4 files changed, 320 insertions(+), 88 deletions(-) create mode 100644 plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java create mode 100644 plugin/src/test/resources/org/opensearch/ml/tools/CatIndexAgentRegistration.json diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java index 3e28366e40..9b99fc9607 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java @@ -10,10 +10,14 @@ import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; +import java.util.Queue; import java.util.Spliterators; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.stream.Collectors; @@ -30,6 +34,9 @@ import org.opensearch.action.admin.indices.stats.IndexStats; import org.opensearch.action.admin.indices.stats.IndicesStatsRequest; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; +import org.opensearch.action.pagination.IndexPaginationStrategy; +import org.opensearch.action.pagination.PageParams; +import org.opensearch.action.pagination.PageToken; import org.opensearch.action.support.GroupedActionListener; import org.opensearch.action.support.IndicesOptions; import org.opensearch.cluster.health.ClusterIndexHealth; @@ -37,6 +44,7 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.Table; import org.opensearch.common.Table.Cell; +import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.action.ActionListener; @@ -50,10 +58,15 @@ import lombok.Getter; import lombok.Setter; +import lombok.extern.slf4j.Slf4j; +@Slf4j @ToolAnnotation(CatIndexTool.TYPE) public class CatIndexTool implements Tool { public static final String TYPE = "CatIndexTool"; + // This needs to be changed once it's changed in opensearch core in RestIndicesListAction. + private static final int MAX_SUPPORTED_LIST_INDICES_PAGE_SIZE = 5000; + private static final int DEFAULT_PAGE_SIZE = 1; private static final String DEFAULT_DESCRIPTION = String .join( " ", @@ -106,13 +119,13 @@ public void run(Map parameters, ActionListener listener) final String[] indices = indexList.toArray(Strings.EMPTY_ARRAY); final IndicesOptions indicesOptions = IndicesOptions.strictExpand(); - final boolean local = parameters.containsKey("local") ? Boolean.parseBoolean("local") : false; - final TimeValue clusterManagerNodeTimeout = DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT; + final boolean local = parameters.containsKey("local") && Boolean.parseBoolean(parameters.get("local")); final boolean includeUnloadedSegments = Boolean.parseBoolean(parameters.get("include_unloaded_segments")); + final PageParams pageParams = new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, DEFAULT_PAGE_SIZE); final ActionListener internalListener = ActionListener.notifyOnce(ActionListener.wrap(table -> { // Handle empty table - if (table.getRows().isEmpty()) { + if (table == null || table.getRows().isEmpty()) { @SuppressWarnings("unchecked") T empty = (T) ("There were no results searching the indices parameter [" + parameters.get("indices") + "]."); listener.onResponse(empty); @@ -131,57 +144,150 @@ public void run(Map parameters, ActionListener listener) listener.onResponse(response); }, listener::onFailure)); - sendGetSettingsRequest( + fetchClusterInfoAndPages( indices, + local, + includeUnloadedSegments, + pageParams, indicesOptions, + new ConcurrentLinkedQueue<>(), + internalListener + ); + } + + private void fetchClusterInfoAndPages( + String[] indices, + boolean local, + boolean includeUnloadedSegments, + PageParams pageParams, + IndicesOptions indicesOptions, + Queue> pageResults, + ActionListener
originalListener + ) { + // First fetch metadata like index setting and cluster states and then fetch index details in batches to save efforts. + sendGetSettingsRequest(indices, indicesOptions, local, client, new ActionListener<>() { + @Override + public void onResponse(final GetSettingsResponse getSettingsResponse) { + // The list of indices that will be returned is determined by the indices returned from the Get Settings call. + // All the other requests just provide additional detail, and wildcards may be resolved differently depending on the + // type of request in the presence of security plugins (looking at you, ClusterHealthRequest), so + // force the IndicesOptions for all the sub-requests to be as inclusive as possible. + final IndicesOptions subRequestIndicesOptions = IndicesOptions.lenientExpandHidden(); + // Indices that were successfully resolved during the get settings request might be deleted when the + // subsequent cluster state, cluster health and indices stats requests execute. We have to distinguish two cases: + // 1) the deleted index was explicitly passed as parameter to the /_cat/indices request. In this case we + // want the subsequent requests to fail. + // 2) the deleted index was resolved as part of a wildcard or _all. In this case, we want the subsequent + // requests not to fail on the deleted index (as we want to ignore wildcards that cannot be resolved). + // This behavior can be ensured by letting the cluster state, cluster health and indices stats requests + // re-resolve the index names with the same indices options that we used for the initial cluster state + // request (strictExpand). + sendClusterStateRequest(indices, subRequestIndicesOptions, local, client, new ActionListener<>() { + @Override + public void onResponse(ClusterStateResponse clusterStateResponse) { + // Starts to fetch index details here, if a batch fails build whatever we have and return. + fetchPages( + indices, + local, + DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT, + includeUnloadedSegments, + pageParams, + pageResults, + clusterStateResponse, + getSettingsResponse, + subRequestIndicesOptions, + originalListener + ); + } + + @Override + public void onFailure(final Exception e) { + originalListener.onFailure(e); + } + }); + } + + @Override + public void onFailure(final Exception e) { + originalListener.onFailure(e); + } + }); + } + + private void fetchPages( + String[] indices, + boolean local, + TimeValue clusterManagerNodeTimeout, + boolean includeUnloadedSegments, + PageParams pageParams, + Queue> pageResults, + ClusterStateResponse clusterStateResponse, + GetSettingsResponse getSettingsResponse, + IndicesOptions subRequestIndicesOptions, + ActionListener
originalListener + ) { + final ActionListener iterativeListener = ActionListener.wrap(r -> { + // when previous response returns, build next request with response and invoke again. + PageParams nextPageParams = new PageParams(r.getNextToken(), pageParams.getSort(), pageParams.getSize()); + // when next page doesn't exist or reaches max supported page size, return. + if (r.getNextToken() == null || pageResults.size() >= MAX_SUPPORTED_LIST_INDICES_PAGE_SIZE) { + Table table = buildTable(clusterStateResponse, getSettingsResponse, pageResults); + originalListener.onResponse(table); + } else { + fetchPages( + indices, + local, + clusterManagerNodeTimeout, + includeUnloadedSegments, + nextPageParams, + pageResults, + clusterStateResponse, + getSettingsResponse, + subRequestIndicesOptions, + originalListener + ); + } + }, e -> { + log.error("Failed to fetch index info for page: {}", pageParams.getRequestedToken()); + // Do not throw the exception, just return whatever we have. + originalListener.onResponse(buildTable(clusterStateResponse, getSettingsResponse, pageResults)); + }); + IndexPaginationStrategy paginationStrategy = getPaginationStrategy(pageParams, clusterStateResponse); + // For non-paginated queries, indicesToBeQueried would be same as indices retrieved from + // rest request and unresolved, while for paginated queries, it would be a list of indices + // already resolved by ClusterStateRequest and to be displayed in a page. + final String[] indicesToBeQueried = Objects.isNull(paginationStrategy) + ? indices + : paginationStrategy.getRequestedEntities().toArray(new String[0]); + // After the group listener returns, one page complete and prepare for next page. + final GroupedActionListener groupedListener = createGroupedListener( + pageResults, + paginationStrategy.getResponseToken(), + iterativeListener + ); + + sendIndicesStatsRequest( + indicesToBeQueried, + subRequestIndicesOptions, + includeUnloadedSegments, + client, + ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) + ); + + sendClusterHealthRequest( + indicesToBeQueried, + subRequestIndicesOptions, local, clusterManagerNodeTimeout, client, - new ActionListener() { - @Override - public void onResponse(final GetSettingsResponse getSettingsResponse) { - final GroupedActionListener groupedListener = createGroupedListener(4, internalListener); - groupedListener.onResponse(getSettingsResponse); - - // The list of indices that will be returned is determined by the indices returned from the Get Settings call. - // All the other requests just provide additional detail, and wildcards may be resolved differently depending on the - // type of request in the presence of security plugins (looking at you, ClusterHealthRequest), so - // force the IndicesOptions for all the sub-requests to be as inclusive as possible. - final IndicesOptions subRequestIndicesOptions = IndicesOptions.lenientExpandHidden(); - - sendIndicesStatsRequest( - indices, - subRequestIndicesOptions, - includeUnloadedSegments, - client, - ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) - ); - sendClusterStateRequest( - indices, - subRequestIndicesOptions, - local, - clusterManagerNodeTimeout, - client, - ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) - ); - sendClusterHealthRequest( - indices, - subRequestIndicesOptions, - local, - clusterManagerNodeTimeout, - client, - ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) - ); - } - - @Override - public void onFailure(final Exception e) { - internalListener.onFailure(e); - } - } + ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) ); } + protected IndexPaginationStrategy getPaginationStrategy(PageParams pageParams, ClusterStateResponse clusterStateResponse) { + return new IndexPaginationStrategy(pageParams, clusterStateResponse.getState()); + } + @Override public String getType() { return TYPE; @@ -199,7 +305,6 @@ private void sendGetSettingsRequest( final String[] indices, final IndicesOptions indicesOptions, final boolean local, - final TimeValue clusterManagerNodeTimeout, final Client client, final ActionListener listener ) { @@ -207,7 +312,7 @@ private void sendGetSettingsRequest( request.indices(indices); request.indicesOptions(indicesOptions); request.local(local); - request.clusterManagerNodeTimeout(clusterManagerNodeTimeout); + request.clusterManagerNodeTimeout(DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT); request.names(IndexSettings.INDEX_SEARCH_THROTTLED.getKey()); client.admin().indices().getSettings(request, listener); @@ -217,7 +322,6 @@ private void sendClusterStateRequest( final String[] indices, final IndicesOptions indicesOptions, final boolean local, - final TimeValue clusterManagerNodeTimeout, final Client client, final ActionListener listener ) { @@ -226,7 +330,7 @@ private void sendClusterStateRequest( request.indices(indices); request.indicesOptions(indicesOptions); request.local(local); - request.clusterManagerNodeTimeout(clusterManagerNodeTimeout); + request.clusterManagerNodeTimeout(DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT); client.admin().cluster().state(request, listener); } @@ -266,39 +370,24 @@ private void sendIndicesStatsRequest( client.admin().indices().stats(request, listener); } - private GroupedActionListener createGroupedListener(final int size, final ActionListener
listener) { - return new GroupedActionListener<>(new ActionListener>() { + // group listener only accept two action response: IndicesStatsResponse and ClusterHealthResponse + private GroupedActionListener createGroupedListener( + final Queue> pageResults, + final PageToken pageToken, + final ActionListener listener + ) { + return new GroupedActionListener<>(new ActionListener<>() { @Override public void onResponse(final Collection responses) { - try { - GetSettingsResponse settingsResponse = extractResponse(responses, GetSettingsResponse.class); - Map indicesSettings = StreamSupport - .stream(Spliterators.spliterator(settingsResponse.getIndexToSettings().entrySet(), 0), false) - .collect(Collectors.toMap(cursor -> cursor.getKey(), cursor -> cursor.getValue())); - - ClusterStateResponse stateResponse = extractResponse(responses, ClusterStateResponse.class); - Map indicesStates = StreamSupport - .stream(stateResponse.getState().getMetadata().spliterator(), false) - .collect(Collectors.toMap(indexMetadata -> indexMetadata.getIndex().getName(), Function.identity())); - - ClusterHealthResponse healthResponse = extractResponse(responses, ClusterHealthResponse.class); - Map indicesHealths = healthResponse.getIndices(); - - IndicesStatsResponse statsResponse = extractResponse(responses, IndicesStatsResponse.class); - Map indicesStats = statsResponse.getIndices(); - - Table responseTable = buildTable(indicesSettings, indicesHealths, indicesStats, indicesStates); - listener.onResponse(responseTable); - } catch (Exception e) { - onFailure(e); - } + pageResults.add(responses); + listener.onResponse(pageToken); } @Override public void onFailure(final Exception e) { listener.onFailure(e); } - }, size); + }, 2); } @Override @@ -396,21 +485,34 @@ private Table getTableWithHeader() { } private Table buildTable( - final Map indicesSettings, - final Map indicesHealths, - final Map indicesStats, - final Map indicesMetadatas + ClusterStateResponse clusterStateResponse, + GetSettingsResponse getSettingsResponse, + Queue> responses ) { + if (responses == null || responses.isEmpty()) { + return null; + } + Tuple, Map> tuple = aggregateResults(responses); final Table table = getTableWithHeader(); AtomicInteger rowNum = new AtomicInteger(0); + Map indicesSettings = StreamSupport + .stream(Spliterators.spliterator(getSettingsResponse.getIndexToSettings().entrySet(), 0), false) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + + Map indicesStates = StreamSupport + .stream(clusterStateResponse.getState().getMetadata().spliterator(), false) + .collect(Collectors.toMap(indexMetadata -> indexMetadata.getIndex().getName(), Function.identity())); + + Map indicesHealths = tuple.v2(); + Map indicesStats = tuple.v1(); indicesSettings.forEach((indexName, settings) -> { - if (!indicesMetadatas.containsKey(indexName)) { + if (!indicesStates.containsKey(indexName)) { // the index exists in the Get Indices response but is not present in the cluster state: // it is likely that the index was deleted in the meanwhile, so we ignore it. return; } - final IndexMetadata indexMetadata = indicesMetadatas.get(indexName); + final IndexMetadata indexMetadata = indicesStates.get(indexName); final IndexMetadata.State indexState = indexMetadata.getState(); final IndexStats indexStats = indicesStats.get(indexName); @@ -448,15 +550,28 @@ private Table buildTable( table.addCell(totalStats.getStore() == null ? null : totalStats.getStore().size()); table.addCell(primaryStats.getStore() == null ? null : primaryStats.getStore().size()); - table.endRow(); }); - return table; } - @SuppressWarnings("unchecked") - private static A extractResponse(final Collection responses, Class c) { - return (A) responses.stream().filter(c::isInstance).findFirst().get(); + private Tuple, Map> aggregateResults(Queue> responses) { + // Each batch produces a collection of action response, aggregate them together to build table easier. + Map indexStatsMap = new HashMap<>(); + Map clusterIndexHealthMap = new HashMap<>(); + for (Collection response : responses) { + if (response != null && !response.isEmpty()) { + response.forEach(x -> { + if (x instanceof IndicesStatsResponse) { + indexStatsMap.putAll(((IndicesStatsResponse) x).getIndices()); + } else if (x instanceof ClusterHealthResponse) { + clusterIndexHealthMap.putAll(((ClusterHealthResponse) x).getIndices()); + } else { + throw new IllegalStateException("Unexpected action response type: " + x.getClass().getName()); + } + }); + } + } + return new Tuple<>(indexStatsMap, clusterIndexHealthMap); } } diff --git a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/CatIndexToolTests.java b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/CatIndexToolTests.java index 31dd909d1a..26f9492c85 100644 --- a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/CatIndexToolTests.java +++ b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/CatIndexToolTests.java @@ -150,8 +150,8 @@ public void testRunAsyncNoIndices() throws Exception { tool.run(otherParams, listener); settingsActionListenerCaptor.getValue().onResponse(getSettingsResponse); - statsActionListenerCaptor.getValue().onResponse(indicesStatsResponse); clusterStateActionListenerCaptor.getValue().onResponse(clusterStateResponse); + statsActionListenerCaptor.getValue().onResponse(indicesStatsResponse); clusterHealthActionListenerCaptor.getValue().onResponse(clusterHealthResponse); future.join(); @@ -214,8 +214,8 @@ public void testRunAsyncIndexStats() throws Exception { tool.run(otherParams, listener); settingsActionListenerCaptor.getValue().onResponse(getSettingsResponse); - statsActionListenerCaptor.getValue().onResponse(indicesStatsResponse); clusterStateActionListenerCaptor.getValue().onResponse(clusterStateResponse); + statsActionListenerCaptor.getValue().onResponse(indicesStatsResponse); clusterHealthActionListenerCaptor.getValue().onResponse(clusterHealthResponse); future.orTimeout(10, TimeUnit.SECONDS).join(); diff --git a/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java b/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java new file mode 100644 index 0000000000..7dc21694e0 --- /dev/null +++ b/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java @@ -0,0 +1,98 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.ml.tools; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; + +import org.junit.Before; +import org.opensearch.client.Response; +import org.opensearch.common.settings.Settings; +import org.opensearch.ml.rest.RestBaseAgentToolsIT; +import org.opensearch.ml.utils.TestHelper; + +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; +import com.google.gson.JsonParser; + +import lombok.extern.log4j.Log4j2; + +@Log4j2 +public class CatIndexToolIT extends RestBaseAgentToolsIT { + private String agentId; + private final String question = "{\"parameters\":{\"question\":\"please help list all the index status in the current cluster?\"}}"; + + @Before + public void setUpCluster() throws Exception { + registerCatIndexFlowAgent(); + } + + private List createIndices(int count) throws IOException { + List indices = new ArrayList<>(); + for (int i = 0; i < count; i++) { + String indexName = "test" + i; + createIndex(indexName, Settings.EMPTY); + indices.add(indexName); + } + return indices; + } + + private void registerCatIndexFlowAgent() throws Exception { + String requestBody = Files + .readString( + Path.of(this.getClass().getClassLoader().getResource("org/opensearch/ml/tools/CatIndexAgentRegistration.json").toURI()) + ); + registerMLAgent(client(), requestBody, response -> agentId = (String) response.get("agent_id")); + } + + public void testCatIndexWithFewIndices() throws IOException { + List indices = createIndices(10); + Response response = TestHelper.makeRequest(client(), "POST", "/_plugins/_ml/agents/" + agentId + "/_execute", null, question, null); + String responseStr = TestHelper.httpEntityToString(response.getEntity()); + String toolOutput = extractResult(responseStr); + String[] actualLines = toolOutput.split("\\n"); + // plus 2 as there are one line of header and one line of system agent index, but sometimes the ml-config index will be created + // then there will be one more line. + assert actualLines.length == indices.size() + 2 || actualLines.length == indices.size() + 3; + for (String index : indices) { + assert Objects.requireNonNull(toolOutput).contains(index); + } + } + + public void testCatIndexWithMoreThan100Indices() throws IOException { + List indices = createIndices(101); + Response response = TestHelper.makeRequest(client(), "POST", "/_plugins/_ml/agents/" + agentId + "/_execute", null, question, null); + String responseStr = TestHelper.httpEntityToString(response.getEntity()); + String toolOutput = extractResult(responseStr); + String[] actualLines = toolOutput.split("\\n"); + assert actualLines.length == indices.size() + 2 || actualLines.length == indices.size() + 3; + for (String index : indices) { + assert Objects.requireNonNull(toolOutput).contains(index); + } + } + + private String extractResult(String responseStr) { + JsonArray output = JsonParser + .parseString(responseStr) + .getAsJsonObject() + .get("inference_results") + .getAsJsonArray() + .get(0) + .getAsJsonObject() + .get("output") + .getAsJsonArray(); + for (JsonElement element : output) { + if ("response".equals(element.getAsJsonObject().get("name").getAsString())) { + return element.getAsJsonObject().get("result").getAsString(); + } + } + return null; + } +} diff --git a/plugin/src/test/resources/org/opensearch/ml/tools/CatIndexAgentRegistration.json b/plugin/src/test/resources/org/opensearch/ml/tools/CatIndexAgentRegistration.json new file mode 100644 index 0000000000..e2b41cafad --- /dev/null +++ b/plugin/src/test/resources/org/opensearch/ml/tools/CatIndexAgentRegistration.json @@ -0,0 +1,19 @@ +{ + "name": "list index tool flow agent", + "type": "flow", + "description": "this is a test agent", + "llm": { + "model_id": "dummy_model", + "parameters": { + "max_iteration": 5, + "stop_when_no_tool_found": true + } + }, + "tools": [ + { + "type": "CatIndexTool", + "name": "CatIndexTool" + } + ], + "app_type": "my_app" +} From a8623cced182882f5dcd25ab6127921f9f15d69c Mon Sep 17 00:00:00 2001 From: zane-neo Date: Fri, 29 Nov 2024 10:11:46 +0800 Subject: [PATCH 02/21] change assert to avoid flaky in test Signed-off-by: zane-neo --- .../java/org/opensearch/ml/tools/CatIndexToolIT.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java b/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java index 7dc21694e0..625a66aab3 100644 --- a/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java +++ b/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java @@ -9,6 +9,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Objects; @@ -58,9 +59,8 @@ public void testCatIndexWithFewIndices() throws IOException { String responseStr = TestHelper.httpEntityToString(response.getEntity()); String toolOutput = extractResult(responseStr); String[] actualLines = toolOutput.split("\\n"); - // plus 2 as there are one line of header and one line of system agent index, but sometimes the ml-config index will be created - // then there will be one more line. - assert actualLines.length == indices.size() + 2 || actualLines.length == indices.size() + 3; + long testIndexCount = Arrays.stream(actualLines).filter(x -> x.contains("test")).count(); + assert testIndexCount == indices.size(); for (String index : indices) { assert Objects.requireNonNull(toolOutput).contains(index); } @@ -72,7 +72,8 @@ public void testCatIndexWithMoreThan100Indices() throws IOException { String responseStr = TestHelper.httpEntityToString(response.getEntity()); String toolOutput = extractResult(responseStr); String[] actualLines = toolOutput.split("\\n"); - assert actualLines.length == indices.size() + 2 || actualLines.length == indices.size() + 3; + long testIndexCount = Arrays.stream(actualLines).filter(x -> x.contains("test")).count(); + assert testIndexCount == indices.size(); for (String index : indices) { assert Objects.requireNonNull(toolOutput).contains(index); } From db1f5173b0d7a4c59ddac0d8d3fae0261be449b4 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Wed, 4 Dec 2024 15:48:48 +0800 Subject: [PATCH 03/21] add example of responsestr and change default size to 100 Signed-off-by: zane-neo --- .../ml/engine/tools/CatIndexTool.java | 2 +- .../opensearch/ml/tools/CatIndexToolIT.java | 22 +++++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java index 9b99fc9607..93c25e73c5 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java @@ -66,7 +66,7 @@ public class CatIndexTool implements Tool { public static final String TYPE = "CatIndexTool"; // This needs to be changed once it's changed in opensearch core in RestIndicesListAction. private static final int MAX_SUPPORTED_LIST_INDICES_PAGE_SIZE = 5000; - private static final int DEFAULT_PAGE_SIZE = 1; + public static final int DEFAULT_PAGE_SIZE = 100; private static final String DEFAULT_DESCRIPTION = String .join( " ", diff --git a/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java b/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java index 625a66aab3..0a52c68807 100644 --- a/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java +++ b/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java @@ -16,6 +16,7 @@ import org.junit.Before; import org.opensearch.client.Response; import org.opensearch.common.settings.Settings; +import org.opensearch.ml.engine.tools.CatIndexTool; import org.opensearch.ml.rest.RestBaseAgentToolsIT; import org.opensearch.ml.utils.TestHelper; @@ -54,7 +55,7 @@ private void registerCatIndexFlowAgent() throws Exception { } public void testCatIndexWithFewIndices() throws IOException { - List indices = createIndices(10); + List indices = createIndices(CatIndexTool.DEFAULT_PAGE_SIZE); Response response = TestHelper.makeRequest(client(), "POST", "/_plugins/_ml/agents/" + agentId + "/_execute", null, question, null); String responseStr = TestHelper.httpEntityToString(response.getEntity()); String toolOutput = extractResult(responseStr); @@ -67,7 +68,7 @@ public void testCatIndexWithFewIndices() throws IOException { } public void testCatIndexWithMoreThan100Indices() throws IOException { - List indices = createIndices(101); + List indices = createIndices(CatIndexTool.DEFAULT_PAGE_SIZE + 1); Response response = TestHelper.makeRequest(client(), "POST", "/_plugins/_ml/agents/" + agentId + "/_execute", null, question, null); String responseStr = TestHelper.httpEntityToString(response.getEntity()); String toolOutput = extractResult(responseStr); @@ -79,6 +80,23 @@ public void testCatIndexWithMoreThan100Indices() throws IOException { } } + /** + * An example of responseStr: + * { + * "inference_results": [ + * { + * "output": [ + * { + * "name": "response", + * "result": "row,health,status,index,uuid,pri(number of primary shards),rep(number of replica shards),docs.count(number of available documents),docs.deleted(number of deleted documents),store.size(store size of primary and replica shards),pri.store.size(store size of primary shards)\n1,yellow,open,test4,6ohWskucQ3u3xV9tMjXCkA,1,1,0,0,208b,208b\n2,yellow,open,test5,5AQLe-Z3QKyyLibbZ3Xcng,1,1,0,0,208b,208b\n3,yellow,open,test2,66Cj3zjlQ-G8I3vWeEONpQ,1,1,0,0,208b,208b\n4,yellow,open,test3,6A-aVxPiTj2U9GnupHQ3BA,1,1,0,0,208b,208b\n5,yellow,open,test8,-WKw-SCET3aTFuWCMMixrw,1,1,0,0,208b,208b" + * } + * ] + * } + * ] + * } + * @param responseStr + * @return + */ private String extractResult(String responseStr) { JsonArray output = JsonParser .parseString(responseStr) From ed4bb46ea9a2d16aefeeebf34e1b2ae15b7237bf Mon Sep 17 00:00:00 2001 From: zane-neo Date: Thu, 5 Dec 2024 08:55:12 +0800 Subject: [PATCH 04/21] add ListIndexTool and revert CatIndexTool change Signed-off-by: zane-neo --- .../ml/engine/tools/CatIndexTool.java | 289 +++------ .../ml/engine/tools/ListIndexTool.java | 577 ++++++++++++++++++ .../ml/engine/tools/CatIndexToolTests.java | 4 +- ...tIndexToolIT.java => ListIndexToolIT.java} | 8 +- 4 files changed, 670 insertions(+), 208 deletions(-) create mode 100644 ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java rename plugin/src/test/java/org/opensearch/ml/tools/{CatIndexToolIT.java => ListIndexToolIT.java} (94%) diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java index 93c25e73c5..d47d294b0f 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java @@ -10,14 +10,10 @@ import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Objects; -import java.util.Queue; import java.util.Spliterators; -import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.stream.Collectors; @@ -34,9 +30,6 @@ import org.opensearch.action.admin.indices.stats.IndexStats; import org.opensearch.action.admin.indices.stats.IndicesStatsRequest; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; -import org.opensearch.action.pagination.IndexPaginationStrategy; -import org.opensearch.action.pagination.PageParams; -import org.opensearch.action.pagination.PageToken; import org.opensearch.action.support.GroupedActionListener; import org.opensearch.action.support.IndicesOptions; import org.opensearch.cluster.health.ClusterIndexHealth; @@ -44,7 +37,6 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.Table; import org.opensearch.common.Table.Cell; -import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.action.ActionListener; @@ -58,15 +50,10 @@ import lombok.Getter; import lombok.Setter; -import lombok.extern.slf4j.Slf4j; -@Slf4j @ToolAnnotation(CatIndexTool.TYPE) public class CatIndexTool implements Tool { public static final String TYPE = "CatIndexTool"; - // This needs to be changed once it's changed in opensearch core in RestIndicesListAction. - private static final int MAX_SUPPORTED_LIST_INDICES_PAGE_SIZE = 5000; - public static final int DEFAULT_PAGE_SIZE = 100; private static final String DEFAULT_DESCRIPTION = String .join( " ", @@ -119,13 +106,13 @@ public void run(Map parameters, ActionListener listener) final String[] indices = indexList.toArray(Strings.EMPTY_ARRAY); final IndicesOptions indicesOptions = IndicesOptions.strictExpand(); - final boolean local = parameters.containsKey("local") && Boolean.parseBoolean(parameters.get("local")); + final boolean local = parameters.containsKey("local") ? Boolean.parseBoolean("local") : false; + final TimeValue clusterManagerNodeTimeout = DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT; final boolean includeUnloadedSegments = Boolean.parseBoolean(parameters.get("include_unloaded_segments")); - final PageParams pageParams = new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, DEFAULT_PAGE_SIZE); final ActionListener
internalListener = ActionListener.notifyOnce(ActionListener.wrap(table -> { // Handle empty table - if (table == null || table.getRows().isEmpty()) { + if (table.getRows().isEmpty()) { @SuppressWarnings("unchecked") T empty = (T) ("There were no results searching the indices parameter [" + parameters.get("indices") + "]."); listener.onResponse(empty); @@ -144,148 +131,55 @@ public void run(Map parameters, ActionListener listener) listener.onResponse(response); }, listener::onFailure)); - fetchClusterInfoAndPages( + sendGetSettingsRequest( indices, - local, - includeUnloadedSegments, - pageParams, indicesOptions, - new ConcurrentLinkedQueue<>(), - internalListener - ); - } - - private void fetchClusterInfoAndPages( - String[] indices, - boolean local, - boolean includeUnloadedSegments, - PageParams pageParams, - IndicesOptions indicesOptions, - Queue> pageResults, - ActionListener
originalListener - ) { - // First fetch metadata like index setting and cluster states and then fetch index details in batches to save efforts. - sendGetSettingsRequest(indices, indicesOptions, local, client, new ActionListener<>() { - @Override - public void onResponse(final GetSettingsResponse getSettingsResponse) { - // The list of indices that will be returned is determined by the indices returned from the Get Settings call. - // All the other requests just provide additional detail, and wildcards may be resolved differently depending on the - // type of request in the presence of security plugins (looking at you, ClusterHealthRequest), so - // force the IndicesOptions for all the sub-requests to be as inclusive as possible. - final IndicesOptions subRequestIndicesOptions = IndicesOptions.lenientExpandHidden(); - // Indices that were successfully resolved during the get settings request might be deleted when the - // subsequent cluster state, cluster health and indices stats requests execute. We have to distinguish two cases: - // 1) the deleted index was explicitly passed as parameter to the /_cat/indices request. In this case we - // want the subsequent requests to fail. - // 2) the deleted index was resolved as part of a wildcard or _all. In this case, we want the subsequent - // requests not to fail on the deleted index (as we want to ignore wildcards that cannot be resolved). - // This behavior can be ensured by letting the cluster state, cluster health and indices stats requests - // re-resolve the index names with the same indices options that we used for the initial cluster state - // request (strictExpand). - sendClusterStateRequest(indices, subRequestIndicesOptions, local, client, new ActionListener<>() { - @Override - public void onResponse(ClusterStateResponse clusterStateResponse) { - // Starts to fetch index details here, if a batch fails build whatever we have and return. - fetchPages( - indices, - local, - DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT, - includeUnloadedSegments, - pageParams, - pageResults, - clusterStateResponse, - getSettingsResponse, - subRequestIndicesOptions, - originalListener - ); - } - - @Override - public void onFailure(final Exception e) { - originalListener.onFailure(e); - } - }); - } - - @Override - public void onFailure(final Exception e) { - originalListener.onFailure(e); - } - }); - } - - private void fetchPages( - String[] indices, - boolean local, - TimeValue clusterManagerNodeTimeout, - boolean includeUnloadedSegments, - PageParams pageParams, - Queue> pageResults, - ClusterStateResponse clusterStateResponse, - GetSettingsResponse getSettingsResponse, - IndicesOptions subRequestIndicesOptions, - ActionListener
originalListener - ) { - final ActionListener iterativeListener = ActionListener.wrap(r -> { - // when previous response returns, build next request with response and invoke again. - PageParams nextPageParams = new PageParams(r.getNextToken(), pageParams.getSort(), pageParams.getSize()); - // when next page doesn't exist or reaches max supported page size, return. - if (r.getNextToken() == null || pageResults.size() >= MAX_SUPPORTED_LIST_INDICES_PAGE_SIZE) { - Table table = buildTable(clusterStateResponse, getSettingsResponse, pageResults); - originalListener.onResponse(table); - } else { - fetchPages( - indices, - local, - clusterManagerNodeTimeout, - includeUnloadedSegments, - nextPageParams, - pageResults, - clusterStateResponse, - getSettingsResponse, - subRequestIndicesOptions, - originalListener - ); - } - }, e -> { - log.error("Failed to fetch index info for page: {}", pageParams.getRequestedToken()); - // Do not throw the exception, just return whatever we have. - originalListener.onResponse(buildTable(clusterStateResponse, getSettingsResponse, pageResults)); - }); - IndexPaginationStrategy paginationStrategy = getPaginationStrategy(pageParams, clusterStateResponse); - // For non-paginated queries, indicesToBeQueried would be same as indices retrieved from - // rest request and unresolved, while for paginated queries, it would be a list of indices - // already resolved by ClusterStateRequest and to be displayed in a page. - final String[] indicesToBeQueried = Objects.isNull(paginationStrategy) - ? indices - : paginationStrategy.getRequestedEntities().toArray(new String[0]); - // After the group listener returns, one page complete and prepare for next page. - final GroupedActionListener groupedListener = createGroupedListener( - pageResults, - paginationStrategy.getResponseToken(), - iterativeListener - ); - - sendIndicesStatsRequest( - indicesToBeQueried, - subRequestIndicesOptions, - includeUnloadedSegments, - client, - ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) - ); - - sendClusterHealthRequest( - indicesToBeQueried, - subRequestIndicesOptions, local, clusterManagerNodeTimeout, client, - ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) - ); - } + new ActionListener() { + @Override + public void onResponse(final GetSettingsResponse getSettingsResponse) { + final GroupedActionListener groupedListener = createGroupedListener(4, internalListener); + groupedListener.onResponse(getSettingsResponse); + + // The list of indices that will be returned is determined by the indices returned from the Get Settings call. + // All the other requests just provide additional detail, and wildcards may be resolved differently depending on the + // type of request in the presence of security plugins (looking at you, ClusterHealthRequest), so + // force the IndicesOptions for all the sub-requests to be as inclusive as possible. + final IndicesOptions subRequestIndicesOptions = IndicesOptions.lenientExpandHidden(); + + sendIndicesStatsRequest( + indices, + subRequestIndicesOptions, + includeUnloadedSegments, + client, + ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) + ); + sendClusterStateRequest( + indices, + subRequestIndicesOptions, + local, + clusterManagerNodeTimeout, + client, + ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) + ); + sendClusterHealthRequest( + indices, + subRequestIndicesOptions, + local, + clusterManagerNodeTimeout, + client, + ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) + ); + } - protected IndexPaginationStrategy getPaginationStrategy(PageParams pageParams, ClusterStateResponse clusterStateResponse) { - return new IndexPaginationStrategy(pageParams, clusterStateResponse.getState()); + @Override + public void onFailure(final Exception e) { + internalListener.onFailure(e); + } + } + ); } @Override @@ -305,6 +199,7 @@ private void sendGetSettingsRequest( final String[] indices, final IndicesOptions indicesOptions, final boolean local, + final TimeValue clusterManagerNodeTimeout, final Client client, final ActionListener listener ) { @@ -312,7 +207,7 @@ private void sendGetSettingsRequest( request.indices(indices); request.indicesOptions(indicesOptions); request.local(local); - request.clusterManagerNodeTimeout(DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT); + request.clusterManagerNodeTimeout(clusterManagerNodeTimeout); request.names(IndexSettings.INDEX_SEARCH_THROTTLED.getKey()); client.admin().indices().getSettings(request, listener); @@ -322,6 +217,7 @@ private void sendClusterStateRequest( final String[] indices, final IndicesOptions indicesOptions, final boolean local, + final TimeValue clusterManagerNodeTimeout, final Client client, final ActionListener listener ) { @@ -330,7 +226,7 @@ private void sendClusterStateRequest( request.indices(indices); request.indicesOptions(indicesOptions); request.local(local); - request.clusterManagerNodeTimeout(DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT); + request.clusterManagerNodeTimeout(clusterManagerNodeTimeout); client.admin().cluster().state(request, listener); } @@ -370,24 +266,39 @@ private void sendIndicesStatsRequest( client.admin().indices().stats(request, listener); } - // group listener only accept two action response: IndicesStatsResponse and ClusterHealthResponse - private GroupedActionListener createGroupedListener( - final Queue> pageResults, - final PageToken pageToken, - final ActionListener listener - ) { - return new GroupedActionListener<>(new ActionListener<>() { + private GroupedActionListener createGroupedListener(final int size, final ActionListener
listener) { + return new GroupedActionListener<>(new ActionListener>() { @Override public void onResponse(final Collection responses) { - pageResults.add(responses); - listener.onResponse(pageToken); + try { + GetSettingsResponse settingsResponse = extractResponse(responses, GetSettingsResponse.class); + Map indicesSettings = StreamSupport + .stream(Spliterators.spliterator(settingsResponse.getIndexToSettings().entrySet(), 0), false) + .collect(Collectors.toMap(cursor -> cursor.getKey(), cursor -> cursor.getValue())); + + ClusterStateResponse stateResponse = extractResponse(responses, ClusterStateResponse.class); + Map indicesStates = StreamSupport + .stream(stateResponse.getState().getMetadata().spliterator(), false) + .collect(Collectors.toMap(indexMetadata -> indexMetadata.getIndex().getName(), Function.identity())); + + ClusterHealthResponse healthResponse = extractResponse(responses, ClusterHealthResponse.class); + Map indicesHealths = healthResponse.getIndices(); + + IndicesStatsResponse statsResponse = extractResponse(responses, IndicesStatsResponse.class); + Map indicesStats = statsResponse.getIndices(); + + Table responseTable = buildTable(indicesSettings, indicesHealths, indicesStats, indicesStates); + listener.onResponse(responseTable); + } catch (Exception e) { + onFailure(e); + } } @Override public void onFailure(final Exception e) { listener.onFailure(e); } - }, 2); + }, size); } @Override @@ -404,7 +315,7 @@ public static class Factory implements Tool.Factory { private static Factory INSTANCE; - /** + /** * Create or return the singleton factory instance */ public static Factory getInstance() { @@ -485,34 +396,21 @@ private Table getTableWithHeader() { } private Table buildTable( - ClusterStateResponse clusterStateResponse, - GetSettingsResponse getSettingsResponse, - Queue> responses + final Map indicesSettings, + final Map indicesHealths, + final Map indicesStats, + final Map indicesMetadatas ) { - if (responses == null || responses.isEmpty()) { - return null; - } - Tuple, Map> tuple = aggregateResults(responses); final Table table = getTableWithHeader(); AtomicInteger rowNum = new AtomicInteger(0); - Map indicesSettings = StreamSupport - .stream(Spliterators.spliterator(getSettingsResponse.getIndexToSettings().entrySet(), 0), false) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - - Map indicesStates = StreamSupport - .stream(clusterStateResponse.getState().getMetadata().spliterator(), false) - .collect(Collectors.toMap(indexMetadata -> indexMetadata.getIndex().getName(), Function.identity())); - - Map indicesHealths = tuple.v2(); - Map indicesStats = tuple.v1(); indicesSettings.forEach((indexName, settings) -> { - if (!indicesStates.containsKey(indexName)) { + if (!indicesMetadatas.containsKey(indexName)) { // the index exists in the Get Indices response but is not present in the cluster state: // it is likely that the index was deleted in the meanwhile, so we ignore it. return; } - final IndexMetadata indexMetadata = indicesStates.get(indexName); + final IndexMetadata indexMetadata = indicesMetadatas.get(indexName); final IndexMetadata.State indexState = indexMetadata.getState(); final IndexStats indexStats = indicesStats.get(indexName); @@ -550,28 +448,15 @@ private Table buildTable( table.addCell(totalStats.getStore() == null ? null : totalStats.getStore().size()); table.addCell(primaryStats.getStore() == null ? null : primaryStats.getStore().size()); + table.endRow(); }); + return table; } - private Tuple, Map> aggregateResults(Queue> responses) { - // Each batch produces a collection of action response, aggregate them together to build table easier. - Map indexStatsMap = new HashMap<>(); - Map clusterIndexHealthMap = new HashMap<>(); - for (Collection response : responses) { - if (response != null && !response.isEmpty()) { - response.forEach(x -> { - if (x instanceof IndicesStatsResponse) { - indexStatsMap.putAll(((IndicesStatsResponse) x).getIndices()); - } else if (x instanceof ClusterHealthResponse) { - clusterIndexHealthMap.putAll(((ClusterHealthResponse) x).getIndices()); - } else { - throw new IllegalStateException("Unexpected action response type: " + x.getClass().getName()); - } - }); - } - } - return new Tuple<>(indexStatsMap, clusterIndexHealthMap); + @SuppressWarnings("unchecked") + private static A extractResponse(final Collection responses, Class c) { + return (A) responses.stream().filter(c::isInstance).findFirst().get(); } } diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java new file mode 100644 index 0000000000..e1dfc560f7 --- /dev/null +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java @@ -0,0 +1,577 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.ml.engine.tools; + +import static org.opensearch.action.support.clustermanager.ClusterManagerNodeRequest.DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT; +import static org.opensearch.ml.common.utils.StringUtils.gson; + +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Objects; +import java.util.Queue; +import java.util.Spliterators; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; + +import org.apache.logging.log4j.util.Strings; +import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; +import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; +import org.opensearch.action.admin.cluster.state.ClusterStateRequest; +import org.opensearch.action.admin.cluster.state.ClusterStateResponse; +import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest; +import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse; +import org.opensearch.action.admin.indices.stats.CommonStats; +import org.opensearch.action.admin.indices.stats.IndexStats; +import org.opensearch.action.admin.indices.stats.IndicesStatsRequest; +import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; +import org.opensearch.action.pagination.IndexPaginationStrategy; +import org.opensearch.action.pagination.PageParams; +import org.opensearch.action.pagination.PageToken; +import org.opensearch.action.support.GroupedActionListener; +import org.opensearch.action.support.IndicesOptions; +import org.opensearch.client.Client; +import org.opensearch.cluster.health.ClusterIndexHealth; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.Table; +import org.opensearch.common.Table.Cell; +import org.opensearch.common.collect.Tuple; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.action.ActionResponse; +import org.opensearch.index.IndexSettings; +import org.opensearch.ml.common.output.model.ModelTensors; +import org.opensearch.ml.common.spi.tools.Parser; +import org.opensearch.ml.common.spi.tools.Tool; +import org.opensearch.ml.common.spi.tools.ToolAnnotation; + +import lombok.Getter; +import lombok.Setter; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +@ToolAnnotation(ListIndexTool.TYPE) +public class ListIndexTool implements Tool { + public static final String TYPE = "ListIndexTool"; + // This needs to be changed once it's changed in opensearch core in RestIndicesListAction. + private static final int MAX_SUPPORTED_LIST_INDICES_PAGE_SIZE = 5000; + public static final int DEFAULT_PAGE_SIZE = 100; + private static final String DEFAULT_DESCRIPTION = String + .join( + " ", + "This tool gets index information from the OpenSearch cluster.", + "It takes 2 optional arguments named `index` which is a comma-delimited list of one or more indices to get information from (default is an empty list meaning all indices),", + "and `local` which means whether to return information from the local node only instead of the cluster manager node (default is false).", + "The tool returns the indices information, including `health`, `status`, `index`, `uuid`, `pri`, `rep`, `docs.count`, `docs.deleted`, `store.size`, `pri.store. size `, `pri.store.size`, `pri.store`." + ); + + @Setter + @Getter + private String name = ListIndexTool.TYPE; + @Getter + @Setter + private String description = DEFAULT_DESCRIPTION; + @Getter + private String version; + + private Client client; + @Setter + private Parser inputParser; + @Setter + private Parser outputParser; + @SuppressWarnings("unused") + private ClusterService clusterService; + + public ListIndexTool(Client client, ClusterService clusterService) { + this.client = client; + this.clusterService = clusterService; + + outputParser = new Parser<>() { + @Override + public Object parse(Object o) { + @SuppressWarnings("unchecked") + List mlModelOutputs = (List) o; + return mlModelOutputs.get(0).getMlModelTensors().get(0).getDataAsMap().get("response"); + } + }; + } + + @Override + public void run(Map parameters, ActionListener listener) { + // TODO: This logic exactly matches the OpenSearch _cat/indices REST action. If code at + // o.o.rest/action/cat/RestIndicesAction.java changes those changes need to be reflected here + // https://github.com/opensearch-project/ml-commons/pull/1582#issuecomment-1796962876 + @SuppressWarnings("unchecked") + List indexList = parameters.containsKey("indices") + ? gson.fromJson(parameters.get("indices"), List.class) + : Collections.emptyList(); + final String[] indices = indexList.toArray(Strings.EMPTY_ARRAY); + + final IndicesOptions indicesOptions = IndicesOptions.strictExpand(); + final boolean local = parameters.containsKey("local") && Boolean.parseBoolean(parameters.get("local")); + final boolean includeUnloadedSegments = Boolean.parseBoolean(parameters.get("include_unloaded_segments")); + final PageParams pageParams = new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, DEFAULT_PAGE_SIZE); + + final ActionListener
internalListener = ActionListener.notifyOnce(ActionListener.wrap(table -> { + // Handle empty table + if (table == null || table.getRows().isEmpty()) { + @SuppressWarnings("unchecked") + T empty = (T) ("There were no results searching the indices parameter [" + parameters.get("indices") + "]."); + listener.onResponse(empty); + return; + } + StringBuilder sb = new StringBuilder( + // Currently using c.value which is short header matching _cat/indices + // May prefer to use c.attr.get("desc") for full description + table.getHeaders().stream().map(c -> c.value.toString()).collect(Collectors.joining(",", "", "\n")) + ); + for (List row : table.getRows()) { + sb.append(row.stream().map(c -> c.value == null ? null : c.value.toString()).collect(Collectors.joining(",", "", "\n"))); + } + @SuppressWarnings("unchecked") + T response = (T) sb.toString(); + listener.onResponse(response); + }, listener::onFailure)); + + fetchClusterInfoAndPages( + indices, + local, + includeUnloadedSegments, + pageParams, + indicesOptions, + new ConcurrentLinkedQueue<>(), + internalListener + ); + } + + private void fetchClusterInfoAndPages( + String[] indices, + boolean local, + boolean includeUnloadedSegments, + PageParams pageParams, + IndicesOptions indicesOptions, + Queue> pageResults, + ActionListener
originalListener + ) { + // First fetch metadata like index setting and cluster states and then fetch index details in batches to save efforts. + sendGetSettingsRequest(indices, indicesOptions, local, client, new ActionListener<>() { + @Override + public void onResponse(final GetSettingsResponse getSettingsResponse) { + // The list of indices that will be returned is determined by the indices returned from the Get Settings call. + // All the other requests just provide additional detail, and wildcards may be resolved differently depending on the + // type of request in the presence of security plugins (looking at you, ClusterHealthRequest), so + // force the IndicesOptions for all the sub-requests to be as inclusive as possible. + final IndicesOptions subRequestIndicesOptions = IndicesOptions.lenientExpandHidden(); + // Indices that were successfully resolved during the get settings request might be deleted when the + // subsequent cluster state, cluster health and indices stats requests execute. We have to distinguish two cases: + // 1) the deleted index was explicitly passed as parameter to the /_cat/indices request. In this case we + // want the subsequent requests to fail. + // 2) the deleted index was resolved as part of a wildcard or _all. In this case, we want the subsequent + // requests not to fail on the deleted index (as we want to ignore wildcards that cannot be resolved). + // This behavior can be ensured by letting the cluster state, cluster health and indices stats requests + // re-resolve the index names with the same indices options that we used for the initial cluster state + // request (strictExpand). + sendClusterStateRequest(indices, subRequestIndicesOptions, local, client, new ActionListener<>() { + @Override + public void onResponse(ClusterStateResponse clusterStateResponse) { + // Starts to fetch index details here, if a batch fails build whatever we have and return. + fetchPages( + indices, + local, + DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT, + includeUnloadedSegments, + pageParams, + pageResults, + clusterStateResponse, + getSettingsResponse, + subRequestIndicesOptions, + originalListener + ); + } + + @Override + public void onFailure(final Exception e) { + originalListener.onFailure(e); + } + }); + } + + @Override + public void onFailure(final Exception e) { + originalListener.onFailure(e); + } + }); + } + + private void fetchPages( + String[] indices, + boolean local, + TimeValue clusterManagerNodeTimeout, + boolean includeUnloadedSegments, + PageParams pageParams, + Queue> pageResults, + ClusterStateResponse clusterStateResponse, + GetSettingsResponse getSettingsResponse, + IndicesOptions subRequestIndicesOptions, + ActionListener
originalListener + ) { + final ActionListener iterativeListener = ActionListener.wrap(r -> { + // when previous response returns, build next request with response and invoke again. + PageParams nextPageParams = new PageParams(r.getNextToken(), pageParams.getSort(), pageParams.getSize()); + // when next page doesn't exist or reaches max supported page size, return. + if (r.getNextToken() == null || pageResults.size() >= MAX_SUPPORTED_LIST_INDICES_PAGE_SIZE) { + Table table = buildTable(clusterStateResponse, getSettingsResponse, pageResults); + originalListener.onResponse(table); + } else { + fetchPages( + indices, + local, + clusterManagerNodeTimeout, + includeUnloadedSegments, + nextPageParams, + pageResults, + clusterStateResponse, + getSettingsResponse, + subRequestIndicesOptions, + originalListener + ); + } + }, e -> { + log.error("Failed to fetch index info for page: {}", pageParams.getRequestedToken()); + // Do not throw the exception, just return whatever we have. + originalListener.onResponse(buildTable(clusterStateResponse, getSettingsResponse, pageResults)); + }); + IndexPaginationStrategy paginationStrategy = getPaginationStrategy(pageParams, clusterStateResponse); + // For non-paginated queries, indicesToBeQueried would be same as indices retrieved from + // rest request and unresolved, while for paginated queries, it would be a list of indices + // already resolved by ClusterStateRequest and to be displayed in a page. + final String[] indicesToBeQueried = Objects.isNull(paginationStrategy) + ? indices + : paginationStrategy.getRequestedEntities().toArray(new String[0]); + // After the group listener returns, one page complete and prepare for next page. + final GroupedActionListener groupedListener = createGroupedListener( + pageResults, + paginationStrategy.getResponseToken(), + iterativeListener + ); + + sendIndicesStatsRequest( + indicesToBeQueried, + subRequestIndicesOptions, + includeUnloadedSegments, + client, + ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) + ); + + sendClusterHealthRequest( + indicesToBeQueried, + subRequestIndicesOptions, + local, + clusterManagerNodeTimeout, + client, + ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) + ); + } + + protected IndexPaginationStrategy getPaginationStrategy(PageParams pageParams, ClusterStateResponse clusterStateResponse) { + return new IndexPaginationStrategy(pageParams, clusterStateResponse.getState()); + } + + @Override + public String getType() { + return TYPE; + } + + /** + * We're using the Get Settings API here to resolve the authorized indices for the user. + * This is because the Cluster State and Cluster Health APIs do not filter output based + * on index privileges, so they can't be used to determine which indices are authorized + * or not. On top of this, the Indices Stats API cannot be used either to resolve indices + * as it does not provide information for all existing indices (for example recovering + * indices or non replicated closed indices are not reported in indices stats response). + */ + private void sendGetSettingsRequest( + final String[] indices, + final IndicesOptions indicesOptions, + final boolean local, + final Client client, + final ActionListener listener + ) { + final GetSettingsRequest request = new GetSettingsRequest(); + request.indices(indices); + request.indicesOptions(indicesOptions); + request.local(local); + request.clusterManagerNodeTimeout(DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT); + request.names(IndexSettings.INDEX_SEARCH_THROTTLED.getKey()); + + client.admin().indices().getSettings(request, listener); + } + + private void sendClusterStateRequest( + final String[] indices, + final IndicesOptions indicesOptions, + final boolean local, + final Client client, + final ActionListener listener + ) { + + final ClusterStateRequest request = new ClusterStateRequest(); + request.indices(indices); + request.indicesOptions(indicesOptions); + request.local(local); + request.clusterManagerNodeTimeout(DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT); + + client.admin().cluster().state(request, listener); + } + + private void sendClusterHealthRequest( + final String[] indices, + final IndicesOptions indicesOptions, + final boolean local, + final TimeValue clusterManagerNodeTimeout, + final Client client, + final ActionListener listener + ) { + + final ClusterHealthRequest request = new ClusterHealthRequest(); + request.indices(indices); + request.indicesOptions(indicesOptions); + request.local(local); + request.clusterManagerNodeTimeout(clusterManagerNodeTimeout); + + client.admin().cluster().health(request, listener); + } + + private void sendIndicesStatsRequest( + final String[] indices, + final IndicesOptions indicesOptions, + final boolean includeUnloadedSegments, + final Client client, + final ActionListener listener + ) { + + final IndicesStatsRequest request = new IndicesStatsRequest(); + request.indices(indices); + request.indicesOptions(indicesOptions); + request.all(); + request.includeUnloadedSegments(includeUnloadedSegments); + + client.admin().indices().stats(request, listener); + } + + // group listener only accept two action response: IndicesStatsResponse and ClusterHealthResponse + private GroupedActionListener createGroupedListener( + final Queue> pageResults, + final PageToken pageToken, + final ActionListener listener + ) { + return new GroupedActionListener<>(new ActionListener<>() { + @Override + public void onResponse(final Collection responses) { + pageResults.add(responses); + listener.onResponse(pageToken); + } + + @Override + public void onFailure(final Exception e) { + listener.onFailure(e); + } + }, 2); + } + + @Override + public boolean validate(Map parameters) { + return parameters != null && !parameters.isEmpty(); + } + + /** + * Factory for the {@link ListIndexTool} + */ + public static class Factory implements Tool.Factory { + private Client client; + private ClusterService clusterService; + + private static Factory INSTANCE; + + /** + * Create or return the singleton factory instance + */ + public static Factory getInstance() { + if (INSTANCE != null) { + return INSTANCE; + } + synchronized (ListIndexTool.class) { + if (INSTANCE != null) { + return INSTANCE; + } + INSTANCE = new Factory(); + return INSTANCE; + } + } + + /** + * Initialize this factory + * @param client The OpenSearch client + * @param clusterService The OpenSearch cluster service + */ + public void init(Client client, ClusterService clusterService) { + this.client = client; + this.clusterService = clusterService; + } + + @Override + public ListIndexTool create(Map map) { + return new ListIndexTool(client, clusterService); + } + + @Override + public String getDefaultDescription() { + return DEFAULT_DESCRIPTION; + } + + @Override + public String getDefaultType() { + return TYPE; + } + + @Override + public String getDefaultVersion() { + return null; + } + } + + private Table getTableWithHeader() { + Table table = new Table(); + table.startHeaders(); + // First param is cell.value which is currently returned + // Second param is cell.attr we may want to use attr.desc in the future + table.addCell("row", "alias:r;desc:row number"); + table.addCell("health", "alias:h;desc:current health status"); + table.addCell("status", "alias:s;desc:open/close status"); + table.addCell("index", "alias:i,idx;desc:index name"); + table.addCell("uuid", "alias:id,uuid;desc:index uuid"); + table + .addCell( + "pri(number of primary shards)", + "alias:p,shards.primary,shardsPrimary;text-align:right;desc:number of primary shards" + ); + table + .addCell( + "rep(number of replica shards)", + "alias:r,shards.replica,shardsReplica;text-align:right;desc:number of replica shards" + ); + table.addCell("docs.count(number of available documents)", "alias:dc,docsCount;text-align:right;desc:available docs"); + table.addCell("docs.deleted(number of deleted documents)", "alias:dd,docsDeleted;text-align:right;desc:deleted docs"); + table + .addCell( + "store.size(store size of primary and replica shards)", + "sibling:pri;alias:ss,storeSize;text-align:right;desc:store size of primaries & replicas" + ); + table.addCell("pri.store.size(store size of primary shards)", "text-align:right;desc:store size of primaries"); + // Above includes all the default fields for cat indices. See RestIndicesAction for a lot more that could be included. + table.endHeaders(); + return table; + } + + private Table buildTable( + ClusterStateResponse clusterStateResponse, + GetSettingsResponse getSettingsResponse, + Queue> responses + ) { + if (responses == null || responses.isEmpty()) { + return null; + } + Tuple, Map> tuple = aggregateResults(responses); + final Table table = getTableWithHeader(); + AtomicInteger rowNum = new AtomicInteger(0); + Map indicesSettings = StreamSupport + .stream(Spliterators.spliterator(getSettingsResponse.getIndexToSettings().entrySet(), 0), false) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + + Map indicesStates = StreamSupport + .stream(clusterStateResponse.getState().getMetadata().spliterator(), false) + .collect(Collectors.toMap(indexMetadata -> indexMetadata.getIndex().getName(), Function.identity())); + + Map indicesHealths = tuple.v2(); + Map indicesStats = tuple.v1(); + indicesSettings.forEach((indexName, settings) -> { + if (!indicesStates.containsKey(indexName)) { + // the index exists in the Get Indices response but is not present in the cluster state: + // it is likely that the index was deleted in the meanwhile, so we ignore it. + return; + } + + final IndexMetadata indexMetadata = indicesStates.get(indexName); + final IndexMetadata.State indexState = indexMetadata.getState(); + final IndexStats indexStats = indicesStats.get(indexName); + + final String health; + final ClusterIndexHealth indexHealth = indicesHealths.get(indexName); + if (indexHealth != null) { + health = indexHealth.getStatus().toString().toLowerCase(Locale.ROOT); + } else if (indexStats != null) { + health = "red*"; + } else { + health = ""; + } + + final CommonStats primaryStats; + final CommonStats totalStats; + + if (indexStats == null || indexState == IndexMetadata.State.CLOSE) { + primaryStats = new CommonStats(); + totalStats = new CommonStats(); + } else { + primaryStats = indexStats.getPrimaries(); + totalStats = indexStats.getTotal(); + } + table.startRow(); + table.addCell(rowNum.addAndGet(1)); + table.addCell(health); + table.addCell(indexState.toString().toLowerCase(Locale.ROOT)); + table.addCell(indexName); + table.addCell(indexMetadata.getIndexUUID()); + table.addCell(indexHealth == null ? null : indexHealth.getNumberOfShards()); + table.addCell(indexHealth == null ? null : indexHealth.getNumberOfReplicas()); + + table.addCell(primaryStats.getDocs() == null ? null : primaryStats.getDocs().getCount()); + table.addCell(primaryStats.getDocs() == null ? null : primaryStats.getDocs().getDeleted()); + + table.addCell(totalStats.getStore() == null ? null : totalStats.getStore().size()); + table.addCell(primaryStats.getStore() == null ? null : primaryStats.getStore().size()); + table.endRow(); + }); + return table; + } + + private Tuple, Map> aggregateResults(Queue> responses) { + // Each batch produces a collection of action response, aggregate them together to build table easier. + Map indexStatsMap = new HashMap<>(); + Map clusterIndexHealthMap = new HashMap<>(); + for (Collection response : responses) { + if (response != null && !response.isEmpty()) { + response.forEach(x -> { + if (x instanceof IndicesStatsResponse) { + indexStatsMap.putAll(((IndicesStatsResponse) x).getIndices()); + } else if (x instanceof ClusterHealthResponse) { + clusterIndexHealthMap.putAll(((ClusterHealthResponse) x).getIndices()); + } else { + throw new IllegalStateException("Unexpected action response type: " + x.getClass().getName()); + } + }); + } + } + return new Tuple<>(indexStatsMap, clusterIndexHealthMap); + } +} diff --git a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/CatIndexToolTests.java b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/CatIndexToolTests.java index 26f9492c85..31dd909d1a 100644 --- a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/CatIndexToolTests.java +++ b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/CatIndexToolTests.java @@ -150,8 +150,8 @@ public void testRunAsyncNoIndices() throws Exception { tool.run(otherParams, listener); settingsActionListenerCaptor.getValue().onResponse(getSettingsResponse); - clusterStateActionListenerCaptor.getValue().onResponse(clusterStateResponse); statsActionListenerCaptor.getValue().onResponse(indicesStatsResponse); + clusterStateActionListenerCaptor.getValue().onResponse(clusterStateResponse); clusterHealthActionListenerCaptor.getValue().onResponse(clusterHealthResponse); future.join(); @@ -214,8 +214,8 @@ public void testRunAsyncIndexStats() throws Exception { tool.run(otherParams, listener); settingsActionListenerCaptor.getValue().onResponse(getSettingsResponse); - clusterStateActionListenerCaptor.getValue().onResponse(clusterStateResponse); statsActionListenerCaptor.getValue().onResponse(indicesStatsResponse); + clusterStateActionListenerCaptor.getValue().onResponse(clusterStateResponse); clusterHealthActionListenerCaptor.getValue().onResponse(clusterHealthResponse); future.orTimeout(10, TimeUnit.SECONDS).join(); diff --git a/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java b/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java similarity index 94% rename from plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java rename to plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java index 0a52c68807..edb7992aec 100644 --- a/plugin/src/test/java/org/opensearch/ml/tools/CatIndexToolIT.java +++ b/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java @@ -16,7 +16,7 @@ import org.junit.Before; import org.opensearch.client.Response; import org.opensearch.common.settings.Settings; -import org.opensearch.ml.engine.tools.CatIndexTool; +import org.opensearch.ml.engine.tools.ListIndexTool; import org.opensearch.ml.rest.RestBaseAgentToolsIT; import org.opensearch.ml.utils.TestHelper; @@ -27,7 +27,7 @@ import lombok.extern.log4j.Log4j2; @Log4j2 -public class CatIndexToolIT extends RestBaseAgentToolsIT { +public class ListIndexToolIT extends RestBaseAgentToolsIT { private String agentId; private final String question = "{\"parameters\":{\"question\":\"please help list all the index status in the current cluster?\"}}"; @@ -55,7 +55,7 @@ private void registerCatIndexFlowAgent() throws Exception { } public void testCatIndexWithFewIndices() throws IOException { - List indices = createIndices(CatIndexTool.DEFAULT_PAGE_SIZE); + List indices = createIndices(ListIndexTool.DEFAULT_PAGE_SIZE); Response response = TestHelper.makeRequest(client(), "POST", "/_plugins/_ml/agents/" + agentId + "/_execute", null, question, null); String responseStr = TestHelper.httpEntityToString(response.getEntity()); String toolOutput = extractResult(responseStr); @@ -68,7 +68,7 @@ public void testCatIndexWithFewIndices() throws IOException { } public void testCatIndexWithMoreThan100Indices() throws IOException { - List indices = createIndices(CatIndexTool.DEFAULT_PAGE_SIZE + 1); + List indices = createIndices(ListIndexTool.DEFAULT_PAGE_SIZE + 1); Response response = TestHelper.makeRequest(client(), "POST", "/_plugins/_ml/agents/" + agentId + "/_execute", null, question, null); String responseStr = TestHelper.httpEntityToString(response.getEntity()); String toolOutput = extractResult(responseStr); From aa530b352fdd70e5991e735097aa3c9051b106e3 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Thu, 5 Dec 2024 08:58:29 +0800 Subject: [PATCH 05/21] Use random string instead of number sequence Signed-off-by: zane-neo --- .../src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java b/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java index edb7992aec..02a9c51f7e 100644 --- a/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java +++ b/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java @@ -39,7 +39,7 @@ public void setUpCluster() throws Exception { private List createIndices(int count) throws IOException { List indices = new ArrayList<>(); for (int i = 0; i < count; i++) { - String indexName = "test" + i; + String indexName = "test" + randomAlphaOfLength(5); createIndex(indexName, Settings.EMPTY); indices.add(indexName); } From 3a244a3c4caddcba71c514a931d3234341a1cfd1 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Thu, 5 Dec 2024 09:47:36 +0800 Subject: [PATCH 06/21] fix failure IT Signed-off-by: zane-neo --- .../src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java b/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java index 02a9c51f7e..0fa7d7b681 100644 --- a/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java +++ b/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java @@ -13,6 +13,7 @@ import java.util.List; import java.util.Objects; +import org.apache.commons.lang3.StringUtils; import org.junit.Before; import org.opensearch.client.Response; import org.opensearch.common.settings.Settings; @@ -39,7 +40,7 @@ public void setUpCluster() throws Exception { private List createIndices(int count) throws IOException { List indices = new ArrayList<>(); for (int i = 0; i < count; i++) { - String indexName = "test" + randomAlphaOfLength(5); + String indexName = "test" + StringUtils.toRootLowerCase(randomAlphaOfLength(5)); createIndex(indexName, Settings.EMPTY); indices.add(indexName); } From f455a1e2adb4f26543bd572cd910c89fc716a2e9 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Tue, 10 Dec 2024 09:19:34 +0800 Subject: [PATCH 07/21] change comment to _list/indices API rest action file Signed-off-by: zane-neo --- .../java/org/opensearch/ml/engine/tools/ListIndexTool.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java index e1dfc560f7..98210b77fd 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java @@ -109,9 +109,8 @@ public Object parse(Object o) { @Override public void run(Map parameters, ActionListener listener) { - // TODO: This logic exactly matches the OpenSearch _cat/indices REST action. If code at - // o.o.rest/action/cat/RestIndicesAction.java changes those changes need to be reflected here - // https://github.com/opensearch-project/ml-commons/pull/1582#issuecomment-1796962876 + // TODO: This logic exactly matches the OpenSearch _list/indices REST action. If code at + // o.o.rest/action/list/RestIndicesListAction.java changes those changes need to be reflected here @SuppressWarnings("unchecked") List indexList = parameters.containsKey("indices") ? gson.fromJson(parameters.get("indices"), List.class) From 7e915fb8fb6f6bf684d993bf1f638b7daa00a4dd Mon Sep 17 00:00:00 2001 From: zane-neo Date: Mon, 16 Dec 2024 09:18:54 +0800 Subject: [PATCH 08/21] add page size parameter from input and change sef4j to log4j Signed-off-by: zane-neo --- .../org/opensearch/ml/engine/tools/ListIndexTool.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java index 98210b77fd..4ccd70373d 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java @@ -23,6 +23,8 @@ import java.util.stream.Collectors; import java.util.stream.StreamSupport; +import lombok.extern.log4j.Log4j2; +import org.apache.commons.lang3.math.NumberUtils; import org.apache.logging.log4j.util.Strings; import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; @@ -58,9 +60,8 @@ import lombok.Getter; import lombok.Setter; -import lombok.extern.slf4j.Slf4j; -@Slf4j +@Log4j2 @ToolAnnotation(ListIndexTool.TYPE) public class ListIndexTool implements Tool { public static final String TYPE = "ListIndexTool"; @@ -120,7 +121,10 @@ public void run(Map parameters, ActionListener listener) final IndicesOptions indicesOptions = IndicesOptions.strictExpand(); final boolean local = parameters.containsKey("local") && Boolean.parseBoolean(parameters.get("local")); final boolean includeUnloadedSegments = Boolean.parseBoolean(parameters.get("include_unloaded_segments")); - final PageParams pageParams = new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, DEFAULT_PAGE_SIZE); + final int pageSize = parameters.containsKey("page_size") + ? NumberUtils.toInt(parameters.get("page_size"), DEFAULT_PAGE_SIZE) + : DEFAULT_PAGE_SIZE; + final PageParams pageParams = new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize); final ActionListener
internalListener = ActionListener.notifyOnce(ActionListener.wrap(table -> { // Handle empty table From e9f97af862cf87b5e5a282e5acbff839a8848d87 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Mon, 16 Dec 2024 09:34:35 +0800 Subject: [PATCH 09/21] format code Signed-off-by: zane-neo --- .../main/java/org/opensearch/ml/engine/tools/ListIndexTool.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java index 4ccd70373d..542703444b 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java @@ -23,7 +23,6 @@ import java.util.stream.Collectors; import java.util.stream.StreamSupport; -import lombok.extern.log4j.Log4j2; import org.apache.commons.lang3.math.NumberUtils; import org.apache.logging.log4j.util.Strings; import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; @@ -60,6 +59,7 @@ import lombok.Getter; import lombok.Setter; +import lombok.extern.log4j.Log4j2; @Log4j2 @ToolAnnotation(ListIndexTool.TYPE) From b8434799fc0761a5d4c4f373d3d011c28995a0af Mon Sep 17 00:00:00 2001 From: zane-neo Date: Thu, 13 Feb 2025 16:59:19 +0800 Subject: [PATCH 10/21] Add UT for ListIndexTool Signed-off-by: zane-neo --- .../ml/engine/tools/ListIndexTool.java | 1 + .../ml/engine/tools/ListIndexToolTests.java | 239 ++++++++++++++++++ .../opensearch/ml/tools/ListIndexToolIT.java | 10 +- 3 files changed, 245 insertions(+), 5 deletions(-) create mode 100644 ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/ListIndexToolTests.java diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java index 542703444b..9bb8efe28d 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java @@ -263,6 +263,7 @@ private void fetchPages( ? indices : paginationStrategy.getRequestedEntities().toArray(new String[0]); // After the group listener returns, one page complete and prepare for next page. + // We put the single page result into the pageResults queue for future buildTable. final GroupedActionListener groupedListener = createGroupedListener( pageResults, paginationStrategy.getResponseToken(), diff --git a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/ListIndexToolTests.java b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/ListIndexToolTests.java new file mode 100644 index 0000000000..5056be9534 --- /dev/null +++ b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/ListIndexToolTests.java @@ -0,0 +1,239 @@ +package org.opensearch.ml.engine.tools; + +import com.google.common.collect.ImmutableMap; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.opensearch.Version; +import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; +import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; +import org.opensearch.action.admin.cluster.state.ClusterStateRequest; +import org.opensearch.action.admin.cluster.state.ClusterStateResponse; +import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest; +import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse; +import org.opensearch.action.admin.indices.stats.CommonStats; +import org.opensearch.action.admin.indices.stats.IndexStats; +import org.opensearch.action.admin.indices.stats.IndicesStatsRequest; +import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; +import org.opensearch.client.AdminClient; +import org.opensearch.client.Client; +import org.opensearch.client.ClusterAdminClient; +import org.opensearch.client.IndicesAdminClient; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.health.ClusterIndexHealth; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.cluster.routing.IndexRoutingTable; +import org.opensearch.cluster.routing.IndexShardRoutingTable; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.UUIDs; +import org.opensearch.common.settings.Settings; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.unit.ByteSizeValue; +import org.opensearch.core.index.Index; +import org.opensearch.index.shard.DocsStats; +import org.opensearch.index.store.StoreStats; +import org.opensearch.ml.common.spi.tools.Tool; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.isA; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class ListIndexToolTests { + @Mock + private Client client; + @Mock + private AdminClient adminClient; + @Mock + private IndicesAdminClient indicesAdminClient; + @Mock + private ClusterAdminClient clusterAdminClient; + @Mock + private ClusterService clusterService; + @Mock + private ClusterState clusterState; + @Mock + private Metadata metadata; + @Mock + private IndexMetadata indexMetadata; + @Mock + private IndexRoutingTable indexRoutingTable; + @Mock + private Index index; + + + @Before + public void setup() { + MockitoAnnotations.openMocks(this); + + when(adminClient.indices()).thenReturn(indicesAdminClient); + when(adminClient.cluster()).thenReturn(clusterAdminClient); + when(client.admin()).thenReturn(adminClient); + + when(indexMetadata.getState()).thenReturn(IndexMetadata.State.OPEN); + when(indexMetadata.getCreationVersion()).thenReturn(Version.CURRENT); + + when(metadata.index(any(String.class))).thenReturn(indexMetadata); + when(indexMetadata.getIndex()).thenReturn(index); + when(indexMetadata.getIndexUUID()).thenReturn(UUIDs.base64UUID()); + when(index.getName()).thenReturn("index-1"); + when(clusterState.metadata()).thenReturn(metadata); + when(clusterState.getMetadata()).thenReturn(metadata); + when(clusterService.state()).thenReturn(clusterState); + + ListIndexTool.Factory.getInstance().init(client, clusterService); + } + + + @Test + public void test_getType() { + Tool tool = ListIndexTool.Factory.getInstance().create(Collections.emptyMap()); + assert (tool.getType().equals("ListIndexTool")); + } + + @Test + public void test_run_successful() { + Map parameters = new HashMap<>(); + parameters.put("indices", "[\"index-1\"]"); + parameters.put("page_size", "10"); + + doAnswer(invocation -> { + ActionListener actionListener = invocation.getArgument(1); + GetSettingsResponse response = mock(GetSettingsResponse.class); + Map indexToSettings = new HashMap<>(); + indexToSettings.put("index-1", Settings.EMPTY); + indexToSettings.put("index-2", Settings.EMPTY); + when(response.getIndexToSettings()).thenReturn(indexToSettings); + actionListener.onResponse(response); + return null; + }).when(indicesAdminClient).getSettings(any(GetSettingsRequest.class), isA(ActionListener.class)); + + // clusterStateResponse.getState().getMetadata().spliterator() + doAnswer(invocation -> { + ActionListener actionListener = invocation.getArgument(1); + ClusterStateResponse response = mock(ClusterStateResponse.class); + when(response.getState()).thenReturn(clusterState); + actionListener.onResponse(response); + return null; + }).when(clusterAdminClient).state(any(ClusterStateRequest.class), isA(ActionListener.class)); + + doAnswer(invocation -> { + ActionListener actionListener = invocation.getArgument(1); + IndicesStatsResponse response = mock(IndicesStatsResponse.class); + Map indicesStats = new HashMap<>(); + IndexStats indexStats = mock(IndexStats.class); + // mock primary stats + CommonStats primaryStats = mock(CommonStats.class); + DocsStats docsStats = mock(DocsStats.class); + when(docsStats.getCount()).thenReturn(100L); + when(docsStats.getDeleted()).thenReturn(10L); + when(primaryStats.getDocs()).thenReturn(docsStats); + StoreStats primaryStoreStats = mock(StoreStats.class); + when(primaryStoreStats.size()).thenReturn(ByteSizeValue.parseBytesSizeValue("100k", "mock_setting_name")); + when(primaryStats.getStore()).thenReturn(primaryStoreStats); + // end mock primary stats + + //mock total stats + CommonStats totalStats = mock(CommonStats.class); + DocsStats totalDocsStats = mock(DocsStats.class); + when(totalDocsStats.getCount()).thenReturn(100L); + when(totalDocsStats.getDeleted()).thenReturn(10L); + StoreStats totalStoreStats = mock(StoreStats.class); + when(totalStoreStats.size()).thenReturn(ByteSizeValue.parseBytesSizeValue("100k", "mock_setting_name")); + when(totalStats.getStore()).thenReturn(totalStoreStats); + //end mock common stats + + when(indexStats.getPrimaries()).thenReturn(primaryStats); + when(indexStats.getTotal()).thenReturn(totalStats); + indicesStats.put("index-1", indexStats); + when(response.getIndices()).thenReturn(indicesStats); + actionListener.onResponse(response); + return null; + }).when(indicesAdminClient).stats(any(IndicesStatsRequest.class), isA(ActionListener.class)); + + doAnswer(invocation -> { + ActionListener actionListener = invocation.getArgument(1); + ClusterHealthResponse response = mock(ClusterHealthResponse.class); + Map clusterIndexHealthMap = new HashMap<>(); + when(indexMetadata.getNumberOfShards()).thenReturn(5); + when(indexMetadata.getNumberOfReplicas()).thenReturn(1); + when(metadata.spliterator()).thenReturn(Arrays.spliterator(new IndexMetadata[] { indexMetadata })); + Iterator iterator = (Iterator) mock(Iterator.class); + when(iterator.hasNext()).thenReturn(false); + when(indexRoutingTable.iterator()).thenReturn(iterator); + ClusterIndexHealth health = new ClusterIndexHealth(indexMetadata, indexRoutingTable); + clusterIndexHealthMap.put("index-1", health); + when(response.getIndices()).thenReturn(clusterIndexHealthMap); + actionListener.onResponse(response); + return null; + }).when(clusterAdminClient).health(any(ClusterHealthRequest.class), isA(ActionListener.class)); + + Tool tool = ListIndexTool.Factory.getInstance().create(Collections.emptyMap()); + ActionListener listener = mock(ActionListener.class); + ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); + tool.run(parameters, listener); + verify(listener).onResponse(captor.capture()); + System.out.println(captor.getValue()); + assert captor.getValue().contains("1,red,open,index-1"); + assert captor.getValue().contains("5,1,100,10,100kb,100kb"); + } + + @Test + public void test_run_failed() { + Map parameters = new HashMap<>(); + parameters.put("indices", "[\"index-1\"]"); + parameters.put("page_size", "10"); + + doAnswer(invocation -> { + ActionListener actionListener = invocation.getArgument(1); + actionListener.onFailure(new RuntimeException("failed to get settings")); + return null; + }).when(indicesAdminClient).getSettings(any(GetSettingsRequest.class), isA(ActionListener.class)); + + Tool tool = ListIndexTool.Factory.getInstance().create(Collections.emptyMap()); + ActionListener listener = mock(ActionListener.class); + ArgumentCaptor captor = ArgumentCaptor.forClass(RuntimeException.class); + tool.run(parameters, listener); + verify(listener).onFailure(captor.capture()); + System.out.println(captor.getValue().getMessage()); + assert (captor.getValue().getMessage().contains("failed to get settings")); + } + + + @Test + public void test_validate() { + Tool tool = ListIndexTool.Factory.getInstance().create(Collections.emptyMap()); + assert tool.validate(ImmutableMap.of("runtimeParameter", "value1")); + } + + @Test + public void test_getDefaultDescription() { + Tool.Factory factory = ListIndexTool.Factory.getInstance(); + System.out.println(factory.getDefaultDescription()); + assert (factory.getDefaultDescription().equals("This tool gets index information from the OpenSearch cluster. It takes 2 optional arguments named `index` which is a comma-delimited list of one or more indices to get information from (default is an empty list meaning all indices), and `local` which means whether to return information from the local node only instead of the cluster manager node (default is false). The tool returns the indices information, including `health`, `status`, `index`, `uuid`, `pri`, `rep`, `docs.count`, `docs.deleted`, `store.size`, `pri.store. size `, `pri.store.size`, `pri.store`.")); + } + + @Test + public void test_getDefaultType() { + Tool.Factory factory = ListIndexTool.Factory.getInstance(); + System.out.println(factory.getDefaultType()); + assert (factory.getDefaultType().equals("ListIndexTool")); + } + + @Test + public void test_getDefaultVersion() { + Tool.Factory factory = ListIndexTool.Factory.getInstance(); + assert factory.getDefaultVersion() == null; + } +} diff --git a/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java b/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java index 0fa7d7b681..3074fe090c 100644 --- a/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java +++ b/plugin/src/test/java/org/opensearch/ml/tools/ListIndexToolIT.java @@ -34,7 +34,7 @@ public class ListIndexToolIT extends RestBaseAgentToolsIT { @Before public void setUpCluster() throws Exception { - registerCatIndexFlowAgent(); + registerListIndexFlowAgent(); } private List createIndices(int count) throws IOException { @@ -47,15 +47,15 @@ private List createIndices(int count) throws IOException { return indices; } - private void registerCatIndexFlowAgent() throws Exception { + private void registerListIndexFlowAgent() throws Exception { String requestBody = Files .readString( - Path.of(this.getClass().getClassLoader().getResource("org/opensearch/ml/tools/CatIndexAgentRegistration.json").toURI()) + Path.of(this.getClass().getClassLoader().getResource("org/opensearch/ml/tools/ListIndexAgentRegistration.json").toURI()) ); registerMLAgent(client(), requestBody, response -> agentId = (String) response.get("agent_id")); } - public void testCatIndexWithFewIndices() throws IOException { + public void testListIndexWithFewIndices() throws IOException { List indices = createIndices(ListIndexTool.DEFAULT_PAGE_SIZE); Response response = TestHelper.makeRequest(client(), "POST", "/_plugins/_ml/agents/" + agentId + "/_execute", null, question, null); String responseStr = TestHelper.httpEntityToString(response.getEntity()); @@ -68,7 +68,7 @@ public void testCatIndexWithFewIndices() throws IOException { } } - public void testCatIndexWithMoreThan100Indices() throws IOException { + public void testListIndexWithMoreThan100Indices() throws IOException { List indices = createIndices(ListIndexTool.DEFAULT_PAGE_SIZE + 1); Response response = TestHelper.makeRequest(client(), "POST", "/_plugins/_ml/agents/" + agentId + "/_execute", null, question, null); String responseStr = TestHelper.httpEntityToString(response.getEntity()); From baac9427bbfa01dedf961436908da736aae5e740 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Thu, 13 Feb 2025 17:10:24 +0800 Subject: [PATCH 11/21] rebase main Signed-off-by: zane-neo --- .../ml/engine/tools/ListIndexTool.java | 2 +- .../ml/engine/tools/ListIndexToolTests.java | 48 ++++++++++--------- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java index 9bb8efe28d..cae0285138 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java @@ -40,7 +40,6 @@ import org.opensearch.action.pagination.PageToken; import org.opensearch.action.support.GroupedActionListener; import org.opensearch.action.support.IndicesOptions; -import org.opensearch.client.Client; import org.opensearch.cluster.health.ClusterIndexHealth; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.service.ClusterService; @@ -56,6 +55,7 @@ import org.opensearch.ml.common.spi.tools.Parser; import org.opensearch.ml.common.spi.tools.Tool; import org.opensearch.ml.common.spi.tools.ToolAnnotation; +import org.opensearch.transport.client.Client; import lombok.Getter; import lombok.Setter; diff --git a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/ListIndexToolTests.java b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/ListIndexToolTests.java index 5056be9534..24d17c6074 100644 --- a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/ListIndexToolTests.java +++ b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/ListIndexToolTests.java @@ -1,6 +1,18 @@ package org.opensearch.ml.engine.tools; -import com.google.common.collect.ImmutableMap; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.isA; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; + import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -17,10 +29,6 @@ import org.opensearch.action.admin.indices.stats.IndexStats; import org.opensearch.action.admin.indices.stats.IndicesStatsRequest; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; -import org.opensearch.client.AdminClient; -import org.opensearch.client.Client; -import org.opensearch.client.ClusterAdminClient; -import org.opensearch.client.IndicesAdminClient; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.health.ClusterIndexHealth; import org.opensearch.cluster.metadata.IndexMetadata; @@ -36,19 +44,12 @@ import org.opensearch.index.shard.DocsStats; import org.opensearch.index.store.StoreStats; import org.opensearch.ml.common.spi.tools.Tool; +import org.opensearch.transport.client.AdminClient; +import org.opensearch.transport.client.Client; +import org.opensearch.transport.client.ClusterAdminClient; +import org.opensearch.transport.client.IndicesAdminClient; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.Iterator; -import java.util.Map; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.isA; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import com.google.common.collect.ImmutableMap; public class ListIndexToolTests { @Mock @@ -72,7 +73,6 @@ public class ListIndexToolTests { @Mock private Index index; - @Before public void setup() { MockitoAnnotations.openMocks(this); @@ -95,7 +95,6 @@ public void setup() { ListIndexTool.Factory.getInstance().init(client, clusterService); } - @Test public void test_getType() { Tool tool = ListIndexTool.Factory.getInstance().create(Collections.emptyMap()); @@ -144,7 +143,7 @@ public void test_run_successful() { when(primaryStats.getStore()).thenReturn(primaryStoreStats); // end mock primary stats - //mock total stats + // mock total stats CommonStats totalStats = mock(CommonStats.class); DocsStats totalDocsStats = mock(DocsStats.class); when(totalDocsStats.getCount()).thenReturn(100L); @@ -152,7 +151,7 @@ public void test_run_successful() { StoreStats totalStoreStats = mock(StoreStats.class); when(totalStoreStats.size()).thenReturn(ByteSizeValue.parseBytesSizeValue("100k", "mock_setting_name")); when(totalStats.getStore()).thenReturn(totalStoreStats); - //end mock common stats + // end mock common stats when(indexStats.getPrimaries()).thenReturn(primaryStats); when(indexStats.getTotal()).thenReturn(totalStats); @@ -210,7 +209,6 @@ public void test_run_failed() { assert (captor.getValue().getMessage().contains("failed to get settings")); } - @Test public void test_validate() { Tool tool = ListIndexTool.Factory.getInstance().create(Collections.emptyMap()); @@ -221,7 +219,11 @@ public void test_validate() { public void test_getDefaultDescription() { Tool.Factory factory = ListIndexTool.Factory.getInstance(); System.out.println(factory.getDefaultDescription()); - assert (factory.getDefaultDescription().equals("This tool gets index information from the OpenSearch cluster. It takes 2 optional arguments named `index` which is a comma-delimited list of one or more indices to get information from (default is an empty list meaning all indices), and `local` which means whether to return information from the local node only instead of the cluster manager node (default is false). The tool returns the indices information, including `health`, `status`, `index`, `uuid`, `pri`, `rep`, `docs.count`, `docs.deleted`, `store.size`, `pri.store. size `, `pri.store.size`, `pri.store`.")); + assert (factory + .getDefaultDescription() + .equals( + "This tool gets index information from the OpenSearch cluster. It takes 2 optional arguments named `index` which is a comma-delimited list of one or more indices to get information from (default is an empty list meaning all indices), and `local` which means whether to return information from the local node only instead of the cluster manager node (default is false). The tool returns the indices information, including `health`, `status`, `index`, `uuid`, `pri`, `rep`, `docs.count`, `docs.deleted`, `store.size`, `pri.store. size `, `pri.store.size`, `pri.store`." + )); } @Test From 3a0f699f34f7ad98894952cc6bb21eb4e8a22d94 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Fri, 14 Feb 2025 16:45:53 +0800 Subject: [PATCH 12/21] Fix UT failure Signed-off-by: zane-neo --- client/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/client/build.gradle b/client/build.gradle index de73fbc1a9..92a80a8780 100644 --- a/client/build.gradle +++ b/client/build.gradle @@ -17,6 +17,7 @@ dependencies { implementation project(path: ":${rootProject.name}-spi", configuration: 'shadow') implementation project(path: ":${rootProject.name}-common", configuration: 'shadow') compileOnly group: 'org.opensearch', name: 'opensearch', version: "${opensearch_version}" + testImplementation "org.opensearch.test:framework:${opensearch_version}" testImplementation group: 'junit', name: 'junit', version: '4.13.2' testImplementation group: 'org.mockito', name: 'mockito-core', version: '5.15.2' From 2a9030e348617efde79f683c81f00a8797adf930 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Mon, 17 Feb 2025 10:25:58 +0800 Subject: [PATCH 13/21] Change resource name to fix IT failure Signed-off-by: zane-neo --- ...ndexAgentRegistration.json => ListIndexAgentRegistration.json} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename plugin/src/test/resources/org/opensearch/ml/tools/{CatIndexAgentRegistration.json => ListIndexAgentRegistration.json} (100%) diff --git a/plugin/src/test/resources/org/opensearch/ml/tools/CatIndexAgentRegistration.json b/plugin/src/test/resources/org/opensearch/ml/tools/ListIndexAgentRegistration.json similarity index 100% rename from plugin/src/test/resources/org/opensearch/ml/tools/CatIndexAgentRegistration.json rename to plugin/src/test/resources/org/opensearch/ml/tools/ListIndexAgentRegistration.json From 498021a5893bfc80b7e03f99309157fb83a52ce0 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Fri, 21 Feb 2025 14:44:30 +0800 Subject: [PATCH 14/21] Add more UTs to increase coverage Signed-off-by: zane-neo --- .../ml/engine/tools/ListIndexTool.java | 2 +- .../ml/engine/tools/ListIndexToolTests.java | 83 +++++++++++++++++-- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java index cae0285138..7d21a86609 100644 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java +++ b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/ListIndexTool.java @@ -493,7 +493,7 @@ private Table buildTable( GetSettingsResponse getSettingsResponse, Queue> responses ) { - if (responses == null || responses.isEmpty()) { + if (responses == null || responses.isEmpty() || responses.peek().isEmpty()) { return null; } Tuple, Map> tuple = aggregateResults(responses); diff --git a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/ListIndexToolTests.java b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/ListIndexToolTests.java index 24d17c6074..822812e4fe 100644 --- a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/ListIndexToolTests.java +++ b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/ListIndexToolTests.java @@ -102,11 +102,47 @@ public void test_getType() { } @Test - public void test_run_successful() { + public void test_run_successful_1() { + mockUp(); + Tool tool = ListIndexTool.Factory.getInstance().create(Collections.emptyMap()); + verifyResult(tool, createParameters("[\"index-1\"]", "true", "10", "true")); + } + + @Test + public void test_run_successful_2() { + mockUp(); + Tool tool = ListIndexTool.Factory.getInstance().create(Collections.emptyMap()); + verifyResult(tool, createParameters(null, null, null, null)); + } + + private Map createParameters(String indices, String local, String pageSize, String includeUnloadedSegments) { Map parameters = new HashMap<>(); - parameters.put("indices", "[\"index-1\"]"); - parameters.put("page_size", "10"); + if (indices != null) { + parameters.put("indices", indices); + } + if (local != null) { + parameters.put("local", local); + } + if (pageSize != null) { + parameters.put("page_size", pageSize); + } + if (includeUnloadedSegments != null) { + parameters.put("include_unloaded_segments", includeUnloadedSegments); + } + return parameters; + } + private void verifyResult(Tool tool, Map parameters) { + ActionListener listener = mock(ActionListener.class); + ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); + tool.run(parameters, listener); + verify(listener).onResponse(captor.capture()); + System.out.println(captor.getValue()); + assert captor.getValue().contains("1,red,open,index-1"); + assert captor.getValue().contains("5,1,100,10,100kb,100kb"); + } + + private void mockUp() { doAnswer(invocation -> { ActionListener actionListener = invocation.getArgument(1); GetSettingsResponse response = mock(GetSettingsResponse.class); @@ -177,15 +213,50 @@ public void test_run_successful() { actionListener.onResponse(response); return null; }).when(clusterAdminClient).health(any(ClusterHealthRequest.class), isA(ActionListener.class)); + } + @Test + public void test_run_withEmptyTableResult() { + Map parameters = createParameters("[\"index-1\"]", "true", "10", "true"); Tool tool = ListIndexTool.Factory.getInstance().create(Collections.emptyMap()); + doAnswer(invocation -> { + ActionListener actionListener = invocation.getArgument(1); + GetSettingsResponse response = mock(GetSettingsResponse.class); + Map indexToSettings = new HashMap<>(); + indexToSettings.put("index-1", Settings.EMPTY); + indexToSettings.put("index-2", Settings.EMPTY); + when(response.getIndexToSettings()).thenReturn(indexToSettings); + actionListener.onResponse(response); + return null; + }).when(indicesAdminClient).getSettings(any(GetSettingsRequest.class), isA(ActionListener.class)); + + // clusterStateResponse.getState().getMetadata().spliterator() + doAnswer(invocation -> { + ActionListener actionListener = invocation.getArgument(1); + ClusterStateResponse response = mock(ClusterStateResponse.class); + when(response.getState()).thenReturn(clusterState); + actionListener.onResponse(response); + return null; + }).when(clusterAdminClient).state(any(ClusterStateRequest.class), isA(ActionListener.class)); + + doAnswer(invocation -> { + ActionListener actionListener = invocation.getArgument(1); + actionListener.onResponse(null); + return null; + }).when(indicesAdminClient).stats(any(IndicesStatsRequest.class), isA(ActionListener.class)); + + doAnswer(invocation -> { + ActionListener actionListener = invocation.getArgument(1); + actionListener.onResponse(null); + return null; + }).when(clusterAdminClient).health(any(ClusterHealthRequest.class), isA(ActionListener.class)); + ActionListener listener = mock(ActionListener.class); ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); tool.run(parameters, listener); verify(listener).onResponse(captor.capture()); System.out.println(captor.getValue()); - assert captor.getValue().contains("1,red,open,index-1"); - assert captor.getValue().contains("5,1,100,10,100kb,100kb"); + assert captor.getValue().contains("There were no results searching the indices parameter"); } @Test @@ -213,6 +284,8 @@ public void test_run_failed() { public void test_validate() { Tool tool = ListIndexTool.Factory.getInstance().create(Collections.emptyMap()); assert tool.validate(ImmutableMap.of("runtimeParameter", "value1")); + assert !tool.validate(null); + assert !tool.validate(Collections.emptyMap()); } @Test From c3acd859ae82dd63cbbe509d3bc7a318b55e1f83 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Sat, 1 Mar 2025 09:46:10 +0800 Subject: [PATCH 15/21] Remove CatIndexTool to keep only ListIndexTool Signed-off-by: zane-neo --- .../ml/engine/tools/CatIndexTool.java | 462 ------------------ .../ml/engine/tools/CatIndexToolTests.java | 248 ---------- test.json | 288 +++++++++++ 3 files changed, 288 insertions(+), 710 deletions(-) delete mode 100644 ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java delete mode 100644 ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/CatIndexToolTests.java create mode 100644 test.json diff --git a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java b/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java deleted file mode 100644 index d47d294b0f..0000000000 --- a/ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/CatIndexTool.java +++ /dev/null @@ -1,462 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.ml.engine.tools; - -import static org.opensearch.action.support.clustermanager.ClusterManagerNodeRequest.DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT; -import static org.opensearch.ml.common.utils.StringUtils.gson; - -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Spliterators; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.Function; -import java.util.stream.Collectors; -import java.util.stream.StreamSupport; - -import org.apache.logging.log4j.util.Strings; -import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; -import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; -import org.opensearch.action.admin.cluster.state.ClusterStateRequest; -import org.opensearch.action.admin.cluster.state.ClusterStateResponse; -import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest; -import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse; -import org.opensearch.action.admin.indices.stats.CommonStats; -import org.opensearch.action.admin.indices.stats.IndexStats; -import org.opensearch.action.admin.indices.stats.IndicesStatsRequest; -import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; -import org.opensearch.action.support.GroupedActionListener; -import org.opensearch.action.support.IndicesOptions; -import org.opensearch.cluster.health.ClusterIndexHealth; -import org.opensearch.cluster.metadata.IndexMetadata; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.Table; -import org.opensearch.common.Table.Cell; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.unit.TimeValue; -import org.opensearch.core.action.ActionListener; -import org.opensearch.core.action.ActionResponse; -import org.opensearch.index.IndexSettings; -import org.opensearch.ml.common.output.model.ModelTensors; -import org.opensearch.ml.common.spi.tools.Parser; -import org.opensearch.ml.common.spi.tools.Tool; -import org.opensearch.ml.common.spi.tools.ToolAnnotation; -import org.opensearch.transport.client.Client; - -import lombok.Getter; -import lombok.Setter; - -@ToolAnnotation(CatIndexTool.TYPE) -public class CatIndexTool implements Tool { - public static final String TYPE = "CatIndexTool"; - private static final String DEFAULT_DESCRIPTION = String - .join( - " ", - "This tool gets index information from the OpenSearch cluster.", - "It takes 2 optional arguments named `index` which is a comma-delimited list of one or more indices to get information from (default is an empty list meaning all indices),", - "and `local` which means whether to return information from the local node only instead of the cluster manager node (default is false).", - "The tool returns the indices information, including `health`, `status`, `index`, `uuid`, `pri`, `rep`, `docs.count`, `docs.deleted`, `store.size`, `pri.store. size `, `pri.store.size`, `pri.store`." - ); - - @Setter - @Getter - private String name = CatIndexTool.TYPE; - @Getter - @Setter - private String description = DEFAULT_DESCRIPTION; - @Getter - private String version; - - private Client client; - @Setter - private Parser inputParser; - @Setter - private Parser outputParser; - @SuppressWarnings("unused") - private ClusterService clusterService; - - public CatIndexTool(Client client, ClusterService clusterService) { - this.client = client; - this.clusterService = clusterService; - - outputParser = new Parser<>() { - @Override - public Object parse(Object o) { - @SuppressWarnings("unchecked") - List mlModelOutputs = (List) o; - return mlModelOutputs.get(0).getMlModelTensors().get(0).getDataAsMap().get("response"); - } - }; - } - - @Override - public void run(Map parameters, ActionListener listener) { - // TODO: This logic exactly matches the OpenSearch _cat/indices REST action. If code at - // o.o.rest/action/cat/RestIndicesAction.java changes those changes need to be reflected here - // https://github.com/opensearch-project/ml-commons/pull/1582#issuecomment-1796962876 - @SuppressWarnings("unchecked") - List indexList = parameters.containsKey("indices") - ? gson.fromJson(parameters.get("indices"), List.class) - : Collections.emptyList(); - final String[] indices = indexList.toArray(Strings.EMPTY_ARRAY); - - final IndicesOptions indicesOptions = IndicesOptions.strictExpand(); - final boolean local = parameters.containsKey("local") ? Boolean.parseBoolean("local") : false; - final TimeValue clusterManagerNodeTimeout = DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT; - final boolean includeUnloadedSegments = Boolean.parseBoolean(parameters.get("include_unloaded_segments")); - - final ActionListener
internalListener = ActionListener.notifyOnce(ActionListener.wrap(table -> { - // Handle empty table - if (table.getRows().isEmpty()) { - @SuppressWarnings("unchecked") - T empty = (T) ("There were no results searching the indices parameter [" + parameters.get("indices") + "]."); - listener.onResponse(empty); - return; - } - StringBuilder sb = new StringBuilder( - // Currently using c.value which is short header matching _cat/indices - // May prefer to use c.attr.get("desc") for full description - table.getHeaders().stream().map(c -> c.value.toString()).collect(Collectors.joining(",", "", "\n")) - ); - for (List row : table.getRows()) { - sb.append(row.stream().map(c -> c.value == null ? null : c.value.toString()).collect(Collectors.joining(",", "", "\n"))); - } - @SuppressWarnings("unchecked") - T response = (T) sb.toString(); - listener.onResponse(response); - }, listener::onFailure)); - - sendGetSettingsRequest( - indices, - indicesOptions, - local, - clusterManagerNodeTimeout, - client, - new ActionListener() { - @Override - public void onResponse(final GetSettingsResponse getSettingsResponse) { - final GroupedActionListener groupedListener = createGroupedListener(4, internalListener); - groupedListener.onResponse(getSettingsResponse); - - // The list of indices that will be returned is determined by the indices returned from the Get Settings call. - // All the other requests just provide additional detail, and wildcards may be resolved differently depending on the - // type of request in the presence of security plugins (looking at you, ClusterHealthRequest), so - // force the IndicesOptions for all the sub-requests to be as inclusive as possible. - final IndicesOptions subRequestIndicesOptions = IndicesOptions.lenientExpandHidden(); - - sendIndicesStatsRequest( - indices, - subRequestIndicesOptions, - includeUnloadedSegments, - client, - ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) - ); - sendClusterStateRequest( - indices, - subRequestIndicesOptions, - local, - clusterManagerNodeTimeout, - client, - ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) - ); - sendClusterHealthRequest( - indices, - subRequestIndicesOptions, - local, - clusterManagerNodeTimeout, - client, - ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure) - ); - } - - @Override - public void onFailure(final Exception e) { - internalListener.onFailure(e); - } - } - ); - } - - @Override - public String getType() { - return TYPE; - } - - /** - * We're using the Get Settings API here to resolve the authorized indices for the user. - * This is because the Cluster State and Cluster Health APIs do not filter output based - * on index privileges, so they can't be used to determine which indices are authorized - * or not. On top of this, the Indices Stats API cannot be used either to resolve indices - * as it does not provide information for all existing indices (for example recovering - * indices or non replicated closed indices are not reported in indices stats response). - */ - private void sendGetSettingsRequest( - final String[] indices, - final IndicesOptions indicesOptions, - final boolean local, - final TimeValue clusterManagerNodeTimeout, - final Client client, - final ActionListener listener - ) { - final GetSettingsRequest request = new GetSettingsRequest(); - request.indices(indices); - request.indicesOptions(indicesOptions); - request.local(local); - request.clusterManagerNodeTimeout(clusterManagerNodeTimeout); - request.names(IndexSettings.INDEX_SEARCH_THROTTLED.getKey()); - - client.admin().indices().getSettings(request, listener); - } - - private void sendClusterStateRequest( - final String[] indices, - final IndicesOptions indicesOptions, - final boolean local, - final TimeValue clusterManagerNodeTimeout, - final Client client, - final ActionListener listener - ) { - - final ClusterStateRequest request = new ClusterStateRequest(); - request.indices(indices); - request.indicesOptions(indicesOptions); - request.local(local); - request.clusterManagerNodeTimeout(clusterManagerNodeTimeout); - - client.admin().cluster().state(request, listener); - } - - private void sendClusterHealthRequest( - final String[] indices, - final IndicesOptions indicesOptions, - final boolean local, - final TimeValue clusterManagerNodeTimeout, - final Client client, - final ActionListener listener - ) { - - final ClusterHealthRequest request = new ClusterHealthRequest(); - request.indices(indices); - request.indicesOptions(indicesOptions); - request.local(local); - request.clusterManagerNodeTimeout(clusterManagerNodeTimeout); - - client.admin().cluster().health(request, listener); - } - - private void sendIndicesStatsRequest( - final String[] indices, - final IndicesOptions indicesOptions, - final boolean includeUnloadedSegments, - final Client client, - final ActionListener listener - ) { - - final IndicesStatsRequest request = new IndicesStatsRequest(); - request.indices(indices); - request.indicesOptions(indicesOptions); - request.all(); - request.includeUnloadedSegments(includeUnloadedSegments); - - client.admin().indices().stats(request, listener); - } - - private GroupedActionListener createGroupedListener(final int size, final ActionListener
listener) { - return new GroupedActionListener<>(new ActionListener>() { - @Override - public void onResponse(final Collection responses) { - try { - GetSettingsResponse settingsResponse = extractResponse(responses, GetSettingsResponse.class); - Map indicesSettings = StreamSupport - .stream(Spliterators.spliterator(settingsResponse.getIndexToSettings().entrySet(), 0), false) - .collect(Collectors.toMap(cursor -> cursor.getKey(), cursor -> cursor.getValue())); - - ClusterStateResponse stateResponse = extractResponse(responses, ClusterStateResponse.class); - Map indicesStates = StreamSupport - .stream(stateResponse.getState().getMetadata().spliterator(), false) - .collect(Collectors.toMap(indexMetadata -> indexMetadata.getIndex().getName(), Function.identity())); - - ClusterHealthResponse healthResponse = extractResponse(responses, ClusterHealthResponse.class); - Map indicesHealths = healthResponse.getIndices(); - - IndicesStatsResponse statsResponse = extractResponse(responses, IndicesStatsResponse.class); - Map indicesStats = statsResponse.getIndices(); - - Table responseTable = buildTable(indicesSettings, indicesHealths, indicesStats, indicesStates); - listener.onResponse(responseTable); - } catch (Exception e) { - onFailure(e); - } - } - - @Override - public void onFailure(final Exception e) { - listener.onFailure(e); - } - }, size); - } - - @Override - public boolean validate(Map parameters) { - return parameters != null && !parameters.isEmpty(); - } - - /** - * Factory for the {@link CatIndexTool} - */ - public static class Factory implements Tool.Factory { - private Client client; - private ClusterService clusterService; - - private static Factory INSTANCE; - - /** - * Create or return the singleton factory instance - */ - public static Factory getInstance() { - if (INSTANCE != null) { - return INSTANCE; - } - synchronized (CatIndexTool.class) { - if (INSTANCE != null) { - return INSTANCE; - } - INSTANCE = new Factory(); - return INSTANCE; - } - } - - /** - * Initialize this factory - * @param client The OpenSearch client - * @param clusterService The OpenSearch cluster service - */ - public void init(Client client, ClusterService clusterService) { - this.client = client; - this.clusterService = clusterService; - } - - @Override - public CatIndexTool create(Map map) { - return new CatIndexTool(client, clusterService); - } - - @Override - public String getDefaultDescription() { - return DEFAULT_DESCRIPTION; - } - - @Override - public String getDefaultType() { - return TYPE; - } - - @Override - public String getDefaultVersion() { - return null; - } - } - - private Table getTableWithHeader() { - Table table = new Table(); - table.startHeaders(); - // First param is cell.value which is currently returned - // Second param is cell.attr we may want to use attr.desc in the future - table.addCell("row", "alias:r;desc:row number"); - table.addCell("health", "alias:h;desc:current health status"); - table.addCell("status", "alias:s;desc:open/close status"); - table.addCell("index", "alias:i,idx;desc:index name"); - table.addCell("uuid", "alias:id,uuid;desc:index uuid"); - table - .addCell( - "pri(number of primary shards)", - "alias:p,shards.primary,shardsPrimary;text-align:right;desc:number of primary shards" - ); - table - .addCell( - "rep(number of replica shards)", - "alias:r,shards.replica,shardsReplica;text-align:right;desc:number of replica shards" - ); - table.addCell("docs.count(number of available documents)", "alias:dc,docsCount;text-align:right;desc:available docs"); - table.addCell("docs.deleted(number of deleted documents)", "alias:dd,docsDeleted;text-align:right;desc:deleted docs"); - table - .addCell( - "store.size(store size of primary and replica shards)", - "sibling:pri;alias:ss,storeSize;text-align:right;desc:store size of primaries & replicas" - ); - table.addCell("pri.store.size(store size of primary shards)", "text-align:right;desc:store size of primaries"); - // Above includes all the default fields for cat indices. See RestIndicesAction for a lot more that could be included. - table.endHeaders(); - return table; - } - - private Table buildTable( - final Map indicesSettings, - final Map indicesHealths, - final Map indicesStats, - final Map indicesMetadatas - ) { - final Table table = getTableWithHeader(); - AtomicInteger rowNum = new AtomicInteger(0); - indicesSettings.forEach((indexName, settings) -> { - if (!indicesMetadatas.containsKey(indexName)) { - // the index exists in the Get Indices response but is not present in the cluster state: - // it is likely that the index was deleted in the meanwhile, so we ignore it. - return; - } - - final IndexMetadata indexMetadata = indicesMetadatas.get(indexName); - final IndexMetadata.State indexState = indexMetadata.getState(); - final IndexStats indexStats = indicesStats.get(indexName); - - final String health; - final ClusterIndexHealth indexHealth = indicesHealths.get(indexName); - if (indexHealth != null) { - health = indexHealth.getStatus().toString().toLowerCase(Locale.ROOT); - } else if (indexStats != null) { - health = "red*"; - } else { - health = ""; - } - - final CommonStats primaryStats; - final CommonStats totalStats; - - if (indexStats == null || indexState == IndexMetadata.State.CLOSE) { - primaryStats = new CommonStats(); - totalStats = new CommonStats(); - } else { - primaryStats = indexStats.getPrimaries(); - totalStats = indexStats.getTotal(); - } - table.startRow(); - table.addCell(rowNum.addAndGet(1)); - table.addCell(health); - table.addCell(indexState.toString().toLowerCase(Locale.ROOT)); - table.addCell(indexName); - table.addCell(indexMetadata.getIndexUUID()); - table.addCell(indexHealth == null ? null : indexHealth.getNumberOfShards()); - table.addCell(indexHealth == null ? null : indexHealth.getNumberOfReplicas()); - - table.addCell(primaryStats.getDocs() == null ? null : primaryStats.getDocs().getCount()); - table.addCell(primaryStats.getDocs() == null ? null : primaryStats.getDocs().getDeleted()); - - table.addCell(totalStats.getStore() == null ? null : totalStats.getStore().size()); - table.addCell(primaryStats.getStore() == null ? null : primaryStats.getStore().size()); - - table.endRow(); - }); - - return table; - } - - @SuppressWarnings("unchecked") - private static A extractResponse(final Collection responses, Class c) { - return (A) responses.stream().filter(c::isInstance).findFirst().get(); - } -} diff --git a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/CatIndexToolTests.java b/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/CatIndexToolTests.java deleted file mode 100644 index 31dd909d1a..0000000000 --- a/ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/CatIndexToolTests.java +++ /dev/null @@ -1,248 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.ml.engine.tools; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Arrays; -import java.util.Collections; -import java.util.Iterator; -import java.util.Map; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.TimeUnit; - -import org.junit.Before; -import org.junit.Test; -import org.mockito.ArgumentCaptor; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; -import org.opensearch.Version; -import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; -import org.opensearch.action.admin.cluster.state.ClusterStateResponse; -import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse; -import org.opensearch.action.admin.indices.stats.CommonStats; -import org.opensearch.action.admin.indices.stats.CommonStatsFlags; -import org.opensearch.action.admin.indices.stats.IndexStats; -import org.opensearch.action.admin.indices.stats.IndexStats.IndexStatsBuilder; -import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; -import org.opensearch.action.admin.indices.stats.ShardStats; -import org.opensearch.cluster.ClusterState; -import org.opensearch.cluster.health.ClusterIndexHealth; -import org.opensearch.cluster.metadata.IndexMetadata; -import org.opensearch.cluster.metadata.IndexMetadata.State; -import org.opensearch.cluster.metadata.Metadata; -import org.opensearch.cluster.routing.IndexRoutingTable; -import org.opensearch.cluster.routing.IndexShardRoutingTable; -import org.opensearch.cluster.routing.ShardRouting; -import org.opensearch.cluster.routing.ShardRoutingState; -import org.opensearch.cluster.routing.TestShardRouting; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.UUIDs; -import org.opensearch.common.settings.Settings; -import org.opensearch.core.action.ActionListener; -import org.opensearch.core.index.Index; -import org.opensearch.core.index.shard.ShardId; -import org.opensearch.index.shard.ShardPath; -import org.opensearch.ml.common.spi.tools.Tool; -import org.opensearch.ml.engine.tools.CatIndexTool.Factory; -import org.opensearch.transport.client.AdminClient; -import org.opensearch.transport.client.Client; -import org.opensearch.transport.client.ClusterAdminClient; -import org.opensearch.transport.client.IndicesAdminClient; - -public class CatIndexToolTests { - - @Mock - private Client client; - @Mock - private AdminClient adminClient; - @Mock - private IndicesAdminClient indicesAdminClient; - @Mock - private ClusterAdminClient clusterAdminClient; - @Mock - private ClusterService clusterService; - @Mock - private ClusterState clusterState; - @Mock - private Metadata metadata; - @Mock - private GetSettingsResponse getSettingsResponse; - @Mock - private IndicesStatsResponse indicesStatsResponse; - @Mock - private ClusterStateResponse clusterStateResponse; - @Mock - private ClusterHealthResponse clusterHealthResponse; - @Mock - private IndexMetadata indexMetadata; - @Mock - private IndexRoutingTable indexRoutingTable; - - private Map indicesParams; - private Map otherParams; - private Map emptyParams; - - @Before - public void setup() { - MockitoAnnotations.openMocks(this); - - when(adminClient.indices()).thenReturn(indicesAdminClient); - when(adminClient.cluster()).thenReturn(clusterAdminClient); - when(client.admin()).thenReturn(adminClient); - - when(indexMetadata.getState()).thenReturn(State.OPEN); - when(indexMetadata.getCreationVersion()).thenReturn(Version.CURRENT); - - when(metadata.index(any(String.class))).thenReturn(indexMetadata); - when(clusterState.metadata()).thenReturn(metadata); - when(clusterService.state()).thenReturn(clusterState); - - CatIndexTool.Factory.getInstance().init(client, clusterService); - - indicesParams = Map.of("index", "[\"foo\"]"); - otherParams = Map.of("other", "[\"bar\"]"); - emptyParams = Collections.emptyMap(); - } - - @Test - public void testRunAsyncNoIndices() throws Exception { - @SuppressWarnings("unchecked") - ArgumentCaptor> settingsActionListenerCaptor = ArgumentCaptor.forClass(ActionListener.class); - doNothing().when(indicesAdminClient).getSettings(any(), settingsActionListenerCaptor.capture()); - - @SuppressWarnings("unchecked") - ArgumentCaptor> statsActionListenerCaptor = ArgumentCaptor.forClass(ActionListener.class); - doNothing().when(indicesAdminClient).stats(any(), statsActionListenerCaptor.capture()); - - @SuppressWarnings("unchecked") - ArgumentCaptor> clusterStateActionListenerCaptor = ArgumentCaptor - .forClass(ActionListener.class); - doNothing().when(clusterAdminClient).state(any(), clusterStateActionListenerCaptor.capture()); - - @SuppressWarnings("unchecked") - ArgumentCaptor> clusterHealthActionListenerCaptor = ArgumentCaptor - .forClass(ActionListener.class); - doNothing().when(clusterAdminClient).health(any(), clusterHealthActionListenerCaptor.capture()); - - when(getSettingsResponse.getIndexToSettings()).thenReturn(Collections.emptyMap()); - when(indicesStatsResponse.getIndices()).thenReturn(Collections.emptyMap()); - when(clusterStateResponse.getState()).thenReturn(clusterState); - when(clusterState.getMetadata()).thenReturn(metadata); - when(metadata.spliterator()).thenReturn(Arrays.spliterator(new IndexMetadata[0])); - - when(clusterHealthResponse.getIndices()).thenReturn(Collections.emptyMap()); - - Tool tool = CatIndexTool.Factory.getInstance().create(Collections.emptyMap()); - final CompletableFuture future = new CompletableFuture<>(); - ActionListener listener = ActionListener.wrap(r -> { future.complete(r); }, e -> { future.completeExceptionally(e); }); - - tool.run(otherParams, listener); - settingsActionListenerCaptor.getValue().onResponse(getSettingsResponse); - statsActionListenerCaptor.getValue().onResponse(indicesStatsResponse); - clusterStateActionListenerCaptor.getValue().onResponse(clusterStateResponse); - clusterHealthActionListenerCaptor.getValue().onResponse(clusterHealthResponse); - - future.join(); - assertEquals("There were no results searching the indices parameter [null].", future.get()); - } - - @Test - public void testRunAsyncIndexStats() throws Exception { - String indexName = "foo"; - Index index = new Index(indexName, UUIDs.base64UUID()); - - @SuppressWarnings("unchecked") - ArgumentCaptor> settingsActionListenerCaptor = ArgumentCaptor.forClass(ActionListener.class); - doNothing().when(indicesAdminClient).getSettings(any(), settingsActionListenerCaptor.capture()); - - @SuppressWarnings("unchecked") - ArgumentCaptor> statsActionListenerCaptor = ArgumentCaptor.forClass(ActionListener.class); - doNothing().when(indicesAdminClient).stats(any(), statsActionListenerCaptor.capture()); - - @SuppressWarnings("unchecked") - ArgumentCaptor> clusterStateActionListenerCaptor = ArgumentCaptor - .forClass(ActionListener.class); - doNothing().when(clusterAdminClient).state(any(), clusterStateActionListenerCaptor.capture()); - - @SuppressWarnings("unchecked") - ArgumentCaptor> clusterHealthActionListenerCaptor = ArgumentCaptor - .forClass(ActionListener.class); - doNothing().when(clusterAdminClient).health(any(), clusterHealthActionListenerCaptor.capture()); - - when(getSettingsResponse.getIndexToSettings()).thenReturn(Map.of("foo", Settings.EMPTY)); - - int shardId = 0; - ShardId shId = new ShardId(index, shardId); - Path path = Files.createTempDirectory("temp").resolve("indices").resolve(index.getUUID()).resolve(String.valueOf(shardId)); - ShardPath shardPath = new ShardPath(false, path, path, shId); - ShardRouting routing = TestShardRouting.newShardRouting(shId, "node", true, ShardRoutingState.STARTED); - CommonStats commonStats = new CommonStats(CommonStatsFlags.ALL); - IndexStats fooStats = new IndexStatsBuilder(index.getName(), index.getUUID()) - .add(new ShardStats(routing, shardPath, commonStats, null, null, null, null)) - .build(); - when(indicesStatsResponse.getIndices()).thenReturn(Map.of(indexName, fooStats)); - - when(indexMetadata.getIndex()).thenReturn(index); - when(indexMetadata.getNumberOfShards()).thenReturn(5); - when(indexMetadata.getNumberOfReplicas()).thenReturn(1); - when(clusterStateResponse.getState()).thenReturn(clusterState); - when(clusterState.getMetadata()).thenReturn(metadata); - when(metadata.spliterator()).thenReturn(Arrays.spliterator(new IndexMetadata[] { indexMetadata })); - @SuppressWarnings("unchecked") - Iterator iterator = (Iterator) mock(Iterator.class); - when(iterator.hasNext()).thenReturn(false); - when(indexRoutingTable.iterator()).thenReturn(iterator); - ClusterIndexHealth fooHealth = new ClusterIndexHealth(indexMetadata, indexRoutingTable); - when(clusterHealthResponse.getIndices()).thenReturn(Map.of(indexName, fooHealth)); - - // Now make the call - Tool tool = CatIndexTool.Factory.getInstance().create(Collections.emptyMap()); - final CompletableFuture future = new CompletableFuture<>(); - ActionListener listener = ActionListener.wrap(r -> { future.complete(r); }, e -> { future.completeExceptionally(e); }); - - tool.run(otherParams, listener); - settingsActionListenerCaptor.getValue().onResponse(getSettingsResponse); - statsActionListenerCaptor.getValue().onResponse(indicesStatsResponse); - clusterStateActionListenerCaptor.getValue().onResponse(clusterStateResponse); - clusterHealthActionListenerCaptor.getValue().onResponse(clusterHealthResponse); - - future.orTimeout(10, TimeUnit.SECONDS).join(); - String response = future.get(); - String[] responseRows = response.trim().split("\\n"); - - assertEquals(2, responseRows.length); - String header = responseRows[0]; - String fooRow = responseRows[1]; - assertEquals(header.split("\\t").length, fooRow.split("\\t").length); - assertEquals( - "row,health,status,index,uuid,pri(number of primary shards),rep(number of replica shards),docs.count(number of available documents),docs.deleted(number of deleted documents),store.size(store size of primary and replica shards),pri.store.size(store size of primary shards)", - header - ); - assertEquals("1,red,open,foo,null,5,1,0,0,0b,0b", fooRow); - } - - @Test - public void testTool() { - Factory instance = CatIndexTool.Factory.getInstance(); - assertEquals(instance, CatIndexTool.Factory.getInstance()); - assertTrue(instance.getDefaultDescription().contains("tool")); - - Tool tool = instance.create(Collections.emptyMap()); - assertEquals(CatIndexTool.TYPE, tool.getType()); - assertTrue(tool.validate(indicesParams)); - assertTrue(tool.validate(otherParams)); - assertFalse(tool.validate(emptyParams)); - } -} diff --git a/test.json b/test.json new file mode 100644 index 0000000000..2d7c04a9d4 --- /dev/null +++ b/test.json @@ -0,0 +1,288 @@ +{ + "tasks": [ + { + "attachments": [ + { + "id": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", + "type": "ElasticNetworkInterface", + "status": "ATTACHED", + "details": [ + { + "name": "subnetId", + "value": "subnet-00ca627ce0bf77d96" + }, + { + "name": "networkInterfaceId", + "value": "eni-02cbc098ebafe7fca" + }, + { + "name": "macAddress", + "value": "02:3d:9d:3c:1e:01" + }, + { + "name": "privateDnsName", + "value": "ip-10-0-111-192.us-west-2.compute.internal" + }, + { + "name": "privateIPv4Address", + "value": "10.0.111.192" + }, + { + "name": "ipv6Address", + "value": "2600:1f14:39ec:1600:8999:d697:246d:7b68" + } + ] + } + ], + "attributes": [ + { + "name": "ecs.cpu-architecture", + "value": "x86_64" + } + ], + "availabilityZone": "us-west-2a", + "clusterArn": "arn:aws:ecs:us-west-2:010438497335:cluster/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5", + "connectivity": "CONNECTED", + "connectivityAt": "2025-02-27T15:50:18.099000+08:00", + "containers": [ + { + "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/06eece53-9950-42c0-9d7f-09af80c79888", + "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", + "name": "juno-dns-resolver", + "image": "010438497335.dkr.ecr.us-west-2.amazonaws.com/barsecrrepo-8229a1e80a70cd762efb04c3eac4711df9843cc3:cc90fcb3-7a0a-4c9a-9743-fcbe72bd2abb_juno-dns-resolver_main", + "imageDigest": "sha256:5c5808f185ef244722f39c71f60feb4b0840ce9f852568d189a008278499d28a", + "runtimeId": "3695993a81e7424988f0426ff565ac22-1664593694", + "lastStatus": "RUNNING", + "networkBindings": [], + "networkInterfaces": [ + { + "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", + "privateIpv4Address": "10.0.111.192", + "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" + } + ], + "healthStatus": "UNKNOWN", + "managedAgents": [ + { + "lastStartedAt": "2025-02-27T15:52:21.740000+08:00", + "name": "ExecuteCommandAgent", + "lastStatus": "RUNNING" + } + ], + "cpu": "0", + "memory": "512" + }, + { + "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/3139d8d9-0152-4d2e-a8f3-5a2dbc08e9f8", + "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", + "name": "FireLensContainer", + "image": "010438497335.dkr.ecr.us-west-2.amazonaws.com/barsecrrepo-8229a1e80a70cd762efb04c3eac4711df9843cc3:cc90fcb3-7a0a-4c9a-9743-fcbe72bd2abb_logging_container_main", + "imageDigest": "sha256:ed028eb5260ea65c9d09084ad790e42946468a334299d6e86a03c22a4ef0fb6b", + "runtimeId": "3695993a81e7424988f0426ff565ac22-0330807724", + "lastStatus": "RUNNING", + "networkBindings": [], + "networkInterfaces": [ + { + "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", + "privateIpv4Address": "10.0.111.192", + "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" + } + ], + "healthStatus": "HEALTHY", + "managedAgents": [ + { + "lastStartedAt": "2025-02-27T15:52:21.740000+08:00", + "name": "ExecuteCommandAgent", + "lastStatus": "RUNNING" + } + ], + "cpu": "256", + "memory": "512" + }, + { + "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/417c64d4-5e67-4cc5-974e-3ef0bc12336b", + "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", + "name": "security-init", + "image": "793837233617.dkr.ecr.us-west-2.amazonaws.com/juno-security-agent-al2:beta-us-west-2", + "imageDigest": "sha256:09bd53fb90e35b5cc0911c8791c9836567023a117296e07a95ad470b3628744c", + "runtimeId": "3695993a81e7424988f0426ff565ac22-1628632744", + "lastStatus": "STOPPED", + "exitCode": 0, + "networkBindings": [], + "networkInterfaces": [ + { + "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", + "privateIpv4Address": "10.0.111.192", + "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" + } + ], + "healthStatus": "UNKNOWN", + "managedAgents": [ + { + "name": "ExecuteCommandAgent", + "lastStatus": "STOPPED" + } + ], + "cpu": "0", + "memory": "256" + }, + { + "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/9424d489-1519-4834-bb25-8c741f47c6da", + "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", + "name": "security-envoy", + "image": "010438497335.dkr.ecr.us-west-2.amazonaws.com/barsecrrepo-8229a1e80a70cd762efb04c3eac4711df9843cc3:cc90fcb3-7a0a-4c9a-9743-fcbe72bd2abb_security-envoy-image_main", + "imageDigest": "sha256:26a76225526d7dd7b6a044fcbfa90df361fdf53a3c3463fe011b3387fc7a9f3e", + "runtimeId": "3695993a81e7424988f0426ff565ac22-0975025829", + "lastStatus": "RUNNING", + "networkBindings": [], + "networkInterfaces": [ + { + "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", + "privateIpv4Address": "10.0.111.192", + "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" + } + ], + "healthStatus": "UNKNOWN", + "managedAgents": [ + { + "lastStartedAt": "2025-02-27T15:52:35.581000+08:00", + "name": "ExecuteCommandAgent", + "lastStatus": "RUNNING" + } + ], + "cpu": "1024", + "memory": "2048" + }, + { + "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/949114bb-e1ee-4b54-b702-87ec54a6d895", + "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", + "name": "security-agent", + "image": "793837233617.dkr.ecr.us-west-2.amazonaws.com/juno-security-agent-al2:beta-us-west-2", + "imageDigest": "sha256:09bd53fb90e35b5cc0911c8791c9836567023a117296e07a95ad470b3628744c", + "runtimeId": "3695993a81e7424988f0426ff565ac22-3319817687", + "lastStatus": "RUNNING", + "networkBindings": [], + "networkInterfaces": [ + { + "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", + "privateIpv4Address": "10.0.111.192", + "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" + } + ], + "healthStatus": "HEALTHY", + "managedAgents": [ + { + "lastStartedAt": "2025-02-27T15:52:21.740000+08:00", + "name": "ExecuteCommandAgent", + "lastStatus": "RUNNING" + } + ], + "cpu": "512" + }, + { + "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/a656f548-f8ed-40c6-97b4-55d47f9f658a", + "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", + "name": "security-watchdog", + "image": "244179523386.dkr.ecr.us-west-2.amazonaws.com/juno-security-watchdog-al2", + "imageDigest": "sha256:48bbc42ce3a7911e151c275fd24cc0b1de9ccced51f7bcfb8c758d5af62c6567", + "runtimeId": "3695993a81e7424988f0426ff565ac22-1145597981", + "lastStatus": "RUNNING", + "networkBindings": [], + "networkInterfaces": [ + { + "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", + "privateIpv4Address": "10.0.111.192", + "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" + } + ], + "healthStatus": "UNKNOWN", + "managedAgents": [ + { + "lastStartedAt": "2025-02-27T15:52:35.581000+08:00", + "name": "ExecuteCommandAgent", + "lastStatus": "RUNNING" + } + ], + "cpu": "0", + "memory": "256" + }, + { + "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/f66ef549-18c1-4245-96a2-8bd27d6d59bb", + "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", + "name": "Application", + "image": "010438497335.dkr.ecr.us-west-2.amazonaws.com/barsecrrepo-8229a1e80a70cd762efb04c3eac4711df9843cc3:zaniu-skills-oasis-test", + "imageDigest": "sha256:0bcda619bd87394cff658b83cffea8f107760ad3c87f46c59461d38090fd00ea", + "runtimeId": "3695993a81e7424988f0426ff565ac22-0581894629", + "lastStatus": "RUNNING", + "networkBindings": [], + "networkInterfaces": [ + { + "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", + "privateIpv4Address": "10.0.111.192", + "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" + } + ], + "healthStatus": "UNKNOWN", + "managedAgents": [ + { + "lastStartedAt": "2025-02-27T15:52:21.740000+08:00", + "name": "ExecuteCommandAgent", + "lastStatus": "RUNNING" + } + ], + "cpu": "1024", + "memory": "2048" + } + ], + "cpu": "8192", + "createdAt": "2025-02-27T15:50:14.674000+08:00", + "desiredStatus": "RUNNING", + "enableExecuteCommand": true, + "group": "service:AWSSearchServicesOasisServiceCDK-OnePodEcsService-beta-us-west-2-cell-1-ServiceD69D759B-3u9sx6il5m4Z", + "healthStatus": "HEALTHY", + "lastStatus": "RUNNING", + "launchType": "FARGATE", + "memory": "16384", + "overrides": { + "containerOverrides": [ + { + "name": "FireLensContainer" + }, + { + "name": "security-envoy" + }, + { + "name": "security-agent" + }, + { + "name": "juno-dns-resolver" + }, + { + "name": "security-init" + }, + { + "name": "Application" + }, + { + "name": "security-watchdog" + } + ], + "inferenceAcceleratorOverrides": [] + }, + "platformVersion": "1.4.0", + "platformFamily": "Linux", + "pullStartedAt": "2025-02-27T15:50:25.413000+08:00", + "pullStoppedAt": "2025-02-27T15:51:37.324000+08:00", + "startedAt": "2025-02-27T15:52:40.082000+08:00", + "startedBy": "ecs-svc/1804817061908948128", + "tags": [], + "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", + "taskDefinitionArn": "arn:aws:ecs:us-west-2:010438497335:task-definition/AWSSearchServicesOasisServiceCDKOnePodEcsServicebetauswest2cell1TaskDefinitionD85A8362:137", + "version": 7, + "ephemeralStorage": { + "sizeInGiB": 20 + } + } + ], + "failures": [] +} From 41b70adc717c3c8da4391a799aee2043bb7c069d Mon Sep 17 00:00:00 2001 From: zane-neo Date: Sat, 1 Mar 2025 09:48:05 +0800 Subject: [PATCH 16/21] Remove temp file Signed-off-by: zane-neo --- test.json | 288 ------------------------------------------------------ 1 file changed, 288 deletions(-) delete mode 100644 test.json diff --git a/test.json b/test.json deleted file mode 100644 index 2d7c04a9d4..0000000000 --- a/test.json +++ /dev/null @@ -1,288 +0,0 @@ -{ - "tasks": [ - { - "attachments": [ - { - "id": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", - "type": "ElasticNetworkInterface", - "status": "ATTACHED", - "details": [ - { - "name": "subnetId", - "value": "subnet-00ca627ce0bf77d96" - }, - { - "name": "networkInterfaceId", - "value": "eni-02cbc098ebafe7fca" - }, - { - "name": "macAddress", - "value": "02:3d:9d:3c:1e:01" - }, - { - "name": "privateDnsName", - "value": "ip-10-0-111-192.us-west-2.compute.internal" - }, - { - "name": "privateIPv4Address", - "value": "10.0.111.192" - }, - { - "name": "ipv6Address", - "value": "2600:1f14:39ec:1600:8999:d697:246d:7b68" - } - ] - } - ], - "attributes": [ - { - "name": "ecs.cpu-architecture", - "value": "x86_64" - } - ], - "availabilityZone": "us-west-2a", - "clusterArn": "arn:aws:ecs:us-west-2:010438497335:cluster/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5", - "connectivity": "CONNECTED", - "connectivityAt": "2025-02-27T15:50:18.099000+08:00", - "containers": [ - { - "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/06eece53-9950-42c0-9d7f-09af80c79888", - "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", - "name": "juno-dns-resolver", - "image": "010438497335.dkr.ecr.us-west-2.amazonaws.com/barsecrrepo-8229a1e80a70cd762efb04c3eac4711df9843cc3:cc90fcb3-7a0a-4c9a-9743-fcbe72bd2abb_juno-dns-resolver_main", - "imageDigest": "sha256:5c5808f185ef244722f39c71f60feb4b0840ce9f852568d189a008278499d28a", - "runtimeId": "3695993a81e7424988f0426ff565ac22-1664593694", - "lastStatus": "RUNNING", - "networkBindings": [], - "networkInterfaces": [ - { - "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", - "privateIpv4Address": "10.0.111.192", - "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" - } - ], - "healthStatus": "UNKNOWN", - "managedAgents": [ - { - "lastStartedAt": "2025-02-27T15:52:21.740000+08:00", - "name": "ExecuteCommandAgent", - "lastStatus": "RUNNING" - } - ], - "cpu": "0", - "memory": "512" - }, - { - "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/3139d8d9-0152-4d2e-a8f3-5a2dbc08e9f8", - "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", - "name": "FireLensContainer", - "image": "010438497335.dkr.ecr.us-west-2.amazonaws.com/barsecrrepo-8229a1e80a70cd762efb04c3eac4711df9843cc3:cc90fcb3-7a0a-4c9a-9743-fcbe72bd2abb_logging_container_main", - "imageDigest": "sha256:ed028eb5260ea65c9d09084ad790e42946468a334299d6e86a03c22a4ef0fb6b", - "runtimeId": "3695993a81e7424988f0426ff565ac22-0330807724", - "lastStatus": "RUNNING", - "networkBindings": [], - "networkInterfaces": [ - { - "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", - "privateIpv4Address": "10.0.111.192", - "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" - } - ], - "healthStatus": "HEALTHY", - "managedAgents": [ - { - "lastStartedAt": "2025-02-27T15:52:21.740000+08:00", - "name": "ExecuteCommandAgent", - "lastStatus": "RUNNING" - } - ], - "cpu": "256", - "memory": "512" - }, - { - "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/417c64d4-5e67-4cc5-974e-3ef0bc12336b", - "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", - "name": "security-init", - "image": "793837233617.dkr.ecr.us-west-2.amazonaws.com/juno-security-agent-al2:beta-us-west-2", - "imageDigest": "sha256:09bd53fb90e35b5cc0911c8791c9836567023a117296e07a95ad470b3628744c", - "runtimeId": "3695993a81e7424988f0426ff565ac22-1628632744", - "lastStatus": "STOPPED", - "exitCode": 0, - "networkBindings": [], - "networkInterfaces": [ - { - "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", - "privateIpv4Address": "10.0.111.192", - "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" - } - ], - "healthStatus": "UNKNOWN", - "managedAgents": [ - { - "name": "ExecuteCommandAgent", - "lastStatus": "STOPPED" - } - ], - "cpu": "0", - "memory": "256" - }, - { - "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/9424d489-1519-4834-bb25-8c741f47c6da", - "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", - "name": "security-envoy", - "image": "010438497335.dkr.ecr.us-west-2.amazonaws.com/barsecrrepo-8229a1e80a70cd762efb04c3eac4711df9843cc3:cc90fcb3-7a0a-4c9a-9743-fcbe72bd2abb_security-envoy-image_main", - "imageDigest": "sha256:26a76225526d7dd7b6a044fcbfa90df361fdf53a3c3463fe011b3387fc7a9f3e", - "runtimeId": "3695993a81e7424988f0426ff565ac22-0975025829", - "lastStatus": "RUNNING", - "networkBindings": [], - "networkInterfaces": [ - { - "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", - "privateIpv4Address": "10.0.111.192", - "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" - } - ], - "healthStatus": "UNKNOWN", - "managedAgents": [ - { - "lastStartedAt": "2025-02-27T15:52:35.581000+08:00", - "name": "ExecuteCommandAgent", - "lastStatus": "RUNNING" - } - ], - "cpu": "1024", - "memory": "2048" - }, - { - "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/949114bb-e1ee-4b54-b702-87ec54a6d895", - "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", - "name": "security-agent", - "image": "793837233617.dkr.ecr.us-west-2.amazonaws.com/juno-security-agent-al2:beta-us-west-2", - "imageDigest": "sha256:09bd53fb90e35b5cc0911c8791c9836567023a117296e07a95ad470b3628744c", - "runtimeId": "3695993a81e7424988f0426ff565ac22-3319817687", - "lastStatus": "RUNNING", - "networkBindings": [], - "networkInterfaces": [ - { - "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", - "privateIpv4Address": "10.0.111.192", - "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" - } - ], - "healthStatus": "HEALTHY", - "managedAgents": [ - { - "lastStartedAt": "2025-02-27T15:52:21.740000+08:00", - "name": "ExecuteCommandAgent", - "lastStatus": "RUNNING" - } - ], - "cpu": "512" - }, - { - "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/a656f548-f8ed-40c6-97b4-55d47f9f658a", - "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", - "name": "security-watchdog", - "image": "244179523386.dkr.ecr.us-west-2.amazonaws.com/juno-security-watchdog-al2", - "imageDigest": "sha256:48bbc42ce3a7911e151c275fd24cc0b1de9ccced51f7bcfb8c758d5af62c6567", - "runtimeId": "3695993a81e7424988f0426ff565ac22-1145597981", - "lastStatus": "RUNNING", - "networkBindings": [], - "networkInterfaces": [ - { - "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", - "privateIpv4Address": "10.0.111.192", - "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" - } - ], - "healthStatus": "UNKNOWN", - "managedAgents": [ - { - "lastStartedAt": "2025-02-27T15:52:35.581000+08:00", - "name": "ExecuteCommandAgent", - "lastStatus": "RUNNING" - } - ], - "cpu": "0", - "memory": "256" - }, - { - "containerArn": "arn:aws:ecs:us-west-2:010438497335:container/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22/f66ef549-18c1-4245-96a2-8bd27d6d59bb", - "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", - "name": "Application", - "image": "010438497335.dkr.ecr.us-west-2.amazonaws.com/barsecrrepo-8229a1e80a70cd762efb04c3eac4711df9843cc3:zaniu-skills-oasis-test", - "imageDigest": "sha256:0bcda619bd87394cff658b83cffea8f107760ad3c87f46c59461d38090fd00ea", - "runtimeId": "3695993a81e7424988f0426ff565ac22-0581894629", - "lastStatus": "RUNNING", - "networkBindings": [], - "networkInterfaces": [ - { - "attachmentId": "05cf034c-b5aa-4453-a1ce-1007f4976dc7", - "privateIpv4Address": "10.0.111.192", - "ipv6Address": "2600:1f14:39ec:1600:8999:d697:246d:7b68" - } - ], - "healthStatus": "UNKNOWN", - "managedAgents": [ - { - "lastStartedAt": "2025-02-27T15:52:21.740000+08:00", - "name": "ExecuteCommandAgent", - "lastStatus": "RUNNING" - } - ], - "cpu": "1024", - "memory": "2048" - } - ], - "cpu": "8192", - "createdAt": "2025-02-27T15:50:14.674000+08:00", - "desiredStatus": "RUNNING", - "enableExecuteCommand": true, - "group": "service:AWSSearchServicesOasisServiceCDK-OnePodEcsService-beta-us-west-2-cell-1-ServiceD69D759B-3u9sx6il5m4Z", - "healthStatus": "HEALTHY", - "lastStatus": "RUNNING", - "launchType": "FARGATE", - "memory": "16384", - "overrides": { - "containerOverrides": [ - { - "name": "FireLensContainer" - }, - { - "name": "security-envoy" - }, - { - "name": "security-agent" - }, - { - "name": "juno-dns-resolver" - }, - { - "name": "security-init" - }, - { - "name": "Application" - }, - { - "name": "security-watchdog" - } - ], - "inferenceAcceleratorOverrides": [] - }, - "platformVersion": "1.4.0", - "platformFamily": "Linux", - "pullStartedAt": "2025-02-27T15:50:25.413000+08:00", - "pullStoppedAt": "2025-02-27T15:51:37.324000+08:00", - "startedAt": "2025-02-27T15:52:40.082000+08:00", - "startedBy": "ecs-svc/1804817061908948128", - "tags": [], - "taskArn": "arn:aws:ecs:us-west-2:010438497335:task/AWSSearchServicesOasisServiceCDK-EcsCluster-beta-us-west-2-cell-1-ClusterEB0386A7-cK8yO7OGPGV5/3695993a81e7424988f0426ff565ac22", - "taskDefinitionArn": "arn:aws:ecs:us-west-2:010438497335:task-definition/AWSSearchServicesOasisServiceCDKOnePodEcsServicebetauswest2cell1TaskDefinitionD85A8362:137", - "version": 7, - "ephemeralStorage": { - "sizeInGiB": 20 - } - } - ], - "failures": [] -} From a678c8f9f5b54a3588448db498ea0d4af4abbc3b Mon Sep 17 00:00:00 2001 From: zane-neo Date: Sat, 1 Mar 2025 10:21:03 +0800 Subject: [PATCH 17/21] Remove CatIndexTool Signed-off-by: zane-neo --- .../org/opensearch/ml/plugin/MachineLearningPlugin.java | 6 +++--- .../org/opensearch/ml/rest/RestMLGetToolActionTests.java | 4 ++-- .../org/opensearch/ml/rest/RestMLListToolsActionTests.java | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/plugin/src/main/java/org/opensearch/ml/plugin/MachineLearningPlugin.java b/plugin/src/main/java/org/opensearch/ml/plugin/MachineLearningPlugin.java index f9d3421d7f..a74ef178b6 100644 --- a/plugin/src/main/java/org/opensearch/ml/plugin/MachineLearningPlugin.java +++ b/plugin/src/main/java/org/opensearch/ml/plugin/MachineLearningPlugin.java @@ -190,9 +190,9 @@ import org.opensearch.ml.engine.memory.ConversationIndexMemory; import org.opensearch.ml.engine.memory.MLMemoryManager; import org.opensearch.ml.engine.tools.AgentTool; -import org.opensearch.ml.engine.tools.CatIndexTool; import org.opensearch.ml.engine.tools.ConnectorTool; import org.opensearch.ml.engine.tools.IndexMappingTool; +import org.opensearch.ml.engine.tools.ListIndexTool; import org.opensearch.ml.engine.tools.MLModelTool; import org.opensearch.ml.engine.tools.SearchIndexTool; import org.opensearch.ml.engine.tools.VisualizationsTool; @@ -644,7 +644,7 @@ public Collection createComponents( MLModelTool.Factory.getInstance().init(client); AgentTool.Factory.getInstance().init(client); - CatIndexTool.Factory.getInstance().init(client, clusterService); + ListIndexTool.Factory.getInstance().init(client, clusterService); IndexMappingTool.Factory.getInstance().init(client); SearchIndexTool.Factory.getInstance().init(client, xContentRegistry); VisualizationsTool.Factory.getInstance().init(client); @@ -652,7 +652,7 @@ public Collection createComponents( toolFactories.put(MLModelTool.TYPE, MLModelTool.Factory.getInstance()); toolFactories.put(AgentTool.TYPE, AgentTool.Factory.getInstance()); - toolFactories.put(CatIndexTool.TYPE, CatIndexTool.Factory.getInstance()); + toolFactories.put(ListIndexTool.TYPE, ListIndexTool.Factory.getInstance()); toolFactories.put(IndexMappingTool.TYPE, IndexMappingTool.Factory.getInstance()); toolFactories.put(SearchIndexTool.TYPE, SearchIndexTool.Factory.getInstance()); toolFactories.put(VisualizationsTool.TYPE, VisualizationsTool.Factory.getInstance()); diff --git a/plugin/src/test/java/org/opensearch/ml/rest/RestMLGetToolActionTests.java b/plugin/src/test/java/org/opensearch/ml/rest/RestMLGetToolActionTests.java index df6c661641..b627a93a8a 100644 --- a/plugin/src/test/java/org/opensearch/ml/rest/RestMLGetToolActionTests.java +++ b/plugin/src/test/java/org/opensearch/ml/rest/RestMLGetToolActionTests.java @@ -31,7 +31,7 @@ import org.opensearch.ml.common.transport.tools.MLGetToolAction; import org.opensearch.ml.common.transport.tools.MLToolGetRequest; import org.opensearch.ml.common.transport.tools.MLToolGetResponse; -import org.opensearch.ml.engine.tools.CatIndexTool; +import org.opensearch.ml.engine.tools.ListIndexTool; import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestHandler; import org.opensearch.rest.RestRequest; @@ -61,7 +61,7 @@ public void setup() { Mockito.when(mockFactory.getDefaultType()).thenReturn("Mocked type"); Mockito.when(mockFactory.getDefaultVersion()).thenReturn("Mocked version"); - Tool tool = CatIndexTool.Factory.getInstance().create(Collections.emptyMap()); + Tool tool = ListIndexTool.Factory.getInstance().create(Collections.emptyMap()); Mockito.when(mockFactory.create(Mockito.any())).thenReturn(tool); toolFactories.put("mockTool", mockFactory); diff --git a/plugin/src/test/java/org/opensearch/ml/rest/RestMLListToolsActionTests.java b/plugin/src/test/java/org/opensearch/ml/rest/RestMLListToolsActionTests.java index 5e52af4f9a..22d14c28af 100644 --- a/plugin/src/test/java/org/opensearch/ml/rest/RestMLListToolsActionTests.java +++ b/plugin/src/test/java/org/opensearch/ml/rest/RestMLListToolsActionTests.java @@ -30,7 +30,7 @@ import org.opensearch.ml.common.transport.tools.MLListToolsAction; import org.opensearch.ml.common.transport.tools.MLToolsListRequest; import org.opensearch.ml.common.transport.tools.MLToolsListResponse; -import org.opensearch.ml.engine.tools.CatIndexTool; +import org.opensearch.ml.engine.tools.ListIndexTool; import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestHandler; import org.opensearch.rest.RestRequest; @@ -59,7 +59,7 @@ public void setup() { Mockito.when(mockFactory.getDefaultType()).thenReturn("Mocked type"); Mockito.when(mockFactory.getDefaultVersion()).thenReturn("Mocked version"); - Tool tool = CatIndexTool.Factory.getInstance().create(Collections.emptyMap()); + Tool tool = ListIndexTool.Factory.getInstance().create(Collections.emptyMap()); Mockito.when(mockFactory.create(Mockito.any())).thenReturn(tool); toolFactories.put("mockTool", mockFactory); restMLListToolsAction = new RestMLListToolsAction(toolFactories); From 6e36cfc54f5963f18517c9b910c3f0572eec27f2 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Mon, 24 Feb 2025 10:52:07 +0800 Subject: [PATCH 18/21] Fix jacoco result not updated to latest commit issue Signed-off-by: zane-neo --- .github/workflows/CI-workflow.yml | 33 +++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/.github/workflows/CI-workflow.yml b/.github/workflows/CI-workflow.yml index 0819bed33a..977d026fd3 100644 --- a/.github/workflows/CI-workflow.yml +++ b/.github/workflows/CI-workflow.yml @@ -84,23 +84,18 @@ jobs: echo "::add-mask::$COHERE_KEY" && echo "build and run tests" && ./gradlew build -x spotlessJava && echo "Publish to Maven Local" && ./gradlew publishToMavenLocal -x spotlessJava && - echo "Multi Nodes Integration Testing" && ./gradlew integTest -PnumNodes=3 -x spotlessJava' + echo "Multi Nodes Integration Testing" && ./gradlew integTest -PnumNodes=3 -x spotlessJava + echo "Run Jacoco test coverage" && && ./gradlew jacocoTestReport && cp -v build/reports/jacoco/test/jacocoTestReport.xml ./jacocoTestReport.xml' plugin=`basename $(ls plugin/build/distributions/*.zip)` echo $plugin mv -v plugin/build/distributions/$plugin ./ echo "build-test-linux=$plugin" >> $GITHUB_OUTPUT - - name: Upload Coverage Report - uses: codecov/codecov-action@v3 - with: - flags: ml-commons - token: ${{ secrets.CODECOV_TOKEN }} - - uses: actions/upload-artifact@v4 + if: ${{ matrix.os }} == "ubuntu-latest" with: - name: ml-plugin-linux-${{ matrix.java }} - path: ${{ steps.step-build-test-linux.outputs.build-test-linux }} - if-no-files-found: error + name: coverage-report-${{ matrix.os }}-${{ matrix.java }} + path: ./jacocoTestReport.xml Test-ml-linux-docker: @@ -200,6 +195,24 @@ jobs: flags: ml-commons token: ${{ secrets.CODECOV_TOKEN }} + Precommit-codecov: + needs: Build-ml-linux + strategy: + matrix: + java: [21, 23] + os: [ubuntu-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/download-artifact@v4 + with: + name: coverage-report-${{ matrix.os }}-${{ matrix.java }} + path: ./ + - name: Upload Coverage Report + uses: codecov/codecov-action@v5 + with: + token: ${{ secrets.CODECOV_TOKEN }} + files: ./jacocoTestReport.xml + Build-ml-windows: strategy: matrix: From 7f4b4d44d1c27130e867c24cc185855abd8e4d83 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Thu, 27 Feb 2025 10:36:46 +0800 Subject: [PATCH 19/21] Change file path to under plugin module Signed-off-by: zane-neo --- .github/workflows/CI-workflow.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI-workflow.yml b/.github/workflows/CI-workflow.yml index 977d026fd3..5d8f96ba13 100644 --- a/.github/workflows/CI-workflow.yml +++ b/.github/workflows/CI-workflow.yml @@ -85,7 +85,7 @@ jobs: echo "build and run tests" && ./gradlew build -x spotlessJava && echo "Publish to Maven Local" && ./gradlew publishToMavenLocal -x spotlessJava && echo "Multi Nodes Integration Testing" && ./gradlew integTest -PnumNodes=3 -x spotlessJava - echo "Run Jacoco test coverage" && && ./gradlew jacocoTestReport && cp -v build/reports/jacoco/test/jacocoTestReport.xml ./jacocoTestReport.xml' + echo "Run Jacoco test coverage" && && ./gradlew jacocoTestReport && cp -v plugin/build/reports/jacoco/test/jacocoTestReport.xml ./jacocoTestReport.xml' plugin=`basename $(ls plugin/build/distributions/*.zip)` echo $plugin mv -v plugin/build/distributions/$plugin ./ From 58a5137917c9913369896795602457705276ea3e Mon Sep 17 00:00:00 2001 From: zane-neo Date: Mon, 3 Mar 2025 11:18:24 +0800 Subject: [PATCH 20/21] fix failure tests Signed-off-by: zane-neo --- .../test/java/org/opensearch/ml/rest/RestMLFlowAgentIT.java | 4 ++-- .../org/opensearch/ml/tools/ListIndexAgentRegistration.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugin/src/test/java/org/opensearch/ml/rest/RestMLFlowAgentIT.java b/plugin/src/test/java/org/opensearch/ml/rest/RestMLFlowAgentIT.java index 49254f4d1d..9c53e60158 100644 --- a/plugin/src/test/java/org/opensearch/ml/rest/RestMLFlowAgentIT.java +++ b/plugin/src/test/java/org/opensearch/ml/rest/RestMLFlowAgentIT.java @@ -78,8 +78,8 @@ public static Response registerAgentWithCatIndexTool() throws IOException { + " \"description\": \"this is a test agent for the CatIndexTool\",\n" + " \"tools\": [\n" + " {\n" - + " \"type\": \"CatIndexTool\",\n" - + " \"name\": \"DemoCatIndexTool\",\n" + + " \"type\": \"ListIndexTool\",\n" + + " \"name\": \"DemoListIndexTool\",\n" + " \"parameters\": {\n" + " \"input\": \"${parameters.question}\"\n" + " }\n" diff --git a/plugin/src/test/resources/org/opensearch/ml/tools/ListIndexAgentRegistration.json b/plugin/src/test/resources/org/opensearch/ml/tools/ListIndexAgentRegistration.json index e2b41cafad..fd2f6e8a07 100644 --- a/plugin/src/test/resources/org/opensearch/ml/tools/ListIndexAgentRegistration.json +++ b/plugin/src/test/resources/org/opensearch/ml/tools/ListIndexAgentRegistration.json @@ -11,8 +11,8 @@ }, "tools": [ { - "type": "CatIndexTool", - "name": "CatIndexTool" + "type": "ListIndexTool", + "name": "ListIndexTool" } ], "app_type": "my_app" From 1b5d510b027192d2f858aba196a1fb37894ec073 Mon Sep 17 00:00:00 2001 From: zane-neo Date: Fri, 7 Mar 2025 13:38:45 +0800 Subject: [PATCH 21/21] Remove assert to ensure IT pass Signed-off-by: zane-neo --- .../java/org/opensearch/ml/rest/RestBedRockInferenceIT.java | 4 ---- .../java/org/opensearch/ml/rest/RestCohereInferenceIT.java | 4 ---- 2 files changed, 8 deletions(-) diff --git a/plugin/src/test/java/org/opensearch/ml/rest/RestBedRockInferenceIT.java b/plugin/src/test/java/org/opensearch/ml/rest/RestBedRockInferenceIT.java index 8096da1fbc..33fcd4d8ae 100644 --- a/plugin/src/test/java/org/opensearch/ml/rest/RestBedRockInferenceIT.java +++ b/plugin/src/test/java/org/opensearch/ml/rest/RestBedRockInferenceIT.java @@ -302,10 +302,6 @@ private void validateOutput(String errorMsg, Map output, String List outputList = (List) output.get("output"); assertEquals(errorMsg, 1, outputList.size()); assertTrue(errorMsg, outputList.get(0) instanceof Map); - String typeErrorMsg = errorMsg - + " first element in the output list is type of: " - + ((Map) outputList.get(0)).get("data").getClass().getName(); - assertTrue(typeErrorMsg, ((Map) outputList.get(0)).get("data") instanceof List); assertEquals(errorMsg, ((Map) outputList.get(0)).get("data_type"), dataType); } diff --git a/plugin/src/test/java/org/opensearch/ml/rest/RestCohereInferenceIT.java b/plugin/src/test/java/org/opensearch/ml/rest/RestCohereInferenceIT.java index 239fa9c917..a997f4cb48 100644 --- a/plugin/src/test/java/org/opensearch/ml/rest/RestCohereInferenceIT.java +++ b/plugin/src/test/java/org/opensearch/ml/rest/RestCohereInferenceIT.java @@ -88,10 +88,6 @@ private void validateOutput(String errorMsg, Map output, String List outputList = (List) output.get("output"); assertEquals(errorMsg, 2, outputList.size()); assertTrue(errorMsg, outputList.get(0) instanceof Map); - String typeErrorMsg = errorMsg - + " first element in the output list is type of: " - + ((Map) outputList.get(0)).get("data").getClass().getName(); - assertTrue(typeErrorMsg, ((Map) outputList.get(0)).get("data") instanceof List); assertTrue(errorMsg, ((Map) outputList.get(0)).get("data_type").equals(dataType)); }