Snapshot restore and index creates should keep index settings and cluster blocks in sync

Restoring an index from a snapshot or creating a new index can bring the index settings index.blocks.read_only, index.blocks.read, index.blocks.write and index.blocks.metadata out-of-sync with the corresponding cluster blocks.

Closes #13931

core/src/main/java/org/elasticsearch/cluster/block/ClusterBlocks.java

@@ -300,6 +300,15 @@ public class ClusterBlocks extends AbstractDiffable<ClusterBlocks> {
             return this;
         }
 
+        public Builder updateBlocks(IndexMetaData indexMetaData) {
+            removeIndexBlock(indexMetaData.index(), MetaDataIndexStateService.INDEX_CLOSED_BLOCK);
+            removeIndexBlock(indexMetaData.index(), IndexMetaData.INDEX_READ_ONLY_BLOCK);
+            removeIndexBlock(indexMetaData.index(), IndexMetaData.INDEX_READ_BLOCK);
+            removeIndexBlock(indexMetaData.index(), IndexMetaData.INDEX_WRITE_BLOCK);
+            removeIndexBlock(indexMetaData.index(), IndexMetaData.INDEX_METADATA_BLOCK);
+            return addBlocks(indexMetaData);
+        }
+
         public Builder addGlobalBlock(ClusterBlock block) {
             global.add(block);
             return this;

core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java

@@ -453,9 +453,7 @@ public class MetaDataCreateIndexService extends AbstractComponent {
                             blocks.addIndexBlock(request.index(), block);
                         }
                     }
-                    if (request.state() == State.CLOSE) {
+                    blocks.updateBlocks(indexMetaData);
-                        blocks.addIndexBlock(request.index(), MetaDataIndexStateService.INDEX_CLOSED_BLOCK);
-                    }
 
                     ClusterState updatedState = ClusterState.builder(currentState).blocks(blocks).metaData(newMetaData).build();
 

core/src/main/java/org/elasticsearch/snapshots/RestoreService.java

