Ensure that the output of node roles are sorted

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.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,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.SortedSet;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -85,7 +87,7 @@ public static boolean isRemoteClusterClient(final Settings settings) {
     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 +194,7 @@ public DiscoveryNode(String nodeName, String nodeId, String ephemeralId, String
             return success;
         };
         assert predicate.test(attributes) : attributes;
-        this.roles = Set.copyOf(roles);
+        this.roles = roles.stream().collect(Sets.toUnmodifiableSortedSet());
     }
 
     /** Creates a DiscoveryNode representing the local node. */
@@ -259,7 +261,7 @@ public DiscoveryNode(StreamInput in) throws IOException {
                 }
             }
         }
-        this.roles = Set.copyOf(roles);
+        this.roles = roles.stream().collect(Sets.toUnmodifiableSortedSet());
         this.version = Version.readVersion(in);
     }
 
@@ -370,8 +372,11 @@ public boolean isRemoteClusterClient() {
     }
 
     /**
-     * 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,15 +20,17 @@
 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.Objects;
 import java.util.Set;
+import java.util.SortedSet;
 
 /**
  * Represents a node role.
  */
-public abstract class DiscoveryNodeRole {
+public abstract class DiscoveryNodeRole implements Comparable<DiscoveryNodeRole> {
 
     private final String roleName;
 
@@ -89,6 +91,11 @@ public final int hashCode() {
         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{" +
@@ -146,9 +153,11 @@ protected Setting<Boolean> roleSetting() {
     /**
      * The built-in node roles.
      */
-    public static Set<DiscoveryNodeRole> BUILT_IN_ROLES = Set.of(DATA_ROLE, INGEST_ROLE, MASTER_ROLE, REMOTE_CLUSTER_CLIENT_ROLE);
+    public static SortedSet<DiscoveryNodeRole> BUILT_IN_ROLES =
+        Set.of(DATA_ROLE, INGEST_ROLE, MASTER_ROLE, REMOTE_CLUSTER_CLIENT_ROLE).stream().collect(Sets.toUnmodifiableSortedSet());
 
-    static Set<DiscoveryNodeRole> LEGACY_ROLES = Set.of(DATA_ROLE, INGEST_ROLE, MASTER_ROLE);
+    static SortedSet<DiscoveryNodeRole> LEGACY_ROLES =
+        Set.of(DATA_ROLE, INGEST_ROLE, MASTER_ROLE).stream().collect(Sets.toUnmodifiableSortedSet());
 
     /**
      * 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 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});