Check reserved state in Metadata.isGlobalStateEquals

docs/changelog/92124.yaml

@@ -0,0 +1,5 @@
+pr: 92124
+summary: Check reserved state in Metadata.isGlobalStateEquals
+area: Infra/Core
+type: bug
+issues: []

server/src/internalClusterTest/java/org/elasticsearch/reservedstate/service/FileSettingsServiceIT.java

@@ -186,6 +186,38 @@ public void testSettingsAppliedOnStart() throws Exception {
         assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2());
     }
 
+    public void testReservedStatePersistsOnRestart() throws Exception {
+        internalCluster().setBootstrapMasterNodeIndex(0);
+        logger.info("--> start master node");
+        final String masterNode = internalCluster().startMasterOnlyNode();
+        assertMasterNode(internalCluster().masterClient(), masterNode);
+        var savedClusterState = setupClusterStateListener(masterNode);
+
+        FileSettingsService masterFileSettingsService = internalCluster().getInstance(FileSettingsService.class, masterNode);
+
+        assertTrue(masterFileSettingsService.watching());
+
+        logger.info("--> write some settings");
+        writeJSONFile(masterNode, testJSON);
+        assertClusterStateSaveOK(savedClusterState.v1(), savedClusterState.v2());
+
+        logger.info("--> restart master");
+        internalCluster().restartNode(masterNode);
+
+        final ClusterStateResponse clusterStateResponse = client().admin().cluster().state(new ClusterStateRequest()).actionGet();
+        assertEquals(
+            1,
+            clusterStateResponse.getState()
+                .metadata()
+                .reservedStateMetadata()
+                .get(FileSettingsService.NAMESPACE)
+                .handlers()
+                .get(ReservedClusterSettingsAction.NAME)
+                .keys()
+                .size()
+        );
+    }
+
     private Tuple<CountDownLatch, AtomicLong> setupClusterStateListenerForError(String node) {
         ClusterService clusterService = internalCluster().clusterService(node);
         CountDownLatch savedClusterState = new CountDownLatch(1);

server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java

@@ -1299,6 +1299,9 @@ public static boolean isGlobalStateEquals(Metadata metadata1, Metadata metadata2
         if (customCount1 != customCount2) {
             return false;
         }
+        if (Objects.equals(metadata1.reservedStateMetadata, metadata2.reservedStateMetadata) == false) {
+            return false;
+        }
         return true;
     }
 

server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java

@@ -29,6 +29,7 @@
 import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.core.Nullable;
+import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.index.IndexSettings;
@@ -44,6 +45,7 @@
 import org.elasticsearch.xcontent.json.JsonXContent;
 
 import java.io.IOException;
+import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -2312,6 +2314,60 @@ public void testChunkedToXContent() throws IOException {
         );
     }
 
+    /**
+     * With this test we ensure that we consider whether a new field added to Metadata should be checked
+     * in Metadata.isGlobalStateEquals. We force the instance fields to be either in the checked list
+     * or in the excluded list.
+     * <p>
+     * This prevents from accidentally forgetting that a new field should be checked in isGlobalStateEquals.
+     */
+    @SuppressForbidden(reason = "need access to all fields, they are mostly private")
+    public void testEnsureMetadataFieldCheckedForGlobalStateChanges() {
+        Set<String> checkedForGlobalStateChanges = Set.of(
+            "coordinationMetadata",
+            "persistentSettings",
+            "hashesOfConsistentSettings",
+            "templates",
+            "clusterUUID",
+            "clusterUUIDCommitted",
+            "customs",
+            "reservedStateMetadata"
+        );
+        Set<String> excludedFromGlobalStateCheck = Set.of(
+            "version",
+            "transientSettings",
+            "settings",
+            "indices",
+            "aliasedIndices",
+            "totalNumberOfShards",
+            "totalOpenIndexShards",
+            "allIndices",
+            "visibleIndices",
+            "allOpenIndices",
+            "visibleOpenIndices",
+            "allClosedIndices",
+            "visibleClosedIndices",
+            "indicesLookup",
+            "mappingsByHash",
+            "oldestIndexVersion"
+        );
+
+        var diff = new HashSet<>(checkedForGlobalStateChanges);
+        diff.removeAll(excludedFromGlobalStateCheck);
+
+        // sanity check that the two field sets are mutually exclusive
+        assertEquals(checkedForGlobalStateChanges, diff);
+
+        // any declared non-static field in metadata must be either in the list of fields
+        // we check for global state changes, or in the fields excluded from the global state check.
+        var unclassifiedFields = Arrays.stream(Metadata.class.getDeclaredFields())
+            .filter(f -> Modifier.isStatic(f.getModifiers()) == false)
+            .map(f -> f.getName())
+            .filter(n -> (checkedForGlobalStateChanges.contains(n) || excludedFromGlobalStateCheck.contains(n)) == false)
+            .collect(Collectors.toSet());
+        assertThat(unclassifiedFields, empty());
+    }
+
     public static Metadata randomMetadata() {
         return randomMetadata(1);
     }