diff --git a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/removepolicy/TransportRemovePolicyAction.kt b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/removepolicy/TransportRemovePolicyAction.kt index de214ff52..684a710a6 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/removepolicy/TransportRemovePolicyAction.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/removepolicy/TransportRemovePolicyAction.kt @@ -44,6 +44,10 @@ import org.opensearch.action.support.master.AcknowledgedResponse import org.opensearch.client.node.NodeClient import org.opensearch.cluster.ClusterState import org.opensearch.cluster.block.ClusterBlockException +import org.opensearch.cluster.metadata.IndexMetadata.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING +import org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_SETTING +import org.opensearch.cluster.metadata.IndexMetadata.SETTING_READ_ONLY +import org.opensearch.cluster.metadata.IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE import org.opensearch.cluster.service.ClusterService import org.opensearch.common.inject.Inject import org.opensearch.common.settings.Settings @@ -88,6 +92,9 @@ class TransportRemovePolicyAction @Inject constructor( private val failedIndices: MutableList = mutableListOf() private val indicesToRemove = mutableMapOf() // uuid: name + private val indicesWithAutoManageBlock = mutableSetOf() + private val indicesWithReadOnlyBlock = mutableSetOf() + private val indicesWithReadOnlyAllowDeleteBlock = mutableSetOf() fun start() { if (user == null) { @@ -144,6 +151,15 @@ class TransportRemovePolicyAction @Inject constructor( val indexMetadatas = response.state.metadata.indices indexMetadatas.forEach { indicesToRemove.putIfAbsent(it.value.indexUUID, it.key) + if (it.value.settings.get(ManagedIndexSettings.AUTO_MANAGE.key) == "false") { + indicesWithAutoManageBlock.add(it.value.indexUUID) + } + if (it.value.settings.get(SETTING_READ_ONLY) == "true") { + indicesWithReadOnlyBlock.add(it.value.indexUUID) + } + if (it.value.settings.get(SETTING_READ_ONLY_ALLOW_DELETE) == "true") { + indicesWithReadOnlyAllowDeleteBlock.add(it.value.indexUUID) + } } populateLists(response.state) } @@ -220,25 +236,62 @@ class TransportRemovePolicyAction @Inject constructor( */ @Suppress("SpreadOperator") fun updateSettings(indices: Map) { - val request = UpdateSettingsRequest() - .indices(*indices.map { it.value }.toTypedArray()) - .settings(Settings.builder().put(ManagedIndexSettings.AUTO_MANAGE.key, false)) + // indices divide to read_only, read_only_allow_delete, normal + val indicesUuidsSet = indices.keys.toSet() - indicesWithAutoManageBlock + val readOnlyIndices = indicesUuidsSet.filter { it in indicesWithReadOnlyBlock } + val readOnlyAllowDeleteIndices = (indicesUuidsSet - readOnlyIndices).filter { it in indicesWithReadOnlyAllowDeleteBlock } + val normalIndices = indicesUuidsSet - readOnlyIndices - readOnlyAllowDeleteIndices + + val updateSettingReqsList = mutableListOf() + if (readOnlyIndices.isNotEmpty()) { + updateSettingReqsList.add( + UpdateSettingsRequest().indices(*readOnlyIndices.map { indices[it] }.toTypedArray()) + .settings( + Settings.builder().put(ManagedIndexSettings.AUTO_MANAGE.key, false) + .put(INDEX_READ_ONLY_SETTING.key, true) + ) + ) + } + if (readOnlyAllowDeleteIndices.isNotEmpty()) { + updateSettingReqsList.add( + UpdateSettingsRequest().indices(*readOnlyAllowDeleteIndices.map { indices[it] }.toTypedArray()) + .settings( + Settings.builder().put(ManagedIndexSettings.AUTO_MANAGE.key, false) + .put(INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.key, true) + ) + ) + } + if (normalIndices.isNotEmpty()) { + updateSettingReqsList.add( + UpdateSettingsRequest().indices(*normalIndices.map { indices[it] }.toTypedArray()) + .settings(Settings.builder().put(ManagedIndexSettings.AUTO_MANAGE.key, false)) + ) + } + + updateSettingCallChain(0, updateSettingReqsList) + } + + fun updateSettingCallChain(current: Int, updateSettingReqsList: List) { + if (updateSettingReqsList.isEmpty()) { + removeManagedIndices() + return + } client.admin().indices().updateSettings( - request, + updateSettingReqsList[current], object : ActionListener { override fun onResponse(response: AcknowledgedResponse) { - if (response.isAcknowledged) { - removeManagedIndices() - } else { - indices.forEach { - failedIndices.add( - FailedIndex( - it.value, it.key, - "Update auto_manage setting to false is not acknowledged, remove policy failed." - ) + if (!response.isAcknowledged) { + actionListener.onFailure( + IndexManagementException.wrap( + Exception("Failed to remove policy because ISM auto_manage setting update requests are not fully acknowledged.") ) - } - actionListener.onResponse(ISMStatusResponse(0, failedIndices)) + ) + return + } + if (current < updateSettingReqsList.size - 1) { + updateSettingCallChain(current + 1, updateSettingReqsList) + } else { + removeManagedIndices() } } @@ -246,7 +299,7 @@ class TransportRemovePolicyAction @Inject constructor( val ex = ExceptionsHelper.unwrapCause(t) as Exception actionListener.onFailure( IndexManagementException.wrap( - Exception("Failed to update auto_manage setting to false.", ex) + Exception("Failed to remove policy because ISM auto_manage setting update requests failed with exception:", ex) ) ) } diff --git a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/IndexStateManagementRestTestCase.kt b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/IndexStateManagementRestTestCase.kt index 1c2c8d576..2da13a7ab 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/IndexStateManagementRestTestCase.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/IndexStateManagementRestTestCase.kt @@ -512,6 +512,22 @@ abstract class IndexStateManagementRestTestCase : IndexManagementRestTestCase() return null } + @Suppress("UNCHECKED_CAST") + protected fun getIndexReadOnlySetting(indexName: String): Boolean? { + val indexSettings = getIndexSettings(indexName) as Map>> + val readOnlySetting = indexSettings[indexName]!!["settings"]!![IndexMetadata.SETTING_READ_ONLY] + if (readOnlySetting != null) return (readOnlySetting as String).toBoolean() + return null + } + + @Suppress("UNCHECKED_CAST") + protected fun getIndexReadOnlyAllowDeleteSetting(indexName: String): Boolean? { + val indexSettings = getIndexSettings(indexName) as Map>> + val readOnlyAllowDeleteSetting = indexSettings[indexName]!!["settings"]!![IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE] + if (readOnlyAllowDeleteSetting != null) return (readOnlyAllowDeleteSetting as String).toBoolean() + return null + } + @Suppress("UNCHECKED_CAST") protected fun getUuid(indexName: String): String { val indexSettings = getIndexSettings(indexName) as Map>> diff --git a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/resthandler/RestRemovePolicyActionIT.kt b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/resthandler/RestRemovePolicyActionIT.kt index 7b2ae34d8..7af04bf35 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/resthandler/RestRemovePolicyActionIT.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/resthandler/RestRemovePolicyActionIT.kt @@ -27,7 +27,9 @@ package org.opensearch.indexmanagement.indexstatemanagement.resthandler import org.opensearch.client.ResponseException +import org.opensearch.cluster.metadata.IndexMetadata import org.opensearch.indexmanagement.indexstatemanagement.IndexStateManagementRestTestCase +import org.opensearch.indexmanagement.indexstatemanagement.settings.ManagedIndexSettings import org.opensearch.indexmanagement.indexstatemanagement.util.FAILED_INDICES import org.opensearch.indexmanagement.indexstatemanagement.util.FAILURES import org.opensearch.indexmanagement.indexstatemanagement.util.UPDATED_INDICES @@ -190,20 +192,48 @@ class RestRemovePolicyActionIT : IndexStateManagementRestTestCase() { } } - fun `test remove policy update auto_manage setting`() { - val index = "movies" + fun `test remove policy on read only index update auto_manage setting`() { + val index1 = "read_only_index" + val index2 = "read_only_allow_delete_index" + val index3 = "normal_index" + val index4 = "auto_manage_false_index" + val indexPattern = "*index" val policy = createRandomPolicy() - createIndex(index, policy.id) - assertEquals("auto manage setting not null at index creation time", null, getIndexAutoManageSetting(index)) + createIndex(index1, policy.id) + createIndex(index2, policy.id) + createIndex(index3, policy.id) + createIndex(index4, policy.id) + updateIndexSetting(index1, IndexMetadata.SETTING_READ_ONLY, "true") + updateIndexSetting(index2, IndexMetadata.SETTING_READ_ONLY_ALLOW_DELETE, "true") + updateIndexSetting(index4, ManagedIndexSettings.AUTO_MANAGE.key, "false") val response = client().makeRequest( POST.toString(), - "${RestRemovePolicyAction.REMOVE_POLICY_BASE_URI}/$index" + "${RestRemovePolicyAction.REMOVE_POLICY_BASE_URI}/$indexPattern" ) assertEquals("Unexpected RestStatus", RestStatus.OK, response.restStatus()) + val actualMessage = response.asMap() + val expectedMessage = mapOf( + UPDATED_INDICES to 4, + FAILURES to false, + FAILED_INDICES to emptyList() + ) + assertAffectedIndicesResponseIsEqual(expectedMessage, actualMessage) waitFor { - assertEquals("auto manage setting not false after removing policy", false, getIndexAutoManageSetting(index)) + assertEquals("auto manage setting not false after removing policy for index $index1", false, getIndexAutoManageSetting(index1)) + assertEquals("read only allow delete setting changed after removing policy for index $index1", null, getIndexReadOnlyAllowDeleteSetting(index1)) + assertEquals("auto manage setting not false after removing policy for index $index2", false, getIndexAutoManageSetting(index2)) + assertEquals("read only setting changed after removing policy for index $index2", null, getIndexReadOnlySetting(index2)) + assertEquals("auto manage setting not false after removing policy for index $index3", false, getIndexAutoManageSetting(index3)) + assertEquals("read only setting changed after removing policy for index $index3", null, getIndexReadOnlySetting(index3)) + assertEquals("read only allow delete setting changed after removing policy for index $index3", null, getIndexReadOnlyAllowDeleteSetting(index3)) + assertEquals("auto manage setting not false after removing policy for index $index4", false, getIndexAutoManageSetting(index3)) + assertEquals("read only setting changed after removing policy for index $index4", null, getIndexReadOnlySetting(index3)) + assertEquals("read only allow delete setting changed after removing policy for index $index4", null, getIndexReadOnlyAllowDeleteSetting(index3)) } + + // otherwise, test cleanup cannot delete this index + updateIndexSetting(index1, IndexMetadata.SETTING_READ_ONLY, "false") } }