Merge pull request #507 from AndreKurait/GithubGradleImprovements

Gradle Execution Improvements and Upgrades

.github/workflows/CI.yml

@@ -16,10 +16,10 @@ jobs:
         python-version: ["3.10"]
     steps:
      - name: Checkout Repository
-       uses: actions/checkout@v3
+       uses: actions/checkout@v4
 
      - name: Set up Python ${{ matrix.python-version }}
-       uses: actions/setup-python@v3
+       uses: actions/setup-python@v5
        with:
          python-version: ${{ matrix.python-version }}
      - name: Install dependencies

.github/workflows/add-untriaged.yml

@@ -8,7 +8,7 @@ jobs:
   apply-label:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/github-script@v6
+      - uses: actions/github-script@v7
         with:
           script: |
             github.rest.issues.addLabels({

.github/workflows/backport.yml

@@ -16,14 +16,14 @@ jobs:
     steps:
       - name: GitHub App token
         id: github_app_token
-        uses: tibdex/github-app-token@v1.5.0
+        uses: tibdex/github-app-token@v2
         with:
           app_id: ${{ secrets.APP_ID }}
           private_key: ${{ secrets.APP_PRIVATE_KEY }}
           installation_id: 22958780
 
       - name: Backport
-        uses: VachaShah/backport@v2.1.0
+        uses: VachaShah/backport@v2
         with:
           github_token: ${{ steps.github_app_token.outputs.token }}
           head_template: backport/backport-<%= number %>-to-<%= base %>

.github/workflows/e2eTest.yml

@@ -14,22 +14,27 @@ jobs:
 
     steps:
       - name: Check out code
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - name: Set up JDK
-        uses: actions/setup-java@v2
+        uses: actions/setup-java@v4
         with:
           java-version: '11'
-          distribution: 'adopt'
+          distribution: 'corretto'
+
+      - name: Setup Gradle
+        uses: gradle/actions/setup-gradle@v3
+        with:
+          gradle-version: 8.0.2
+          gradle-home-cache-cleanup: true
+          cache-read-only: false
 
       - name: Start Docker Solution
-        run: |
-          cd TrafficCapture
-          chmod +x ./gradlew
-          ./gradlew dockerSolution:ComposeUp
+        run: ./gradlew dockerSolution:ComposeUp -x test
+        working-directory: TrafficCapture
 
       - name: Set up Python
-        uses: actions/setup-python@v3
+        uses: actions/setup-python@v5
         with:
           python-version: '3.10'
 

.github/workflows/gradle-build-and-test.yml

@@ -8,23 +8,31 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v2
+      - name: Checkout project sources
+        uses: actions/checkout@v4
+
       - name: Set up JDK 11
-        uses: actions/setup-java@v2
+        uses: actions/setup-java@v4
         with:
           java-version: '11'
-          distribution: 'adopt'
+          distribution: 'corretto'
+
+      - name: Setup Gradle
+        uses: gradle/actions/setup-gradle@v3
+        with:
+          gradle-version: 8.0.2
+          gradle-home-cache-cleanup: true
 
       - name: Run Gradle Build
-        run: ./gradlew build -x test
+        run: ./gradlew build -x test --info
         working-directory: TrafficCapture
 
       - name: Run Tests with Coverage
         run: ./gradlew jacocoTestReport --info
         working-directory: TrafficCapture
 
       - name: Upload to Codecov
-        uses: codecov/codecov-action@v3
+        uses: codecov/codecov-action@v4
         with:
           files: "TrafficCapture/**/jacocoTestReport.xml"
           flags: unittests

.github/workflows/linkCheck.yml

@@ -13,10 +13,10 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
       - name: lychee Link Checker
         id: lychee
-        uses: lycheeverse/lychee-action@v1.5.0
+        uses: lycheeverse/lychee-action@v1
         with:
           args: --verbose --accept=200,403,429  "**/*.html" "**/*.md" "**/*.txt" "**/*.json"
             --exclude "file:///github/workspace/*"

.github/workflows/python-tests.yml

@@ -20,9 +20,9 @@ jobs:
         working-directory: ./experimental/cluster_migration_core
     steps:
       - name: Checkout Repository
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v3
+        uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
       - name: Install Dependencies for Framework and Test Coverage
@@ -32,6 +32,6 @@ jobs:
         run: |
           python -m pytest unit_tests/ --cov=cluster_migration_core --cov-report=xml --cov-branch
       - name: Upload Coverage Report
-        uses: codecov/codecov-action@v3
+        uses: codecov/codecov-action@v4
         with:
           files: cluster_migration_core/coverage.xml

.github/workflows/release-drafter.yml

@@ -11,7 +11,7 @@ jobs:
     name: Draft a release
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
       - id: get_data
         run: |
           echo "approvers=$(cat .github/CODEOWNERS | grep @ | tr -d '*\n ' | sed 's/@/,/g' | sed 's/,//1')" >> $GITHUB_OUTPUT

TrafficCapture/build.gradle

@@ -3,8 +3,10 @@ plugins {
     id 'org.owasp.dependencycheck' version '8.2.1'
 }
 
-repositories {
-    mavenCentral()
+allprojects {
+    repositories {
+        mavenCentral()
+    }
 }
 
 allprojects {
@@ -19,38 +21,34 @@ allprojects {
     }
 
     jacoco {
-        toolVersion = '0.8.9'
+        toolVersion = '0.8.11'
     }
 
-    test {
+    tasks.withType(Test) {
         // Provide way to exclude particular tests from CLI
         // e.g. ./gradlew test -PexcludeTests=**/KafkaProtobufConsumerLongTermTest*
         if (project.hasProperty('excludeTests')) {
             exclude project.property('excludeTests')
         }
+        useJUnitPlatform {
+            systemProperty 'junit.jupiter.execution.parallel.enabled', 'true'
+            systemProperty 'junit.jupiter.execution.parallel.mode.default', "concurrent"
+            systemProperty 'junit.jupiter.execution.parallel.mode.classes.default', 'concurrent'
+        }
+    }
+
+    test {
         systemProperty 'disableMemoryLeakTests', 'true'
         useJUnitPlatform {
             excludeTags 'longTest'
         }
-        jvmArgs '-ea'
-
         jacoco {
             enabled = false
         }
     }
 
     task slowTest(type: Test) {
-        dependsOn test
-        // Provide way to exclude particular tests from CLI
-        // e.g. ./gradlew test -PexcludeTests=**/KafkaProtobufConsumerLongTermTest*
-        if (project.hasProperty('excludeTests')) {
-            exclude project.property('excludeTests')
-        }
         systemProperty 'disableMemoryLeakTests', 'false'
-        useJUnitPlatform {
-        }
-        jvmArgs '-ea'
-
         jacoco {
             enabled = true
         }
@@ -86,4 +84,4 @@ jacocoTestReport {
         html.required = true
         html.destination file("${buildDir}/reports/jacoco/test/html")
     }
-}
+}

TrafficCapture/gradle.properties

@@ -0,0 +1,7 @@
+org.gradle.caching=true
+org.gradle.configuration-cache=true
+org.gradle.configureondemand=true
+
+# Set Gradle Daemon's idle timeout to 30 minutes
+org.gradle.daemon.idletimeout=1800000
+org.gradle.parallel=true

TrafficCapture/nettyWireLogging/src/test/java/org/opensearch/migrations/trafficcapture/netty/ConditionallyReliableLoggingHttpHandlerTest.java

@@ -11,6 +11,7 @@
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.ResourceLock;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.opensearch.migrations.testutils.TestUtilities;
@@ -128,6 +129,7 @@ private static byte[] consumeIntoArray(ByteBuf m) {
 
     @ParameterizedTest
     @ValueSource(booleans = {true, false})
+    @ResourceLock("OpenTelemetryExtension")
     public void testThatAPostInASinglePacketBlocksFutureActivity(boolean usePool) throws IOException {
         byte[] fullTrafficBytes = SimpleRequests.SMALL_POST.getBytes(StandardCharsets.UTF_8);
         var bb = TestUtilities.getByteBuf(fullTrafficBytes, usePool);
@@ -137,6 +139,7 @@ public void testThatAPostInASinglePacketBlocksFutureActivity(boolean usePool) th
 
     @ParameterizedTest
     @ValueSource(booleans = {true, false})
+    @ResourceLock("OpenTelemetryExtension")
     public void testThatAPostInTinyPacketsBlocksFutureActivity(boolean usePool) throws IOException {
         byte[] fullTrafficBytes = SimpleRequests.SMALL_POST.getBytes(StandardCharsets.UTF_8);
         writeMessageAndVerify(fullTrafficBytes, getSingleByteAtATimeWriter(usePool, fullTrafficBytes));
@@ -261,6 +264,7 @@ private Consumer<EmbeddedChannel> getWriter(boolean singleBytes, boolean usePool
     @ParameterizedTest
     @ValueSource(booleans = {true, false})
     @WrapWithNettyLeakDetection(repetitions = 16)
+    @ResourceLock("OpenTelemetryExtension")
     public void testThatAPostInTinyPacketsBlocksFutureActivity_withLeakDetection(boolean usePool) throws Exception {
         testThatAPostInTinyPacketsBlocksFutureActivity(usePool);
         //MyResourceLeakDetector.dumpHeap("nettyWireLogging_"+COUNT+"_"+ Instant.now() +".hprof", true);
@@ -269,6 +273,7 @@ public void testThatAPostInTinyPacketsBlocksFutureActivity_withLeakDetection(boo
     @ParameterizedTest
     @ValueSource(booleans = {true, false})
     @WrapWithNettyLeakDetection(repetitions = 32)
+    @ResourceLock("OpenTelemetryExtension")
     public void testThatAPostInASinglePacketBlocksFutureActivity_withLeakDetection(boolean usePool) throws Exception {
         testThatAPostInASinglePacketBlocksFutureActivity(usePool);
         //MyResourceLeakDetector.dumpHeap("nettyWireLogging_"+COUNT+"_"+ Instant.now() +".hprof", true);

TrafficCapture/replayerPlugins/build.gradle

@@ -2,4 +2,4 @@ subprojects {
     repositories {
         mavenCentral()
     }
-}
+}

TrafficCapture/trafficCaptureProxyServer/src/main/java/org/opensearch/migrations/trafficcapture/proxyserver/netty/ExpiringSubstitutableItemPool.java

@@ -319,8 +319,6 @@ private String toStringOnThread() {
             sb.append(", numInProgressItems=").append(inProgressItems.size());
             sb.append(", numReadyItems=").append(readyItems.size());
         }
-        sb.append(", inProgressItems=").append(inProgressItems);
-        sb.append(", readyItems=").append(readyItems);
         sb.append(", itemSupplier=").append(itemSupplier);
         sb.append(", onExpirationConsumer=").append(onExpirationConsumer);
         sb.append(", eventLoop=").append(eventLoop);

TrafficCapture/trafficCaptureProxyServer/src/test/java/org/opensearch/migrations/trafficcapture/proxyserver/netty/ExpiringSubstitutableItemPoolTest.java

@@ -20,8 +20,10 @@
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
+import org.junit.jupiter.api.parallel.Isolated;
 
 @Slf4j
+@Isolated("Isolation based on temporal checks")
 class ExpiringSubstitutableItemPoolTest {
 
     public static final int NUM_POOLED_ITEMS = 5;
@@ -69,7 +71,6 @@ void get() throws Exception {
                     return rval;
                 },
                 item->{
-                    expireCountdownLatch.countDown();
                     log.info("Expiring item: "+item);
                     try {
                         expiredItems.add(item.get());
@@ -79,6 +80,7 @@ void get() throws Exception {
                     } catch (ExecutionException e) {
                         throw Lombok.sneakyThrow(e);
                     }
+                    expireCountdownLatch.countDown();
                 });
         for (int i = 0; i<NUM_POOLED_ITEMS; ++i) {
             Thread.sleep(TIME_BETWEEN_INITIAL_ITEMS.toMillis());

TrafficCapture/trafficReplayer/build.gradle

@@ -119,3 +119,10 @@ jar {
         attributes 'Main-Class': application.mainClass
     }
 }
+
+// TODO: Fix Parallel Execution for junit trafficReplayer tests
+//  with resourceSharing of TrafficReplayerRunner to eliminate forking and rely on threads
+slowTest {
+    forkEvery(1)
+    maxParallelForks = Runtime.runtime.availableProcessors()
+}

TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/ResultsToLogsConsumer.java

@@ -19,8 +19,17 @@ public class ResultsToLogsConsumer implements BiConsumer<SourceTargetCaptureTupl
     public static final String TRANSACTION_SUMMARY_LOGGER = "TransactionSummaryLogger";
     private static final ObjectMapper PLAIN_MAPPER = new ObjectMapper();
 
-    Logger tupleLogger = LoggerFactory.getLogger(OUTPUT_TUPLE_JSON_LOGGER);
-    Logger progressLogger = makeTransactionSummaryLogger();
+    private final Logger tupleLogger;
+    private final Logger progressLogger;
+
+    public ResultsToLogsConsumer() {
+        this(null, null);
+    }
+
+    public ResultsToLogsConsumer(Logger tupleLogger, Logger progressLogger) {
+        this.tupleLogger = tupleLogger != null ? tupleLogger : LoggerFactory.getLogger(OUTPUT_TUPLE_JSON_LOGGER);
+        this.progressLogger = progressLogger != null ? progressLogger : makeTransactionSummaryLogger();
+    }
 
     // set this up so that the preamble prints out once, right after we have a logger
     // if it's configured to output at all

TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/BlockingTrafficSourceTest.java

@@ -69,7 +69,7 @@ void readNextChunkTest() throws Exception {
         blockingSource.stopReadsPast(sourceStartTime.plus(Duration.ofMillis(nStreamsToCreate)));
         var exception = Assertions.assertThrows(ExecutionException.class,
                 ()->blockingSource.readNextTrafficStreamChunk(rootContext::createReadChunkContext)
-                        .get(10, TimeUnit.MILLISECONDS));
+                        .get(10000, TimeUnit.MILLISECONDS));
         Assertions.assertInstanceOf(EOFException.class, exception.getCause());
     }
 

TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/FullReplayerWithTracingChecksTest.java

@@ -7,6 +7,7 @@
 import lombok.extern.slf4j.Slf4j;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.ResourceLock;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.opensearch.migrations.replay.traffic.source.BlockingTrafficSource;
@@ -41,6 +42,7 @@ protected TestContext makeInstrumentationContext() {
     }
 
     @Test
+    @ResourceLock("TrafficReplayerRunner")
     public void testSingleStreamWithCloseIsCommitted() throws Throwable {
         var random = new Random(1);
         var httpServer = SimpleNettyHttpServer.makeServer(false, Duration.ofMillis(2),
@@ -57,11 +59,13 @@ public void testSingleStreamWithCloseIsCommitted() throws Throwable {
                 () -> TestContext.withAllTracking(),
                 trafficSourceSupplier);
         Assertions.assertEquals(1, trafficSourceSupplier.nextReadCursor.get());
+        httpServer.close();
         log.info("done");
     }
 
     @ParameterizedTest
     @ValueSource(ints = {1,2})
+    @ResourceLock("TrafficReplayerRunner")
     public void testStreamWithRequestsWithCloseIsCommittedOnce(int numRequests) throws Throwable {
         var random = new Random(1);
         var httpServer = SimpleNettyHttpServer.makeServer(false, Duration.ofMillis(2),
@@ -108,7 +112,8 @@ public void testStreamWithRequestsWithCloseIsCommittedOnce(int numRequests) thro
                         Assertions.assertTrue(wasNew);
                     });
         } finally {
-            tr.shutdown(null);
+            tr.shutdown(null).join();
+            httpServer.close();
         }
 
         Assertions.assertEquals(numRequests, tuplesReceived.size());