[Backport main] [3.4] Prevent use of shared distribution on gradle builds #20228

opensearch-trigger-bot · 2025-12-13T18:30:08Z

Backport 0033614 from #20227.

Summary by CodeRabbit

Bug Fixes
- Fixed distribution handling for improved compatibility with Gradle 8.6 and later.
Tests
- Updated test assertions for distribution validation logic.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

* Prevent use of shared distribution on gradle builds Signed-off-by: Daniel Widdis <[email protected]> * Test the new workaround condition Signed-off-by: Daniel Widdis <[email protected]> --------- Signed-off-by: Daniel Widdis <[email protected]> (cherry picked from commit 0033614) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

coderabbitai · 2025-12-13T18:30:30Z

Walkthrough

The PR modifies test cluster and distro selection logic across two files: a test assertion for custom distro validation is inverted to check for existence rather than absence, and the shared distribution selection logic is disabled to unconditionally force usage of the working directory distro path.

Changes

Cohort / File(s)	Summary
Test Cluster Distro Handling `buildSrc/src/integTest/groovy/org/opensearch/gradle/TestClustersPluginFuncTest.groovy`	Modified `assertNoCustomDistro` assertion to validate custom distro folder existence (positive check) instead of non-existence (negative check)
Shared Distribution Selection `buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchNode.java`	Changed `canUseSharedDistribution()` to unconditionally return false, forcing all builds to use working directory distro path instead of conditionally checking Windows platform and jar/module/plugin status; added Gradle 8.6+ GC logging workaround comment

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Verify that unconditionally disabling shared distribution selection doesn't have unintended side effects on other test cluster initialization paths
Confirm the test assertion flip correctly validates the expected behavior for custom distro scenarios
Review the Gradle 8.6+ JVM GC logging workaround context to understand the motivation for the change

Poem

