Implementation of allow actions in red cluster feature

[skumarp7]

Add opt-in setting to run ISM jobs on a red cluster

Problem

ISM does not execute managed-index jobs while the cluster health is red. When a
cluster turns red (for example due to disk pressure), delete policies that would
free up space never run, so the cluster cannot recover on its own without manual
intervention (issue #1127).

Approach

Introduce an opt-in, dynamic cluster setting that lets operators allow ISM to keep
running while the cluster is red, while still protecting the cluster from actions
that would make a red cluster worse.

• New dynamic setting plugins.index_state_management.allow_running_on_red_cluster
  (NodeScope, Dynamic, default false) so existing behavior is unchanged and it can
  be toggled at runtime. It is registered in IndexManagementPlugin and tracked live
  in ManagedIndexRunner via addSettingsUpdateConsumer.

• Gate the existing red-cluster short-circuit in ManagedIndexRunner so jobs are
  skipped on a red cluster only when the setting is false (today's behavior). When
  the setting is true, execution proceeds.

• Safety guard: even when the override is enabled, a curated set of actions that
  create indices, require shard allocation, or add heavy I/O (force_merge,
  replica_count, shrink, snapshot, rollover, open, allocation,
  convert_index_to_remote, transform, rollup) are still skipped on a red cluster,
  because running them could further degrade an already red cluster. Recovery
  oriented actions such as delete continue to run so policies can help the cluster
  recover. This is expressed via RED_CLUSTER_RESTRICTED_ACTIONS and
  isActionAllowedOnRedCluster() in ManagedIndexSettings.

Tests

• ManagedIndexSettingsTests: default value, dynamic node-scope properties, and the
  allowed vs. restricted action classification (delete allowed, resource-intensive
  actions restricted).
• IndexManagementSettingsTests: the new setting is returned by the plugin.
• ManagedIndexRunnerTests: registers the new setting so consumer wiring resolves.
• ManagedIndexRunnerIT: the setting round-trips through the cluster settings API.

Relates to #1127

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 656bb29)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Inconsistent Case Check RED_CLUSTER_ALLOWED_ACTIONS is populated with lowercased names, but the assertions like RED_CLUSTER_ALLOWED_ACTIONS.contains(DeleteAction.name) will fail if any action's name constant contains uppercase characters. The test relies on all action name constants being already lowercase. If any action name contains uppercase letters (e.g., camelCase), the set-contains assertion will fail while isActionAllowedOnRedCluster (which lowercases the input) would still return true. Consider comparing against DeleteAction.name.lowercase() to keep tests aligned with the storage convention. assertTrue("$action should be allowed on a red cluster", ManagedIndexSettings.isActionAllowedOnRedCluster(action)) assertTrue("$action should be in the allowed set", ManagedIndexSettings.RED_CLUSTER_ALLOWED_ACTIONS.contains(action)) } Test Setting Reset Value In cleanupRedClusterTest, the setting is reset by sending the string "null" as the value with escapeValue = false. Depending on how updateClusterSetting renders this, it may set the persistent setting to the literal string "null" rather than removing/resetting it, which could leak state to subsequent tests. Verify that this produces a proper JSON null (setting removal) and not a string value. private fun cleanupRedClusterTest(indexName: String) { try { deleteRedHealthTrigger(indexName) } finally { updateClusterSetting(ManagedIndexSettings.ALLOW_RUNNING_ON_RED_CLUSTER.key, "null", escapeValue = false) } }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 656bb29

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Keep allowed-action set consistent with lookup The test test recovery oriented actions are allowed on red cluster asserts RED_CLUSTER_ALLOWED_ACTIONS.contains(action) using the raw action names, but the set stores lowercased names. If any action's name contains uppercase characters, this contains-check will fail. Either store the original names (and lowercase only in the lookup function) or update the tests to compare against lowercased names to keep both consistent. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/settings/ManagedIndexSettings.kt [38-49] val RED_CLUSTER_ALLOWED_ACTIONS: Set<String> = setOf( DeleteAction.name, CloseAction.name, ReadOnlyAction.name, NotificationAction.name, IndexPriorityAction.name, AliasAction.name, StopReplicationAction.name, SearchOnlyAction.name, TransitionsAction.name, - ).map { it.lowercase() }.toSet() + ) +fun isActionAllowedOnRedCluster(actionType: String?): Boolean = + actionType == null || RED_CLUSTER_ALLOWED_ACTIONS.any { it.equals(actionType, ignoreCase = true) } + Suggestion importance[1-10]: 6 __ Why: Valid observation: the set stores lowercased names while the test asserts contains(action) with the raw action.name. If any action name contains uppercase characters, the test would fail. This is a legitimate consistency concern between the implementation and tests.	Low
General	Improve visibility when skipping restricted action When action is null, isActionAllowedOnRedCluster(null) returns true, so nothing changes there; but when the action is non-null and restricted, this early-return silently skips execution without updating any metadata or retry info. Consider surfacing this as an info-level log (rather than debug) and/or recording an info message in the managed index metadata so operators can understand why a policy is stalled on a red cluster. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexRunner.kt [346-351] if (isClusterRed && !isActionAllowedOnRedCluster(action?.type)) { - logger.debug( + logger.info( "Skipping action=${action?.type} for ${managedIndexConfig.index} because it is not allowed to run on a red cluster", ) return } Suggestion importance[1-10]: 3 __ Why: A minor readability/observability improvement suggesting an info-level log instead of debug. It's a stylistic recommendation with low impact.	Low

---

Previous suggestions

Suggestions up to commit 5cf9135

Category	Suggestion	Impact
General	Align set contents with lookup casing The test test recovery oriented actions are allowed on red cluster asserts RED_CLUSTER_ALLOWED_ACTIONS.contains(action) using the original action names, but the set stores lowercased values. If any action's name contains uppercase characters, the test will fail and the runtime check will still work only because isActionAllowedOnRedCluster also lowercases input. Either drop the .lowercase() normalization (since action names are already canonical) or update the tests to compare against lowercased names, to keep the API self-consistent. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/settings/ManagedIndexSettings.kt [38-49] val RED_CLUSTER_ALLOWED_ACTIONS: Set<String> = setOf( DeleteAction.name, CloseAction.name, ReadOnlyAction.name, NotificationAction.name, IndexPriorityAction.name, AliasAction.name, StopReplicationAction.name, SearchOnlyAction.name, TransitionsAction.name, - ).map { it.lowercase() }.toSet() + ) Suggestion importance[1-10]: 6 __ Why: Valid concern: the test asserts RED_CLUSTER_ALLOWED_ACTIONS.contains(action) against original (potentially non-lowercased) names, while the set stores lowercased values. This is a real inconsistency between the API and its tests that could cause test failures if any action name contains uppercase characters.	Low
	Surface red-cluster skip reason in metadata Returning early here bypasses the subsequent !indexStateManagementEnabled disable-job logic and any metadata updates further down the method, which may leave the managed index in an inconsistent state (e.g. no info/message written to indicate why execution was skipped). Consider writing a metadata info message like the ALLOW_LIST skip path does, so users can see via the explain API that the action was skipped due to red cluster health rather than silently returning. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexRunner.kt [346-351] if (isClusterRed && !isActionAllowedOnRedCluster(action?.type)) { logger.debug( "Skipping action=${action?.type} for ${managedIndexConfig.index} because it is not allowed to run on a red cluster", ) + val info = mapOf("message" to "Skipping action=${action?.type} because it is not allowed to run on a red cluster") + val updatedMetadata = managedIndexMetaData.copy(info = info) + managedIndexMetaData.let { updateManagedIndexMetaData(updatedMetadata) } return } Suggestion importance[1-10]: 5 __ Why: Reasonable UX improvement to surface the skip reason via explain API, but the suggested improved_code is speculative and may not compile as-is (references to updateManagedIndexMetaData and metadata copy semantics need verification). It's a nice-to-have rather than critical.	Low

Suggestions up to commit d179596

Category	Suggestion	Impact
Possible issue	Release lock before early-return on red cluster When the cluster is red and the action is not in the allow-list, the code returns early without releasing the job lock acquired earlier in runManagedIndexConfig. Compare with the earlier red-cluster short-circuit which returns before any lock is held. Ensure any acquired lock/context is released (e.g., via the same cleanup path used by other early returns) before returning to avoid leaking locks. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexRunner.kt [346-351] if (isClusterRed && !isActionAllowedOnRedCluster(action?.type)) { logger.debug( "Skipping action=${action?.type} for ${managedIndexConfig.index} because it is not allowed to run on a red cluster", ) + releaseLockForScheduledJob(jobContext, lock) return } Suggestion importance[1-10]: 6 __ Why: Potentially valid concern about leaking a lock on early return, but without visibility into whether the lock is acquired before this point in the method, the impact is uncertain. The suggestion references releaseLockForScheduledJob and lock variables not shown in the diff.	Low
General	Verify action name/type consistency for lookup The allow-list is stored lowercased but Action.type values may not be lowercase in all cases; consider also lowercasing the constants when building the set to guarantee consistent matching, and confirm the action name constants (e.g., DeleteAction.name) match the runtime action.type string. Otherwise valid actions may be incorrectly blocked on a red cluster. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/settings/ManagedIndexSettings.kt [56-57] +fun isActionAllowedOnRedCluster(actionType: String?): Boolean = + actionType == null || actionType.lowercase() in RED_CLUSTER_ALLOWED_ACTIONS - Suggestion importance[1-10]: 2 __ Why: The suggestion only asks to verify consistency and provides an improved_code identical to the existing_code, offering no actual code change.	Low

Suggestions up to commit 25efabd

Category	Suggestion	Impact
General	Use locale-independent lowercase for matching String.lowercase() uses the default locale, which can produce unexpected results in locales like Turkish (e.g., "I" -> "ı"), potentially breaking allow-list matching. Use lowercase(Locale.ROOT) for locale-independent comparison to ensure consistent behavior across environments. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/settings/ManagedIndexSettings.kt [38-57] val RED_CLUSTER_ALLOWED_ACTIONS: Set<String> = setOf( DeleteAction.name, CloseAction.name, ReadOnlyAction.name, NotificationAction.name, IndexPriorityAction.name, AliasAction.name, StopReplicationAction.name, SearchOnlyAction.name, TransitionsAction.name, - ).map { it.lowercase() }.toSet() + ).map { it.lowercase(Locale.ROOT) }.toSet() -/** - * Whether the given action type is safe to run while the cluster health is red. - * A null action (nothing to execute) is treated as allowed. Any action that is not present - * in [RED_CLUSTER_ALLOWED_ACTIONS] is treated as restricted. - */ fun isActionAllowedOnRedCluster(actionType: String?): Boolean = - actionType == null || actionType.lowercase() in RED_CLUSTER_ALLOWED_ACTIONS + actionType == null || actionType.lowercase(Locale.ROOT) in RED_CLUSTER_ALLOWED_ACTIONS Suggestion importance[1-10]: 5 __ Why: Valid concern about locale-dependent lowercase() behavior, though action names are ASCII so this is a defensive improvement rather than a critical bug fix.	Low
Possible issue	Release lock before early return on red cluster The red-cluster action gate returns early without releasing the job lock or updating metadata, which can leave the job in an inconsistent state and prevent progress once the cluster recovers. Consider releasing any acquired lock (or restructuring the check to occur before lock acquisition) so subsequent executions can proceed cleanly. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexRunner.kt [343-351] // When running on a red cluster (only possible when allowRunningOnRedCluster is enabled), some actions // are still blocked because executing them could further degrade an already red cluster. Recovery-oriented // actions such as delete are still allowed so policies can help the cluster recover on its own. if (isClusterRed && !isActionAllowedOnRedCluster(action?.type)) { logger.debug( "Skipping action=${action?.type} for ${managedIndexConfig.index} because it is not allowed to run on a red cluster", ) + releaseLockForScheduledJob(jobContext, lock) return } Suggestion importance[1-10]: 4 __ Why: The concern about job state consistency is potentially valid, but the suggested fix references releaseLockForScheduledJob and lock which are not visible in the diff context, and the existing red-cluster check earlier in the method also uses an early return without lock release, suggesting this pattern may already be handled elsewhere.	Low

Suggestions up to commit c7308c2

Category	Suggestion	Impact
Possible issue	Recheck cluster health before action gating Re-check cluster health here instead of relying on the value captured at the top of runManagedIndexConfig. Between the initial clusterIsRed() check and this point, getActionToExecute may have made cluster calls, so using a stale value can either skip actions on a cluster that has since recovered, or run disallowed actions on a cluster that just turned red. Additionally, guard the branch on action != null so the runner can still progress steps like transitions when there is no action to execute. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexRunner.kt [346-351] -if (isClusterRed && !isActionAllowedOnRedCluster(action?.type)) { +if (action != null && clusterIsRed() && !isActionAllowedOnRedCluster(action.type)) { logger.debug( - "Skipping action=${action?.type} for ${managedIndexConfig.index} because it is not allowed to run on a red cluster", + "Skipping action=${action.type} for ${managedIndexConfig.index} because it is not allowed to run on a red cluster", ) return } Suggestion importance[1-10]: 5 __ Why: The point about a null action is reasonable since isActionAllowedOnRedCluster(null) returns true anyway, so the added null guard is minor. Re-checking cluster health is a valid concern about staleness, though the impact is limited and rechecking adds overhead.	Low
General	Reconsider allowing close on red cluster Including CloseAction in the red-cluster allow-list is risky: closing an index while the cluster is red can prevent recovery of unassigned shards belonging to that index and may leave data inaccessible. Consider removing CloseAction from the allowed set unless there is a strong recovery-oriented justification, as closing indices generally makes red-state recovery harder rather than easier. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/settings/ManagedIndexSettings.kt [38-49] val RED_CLUSTER_ALLOWED_ACTIONS: Set<String> = setOf( DeleteAction.name, - CloseAction.name, ReadOnlyAction.name, NotificationAction.name, IndexPriorityAction.name, AliasAction.name, StopReplicationAction.name, SearchOnlyAction.name, TransitionsAction.name, ).map { it.lowercase() }.toSet() Suggestion importance[1-10]: 4 __ Why: The suggestion raises a valid concern about CloseAction on a red cluster, but the safety of closing indices during red state is debatable and depends on operational context. This is more of an opinion/design discussion than a clear bug.	Low

Suggestions up to commit f71095e

Category	Suggestion	Impact
Possible issue	Fix inconsistent casing in allowed-actions set The allow-set is stored lowercased, but the test test recovery oriented actions are allowed on red cluster asserts RED_CLUSTER_ALLOWED_ACTIONS.contains(action) using the raw Action.name. If any action name contains uppercase characters, that assertion will fail and, more importantly, the lookup semantics are inconsistent. Either keep the set with original casing and lowercase both sides in the lookup, or ensure names are already lowercase and drop the .map { it.lowercase() }. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/settings/ManagedIndexSettings.kt [39-59] val RED_CLUSTER_ALLOWED_ACTIONS: Set<String> = setOf( DeleteAction.name, CloseAction.name, ReadOnlyAction.name, ReadWriteAction.name, NotificationAction.name, IndexPriorityAction.name, AliasAction.name, StopReplicationAction.name, SearchOnlyAction.name, TransitionsAction.name, - ).map { it.lowercase() }.toSet() + ) -/** - * Whether the given action type is safe to run while the cluster health is red. - * A null action (nothing to execute) is treated as allowed. Any action that is not present - * in [RED_CLUSTER_ALLOWED_ACTIONS] is treated as restricted. - */ fun isActionAllowedOnRedCluster(actionType: String?): Boolean = - actionType == null || actionType.lowercase() in RED_CLUSTER_ALLOWED_ACTIONS + actionType == null || RED_CLUSTER_ALLOWED_ACTIONS.any { it.equals(actionType, ignoreCase = true) } Suggestion importance[1-10]: 6 __ Why: Valid observation: the set is stored lowercased, but the test asserts RED_CLUSTER_ALLOWED_ACTIONS.contains(action) with raw action names. If any action name has uppercase letters, the test would fail, and the semantics are inconsistent. This is a legitimate consistency issue, though in practice most action names are already lowercase.	Low
General	Surface skip reason via metadata update Returning early here skips updating the managed index metadata, which can leave the managed index in a stuck/inconsistent state (e.g. no info message explaining why nothing is happening, and retry/step state remains unchanged). Consider publishing an informational ManagedIndexMetaData update (similar to other early-return paths in this method) so users can see that the action was skipped due to red cluster health, instead of silently returning. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexRunner.kt [343-351] -val action: Action? = state?.getActionToExecute(managedIndexMetaData, indexMetadataProvider) -val stepContext = - StepContext( - managedIndexMetaData, clusterService, client, threadPool.threadContext, policy.user, scriptService, settings, jobContext.lockService, - ) -val step: Step? = action?.getStepToExecute(stepContext) -val currentActionMetaData = action?.getUpdatedActionMetadata(managedIndexMetaData, state.name) - -// When running on a red cluster (only possible when allowRunningOnRedCluster is enabled), some actions -// are still blocked because executing them could further degrade an already red cluster. Recovery-oriented -// actions such as delete are still allowed so policies can help the cluster recover on its own. if (isClusterRed && !isActionAllowedOnRedCluster(action?.type)) { logger.debug( "Skipping action=${action?.type} for ${managedIndexConfig.index} because it is not allowed to run on a red cluster", ) + val info = mapOf("message" to "Skipping action [${action?.type}] because cluster health is red") + val updatedMetaData = managedIndexMetaData.copy(info = info) + if (!updateManagedIndexMetaData(updatedMetaData).metadataSaved) { + logger.error("Failed to update managed index metadata for ${managedIndexConfig.index} while skipping on red cluster") + } return } Suggestion importance[1-10]: 5 __ Why: Reasonable UX improvement to surface the skip reason in managed index metadata, but the improved code references updateManagedIndexMetaData without verifying it exists in this scope, and silent skipping matches the existing pattern for red cluster skipping. Moderate impact.	Low
	Document red-cluster snapshot semantics clusterIsRed() is called once and its value is reused later for the per-action gate, but cluster health can change during a long run and, more importantly, computing it up-front is fine only if the value is truly needed later. Ensure the variable is passed into runManagedIndexConfig scope correctly (it already is) — however, consider recomputing (or at minimum re-checking) closer to the action-allow gate to avoid acting on stale state if the runner logic is refactored to be longer-running. At the very least, add a comment noting that isClusterRed is a snapshot to prevent future regressions. src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexRunner.kt [272-276] +// Snapshot of cluster health taken once per run; reused below to gate individual actions +// so we don't hammer the cluster manager with repeated health lookups. val isClusterRed = clusterIsRed() if (isClusterRed && !allowRunningOnRedCluster) { logger.debug("Skipping current execution of ${managedIndexConfig.index} because of red cluster health") return } Suggestion importance[1-10]: 2 __ Why: Only suggests adding a clarifying comment; low impact and mostly cosmetic.	Low

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.96%. Comparing base (aed42e5) to head (f71095e).

Files with missing lines	Patch %	Lines
...agement/indexstatemanagement/ManagedIndexRunner.kt	44.44%	3 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1691      +/-   ##
==========================================
+ Coverage   76.85%   76.96%   +0.10%     
==========================================
  Files         388      388              
  Lines       18655    18684      +29     
  Branches     2619     2622       +3     
==========================================
+ Hits        14338    14380      +42     
+ Misses       3024     3011      -13     
  Partials     1293     1293              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[skumarp7]

Hi @bowenlan-amzn,

Please provide feedback :)

