Stop suggesting feature migrations for 7.x indices (elastic#93666) (elastic#93708)

williamrandolph · web-flow · commit 2d58d0f13614 · 2023-02-10T12:40:37.000-05:00
* Don't report MIGRATION_NEEDED for 7.x indices

Eventually, we will need to migrate 7.x indices to 8.x before doing a
significant upgrade of Lucene. However, the migrations to 8.x are not
adequately tested: while they will eventually be needed, they are not
currently needed, and may in fact produce bugs.

This change will ensure that the GET _migration/system_feature API
returns NO_MIGRATION_NEEDED in 8.x. We will begin to require
migrations once we start testing for the next major version upgrade.
diff --git a/docs/changelog/93666.yaml b/docs/changelog/93666.yaml
@@ -0,0 +1,5 @@
+pr: 93666
+summary: Don't report MIGRATION_NEEDED for 7.x indices
+area: Infra/Core
+type: bug
+issues: []
diff --git a/modules/ingest-geoip/qa/full-cluster-restart/build.gradle b/modules/ingest-geoip/qa/full-cluster-restart/build.gradle
@@ -7,6 +7,7 @@
  */
 
 import org.elasticsearch.gradle.Version
+import org.elasticsearch.gradle.VersionProperties
 import org.elasticsearch.gradle.internal.info.BuildParams
 import org.elasticsearch.gradle.testclusters.StandaloneRestIntegTestTask
 
@@ -37,7 +38,12 @@ tasks.withType(Test).configureEach {
   }
 }
 
