Skip to content

Commit

Permalink
Stop suggesting feature migrations for 7.x indices (#93666) (#93708)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
williamrandolph authored Feb 10, 2023
1 parent 173db6d commit 2d58d0f
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 9 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/93666.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 93666
summary: Don't report MIGRATION_NEEDED for 7.x indices
area: Infra/Core
type: bug
issues: []
8 changes: 7 additions & 1 deletion modules/ingest-geoip/qa/full-cluster-restart/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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-*")
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
}

Expand All @@ -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"));
}
}
Expand All @@ -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();
Expand Down

0 comments on commit 2d58d0f

Please sign in to comment.