-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Fix issue with socket connection in windows tests due to java agent and jdk24 #18752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…nd jdk24 Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
| grant codeBase "${codebase.opensearch-nio}" { | ||
| // opensearch-nio makes and accepts socket connections | ||
| permission java.net.SocketPermission "*", "accept,resolve,connect"; | ||
| permission java.io.FilePermission "${java.io.tmpdir}${/}-", "delete"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be full permission to tmp, not just delete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not look like we need that, BootstrapForTesting should cover it: https://github.com/opensearch-project/OpenSearch/blob/main/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java#L150
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that permission globally applicable?
In the case of job-scheduler its failing saying that the test-framework does not have permission.
nested: SecurityException[Denied DELETE access to file: C:\Users\RUNNER~1\AppData\Local\Temp\socket_352975284, domain: ProtectionDomain (file:/C:/Users/runneradmin/.gradle/caches/modules-2/files-2.1/org.opensearch.test/framework/3.2.0-SNAPSHOT/9abe315135b637ba8f0541717563257b6c68262e/framework-3.2.0-SNAPSHOT.jar <no signer certificates>)
jdk.internal.loader.ClassLoaders$AppClassLoader@1540e19d
<no principals>
java.security.Permissions@6ad8bb7e (
)
];
at __randomizedtesting.SeedInfo.seed([7BBA49E01B0C78C6:C0E9A00E7D144B7]:0)
at app//org.opensearch.test.ExternalTestCluster.<init>(ExternalTestCluster.java:188)
at app//org.opensearch.test.ExternalTestCluster.<init>(ExternalTestCluster.java:200)
at app//org.opensearch.test.OpenSearchIntegTestCase.buildExternalCluster(OpenSearchIntegTestCase.java:2029)
at app//org.opensearch.test.OpenSearchIntegTestCase.buildTestCluster(OpenSearchIntegTestCase.java:2060)
at app//org.opensearch.test.OpenSearchTestClusterRule.lambda$buildWithPrivateContext$3(OpenSearchTestClusterRule.java:230)
at app//com.carrotsearch.randomizedtesting.RandomizedContext.runWithPrivateRandomness(RandomizedContext.java:187)
at app//com.carrotsearch.randomizedtesting.RandomizedContext.runWithPrivateRandomness(RandomizedContext.java:211)
at app//org.opensearch.test.OpenSearchTestClusterRule.buildWithPrivateContext(OpenSearchTestClusterRule.java:230)
at app//org.opensearch.test.OpenSearchTestClusterRule.buildAndPutCluster(OpenSearchTestClusterRule.java:272)
at app//org.opensearch.test.OpenSearchTestClusterRule.beforeInternal(OpenSearchTestClusterRule.java:156)
at app//org.opensearch.test.OpenSearchTestClusterRule.before(OpenSearchTestClusterRule.java:172)
at app//org.opensearch.test.OpenSearchTestClusterRule$1.evaluate(OpenSearchTestClusterRule.java:365)
at app//com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@reta could it be a problem that the permission is not being applied recursively?
Edit: ah nvm, it is being applied recursively: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/bootstrap/FilePermissionUtils.java#L104
Edit 2: actually it isn't being applied recursively because its hitting here: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/bootstrap/FilePermissionUtils.java#L100-L102
|
Job Scheduler error is slightly different: https://github.com/opensearch-project/job-scheduler/actions/runs/16224765197/job/45813987316 |
Signed-off-by: Craig Perkins <[email protected]>
|
❌ Gradle check result for b0d4384: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for b0d4384: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
|
❌ Gradle check result for 914e3e6: null Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for 30e4602: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Craig Perkins <[email protected]>
|
❌ Gradle check result for a96524b: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #18752 +/- ##
============================================
- Coverage 72.79% 72.77% -0.03%
+ Complexity 68525 68491 -34
============================================
Files 5574 5563 -11
Lines 314807 314440 -367
Branches 45675 45627 -48
============================================
- Hits 229178 228830 -348
- Misses 67046 67054 +8
+ Partials 18583 18556 -27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Closing in favor of #18764 |
Description
This is an attempt to fix CI issues seen on PRs like opensearch-project/k-NN#2792 and opensearch-project/job-scheduler#797 after running tests with JDK 24.
Ref workflow run: https://github.com/opensearch-project/k-NN/actions/runs/16281232544/job/45971164781?pr=2792
Full snapshot of error:
Link to error seen in job-scheduler: https://github.com/opensearch-project/job-scheduler/actions/runs/16224765197/job/45813987316
Related to this change in the JDK: openjdk/jdk24u@922b12f#diff-e3b15b354f90e593cfde2784f4b18b0dfb79b9845c08c5e90aac94c0ee53065a
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.