diff --git a/x-pack/plugin/migrate/src/internalClusterTest/java/org/elasticsearch/xpack/migrate/action/ReindexDatastreamIndexTransportActionIT.java b/x-pack/plugin/migrate/src/internalClusterTest/java/org/elasticsearch/xpack/migrate/action/ReindexDatastreamIndexTransportActionIT.java index 46af8ab2fb4c2..32e31929fc2c1 100644 --- a/x-pack/plugin/migrate/src/internalClusterTest/java/org/elasticsearch/xpack/migrate/action/ReindexDatastreamIndexTransportActionIT.java +++ b/x-pack/plugin/migrate/src/internalClusterTest/java/org/elasticsearch/xpack/migrate/action/ReindexDatastreamIndexTransportActionIT.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.migrate.action; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; import org.elasticsearch.action.admin.indices.get.GetIndexRequest; @@ -223,16 +224,46 @@ public void testMappingsAddedToDestIndex() throws Exception { assertEquals("text", XContentMapValues.extractValue("properties.foo1.type", destMappings)); } - public void testReadOnlyAddedBack() { + public void testFailIfMetadataBlockSet() { + assumeTrue("requires the migration reindex feature flag", REINDEX_DATA_STREAM_FEATURE_FLAG.isEnabled()); + + var sourceIndex = randomAlphaOfLength(20).toLowerCase(Locale.ROOT); + var settings = Settings.builder().put(IndexMetadata.SETTING_BLOCKS_METADATA, true).build(); + indicesAdmin().create(new CreateIndexRequest(sourceIndex, settings)).actionGet(); + + try { + client().execute(ReindexDataStreamIndexAction.INSTANCE, new ReindexDataStreamIndexAction.Request(sourceIndex)).actionGet(); + } catch (ElasticsearchException e) { + assertTrue(e.getMessage().contains("Cannot reindex index") || e.getCause().getMessage().equals("Cannot reindex index")); + } + + cleanupMetadataBlocks(sourceIndex); + } + + public void testFailIfReadBlockSet() { + assumeTrue("requires the migration reindex feature flag", REINDEX_DATA_STREAM_FEATURE_FLAG.isEnabled()); + + var sourceIndex = randomAlphaOfLength(20).toLowerCase(Locale.ROOT); + var settings = Settings.builder().put(IndexMetadata.SETTING_BLOCKS_READ, true).build(); + indicesAdmin().create(new CreateIndexRequest(sourceIndex, settings)).actionGet(); + + try { + client().execute(ReindexDataStreamIndexAction.INSTANCE, new ReindexDataStreamIndexAction.Request(sourceIndex)).actionGet(); + } catch (ElasticsearchException e) { + assertTrue(e.getMessage().contains("Cannot reindex index") || e.getCause().getMessage().equals("Cannot reindex index")); + } + + cleanupMetadataBlocks(sourceIndex); + } + + public void testReadOnlyBlocksNotAddedBack() { assumeTrue("requires the migration reindex feature flag", REINDEX_DATA_STREAM_FEATURE_FLAG.isEnabled()); - // Create source index with read-only and/or block-writes var sourceIndex = randomAlphaOfLength(20).toLowerCase(Locale.ROOT); - boolean isReadOnly = randomBoolean(); - boolean isBlockWrites = randomBoolean(); var settings = Settings.builder() - .put(IndexMetadata.SETTING_READ_ONLY, isReadOnly) - .put(IndexMetadata.SETTING_BLOCKS_WRITE, isBlockWrites) + .put(IndexMetadata.SETTING_READ_ONLY, randomBoolean()) + .put(IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE, randomBoolean()) + .put(IndexMetadata.SETTING_BLOCKS_WRITE, randomBoolean()) .build(); indicesAdmin().create(new CreateIndexRequest(sourceIndex, settings)).actionGet(); @@ -241,13 +272,13 @@ public void testReadOnlyAddedBack() { .actionGet() .getDestIndex(); - // assert read-only settings added back to dest index var settingsResponse = indicesAdmin().getSettings(new GetSettingsRequest().indices(destIndex)).actionGet(); - assertEquals(isReadOnly, Boolean.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_READ_ONLY))); - assertEquals(isBlockWrites, Boolean.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_BLOCKS_WRITE))); + assertFalse(Boolean.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_READ_ONLY))); + assertFalse(Boolean.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE))); + assertFalse(Boolean.parseBoolean(settingsResponse.getSetting(destIndex, IndexMetadata.SETTING_BLOCKS_WRITE))); - removeReadOnly(sourceIndex); - removeReadOnly(destIndex); + cleanupMetadataBlocks(sourceIndex); + cleanupMetadataBlocks(destIndex); } public void testUpdateSettingsDefaultsRestored() { @@ -426,10 +457,11 @@ public void testTsdbStartEndSet() throws Exception { // TODO check other IndexMetadata fields that need to be fixed after the fact // TODO what happens if don't have necessary perms for a given index? - private static void removeReadOnly(String index) { + private static void cleanupMetadataBlocks(String index) { var settings = Settings.builder() - .put(IndexMetadata.SETTING_READ_ONLY, false) - .put(IndexMetadata.SETTING_BLOCKS_WRITE, false) + .putNull(IndexMetadata.SETTING_READ_ONLY) + .putNull(IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE) + .putNull(IndexMetadata.SETTING_BLOCKS_METADATA) .build(); assertAcked(indicesAdmin().updateSettings(new UpdateSettingsRequest(settings, index)).actionGet()); } diff --git a/x-pack/plugin/migrate/src/main/java/org/elasticsearch/xpack/migrate/action/ReindexDataStreamIndexTransportAction.java b/x-pack/plugin/migrate/src/main/java/org/elasticsearch/xpack/migrate/action/ReindexDataStreamIndexTransportAction.java index ff350429dae01..7bb440bc52a15 100644 --- a/x-pack/plugin/migrate/src/main/java/org/elasticsearch/xpack/migrate/action/ReindexDataStreamIndexTransportAction.java +++ b/x-pack/plugin/migrate/src/main/java/org/elasticsearch/xpack/migrate/action/ReindexDataStreamIndexTransportAction.java @@ -40,7 +40,6 @@ import java.util.Locale; import java.util.Map; -import static org.elasticsearch.cluster.metadata.IndexMetadata.APIBlock.READ_ONLY; import static org.elasticsearch.cluster.metadata.IndexMetadata.APIBlock.WRITE; public class ReindexDataStreamIndexTransportAction extends HandledTransportAction< @@ -93,13 +92,22 @@ protected void doExecute( ); } + if (settingsBefore.getAsBoolean(IndexMetadata.SETTING_BLOCKS_READ, false)) { + var errorMessage = String.format(Locale.ROOT, "Cannot reindex index [%s] which has a read block.", destIndexName); + listener.onFailure(new ElasticsearchException(errorMessage)); + return; + } + if (settingsBefore.getAsBoolean(IndexMetadata.SETTING_BLOCKS_METADATA, false)) { + var errorMessage = String.format(Locale.ROOT, "Cannot reindex index [%s] which has a metadata block.", destIndexName); + listener.onFailure(new ElasticsearchException(errorMessage)); + return; + } + SubscribableListener.newForked(l -> setBlockWrites(sourceIndexName, l, taskId)) .andThen(l -> deleteDestIfExists(destIndexName, l, taskId)) .andThen(l -> createIndex(sourceIndex, destIndexName, l, taskId)) .andThen(l -> reindex(sourceIndexName, destIndexName, l, taskId)) .andThen(l -> copyOldSourceSettingsToDest(settingsBefore, destIndexName, l, taskId)) - .andThen(l -> addBlockIfFromSource(WRITE, settingsBefore, destIndexName, l, taskId)) - .andThen(l -> addBlockIfFromSource(READ_ONLY, settingsBefore, destIndexName, l, taskId)) .andThenApply(ignored -> new ReindexDataStreamIndexAction.Response(destIndexName)) .addListener(listener); } @@ -120,7 +128,8 @@ public void onResponse(AddIndexBlockResponse response) { @Override public void onFailure(Exception e) { if (e instanceof ClusterBlockException || e.getCause() instanceof ClusterBlockException) { - // It's fine if block-writes is already set + // Could fail with a cluster block exception if read-only or read-only-allow-delete is already set + // In this case, we can proceed listener.onResponse(null); } else { listener.onFailure(e); @@ -146,10 +155,12 @@ private void createIndex( ) { logger.debug("Creating destination index [{}] for source index [{}]", destIndexName, sourceIndex.getIndex().getName()); - // override read-only settings if they exist var removeReadOnlyOverride = Settings.builder() + // remove read-only settings if they exist .putNull(IndexMetadata.SETTING_READ_ONLY) + .putNull(IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE) .putNull(IndexMetadata.SETTING_BLOCKS_WRITE) + // settings to optimize reindex .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) .put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), -1) .build(); @@ -192,6 +203,18 @@ private void addBlockIfFromSource( } } + private void updateSettings( + String index, + Settings.Builder settings, + ActionListener listener, + TaskId parentTaskId + ) { + var updateSettingsRequest = new UpdateSettingsRequest(settings.build(), index); + updateSettingsRequest.setParentTask(parentTaskId); + var errorMessage = String.format(Locale.ROOT, "Could not update settings on index [%s]", index); + client.admin().indices().updateSettings(updateSettingsRequest, failIfNotAcknowledged(listener, errorMessage)); + } + private void copyOldSourceSettingsToDest( Settings settingsBefore, String destIndexName, @@ -199,15 +222,10 @@ private void copyOldSourceSettingsToDest( TaskId parentTaskId ) { logger.debug("Updating settings on destination index after reindex completes"); - var settings = Settings.builder(); copySettingOrUnset(settingsBefore, settings, IndexMetadata.SETTING_NUMBER_OF_REPLICAS); copySettingOrUnset(settingsBefore, settings, IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey()); - - var updateSettingsRequest = new UpdateSettingsRequest(settings.build(), destIndexName); - updateSettingsRequest.setParentTask(parentTaskId); - var errorMessage = String.format(Locale.ROOT, "Could not update settings on index [%s]", destIndexName); - client.admin().indices().updateSettings(updateSettingsRequest, failIfNotAcknowledged(listener, errorMessage)); + updateSettings(destIndexName, settings, listener, parentTaskId); } private static void copySettingOrUnset(Settings settingsBefore, Settings.Builder builder, String setting) {