Skip to content
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.

Sign up for GitHub

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

Consume numNodes sent during integTestRemote #1085

Merged
merged 3 commits into from
Jul 19, 2023
Merged

Consume numNodes sent during integTestRemote #1085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b15905
Consume numNodes sent during integTestRemote
monusingh-1 Jul 19, 2023
98fc661
Consume numNodes sent during integTestRemote
monusingh-1 Jul 19, 2023
4e91be6
Address comments
monusingh-1 Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,9 @@ task integTestRemote (type: RestIntegTestTask) {
systemProperty "tests.cluster.leaderCluster.security_enabled", System.getProperty("security_enabled")

nonInputProperties.systemProperty('tests.integTestRemote', "true")
var numberOfNodes = findProperty('numNodes') as Integer
systemProperty "tests.cluster.followCluster.total_nodes", "${-> numberOfNodes.toString()}"
systemProperty "tests.cluster.leaderCluster.total_nodes", "${-> numberOfNodes.toString()}"
systemProperty "build.dir", "${buildDir}"
Copy link
Collaborator Author

@monusingh-1 monusingh-1 Jul 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For integTestRemote set total_nodes


}
Expand Down
18 changes: 13 additions & 5 deletions src/test/kotlin/org/opensearch/replication/MultiClusterRestTestCase.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ abstract class MultiClusterRestTestCase : OpenSearchTestCase() {
lateinit var testClusters : Map<String, TestCluster>
var isSecurityPropertyEnabled = false
var forceInitSecurityConfiguration = false
var isMultiNodeClusterConfiguration = true

internal fun createTestCluster(configuration: ClusterConfiguration) : TestCluster {
return createTestCluster(configuration.clusterName, configuration.preserveSnapshots, configuration.preserveIndices,
Expand All @@ -131,7 +130,6 @@ abstract class MultiClusterRestTestCase : OpenSearchTestCase() {
val httpHostsProp = systemProperties.get("tests.cluster.${cluster}.http_hosts") as String?
val transportHostsProp = systemProperties.get("tests.cluster.${cluster}.transport_hosts") as String?
val securityEnabled = systemProperties.get("tests.cluster.${cluster}.security_enabled") as String?
val totalNodes = systemProperties.get("tests.cluster.${cluster}.total_nodes") as String?

requireNotNull(httpHostsProp) { "Missing http hosts property for cluster: $cluster."}
requireNotNull(transportHostsProp) { "Missing transport hosts property for cluster: $cluster."}
Expand All @@ -143,9 +141,6 @@ abstract class MultiClusterRestTestCase : OpenSearchTestCase() {
isSecurityPropertyEnabled = true
}

if(totalNodes != null && totalNodes < "2") {
isMultiNodeClusterConfiguration = false
}

forceInitSecurityConfiguration = isSecurityPropertyEnabled && initSecurityConfiguration

Expand Down Expand Up @@ -664,6 +659,19 @@ abstract class MultiClusterRestTestCase : OpenSearchTestCase() {
return integTestRemote.equals("true")
}

protected fun isMultiNodeClusterConfiguration(leaderCluster: String, followerCluster: String): Boolean{
val systemProperties = BootstrapInfo.getSystemProperties()
val totalLeaderNodes = systemProperties.get("tests.cluster.${leaderCluster}.total_nodes") as String
val totalFollowerNodes = systemProperties.get("tests.cluster.${followerCluster}.total_nodes") as String

assertNotNull(totalLeaderNodes)
assertNotNull(totalFollowerNodes)
if(totalLeaderNodes < "2" || totalFollowerNodes < "2" ) {
return false
}
return true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why true is the default behaviour here? If the system property is not getting stuck, we'll end up not skipping the tests and thus failing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If integ tests are run locally, we are always setting the property and same with integTest Remote

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then let's remove the null check

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and add an assertion instead

}

protected fun docCount(cluster: RestHighLevelClient, indexName: String) : Int {
val persistentConnectionRequest = Request("GET", "/$indexName/_search?pretty&q=*")

Expand Down
2 changes: 1 addition & 1 deletion src/test/kotlin/org/opensearch/replication/integ/rest/ClusterRerouteFollowerIT.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ClusterRerouteFollowerIT : MultiClusterRestTestCase() {

@Before
fun beforeTest() {
Assume.assumeTrue(isMultiNodeClusterConfiguration)
Assume.assumeTrue(isMultiNodeClusterConfiguration(LEADER, FOLLOWER))
}

fun `test replication works after rerouting a shard from one node to another in follower cluster`() {
Expand Down
2 changes: 1 addition & 1 deletion src/test/kotlin/org/opensearch/replication/integ/rest/ClusterRerouteLeaderIT.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ClusterRerouteLeaderIT : MultiClusterRestTestCase() {

@Before
fun beforeTest() {
Assume.assumeTrue(isMultiNodeClusterConfiguration)
Assume.assumeTrue(isMultiNodeClusterConfiguration(LEADER, FOLLOWER))
}

fun `test replication works after rerouting a shard from one node to another in leader cluster`() {
Expand Down
2 changes: 1 addition & 1 deletion src/test/kotlin/org/opensearch/replication/integ/rest/StartReplicationIT.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1223,7 +1223,7 @@ class StartReplicationIT: MultiClusterRestTestCase() {
fun `test that wait_for_active_shards setting is updated on follower through start replication api`() {

//Ignore this test if clusters dont have multiple nodes
if(!isMultiNodeClusterConfiguration){
if(!isMultiNodeClusterConfiguration(LEADER, FOLLOWER)){
return
}

Expand Down