Ensure that the output of node roles are sorted (#54376)

This commit ensures that node roles are sorted by node role name, which
makes the output easier to consume, and also makes it easier to rely on
the behavior of the output in assertions.

rest-api-spec/src/main/resources/rest-api-spec/test/nodes.info/10_basic.yml

@@ -10,4 +10,23 @@ setup:
 
   - is_true: nodes
   - is_true: cluster_name
+
+---
+"node_info role test":
+  - skip:
+      version: " - 7.99.99"
+      reason: "node roles were not sorted before 8.0.0"
+      features: [no_xpack]
+
+  - do:
+      nodes.info: {}
+  - set:
+      nodes._arbitrary_key_: node_id
+
   - is_true: nodes.$node_id.roles
+  # the roles output is sorted
+  - match: { nodes.$node_id.roles.0: "data" }
+  - match: { nodes.$node_id.roles.1: "ingest" }
+  - match: { nodes.$node_id.roles.2: "master" }
+  - match: { nodes.$node_id.roles.3: "remote_cluster_client" }
+

server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java

@@ -26,6 +26,7 @@ import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.TransportAddress;
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.node.Node;
@@ -37,6 +38,8 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -85,7 +88,7 @@ public class DiscoveryNode implements Writeable, ToXContentFragment {
     private final TransportAddress address;
     private final Map<String, String> attributes;
     private final Version version;
-    private final Set<DiscoveryNodeRole> roles;
+    private final SortedSet<DiscoveryNodeRole> roles;
 
     /**
      * Creates a new {@link DiscoveryNode}
@@ -192,7 +195,7 @@ public class DiscoveryNode implements Writeable, ToXContentFragment {
             return success;
         };
         assert predicate.test(attributes) : attributes;
-        this.roles = Collections.unmodifiableSet(new HashSet<>(roles));
+        this.roles = Collections.unmodifiableSortedSet(new TreeSet<>(roles));
     }
 
     /** Creates a DiscoveryNode representing the local node. */
@@ -260,7 +263,7 @@ public class DiscoveryNode implements Writeable, ToXContentFragment {
                 }
             }
         }
-        this.roles = Collections.unmodifiableSet(new HashSet<>(roles));
+        this.roles = Collections.unmodifiableSortedSet(new TreeSet<>(roles));
         this.version = Version.readVersion(in);
     }
 
@@ -371,8 +374,11 @@ public class DiscoveryNode implements Writeable, ToXContentFragment {
     }
 
     /**
-     * Returns a set of all the roles that the node fulfills.
-     * If the node doesn't have any specific role, the set is returned empty, which means that the node is a coordinating only node.
+     * Returns a set of all the roles that the node has. The roles are returned in sorted order by the role name.
+     * <p>
+     * If a node does not have any specific role, the returned set is empty, which means that the node is a coordinating-only node.
+     *
+     * @return the sorted set of roles
      */
     public Set<DiscoveryNodeRole> getRoles() {
         return roles;

server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeRole.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.cluster.node;
 
 import org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.node.Node;
 
 import java.util.Arrays;
@@ -27,11 +28,13 @@ import java.util.Collections;
 import java.util.HashSet;
 import java.util.Objects;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
 
 /**
  * Represents a node role.
  */
-public abstract class DiscoveryNodeRole {
+public abstract class DiscoveryNodeRole implements Comparable<DiscoveryNodeRole> {
 
     private final String roleName;
 
@@ -92,6 +95,11 @@ public abstract class DiscoveryNodeRole {
         return Objects.hash(isKnownRole, roleName(), roleNameAbbreviation());
     }
 
+    @Override
+    public final int compareTo(final DiscoveryNodeRole o) {
+        return roleName.compareTo(o.roleName);
+    }
+
     @Override
     public final String toString() {
         return "DiscoveryNodeRole{" +
@@ -149,11 +157,11 @@ public abstract class DiscoveryNodeRole {
     /**
      * The built-in node roles.
      */
-    public static Set<DiscoveryNodeRole> BUILT_IN_ROLES =
-            Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DATA_ROLE, INGEST_ROLE, MASTER_ROLE, REMOTE_CLUSTER_CLIENT_ROLE)));
+    public static SortedSet<DiscoveryNodeRole> BUILT_IN_ROLES =
+            Collections.unmodifiableSortedSet(new TreeSet<>(Arrays.asList(DATA_ROLE, INGEST_ROLE, MASTER_ROLE, REMOTE_CLUSTER_CLIENT_ROLE)));
 
-    static Set<DiscoveryNodeRole> LEGACY_ROLES =
-        Collections.unmodifiableSet(new HashSet<>(Arrays.asList(DATA_ROLE, INGEST_ROLE, MASTER_ROLE)));
+    static SortedSet<DiscoveryNodeRole> LEGACY_ROLES =
+        Collections.unmodifiableSortedSet(new TreeSet<>(Arrays.asList(DATA_ROLE, INGEST_ROLE, MASTER_ROLE)));
 
     /**
      * Represents an unknown role. This can occur if a newer version adds a role that an older version does not know about, or a newer

server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeTests.java

@@ -34,9 +34,30 @@ import java.util.Set;
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 
 public class DiscoveryNodeTests extends ESTestCase {
 
+    public void testRolesAreSorted() {
+        final Set<DiscoveryNodeRole> roles = new HashSet<>(randomSubsetOf(DiscoveryNodeRole.BUILT_IN_ROLES));
+        final DiscoveryNode node = new DiscoveryNode(
+            "name",
+            "id",
+            new TransportAddress(TransportAddress.META_ADDRESS, 9200),
+            emptyMap(),
+            roles,
+            Version.CURRENT
+        );
+        DiscoveryNodeRole previous = null;
+        for (final DiscoveryNodeRole current : node.getRoles()) {
+            if (previous != null) {
+                assertThat(current, greaterThanOrEqualTo(previous));
+            }
+            previous = current;
+        }
+
+    }
+
     public void testDiscoveryNodeIsCreatedWithHostFromInetAddress() throws Exception {
         InetAddress inetAddress = randomBoolean() ? InetAddress.getByName("192.0.2.1") :
             InetAddress.getByAddress("name1", new byte[] { (byte) 192, (byte) 168, (byte) 0, (byte) 1});