Debug

Signed-off-by: Daniel Widdis <[email protected]>
dbwiddis · Jan 28, 2024 · 73c2fcf · 73c2fcf
1 parent 693b71d
commit 73c2fcf
Show file tree

Hide file tree

Showing 10 changed files with 43 additions and 93 deletions.
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
@@ -9,83 +9,12 @@ on:
     types: [opened, synchronize, reopened]
 
 jobs:
-  spotless:
-    if: github.repository == 'opensearch-project/flow-framework'
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-      # Spotless requires JDK 17+
-      - uses: actions/setup-java@v4
-        with:
-          java-version: 17
-          distribution: temurin
-      - name: Spotless Check
-        run: ./gradlew spotlessCheck
-  javadoc:
-    if: github.repository == 'opensearch-project/flow-framework'
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-      - name: Javadoc CheckStyle
-        run: ./gradlew checkstyleMain
-      - name: Javadoc Check
-        run: ./gradlew javadoc
-  build:
-    needs: [spotless, javadoc]
-    strategy:
-      matrix:
-        os: [ubuntu-latest, macos-latest, windows-latest]
-        java: [11, 21]
-        include:
-          - os: ubuntu-latest
-            java: 17
-            codecov: yes
-    name: Test JDK${{ matrix.java }}, ${{ matrix.os }}
-    runs-on: ${{ matrix.os }}
-    steps:
-      - uses: actions/checkout@v4
-      - name: Set up JDK ${{ matrix.java }}
-        uses: actions/setup-java@v4
-        with:
-          java-version: ${{ matrix.java }}
-          distribution: temurin
-      - name: Build and Run Tests
-        run: |
-          ./gradlew check -x integTest -x yamlRestTest -x spotlessJava
-      - name: Upload Coverage Report
-        if: ${{ matrix.codecov }}
-        uses: codecov/codecov-action@v3
-        with:
-          file: ./build/reports/jacoco/test/jacocoTestReport.xml
-  integTest:
-    needs: [spotless, javadoc]
-    strategy:
-      fail-fast: false
-      matrix:
-        os: [ubuntu-latest, macos-latest, windows-latest]
-        java: [11, 21]
-        include:
-          - os: ubuntu-latest
-            java: 17
-    name: Integ Test JDK${{ matrix.java }}, ${{ matrix.os }}
-    runs-on: ${{ matrix.os }}
-    steps:
-      - uses: actions/checkout@v4
-      - name: Set up JDK ${{ matrix.java }}
-        uses: actions/setup-java@v4
-        with:
-          java-version: ${{ matrix.java }}
-          distribution: temurin
-      - name: Build and Run Tests
-        run: |
-          ./gradlew integTest yamlRestTest
   integMultiNodeTest:
-    needs: [spotless, javadoc]
     strategy:
       fail-fast: false
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
-        java: [21]
+        java: [11, 17, 18, 19, 20, 21]
     name: Multi-Node Integ Test JDK${{ matrix.java }}, ${{ matrix.os }}
     runs-on: ${{ matrix.os }}
     steps:

diff --git a/build.gradle b/build.gradle
@@ -255,6 +255,8 @@ integTest {
         // not being written, the waitForAllConditions ensures it's written
         getClusters().forEach { cluster ->
             cluster.waitForAllConditions()
+            // we can't change the timeout and it's flaky if we don't wait long enough, so call it again
+            cluster.waitForAllConditions()
         }
     }
 
@@ -415,7 +417,7 @@ allprojects {
             if (System.getenv().containsKey("CI")) {
                 maxRetries = 1
                 maxFailures = 3
-                failOnPassedAfterRetry = false
+                failOnPassedAfterRetry = true
             }
         }
     }

