Add deprecation message for setting master role

Signed-off-by: Tianli Feng <[email protected]>
opensearch-project · Mar 17, 2022 · f59a27b · f59a27b
1 parent 47961ce
commit f59a27b
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 8 deletions.
diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java
@@ -34,6 +34,7 @@
 
 import org.opensearch.LegacyESVersion;
 import org.opensearch.Version;
+import org.opensearch.common.logging.DeprecationLogger;
 import org.opensearch.common.settings.Setting;
 import org.opensearch.common.settings.Setting.Property;
 import org.opensearch.common.settings.Settings;
@@ -52,6 +53,10 @@
  */
 public abstract class DiscoveryNodeRole implements Comparable<DiscoveryNodeRole> {
 
+    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(DiscoveryNodeRole.class);
+    public static final String MASTER_ROLE_DEPRECATION_MESSAGE =
+        "[master] role is deprecated. To promote inclusive language, please use [cluster_manager] role instead.";
+
     private final String roleName;
 
     /**
@@ -214,6 +219,11 @@ public Setting<Boolean> legacySetting() {
             return Setting.boolSetting("node.master", false, Property.Deprecated, Property.NodeScope);
         }
 
+        @Override
+        public void validateRole(List<DiscoveryNodeRole> roles) {
+            deprecationLogger.deprecate("node_role_master", MASTER_ROLE_DEPRECATION_MESSAGE);
+        }
+
     };
 
     /**
@@ -242,16 +252,15 @@ public void validateRole(List<DiscoveryNodeRole> roles) {
                 throw new IllegalArgumentException(
                     String.format(
                         Locale.ROOT,
-                        "The two roles [%s, %s] can not be assigned together to a node. "
-                            + "To promote inclusive language, [%s] role is deprecated, and replaced by [%s] role.",
+                        "The two roles [%s, %s] can not be assigned together to a node. %s",
                         DiscoveryNodeRole.MASTER_ROLE.roleName(),
                         DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(),
-                        DiscoveryNodeRole.MASTER_ROLE.roleName(),
-                        DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName()
+                        MASTER_ROLE_DEPRECATION_MESSAGE
                     )
                 );
             }
         }
+
     };
 
     public static final DiscoveryNodeRole REMOTE_CLUSTER_CLIENT_ROLE = new DiscoveryNodeRole("remote_cluster_client", "r") {

diff --git a/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java b/server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java
@@ -15,7 +15,6 @@
 
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.List;
 
 import static org.hamcrest.Matchers.containsString;
 
@@ -38,8 +37,21 @@ public void testClusterManagerAndMasterRoleCanNotCoexist() {
      */
     public void testClusterManagerAndDataNodeRoles() {
         Settings roleSettings = Settings.builder().put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), "cluster_manager, data").build();
-        List<DiscoveryNodeRole> actualNodeRoles = NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings);
-        List<DiscoveryNodeRole> expectedNodeRoles = Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE);
-        assertEquals(expectedNodeRoles, actualNodeRoles);
+        assertEquals(
+            Arrays.asList(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE, DiscoveryNodeRole.DATA_ROLE),
+            NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings)
+        );
+    }
+
+    /**
+     * Validate setting master role will result a deprecation message.
+     * Remove the test after removing MASTER_ROLE.
+     */
+    public void testMasterRoleDeprecationMessage() {
+        // It's used to add MASTER_ROLE into 'roleMap', because MASTER_ROLE is removed from DiscoveryNodeRole.BUILT_IN_ROLES in 2.0.
+        DiscoveryNode.setAdditionalRoles(Collections.emptySet());
+        Settings roleSettings = Settings.builder().put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), "master").build();
+        assertEquals(Collections.singletonList(DiscoveryNodeRole.MASTER_ROLE), NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings));
+        assertWarnings(DiscoveryNodeRole.MASTER_ROLE_DEPRECATION_MESSAGE);
     }
 }