🐰 A distro path, now always true,
No sharing today—just me and you.
Tests flip and dance in assertion light,
Working directories set things right! ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description lacks required template sections including a detailed Description, Related Issues reference, and Check List verification, providing only a minimal backport reference.	Add the required Description section explaining the change purpose, reference the related issue (#20227), and complete the Check List to confirm testing and compliance requirements.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly indicates this is a backport PR that prevents shared distribution use in gradle builds, directly matching the changes shown in the raw summary.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch backport/backport-20227-to-main

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

prudhvigodithi · 2025-12-13T18:31:42Z

Seeing #20227 should we wait on this PR and have a permanent fix ?

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchNode.java (1)
590-596: Workaround disables shared distribution — consider cleanup and documentation.

The unconditional return false prevents shared distribution usage on all platforms, forcing custom distro creation in the working directory. While the workaround addresses a Gradle 8.6+ JVM GC logging issue, consider:

Commented code: Remove the commented logic or move it to a dedicated issue/TODO if this is temporary

Performance note: Document that this increases disk I/O and space usage since each test cluster now receives its own distro copy

Tracking: Ensure there's an issue to revisit this once the root cause is resolved

Apply this diff to clean up the commented code:
 private boolean canUseSharedDistribution() {
-    // using original location can be too long due to MAX_PATH restrictions on windows CI
-    // TODO revisit when moving to shorter paths on CI by using Teamcity
-    // return OS.current() != OS.WINDOWS && extraJarFiles.size() == 0 && modules.size() == 0 && plugins.size() == 0;
     // Workaround on Gradle 8.6+ due to JVM GC logging to OpenSearch execution directory in immutable workspace
+    // TODO: Restore conditional logic once Gradle GC logging issue is resolved (see #20227)
     return false;
 }
buildSrc/src/integTest/groovy/org/opensearch/gradle/TestClustersPluginFuncTest.groovy (1)
123-128: Method name contradicts its behavior — refactor for clarity.

The method assertNoCustomDistro now asserts that the custom distro folder exists, which is the opposite of what the name implies. This contradicts the method's intent and could confuse future maintainers.

Consider one of these approaches:

Option 1: Rename the method to reflect current behavior
-    boolean assertNoCustomDistro(String clusterName) {
+    boolean assertCustomDistro(String clusterName) {
         // Temporarily disabled, see https://github.com/opensearch-project/OpenSearch/pull/20227
         // assert !customDistroFolder(clusterName).exists()
         assert customDistroFolder(clusterName).exists()
         true
     }
And update the call site on line 82:
-        assertNoCustomDistro('myCluster')
+        assertCustomDistro('myCluster')
Option 2: Keep both behaviors and switch based on a flag
boolean assertNoCustomDistro(String clusterName) {
    // Temporarily expect custom distro due to shared distribution being disabled (PR #20227)
    boolean expectCustomDistro = true // TODO: Set to false once shared distribution is re-enabled
    if (expectCustomDistro) {
        assert customDistroFolder(clusterName).exists()
    } else {
        assert !customDistroFolder(clusterName).exists()
    }
    true
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b09dcc9 and 3468bf0.

📒 Files selected for processing (2)

buildSrc/src/integTest/groovy/org/opensearch/gradle/TestClustersPluginFuncTest.groovy (1 hunks)
buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchNode.java (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

GitHub Check: gradle-check
GitHub Check: precommit (25, windows-latest)
GitHub Check: precommit (25, macos-15)
GitHub Check: precommit (21, macos-15-intel)
GitHub Check: precommit (25, macos-15-intel)
GitHub Check: precommit (25, ubuntu-latest)
GitHub Check: precommit (21, ubuntu-latest)
GitHub Check: precommit (25, ubuntu-24.04-arm)
GitHub Check: precommit (21, windows-2025, true)
GitHub Check: precommit (21, windows-latest)
GitHub Check: precommit (21, macos-15)
GitHub Check: precommit (21, ubuntu-24.04-arm)
GitHub Check: assemble (25, ubuntu-latest)
GitHub Check: assemble (21, ubuntu-24.04-arm)
GitHub Check: assemble (25, ubuntu-24.04-arm)
GitHub Check: assemble (21, windows-latest)
GitHub Check: assemble (25, windows-latest)
GitHub Check: assemble (21, ubuntu-latest)
GitHub Check: Analyze (java)
GitHub Check: detect-breaking-change

dbwiddis · 2025-12-13T18:35:46Z

Seeing #20227 should we wait on this PR and have a permanent fix ?

Yes, that was what I was just coming here to say!

Let's take our time and try to find a better solution by 3.5.

My only thought on having this on main is to prevent future merge conflicts for the permanent fix if we ever have a need to backport to 3.4 for a patch release ... and to allow CCR integ tests (without security) to pass until we fix it.

github-actions · 2025-12-13T19:52:01Z

✅ Gradle check result for 3468bf0: SUCCESS

codecov · 2025-12-13T19:52:23Z

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.28%. Comparing base (66ed5cb) to head (3468bf0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...opensearch/gradle/testclusters/OpenSearchNode.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #20228      +/-   ##
============================================
+ Coverage     72.82%   73.28%   +0.46%     
- Complexity    71315    71814     +499     
============================================
  Files          5795     5795              
  Lines        328297   328302       +5     
  Branches      47282    47283       +1     
============================================
+ Hits         239089   240612    +1523     
+ Misses        69893    68431    -1462     
+ Partials      19315    19259      -56

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dbwiddis

Better fix: #20229

peterzhuamazon · 2025-12-14T06:26:55Z

Put to draft to see if #20229 can be merged for permanent fix. Thanks.

opensearch-trigger-bot bot requested a review from a team as a code owner December 13, 2025 18:30

coderabbitai bot reviewed Dec 13, 2025

View reviewed changes

dbwiddis mentioned this pull request Dec 14, 2025

Fix OpenSearchNode working directory to avoid executing in immutable Gradle cache directory #20229

Open

1 task

dbwiddis requested changes Dec 14, 2025

View reviewed changes

peterzhuamazon marked this pull request as draft December 14, 2025 06:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Backport main] [3.4] Prevent use of shared distribution on gradle builds #20228

[Backport main] [3.4] Prevent use of shared distribution on gradle builds #20228

opensearch-trigger-bot bot commented Dec 13, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 13, 2025 •

edited

Loading

Uh oh!

prudhvigodithi commented Dec 13, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

dbwiddis commented Dec 13, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Dec 13, 2025

Uh oh!

codecov bot commented Dec 13, 2025

Uh oh!

dbwiddis left a comment

Uh oh!

peterzhuamazon commented Dec 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[Backport main] [3.4] Prevent use of shared distribution on gradle builds #20228

Are you sure you want to change the base?

[Backport main] [3.4] Prevent use of shared distribution on gradle builds #20228

Conversation

opensearch-trigger-bot bot commented Dec 13, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

prudhvigodithi commented Dec 13, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

dbwiddis commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 13, 2025

Uh oh!

codecov bot commented Dec 13, 2025

Codecov Report

Uh oh!

dbwiddis left a comment

Choose a reason for hiding this comment

Uh oh!

peterzhuamazon commented Dec 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

opensearch-trigger-bot bot commented Dec 13, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 13, 2025 •

edited

Loading

dbwiddis commented Dec 13, 2025 •

edited

Loading