diff --git a/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java b/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java
@@ -185,7 +185,8 @@ public List<ExecutorBuilder<?>> getExecutorBuilders(Settings settings) {
             new ScalingExecutorBuilder(
                 WORKFLOW_THREAD_POOL,
                 1,
-                OpenSearchExecutors.allocatedProcessors(settings),
+                // use no more than half the processors
+                Math.max(1, OpenSearchExecutors.allocatedProcessors(settings) / 2),
                 TimeValue.timeValueMinutes(5),
                 FLOW_FRAMEWORK_THREAD_POOL_PREFIX + WORKFLOW_THREAD_POOL
             )

diff --git a/src/main/java/org/opensearch/flowframework/rest/RestDeleteWorkflowAction.java b/src/main/java/org/opensearch/flowframework/rest/RestDeleteWorkflowAction.java
@@ -71,8 +71,10 @@ protected BaseRestHandler.RestChannelConsumer prepareRequest(RestRequest request
             }
             // Validate content
             if (request.hasContent()) {
-                // BaseRestHandler will give appropriate error message
-                return channel -> channel.sendResponse(null);
+                throw new FlowFrameworkException(
+                    "request [" + request.method() + " " + request.path() + "] does not support having a body",
+                    RestStatus.BAD_REQUEST
+                );
             }
             // Validate params
             if (workflowId == null) {

diff --git a/src/main/java/org/opensearch/flowframework/rest/RestDeprovisionWorkflowAction.java b/src/main/java/org/opensearch/flowframework/rest/RestDeprovisionWorkflowAction.java
@@ -66,8 +66,10 @@ protected BaseRestHandler.RestChannelConsumer prepareRequest(RestRequest request
             }
             // Validate content
             if (request.hasContent()) {
-                // BaseRestHandler will give appropriate error message
-                return channel -> channel.sendResponse(null);
+                throw new FlowFrameworkException(
+                    "request [" + request.method() + " " + request.path() + "] does not support having a body",
+                    RestStatus.BAD_REQUEST
+                );
             }
             // Validate params
             if (workflowId == null) {

diff --git a/src/main/java/org/opensearch/flowframework/rest/RestGetWorkflowAction.java b/src/main/java/org/opensearch/flowframework/rest/RestGetWorkflowAction.java
@@ -71,8 +71,10 @@ protected BaseRestHandler.RestChannelConsumer prepareRequest(RestRequest request
             }
             // Validate content
             if (request.hasContent()) {
-                // BaseRestHandler will give appropriate error message
-                return channel -> channel.sendResponse(null);
+                throw new FlowFrameworkException(
+                    "request [" + request.method() + " " + request.path() + "] does not support having a body",
+                    RestStatus.BAD_REQUEST
+                );
             }
             // Validate params
             if (workflowId == null) {

diff --git a/src/main/java/org/opensearch/flowframework/rest/RestGetWorkflowStateAction.java b/src/main/java/org/opensearch/flowframework/rest/RestGetWorkflowStateAction.java
@@ -67,8 +67,10 @@ protected BaseRestHandler.RestChannelConsumer prepareRequest(RestRequest request
 
             // Validate content
             if (request.hasContent()) {
-                // BaseRestHandler will give appropriate error message
-                return channel -> channel.sendResponse(null);
+                throw new FlowFrameworkException(
+                    "request [" + request.method() + " " + request.path() + "] does not support having a body",
+                    RestStatus.BAD_REQUEST
+                );
             }
             // Validate params
             if (workflowId == null) {

diff --git a/src/main/java/org/opensearch/flowframework/rest/RestProvisionWorkflowAction.java b/src/main/java/org/opensearch/flowframework/rest/RestProvisionWorkflowAction.java
@@ -77,8 +77,10 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
             }
             // Validate content
             if (request.hasContent()) {
-                // BaseRestHandler will give appropriate error message
-                return channel -> channel.sendResponse(null);
+                throw new FlowFrameworkException(
+                    "request [" + request.method() + " " + request.path() + "] does not support having a body",
+                    RestStatus.BAD_REQUEST
+                );
             }
             // Validate params
             if (workflowId == null) {

diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java
@@ -572,8 +572,8 @@ protected void getAndAssertWorkflowStatus(
         assertEquals(RestStatus.OK, TestHelpers.restStatus(response));
 
         Map<String, Object> responseMap = entityAsMap(response);
-        assertEquals(stateStatus.name(), (String) responseMap.get(CommonValue.STATE_FIELD));
-        assertEquals(provisioningStatus.name(), (String) responseMap.get(CommonValue.PROVISIONING_PROGRESS_FIELD));
+        assertEquals(stateStatus.name(), responseMap.get(CommonValue.STATE_FIELD));
+        assertEquals(provisioningStatus.name(), responseMap.get(CommonValue.PROVISIONING_PROGRESS_FIELD));
 
     }
 

diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java
@@ -22,8 +22,10 @@
 import org.opensearch.flowframework.model.WorkflowEdge;
 import org.opensearch.flowframework.model.WorkflowNode;
 import org.opensearch.flowframework.model.WorkflowState;
+import org.junit.Before;
 import org.junit.ComparisonFailure;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -39,6 +41,14 @@
 
 public class FlowFrameworkRestApiIT extends FlowFrameworkRestTestCase {
 
+    @Before
+    public void reportThreads() {
+        System.err.println("Active threads: " + Thread.activeCount());
+        for (Map.Entry<Thread, StackTraceElement[]> entry : Thread.getAllStackTraces().entrySet()) {
+            System.err.println(entry.getKey() + ": " + Arrays.toString(entry.getValue()));
+        }
+    }
+
     public void testSearchWorkflows() throws Exception {
 
         // Create a Workflow that has a credential 12345
@@ -69,7 +79,7 @@ public void testSearchWorkflows() throws Exception {
         }
     }
 
-    public void testFailedUpdateWorkflow() throws Exception {
+    public void XXtestFailedUpdateWorkflow() throws Exception {
         Template templateCreation = TestHelpers.createTemplateFromFile("createconnector-registerremotemodel-deploymodel.json");
         Response responseCreate = createWorkflow(client(), templateCreation);
         assertEquals(RestStatus.CREATED, TestHelpers.restStatus(responseCreate));
@@ -105,7 +115,7 @@ public void testFailedUpdateWorkflow() throws Exception {
 
     }
 
-    public void testCreateAndProvisionLocalModelWorkflow() throws Exception {
+    public void XXtestCreateAndProvisionLocalModelWorkflow() throws Exception {
         // Using a 1 step template to register a local model and deploy model
         Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json");
 
@@ -240,17 +250,15 @@ public void testCreateAndProvisionRemoteModelWorkflow() throws Exception {
         assertNotNull(resourcesCreated.get(2).resourceId());
     }
 
-    public void testCreateAndProvisionAgentFrameworkWorkflow() throws Exception {
+    public void xtestCreateAndProvisionAgentFrameworkWorkflow() 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;
         if (!indexExistsWithAdminClient(".plugins-ml-config")) {
-            assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 40, TimeUnit.SECONDS);
-            response = createWorkflowWithProvision(client(), template);
-        } else {
-            response = createWorkflowWithProvision(client(), template);
+            assertBusy(() -> assertTrue(indexExistsWithAdminClient(".plugins-ml-config")), 60, TimeUnit.SECONDS);
         }
+        response = createWorkflowWithProvision(client(), template);
         assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
         Map<String, Object> responseMap = entityAsMap(response);
         String workflowId = (String) responseMap.get(WORKFLOW_ID);