Skip to content

Commit

Permalink
Backport Issue #998 (#1067)
Browse files Browse the repository at this point in the history
* Backport issue #998 to 2.x

Signed-off-by: Joshua Au <[email protected]>

* Issue651 tests (#1046)

* Implemented filtering on the ISM eplain API

Signed-off-by: Joshua Au <[email protected]>

* Fixed tests for ExplainRequest

Signed-off-by: Joshua Au <[email protected]>

* Added filtering on query and metadata map

Signed-off-by: Joshua Au <[email protected]>

* Filtered on indexNames in metadata

Signed-off-by: Joshua Au <[email protected]>

* Fixed github workflow check errors

Signed-off-by: Joshua Au <[email protected]>

* Removed debugging comments

Signed-off-by: Joshua Au <[email protected]>

* Updated code styling to make more clear

Signed-off-by: Joshua Au <[email protected]>

* Refactored code to match suggestions

Signed-off-by: Joshua Au <[email protected]>

* Added test case for the ExplainFilter.byMetaData and parse methods

Signed-off-by: Joshua Au <[email protected]>

* Started implementation of explain filter IT

Signed-off-by: Joshua Au <[email protected]>

* Implemented test explain filter method

Signed-off-by: Joshua Au <[email protected]>

* Implemented explain filter test on failure

Signed-off-by: Joshua Au <[email protected]>

* Cleaned up log statements

Signed-off-by: Joshua Au <[email protected]>

* Added explain filter test for success

Signed-off-by: Joshua Au <[email protected]>

* Fixed lint errors

Signed-off-by: Joshua Au <[email protected]>

* Removed policy from index to fix flaky tests

Signed-off-by: Joshua Au <[email protected]>

---------

Signed-off-by: Joshua Au <[email protected]>
Signed-off-by: bowenlan-amzn <[email protected]>
Co-authored-by: bowenlan-amzn <[email protected]>

* Fixed content type import

Signed-off-by: Joshua Au <[email protected]>

* Implemented filtering on the ISM eplain API (#998)

* Implemented filtering on the ISM eplain API

Signed-off-by: Joshua Au <[email protected]>

* Fixed tests for ExplainRequest

Signed-off-by: Joshua Au <[email protected]>

* Added filtering on query and metadata map

Signed-off-by: Joshua Au <[email protected]>

* Filtered on indexNames in metadata

Signed-off-by: Joshua Au <[email protected]>

* Fixed github workflow check errors

Signed-off-by: Joshua Au <[email protected]>

* Removed debugging comments

Signed-off-by: Joshua Au <[email protected]>

* Updated code styling to make more clear

Signed-off-by: Joshua Au <[email protected]>

* Refactored code to match suggestions

Signed-off-by: Joshua Au <[email protected]>

---------

Signed-off-by: Joshua Au <[email protected]>
Co-authored-by: bowenlan-amzn <[email protected]>

* Issue651 tests (#1046)

* Implemented filtering on the ISM eplain API

Signed-off-by: Joshua Au <[email protected]>

* Fixed tests for ExplainRequest

Signed-off-by: Joshua Au <[email protected]>

* Added filtering on query and metadata map

Signed-off-by: Joshua Au <[email protected]>

* Filtered on indexNames in metadata

Signed-off-by: Joshua Au <[email protected]>

* Fixed github workflow check errors

Signed-off-by: Joshua Au <[email protected]>

* Removed debugging comments

Signed-off-by: Joshua Au <[email protected]>

* Updated code styling to make more clear

Signed-off-by: Joshua Au <[email protected]>

* Refactored code to match suggestions

Signed-off-by: Joshua Au <[email protected]>

* Added test case for the ExplainFilter.byMetaData and parse methods

Signed-off-by: Joshua Au <[email protected]>

* Started implementation of explain filter IT

Signed-off-by: Joshua Au <[email protected]>

* Implemented test explain filter method

Signed-off-by: Joshua Au <[email protected]>

* Implemented explain filter test on failure

Signed-off-by: Joshua Au <[email protected]>

* Cleaned up log statements

Signed-off-by: Joshua Au <[email protected]>

* Added explain filter test for success

Signed-off-by: Joshua Au <[email protected]>

* Fixed lint errors

Signed-off-by: Joshua Au <[email protected]>

* Removed policy from index to fix flaky tests

Signed-off-by: Joshua Au <[email protected]>

---------

Signed-off-by: Joshua Au <[email protected]>
Signed-off-by: bowenlan-amzn <[email protected]>
Co-authored-by: bowenlan-amzn <[email protected]>

* Backport explain filter tests

* Sleep thread to allow jobs to finish

Signed-off-by: Joshua Au <[email protected]>

---------

Signed-off-by: Joshua Au <[email protected]>
Signed-off-by: bowenlan-amzn <[email protected]>
Co-authored-by: bowenlan-amzn <[email protected]>
  • Loading branch information
Joshua152 and bowenlan-amzn authored Jan 4, 2024
1 parent e5997e8 commit d65545a
Show file tree
Hide file tree
Showing 12 changed files with 536 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.indexmanagement.indexstatemanagement.model

import org.opensearch.core.common.io.stream.StreamInput
import org.opensearch.core.common.io.stream.StreamOutput
import org.opensearch.core.common.io.stream.Writeable
import org.opensearch.core.xcontent.ToXContent
import org.opensearch.core.xcontent.ToXContentObject
import org.opensearch.core.xcontent.XContentBuilder
import org.opensearch.core.xcontent.XContentParser
import org.opensearch.core.xcontent.XContentParser.Token
import org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken
import org.opensearch.index.query.BoolQueryBuilder
import org.opensearch.index.query.QueryBuilders
import org.opensearch.indexmanagement.indexstatemanagement.util.MANAGED_INDEX_POLICY_ID_FIELD
import org.opensearch.indexmanagement.spi.indexstatemanagement.model.ManagedIndexMetaData
import java.io.IOException

data class ExplainFilter(
val policyID: String? = null,
val state: String? = null,
val actionType: String? = null,
val failed: Boolean? = null
) : ToXContentObject, Writeable {

@Throws(IOException::class)
constructor(sin: StreamInput) : this(
policyID = sin.readOptionalString(),
state = sin.readOptionalString(),
actionType = sin.readOptionalString(),
failed = sin.readOptionalBoolean()
)

override fun toXContent(builder: XContentBuilder, params: ToXContent.Params): XContentBuilder {
builder.startObject()
builder.startObject(FILTER_FIELD)

if (policyID != null) builder.field(POLICY_ID_FIELD, policyID)
if (state != null) builder.field(STATE_FIELD, state)
if (actionType != null) builder.field(ACTION_FIELD, actionType)
if (failed != null) builder.field(FAILED_FIELD, failed)

builder.endObject()
return builder.endObject()
}

@Throws(IOException::class)
override fun writeTo(out: StreamOutput) {
out.writeOptionalString(policyID)
out.writeOptionalString(state)
out.writeOptionalString(actionType)
out.writeOptionalBoolean(failed)
}

fun byMetaData(metaData: ManagedIndexMetaData): Boolean {
var isValid = true

val stateMetaData = metaData.stateMetaData
if (state != null && (stateMetaData == null || stateMetaData.name != state)) {
isValid = false
}

val actionMetaData = metaData.actionMetaData
if (actionType != null && (actionMetaData == null || actionMetaData.name != actionType)) {
isValid = false
}

val retryInfoMetaData = metaData.policyRetryInfo
val actionFailedNotValid = actionMetaData == null || actionMetaData.failed != failed
val retryFailedNotValid = retryInfoMetaData == null || retryInfoMetaData.failed != failed
if (failed != null && actionFailedNotValid && retryFailedNotValid) {
isValid = false
}

return isValid
}

companion object {
const val FILTER_FIELD = "filter"
const val POLICY_ID_FIELD = "policy_id"
const val STATE_FIELD = "state"
const val ACTION_FIELD = "action_type"
const val FAILED_FIELD = "failed"

@JvmStatic
@Throws(IOException::class)
fun parse(xcp: XContentParser): ExplainFilter {
var policyID: String? = null
var state: String? = null
var actionType: String? = null
var failed: Boolean? = null

ensureExpectedToken(Token.START_OBJECT, xcp.currentToken(), xcp)
while (xcp.nextToken() != Token.END_OBJECT) {
val fieldName = xcp.currentName()
xcp.nextToken()

when (fieldName) {
FILTER_FIELD -> {
ensureExpectedToken(Token.START_OBJECT, xcp.currentToken(), xcp)
while (xcp.nextToken() != Token.END_OBJECT) {
val filter = xcp.currentName()
xcp.nextToken()

when (filter) {
POLICY_ID_FIELD -> policyID = xcp.text()
STATE_FIELD -> state = xcp.text()
ACTION_FIELD -> actionType = xcp.text()
FAILED_FIELD -> failed = xcp.booleanValue()
}
}
}
}
}

return ExplainFilter(policyID, state, actionType, failed)
}
}
}

fun BoolQueryBuilder.filterByPolicyID(explainFilter: ExplainFilter?): BoolQueryBuilder {
if (explainFilter?.policyID != null) {
this.filter(QueryBuilders.termsQuery(MANAGED_INDEX_POLICY_ID_FIELD, explainFilter.policyID))
}

return this
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ import org.apache.logging.log4j.LogManager
import org.opensearch.client.node.NodeClient
import org.opensearch.core.common.Strings
import org.opensearch.common.logging.DeprecationLogger
import org.opensearch.core.xcontent.XContentParser.Token
import org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken
import org.opensearch.indexmanagement.IndexManagementPlugin.Companion.ISM_BASE_URI
import org.opensearch.indexmanagement.IndexManagementPlugin.Companion.LEGACY_ISM_BASE_URI
import org.opensearch.indexmanagement.indexstatemanagement.model.ExplainFilter
import org.opensearch.indexmanagement.indexstatemanagement.transport.action.explain.ExplainAction
import org.opensearch.indexmanagement.indexstatemanagement.transport.action.explain.ExplainRequest
import org.opensearch.indexmanagement.indexstatemanagement.util.DEFAULT_EXPLAIN_VALIDATE_ACTION
Expand All @@ -28,6 +31,7 @@ import org.opensearch.rest.RestHandler.ReplacedRoute
import org.opensearch.rest.RestHandler.Route
import org.opensearch.rest.RestRequest
import org.opensearch.rest.RestRequest.Method.GET
import org.opensearch.rest.RestRequest.Method.POST
import org.opensearch.rest.action.RestToXContentListener

private val log = LogManager.getLogger(RestExplainAction::class.java)
Expand All @@ -52,6 +56,14 @@ class RestExplainAction : BaseRestHandler() {
ReplacedRoute(
GET, "$EXPLAIN_BASE_URI/{index}",
GET, "$LEGACY_EXPLAIN_BASE_URI/{index}"
),
ReplacedRoute(
POST, EXPLAIN_BASE_URI,
POST, LEGACY_EXPLAIN_BASE_URI
),
ReplacedRoute(
POST, "$EXPLAIN_BASE_URI/{index}",
POST, "$LEGACY_EXPLAIN_BASE_URI/{index}"
)
)
}
Expand All @@ -69,6 +81,14 @@ class RestExplainAction : BaseRestHandler() {

val indexType = request.param(TYPE_PARAM_KEY, DEFAULT_INDEX_TYPE)

val explainFilter = if (request.method() == RestRequest.Method.POST) {
val xcp = request.contentParser()
ensureExpectedToken(Token.START_OBJECT, xcp.nextToken(), xcp)
ExplainFilter.parse(xcp)
} else {
null
}

val clusterManagerTimeout = parseClusterManagerTimeout(
request, DeprecationLogger.getLogger(RestExplainAction::class.java), name
)
Expand All @@ -78,6 +98,7 @@ class RestExplainAction : BaseRestHandler() {
request.paramAsBoolean("local", false),
clusterManagerTimeout,
searchParams,
explainFilter,
request.paramAsBoolean(SHOW_POLICY_QUERY_PARAM, DEFAULT_EXPLAIN_SHOW_POLICY),
request.paramAsBoolean(SHOW_VALIDATE_ACTION, DEFAULT_EXPLAIN_VALIDATE_ACTION),
indexType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import org.opensearch.core.common.io.stream.StreamInput
import org.opensearch.core.common.io.stream.StreamOutput
import org.opensearch.common.unit.TimeValue
import org.opensearch.indexmanagement.common.model.rest.SearchParams
import org.opensearch.indexmanagement.indexstatemanagement.model.ExplainFilter
import org.opensearch.indexmanagement.indexstatemanagement.util.DEFAULT_INDEX_TYPE
import java.io.IOException

Expand All @@ -21,6 +22,7 @@ class ExplainRequest : ActionRequest {
val local: Boolean
val clusterManagerTimeout: TimeValue
val searchParams: SearchParams
val explainFilter: ExplainFilter?
val showPolicy: Boolean
val validateAction: Boolean
val indexType: String
Expand All @@ -31,6 +33,7 @@ class ExplainRequest : ActionRequest {
local: Boolean,
clusterManagerTimeout: TimeValue,
searchParams: SearchParams,
explainFilter: ExplainFilter?,
showPolicy: Boolean,
validateAction: Boolean,
indexType: String
Expand All @@ -39,6 +42,7 @@ class ExplainRequest : ActionRequest {
this.local = local
this.clusterManagerTimeout = clusterManagerTimeout
this.searchParams = searchParams
this.explainFilter = explainFilter
this.showPolicy = showPolicy
this.validateAction = validateAction
this.indexType = indexType
Expand All @@ -50,6 +54,7 @@ class ExplainRequest : ActionRequest {
local = sin.readBoolean(),
clusterManagerTimeout = sin.readTimeValue(),
searchParams = SearchParams(sin),
explainFilter = sin.readOptionalWriteable(::ExplainFilter),
showPolicy = sin.readBoolean(),
validateAction = sin.readBoolean(),
indexType = sin.readString()
Expand All @@ -72,6 +77,7 @@ class ExplainRequest : ActionRequest {
out.writeBoolean(local)
out.writeTimeValue(clusterManagerTimeout)
searchParams.writeTo(out)
out.writeOptionalWriteable(explainFilter)
out.writeBoolean(showPolicy)
out.writeBoolean(validateAction)
out.writeString(indexType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import org.opensearch.indexmanagement.indexstatemanagement.model.ManagedIndexCon
import org.opensearch.indexmanagement.indexstatemanagement.model.Policy
import org.opensearch.indexmanagement.common.model.rest.SearchParams
import org.opensearch.indexmanagement.indexstatemanagement.ManagedIndexRunner.actionValidation
import org.opensearch.indexmanagement.indexstatemanagement.model.filterByPolicyID
import org.opensearch.indexmanagement.indexstatemanagement.opensearchapi.getManagedIndexMetadata
import org.opensearch.indexmanagement.indexstatemanagement.transport.action.managedIndex.ManagedIndexAction
import org.opensearch.indexmanagement.indexstatemanagement.transport.action.managedIndex.ManagedIndexRequest
Expand Down Expand Up @@ -162,9 +163,10 @@ class TransportExplainAction @Inject constructor(
.must(
QueryBuilders
.queryStringQuery(params.queryString)
.defaultField(MANAGED_INDEX_NAME_KEYWORD_FIELD)
.field(MANAGED_INDEX_NAME_KEYWORD_FIELD)
.defaultOperator(Operator.AND)
).filter(QueryBuilders.termsQuery(MANAGED_INDEX_INDEX_UUID_FIELD, indexUUIDs))
.filterByPolicyID(request.explainFilter)

val searchSourceBuilder = SearchSourceBuilder()
.from(params.from)
Expand Down Expand Up @@ -294,8 +296,32 @@ class TransportExplainAction @Inject constructor(
mgetMetadataReq,
object : ActionListener<MultiGetResponse> {
override fun onResponse(response: MultiGetResponse) {
val metadataMap: Map<ManagedIndexMetadataDocUUID, ManagedIndexMetadataMap?> =
var metadataMap: Map<ManagedIndexMetadataDocUUID, ManagedIndexMetadataMap?> =
response.responses.associate { it.id to getMetadata(it.response)?.toMap() }

if (request.explainFilter != null) {
metadataMap = metadataMap.filter { (_, value) ->
var isValid = true

if (value != null) {
val metaData = ManagedIndexMetaData.fromMap(value)

if (!request.explainFilter.byMetaData(metaData)) {
indexNames.remove(metaData.index)
indexNamesToUUIDs.remove(metaData.index)

if (managedIndices.contains(metaData.index)) {
totalManagedIndices--
}

isValid = false
}
}

isValid
}
}

buildResponse(indexNamesToUUIDs, metadataMap, clusterStateIndexMetadatas, threadContext)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const val MANAGED_INDEX_FIELD = "managed_index"
const val MANAGED_INDEX_NAME_KEYWORD_FIELD = "$MANAGED_INDEX_FIELD.name.keyword"
const val MANAGED_INDEX_INDEX_FIELD = "$MANAGED_INDEX_FIELD.index"
const val MANAGED_INDEX_INDEX_UUID_FIELD = "$MANAGED_INDEX_FIELD.index_uuid"
const val MANAGED_INDEX_POLICY_ID_FIELD = "$MANAGED_INDEX_FIELD.policy_id"

const val DEFAULT_JOB_SORT_FIELD = MANAGED_INDEX_INDEX_FIELD
const val DEFAULT_POLICY_SORT_FIELD = "policy.policy_id.keyword"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,7 @@ import org.opensearch.indexmanagement.IndexManagementPlugin.Companion.INDEX_STAT
import org.opensearch.indexmanagement.IndexManagementPlugin.Companion.ISM_BASE_URI
import org.opensearch.indexmanagement.IndexManagementPlugin.Companion.POLICY_BASE_URI
import org.opensearch.indexmanagement.IndexManagementRestTestCase
import org.opensearch.indexmanagement.indexstatemanagement.model.ChangePolicy
import org.opensearch.indexmanagement.indexstatemanagement.model.ISMTemplate
import org.opensearch.indexmanagement.indexstatemanagement.model.ManagedIndexConfig
import org.opensearch.indexmanagement.indexstatemanagement.model.Policy
import org.opensearch.indexmanagement.indexstatemanagement.model.Policy.Companion.POLICY_TYPE
import org.opensearch.indexmanagement.indexstatemanagement.model.StateFilter
import org.opensearch.indexmanagement.indexstatemanagement.resthandler.RestExplainAction
import org.opensearch.indexmanagement.indexstatemanagement.settings.ManagedIndexSettings
import org.opensearch.indexmanagement.indexstatemanagement.util.FAILED_INDICES
Expand Down Expand Up @@ -71,6 +66,12 @@ import org.opensearch.jobscheduler.spi.schedule.IntervalSchedule
import org.opensearch.rest.RestRequest
import org.opensearch.search.SearchModule
import org.opensearch.core.rest.RestStatus
import org.opensearch.indexmanagement.indexstatemanagement.model.ChangePolicy
import org.opensearch.indexmanagement.indexstatemanagement.model.ExplainFilter
import org.opensearch.indexmanagement.indexstatemanagement.model.ISMTemplate
import org.opensearch.indexmanagement.indexstatemanagement.model.ManagedIndexConfig
import org.opensearch.indexmanagement.indexstatemanagement.model.Policy
import org.opensearch.indexmanagement.indexstatemanagement.model.StateFilter
import org.opensearch.indexmanagement.rollup.randomTermQuery
import org.opensearch.test.OpenSearchTestCase
import java.io.IOException
Expand Down Expand Up @@ -473,6 +474,8 @@ abstract class IndexStateManagementRestTestCase : IndexManagementRestTestCase()

protected fun ManagedIndexConfig.toHttpEntity(): HttpEntity = StringEntity(toJsonString(), APPLICATION_JSON)

protected fun ExplainFilter.toHttpEntity(): HttpEntity = StringEntity(toJsonString(), APPLICATION_JSON)

protected fun ChangePolicy.toHttpEntity(): HttpEntity {
var string = "{\"${ChangePolicy.POLICY_ID_FIELD}\":\"$policyID\","
if (state != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import org.opensearch.indexmanagement.indexstatemanagement.action.SnapshotAction
import org.opensearch.indexmanagement.indexstatemanagement.model.ChangePolicy
import org.opensearch.indexmanagement.indexstatemanagement.model.Conditions
import org.opensearch.indexmanagement.indexstatemanagement.model.ErrorNotification
import org.opensearch.indexmanagement.indexstatemanagement.model.ExplainFilter
import org.opensearch.indexmanagement.indexstatemanagement.model.ISMTemplate
import org.opensearch.indexmanagement.indexstatemanagement.model.ManagedIndexConfig
import org.opensearch.indexmanagement.indexstatemanagement.model.Policy
Expand Down Expand Up @@ -305,6 +306,15 @@ fun randomByteSizeValue(): ByteSizeValue =
* End - Conditions helper functions
*/

fun randomExplainFilter(
policyID: String? = if (OpenSearchRestTestCase.randomBoolean()) OpenSearchRestTestCase.randomAlphaOfLength(10) else null,
state: String? = if (OpenSearchRestTestCase.randomBoolean()) OpenSearchRestTestCase.randomAlphaOfLength(10) else null,
actionType: String? = if (OpenSearchRestTestCase.randomBoolean()) OpenSearchRestTestCase.randomAlphaOfLength(10) else null,
failed: Boolean? = if (OpenSearchRestTestCase.randomBoolean()) OpenSearchRestTestCase.randomBoolean() else null
): ExplainFilter {
return ExplainFilter(policyID, state, actionType, failed)
}

fun randomChangePolicy(
policyID: String = OpenSearchRestTestCase.randomAlphaOfLength(10),
state: String? = if (OpenSearchRestTestCase.randomBoolean()) OpenSearchRestTestCase.randomAlphaOfLength(10) else null,
Expand Down Expand Up @@ -470,6 +480,11 @@ fun AllocationAction.toJsonString(): String {
return this.toXContent(builder, ToXContent.EMPTY_PARAMS).string()
}

fun ExplainFilter.toJsonString(): String {
val builder = XContentFactory.jsonBuilder()
return this.toXContent(builder, ToXContent.EMPTY_PARAMS).string()
}

fun ChangePolicy.toJsonString(): String {
val builder = XContentFactory.jsonBuilder()
return this.toXContent(builder, ToXContent.EMPTY_PARAMS).string()
Expand Down
Loading

0 comments on commit d65545a

Please sign in to comment.