Error prevention stage 1 #579

Angie-Zhang · 2022-10-27T17:29:58Z

Issue #, if available:
#432

Description of changes:

Added error prevention basic framework
Added action validation for:

Rollover
Force merge
Replica count
Open
Read only
Read write

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Joanne Wang <jowg@amazon.com>

…situations (opensearch-project#419) Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Signed-off-by: Joanne Wang <jowg@amazon.com>

Signed-off-by: Joanne Wang <jowg@amazon.com>

…integration tests Signed-off-by: Joanne Wang <jowg@amazon.com>

Signed-off-by: Joanne Wang <jowg@amazon.com>

...in/kotlin/org/opensearch/indexmanagement/indexstatemanagement/validation/ValidateReadOnly.kt

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/validation/ValidateOpen.kt

...main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/validation/ValidateDelete.kt

deshsidd · 2022-10-27T21:22:42Z

@petardz Please review if you get the time. Thanks!

...main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/validation/ValidateDelete.kt

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/validation/ValidateOpen.kt

deshsidd · 2022-10-27T22:22:29Z

Let's consider disabling the feature by default in the first iteration and enable it in the future releases as discussed. Thanks!

deshsidd · 2022-10-27T22:41:56Z

.../kotlin/org/opensearch/indexmanagement/indexstatemanagement/settings/ManagedIndexSettings.kt

@@ -14,6 +14,7 @@ import java.util.function.Function
 class ManagedIndexSettings {
    companion object {
        const val DEFAULT_ISM_ENABLED = true
+        const val DEFAULT_VALIDATION_SERVICE_ENABLED = true


@bowenlan-amzn @khushbr
We are considering to set the validation service to disabled by default for 2.4. Please let us know your thoughts?

Few questions here:

Is the Validation operation blocking / non-blocking for ISM actions ? On high level, if there are bugs/false positives from validation, will it prevent the ISM action ?

Is there a kill switch to disable the Validation framework ?

I would prefer going "enabled" by default; given we have enough confidence in the code/feature.

@khushbr

It will block checked ISM actions if the validation fails.

We use a parameter to enable/disable it.

For 2.4 let's keep it disable by default. I don't see we are actually refactoring out the validation logic from the step execution but just duplicate the validation logic. This is the first step of this framework.
We can target 2.5 to enable by default

Signed-off-by: Angie Zhang <langelzh@amazon.com>

src/main/kotlin/org/opensearch/indexmanagement/IndexManagementPlugin.kt

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/util/RestHandlerUtils.kt

bowenlan-amzn · 2022-11-01T17:36:56Z

.../kotlin/org/opensearch/indexmanagement/indexstatemanagement/settings/ManagedIndexSettings.kt

@@ -14,6 +14,7 @@ import java.util.function.Function
 class ManagedIndexSettings {
    companion object {
        const val DEFAULT_ISM_ENABLED = true
+        const val DEFAULT_VALIDATION_SERVICE_ENABLED = true


For 2.4 let's keep it disable by default. I don't see we are actually refactoring out the validation logic from the step execution but just duplicate the validation logic. This is the first step of this framework.
We can target 2.5 to enable by default

...n/kotlin/org/opensearch/indexmanagement/indexstatemanagement/validation/ValidationService.kt

bowenlan-amzn · 2022-11-01T17:44:41Z

...in/kotlin/org/opensearch/indexmanagement/indexstatemanagement/validation/ValidateRollover.kt

+    private fun skipRollover(indexName: String): Boolean {
+        val skipRollover = clusterService.state().metadata.index(indexName).getRolloverSkip()
+        if (skipRollover) {
+            validationStatus = ValidationStatus.PASSED


We need another validationStatus for this kind of skipping logic. PASSED means ISM can continue to execute step, but here ISM actually should not execute step but directly skip executing.

bowenlan-amzn · 2022-11-01T17:51:55Z

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexRunner.kt

+                val validationResult = validationService.validate(action.type, stepContext.metadata.index)
+                if (validationResult.validationStatus == Validate.ValidationStatus.RE_VALIDATING) {
+                    logger.warn("Validation Status is: RE_VALIDATING")
+                    publishErrorNotification(policy, managedIndexMetaData)


This is not gonna publish notification for validation message out of the box.
Need some modification to make it publish validationResult.validationMessage

TODO in next PR or release

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexRunner.kt

bowenlan-amzn · 2022-11-01T18:01:00Z

...arch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt

@@ -115,8 +119,10 @@ class TransportExplainAction @Inject constructor(
        private val enabledState: MutableMap<IndexName, Boolean> = mutableMapOf()
        private val indexPolicyIDs = mutableListOf<PolicyID?>()
        private val indexMetadatas = mutableListOf<ManagedIndexMetaData?>()
+        private val validationResults = mutableListOf<ValidationResult?>()


Prefer using a map here, managed index: validation result

bowenlan-amzn · 2022-11-01T18:03:29Z

Provide examples of how user will see the validation results in the description

Signed-off-by: Angie Zhang <langelzh@amazon.com>

codecov-commenter · 2022-11-02T06:07:15Z

Codecov Report

Merging #579 (b455ce6) into main (63984b2) will decrease coverage by 0.56%.
The diff coverage is 51.43%.

@@             Coverage Diff              @@
##               main     #579      +/-   ##
============================================
- Coverage     75.84%   75.28%   -0.57%     
- Complexity     2482     2545      +63     
============================================
  Files           316      328      +12     
  Lines         14551    14940     +389     
  Branches       2250     2301      +51     
============================================
+ Hits          11036    11247     +211     
- Misses         2251     2381     +130     
- Partials       1264     1312      +48

Impacted Files	Coverage Δ
...ment/indexstatemanagement/util/RestHandlerUtils.kt	`88.88% <ø> (ø)`
...statemanagement/validation/ValidateReplicaCount.kt	`0.00% <0.00%> (ø)`
...agement/indexstatemanagement/ManagedIndexRunner.kt	`45.55% <29.41%> (-0.64%)`	⬇️
...nt/indexstatemanagement/validation/ValidateOpen.kt	`29.62% <29.62%> (ø)`
...ndexstatemanagement/validation/ValidateReadOnly.kt	`44.00% <44.00%> (ø)`
.../indexstatemanagement/validation/ValidateDelete.kt	`47.05% <47.05%> (ø)`
...dexstatemanagement/validation/ValidateReadWrite.kt	`50.00% <50.00%> (ø)`
...exstatemanagement/validation/ValidateForceMerge.kt	`52.94% <52.94%> (ø)`
...transport/action/explain/TransportExplainAction.kt	`70.75% <58.62%> (-1.59%)`	⬇️
...ndexstatemanagement/validation/ValidateRollover.kt	`66.66% <66.66%> (ø)`
... and 23 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Angie Zhang <langelzh@amazon.com>

* initial framework Signed-off-by: Joanne Wang <jowg@amazon.com> * Removed recursion from Explain Action to avoid stackoverflow in some situations (#419) Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Signed-off-by: Joanne Wang <jowg@amazon.com> * enabled by default integrated Signed-off-by: Joanne Wang <jowg@amazon.com> * cleaned up comments and logs, created unit test and updated previous integration tests Signed-off-by: Joanne Wang <jowg@amazon.com> * added delete validation logic Signed-off-by: Joanne Wang <jowg@amazon.com> * fixed rollover validation unit tests Signed-off-by: Joanne Wang <jowg@amazon.com> * added validation info field to ManagedIndexMetaData Signed-off-by: Joanne Wang <jowg@amazon.com> * removed step context as input Signed-off-by: Joanne Wang <jowg@amazon.com> * added validationmetadata class Signed-off-by: Joanne Wang <jowg@amazon.com> * restored old integration tests and changed validation service output Signed-off-by: Joanne Wang <jowg@amazon.com> * before integrated validation meta data into managed index meta data Signed-off-by: Joanne Wang <jowg@amazon.com> * integrated validation meta data Signed-off-by: Joanne Wang <jowg@amazon.com> * working version Signed-off-by: Joanne Wang <jowg@amazon.com> * added validation mapping Signed-off-by: Joanne Wang <jowg@amazon.com> * fixed integ tests Signed-off-by: Joanne Wang <jowg@amazon.com> * renamed some values Signed-off-by: Joanne Wang <jowg@amazon.com> * before removing from managed index meta data Signed-off-by: Joanne Wang <jowg@amazon.com> * created validation result object in explain Signed-off-by: Joanne Wang <jowg@amazon.com> * testing Signed-off-by: Joanne Wang <jowg@amazon.com> * run fails Signed-off-by: Joanne Wang <jowg@amazon.com> * integration test for delete + added framework for force merge Signed-off-by: Joanne Wang <jowg@amazon.com> * removed step validation metadata and still testing explain results Signed-off-by: Joanne Wang <jowg@amazon.com> * before removing from managed index runner Signed-off-by: Joanne Wang <jowg@amazon.com> * removed from managed index runner Signed-off-by: Joanne Wang <jowg@amazon.com> * clean up and tests Signed-off-by: Joanne Wang <jowg@amazon.com> * all validation tests pass Signed-off-by: Joanne Wang <jowg@amazon.com> * removed validation result from all managed index meta data Signed-off-by: Joanne Wang <jowg@amazon.com> * restored old IT tests Signed-off-by: Joanne Wang <jowg@amazon.com> * fixed it tests, set explain validation to false Signed-off-by: Joanne Wang <jowg@amazon.com> * clean up Signed-off-by: Joanne Wang <jowg@amazon.com> * Implemented replica_count, open, read_only, read_write Signed-off-by: Angie Zhang <langelzh@amazon.com> * Implemented replica_count, open, read_only, read_write Signed-off-by: Angie Zhang <langelzh@amazon.com> * Fix Test cases Signed-off-by: Angie Zhang <langelzh@amazon.com> * Fix Test cases Signed-off-by: Angie Zhang <langelzh@amazon.com> * Fix messages Signed-off-by: Angie Zhang <langelzh@amazon.com> * Fix comments; set validation disabled by default Signed-off-by: Angie Zhang <langelzh@amazon.com> * Rename validation_service to action_validation; Fix some detekt issues Signed-off-by: Angie Zhang <langelzh@amazon.com> Signed-off-by: Joanne Wang <jowg@amazon.com> Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Signed-off-by: Angie Zhang <langelzh@amazon.com> Co-authored-by: Joanne Wang <jowg@amazon.com> Co-authored-by: Petar <petar.dzepina@gmail.com> Co-authored-by: Joanne Wang <109310487+jowg-amazon@users.noreply.github.com> (cherry picked from commit 2438f48)

* Error prevention stage 1 * Removed recursion from Explain Action to avoid stackoverflow in some situations (#419) * enabled by default integrated * Fix Test cases * Fix comments; set validation disabled by default * Rename validation_service to action_validation; Fix some detekt issues Signed-off-by: Joanne Wang <jowg@amazon.com> Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Signed-off-by: Angie Zhang <langelzh@amazon.com> Co-authored-by: Joanne Wang <jowg@amazon.com> Co-authored-by: Petar <petar.dzepina@gmail.com> Co-authored-by: Angie Zhang <langelzh@amazon.com>

* Removed recursion from Explain Action to avoid stackoverflow in some situations (#419) * enabled by default integrated * Fix Test cases * Fix comments; set validation disabled by default * Rename validation_service to action_validation; Fix some detekt issues Signed-off-by: Joanne Wang <jowg@amazon.com> Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Signed-off-by: Angie Zhang <langelzh@amazon.com> Co-authored-by: Joanne Wang <jowg@amazon.com> Co-authored-by: Petar <petar.dzepina@gmail.com> Co-authored-by: Angie Zhang <langelzh@amazon.com>

…#591) * Error prevention stage 1 * Removed recursion from Explain Action to avoid stackoverflow in some situations (opensearch-project#419) * enabled by default integrated * Fix Test cases * Fix comments; set validation disabled by default * Rename validation_service to action_validation; Fix some detekt issues Signed-off-by: Joanne Wang <jowg@amazon.com> Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Signed-off-by: Angie Zhang <langelzh@amazon.com> Co-authored-by: Joanne Wang <jowg@amazon.com> Co-authored-by: Petar <petar.dzepina@gmail.com> Co-authored-by: Angie Zhang <langelzh@amazon.com>

…#591) * Error prevention stage 1 * Removed recursion from Explain Action to avoid stackoverflow in some situations (opensearch-project#419) * enabled by default integrated * Fix Test cases * Fix comments; set validation disabled by default * Rename validation_service to action_validation; Fix some detekt issues Signed-off-by: Joanne Wang <jowg@amazon.com> Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Signed-off-by: Angie Zhang <langelzh@amazon.com> Co-authored-by: Joanne Wang <jowg@amazon.com> Co-authored-by: Petar <petar.dzepina@gmail.com> Co-authored-by: Angie Zhang <langelzh@amazon.com> Signed-off-by: Ronnak Saxena <ronsax@amazon.com>

jowg-amazon and others added 30 commits August 18, 2022 23:13

initial framework

8bae0e3

Signed-off-by: Joanne Wang <jowg@amazon.com>

Removed recursion from Explain Action to avoid stackoverflow in some …

60edc73

…situations (opensearch-project#419) Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Signed-off-by: Joanne Wang <jowg@amazon.com>

enabled by default integrated

0060bf0

Signed-off-by: Joanne Wang <jowg@amazon.com>

cleaned up comments and logs, created unit test and updated previous …

2aa150a

…integration tests Signed-off-by: Joanne Wang <jowg@amazon.com>

added delete validation logic

8add902

Signed-off-by: Joanne Wang <jowg@amazon.com>

fixed rollover validation unit tests

9041646

Signed-off-by: Joanne Wang <jowg@amazon.com>

added validation info field to ManagedIndexMetaData

0cda31a

Signed-off-by: Joanne Wang <jowg@amazon.com>

removed step context as input

daa7cdb

Signed-off-by: Joanne Wang <jowg@amazon.com>

added validationmetadata class

acec69f

Signed-off-by: Joanne Wang <jowg@amazon.com>

restored old integration tests and changed validation service output

78eabc9

Signed-off-by: Joanne Wang <jowg@amazon.com>

before integrated validation meta data into managed index meta data

31d7d45

Signed-off-by: Joanne Wang <jowg@amazon.com>

integrated validation meta data

8515efd

Signed-off-by: Joanne Wang <jowg@amazon.com>

working version

3771eca

Signed-off-by: Joanne Wang <jowg@amazon.com>

added validation mapping

1492da9

Signed-off-by: Joanne Wang <jowg@amazon.com>

fixed integ tests

2905ddf

Signed-off-by: Joanne Wang <jowg@amazon.com>

renamed some values

aceaf52

Signed-off-by: Joanne Wang <jowg@amazon.com>

before removing from managed index meta data

c5cd789

Signed-off-by: Joanne Wang <jowg@amazon.com>

created validation result object in explain

e060642

Signed-off-by: Joanne Wang <jowg@amazon.com>

testing

28e8715

Signed-off-by: Joanne Wang <jowg@amazon.com>

run fails

5e5244c

Signed-off-by: Joanne Wang <jowg@amazon.com>

integration test for delete + added framework for force merge

e1ff73a

Signed-off-by: Joanne Wang <jowg@amazon.com>

removed step validation metadata and still testing explain results

ace4998

Signed-off-by: Joanne Wang <jowg@amazon.com>

before removing from managed index runner

2a1203e

Signed-off-by: Joanne Wang <jowg@amazon.com>

removed from managed index runner

b0fc77c

Signed-off-by: Joanne Wang <jowg@amazon.com>

clean up and tests

bfb0c39

Signed-off-by: Joanne Wang <jowg@amazon.com>

all validation tests pass

a3edbe9

Signed-off-by: Joanne Wang <jowg@amazon.com>

removed validation result from all managed index meta data

06cd2fc

Signed-off-by: Joanne Wang <jowg@amazon.com>

restored old IT tests

40e5f8e

Signed-off-by: Joanne Wang <jowg@amazon.com>

fixed it tests, set explain validation to false

71a2b9f

Signed-off-by: Joanne Wang <jowg@amazon.com>

clean up

9169b5b

Signed-off-by: Joanne Wang <jowg@amazon.com>