[skumarp7]

Hi @vikasvb90 , @bowenlan-amzn , @Tarun-kishore

Any feedback on the PR is welcomed :)

[shiv0408]

I would rather have a list of allowed action and block all rest. Otherwise, any new action that is added would be allowed to run on red cluster by default.

[skumarp7]

Done.

[github-actions]

Persistent review updated to latest commit f71095e

[Tarun-kishore]

moving index to read_write state can potentially degrade the cluster stability, as indexing load will be increased on an existing red cluster. Should we consider skipping it?

[skumarp7]

Yes, it makes sense to remove read_write from the allow-list. Although the action itself is lightweight, it re-enables indexing, which can increase disk, CPU, memory, and recovery pressure on an already red cluster. Since it is not recovery-oriented, skipping it is the safer behavior.

[skumarp7]

@Tarun-kishore , have pushed a new commit with the changes. Please share your comments.

[github-actions]

Persistent review updated to latest commit c7308c2

[Tarun-kishore]

New changes lgtm.

[Tarun-kishore]

@skumarp7 Can you fix the CI failures.

[github-actions]

Persistent review updated to latest commit 25efabd

[github-actions]

Persistent review updated to latest commit d179596

[skumarp7]

@Tarun-kishore

@skumarp7 Can you fix the CI failures.

The CI seems to be failing due to a Kotlin compilation error in TransformRestTestCase.kt. The issue is with the deprecated RestClient class from the Java/OpenSearch client library. Should i supress the deprecation warnings ?

[Tarun-kishore]

I checked the failure as well, those are because of RestClient deprecation warning. Since those are unrelated to this change, created a separate PR for that: #1702.

[Tarun-kishore]

@skumarp7 Can you add ITs for these changes.

[github-actions]

Persistent review updated to latest commit 5cf9135

[github-actions]

Persistent review updated to latest commit 656bb29

[skumarp7]

@Tarun-kishore,

@skumarp7 Can you add ITs for these changes.

Have added some ITs, let me know if they are okay?