-
Notifications
You must be signed in to change notification settings - Fork 125
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
Adding gradle task for running integ tests in remote cluster #266
Adding gradle task for running integ tests in remote cluster #266
Conversation
Signed-off-by: Martin Gaievski <gaievski@amazon.com>
Codecov Report
@@ Coverage Diff @@
## main #266 +/- ##
=========================================
Coverage 83.37% 83.37%
Complexity 884 884
=========================================
Files 127 127
Lines 3832 3832
Branches 361 361
=========================================
Hits 3195 3195
Misses 475 475
Partials 162 162 Continue to review full report at Codecov.
|
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.
@martin-gaievski When I ran it, BWC failed:
REPRODUCE WITH: ./gradlew ':integTestRemote' --tests "org.opensearch.knn.bwc.KNNBackwardsCompatibilityIT.testBackwardsCompatibility" -Dtests.seed=47B9EB07745E3976 -Dtests.security.manager=true -Dtests.locale=cs -Dtests.timezone=Pacific/Port_Moresby -Druntime.java=14
org.opensearch.knn.bwc.KNNBackwardsCompatibilityIT > testBackwardsCompatibility FAILED
java.lang.NullPointerException
at __randomizedtesting.SeedInfo.seed([47B9EB07745E3976:AC86D6C90E85ECFC]:0)
at org.opensearch.knn.bwc.KNNBackwardsCompatibilityIT$ClusterType.parse(KNNBackwardsCompatibilityIT.java:77)
at org.opensearch.knn.bwc.KNNBackwardsCompatibilityIT.getClusterType(KNNBackwardsCompatibilityIT.java:91)
at org.opensearch.knn.bwc.KNNBackwardsCompatibilityIT.testBackwardsCompatibility(KNNBackwardsCompatibilityIT.java:98)
Suite: Test class org.opensearch.knn.bwc.KNNBackwardsCompatibilityIT
1> [2022-01-25T03:25:47,877][INFO ][o.o.k.b.KNNBackwardsCompatibilityIT] [testBackwardsCompatibility] before test
1> [2022-01-25T03:25:47,883][INFO ][o.o.k.b.KNNBackwardsCompatibilityIT] [testBackwardsCompatibility] initializing REST clients against [http://localhost:9200]
1> [2022-01-25T03:25:48,046][INFO ][o.o.k.b.KNNBackwardsCompatibilityIT] [testBackwardsCompatibility] after test
2> REPRODUCE WITH: ./gradlew ':integTestRemote' --tests "org.opensearch.knn.bwc.KNNBackwardsCompatibilityIT.testBackwardsCompatibility" -Dtests.seed=47B9EB07745E3976 -Dtests.security.manager=true -Dtests.locale=cs -Dtests.timezone=Pacific/Port_Moresby -Druntime.java=14
2> java.lang.NullPointerException
at __randomizedtesting.SeedInfo.seed([47B9EB07745E3976:AC86D6C90E85ECFC]:0)
at org.opensearch.knn.bwc.KNNBackwardsCompatibilityIT$ClusterType.parse(KNNBackwardsCompatibilityIT.java:77)
at org.opensearch.knn.bwc.KNNBackwardsCompatibilityIT.getClusterType(KNNBackwardsCompatibilityIT.java:91)
at org.opensearch.knn.bwc.KNNBackwardsCompatibilityIT.testBackwardsCompatibility(KNNBackwardsCompatibilityIT.java:98)
2> NOTE: leaving temporary files on disk at: /home/test/k-NN/build/testrun/integTestRemote/temp/org.opensearch.knn.bwc.KNNBackwardsCompatibilityIT_47B9EB07745E3976-001
2> NOTE: test params are: codec=Asserting(Lucene87): {}, docValues:{}, maxPointsInLeafNode=1640, maxMBSortInHeap=5.364859996135681, sim=Asserting(RandomSimilarity(queryNorm=false): {}), locale=cs, timezone=Pacific/Port_Moresby
2> NOTE: Linux 5.10.25-linuxkit amd64/Oracle Corporation 14 (64-bit)/cpus=2,threads=1,free=356098048,total=536870912
2> NOTE: All tests run in this JVM: [PainlessScriptScoringIT, KNNScriptScoringIT, RestLegacyKNNStatsHandlerIT, RestLegacyKNNWarmupHandlerIT, RestDeleteModelHandlerIT, RestKNNStatsHandlerIT, RestGetModelHandlerIT, RestSearchModelHandlerIT, RestKNNWarmupHandlerIT, RestTrainModelHandlerIT, KNNBackwardsCompatibilityIT]
// Run tests with remote cluster only if rest case is defined | ||
if (System.getProperty("tests.rest.cluster") != null) { | ||
filter { | ||
includeTestsMatching "org.opensearch.knn.*IT" |
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.
I think, you should add this in the filter to fix the BWC issue which Jack mentioned in review comments
excludeTestsMatching "org.opensearch.knn.bwc.*IT"
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.
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.
Thanks Naveen, it's a good point as we need to have multiple external clusters for BWC. For now I've added bwc tests to exclusion matching rule
Signed-off-by: Martin Gaievski <gaievski@amazon.com>
DEVELOPER_GUIDE.md
Outdated
Integration tests can be run with remote cluster. For that run the following command and replace host/port/cluster name values with ones for the target cluster: | ||
|
||
``` | ||
./gradlew :integTestRemote -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="integTest-0" -Dhttps=false |
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.
shall we include an example with secure cluster (username/password) as well?
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.
Sure, that makes sense. Let me update the guide
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.
Added instructions for secure cluster
Signed-off-by: Martin Gaievski <gaievski@amazon.com>
@@ -33,7 +33,7 @@ private void tripCb() throws Exception { | |||
updateClusterSettings("knn.memory.circuit_breaker.limit", "1kb"); | |||
|
|||
// Create index with 1 primary and numNodes-1 replicas so that the data will be on every node in the cluster | |||
int numNodes = Integer.parseInt(System.getProperty("cluster.number_of_nodes")); | |||
int numNodes = Integer.parseInt(System.getProperty("cluster.number_of_nodes", "1")); |
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.
Added default value for number of clusters as 1, this may be missing for remote tests and lead to NPE
…script Signed-off-by: Martin Gaievski <gaievski@amazon.com>
…rch-project#266) * Adding gradle task for running integ tests in remote cluster Signed-off-by: Martin Gaievski <gaievski@amazon.com>
…rch-project#266) * Adding gradle task for running integ tests in remote cluster Signed-off-by: Martin Gaievski <gaievski@amazon.com>
…rch-project#266) * Adding gradle task for running integ tests in remote cluster Signed-off-by: Martin Gaievski <gaievski@amazon.com>
Signed-off-by: Martin Gaievski gaievski@amazon.com
Description
Adding integTestRemote gradle task that allows to run kNN integration tests in remote cluster
Issues Resolved
#198
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.