From af5b175944fdd3d7d91079cd06d04b4de10ac017 Mon Sep 17 00:00:00 2001 From: Clay Downs <89109232+downsrob@users.noreply.github.com> Date: Mon, 20 Dec 2021 11:09:05 -0800 Subject: [PATCH] Implement ReadOnlyAction using new interface (#227) * Refactors ReadOnlyAction Signed-off-by: Robert Downs <downsrob@amazon.com> --- build.gradle | 1 - .../indexstatemanagement/ISMActionsParser.kt | 4 +- .../action/ReadOnlyAction.kt | 9 +-- .../action/ReadOnlyActionParser.kt | 11 +++- .../step/readonly/SetReadOnlyStep.kt | 64 +++++++++++++++++-- .../indexstatemanagement/model/ActionTests.kt | 5 ++ .../model/XContentTests.kt | 10 +-- .../step/SetReadOnlyStepTests.kt | 45 +++++++++---- 8 files changed, 114 insertions(+), 35 deletions(-) diff --git a/build.gradle b/build.gradle index cb1b3b5de..7028c4431 100644 --- a/build.gradle +++ b/build.gradle @@ -307,7 +307,6 @@ integTest { exclude 'org/opensearch/indexmanagement/indexstatemanagement/action/IndexStateManagementHistoryIT.class' exclude 'org/opensearch/indexmanagement/indexstatemanagement/action/NotificationActionIT.class' exclude 'org/opensearch/indexmanagement/indexstatemanagement/action/OpenActionIT.class' - exclude 'org/opensearch/indexmanagement/indexstatemanagement/action/ReadOnlyActionIT.class' exclude 'org/opensearch/indexmanagement/indexstatemanagement/action/ReadWriteActionIT.class' exclude 'org/opensearch/indexmanagement/indexstatemanagement/action/ReplicaCountActionIT.class' exclude 'org/opensearch/indexmanagement/indexstatemanagement/action/RolloverActionIT.class' diff --git a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ISMActionsParser.kt b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ISMActionsParser.kt index 00b64f90f..2463de695 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ISMActionsParser.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ISMActionsParser.kt @@ -10,6 +10,7 @@ import org.opensearch.common.xcontent.XContentParser import org.opensearch.common.xcontent.XContentParserUtils import org.opensearch.indexmanagement.indexstatemanagement.action.CloseActionParser import org.opensearch.indexmanagement.indexstatemanagement.action.DeleteActionParser +import org.opensearch.indexmanagement.indexstatemanagement.action.ReadOnlyActionParser import org.opensearch.indexmanagement.spi.indexstatemanagement.Action import org.opensearch.indexmanagement.spi.indexstatemanagement.ActionParser import org.opensearch.indexmanagement.spi.indexstatemanagement.model.ActionRetry @@ -24,7 +25,8 @@ class ISMActionsParser private constructor() { // TODO: Add other action parsers as they are implemented val parsers = mutableListOf<ActionParser>( CloseActionParser(), - DeleteActionParser() + DeleteActionParser(), + ReadOnlyActionParser() ) fun addParser(parser: ActionParser) { diff --git a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/ReadOnlyAction.kt b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/ReadOnlyAction.kt index 4be076ea0..e17e1ae5c 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/ReadOnlyAction.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/ReadOnlyAction.kt @@ -5,6 +5,7 @@ package org.opensearch.indexmanagement.indexstatemanagement.action +import org.opensearch.indexmanagement.indexstatemanagement.step.readonly.SetReadOnlyStep import org.opensearch.indexmanagement.spi.indexstatemanagement.Action import org.opensearch.indexmanagement.spi.indexstatemanagement.Step import org.opensearch.indexmanagement.spi.indexstatemanagement.model.StepContext @@ -16,12 +17,12 @@ class ReadOnlyAction( companion object { const val name = "read_only" } + private val setReadOnlyStep = SetReadOnlyStep() + private val steps = listOf(setReadOnlyStep) override fun getStepToExecute(context: StepContext): Step { - TODO("Not yet implemented") + return setReadOnlyStep } - override fun getSteps(): List<Step> { - TODO("Not yet implemented") - } + override fun getSteps(): List<Step> = steps } diff --git a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/ReadOnlyActionParser.kt b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/ReadOnlyActionParser.kt index 34ec27922..43858a12f 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/ReadOnlyActionParser.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/ReadOnlyActionParser.kt @@ -7,19 +7,24 @@ package org.opensearch.indexmanagement.indexstatemanagement.action import org.opensearch.common.io.stream.StreamInput import org.opensearch.common.xcontent.XContentParser +import org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken import org.opensearch.indexmanagement.spi.indexstatemanagement.Action import org.opensearch.indexmanagement.spi.indexstatemanagement.ActionParser class ReadOnlyActionParser : ActionParser() { override fun fromStreamInput(sin: StreamInput): Action { - TODO("Not yet implemented") + val index = sin.readInt() + return ReadOnlyAction(index) } override fun fromXContent(xcp: XContentParser, index: Int): Action { - TODO("Not yet implemented") + ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.currentToken(), xcp) + ensureExpectedToken(XContentParser.Token.END_OBJECT, xcp.nextToken(), xcp) + + return ReadOnlyAction(index) } override fun getActionType(): String { - TODO("Not yet implemented") + return ReadOnlyAction.name } } diff --git a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/step/readonly/SetReadOnlyStep.kt b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/step/readonly/SetReadOnlyStep.kt index 4028ca91b..b821d5b59 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/step/readonly/SetReadOnlyStep.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/step/readonly/SetReadOnlyStep.kt @@ -5,25 +5,75 @@ package org.opensearch.indexmanagement.indexstatemanagement.step.readonly +import org.apache.logging.log4j.LogManager +import org.opensearch.ExceptionsHelper +import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequest +import org.opensearch.action.support.master.AcknowledgedResponse +import org.opensearch.cluster.metadata.IndexMetadata.SETTING_BLOCKS_WRITE +import org.opensearch.common.settings.Settings +import org.opensearch.indexmanagement.opensearchapi.suspendUntil import org.opensearch.indexmanagement.spi.indexstatemanagement.Step import org.opensearch.indexmanagement.spi.indexstatemanagement.model.ManagedIndexMetaData +import org.opensearch.indexmanagement.spi.indexstatemanagement.model.StepMetaData +import org.opensearch.transport.RemoteTransportException class SetReadOnlyStep : Step(name) { + + private val logger = LogManager.getLogger(javaClass) + private var stepStatus = StepStatus.STARTING + private var info: Map<String, Any>? = null + override suspend fun execute(): Step { - TODO("Not yet implemented") + val context = this.context ?: return this + val indexName = context.metadata.index + try { + val updateSettingsRequest = UpdateSettingsRequest() + .indices(indexName) + .settings(Settings.builder().put(SETTING_BLOCKS_WRITE, true)) + val response: AcknowledgedResponse = context.client.admin().indices() + .suspendUntil { updateSettings(updateSettingsRequest, it) } + + if (response.isAcknowledged) { + stepStatus = StepStatus.COMPLETED + info = mapOf("message" to getSuccessMessage(indexName)) + } else { + val message = getFailedMessage(indexName) + logger.warn(message) + stepStatus = StepStatus.FAILED + info = mapOf("message" to message) + } + } catch (e: RemoteTransportException) { + handleException(indexName, ExceptionsHelper.unwrapCause(e) as Exception) + } catch (e: Exception) { + handleException(indexName, e) + } + + return this } - override fun getUpdatedManagedIndexMetadata(currentMetadata: ManagedIndexMetaData): ManagedIndexMetaData { - TODO("Not yet implemented") + private fun handleException(indexName: String, e: Exception) { + val message = getFailedMessage(indexName) + logger.error(message, e) + stepStatus = StepStatus.FAILED + val mutableInfo = mutableMapOf("message" to message) + val errorMessage = e.message + if (errorMessage != null) mutableInfo["cause"] = errorMessage + info = mutableInfo.toMap() } - override fun isIdempotent(): Boolean { - TODO("Not yet implemented") + override fun getUpdatedManagedIndexMetadata(currentMetadata: ManagedIndexMetaData): ManagedIndexMetaData { + return currentMetadata.copy( + stepMetaData = StepMetaData(name, getStepStartTime(currentMetadata).toEpochMilli(), stepStatus), + transitionTo = null, + info = info + ) } + override fun isIdempotent(): Boolean = true + companion object { const val name = "set_read_only" - // TODO: fixme - fun getSuccessMessage(indexName: String) = "" + fun getFailedMessage(index: String) = "Failed to set index to read-only [index=$index]" + fun getSuccessMessage(index: String) = "Successfully set index to read-only [index=$index]" } } diff --git a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ActionTests.kt b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ActionTests.kt index 957389982..1681051e3 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ActionTests.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ActionTests.kt @@ -18,6 +18,7 @@ import org.opensearch.indexmanagement.indexstatemanagement.randomAllocationActio import org.opensearch.indexmanagement.indexstatemanagement.randomForceMergeActionConfig import org.opensearch.indexmanagement.indexstatemanagement.randomIndexPriorityActionConfig import org.opensearch.indexmanagement.indexstatemanagement.randomNotificationActionConfig +import org.opensearch.indexmanagement.indexstatemanagement.randomReadOnlyActionConfig import org.opensearch.indexmanagement.indexstatemanagement.randomReplicaCountActionConfig import org.opensearch.indexmanagement.indexstatemanagement.randomRolloverActionConfig import org.opensearch.indexmanagement.indexstatemanagement.randomSnapshotActionConfig @@ -74,6 +75,10 @@ class ActionTests : OpenSearchTestCase() { } } + fun `test set read only action round trip`() { + roundTripAction(randomReadOnlyActionConfig()) + } + // TODO: fixme - enable the test private fun `test rollover action round trip`() { roundTripAction(randomRolloverActionConfig()) diff --git a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/XContentTests.kt b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/XContentTests.kt index ddebb05ac..331fff582 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/XContentTests.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/XContentTests.kt @@ -95,12 +95,12 @@ class XContentTests : OpenSearchTestCase() { assertEquals("Round tripping RolloverActionConfig doesn't work", rolloverActionConfig, parsedRolloverActionConfig) } - private fun `test read_only action config parsing`() { - val readOnlyActionConfig = randomReadOnlyActionConfig() + fun `test read_only action config parsing`() { + val readOnlyAction = randomReadOnlyActionConfig() - val readOnlyActionConfigString = readOnlyActionConfig.toJsonString() - val parsedReadOnlyActionConfig = ISMActionsParser.instance.parse(parser(readOnlyActionConfigString), 0) - assertEquals("Round tripping ReadOnlyActionConfig doesn't work", readOnlyActionConfig, parsedReadOnlyActionConfig) + val readOnlyActionString = readOnlyAction.toJsonString() + val parsedReadOnlyAction = ISMActionsParser.instance.parse(parser(readOnlyActionString), 0) + assertEquals("Round tripping ReadOnlyAction doesn't work", readOnlyAction.convertToMap(), parsedReadOnlyAction.convertToMap()) } private fun `test read_write action config parsing`() { diff --git a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/step/SetReadOnlyStepTests.kt b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/step/SetReadOnlyStepTests.kt index faaaad5f0..5df1b5a06 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/step/SetReadOnlyStepTests.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/step/SetReadOnlyStepTests.kt @@ -5,22 +5,39 @@ package org.opensearch.indexmanagement.indexstatemanagement.step +import com.nhaarman.mockitokotlin2.any +import com.nhaarman.mockitokotlin2.doAnswer +import com.nhaarman.mockitokotlin2.doReturn +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.whenever +import kotlinx.coroutines.runBlocking +import org.opensearch.action.ActionListener +import org.opensearch.action.support.master.AcknowledgedResponse +import org.opensearch.client.AdminClient +import org.opensearch.client.Client +import org.opensearch.client.IndicesAdminClient +import org.opensearch.cluster.service.ClusterService +import org.opensearch.indexmanagement.indexstatemanagement.step.readonly.SetReadOnlyStep +import org.opensearch.indexmanagement.spi.indexstatemanagement.Step +import org.opensearch.indexmanagement.spi.indexstatemanagement.model.ManagedIndexMetaData +import org.opensearch.indexmanagement.spi.indexstatemanagement.model.StepContext import org.opensearch.test.OpenSearchTestCase +import org.opensearch.transport.RemoteTransportException class SetReadOnlyStepTests : OpenSearchTestCase() { - /*private val clusterService: ClusterService = mock() + private val clusterService: ClusterService = mock() fun `test read only step sets step status to failed when not acknowledged`() { val setReadOnlyResponse = AcknowledgedResponse(false) val client = getClient(getAdminClient(getIndicesAdminClient(setReadOnlyResponse, null))) runBlocking { - val readOnlyActionConfig = ReadOnlyActionConfig(0) val managedIndexMetaData = ManagedIndexMetaData("test", "indexUuid", "policy_id", null, null, null, null, null, null, null, null, null, null) - val setReadOnlyStep = SetReadOnlyStep(clusterService, client, readOnlyActionConfig, managedIndexMetaData) - setReadOnlyStep.execute() - val updatedManagedIndexMetaData = setReadOnlyStep.getUpdatedManagedIndexMetaData(managedIndexMetaData) + val setReadOnlyStep = SetReadOnlyStep() + val context = StepContext(managedIndexMetaData, clusterService, client, null, null) + setReadOnlyStep.preExecute(logger, context).execute() + val updatedManagedIndexMetaData = setReadOnlyStep.getUpdatedManagedIndexMetadata(managedIndexMetaData) assertEquals("Step status is not FAILED", Step.StepStatus.FAILED, updatedManagedIndexMetaData.stepMetaData?.stepStatus) } } @@ -30,11 +47,11 @@ class SetReadOnlyStepTests : OpenSearchTestCase() { val client = getClient(getAdminClient(getIndicesAdminClient(null, exception))) runBlocking { - val readOnlyActionConfig = ReadOnlyActionConfig(0) val managedIndexMetaData = ManagedIndexMetaData("test", "indexUuid", "policy_id", null, null, null, null, null, null, null, null, null, null) - val setReadOnlyStep = SetReadOnlyStep(clusterService, client, readOnlyActionConfig, managedIndexMetaData) - setReadOnlyStep.execute() - val updatedManagedIndexMetaData = setReadOnlyStep.getUpdatedManagedIndexMetaData(managedIndexMetaData) + val setReadOnlyStep = SetReadOnlyStep() + val context = StepContext(managedIndexMetaData, clusterService, client, null, null) + setReadOnlyStep.preExecute(logger, context).execute() + val updatedManagedIndexMetaData = setReadOnlyStep.getUpdatedManagedIndexMetadata(managedIndexMetaData) assertEquals("Step status is not FAILED", Step.StepStatus.FAILED, updatedManagedIndexMetaData.stepMetaData?.stepStatus) } } @@ -44,11 +61,11 @@ class SetReadOnlyStepTests : OpenSearchTestCase() { val client = getClient(getAdminClient(getIndicesAdminClient(null, exception))) runBlocking { - val readOnlyActionConfig = ReadOnlyActionConfig(0) val managedIndexMetaData = ManagedIndexMetaData("test", "indexUuid", "policy_id", null, null, null, null, null, null, null, null, null, null) - val setReadOnlyStep = SetReadOnlyStep(clusterService, client, readOnlyActionConfig, managedIndexMetaData) - setReadOnlyStep.execute() - val updatedManagedIndexMetaData = setReadOnlyStep.getUpdatedManagedIndexMetaData(managedIndexMetaData) + val setReadOnlyStep = SetReadOnlyStep() + val context = StepContext(managedIndexMetaData, clusterService, client, null, null) + setReadOnlyStep.preExecute(logger, context).execute() + val updatedManagedIndexMetaData = setReadOnlyStep.getUpdatedManagedIndexMetadata(managedIndexMetaData) assertEquals("Step status is not FAILED", Step.StepStatus.FAILED, updatedManagedIndexMetaData.stepMetaData?.stepStatus) assertEquals("Did not get cause from nested exception", "nested", updatedManagedIndexMetaData.info!!["cause"]) } @@ -65,5 +82,5 @@ class SetReadOnlyStepTests : OpenSearchTestCase() { else listener.onFailure(exception) }.whenever(this.mock).updateSettings(any(), any()) } - }*/ + } }