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

Remove 'cluster_manager' role attachment when using 'node.master' deprecated setting #6331

Merged
merged 12 commits into from
Mar 28, 2023
+165 −28
Merged

Remove 'cluster_manager' role attachment when using 'node.master' deprecated setting #6331

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c82bc5f
Allow only cluster_manager role when leagcy setting node.master is pr…
sandeshkr419 Mar 2, 2023
c8dff61
test modificaiton
sandeshkr419 Mar 9, 2023
7b93d8c
Add Locale.Root in String format()
sandeshkr419 Mar 9, 2023
e9bad81
./gradlew :server:spotlessApply code formatting
sandeshkr419 Mar 9, 2023
ec35774
modify settings filtering by using getRolesFromSettings()
sandeshkr419 Mar 23, 2023
ca821e0
Edit Changelog
sandeshkr419 Mar 23, 2023
a94f6e1
Add test with deprecated master and cluster manager role
sandeshkr419 Mar 24, 2023
53d9d2e
Fix role filtering from legacy settings
sandeshkr419 Mar 24, 2023
a8fc151
Override isEnabledByDefault() for cluster manager role
sandeshkr419 Mar 24, 2023
b7cda75
minor refactoring with boolean condition
sandeshkr419 Mar 28, 2023
6dffd09
Remove test flakiness
sandeshkr419 Mar 28, 2023
35327e1
Test reduce flakiness
sandeshkr419 Mar 28, 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
Prev Previous commit
Next Next commit
test modificaiton 
Signed-off-by: Sandesh Kumar <sandeshkr419@gmail.com>
  • Loading branch information
@sandeshkr419
sandeshkr419 committed Mar 28, 2023
commit c8dff611b86e07309be5914d418338fe91536aa9
22 changes: 13 additions & 9 deletions ...rc/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,11 +334,11 @@ public void testNodeRolesWithMasterLegacySettings() throws ExecutionException, I

public void testNodeRolesWithClusterManagerRole() throws ExecutionException, InterruptedException {
int total = 1;
Settings legacyMasterSettings = Settings.builder()
Settings clusterManagerNodeRoleSettings = Settings.builder()
.put("node.roles", String.format("%s, %s", DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName()))
.build();

internalCluster().startNodes(legacyMasterSettings);
internalCluster().startNodes(clusterManagerNodeRoleSettings);
waitForNodes(total);

Map<String, Integer> expectedCounts = getExpectedCounts(0, 1, 1, 0, 1, 0, 0);
Expand All @@ -352,12 +352,12 @@ public void testNodeRolesWithClusterManagerRole() throws ExecutionException, Int

public void testNodeRolesWithSeedDataNodeLegacySettings() throws ExecutionException, InterruptedException {
int total = 1;
Settings legacyDataNodeSettings = Settings.builder()
Settings legacySeedDataNodeSettings = Settings.builder()
.put("node.master", true)
.put("node.data", true)
.put("node.ingest", false).build();

internalCluster().startNodes(legacyDataNodeSettings);
internalCluster().startNodes(legacySeedDataNodeSettings);
waitForNodes(total);

Map<String, Integer> expectedRoleCounts = getExpectedCounts(1, 1, 1, 0, 1, 0, 0);
Expand All @@ -370,13 +370,15 @@ public void testNodeRolesWithSeedDataNodeLegacySettings() throws ExecutionExcept
assertEquals(expectedRoles, getNodeRoles(0));
}

public void testNodeRolesWithLegacyDataNodeSettings() throws ExecutionException, InterruptedException {
int total = 1;
public void testNodeRolesWithDataNodeLegacySettings() throws ExecutionException, InterruptedException {
int total = 2;
Settings legacyDataNodeSettings = Settings.builder()
.put("node.master", false)
.put("node.data", true)
.put("node.ingest", false).build();

// can't start data-only node without assigning cluster-manager
internalCluster().startClusterManagerOnlyNodes(1);
internalCluster().startNodes(legacyDataNodeSettings);
waitForNodes(total);

Expand All @@ -385,9 +387,11 @@ public void testNodeRolesWithLegacyDataNodeSettings() throws ExecutionException
ClusterStatsResponse clusterStatsResponse = client().admin().cluster().prepareClusterStats().get();
assertCounts(clusterStatsResponse.getNodesStats().getCounts(), total, expectedRoleCounts);

Set<String> expectedRoles = Set.of(DiscoveryNodeRole.MASTER_ROLE.roleName(),
DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName(), DiscoveryNodeRole.DATA_ROLE.roleName());
assertEquals(expectedRoles, getNodeRoles(0));
Set<String> expectedClusterManagerOnlyRoles = Set.of(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName());
assertEquals(expectedClusterManagerOnlyRoles, getNodeRoles(0));

Set<String> expectedDataOnlyNodeRoles = Set.of(DiscoveryNodeRole.DATA_ROLE.roleName(), DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName());
assertEquals(expectedDataOnlyNodeRoles, getNodeRoles(1));

}
private Map<String, Integer> getExpectedCounts(int dataRoleCount, int masterRoleCount, int clusterManagerRoleCount, int ingestRoleCount,
Copy link
Member

Choose a reason for hiding this comment

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

It looks like searchRoleCount and coordinatingOnlyCount are always zero, so you can omit them from the method signature and just set them to zero in the method body.

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 true for the existing tests. But keeping the method definition for future test cases instead of hardcoding the values. Adding those test-cases in this CR will be out of scope for these changes.

Expand Down