Remove 'node.master' legacy setting from 'cluster_manager' role

Signed-off-by: Sandesh Kumar <[email protected]>
opensearch-project · Mar 2, 2023 · ec76209 · ec76209
1 parent 74b94ca
commit ec76209
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 15 deletions.
diff --git a/...rc/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java b/...rc/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java
@@ -339,11 +339,11 @@ public void testNodeCountsWithLegacyMasterSetting() throws ExecutionException, I
         ClusterStatsResponse clusterStatsResponse = client().admin().cluster().prepareClusterStats().get();
         assertCounts(clusterStatsResponse.getNodesStats().getCounts(), total, expectedCounts);
 
-        Set<String> expectedRoles = Set.of(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName());
+        Set<String> expectedRoles = Set.of(DiscoveryNodeRole.MASTER_ROLE.roleName(), DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName());
         assertEquals(expectedRoles, getNodeRoles(0));
     }
 
-    public void testNodeCountsWithLegacyDataNodeSetting() throws ExecutionException, InterruptedException {
+    public void testNodeCountsWithLegacySeedDataNodeSetting() throws ExecutionException, InterruptedException {
         int total = 1;
         Settings legacyDataNodeSettings = Settings.builder()
             .put("node.master", true)
@@ -358,11 +358,31 @@ public void testNodeCountsWithLegacyDataNodeSetting() throws ExecutionException,
         ClusterStatsResponse clusterStatsResponse = client().admin().cluster().prepareClusterStats().get();
         assertCounts(clusterStatsResponse.getNodesStats().getCounts(), total, expectedRoleCounts);
 
-        Set<String> expectedRoles = Set.of(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(),
+        Set<String> expectedRoles = Set.of(DiscoveryNodeRole.MASTER_ROLE.roleName(),
             DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName(), DiscoveryNodeRole.DATA_ROLE.roleName());
         assertEquals(expectedRoles, getNodeRoles(0));
     }
 
+    public void testNodeCountsWithLegacyDataNodeSetting() throws  ExecutionException, InterruptedException {
+        int total = 1;
+        Settings legacyDataNodeSettings = Settings.builder()
+            .put("node.master", false)
+            .put("node.data", true)
+            .put("node.ingest", false).build();
+
+        internalCluster().startNodes(legacyDataNodeSettings);
+        waitForNodes(total);
+
+        Map<String, Integer> expectedRoleCounts = getExpectedCounts(1, 1, 1, 0, 1, 0, 0);
+
+        ClusterStatsResponse clusterStatsResponse = client().admin().cluster().prepareClusterStats().get();
+        assertCounts(clusterStatsResponse.getNodesStats().getCounts(), total, expectedRoleCounts);
+
+        Set<String> expectedRoles = Set.of(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(),
+            DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName(), DiscoveryNodeRole.DATA_ROLE.roleName());
+        assertEquals(expectedRoles, getNodeRoles(0));
+
+    }
     private Map<String, Integer> getExpectedCounts(int dataRoleCount, int masterRoleCount, int clusterManagerRoleCount, int ingestRoleCount,
                                                    int remoteClusterClientRoleCount, int searchRoleCount, int coordinatingOnlyCount) {
         Map<String, Integer> expectedCounts = new HashMap<>();
@@ -380,6 +400,4 @@ private Set<String> getNodeRoles(int nodeNumber) throws ExecutionException, Inte
         NodesStatsResponse nodesStatsResponse = client().admin().cluster().nodesStats(new NodesStatsRequest()).get();
         return nodesStatsResponse.getNodes().get(nodeNumber).getNode().getRoles().stream().map(DiscoveryNodeRole::roleName).collect(Collectors.toSet());
     }
-
-    //TODO: Add multi-node cluster tests
 }
diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java
@@ -283,16 +283,10 @@ public static Set<DiscoveryNodeRole> getRolesFromSettings(final Settings setting
             validateLegacySettings(settings, roleMap);
             return Collections.unmodifiableSet(new HashSet<>(NODE_ROLES_SETTING.get(settings)));
         } else {
-            Map<String, DiscoveryNodeRole> rolesFromSettingsMap = roleMap.values()
+            return roleMap.values()
                 .stream()
                 .filter(s -> s.legacySetting() != null && s.legacySetting().get(settings))
-                .collect(Collectors.toMap(DiscoveryNodeRole::roleName, Function.identity()));
-            // Legacy Setting - "node.master": true - adds both "cluster_manager" & deprecated "master" to node roles
-            // because roleMap contains "master" as known role - so explicitly removing deprecated "master" role
-            if (rolesFromSettingsMap.keySet().containsAll(Set.of(DiscoveryNodeRole.MASTER_ROLE.roleName(), DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName()))) {
-                rolesFromSettingsMap.remove(DiscoveryNodeRole.MASTER_ROLE.roleName());
-            }
-            return new HashSet<>(rolesFromSettingsMap.values());
+                .collect(Collectors.toSet());
         }
     }
 

diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java
@@ -245,8 +245,8 @@ public void validateRole(List<DiscoveryNodeRole> roles) {
 
         @Override
         public Setting<Boolean> legacySetting() {
-            // copy the setting here so we can mark it private in org.opensearch.node.Node
-            return Setting.boolSetting("node.master", true, Property.Deprecated, Property.NodeScope);
+            // 'cluster_manager' role should not configure legacy setting since deprecated 'master' role is supported till OS 2.x
+            return null;
         }
 
         @Override