No system index in MetadataIndexUpgradeService (elastic#67941)

This commit removes the system index specific upgrade logic in the
MetadataIndexUpgradeService as this is not guaranteed to fully do the
upgrade and another mechanism exists in the
SystemIndexMetadataUpgradeService. This removal allows for
simplification as we now have a single place to do this upgrade of the
metadata for system indices.

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

@@ -35,7 +35,6 @@
 import org.elasticsearch.index.analysis.NamedAnalyzer;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.similarity.SimilarityService;
-import org.elasticsearch.indices.SystemIndices;
 import org.elasticsearch.indices.mapper.MapperRegistry;
 import org.elasticsearch.script.ScriptService;
 
@@ -60,16 +59,14 @@ public class MetadataIndexUpgradeService {
     private final NamedXContentRegistry xContentRegistry;
     private final MapperRegistry mapperRegistry;
     private final IndexScopedSettings indexScopedSettings;
-    private final SystemIndices systemIndices;
     private final ScriptService scriptService;
 
     public MetadataIndexUpgradeService(Settings settings, NamedXContentRegistry xContentRegistry, MapperRegistry mapperRegistry,
-                                       IndexScopedSettings indexScopedSettings, SystemIndices systemIndices, ScriptService scriptService) {
+                                       IndexScopedSettings indexScopedSettings, ScriptService scriptService) {
         this.settings = settings;
         this.xContentRegistry = xContentRegistry;
         this.mapperRegistry = mapperRegistry;
         this.indexScopedSettings = indexScopedSettings;
-        this.systemIndices = systemIndices;
         this.scriptService = scriptService;
     }
 
@@ -84,17 +81,15 @@ public IndexMetadata upgradeIndexMetadata(IndexMetadata indexMetadata, Version m
         if (isUpgraded(indexMetadata)) {
             /*
              * We still need to check for broken index settings since it might be that a user removed a plugin that registers a setting
-             * needed by this index. Additionally, the system flag could have been lost during a rolling upgrade where the previous version
-             * did not know about the flag.
+             * needed by this index.
              */
-            return archiveBrokenIndexSettings(maybeMarkAsSystemIndex(indexMetadata));
+            return archiveBrokenIndexSettings(indexMetadata);
         }
         // Throws an exception if there are too-old segments:
         checkSupportedVersion(indexMetadata, minimumIndexCompatibilityVersion);
-        final IndexMetadata metadataWithSystemMarked = maybeMarkAsSystemIndex(indexMetadata);
         // we have to run this first otherwise in we try to create IndexSettings
         // with broken settings and fail in checkMappingsCompatibility
-        final IndexMetadata newMetadata = archiveBrokenIndexSettings(metadataWithSystemMarked);
+        final IndexMetadata newMetadata = archiveBrokenIndexSettings(indexMetadata);
         // only run the check with the upgraded settings!!
         checkMappingsCompatibility(newMetadata);
         return markAsUpgraded(newMetadata);
@@ -219,12 +214,4 @@ IndexMetadata archiveBrokenIndexSettings(IndexMetadata indexMetadata) {
             return indexMetadata;
         }
     }
-
-    IndexMetadata maybeMarkAsSystemIndex(IndexMetadata indexMetadata) {
-        final boolean isSystem = systemIndices.isSystemIndex(indexMetadata.getIndex());
-        if (isSystem != indexMetadata.isSystem()) {
-            return IndexMetadata.builder(indexMetadata).system(isSystem).build();
-        }
-        return indexMetadata;
-    }
 }

server/src/main/java/org/elasticsearch/node/Node.java

@@ -578,7 +578,7 @@ protected Node(final Environment initialEnvironment,
                     .collect(Collectors.toList());
             final MetadataUpgrader metadataUpgrader = new MetadataUpgrader(indexTemplateMetadataUpgraders);
             final MetadataIndexUpgradeService metadataIndexUpgradeService = new MetadataIndexUpgradeService(settings, xContentRegistry,
-                indicesModule.getMapperRegistry(), settingsModule.getIndexScopedSettings(), systemIndices, scriptService);
+                indicesModule.getMapperRegistry(), settingsModule.getIndexScopedSettings(), scriptService);
             if (DiscoveryNode.isMasterNode(settings)) {
                 clusterService.addListener(new SystemIndexMetadataUpgradeService(systemIndices, clusterService));
             }

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

@@ -22,16 +22,12 @@
 import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.settings.IndexScopedSettings;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.indices.SystemIndexDescriptor;
-import org.elasticsearch.indices.SystemIndices;
 import org.elasticsearch.indices.mapper.MapperRegistry;
 import org.elasticsearch.plugins.MapperPlugin;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.VersionUtils;
 
 import java.util.Collections;
-import java.util.List;
-import java.util.Map;
 
 import static org.hamcrest.Matchers.equalTo;
 
@@ -136,32 +132,12 @@ public void testFailUpgrade() {
         service.upgradeIndexMetadata(goodMeta, Version.CURRENT.minimumIndexCompatibilityVersion());
     }
 
-    public void testMaybeMarkAsSystemIndex() {
-        MetadataIndexUpgradeService service = getMetadataIndexUpgradeService();
-        IndexMetadata src = newIndexMeta("foo", Settings.EMPTY);
-        assertFalse(src.isSystem());
-        IndexMetadata indexMetadata = service.maybeMarkAsSystemIndex(src);
-        assertSame(indexMetadata, src);
-
-        src = newIndexMeta(".system", Settings.EMPTY);
-        assertFalse(src.isSystem());
-        indexMetadata = service.maybeMarkAsSystemIndex(src);
-        assertNotSame(indexMetadata, src);
-        assertTrue(indexMetadata.isSystem());
-
-        // test with the whole upgrade
-        assertFalse(src.isSystem());
-        indexMetadata = service.upgradeIndexMetadata(src, Version.CURRENT.minimumIndexCompatibilityVersion());
-        assertTrue(indexMetadata.isSystem());
-    }
-
     private MetadataIndexUpgradeService getMetadataIndexUpgradeService() {
         return new MetadataIndexUpgradeService(
             Settings.EMPTY,
             xContentRegistry(),
             new MapperRegistry(Collections.emptyMap(), Collections.emptyMap(), null, Collections.emptyMap(),
                 MapperPlugin.NOOP_FIELD_FILTER), IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
-            new SystemIndices(Map.of("system-plugin", List.of(new SystemIndexDescriptor(".system", "a system index")))),
             null
         );
     }

server/src/test/java/org/elasticsearch/gateway/GatewayMetaStateTests.java

@@ -159,7 +159,7 @@ private static class MockMetadataIndexUpgradeService extends MetadataIndexUpgrad
         private final boolean upgrade;
 
         MockMetadataIndexUpgradeService(boolean upgrade) {
-            super(Settings.EMPTY, null, null, null, null, null);
+            super(Settings.EMPTY, null, null, null, null);
             this.upgrade = upgrade;
         }
 

server/src/test/java/org/elasticsearch/indices/cluster/ClusterStateChanges.java

@@ -202,7 +202,6 @@ public ClusterStateChanges(NamedXContentRegistry xContentRegistry, ThreadPool th
             xContentRegistry,
             null,
             null,
-            null,
             null
         ) {
             // metadata upgrader should do nothing

server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java

@@ -1577,7 +1577,6 @@ clusterService, indicesService, threadPool, shardStateAction, mappingUpdatedActi
                         settings, namedXContentRegistry,
                         mapperRegistry,
                         indexScopedSettings,
-                        systemIndices,
                         null),
                     clusterSettings,
                         shardLimitValidator