Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BWC for rollover skip, restricted index pattern #371

Merged
merged 2 commits into from
May 23, 2022
Merged

BWC for rollover skip, restricted index pattern #371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
99b4694
BWC for rollover skip, restricted index pattern
bowenlan-amzn May 23, 2022
917dcc1
Remove VisibleForTesting
bowenlan-amzn May 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/main/kotlin/org/opensearch/indexmanagement/IndexManagementPlugin.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ class IndexManagementPlugin : JobSchedulerExtension, NetworkPlugin, ActionPlugin
LegacyOpenDistroManagedIndexSettings.HISTORY_NUMBER_OF_REPLICAS,
LegacyOpenDistroManagedIndexSettings.POLICY_ID,
LegacyOpenDistroManagedIndexSettings.ROLLOVER_ALIAS,
LegacyOpenDistroManagedIndexSettings.ROLLOVER_SKIP,
LegacyOpenDistroManagedIndexSettings.INDEX_STATE_MANAGEMENT_ENABLED,
LegacyOpenDistroManagedIndexSettings.METADATA_SERVICE_ENABLED,
LegacyOpenDistroManagedIndexSettings.JOB_INTERVAL,
Expand All @@ -462,6 +463,7 @@ class IndexManagementPlugin : JobSchedulerExtension, NetworkPlugin, ActionPlugin
LegacyOpenDistroManagedIndexSettings.AUTO_MANAGE,
LegacyOpenDistroManagedIndexSettings.METADATA_SERVICE_STATUS,
LegacyOpenDistroManagedIndexSettings.TEMPLATE_MIGRATION_CONTROL,
LegacyOpenDistroManagedIndexSettings.RESTRICTED_INDEX_PATTERN,
LegacyOpenDistroRollupSettings.ROLLUP_INGEST_BACKOFF_COUNT,
LegacyOpenDistroRollupSettings.ROLLUP_INGEST_BACKOFF_MILLIS,
LegacyOpenDistroRollupSettings.ROLLUP_SEARCH_BACKOFF_COUNT,
Expand Down
3 changes: 3 additions & 0 deletions ...org/opensearch/indexmanagement/indexstatemanagement/opensearchapi/OpenSearchExtensions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ fun IndexMetadata.getRolloverAlias(): String? {
}

fun IndexMetadata.getRolloverSkip(): Boolean {
if (this.settings.get(ManagedIndexSettings.ROLLOVER_SKIP.key).isNullOrBlank()) {
return this.settings.getAsBoolean(LegacyOpenDistroManagedIndexSettings.ROLLOVER_SKIP.key, false)
}
Comment on lines +58 to +60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is unnecessary because the fallback setting for ManagedIndexSettings.ROLLOVER_SKIP is set to LegacyOpenDistroManagedIndexSettings.ROLLOVER_SKIP when initialized, so the open distro setting would be checked anyways

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on above getRolloverAlias, probably this is not working as that. Write like this should be safe.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I looked into it deeper and it seems that the fallback settings do not operate as I thought for index settings.

return this.settings.getAsBoolean(ManagedIndexSettings.ROLLOVER_SKIP.key, false)
}

Expand Down
16 changes: 16 additions & 0 deletions ...rch/indexmanagement/indexstatemanagement/settings/LegacyOpenDistroManagedIndexSettings.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ class LegacyOpenDistroManagedIndexSettings {
Setting.Property.Deprecated
)

val ROLLOVER_SKIP: Setting<Boolean> = Setting.boolSetting(
"index.opendistro.index_state_management.rollover_skip",
false,
Setting.Property.IndexScope,
Setting.Property.Dynamic,
Setting.Property.Deprecated
)