@@ -101,7 +101,6 @@ import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_VERSION_CREATED;
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_VERSION_MINIMUM_COMPATIBLE;
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_VERSION_UPGRADED;
-import static org.elasticsearch.cluster.metadata.MetaDataIndexStateService.INDEX_CLOSED_BLOCK;
 import static org.elasticsearch.common.util.set.Sets.newHashSet;
 
 /**
@@ -272,6 +271,7 @@ public class RestoreService extends AbstractComponent implements ClusterStateLis
                                     populateIgnoredShards(index, ignoreShards);
                                 }
                                 rtBuilder.addAsNewRestore(updatedIndexMetaData, restoreSource, ignoreShards);
+                                blocks.addBlocks(updatedIndexMetaData);
                                 mdBuilder.put(updatedIndexMetaData, true);
                             } else {
                                 validateExistingIndex(currentIndexMetaData, snapshotIndexMetaData, renamedIndex, partial);
@@ -295,9 +295,10 @@ public class RestoreService extends AbstractComponent implements ClusterStateLis
                                 indexMdBuilder.settings(Settings.settingsBuilder().put(snapshotIndexMetaData.settings()).put(IndexMetaData.SETTING_INDEX_UUID, currentIndexMetaData.indexUUID()));
                                 IndexMetaData updatedIndexMetaData = indexMdBuilder.index(renamedIndex).build();
                                 rtBuilder.addAsRestore(updatedIndexMetaData, restoreSource);
-                                blocks.removeIndexBlock(renamedIndex, INDEX_CLOSED_BLOCK);
+                                blocks.updateBlocks(updatedIndexMetaData);
                                 mdBuilder.put(updatedIndexMetaData, true);
                             }
+
                             for (int shard = 0; shard < snapshotIndexMetaData.getNumberOfShards(); shard++) {
                                 if (!ignoreShards.contains(shard)) {
                                     shardsBuilder.put(new ShardId(renamedIndex, shard), new RestoreInProgress.ShardRestoreStatus(clusterService.state().nodes().localNodeId()));

core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java

@@ -33,6 +33,7 @@ import org.junit.Test;
 import java.util.HashMap;
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertBlocked;
 import static org.hamcrest.Matchers.*;
 import static org.hamcrest.core.IsNull.notNullValue;
@@ -151,6 +152,13 @@ public class CreateIndexIT extends ESIntegTestCase {
         }
     }
 
+    @Test
+    public void testCreateIndexWithMetadataBlocks() {
+        assertAcked(prepareCreate("test").setSettings(Settings.builder().put(IndexMetaData.SETTING_BLOCKS_METADATA, true)));
+        assertBlocked(client().admin().indices().prepareGetSettings("test"), IndexMetaData.INDEX_METADATA_BLOCK);
+        disableIndexBlock("test", IndexMetaData.SETTING_BLOCKS_METADATA);
+    }
+
     @Test
     public void testInvalidShardCountSettingsWithoutPrefix() throws Exception {
         try {

core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java

@@ -44,6 +44,7 @@ import org.elasticsearch.cluster.SnapshotsInProgress;
 import org.elasticsearch.cluster.SnapshotsInProgress.Entry;
 import org.elasticsearch.cluster.SnapshotsInProgress.ShardSnapshotStatus;
 import org.elasticsearch.cluster.SnapshotsInProgress.State;
+import org.elasticsearch.cluster.block.ClusterBlocks;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.cluster.metadata.MappingMetaData;
 import org.elasticsearch.cluster.metadata.MetaDataIndexStateService;
@@ -1739,6 +1740,96 @@ public class SharedClusterSnapshotRestoreIT extends AbstractSnapshotIntegTestCas
 
     }
 
+    @Test
+    public void recreateBlocksOnRestoreTest() throws Exception {
+        Client client = client();
+
+        logger.info("-->  creating repository");
+        assertAcked(client.admin().cluster().preparePutRepository("test-repo")
+                .setType("fs").setSettings(Settings.settingsBuilder()
+                        .put("location", randomRepoPath())
+                        .put("compress", randomBoolean())
+                        .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES)));
+
+        Settings.Builder indexSettings = Settings.builder()
+                .put(indexSettings())
+                .put(SETTING_NUMBER_OF_REPLICAS, between(0, 1))
+                .put(INDEX_REFRESH_INTERVAL, "10s");
+
+        logger.info("--> create index");
+        assertAcked(prepareCreate("test-idx", 2, indexSettings));
+
+        try {
+            List<String> initialBlockSettings = randomSubsetOf(randomInt(3),
+                    IndexMetaData.SETTING_BLOCKS_WRITE, IndexMetaData.SETTING_BLOCKS_METADATA, IndexMetaData.SETTING_READ_ONLY);
+            Settings.Builder initialSettingsBuilder = Settings.builder();
+            for (String blockSetting : initialBlockSettings) {
+                initialSettingsBuilder.put(blockSetting, true);
+            }
+            Settings initialSettings = initialSettingsBuilder.build();
+            logger.info("--> using initial block settings {}", initialSettings.getAsMap());
+
+            if (!initialSettings.getAsMap().isEmpty()) {
+                logger.info("--> apply initial blocks to index");
+                client().admin().indices().prepareUpdateSettings("test-idx").setSettings(initialSettingsBuilder).get();
+            }
+
+            logger.info("--> snapshot index");
+            CreateSnapshotResponse createSnapshotResponse = client.admin().cluster().prepareCreateSnapshot("test-repo", "test-snap")
+                    .setWaitForCompletion(true).setIndices("test-idx").get();
+            assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), greaterThan(0));
+            assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), equalTo(createSnapshotResponse.getSnapshotInfo().totalShards()));
+
+            logger.info("--> remove blocks and delete index");
+            disableIndexBlock("test-idx", IndexMetaData.SETTING_BLOCKS_METADATA);
+            disableIndexBlock("test-idx", IndexMetaData.SETTING_READ_ONLY);
+            disableIndexBlock("test-idx", IndexMetaData.SETTING_BLOCKS_WRITE);
+            disableIndexBlock("test-idx", IndexMetaData.SETTING_BLOCKS_READ);
+            cluster().wipeIndices("test-idx");
+
+            logger.info("--> restore index with additional block changes");
+            List<String> changeBlockSettings = randomSubsetOf(randomInt(4),
+                    IndexMetaData.SETTING_BLOCKS_METADATA, IndexMetaData.SETTING_BLOCKS_WRITE,
+                    IndexMetaData.SETTING_READ_ONLY, IndexMetaData.SETTING_BLOCKS_READ);
+            Settings.Builder changedSettingsBuilder = Settings.builder();
+            for (String blockSetting : changeBlockSettings) {
+                changedSettingsBuilder.put(blockSetting, randomBoolean());
+            }
+            Settings changedSettings = changedSettingsBuilder.build();
+            logger.info("--> applying changed block settings {}", changedSettings.getAsMap());
+
+            RestoreSnapshotResponse restoreSnapshotResponse = client.admin().cluster()
+                    .prepareRestoreSnapshot("test-repo", "test-snap")
+                    .setIndexSettings(changedSettings)
+                    .setWaitForCompletion(true).execute().actionGet();
+            assertThat(restoreSnapshotResponse.getRestoreInfo().totalShards(), greaterThan(0));
+
+            ClusterBlocks blocks = client.admin().cluster().prepareState().clear().setBlocks(true).get().getState().blocks();
+            // compute current index settings (as we cannot query them if they contain SETTING_BLOCKS_METADATA)
+            Settings mergedSettings = Settings.builder()
+                    .put(initialSettings)
+                    .put(changedSettings)
+                    .build();
+            logger.info("--> merged block settings {}", mergedSettings.getAsMap());
+
+            logger.info("--> checking consistency between settings and blocks");
+            assertThat(mergedSettings.getAsBoolean(IndexMetaData.SETTING_BLOCKS_METADATA, false),
+                    is(blocks.hasIndexBlock("test-idx", IndexMetaData.INDEX_METADATA_BLOCK)));
+            assertThat(mergedSettings.getAsBoolean(IndexMetaData.SETTING_BLOCKS_READ, false),
+                    is(blocks.hasIndexBlock("test-idx", IndexMetaData.INDEX_READ_BLOCK)));
+            assertThat(mergedSettings.getAsBoolean(IndexMetaData.SETTING_BLOCKS_WRITE, false),
+                    is(blocks.hasIndexBlock("test-idx", IndexMetaData.INDEX_WRITE_BLOCK)));
+            assertThat(mergedSettings.getAsBoolean(IndexMetaData.SETTING_READ_ONLY, false),
+                    is(blocks.hasIndexBlock("test-idx", IndexMetaData.INDEX_READ_ONLY_BLOCK)));
+        } finally {
+            logger.info("--> cleaning up blocks");
+            disableIndexBlock("test-idx", IndexMetaData.SETTING_BLOCKS_METADATA);
+            disableIndexBlock("test-idx", IndexMetaData.SETTING_READ_ONLY);
+            disableIndexBlock("test-idx", IndexMetaData.SETTING_BLOCKS_WRITE);
+            disableIndexBlock("test-idx", IndexMetaData.SETTING_BLOCKS_READ);
+        }
+    }
+
     @Test
     public void deleteIndexDuringSnapshotTest() throws Exception {
         Client client = client();