Adds back opendistro policy_id prefix to explain response (#129)

* Explain response still use old opendistro policy id (#109) * Explain response still use old opendistro policy id * Use hardcoded policyid setting in tests for explain response * Trying to fix flaky tests * Release notes for 1.0.0.0 (#96) * Updates build.gradle/github workflows, version, and adds release notes Signed-off-by: Drew Baugher <[email protected]> Co-authored-by: Bowen Lan <[email protected]> Co-authored-by: Ravi <[email protected]>
opensearch-project · Aug 25, 2021 · 6b8d454 · 6b8d454
1 parent c26dbf9
commit 6b8d454
Show file tree

Hide file tree

Showing 15 changed files with 109 additions and 83 deletions.
diff --git a/.github/workflows/multi-node-test-workflow.yml b/.github/workflows/multi-node-test-workflow.yml
@@ -26,7 +26,7 @@ jobs:
         with:
           repository: 'opensearch-project/OpenSearch'
           path: OpenSearch
-          ref: '1.0'
+          ref: refs/tags/1.0.0
       - name: Build OpenSearch
         working-directory: ./OpenSearch
         run: ./gradlew publishToMavenLocal -Dbuild.snapshot=false

diff --git a/.github/workflows/test-and-build-workflow.yml b/.github/workflows/test-and-build-workflow.yml
@@ -26,7 +26,7 @@ jobs:
         with:
           repository: 'opensearch-project/OpenSearch'
           path: OpenSearch
-          ref: '1.0'
+          ref: refs/tags/1.0.0
       - name: Build OpenSearch
         working-directory: ./OpenSearch
         run: ./gradlew publishToMavenLocal -Dbuild.snapshot=false

diff --git a/gradle.properties b/gradle.properties
@@ -23,4 +23,4 @@
 #   permissions and limitations under the License.
 #
 
-version = 1.0.0
+version = 1.0.1
diff --git a/release-notes/opensearch-index-management.release-notes-1.0.0.0.md b/release-notes/opensearch-index-management.release-notes-1.0.0.0.md
@@ -0,0 +1,34 @@
+## Version 1.0.0.0 2021-07-12
+
+Compatible with OpenSearch 1.0.0
+
+### Infrastructure
+
+* Update issue template with multiple labels ([#10](https://github.com/opensearch-project/index-management/pull/10))
+
+### Maintenance
+
+* Renaming Namespaces ([#14](https://github.com/opensearch-project/index-management/pull/14))
+* Rest APIs Backward Compatibility ([#15](https://github.com/opensearch-project/index-management/pull/15))
+* Settings Backwards Compatibility ([#16](https://github.com/opensearch-project/index-management/pull/16))
+* Point to correct version of notification ([#18](https://github.com/opensearch-project/index-management/pull/18))
+* Enable security plugin integrated tests ([#93](https://github.com/opensearch-project/index-management/pull/93))
+
+### Features
+
+* Adds support for Transform feature ([#17](https://github.com/opensearch-project/index-management/pull/17))
+
+### Enhancements
+
+* Improve integration with data streams ([#13](https://github.com/opensearch-project/index-management/pull/13))
+* Skip rollover on non-write indices ([#88](https://github.com/opensearch-project/index-management/pull/88))
+
+### Documentation 
+
+* Update and add documentation files ([#84](https://github.com/opensearch-project/index-management/pull/84))
+
+### Bug Fixes
+
+* Fix 7.10 rollover blocking thread issue ([#70](https://github.com/opensearch-project/index-management/pull/70))
+* Fix issue with Rollup start/stop API ([#79](https://github.com/opensearch-project/index-management/pull/79))
+* Fix to support custom source index type in Rollup jobs ([#4](https://github.com/opensearch-project/index-management/pull/4)) 
diff --git a/release-notes/opensearch-index-management.release-notes-1.0.1.0.md b/release-notes/opensearch-index-management.release-notes-1.0.1.0.md
@@ -0,0 +1,7 @@
+## Version 1.0.1.0 2021-08-24
+
+Compatible with OpenSearch 1.0.1
+
+### Bug Fixes
+
+* Adds back opendistro policy_id setting to explain response ([#127](https://github.com/opensearch-project/index-management/pull/127))
diff --git a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexRunner.kt b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexRunner.kt
@@ -36,7 +36,6 @@ import kotlinx.coroutines.withContext
 import org.apache.logging.log4j.LogManager
 import org.opensearch.action.admin.cluster.state.ClusterStateRequest
 import org.opensearch.action.admin.cluster.state.ClusterStateResponse
-import org.opensearch.action.admin.indices.settings.put.UpdateSettingsRequest
 import org.opensearch.action.bulk.BackoffPolicy
 import org.opensearch.action.get.GetRequest
 import org.opensearch.action.get.GetResponse
@@ -722,17 +721,7 @@ object ManagedIndexRunner :
         if (!updated.metadataSaved || policy == null) return
 
         // this will save the new policy on the job and reset the change policy back to null
-        val saved = savePolicyToManagedIndexConfig(managedIndexConfig, policy)
-
-        if (saved) {
-            /*
-            * If we successfully saved the the new policy then the last thing we need to do is update the
-            * opendistro.indexstatemanagement.policy_id setting to the new policy id we don't care that much if this fails, because we'll
-            * have a check in the beginning of the runner to read in the setting and compare it with the policy_id on the job and update
-            * the setting if they ever differ, as we do not allow someone to change an existing policy using _settings API
-            * */
-            updateIndexPolicyIDSetting(managedIndexConfig.index, changePolicy.policyID)
-        }
+        savePolicyToManagedIndexConfig(managedIndexConfig, policy)
     }
 
     @Suppress("TooGenericExceptionCaught")
@@ -750,29 +739,6 @@ object ManagedIndexRunner :
         }
     }
 
-    /**
-     * Once we successfully swap over a ChangePolicy then we need to update the [ManagedIndexSettings.POLICY_ID] setting.
-     *
-     * We will constantly check the [ManagedIndexSettings.POLICY_ID] against the [ManagedIndexConfig] policyID and if
-     * there is ever a mismatch we will overwrite the [ManagedIndexSettings.POLICY_ID] with the [ManagedIndexConfig] policyID.
-     *
-     * We do this because if this fails we want to ensure we try again on the next execution of the job. At the same time, this
-     * will disallow the user from directly using the _settings API to change the policy_id. We do not want to allow this,
-     * they must use the ChangePolicy API as the [ManagedIndexSettings.POLICY_ID] is referring to the currently running policy.
-     */
-    private suspend fun updateIndexPolicyIDSetting(index: String, policyID: String) {
-        try {
-            val settings = Settings.builder().put(ManagedIndexSettings.POLICY_ID.key, policyID).build()
-            val updateSettingsRequest = UpdateSettingsRequest(index).settings(settings)
-            val response: AcknowledgedResponse = client.admin().indices().suspendUntil { updateSettings(updateSettingsRequest, it) }
-            if (!response.isAcknowledged) {
-                logger.warn("Updating policy_id ($policyID) for $index was not acknowledged")
-            }
-        } catch (e: Exception) {
-            logger.error("There was an error while trying to update the policy_id ($policyID) setting for $index", e)
-        }
-    }
-
     private suspend fun publishErrorNotification(policy: Policy, managedIndexMetaData: ManagedIndexMetaData) {
         policy.errorNotification?.run {
             errorNotificationRetryPolicy.retry(logger) {

diff --git a/...earch/indexmanagement/indexstatemanagement/transport/action/explain/ExplainAllResponse.kt b/...earch/indexmanagement/indexstatemanagement/transport/action/explain/ExplainAllResponse.kt
@@ -32,6 +32,7 @@ import org.opensearch.common.xcontent.ToXContent
 import org.opensearch.common.xcontent.ToXContentObject
 import org.opensearch.common.xcontent.XContentBuilder
 import org.opensearch.indexmanagement.indexstatemanagement.model.ManagedIndexMetaData
+import org.opensearch.indexmanagement.indexstatemanagement.settings.LegacyOpenDistroManagedIndexSettings
 import org.opensearch.indexmanagement.indexstatemanagement.settings.ManagedIndexSettings
 import java.io.IOException
 
@@ -71,6 +72,7 @@ class ExplainAllResponse : ExplainResponse, ToXContentObject {
         builder.startObject()
         indexNames.forEachIndexed { ind, name ->
             builder.startObject(name)
+            builder.field(LegacyOpenDistroManagedIndexSettings.POLICY_ID.key, indexPolicyIDs[ind])
             builder.field(ManagedIndexSettings.POLICY_ID.key, indexPolicyIDs[ind])
             indexMetadatas[ind]?.toXContent(builder, ToXContent.EMPTY_PARAMS)
             builder.field("enabled", enabledState[name])

diff --git a/...ensearch/indexmanagement/indexstatemanagement/transport/action/explain/ExplainResponse.kt b/...ensearch/indexmanagement/indexstatemanagement/transport/action/explain/ExplainResponse.kt
@@ -33,6 +33,7 @@ import org.opensearch.common.xcontent.ToXContent
 import org.opensearch.common.xcontent.ToXContentObject
 import org.opensearch.common.xcontent.XContentBuilder
 import org.opensearch.indexmanagement.indexstatemanagement.model.ManagedIndexMetaData
+import org.opensearch.indexmanagement.indexstatemanagement.settings.LegacyOpenDistroManagedIndexSettings
 import org.opensearch.indexmanagement.indexstatemanagement.settings.ManagedIndexSettings
 import java.io.IOException
 
@@ -71,6 +72,7 @@ open class ExplainResponse : ActionResponse, ToXContentObject {
         builder.startObject()
         indexNames.forEachIndexed { ind, name ->
             builder.startObject(name)
+            builder.field(LegacyOpenDistroManagedIndexSettings.POLICY_ID.key, indexPolicyIDs[ind])
             builder.field(ManagedIndexSettings.POLICY_ID.key, indexPolicyIDs[ind])
             indexMetadatas[ind]?.toXContent(builder, ToXContent.EMPTY_PARAMS)
             builder.endObject()

diff --git a/...n/org/opensearch/indexmanagement/indexstatemanagement/IndexStateManagementRestTestCase.kt b/...n/org/opensearch/indexmanagement/indexstatemanagement/IndexStateManagementRestTestCase.kt
@@ -92,6 +92,9 @@ import java.util.Locale
 
 abstract class IndexStateManagementRestTestCase : IndexManagementRestTestCase() {
 
+    val explainResponseOpendistroPolicyIdSetting = "index.opendistro.index_state_management.policy_id"
+    val explainResponseOpenSearchPolicyIdSetting = "index.plugins.index_state_management.policy_id"
+
     protected fun createPolicy(
         policy: Policy,
         policyId: String = OpenSearchTestCase.randomAlphaOfLength(10),
@@ -244,12 +247,6 @@ abstract class IndexStateManagementRestTestCase : IndexManagementRestTestCase()
         client().makeRequest("POST", "/_opendistro/_ism/remove/$index")
     }
 
-    @Suppress("UNCHECKED_CAST")
-    protected fun getPolicyFromIndex(index: String): String? {
-        val indexSettings = getIndexSettings(index) as Map<String, Map<String, Map<String, Any?>>>
-        return indexSettings[index]!!["settings"]!![ManagedIndexSettings.POLICY_ID.key] as? String
-    }
-
     protected fun getPolicyIDOfManagedIndex(index: String): String? {
         val managedIndex = getManagedIndexConfig(index)
         return managedIndex?.policyID

diff --git a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/ActionRetryIT.kt b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/action/ActionRetryIT.kt
@@ -32,7 +32,6 @@ import org.opensearch.indexmanagement.indexstatemanagement.model.managedindexmet
 import org.opensearch.indexmanagement.indexstatemanagement.model.managedindexmetadata.PolicyRetryInfoMetaData
 import org.opensearch.indexmanagement.indexstatemanagement.model.managedindexmetadata.StateMetaData
 import org.opensearch.indexmanagement.indexstatemanagement.model.managedindexmetadata.StepMetaData
-import org.opensearch.indexmanagement.indexstatemanagement.settings.ManagedIndexSettings
 import org.opensearch.indexmanagement.indexstatemanagement.step.Step
 import org.opensearch.indexmanagement.indexstatemanagement.step.rollover.AttemptRolloverStep
 import org.opensearch.indexmanagement.waitFor
@@ -159,7 +158,8 @@ class ActionRetryIT : IndexStateManagementRestTestCase() {
             assertPredicatesOnMetaData(
                 listOf(
                     indexName to listOf(
-                        ManagedIndexSettings.POLICY_ID.key to policyID::equals,
+                        explainResponseOpendistroPolicyIdSetting to policyID::equals,
+                        explainResponseOpenSearchPolicyIdSetting to policyID::equals,
                         ManagedIndexMetaData.INDEX to managedIndexConfig.index::equals,
                         ManagedIndexMetaData.INDEX_UUID to managedIndexConfig.indexUuid::equals,
                         ManagedIndexMetaData.POLICY_ID to managedIndexConfig.policyID::equals,

diff --git a/.../opensearch/indexmanagement/indexstatemanagement/coordinator/ManagedIndexCoordinatorIT.kt b/.../opensearch/indexmanagement/indexstatemanagement/coordinator/ManagedIndexCoordinatorIT.kt
@@ -127,7 +127,9 @@ class ManagedIndexCoordinatorIT : IndexStateManagementRestTestCase() {
             assertPredicatesOnMetaData(
                 listOf(
                     index to listOf(
-                        ManagedIndexSettings.POLICY_ID.key to fun(policyID: Any?): Boolean =
+                        explainResponseOpendistroPolicyIdSetting to fun(policyID: Any?): Boolean =
+                            policyID == null,
+                        explainResponseOpenSearchPolicyIdSetting to fun(policyID: Any?): Boolean =
                             policyID == null
                     )
                 ),
@@ -269,7 +271,9 @@ class ManagedIndexCoordinatorIT : IndexStateManagementRestTestCase() {
         }
 
         // TODO seen version conflict flaky failure here
-        // could be same reason as the test failure in ChangePolicyActionIT
+        logger.info("Config we use on update: $enabledManagedIndexConfig")
+        logger.info("Latest config: ${getExistingManagedIndexConfig(indexName)}")
+        // seems the config from above waitFor, after that, config got updated again?
         updateManagedIndexConfigStartTime(enabledManagedIndexConfig)
 
         waitFor {

diff --git a/...ensearch/indexmanagement/indexstatemanagement/coordinator/ManagedIndexCoordinatorTests.kt b/...ensearch/indexmanagement/indexstatemanagement/coordinator/ManagedIndexCoordinatorTests.kt
@@ -31,8 +31,6 @@ import org.mockito.Mockito
 import org.opensearch.Version
 import org.opensearch.client.Client
 import org.opensearch.cluster.OpenSearchAllocationTestCase
-import org.opensearch.cluster.metadata.IndexMetadata
-import org.opensearch.cluster.metadata.IndexMetadata.SETTING_INDEX_UUID
 import org.opensearch.cluster.node.DiscoveryNode
 import org.opensearch.cluster.service.ClusterService
 import org.opensearch.common.settings.ClusterSettings
@@ -139,18 +137,6 @@ class ManagedIndexCoordinatorTests : OpenSearchAllocationTestCase() {
         Mockito.verify(threadPool, Mockito.times(2)).scheduleWithFixedDelay(Mockito.any(), Mockito.any(), Mockito.anyString())
     }
 
-    private fun createIndexMetaData(indexName: String, replicaNumber: Int, shardNumber: Int, policyID: String?): IndexMetadata.Builder {
-        val defaultSettings = Settings.builder()
-            .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
-            .put(ManagedIndexSettings.POLICY_ID.key, policyID)
-            .put(SETTING_INDEX_UUID, randomAlphaOfLength(20))
-            .build()
-        return IndexMetadata.Builder(indexName)
-            .settings(defaultSettings)
-            .numberOfReplicas(replicaNumber)
-            .numberOfShards(shardNumber)
-    }
-
     private fun <T> any(): T {
         Mockito.any<T>()
         return uninitialized()

diff --git a/...n/org/opensearch/indexmanagement/indexstatemanagement/resthandler/ISMTemplateRestAPIIT.kt b/...n/org/opensearch/indexmanagement/indexstatemanagement/resthandler/ISMTemplateRestAPIIT.kt
@@ -35,7 +35,6 @@ import org.opensearch.indexmanagement.indexstatemanagement.model.State
 import org.opensearch.indexmanagement.indexstatemanagement.model.action.ReadOnlyActionConfig
 import org.opensearch.indexmanagement.indexstatemanagement.randomErrorNotification
 import org.opensearch.indexmanagement.indexstatemanagement.randomPolicy
-import org.opensearch.indexmanagement.indexstatemanagement.settings.ManagedIndexSettings
 import org.opensearch.indexmanagement.indexstatemanagement.util.INDEX_HIDDEN
 import org.opensearch.indexmanagement.randomInstant
 import org.opensearch.indexmanagement.waitFor
@@ -121,15 +120,25 @@ class ISMTemplateRestAPIIT : IndexStateManagementRestTestCase() {
 
         // only index create after template can be managed
         assertPredicatesOnMetaData(
-            listOf(indexName1 to listOf(ManagedIndexSettings.POLICY_ID.key to fun(policyID: Any?): Boolean = policyID == null)),
+            listOf(
+                indexName1 to listOf(
+                    explainResponseOpendistroPolicyIdSetting to fun(policyID: Any?): Boolean = policyID == null,
+                    explainResponseOpenSearchPolicyIdSetting to fun(policyID: Any?): Boolean = policyID == null
+                )
+            ),
             getExplainMap(indexName1),
             true
         )
         assertNull(getManagedIndexConfig(indexName1))
 
         // hidden index will not be manage
         assertPredicatesOnMetaData(
-            listOf(indexName1 to listOf(ManagedIndexSettings.POLICY_ID.key to fun(policyID: Any?): Boolean = policyID == null)),
+            listOf(
+                indexName1 to listOf(
+                    explainResponseOpendistroPolicyIdSetting to fun(policyID: Any?): Boolean = policyID == null,
+                    explainResponseOpenSearchPolicyIdSetting to fun(policyID: Any?): Boolean = policyID == null
+                )
+            ),
             getExplainMap(indexName1),
             true
         )