Allow only cluster_manager role when leagcy setting node.master is pr…

…ovided Signed-off-by: Sandesh Kumar <[email protected]>
opensearch-project · Mar 2, 2023 · e16a4f7 · e16a4f7
1 parent 415b06c
commit e16a4f7
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 3 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
@@ -35,6 +35,7 @@
 import org.opensearch.Version;
 import org.opensearch.action.admin.cluster.health.ClusterHealthResponse;
 import org.opensearch.action.admin.cluster.node.stats.NodeStats;
+import org.opensearch.action.admin.cluster.node.stats.NodesStatsRequest;
 import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse;
 import org.opensearch.client.Requests;
 import org.opensearch.cluster.health.ClusterHealthStatus;
@@ -322,4 +323,63 @@ public void testFieldTypes() {
             }
         }
     }
+
+    public void testNodeCountsWithLegacyMasterSetting() throws ExecutionException, InterruptedException {
+        int total = 1;
+        Settings legacyMasterSettings = Settings.builder()
+            .put("node.master", true)
+            .put("node.data", false)
+            .put("node.ingest", false).build();
+
+        internalCluster().startNodes(legacyMasterSettings);
+        waitForNodes(total);
+
+        Map<String, Integer> expectedCounts = getExpectedCounts(0, 1, 1, 0, 1, 0, 0);
+
+        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());
+        assertEquals(expectedRoles, getNodeRoles(0));
+    }
+
+    public void testNodeCountsWithLegacyDataNodeSetting() throws ExecutionException, InterruptedException {
+        int total = 1;
+        Settings legacyDataNodeSettings = Settings.builder()
+            .put("node.master", true)
+            .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<>();
+        expectedCounts.put(DiscoveryNodeRole.DATA_ROLE.roleName(), dataRoleCount);
+        expectedCounts.put(DiscoveryNodeRole.MASTER_ROLE.roleName(), masterRoleCount);
+        expectedCounts.put(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), clusterManagerRoleCount);
+        expectedCounts.put(DiscoveryNodeRole.INGEST_ROLE.roleName(), ingestRoleCount);
+        expectedCounts.put(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName(), remoteClusterClientRoleCount);
+        expectedCounts.put(DiscoveryNodeRole.SEARCH_ROLE.roleName(), searchRoleCount);
+        expectedCounts.put(ClusterStatsNodes.Counts.COORDINATING_ONLY, coordinatingOnlyCount);
+        return expectedCounts;
+    }
+
+    private Set<String> getNodeRoles(int nodeNumber) throws ExecutionException, InterruptedException {
+        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,10 +283,16 @@ public static Set<DiscoveryNodeRole> getRolesFromSettings(final Settings setting
             validateLegacySettings(settings, roleMap);
             return Collections.unmodifiableSet(new HashSet<>(NODE_ROLES_SETTING.get(settings)));
         } else {
-            return roleMap.values()
+            Map<String, DiscoveryNodeRole> rolesFromSettingsMap = roleMap.values()
                 .stream()
-                .filter(s -> s.legacySetting() != null && s.legacySetting().get(settings))
-                .collect(Collectors.toSet());
+                .filter(s -> s.legacySetting() != null && s.legacySetting().get(settings) && !s.roleName().equals(DiscoveryNodeRole.MASTER_ROLE.roleName()))
+                .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());
         }
     }