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.

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

Integration test for cluster reroute in #81

Closed
wants to merge 2 commits into from
Closed

Conversation

tbhanu-amzn
Copy link
Contributor

Description

Integration test for cluster reroute case

Check List

  • Commits are signed per the DCO using --signoff

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.

@@ -334,6 +335,87 @@ abstract class MultiClusterRestTestCase : ESTestCase() {
assertEquals(HttpStatus.SC_OK.toLong(), persistentConnectionResponse.statusLine.statusCode.toLong())
}

protected fun getPrimaryNodeForShard(clusterName: String,indexname: String, shardNumber: String) :String {
val cluster = getNamedCluster(clusterName)
val persistentConnectionRequest = Request("GET", "/_cat/shards")
Copy link
Member

Choose a reason for hiding this comment

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

You can call _cat/shards?format=json to make it easier to parse the response.

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


protected fun getNodesInCluster(clusterName: String) : List<String>{
val cluster = getNamedCluster(clusterName)
val persistentConnectionRequest = Request("GET", "/_cat/nodes")
Copy link
Member

Choose a reason for hiding this comment

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

Same suggestion as above to use _cat/nodes?format=json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

persistentConnectionRequest.entity = NStringEntity(entityAsString, ContentType.APPLICATION_JSON)
cluster.lowLevelClient.performRequest(persistentConnectionRequest)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

minor: new lines at the end of new files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +40 to +42
assertBusy ({
Assertions.assertThat(followerClient.indices().exists(GetIndexRequest(followerIndexName), RequestOptions.DEFAULT)).isEqualTo(true)
}, 1, TimeUnit.MINUTES)
Copy link
Member

Choose a reason for hiding this comment

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

I dont think you need to wait here to ensure that follower index exists. You can proceed to add data to leader.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@saikaranam-amazon saikaranam-amazon self-requested a review August 5, 2021 06:45
Comment on lines +66 to +73
//TODO: Fix this and uncomment this code
/*assertBusy ({
try {
Assertions.assertThat(docs(FOLLOWER, followerIndexName)).contains("dummy data 2")
} catch (ex: Exception) {
Assertions.assertThat(true).isEqualTo(false)
}
}, 1, TimeUnit.MINUTES)*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the current issue here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is solved in the pr #114

Copy link
Collaborator

@krishna-ggk krishna-ggk left a comment

Choose a reason for hiding this comment

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

Should we drop this since these are covered in diff PR?

@tbhanu-amzn
Copy link
Contributor Author

This is already addressed in #114

@tbhanu-amzn tbhanu-amzn closed this Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants