Skip to content

Conversation

@andrershov
Copy link
Contributor

ZenDiscoveryIT contained 5 tests. 3 run without changes, testNodeRejectsClusterStateWithWrongMasterNode removed, testHandleNodeJoin_incompatibleClusterState changed.

@andrershov andrershov added >test Issues or PRs that are addressing/adding tests v7.0.0 :Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. labels Jan 15, 2019
@andrershov andrershov requested a review from ywelsch January 15, 2019 10:30
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed


assertThat(holder.get(), notNullValue());
assertThat(holder.get().getMessage(), equalTo("failure when sending a validation request to node"));
transportService.sendRequest(node, PublicationTransportHandler.PUBLISH_STATE_ACTION_NAME,
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not the situation where we handle a join anymore but cluster state publishing?
The goal of this test was to make sure that join validation ensures that the node can understand the cluster state before being added to the cluster by the master.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, actually I did not get what the test is supposed to do.
Now it's clear that it should check failing JoinValidation.
I still think that ZenDiscoveryIT is a better place for this test than CoordinatorTests because there is no easy way to force the master node to send state to joining node, which deserialization will fail. However, to have it in ZenDiscoveryIT I had to extract one method in Coordinator and make it public for tests (similarly we were calling zenDiscovery.handleJoinRequest previously).
ae39525

@andrershov
Copy link
Contributor Author

@ywelsch I've made necessary, please have a second look.

@andrershov
Copy link
Contributor Author

run elasticsearch-ci/1

}

// public for tests
public void sendValidateJoinRequest(ClusterState stateForJoinValidation, JoinRequest joinRequest,
Copy link
Contributor

Choose a reason for hiding this comment

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

can you move this test to the same package as coordinator, and make this method package-visible instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

@andrershov andrershov merged commit 9e7fd8c into elastic:master Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. >test Issues or PRs that are addressing/adding tests v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants