elastic · droberts195 · Oct 14, 2021 · Oct 12, 2021 · Oct 13, 2021 · Oct 13, 2021
diff --git a/...lugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java b/...lugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java
@@ -15,6 +15,7 @@
 import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest;
 import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
 import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
+import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequestBuilder;
 import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
 import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
@@ -30,6 +31,7 @@
 import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
 import org.elasticsearch.xpack.core.template.TemplateUtils;
 
+import java.util.List;
 import java.util.SortedMap;
 
 import static org.elasticsearch.xpack.core.ClientHelper.ML_ORIGIN;
@@ -41,8 +43,15 @@ public class AnnotationIndex {
 
     public static final String READ_ALIAS_NAME = ".ml-annotations-read";
     public static final String WRITE_ALIAS_NAME = ".ml-annotations-write";
-    // Exposed for testing, but always use the aliases in non-test code
-    public static final String INDEX_NAME = ".ml-annotations-6";
+
+    // Exposed for testing, but always use the aliases in non-test code.
+    public static final String LATEST_INDEX_NAME = ".ml-annotations-000001";
+    // Due to historical bugs this index may not have the correct mappings
+    // in some production clusters. Therefore new annotations should be
+    // written to the latest index. If we ever switch to another new annotations
+    // index then this list should be adjusted to include the previous latest
+    // index.
+    public static final List<String> OLD_INDEX_NAMES = List.of(".ml-annotations-6");
 
     private static final String MAPPINGS_VERSION_VARIABLE = "xpack.ml.version";
 
@@ -85,12 +94,18 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
             finalListener::onFailure);
 
         final ActionListener<Boolean> createAliasListener = ActionListener.wrap(success -> {
-            final IndicesAliasesRequest request =
+            final IndicesAliasesRequestBuilder requestBuilder =
                 client.admin().indices().prepareAliases()
-                    .addAliasAction(IndicesAliasesRequest.AliasActions.add().index(INDEX_NAME).alias(READ_ALIAS_NAME).isHidden(true))
-                    .addAliasAction(IndicesAliasesRequest.AliasActions.add().index(INDEX_NAME).alias(WRITE_ALIAS_NAME).isHidden(true))
-                    .request();
-            executeAsyncWithOrigin(client.threadPool().getThreadContext(), ML_ORIGIN, request,
+                    .addAliasAction(IndicesAliasesRequest.AliasActions.add()
+                        .index(LATEST_INDEX_NAME).alias(READ_ALIAS_NAME).isHidden(true))
+                    .addAliasAction(IndicesAliasesRequest.AliasActions.add()
+                        .index(LATEST_INDEX_NAME).alias(WRITE_ALIAS_NAME).isHidden(true));
+            for (String oldIndexName : OLD_INDEX_NAMES) {
+                if (state.getMetadata().getIndicesLookup().containsKey(oldIndexName)) {
+                    requestBuilder.removeAlias(oldIndexName, WRITE_ALIAS_NAME);
+                }
+            }
+            executeAsyncWithOrigin(client.threadPool().getThreadContext(), ML_ORIGIN, requestBuilder.request(),
                 ActionListener.<AcknowledgedResponse>wrap(
                     r -> checkMappingsListener.onResponse(r.isAcknowledged()), finalListener::onFailure),
                 client.admin().indices()::aliases);
@@ -104,18 +119,18 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
             mlLookup.isEmpty() == false && mlLookup.firstKey().startsWith(".ml")) {
 
             // Create the annotations index if it doesn't exist already.
-            if (mlLookup.containsKey(INDEX_NAME) == false) {
+            if (mlLookup.containsKey(LATEST_INDEX_NAME) == false) {
                 logger.debug(
                     () -> new ParameterizedMessage(
                         "Creating [{}] because [{}] exists; trace {}",
-                        INDEX_NAME,
+                        LATEST_INDEX_NAME,
                         mlLookup.firstKey(),
                         org.elasticsearch.ExceptionsHelper.formatStackTrace(Thread.currentThread().getStackTrace())
                     )
                 );
 
                 CreateIndexRequest createIndexRequest =
-                    new CreateIndexRequest(INDEX_NAME)
+                    new CreateIndexRequest(LATEST_INDEX_NAME)
                         .mapping(annotationsMapping())
                         .settings(Settings.builder()
                             .put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "0-1")
@@ -140,7 +155,14 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
             }
 
             // Recreate the aliases if they've gone even though the index still exists.
-            if (mlLookup.containsKey(READ_ALIAS_NAME) == false || mlLookup.containsKey(WRITE_ALIAS_NAME) == false) {
+            IndexAbstraction writeAliasDefinition = mlLookup.get(WRITE_ALIAS_NAME);
+            if (mlLookup.containsKey(READ_ALIAS_NAME) == false || writeAliasDefinition == null) {
+                createAliasListener.onResponse(true);
+                return;
+            }
+
+            List<IndexMetadata> writeAliasMetadata = writeAliasDefinition.getIndices();
+            if (writeAliasMetadata.size() != 1 || LATEST_INDEX_NAME.equals(writeAliasMetadata.get(0).getIndex().getName()) == false) {
                 createAliasListener.onResponse(true);
                 return;
             }
@@ -154,7 +176,7 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
         finalListener.onResponse(false);
     }
 
-    private static String annotationsMapping() {
+    public static String annotationsMapping() {
         return TemplateUtils.loadTemplate(
             "/org/elasticsearch/xpack/core/ml/annotations_index_mappings.json", Version.CURRENT.toString(), MAPPINGS_VERSION_VARIABLE);
     }

diff --git a/...n/core/src/main/resources/org/elasticsearch/xpack/core/ml/annotations_index_mappings.json b/...n/core/src/main/resources/org/elasticsearch/xpack/core/ml/annotations_index_mappings.json
@@ -3,6 +3,7 @@
     "_meta" : {
       "version" : "${xpack.ml.version}"
     },
+    "dynamic" : false,
     "properties" : {
       "annotation" : {
         "type" : "text"

diff --git a/.../test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java b/.../test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java
@@ -1478,7 +1478,7 @@ public void testMachineLearningAdminRole() {
         assertOnlyReadAllowed(role, AnomalyDetectorsIndexFields.STATE_INDEX_PREFIX);
         assertOnlyReadAllowed(role, AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX + AnomalyDetectorsIndexFields.RESULTS_INDEX_DEFAULT);
         assertOnlyReadAllowed(role, NotificationsIndex.NOTIFICATIONS_INDEX);
-        assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.INDEX_NAME);
+        assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.LATEST_INDEX_NAME);
 
         assertNoAccessAllowed(role, RestrictedIndicesNames.RESTRICTED_NAMES);
         assertNoAccessAllowed(role, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2));
@@ -1632,7 +1632,7 @@ public void testMachineLearningUserRole() {
         assertNoAccessAllowed(role, AnomalyDetectorsIndexFields.STATE_INDEX_PREFIX);
         assertOnlyReadAllowed(role, AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX + AnomalyDetectorsIndexFields.RESULTS_INDEX_DEFAULT);
         assertOnlyReadAllowed(role, NotificationsIndex.NOTIFICATIONS_INDEX);
-        assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.INDEX_NAME);
+        assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.LATEST_INDEX_NAME);
 
         assertNoAccessAllowed(role, RestrictedIndicesNames.RESTRICTED_NAMES);
         assertNoAccessAllowed(role, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2));

diff --git a/...RestTest/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java b/...RestTest/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java
@@ -264,7 +264,7 @@ protected void waitForecastStatus(int maxWaitTimeSeconds,
 
     protected void assertThatNumberOfAnnotationsIsEqualTo(int expectedNumberOfAnnotations) throws IOException {
         // Refresh the annotations index so that recently indexed annotation docs are visible.
-        client().admin().indices().prepareRefresh(AnnotationIndex.INDEX_NAME)
+        client().admin().indices().prepareRefresh(AnnotationIndex.LATEST_INDEX_NAME)
             .setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN)
             .execute()
             .actionGet();

diff --git a/...rc/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/AnnotationIndexIT.java b/...rc/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/AnnotationIndexIT.java
@@ -7,13 +7,18 @@
 package org.elasticsearch.xpack.ml.integration;
 
 import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
+
+import org.elasticsearch.action.admin.indices.alias.Alias;
+import org.elasticsearch.action.admin.indices.create.CreateIndexAction;
+import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
 import org.elasticsearch.action.index.IndexRequest;
 import org.elasticsearch.action.index.IndexResponse;
 import org.elasticsearch.action.search.SearchPhaseExecutionException;
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.cluster.metadata.AliasMetadata;
+import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.settings.Settings;
@@ -26,11 +31,12 @@
 import org.elasticsearch.xpack.core.ml.annotations.AnnotationIndex;
 import org.elasticsearch.xpack.ml.MlSingleNodeTestCase;
 import org.elasticsearch.xpack.ml.notifications.AnomalyDetectionAuditor;
-import org.junit.Before;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
+import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.is;
 
 public class AnnotationIndexIT extends MlSingleNodeTestCase {
@@ -44,11 +50,6 @@ protected Settings nodeSettings()  {
         return newSettings.build();
     }
 
-    @Before
-    public void createComponents() throws Exception {
-        waitForMlTemplates();
-    }
-
     public void testNotCreatedWhenNoOtherMlIndices() {
 
         // Ask a few times to increase the chance of failure if the .ml-annotations index is created when no other ML index exists
@@ -71,6 +72,52 @@ public void testCreatedWhenAfterOtherMlIndex() throws Exception {
         });
     }
 
+    public void testAliasesMovedFromOldToNew() throws Exception {
+
+        // Create an old annotations index with both read and write aliases pointing at it.
+        String oldIndex = randomFrom(AnnotationIndex.OLD_INDEX_NAMES);
+        CreateIndexRequest createIndexRequest =
+            new CreateIndexRequest(oldIndex)
+                .mapping(AnnotationIndex.annotationsMapping())
+                .settings(Settings.builder()
+                    .put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "0-1")
+                    .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, "1")
+                    .put(IndexMetadata.SETTING_INDEX_HIDDEN, true))
+                .alias(new Alias(AnnotationIndex.READ_ALIAS_NAME).isHidden(true))
+                .alias(new Alias(AnnotationIndex.WRITE_ALIAS_NAME).isHidden(true));
+        client().execute(CreateIndexAction.INSTANCE, createIndexRequest).actionGet();
+
+        // Because the old annotations index name began with .ml, it will trigger the new annotations index to be created.
+        // When this happens the read alias should be changed to cover both indices, and the write alias should be
+        // switched to only point at the new index.
+        assertBusy(() -> {
+            assertTrue(annotationsIndexExists());
+            ImmutableOpenMap<String, List<AliasMetadata>> aliases = client().admin().indices()
+                .prepareGetAliases(AnnotationIndex.READ_ALIAS_NAME, AnnotationIndex.WRITE_ALIAS_NAME)
+                .setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN)
+                .get()
+                .getAliases();
+            assertNotNull(aliases);
+            List<String> indicesWithReadAlias = new ArrayList<>();
+            for (ObjectObjectCursor<String, List<AliasMetadata>> entry : aliases) {
+                for (AliasMetadata aliasMetadata : entry.value) {
+                    switch (aliasMetadata.getAlias()) {
+                        case AnnotationIndex.WRITE_ALIAS_NAME:
+                            assertThat(entry.key, is(AnnotationIndex.LATEST_INDEX_NAME));
+                            break;
+                        case AnnotationIndex.READ_ALIAS_NAME:
+                            indicesWithReadAlias.add(entry.key);
+                            break;
+                        default:
+                            fail("Found unexpected alias " + aliasMetadata.getAlias() + " on index " + entry.key);
+                            break;
+                    }
+                }
+            }
+            assertThat(indicesWithReadAlias, containsInAnyOrder(oldIndex, AnnotationIndex.LATEST_INDEX_NAME));
+        });
+    }
+
     public void testNotCreatedWhenAfterOtherMlIndexAndUpgradeInProgress() throws Exception {
 
         client().execute(SetUpgradeModeAction.INSTANCE, new SetUpgradeModeAction.Request(true)).actionGet();
@@ -123,7 +170,7 @@ public void testNotCreatedWhenAfterOtherMlIndexAndResetInProgress() throws Excep
     }
 
     private boolean annotationsIndexExists() {
-        return ESIntegTestCase.indexExists(AnnotationIndex.INDEX_NAME, client());
+        return ESIntegTestCase.indexExists(AnnotationIndex.LATEST_INDEX_NAME, client());
     }
 
     private int numberOfAnnotationsAliases() {

diff --git a/...lClusterTest/java/org/elasticsearch/xpack/ml/integration/AutodetectResultProcessorIT.java b/...lClusterTest/java/org/elasticsearch/xpack/ml/integration/AutodetectResultProcessorIT.java
@@ -731,7 +731,7 @@ private QueryPage<ModelSnapshot> getModelSnapshots() throws Exception {
 
     private List<Annotation> getAnnotations() throws Exception {
         // Refresh the annotations index so that recently indexed annotation docs are visible.
-        client().admin().indices().prepareRefresh(AnnotationIndex.INDEX_NAME)
+        client().admin().indices().prepareRefresh(AnnotationIndex.LATEST_INDEX_NAME)
             .setIndicesOptions(IndicesOptions.STRICT_EXPAND_OPEN_HIDDEN_FORBID_CLOSED)
             .execute()
             .actionGet();

diff --git a/...java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManagerTests.java b/...java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManagerTests.java
@@ -92,6 +92,7 @@
 import java.util.function.Consumer;
 
 import static org.elasticsearch.action.support.master.MasterNodeRequest.DEFAULT_MASTER_NODE_TIMEOUT;
+import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_INDEX_HIDDEN;
 import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
 import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
 import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
@@ -184,21 +185,23 @@ public void setup() throws Exception {
                             Settings.builder()
                                 .put(SETTING_NUMBER_OF_SHARDS, 1)
                                 .put(SETTING_NUMBER_OF_REPLICAS, 0)
+                                .put(SETTING_INDEX_HIDDEN, true)
                                 .put(SETTING_VERSION_CREATED, Version.CURRENT)
                                 .build())
-                        .putAlias(AliasMetadata.builder(AnomalyDetectorsIndex.jobStateIndexWriteAlias()).build())
+                        .putAlias(AliasMetadata.builder(AnomalyDetectorsIndex.jobStateIndexWriteAlias()).isHidden(true).build())
                         .build())
                 .fPut(
-                    AnnotationIndex.INDEX_NAME,
-                    IndexMetadata.builder(AnnotationIndex.INDEX_NAME)
+                    AnnotationIndex.LATEST_INDEX_NAME,
+                    IndexMetadata.builder(AnnotationIndex.LATEST_INDEX_NAME)
                         .settings(
                             Settings.builder()
                                 .put(SETTING_NUMBER_OF_SHARDS, 1)
                                 .put(SETTING_NUMBER_OF_REPLICAS, 0)
+                                .put(SETTING_INDEX_HIDDEN, true)
                                 .put(SETTING_VERSION_CREATED, Version.CURRENT)
                                 .build())
-                        .putAlias(AliasMetadata.builder(AnnotationIndex.READ_ALIAS_NAME).build())
-                        .putAlias(AliasMetadata.builder(AnnotationIndex.WRITE_ALIAS_NAME).build())
+                        .putAlias(AliasMetadata.builder(AnnotationIndex.READ_ALIAS_NAME).isHidden(true).build())
+                        .putAlias(AliasMetadata.builder(AnnotationIndex.WRITE_ALIAS_NAME).isHidden(true).build())
                         .build())
                 .build())
             .build();

diff --git a/...olling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/30_ml_jobs_crud.yml b/...olling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/30_ml_jobs_crud.yml
@@ -157,6 +157,6 @@ setup:
       indices.get_mapping:
         index: .ml-annotations-write
 
-  - match: { \.ml-annotations-6.mappings.properties.type.type: "keyword" }
-  - match: { \.ml-annotations-6.mappings.properties.event.type: "keyword" }
-  - match: { \.ml-annotations-6.mappings.properties.detector_index.type: "integer" }
+  - match: { \.ml-annotations-000001.mappings.properties.type.type: "keyword" }
+  - match: { \.ml-annotations-000001.mappings.properties.event.type: "keyword" }
+  - match: { \.ml-annotations-000001.mappings.properties.detector_index.type: "integer" }