From d09c12ce2c6b8dc603de3aee511db450d7b00ddc Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Sun, 21 Jan 2024 02:06:30 +0000 Subject: [PATCH 01/11] Wiping system indices, moving ml config check to individual tests that create a connector Signed-off-by: Joshua Palis --- .github/workflows/CI.yml | 2 +- .../FlowFrameworkRestTestCase.java | 132 +++++++++++------- .../rest/FlowFrameworkRestApiIT.java | 20 ++- .../rest/FlowFrameworkSecureRestApiIT.java | 8 +- 4 files changed, 103 insertions(+), 59 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 14c2ca25d..8bbc30658 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -81,6 +81,6 @@ jobs: run: | ./gradlew integTest yamlRestTest - name: Multi Nodes Integration Testing - if: matrix.java == 21 + if: matrix.java == '21.0.1' run: | ./gradlew integTest -PnumNodes=3 diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index 1d450cd26..b8b67cbd4 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -45,6 +45,7 @@ import org.opensearch.flowframework.model.Template; import org.opensearch.flowframework.model.WorkflowState; import org.opensearch.test.rest.OpenSearchRestTestCase; +import org.junit.After; import org.junit.Before; import java.io.IOException; @@ -75,56 +76,49 @@ public abstract class FlowFrameworkRestTestCase extends OpenSearchRestTestCase { @Before protected void setUpSettings() throws Exception { - if (!indexExistsWithAdminClient(".plugins-ml-config")) { - // Initial cluster set up - - // Enable Flow Framework Plugin Rest APIs - Response response = TestHelpers.makeRequest( - client(), - "PUT", - "_cluster/settings", - null, - "{\"transient\":{\"plugins.flow_framework.enabled\":true}}", - List.of(new BasicHeader(HttpHeaders.USER_AGENT, "")) - ); - assertEquals(200, response.getStatusLine().getStatusCode()); - - // Enable ML Commons to run on non-ml nodes - response = TestHelpers.makeRequest( - client(), - "PUT", - "_cluster/settings", - null, - "{\"persistent\":{\"plugins.ml_commons.only_run_on_ml_node\":false}}", - List.of(new BasicHeader(HttpHeaders.USER_AGENT, "")) - ); - assertEquals(200, response.getStatusLine().getStatusCode()); - - // Enable local model registration via URL - response = TestHelpers.makeRequest( - client(), - "PUT", - "_cluster/settings", - null, - "{\"persistent\":{\"plugins.ml_commons.allow_registering_model_via_url\":true}}", - List.of(new BasicHeader(HttpHeaders.USER_AGENT, "")) - ); - assertEquals(200, response.getStatusLine().getStatusCode()); - - // Set ML jvm heap memory threshold to 100 to avoid opening the circuit breaker during tests - response = TestHelpers.makeRequest( - client(), - "PUT", - "_cluster/settings", - null, - "{\"persistent\":{\"plugins.ml_commons.jvm_heap_memory_threshold\":100}}", - List.of(new BasicHeader(HttpHeaders.USER_AGENT, "")) - ); - assertEquals(200, response.getStatusLine().getStatusCode()); + // Enable Flow Framework Plugin Rest APIs + Response response = TestHelpers.makeRequest( + client(), + "PUT", + "_cluster/settings", + null, + "{\"transient\":{\"plugins.flow_framework.enabled\":true}}", + List.of(new BasicHeader(HttpHeaders.USER_AGENT, "")) + ); + assertEquals(200, response.getStatusLine().getStatusCode()); - // Ensure .plugins-ml-config is created before proceeding with integration tests - assertBusy(() -> { assertTrue(indexExistsWithAdminClient(".plugins-ml-config")); }, 60, TimeUnit.SECONDS); - } + // Enable ML Commons to run on non-ml nodes + response = TestHelpers.makeRequest( + client(), + "PUT", + "_cluster/settings", + null, + "{\"persistent\":{\"plugins.ml_commons.only_run_on_ml_node\":false}}", + List.of(new BasicHeader(HttpHeaders.USER_AGENT, "")) + ); + assertEquals(200, response.getStatusLine().getStatusCode()); + + // Enable local model registration via URL + response = TestHelpers.makeRequest( + client(), + "PUT", + "_cluster/settings", + null, + "{\"persistent\":{\"plugins.ml_commons.allow_registering_model_via_url\":true}}", + List.of(new BasicHeader(HttpHeaders.USER_AGENT, "")) + ); + assertEquals(200, response.getStatusLine().getStatusCode()); + + // Set ML jvm heap memory threshold to 100 to avoid opening the circuit breaker during tests + response = TestHelpers.makeRequest( + client(), + "PUT", + "_cluster/settings", + null, + "{\"persistent\":{\"plugins.ml_commons.jvm_heap_memory_threshold\":100}}", + List.of(new BasicHeader(HttpHeaders.USER_AGENT, "")) + ); + assertEquals(200, response.getStatusLine().getStatusCode()); // Set up clients if running in security enabled cluster if (isHttps()) { @@ -132,7 +126,7 @@ protected void setUpSettings() throws Exception { String readAccessUserPassword = generatePassword(READ_ACCESS_USER); // Configure full access user and client, needs ML Full Access role as well - Response response = createUser( + response = createUser( FULL_ACCESS_USER, fullAccessUserPassword, List.of(FLOW_FRAMEWORK_FULL_ACCESS_ROLE, ML_COMMONS_FULL_ACCESS_ROLE) @@ -187,7 +181,7 @@ protected static void deleteIndexWithAdminClient(String name) throws IOException // Utility fn for checking if an index exists. Should only be used when not allowed in a regular context // (e.g., checking existence of system indices) - protected static boolean indexExistsWithAdminClient(String indexName) throws IOException { + public static boolean indexExistsWithAdminClient(String indexName) throws IOException { Request request = new Request("HEAD", "/" + indexName); Response response = adminClient().performRequest(request); return RestStatus.OK.getStatus() == response.getStatusLine().getStatusCode(); @@ -253,6 +247,37 @@ protected static void configureHttpsClient(RestClientBuilder builder, Settings s } } + @SuppressWarnings("unchecked") + @After + protected void wipeAllODFEIndices() throws IOException { + Response response = adminClient().performRequest(new Request("GET", "/_cat/indices?format=json&expand_wildcards=all")); + MediaType xContentType = MediaType.fromMediaType(response.getEntity().getContentType()); + try ( + XContentParser parser = xContentType.xContent() + .createParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.THROW_UNSUPPORTED_OPERATION, + response.getEntity().getContent() + ) + ) { + XContentParser.Token token = parser.nextToken(); + List> parserList = null; + if (token == XContentParser.Token.START_ARRAY) { + parserList = parser.listOrderedMap().stream().map(obj -> (Map) obj).collect(Collectors.toList()); + } else { + parserList = Collections.singletonList(parser.mapOrdered()); + } + + for (Map index : parserList) { + String indexName = (String) index.get("index"); + // Do not reset ML encryption index as this is needed to encrypt connector credentials + if (indexName != null && !".opendistro_security".equals(indexName) && !".plugins-ml-config".equals(indexName)) { + adminClient().performRequest(new Request("DELETE", "/" + indexName)); + } + } + } + } + /** * wipeAllIndices won't work since it cannot delete security index. Use wipeAllSystemIndices instead. */ @@ -290,12 +315,13 @@ protected Response createWorkflow(RestClient client, Template template) throws E /** * Helper method to invoke the Create Workflow Rest Action with provision + * @param client the rest client * @param template the template to create * @throws Exception if the request fails * @return a rest response */ - protected Response createWorkflowWithProvision(Template template) throws Exception { - return TestHelpers.makeRequest(client(), "POST", WORKFLOW_URI + "?provision=true", Collections.emptyMap(), template.toJson(), null); + protected Response createWorkflowWithProvision(RestClient client, Template template) throws Exception { + return TestHelpers.makeRequest(client, "POST", WORKFLOW_URI + "?provision=true", Collections.emptyMap(), template.toJson(), null); } /** diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java index c097fce6b..dc24fc4cb 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java @@ -74,7 +74,7 @@ public void testFailedUpdateWorkflow() throws Exception { assertEquals(RestStatus.CREATED, TestHelpers.restStatus(responseCreate)); Template template = TestHelpers.createTemplateFromFile("createconnector-registerremotemodel-deploymodel.json"); - Thread.sleep(1000); + ResponseException exception = expectThrows(ResponseException.class, () -> updateWorkflow(client(), "123", template)); assertTrue(exception.getMessage().contains("Failed to get template: 123")); @@ -215,8 +215,14 @@ public void testCreateAndProvisionRemoteModelWorkflow() throws Exception { String workflowId = (String) responseMap.get(WORKFLOW_ID); getAndAssertWorkflowStatus(client(), workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED); - // Hit Provision API and assert status - response = provisionWorkflow(client(), workflowId); + // Ensure Ml config index is initialized as creating a connector requires this, then hit Provision API and assert status + if(!indexExistsWithAdminClient(".plugins-ml-config")) { + assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 25, TimeUnit.SECONDS); + response = provisionWorkflow(client(), workflowId); + } else { + response = provisionWorkflow(client(), workflowId); + } + assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); getAndAssertWorkflowStatus(client(), workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS); @@ -243,7 +249,13 @@ public void testCreateAndProvisionAgentFrameworkWorkflow() throws Exception { Template template = TestHelpers.createTemplateFromFile("agent-framework.json"); // Hit Create Workflow API to create agent-framework template, with template validation check and provision parameter - Response response = createWorkflowWithProvision(template); + Response response; + if(!indexExistsWithAdminClient(".plugins-ml-config")) { + assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 25, TimeUnit.SECONDS); + response = createWorkflowWithProvision(client(), template); + } else { + response = createWorkflowWithProvision(client(), template); + } assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response)); Map responseMap = entityAsMap(response); String workflowId = (String) responseMap.get(WORKFLOW_ID); diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java index e83e7f08e..c9d6641c6 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.util.Map; +import java.util.concurrent.TimeUnit; import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_ID; @@ -116,7 +117,12 @@ public void testCreateProvisionDeprovisionWorkflowWithFullAccess() throws Except assertEquals(RestStatus.OK, searchResponse.status()); // Invoke provision API - response = provisionWorkflow(fullAccessClient(), workflowId); + if(!indexExistsWithAdminClient(".plugins-ml-config")) { + assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 25, TimeUnit.SECONDS); + response = provisionWorkflow(fullAccessClient(), workflowId); + } else { + response = provisionWorkflow(fullAccessClient(), workflowId); + } assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); // Invoke status API From 9405feb2fcbf7123f649b752df3d24e61b7e7f34 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Sun, 21 Jan 2024 02:09:04 +0000 Subject: [PATCH 02/11] spotless Signed-off-by: Joshua Palis --- .../flowframework/rest/FlowFrameworkRestApiIT.java | 6 +++--- .../flowframework/rest/FlowFrameworkSecureRestApiIT.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java index dc24fc4cb..7b9a9f0ba 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java @@ -216,13 +216,13 @@ public void testCreateAndProvisionRemoteModelWorkflow() throws Exception { getAndAssertWorkflowStatus(client(), workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED); // Ensure Ml config index is initialized as creating a connector requires this, then hit Provision API and assert status - if(!indexExistsWithAdminClient(".plugins-ml-config")) { + if (!indexExistsWithAdminClient(".plugins-ml-config")) { assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 25, TimeUnit.SECONDS); response = provisionWorkflow(client(), workflowId); } else { response = provisionWorkflow(client(), workflowId); } - + assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); getAndAssertWorkflowStatus(client(), workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS); @@ -250,7 +250,7 @@ public void testCreateAndProvisionAgentFrameworkWorkflow() throws Exception { // Hit Create Workflow API to create agent-framework template, with template validation check and provision parameter Response response; - if(!indexExistsWithAdminClient(".plugins-ml-config")) { + if (!indexExistsWithAdminClient(".plugins-ml-config")) { assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 25, TimeUnit.SECONDS); response = createWorkflowWithProvision(client(), template); } else { diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java index c9d6641c6..9a8877225 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java @@ -117,7 +117,7 @@ public void testCreateProvisionDeprovisionWorkflowWithFullAccess() throws Except assertEquals(RestStatus.OK, searchResponse.status()); // Invoke provision API - if(!indexExistsWithAdminClient(".plugins-ml-config")) { + if (!indexExistsWithAdminClient(".plugins-ml-config")) { assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 25, TimeUnit.SECONDS); response = provisionWorkflow(fullAccessClient(), workflowId); } else { From cd51dd29abf43d7c981362a4658e83a342b7d0f1 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Sun, 21 Jan 2024 02:22:25 +0000 Subject: [PATCH 03/11] Increasing ML config index timeout for multi-node case Signed-off-by: Joshua Palis --- build.gradle | 8 ++++++++ .../flowframework/rest/FlowFrameworkRestApiIT.java | 4 ++-- .../flowframework/rest/FlowFrameworkSecureRestApiIT.java | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index a7eb23f7c..1ed9bc8b5 100644 --- a/build.gradle +++ b/build.gradle @@ -424,6 +424,14 @@ task integTestRemote(type: RestIntegTestTask) { // Automatically sets up the integration test cluster locally run { + doFirst { + // There seems to be an issue when running multi node run or integ tasks with unicast_hosts + // not being written, the waitForAllConditions ensures it's written + getClusters().forEach { cluster -> + cluster.waitForAllConditions() + } + } + useCluster testClusters.integTest } diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java index 7b9a9f0ba..815ff1298 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java @@ -217,7 +217,7 @@ public void testCreateAndProvisionRemoteModelWorkflow() throws Exception { // Ensure Ml config index is initialized as creating a connector requires this, then hit Provision API and assert status if (!indexExistsWithAdminClient(".plugins-ml-config")) { - assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 25, TimeUnit.SECONDS); + assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 40, TimeUnit.SECONDS); response = provisionWorkflow(client(), workflowId); } else { response = provisionWorkflow(client(), workflowId); @@ -251,7 +251,7 @@ public void testCreateAndProvisionAgentFrameworkWorkflow() throws Exception { // Hit Create Workflow API to create agent-framework template, with template validation check and provision parameter Response response; if (!indexExistsWithAdminClient(".plugins-ml-config")) { - assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 25, TimeUnit.SECONDS); + assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 40, TimeUnit.SECONDS); response = createWorkflowWithProvision(client(), template); } else { response = createWorkflowWithProvision(client(), template); diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java index 9a8877225..5985e928c 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java @@ -118,7 +118,7 @@ public void testCreateProvisionDeprovisionWorkflowWithFullAccess() throws Except // Invoke provision API if (!indexExistsWithAdminClient(".plugins-ml-config")) { - assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 25, TimeUnit.SECONDS); + assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 40, TimeUnit.SECONDS); response = provisionWorkflow(fullAccessClient(), workflowId); } else { response = provisionWorkflow(fullAccessClient(), workflowId); From 7afdee85d734d619a8d256bd35819436eb35b8d3 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Sun, 21 Jan 2024 02:28:38 +0000 Subject: [PATCH 04/11] Including flow framework config in wipeAllODFEIndices method Signed-off-by: Joshua Palis --- .../flowframework/FlowFrameworkRestTestCase.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index b8b67cbd4..f3b275117 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -270,8 +270,11 @@ protected void wipeAllODFEIndices() throws IOException { for (Map index : parserList) { String indexName = (String) index.get("index"); - // Do not reset ML encryption index as this is needed to encrypt connector credentials - if (indexName != null && !".opendistro_security".equals(indexName) && !".plugins-ml-config".equals(indexName)) { + // Do not reset ML/Flow Framework encryption index as this is needed to encrypt connector credentials + if (indexName != null + && !".opendistro_security".equals(indexName) + && !".plugins-ml-config".equals(indexName) + && !".plugins-flow-framework-config".equals(indexName)) { adminClient().performRequest(new Request("DELETE", "/" + indexName)); } } From b1a78f6f4728e5237f3ad521d8a8f89eb0a868f6 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Sun, 21 Jan 2024 02:44:36 +0000 Subject: [PATCH 05/11] exluding model group index from wipe Signed-off-by: Joshua Palis --- .../org/opensearch/flowframework/FlowFrameworkRestTestCase.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index f3b275117..52be4f778 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -273,6 +273,7 @@ protected void wipeAllODFEIndices() throws IOException { // Do not reset ML/Flow Framework encryption index as this is needed to encrypt connector credentials if (indexName != null && !".opendistro_security".equals(indexName) + && !".plugins-ml-model-group".equals(indexName) && !".plugins-ml-config".equals(indexName) && !".plugins-flow-framework-config".equals(indexName)) { adminClient().performRequest(new Request("DELETE", "/" + indexName)); From ef6f47be4004bc5c229158b6a2568b636fb5e923 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Sun, 21 Jan 2024 02:59:33 +0000 Subject: [PATCH 06/11] testing wipe all indices with client Signed-off-by: Joshua Palis --- .../org/opensearch/flowframework/FlowFrameworkRestTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index 52be4f778..d0d154dc7 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -276,7 +276,7 @@ protected void wipeAllODFEIndices() throws IOException { && !".plugins-ml-model-group".equals(indexName) && !".plugins-ml-config".equals(indexName) && !".plugins-flow-framework-config".equals(indexName)) { - adminClient().performRequest(new Request("DELETE", "/" + indexName)); + client().performRequest(new Request("DELETE", "/" + indexName)); } } } From 814ad8a4d5b53c409199c744c93487466dedb865 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Sun, 21 Jan 2024 03:05:55 +0000 Subject: [PATCH 07/11] Reverting wipe method back to adminClient Signed-off-by: Joshua Palis --- .../org/opensearch/flowframework/FlowFrameworkRestTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index d0d154dc7..52be4f778 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -276,7 +276,7 @@ protected void wipeAllODFEIndices() throws IOException { && !".plugins-ml-model-group".equals(indexName) && !".plugins-ml-config".equals(indexName) && !".plugins-flow-framework-config".equals(indexName)) { - client().performRequest(new Request("DELETE", "/" + indexName)); + adminClient().performRequest(new Request("DELETE", "/" + indexName)); } } } From 21cb21fdc76bcb006065de5fa1eb546be51255dc Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Sun, 21 Jan 2024 03:18:10 +0000 Subject: [PATCH 08/11] removing deprovision from model tests, not necessary since we're wiping indices after each test run Signed-off-by: Joshua Palis --- .../flowframework/rest/FlowFrameworkRestApiIT.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java index 815ff1298..b423f7560 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java @@ -163,12 +163,6 @@ public void testCreateAndProvisionLocalModelWorkflow() throws Exception { assertNotNull(resourcesCreated.get(0).resourceId()); assertEquals("deploy_model", resourcesCreated.get(1).workflowStepName()); assertNotNull(resourcesCreated.get(1).resourceId()); - - // Deprovision the workflow to avoid opening circut breaker when running additional tests - Response deprovisionResponse = deprovisionWorkflow(client(), workflowId); - - // wait for deprovision to complete - Thread.sleep(5000); } public void testCreateAndProvisionCyclicalTemplate() throws Exception { @@ -237,12 +231,6 @@ public void testCreateAndProvisionRemoteModelWorkflow() throws Exception { assertNotNull(resourcesCreated.get(1).resourceId()); assertEquals("deploy_model", resourcesCreated.get(2).workflowStepName()); assertNotNull(resourcesCreated.get(2).resourceId()); - - // Deprovision the workflow to avoid opening circuit breaker when running additional tests - Response deprovisionResponse = deprovisionWorkflow(client(), workflowId); - - // wait for deprovision to complete - Thread.sleep(5000); } public void testCreateAndProvisionAgentFrameworkWorkflow() throws Exception { From 0688286ff4f82a8e65f702afbd158a2e13cf297d Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Sun, 21 Jan 2024 03:30:33 +0000 Subject: [PATCH 09/11] Wrapping testFailedUpdateWorkflow provision in ml config check Signed-off-by: Joshua Palis --- .../flowframework/rest/FlowFrameworkRestApiIT.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java index b423f7560..d580794f3 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java @@ -84,8 +84,14 @@ public void testFailedUpdateWorkflow() throws Exception { Map responseMap = entityAsMap(response); String workflowId = (String) responseMap.get(WORKFLOW_ID); - // Provision Template - Response provisionResponse = provisionWorkflow(client(), workflowId); + // Ensure Ml config index is initialized as creating a connector requires this, then hit Provision API and assert status + Response provisionResponse; + if (!indexExistsWithAdminClient(".plugins-ml-config")) { + assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 40, TimeUnit.SECONDS); + provisionResponse = provisionWorkflow(client(), workflowId); + } else { + provisionResponse = provisionWorkflow(client(), workflowId); + } assertEquals(RestStatus.OK, TestHelpers.restStatus(provisionResponse)); getAndAssertWorkflowStatus(client(), workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS); From 4080129db14ab4cf52acf917b879fc3a3d407ba5 Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Sun, 21 Jan 2024 03:46:48 +0000 Subject: [PATCH 10/11] Fixing failing security tests Signed-off-by: Joshua Palis --- build.gradle | 12 ------------ .../flowframework/FlowFrameworkRestTestCase.java | 1 - 2 files changed, 13 deletions(-) diff --git a/build.gradle b/build.gradle index 1ed9bc8b5..6f0817bc8 100644 --- a/build.gradle +++ b/build.gradle @@ -238,18 +238,6 @@ ext{ cluster.setting("plugins.security.authcz.admin_dn", "\n- CN=kirk,OU=client,O=client,L=test, C=de") cluster.setting('plugins.security.restapi.roles_enabled', '["all_access", "security_rest_api_access"]') cluster.setting('plugins.security.system_indices.enabled', "true") - cluster.setting('plugins.security.system_indices.indices', '[' + - '".plugins-ml-config", ' + - '".plugins-ml-connector", ' + - '".plugins-ml-model-group", ' + - '".plugins-ml-model", ".plugins-ml-task", ' + - '".plugins-ml-conversation-meta", ' + - '".plugins-ml-conversation-interactions", ' + - '".plugins-flow-framework-config", ' + - '".plugins-flow-framework-templates", ' + - '".plugins-flow-framework-state"' + - ']' - ) cluster.setSecure(true) } } diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index 52be4f778..f3b275117 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -273,7 +273,6 @@ protected void wipeAllODFEIndices() throws IOException { // Do not reset ML/Flow Framework encryption index as this is needed to encrypt connector credentials if (indexName != null && !".opendistro_security".equals(indexName) - && !".plugins-ml-model-group".equals(indexName) && !".plugins-ml-config".equals(indexName) && !".plugins-flow-framework-config".equals(indexName)) { adminClient().performRequest(new Request("DELETE", "/" + indexName)); From fd9c64b265ee7aef0413b5a6a886f700c01c015b Mon Sep 17 00:00:00 2001 From: Joshua Palis Date: Sun, 21 Jan 2024 03:58:29 +0000 Subject: [PATCH 11/11] Testing model redeploy settings Signed-off-by: Joshua Palis --- .../FlowFrameworkRestTestCase.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index f3b275117..666a1541d 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -120,6 +120,28 @@ protected void setUpSettings() throws Exception { ); assertEquals(200, response.getStatusLine().getStatusCode()); + // Set ML auto redeploy to false + response = TestHelpers.makeRequest( + client(), + "PUT", + "_cluster/settings", + null, + "{\"persistent\":{\"plugins.ml_commons.model_auto_redeploy.enable\":false}}", + List.of(new BasicHeader(HttpHeaders.USER_AGENT, "")) + ); + assertEquals(200, response.getStatusLine().getStatusCode()); + + // Set ML auto redeploy retries to 0 + response = TestHelpers.makeRequest( + client(), + "PUT", + "_cluster/settings", + null, + "{\"persistent\":{\"plugins.ml_commons.model_auto_redeploy.lifetime_retry_times\":0}}", + List.of(new BasicHeader(HttpHeaders.USER_AGENT, "")) + ); + assertEquals(200, response.getStatusLine().getStatusCode()); + // Set up clients if running in security enabled cluster if (isHttps()) { String fullAccessUserPassword = generatePassword(FULL_ACCESS_USER);