-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Replace internal usages of 'master' term in 'server/src/test' directory #2520
Changes from 2 commits
6b0ce71
36ebbcb
63e5728
47c5c0b
a130464
3747411
33ebf86
9751c84
8db5521
d6dda60
b4953ad
44c6122
bddb976
4f8cf38
a01fd4e
6b756ec
fa6e5ed
47cfbab
dbb085b
4f71700
50fcf3b
737843f
a87cede
37a0b17
51d5007
704f00c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,9 +112,9 @@ public void testClusterHealth() throws IOException { | |
assertThat(clusterHealth.getActiveShardsPercent(), is(allOf(greaterThanOrEqualTo(0.0), lessThanOrEqualTo(100.0)))); | ||
} | ||
|
||
public void testClusterHealthVerifyMasterNodeDiscovery() throws IOException { | ||
public void testClusterHealthVerifyClusterManagerNodeDiscovery() throws IOException { | ||
DiscoveryNode localNode = new DiscoveryNode("node", OpenSearchTestCase.buildNewFakeTransportAddress(), Version.CURRENT); | ||
// set the node information to verify master_node discovery in ClusterHealthResponse | ||
// set the node information to verify cluster_manager_node discovery in ClusterHealthResponse | ||
ClusterState clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)) | ||
.nodes(DiscoveryNodes.builder().add(localNode).localNodeId(localNode.getId()).masterNodeId(localNode.getId())) | ||
owaiskazi19 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.build(); | ||
|
@@ -150,7 +150,7 @@ private void assertClusterHealth(ClusterHealthResponse clusterHealth) { | |
} | ||
|
||
public void testVersionCompatibleSerialization() throws IOException { | ||
boolean hasDiscoveredMaster = false; | ||
boolean hasDiscoveredClusterManager = false; | ||
int indicesSize = randomInt(20); | ||
Map<String, ClusterIndexHealth> indices = new HashMap<>(indicesSize); | ||
if ("indices".equals(level) || "shards".equals(level)) { | ||
|
@@ -167,12 +167,12 @@ public void testVersionCompatibleSerialization() throws IOException { | |
randomInt(100), | ||
randomInt(100), | ||
randomInt(100), | ||
hasDiscoveredMaster, | ||
hasDiscoveredClusterManager, | ||
randomDoubleBetween(0d, 100d, true), | ||
randomFrom(ClusterHealthStatus.values()), | ||
indices | ||
); | ||
// Create the Cluster Health Response object with discovered master as false, | ||
// Create the Cluster Health Response object with discovered cluster manager as false, | ||
// to verify serialization puts default value for the field | ||
ClusterHealthResponse clusterHealth = new ClusterHealthResponse( | ||
"test-cluster", | ||
|
@@ -209,7 +209,7 @@ public void testVersionCompatibleSerialization() throws IOException { | |
StreamInput in_gt_7_0 = out_gt_1_0.bytes().streamInput(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we change the text of the comments above from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes; lets get the comments as we catch them in the PR review There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @owaiskazi19 Thanks a lot for your review! You are so careful. 😄 I will modify those |
||
in_gt_7_0.setVersion(new_version); | ||
clusterHealth = ClusterHealthResponse.readResponseFrom(in_gt_7_0); | ||
assertThat(clusterHealth.hasDiscoveredMaster(), Matchers.equalTo(hasDiscoveredMaster)); | ||
assertThat(clusterHealth.hasDiscoveredMaster(), Matchers.equalTo(hasDiscoveredClusterManager)); | ||
} | ||
|
||
ClusterHealthResponse maybeSerialize(ClusterHealthResponse clusterHealth) throws IOException { | ||
|
@@ -222,7 +222,7 @@ ClusterHealthResponse maybeSerialize(ClusterHealthResponse clusterHealth) throws | |
return clusterHealth; | ||
} | ||
|
||
public void testParseFromXContentWithDiscoveredMasterField() throws IOException { | ||
public void testParseFromXContentWithDiscoveredClusterManagerField() throws IOException { | ||
try ( | ||
XContentParser parser = JsonXContent.jsonXContent.createParser( | ||
NamedXContentRegistry.EMPTY, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -168,7 +168,7 @@ public void testUpdatesNodeWithNewRoles() throws Exception { | |
|
||
final JoinTaskExecutor joinTaskExecutor = new JoinTaskExecutor(Settings.EMPTY, allocationService, logger, rerouteService, null); | ||
|
||
final DiscoveryNode masterNode = new DiscoveryNode(UUIDs.base64UUID(), buildNewFakeTransportAddress(), Version.CURRENT); | ||
final DiscoveryNode clusterManagerNode = new DiscoveryNode(UUIDs.base64UUID(), buildNewFakeTransportAddress(), Version.CURRENT); | ||
|
||
final DiscoveryNode actualNode = new DiscoveryNode(UUIDs.base64UUID(), buildNewFakeTransportAddress(), Version.CURRENT); | ||
final DiscoveryNode bwcNode = new DiscoveryNode( | ||
|
@@ -183,7 +183,13 @@ public void testUpdatesNodeWithNewRoles() throws Exception { | |
actualNode.getVersion() | ||
); | ||
final ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT) | ||
.nodes(DiscoveryNodes.builder().add(masterNode).localNodeId(masterNode.getId()).masterNodeId(masterNode.getId()).add(bwcNode)) | ||
.nodes( | ||
DiscoveryNodes.builder() | ||
.add(clusterManagerNode) | ||
.localNodeId(clusterManagerNode.getId()) | ||
.masterNodeId(clusterManagerNode.getId()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. masterNodeId -> clusterManagerNodeId? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will rename |
||
.add(bwcNode) | ||
) | ||
.build(); | ||
|
||
final ClusterStateTaskExecutor.ClusterTasksResult<JoinTaskExecutor.Task> result = joinTaskExecutor.execute( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,12 +44,12 @@ | |
|
||
public class NoMasterBlockServiceTests extends OpenSearchTestCase { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. I changed the method name |
||
|
||
private NoMasterBlockService noMasterBlockService; | ||
private NoMasterBlockService noClusterManagerBlockService; | ||
private ClusterSettings clusterSettings; | ||
|
||
private void createService(Settings settings) { | ||
clusterSettings = new ClusterSettings(settings, BUILT_IN_CLUSTER_SETTINGS); | ||
noMasterBlockService = new NoMasterBlockService(settings, clusterSettings); | ||
noClusterManagerBlockService = new NoMasterBlockService(settings, clusterSettings); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Changed the variable name. 👍 |
||
} | ||
|
||
private void assertDeprecatedWarningEmitted() { | ||
|
@@ -61,22 +61,22 @@ private void assertDeprecatedWarningEmitted() { | |
|
||
public void testBlocksWritesByDefault() { | ||
createService(Settings.EMPTY); | ||
assertThat(noMasterBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_WRITES)); | ||
assertThat(noClusterManagerBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_WRITES)); | ||
owaiskazi19 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
public void testBlocksWritesIfConfiguredBySetting() { | ||
createService(Settings.builder().put(NO_CLUSTER_MANAGER_BLOCK_SETTING.getKey(), "write").build()); | ||
assertThat(noMasterBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_WRITES)); | ||
assertThat(noClusterManagerBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_WRITES)); | ||
} | ||
|
||
public void testBlocksAllIfConfiguredBySetting() { | ||
createService(Settings.builder().put(NO_CLUSTER_MANAGER_BLOCK_SETTING.getKey(), "all").build()); | ||
assertThat(noMasterBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_ALL)); | ||
assertThat(noClusterManagerBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_ALL)); | ||
} | ||
|
||
public void testBlocksMetadataWritesIfConfiguredBySetting() { | ||
createService(Settings.builder().put(NO_CLUSTER_MANAGER_BLOCK_SETTING.getKey(), "metadata_write").build()); | ||
assertThat(noMasterBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_METADATA_WRITES)); | ||
assertThat(noClusterManagerBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_METADATA_WRITES)); | ||
} | ||
|
||
public void testRejectsInvalidSetting() { | ||
|
@@ -88,12 +88,12 @@ public void testRejectsInvalidSetting() { | |
|
||
public void testSettingCanBeUpdated() { | ||
createService(Settings.builder().put(NO_CLUSTER_MANAGER_BLOCK_SETTING.getKey(), "all").build()); | ||
assertThat(noMasterBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_ALL)); | ||
assertThat(noClusterManagerBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_ALL)); | ||
|
||
clusterSettings.applySettings(Settings.builder().put(NO_CLUSTER_MANAGER_BLOCK_SETTING.getKey(), "write").build()); | ||
assertThat(noMasterBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_WRITES)); | ||
assertThat(noClusterManagerBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_WRITES)); | ||
|
||
clusterSettings.applySettings(Settings.builder().put(NO_CLUSTER_MANAGER_BLOCK_SETTING.getKey(), "metadata_write").build()); | ||
assertThat(noMasterBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_METADATA_WRITES)); | ||
assertThat(noClusterManagerBlockService.getNoMasterBlock(), sameInstance(NO_MASTER_BLOCK_METADATA_WRITES)); | ||
} | ||
} |
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.
discovered_master
->discovered_cluster_manager
hereThere 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 will address it in separate PR, and will paste the link later.
discovered_master
is still a existing field in cluster health API, so it will not be renamed directly. I will add additional tests for 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.
Created new PR #3432 to rename
discovered_master
in the unit test.