val AUTO_MANAGE: Setting<Boolean> = Setting.boolSetting(
"index.opendistro.index_state_management.auto_manage",
true,
Expand Down Expand Up @@ -191,5 +199,13 @@ class LegacyOpenDistroManagedIndexSettings {
Setting.Property.Dynamic,
Setting.Property.Deprecated
)

val RESTRICTED_INDEX_PATTERN = Setting.simpleString(
"opendistro.index_state_management.restricted_index_pattern",
ManagedIndexSettings.DEFAULT_RESTRICTED_PATTERN,
Setting.Property.NodeScope,
Setting.Property.Dynamic,
Setting.Property.Deprecated
)
Comment on lines +203 to +209
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this too Bowen

}
}
4 changes: 2 additions & 2 deletions ...tlin/org/opensearch/indexmanagement/indexstatemanagement/settings/ManagedIndexSettings.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class ManagedIndexSettings {

val ROLLOVER_SKIP: Setting<Boolean> = Setting.boolSetting(
"index.plugins.index_state_management.rollover_skip",
false,
LegacyOpenDistroManagedIndexSettings.ROLLOVER_SKIP,
Setting.Property.IndexScope,
Setting.Property.Dynamic
)
Expand Down Expand Up @@ -194,7 +194,7 @@ class ManagedIndexSettings {

val RESTRICTED_INDEX_PATTERN = Setting.simpleString(
"plugins.index_state_management.restricted_index_pattern",
DEFAULT_RESTRICTED_PATTERN,
LegacyOpenDistroManagedIndexSettings.RESTRICTED_INDEX_PATTERN,
Setting.Property.NodeScope,
Setting.Property.Dynamic
)
Expand Down
2 changes: 0 additions & 2 deletions .../org/opensearch/indexmanagement/indexstatemanagement/step/shrink/AttemptMoveShardsStep.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import org.opensearch.indexmanagement.spi.indexstatemanagement.model.ShrinkActio
import org.opensearch.indexmanagement.spi.indexstatemanagement.model.StepContext
import org.opensearch.indexmanagement.spi.indexstatemanagement.model.StepMetaData
import org.opensearch.indices.InvalidIndexNameException
import org.opensearch.jobscheduler.repackage.com.cronutils.utils.VisibleForTesting
import org.opensearch.jobscheduler.spi.LockModel
import org.opensearch.jobscheduler.spi.utils.LockService
import org.opensearch.script.Script
Expand Down Expand Up @@ -289,7 +288,6 @@ class AttemptMoveShardsStep(private val action: ShrinkAction) : Step(name) {
/*
* Returns the list of node names for nodes with enough space to shrink to, in increasing order of space available
*/
@VisibleForTesting
@SuppressWarnings("NestedBlockDepth", "ComplexMethod")
private suspend fun findSuitableNodes(
stepContext: StepContext,
Expand Down
22 changes: 22 additions & 0 deletions src/test/kotlin/org/opensearch/indexmanagement/IndexManagementSettingsTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class IndexManagementSettingsTests : OpenSearchTestCase() {
LegacyOpenDistroManagedIndexSettings.HISTORY_NUMBER_OF_REPLICAS,
LegacyOpenDistroManagedIndexSettings.POLICY_ID,
LegacyOpenDistroManagedIndexSettings.ROLLOVER_ALIAS,
LegacyOpenDistroManagedIndexSettings.ROLLOVER_SKIP,
LegacyOpenDistroManagedIndexSettings.INDEX_STATE_MANAGEMENT_ENABLED,
LegacyOpenDistroManagedIndexSettings.METADATA_SERVICE_ENABLED,
LegacyOpenDistroManagedIndexSettings.JOB_INTERVAL,
Expand All @@ -46,6 +47,7 @@ class IndexManagementSettingsTests : OpenSearchTestCase() {
LegacyOpenDistroManagedIndexSettings.COORDINATOR_BACKOFF_MILLIS,
LegacyOpenDistroManagedIndexSettings.ALLOW_LIST,
LegacyOpenDistroManagedIndexSettings.SNAPSHOT_DENY_LIST,
LegacyOpenDistroManagedIndexSettings.RESTRICTED_INDEX_PATTERN,
LegacyOpenDistroRollupSettings.ROLLUP_INGEST_BACKOFF_COUNT,
LegacyOpenDistroRollupSettings.ROLLUP_INGEST_BACKOFF_MILLIS,
LegacyOpenDistroRollupSettings.ROLLUP_SEARCH_BACKOFF_COUNT,
Expand Down Expand Up @@ -110,6 +112,23 @@ class IndexManagementSettingsTests : OpenSearchTestCase() {
assertEquals(LegacyOpenDistroManagedIndexSettings.JOB_INTERVAL.get(settings), 5)
}

fun testIndexSettingLegacyFallback() {
var settings = Settings.builder()
.put("index.opendistro.index_state_management.rollover_skip", true)
.build()
assertEquals(ManagedIndexSettings.ROLLOVER_SKIP.get(settings), true)

settings = Settings.builder()
.put("index.opendistro.index_state_management.rollover_skip", true)
.put("index.plugins.index_state_management.rollover_skip", false)
.build()
assertEquals(ManagedIndexSettings.ROLLOVER_SKIP.get(settings), false)

assertSettingDeprecationsAndWarnings(
arrayOf(LegacyOpenDistroManagedIndexSettings.ROLLOVER_SKIP)
)
}

fun testSettingsGetValueWithLegacyFallback() {
val settings = Settings.builder()
.put("opendistro.index_state_management.enabled", false)
Expand All @@ -127,6 +146,7 @@ class IndexManagementSettingsTests : OpenSearchTestCase() {
.put("opendistro.index_state_management.history.number_of_replicas", 2)
.putList("opendistro.index_state_management.allow_list", listOf("1"))
.putList("opendistro.index_state_management.snapshot.deny_list", listOf("1"))
.put("opendistro.index_state_management.restricted_index_pattern", "blocked_index_pattern")
.put("opendistro.rollup.enabled", false)
.put("opendistro.rollup.search.enabled", false)
.put("opendistro.rollup.ingest.backoff_millis", "1ms")
Expand All @@ -151,6 +171,7 @@ class IndexManagementSettingsTests : OpenSearchTestCase() {
assertEquals(ManagedIndexSettings.HISTORY_NUMBER_OF_REPLICAS.get(settings), 2)
assertEquals(ManagedIndexSettings.ALLOW_LIST.get(settings), listOf("1"))
assertEquals(ManagedIndexSettings.SNAPSHOT_DENY_LIST.get(settings), listOf("1"))
assertEquals(ManagedIndexSettings.RESTRICTED_INDEX_PATTERN.get(settings), "blocked_index_pattern")
assertEquals(RollupSettings.ROLLUP_ENABLED.get(settings), false)
assertEquals(RollupSettings.ROLLUP_SEARCH_ENABLED.get(settings), false)
assertEquals(RollupSettings.ROLLUP_SEARCH_ALL_JOBS.get(settings), false)
Expand All @@ -177,6 +198,7 @@ class IndexManagementSettingsTests : OpenSearchTestCase() {
LegacyOpenDistroManagedIndexSettings.HISTORY_NUMBER_OF_REPLICAS,
LegacyOpenDistroManagedIndexSettings.ALLOW_LIST,
LegacyOpenDistroManagedIndexSettings.SNAPSHOT_DENY_LIST,
LegacyOpenDistroManagedIndexSettings.RESTRICTED_INDEX_PATTERN,
LegacyOpenDistroRollupSettings.ROLLUP_ENABLED,
LegacyOpenDistroRollupSettings.ROLLUP_SEARCH_ENABLED,
LegacyOpenDistroRollupSettings.ROLLUP_INGEST_BACKOFF_MILLIS,
Expand Down