Register system index descriptors through SystemIndexPlugin.getSystemIndexDescriptors #2586

cwperks · 2024-06-21T19:01:14Z

Description

This PR registers the system indices in this plugin through the SystemIndexPlugin extension point in core. These indices will not be functionally different than they are today, its just a formal registration as a system index.

Issues Resolved

Related to opensearch-project/security#4439

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

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.

…IndexDescriptors Signed-off-by: Craig Perkins <[email protected]>

shikharj05 · 2024-06-26T04:23:23Z

plugin/src/main/java/org/opensearch/ml/plugin/MachineLearningPlugin.java

+        List<SystemIndexDescriptor> systemIndexDescriptors = new ArrayList<>();
+        systemIndexDescriptors.add(new SystemIndexDescriptor(ML_AGENT_INDEX, "ML Commons Agent Index"));
+        systemIndexDescriptors.add(new SystemIndexDescriptor(ML_CONFIG_INDEX, "ML Commons Configuration Index"));
+        systemIndexDescriptors.add(new SystemIndexDescriptor(ML_CONNECTOR_INDEX, "ML Commons Connector Index"));
+        systemIndexDescriptors.add(new SystemIndexDescriptor(ML_CONTROLLER_INDEX, "ML Commons Controller Index"));
+        systemIndexDescriptors.add(new SystemIndexDescriptor(ML_MODEL_GROUP_INDEX, "ML Commons Model Group Index"));
+        systemIndexDescriptors.add(new SystemIndexDescriptor(ML_MODEL_INDEX, "ML Commons Model Index"));
+        systemIndexDescriptors.add(new SystemIndexDescriptor(ML_TASK_INDEX, "ML Commons Task Index"));
+        systemIndexDescriptors.add(new SystemIndexDescriptor(ML_CONVERSATION_META_INDEX, "ML Commons Conversation Meta Index"));
+        systemIndexDescriptors
+            .add(new SystemIndexDescriptor(ML_CONVERSATION_INTERACTIONS_INDEX, "ML Commons Conversation Interactions Index"));
+        systemIndexDescriptors.add(new SystemIndexDescriptor(ML_MEMORY_META_INDEX, "ML Commons Memory Meta Index"));
+        systemIndexDescriptors.add(new SystemIndexDescriptor(ML_MEMORY_MESSAGE_INDEX, "ML Commons Memory Message Index"));
+        systemIndexDescriptors.add(new SystemIndexDescriptor(ML_STOP_WORDS_INDEX, "ML Commons Stop Words Index"));


should this be supported with a Pattern as well? For e.g. .plugins-ml-*

It could make sense in this case since the plugin has many system indices that follow the same naming convention.

My advice to plugins is to use a pattern if they create a variable amount of system indices based on a pattern.

An example of a variable amount of indices is multi-tenancy where each tenant has a .kibana index that follows a pattern .kibana-{tenantName}. If the names of all of the indices are known then they should be listed out.

cwperks · 2024-07-02T18:22:29Z

Can a maintainer please review this PR?

plugin/src/main/java/org/opensearch/ml/plugin/MachineLearningPlugin.java

Signed-off-by: Craig Perkins <[email protected]>

ylwu-amzn · 2024-07-10T16:17:52Z

RestMLDeployModelActionIT > testReDeployModel FAILED
    java.lang.Exception: Test abandoned because suite timeout was reached.
        at __randomizedtesting.SeedInfo.seed([BC9B3B175CC25BB4]:0)

dhrubo-os · 2024-07-16T20:26:46Z

common/src/main/java/org/opensearch/ml/common/CommonValue.java