-BuildParams.bwcVersions.withWireCompatible(v -> v.before("8.0.0")) { bwcVersion, baseName ->
+assert Version.fromString(VersionProperties.getVersions().get("elasticsearch")).getMajor() == 8 :
+  "If we are targeting a branch other than 8, we should enable migration tests"
+
+// once we are ready to test migrations from 8.x to 9.x, we can set the compatible version to 8.0.0
+// see https://github.com/elastic/elasticsearch/pull/93666
+BuildParams.bwcVersions.withWireCompatible(v -> v.before("7.0.0")) { bwcVersion, baseName ->
   def baseCluster = testClusters.register(baseName) {
     testDistribution = "DEFAULT"
     if (bwcVersion.before(BuildParams.bwcVersions.minimumWireCompatibleVersion)) {
diff --git a/modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/AbstractFeatureMigrationIntegTest.java b/modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/AbstractFeatureMigrationIntegTest.java
@@ -10,6 +10,7 @@
 
 import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.admin.cluster.migration.TransportGetFeatureUpgradeStatusAction;
 import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
 import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
 import org.elasticsearch.action.admin.indices.stats.IndexStats;
@@ -64,7 +65,7 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
     static final String INTERNAL_MANAGED_INDEX_NAME = ".int-man-old";
     static final int INDEX_DOC_COUNT = 100; // arbitrarily chosen
     static final int INTERNAL_MANAGED_FLAG_VALUE = 1;
-    public static final Version NEEDS_UPGRADE_VERSION = Version.V_7_0_0;
+    public static final Version NEEDS_UPGRADE_VERSION = TransportGetFeatureUpgradeStatusAction.NO_UPGRADE_REQUIRED_VERSION.previousMajor();
 
     static final SystemIndexDescriptor EXTERNAL_UNMANAGED = SystemIndexDescriptor.builder()
         .setIndexPattern(".ext-unman-*")
@@ -131,6 +132,11 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
 
     @Before
     public void setup() {
+        assumeTrue(
+            "We can only create the test indices we need if they're in the previous major version",
+            NEEDS_UPGRADE_VERSION.onOrAfter(Version.CURRENT.previousMajor())
+        );
+
         internalCluster().setBootstrapMasterNodeIndex(0);
         masterName = internalCluster().startMasterOnlyNode();
         masterAndDataNode = internalCluster().startNode();
diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java
@@ -8,7 +8,7 @@
 
 package org.elasticsearch.upgrades;
 
-import org.elasticsearch.Version;
+import org.elasticsearch.action.admin.cluster.migration.TransportGetFeatureUpgradeStatusAction;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.ResponseException;
 import org.elasticsearch.test.XContentTestUtils;
@@ -94,7 +94,7 @@ public void testGetFeatureUpgradeStatus() throws Exception {
 
                 assertThat(feature.size(), equalTo(4));
                 assertThat(feature.get("minimum_index_version"), equalTo(UPGRADE_FROM_VERSION.toString()));
-                if (UPGRADE_FROM_VERSION.before(Version.V_8_0_0)) {
+                if (UPGRADE_FROM_VERSION.before(TransportGetFeatureUpgradeStatusAction.NO_UPGRADE_REQUIRED_VERSION)) {
                     assertThat(feature.get("migration_status"), equalTo("MIGRATION_NEEDED"));
                 } else {
                     assertThat(feature.get("migration_status"), equalTo("NO_MIGRATION_NEEDED"));
diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java
@@ -49,9 +49,9 @@ public class TransportGetFeatureUpgradeStatusAction extends TransportMasterNodeA
     GetFeatureUpgradeStatusResponse> {
 
     /**
-     * This version is only valid for >=8.0.0 and should be changed on backport.
+     * Once all feature migrations for 8.x -> 9.x have been tested, we can bump this to Version.V_8_0_0
      */
-    public static final Version NO_UPGRADE_REQUIRED_VERSION = Version.V_8_0_0;
+    public static final Version NO_UPGRADE_REQUIRED_VERSION = Version.V_7_0_0;
 
     private final SystemIndices systemIndices;
     PersistentTasksService persistentTasksService;
@@ -77,6 +77,9 @@ public TransportGetFeatureUpgradeStatusAction(
             GetFeatureUpgradeStatusResponse::new,
             ThreadPool.Names.SAME
         );
+
+        assert Version.CURRENT.major == 8 : "Once we begin working on 9.x, we need to update our migration classes";
+
         this.systemIndices = systemIndices;
         this.persistentTasksService = persistentTasksService;
     }
diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusActionTests.java
@@ -29,6 +29,7 @@ public class TransportGetFeatureUpgradeStatusActionTests extends ESTestCase {
     public static String TEST_SYSTEM_INDEX_PATTERN = ".test*";
     private static final ClusterState CLUSTER_STATE = getClusterState();
     private static final SystemIndices.Feature FEATURE = getFeature();
+    private static final Version TEST_OLD_VERSION = Version.fromString("6.0.0");
 
     public void testGetFeatureStatus() {
         GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus status = TransportGetFeatureUpgradeStatusAction.getFeatureUpgradeStatus(
@@ -38,7 +39,7 @@ public void testGetFeatureStatus() {
 
         assertThat(status.getUpgradeStatus(), equalTo(MIGRATION_NEEDED));
         assertThat(status.getFeatureName(), equalTo("test-feature"));
-        assertThat(status.getMinimumIndexVersion(), equalTo(Version.V_7_0_0));
+        assertThat(status.getMinimumIndexVersion(), equalTo(TEST_OLD_VERSION));
         assertThat(status.getIndexVersions().size(), equalTo(2)); // additional testing below
     }
 
@@ -57,7 +58,7 @@ public void testGetIndexInfos() {
         }
         {
             GetFeatureUpgradeStatusResponse.IndexInfo version = versions.get(1);
-            assertThat(version.getVersion(), equalTo(Version.V_7_0_0));
+            assertThat(version.getVersion(), equalTo(TEST_OLD_VERSION));
             assertThat(version.getIndexName(), equalTo(".test-index-2"));
         }
     }
@@ -80,8 +81,11 @@ private static ClusterState getClusterState() {
             .numberOfReplicas(0)
             .build();
 
+        // Once we start testing 9.x, we should update this test to use a 7.x "version created"
+        assert Version.CURRENT.major < 9;
+
         IndexMetadata indexMetadata2 = IndexMetadata.builder(".test-index-2")
-            .settings(Settings.builder().put("index.version.created", Version.V_7_0_0).build())
+            .settings(Settings.builder().put("index.version.created", Version.fromString("6.0.0")).build())
             .numberOfShards(1)
             .numberOfReplicas(0)
             .build();