@@ -62,6 +62,8 @@ public class CommonValue {
        public static final Integer ML_MODEL_GROUP_INDEX_SCHEMA_VERSION = 2;
        public static final Integer ML_MODEL_INDEX_SCHEMA_VERSION = 11;
        public static final String ML_CONNECTOR_INDEX = ".plugins-ml-connector";
+        public static final String ML_CONVERSATION_META_INDEX = ".plugins-ml-conversation-meta";


Why do we need these two indices? I don't think we are using these?

Removed these. I added them in a prior commit since they are still tracked in the security plugin: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java#L55-L56

Signed-off-by: Craig Perkins <[email protected]>

…IndexDescriptors (#2586) * Register system index descriptors through SystemIndexPlugin.getSystemIndexDescriptors Signed-off-by: Craig Perkins <[email protected]> * Address code review feedback Signed-off-by: Craig Perkins <[email protected]> * Remove unused imports Signed-off-by: Craig Perkins <[email protected]> * Remove unused comments Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]> (cherry picked from commit 6c8720e)

…IndexDescriptors (#2586) (#2668) * Register system index descriptors through SystemIndexPlugin.getSystemIndexDescriptors Signed-off-by: Craig Perkins <[email protected]> * Address code review feedback Signed-off-by: Craig Perkins <[email protected]> * Remove unused imports Signed-off-by: Craig Perkins <[email protected]> * Remove unused comments Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]> (cherry picked from commit 6c8720e) Co-authored-by: Craig Perkins <[email protected]>

Register system index descriptors through SystemIndexPlugin.getSystem…

6a9b9f8

…IndexDescriptors Signed-off-by: Craig Perkins <[email protected]>

cwperks requested review from b4sjoo, dhrubo-os, jngz-es, model-collapse, rbhavna, ylwu-amzn, zane-neo, Zhangxunmt, austintlee, HenryL27, samuel-oci and xinyual as code owners June 21, 2024 19:01

cwperks had a problem deploying to ml-commons-cicd-env June 21, 2024 19:01 — with GitHub Actions Error

cwperks had a problem deploying to ml-commons-cicd-env June 21, 2024 19:01 — with GitHub Actions Failure

cwperks had a problem deploying to ml-commons-cicd-env June 21, 2024 19:01 — with GitHub Actions Error

cwperks had a problem deploying to ml-commons-cicd-env June 21, 2024 19:01 — with GitHub Actions Failure

Merge branch 'main' into system-index-plugin

fa21c29

cwperks temporarily deployed to ml-commons-cicd-env June 24, 2024 18:45 — with GitHub Actions Inactive

cwperks temporarily deployed to ml-commons-cicd-env June 24, 2024 19:43 — with GitHub Actions Inactive

shikharj05 reviewed Jun 26, 2024

View reviewed changes

shikharj05 approved these changes Jul 3, 2024

View reviewed changes

ylwu-amzn reviewed Jul 4, 2024

View reviewed changes

plugin/src/main/java/org/opensearch/ml/plugin/MachineLearningPlugin.java Outdated Show resolved Hide resolved

ylwu-amzn reviewed Jul 4, 2024

View reviewed changes

plugin/src/main/java/org/opensearch/ml/plugin/MachineLearningPlugin.java Outdated Show resolved Hide resolved

Address code review feedback

67b1b29

Signed-off-by: Craig Perkins <[email protected]>

cwperks had a problem deploying to ml-commons-cicd-env July 5, 2024 15:24 — with GitHub Actions Failure

Remove unused imports

e7161dc

Signed-off-by: Craig Perkins <[email protected]>

cwperks temporarily deployed to ml-commons-cicd-env July 9, 2024 20:35 — with GitHub Actions Inactive

cwperks had a problem deploying to ml-commons-cicd-env July 9, 2024 20:35 — with GitHub Actions Failure

Hailong-am approved these changes Jul 10, 2024

View reviewed changes

cwperks temporarily deployed to ml-commons-cicd-env July 12, 2024 18:02 — with GitHub Actions Inactive

cwperks had a problem deploying to ml-commons-cicd-env July 12, 2024 18:02 — with GitHub Actions Failure

cwperks temporarily deployed to ml-commons-cicd-env July 15, 2024 14:57 — with GitHub Actions Inactive

cwperks had a problem deploying to ml-commons-cicd-env July 15, 2024 14:57 — with GitHub Actions Failure

Merge branch 'main' into system-index-plugin

4068eff

cwperks temporarily deployed to ml-commons-cicd-env July 16, 2024 15:33 — with GitHub Actions Inactive

cwperks temporarily deployed to ml-commons-cicd-env July 16, 2024 16:31 — with GitHub Actions Inactive

ylwu-amzn previously approved these changes Jul 16, 2024

View reviewed changes

ylwu-amzn added the backport 2.x label Jul 16, 2024

dhrubo-os reviewed Jul 16, 2024

View reviewed changes

shikharj05 approved these changes Jul 17, 2024

View reviewed changes

Remove unused comments

16e123c

Signed-off-by: Craig Perkins <[email protected]>

cwperks dismissed ylwu-amzn’s stale review via 16e123c July 17, 2024 15:19

cwperks temporarily deployed to ml-commons-cicd-env July 17, 2024 15:19 — with GitHub Actions Inactive

cwperks temporarily deployed to ml-commons-cicd-env July 17, 2024 16:17 — with GitHub Actions Inactive

dhrubo-os approved these changes Jul 17, 2024

View reviewed changes

shikharj05 approved these changes Jul 17, 2024

View reviewed changes

ylwu-amzn approved these changes Jul 17, 2024

View reviewed changes

ylwu-amzn merged commit 6c8720e into opensearch-project:main Jul 17, 2024
6 checks passed

opensearch-trigger-bot bot mentioned this pull request Jul 17, 2024

[Backport 2.x] Register system index descriptors through SystemIndexPlugin.getSystemIndexDescriptors #2668

Merged

b4sjoo added the v2.16.0 Issues targeting release v2.16.0 label Jul 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Register system index descriptors through SystemIndexPlugin.getSystemIndexDescriptors #2586

Register system index descriptors through SystemIndexPlugin.getSystemIndexDescriptors #2586

cwperks commented Jun 21, 2024

shikharj05 Jun 26, 2024

cwperks Jun 26, 2024

cwperks commented Jul 2, 2024

ylwu-amzn commented Jul 10, 2024

dhrubo-os Jul 16, 2024

cwperks Jul 17, 2024

Register system index descriptors through SystemIndexPlugin.getSystemIndexDescriptors #2586

Register system index descriptors through SystemIndexPlugin.getSystemIndexDescriptors #2586

Conversation

cwperks commented Jun 21, 2024

Description

Issues Resolved

Check List

shikharj05 Jun 26, 2024

Choose a reason for hiding this comment

cwperks Jun 26, 2024

Choose a reason for hiding this comment

cwperks commented Jul 2, 2024

ylwu-amzn commented Jul 10, 2024

dhrubo-os Jul 16, 2024

Choose a reason for hiding this comment

cwperks Jul 17, 2024

Choose a reason for hiding this comment