Skip to content

Commit 385e0d9

Browse files
ldematteldematte
authored
[BC Upgrage] Fix incorrect version parsing in tests (#129243)
This PR introduces several fixes to various IT tests, related to the use and misuse of the version identifier for the start cluster: wherever we can, we replace of versions in test code with features where we can't, we make sure we use the actual stack version (the one provided by -Dtests.bwc.main.version and not the bogus "0.0.0" version string) when requesting the cluster version we make sure we do use the "unresolved" version identifier (the value of the tests.old_cluster_version system property e.g. 0.0.0 ) so we resolve the right distribution These changes enabled the tests to be used in BC upgrade tests (and potentially in serverless upgrade tests too, where they would have also failed) Relates to ES-12010 Precedes #128614, #128823 and #128983
1 parent f7c1294 commit 385e0d9

File tree

16 files changed

+128
-97
lines changed

16 files changed

+128
-97
lines changed

‎qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/ParameterizedFullClusterRestartTestCase.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,11 @@ public boolean isRunningAgainstOldCluster() {
134134
return requestedUpgradeStatus == OLD;
135135
}
136136

137-
public static String getOldClusterVersion() {
137+
/**
138+
* The version of the "old" (initial) cluster. It is an opaque string, do not even think about parsing it for version
139+
* comparison. Use (test) cluster features and {@link ParameterizedFullClusterRestartTestCase#oldClusterHasFeature} instead.
140+
*/
141+
protected static String getOldClusterVersion() {
138142
return System.getProperty("tests.bwc.main.version", OLD_CLUSTER_VERSION);
139143
}
140144

‎qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/AbstractRollingUpgradeTestCase.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,11 @@ public abstract class AbstractRollingUpgradeTestCase extends ParameterizedRollin
3030
private static final ElasticsearchCluster cluster = buildCluster();
3131

3232
private static ElasticsearchCluster buildCluster() {
33-
Version oldVersion = Version.fromString(OLD_CLUSTER_VERSION);
33+
// Note we need to use OLD_CLUSTER_VERSION directly here, as it may contain special values (e.g. 0.0.0) the ElasticsearchCluster
34+
// builder uses to lookup a particular distribution
3435
var cluster = ElasticsearchCluster.local()
3536
.distribution(DistributionType.DEFAULT)
36-
.version(getOldClusterTestVersion())
37+
.version(OLD_CLUSTER_VERSION)
3738
.nodes(NODE_NUM)
3839
.setting("path.repo", new Supplier<>() {
3940
@Override
@@ -46,8 +47,9 @@ public String get() {
4647
.feature(FeatureFlag.TIME_SERIES_MODE);
4748

4849
// Avoid triggering bogus assertion when serialized parsed mappings don't match with original mappings, because _source key is
49-
// inconsistent
50-
if (oldVersion.before(Version.fromString("8.18.0"))) {
50+
// inconsistent. As usual, we operate under the premise that "versionless" clusters (serverless) are on the latest code and
51+
// do not need this.
52+
if (Version.tryParse(getOldClusterVersion()).map(v -> v.before(Version.fromString("8.18.0"))).orElse(false)) {
5153
cluster.jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper");
5254
cluster.jvmArg("-da:org.elasticsearch.index.mapper.MapperService");
5355
}

‎qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/AbstractRollingUpgradeWithSecurityTestCase.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,11 @@ public abstract class AbstractRollingUpgradeWithSecurityTestCase extends Paramet
3535
private static final ElasticsearchCluster cluster = buildCluster();
3636

3737
private static ElasticsearchCluster buildCluster() {
38-
Version oldVersion = Version.fromString(OLD_CLUSTER_VERSION);
38+
// Note we need to use OLD_CLUSTER_VERSION directly here, as it may contain special values (e.g. 0.0.0) the ElasticsearchCluster
39+
// builder uses to lookup a particular distribution
3940
var cluster = ElasticsearchCluster.local()
4041
.distribution(DistributionType.DEFAULT)
41-
.version(getOldClusterTestVersion())
42+
.version(OLD_CLUSTER_VERSION)
4243
.nodes(NODE_NUM)
4344
.user(USER, PASS)
4445
.setting("xpack.security.autoconfiguration.enabled", "false")
@@ -51,8 +52,8 @@ public String get() {
5152
});
5253

5354
// Avoid triggering bogus assertion when serialized parsed mappings don't match with original mappings, because _source key is
54-
// inconsistent
55-
if (oldVersion.before(Version.fromString("8.18.0"))) {
55+
// inconsistent. Assume non-parseable versions (serverless) do not need this.
56+
if (Version.tryParse(getOldClusterVersion()).map(v -> v.before(Version.fromString("8.18.0"))).orElse(false)) {
5657
cluster.jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper");
5758
cluster.jvmArg("-da:org.elasticsearch.index.mapper.MapperService");
5859
}

‎qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/DenseVectorMappingUpdateIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public DenseVectorMappingUpdateIT(@Name("upgradedNodes") int upgradedNodes) {
8282
}
8383

8484
public void testDenseVectorMappingUpdateOnOldCluster() throws IOException {
85-
if (getOldClusterTestVersion().after(Version.V_8_7_0.toString())) {
85+
if (oldClusterHasFeature("gte_v8.7.1")) {
8686
String indexName = "test_index";
8787
if (isOldCluster()) {
8888
Request createIndex = new Request("PUT", "/" + indexName);

‎qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/FileSettingsUpgradeIT.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.client.Request;
1515
import org.elasticsearch.common.xcontent.support.XContentMapValues;
1616
import org.elasticsearch.core.SuppressForbidden;
17+
import org.elasticsearch.core.UpdateForV10;
1718
import org.elasticsearch.test.cluster.ElasticsearchCluster;
1819
import org.elasticsearch.test.cluster.FeatureFlag;
1920
import org.elasticsearch.test.cluster.local.distribution.DistributionType;
@@ -33,8 +34,12 @@
3334

3435
public class FileSettingsUpgradeIT extends ParameterizedRollingUpgradeTestCase {
3536

37+
@UpdateForV10(owner = UpdateForV10.Owner.CORE_INFRA) // Remove this rule entirely
3638
private static final RunnableTestRuleAdapter versionLimit = new RunnableTestRuleAdapter(
37-
() -> assumeTrue("Only valid when upgrading from pre-file settings", getOldClusterTestVersion().before(new Version(8, 4, 0)))
39+
() -> assumeTrue(
40+
"Only valid when upgrading from pre-file settings",
41+
Version.tryParse(getOldClusterVersion()).map(v -> v.before(new Version(8, 4, 0))).orElse(false)
42+
)
3843
);
3944

4045
private static final String settingsJSON = """
@@ -52,9 +57,11 @@ public class FileSettingsUpgradeIT extends ParameterizedRollingUpgradeTestCase {
5257

5358
private static final TemporaryFolder repoDirectory = new TemporaryFolder();
5459

60+
// Note we need to use OLD_CLUSTER_VERSION directly here, as it may contain special values (e.g. 0.0.0) the ElasticsearchCluster
61+
// builder uses to lookup a particular distribution
5562
private static final ElasticsearchCluster cluster = ElasticsearchCluster.local()
5663
.distribution(DistributionType.DEFAULT)
57-
.version(getOldClusterTestVersion())
64+
.version(OLD_CLUSTER_VERSION)
5865
.nodes(NODE_NUM)
5966
.setting("path.repo", new Supplier<>() {
6067
@Override

‎qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/LogsUsageRollingUpgradeIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public LogsUsageRollingUpgradeIT(@Name("upgradedNodes") int upgradedNodes) {
3030
}
3131

3232
public void testUsage() throws Exception {
33-
assumeTrue("logsdb.prior_logs_usage only gets set in 8.x", getOldClusterTestVersion().before("9.0.0"));
33+
assumeFalse("logsdb.prior_logs_usage only gets set in 8.x", oldClusterHasFeature("gte_v9.0.0"));
3434
String dataStreamName = "logs-mysql-error";
3535
if (isOldCluster()) {
3636
bulkIndex(dataStreamName, 4, 256, Instant.now());

‎qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/ParameterizedRollingUpgradeTestCase.java‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,6 @@ public static void resetNodes() {
127127
upgradeFailed = false;
128128
}
129129

130-
@Deprecated // Use the new testing framework and oldClusterHasFeature(feature) instead
131-
protected static String getOldClusterVersion() {
132-
return OLD_CLUSTER_VERSION;
133-
}
134-
135130
protected static boolean oldClusterHasFeature(String featureId) {
136131
assert oldClusterTestFeatureService != null;
137132
return oldClusterTestFeatureService.clusterHasFeature(featureId);
@@ -146,12 +141,20 @@ protected static IndexVersion getOldClusterIndexVersion() {
146141
return oldIndexVersion;
147142
}
148143

149-
protected static Version getOldClusterTestVersion() {
150-
return Version.fromString(OLD_CLUSTER_VERSION);
144+
/**
145+
* The version of the "old" (initial) cluster. It is an opaque string, do not even think about parsing it for version
146+
* comparison. Use (test) cluster features and {@link ParameterizedRollingUpgradeTestCase#oldClusterHasFeature} instead.
147+
*/
148+
protected static String getOldClusterVersion() {
149+
return System.getProperty("tests.bwc.main.version", OLD_CLUSTER_VERSION);
151150
}
152151

153-
protected static boolean isOldClusterVersion(String nodeVersion) {
154-
return OLD_CLUSTER_VERSION.equals(nodeVersion);
152+
protected static boolean isOldClusterVersion(String nodeVersion, String buildHash) {
153+
String bwcRefSpec = System.getProperty("tests.bwc.refspec.main");
154+
if (bwcRefSpec != null) {
155+
return bwcRefSpec.equals(buildHash);
156+
}
157+
return getOldClusterVersion().equals(nodeVersion);
155158
}
156159

157160
protected static boolean isOldCluster() {

‎qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/SnapshotBasedRecoveryIT.java‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,21 +88,22 @@ public void testSnapshotBasedRecovery() throws Exception {
8888
}
8989

9090
String primaryNodeId = getPrimaryNodeIdOfShard(indexName, 0);
91-
String primaryNodeVersion = getNodeVersion(primaryNodeId);
91+
var primaryNodeVersion = getNodeVersion(primaryNodeId);
9292

9393
// Sometimes the primary shard ends on the upgraded node (i.e. after a rebalance)
9494
// This causes issues when removing and adding replicas, since then we cannot allocate to any of the old nodes.
9595
// That is an issue only for the first mixed round.
9696
// In that case we exclude the upgraded node from the shard allocation and cancel the shard to force moving
9797
// the primary to a node in the old version, this allows adding replicas in the first mixed round.
9898
logger.info("--> Primary node in first mixed round {} / {}", primaryNodeId, primaryNodeVersion);
99-
if (isOldClusterVersion(primaryNodeVersion) == false) {
99+
if (isOldClusterVersion(primaryNodeVersion.version(), primaryNodeVersion.buildHash()) == false) {
100100
logger.info("--> cancelling primary shard on node [{}]", primaryNodeId);
101101
cancelShard(indexName, 0, primaryNodeId);
102102
logger.info("--> done cancelling primary shard on node [{}]", primaryNodeId);
103103

104104
String currentPrimaryNodeId = getPrimaryNodeIdOfShard(indexName, 0);
105-
assertTrue(isOldClusterVersion(getNodeVersion(currentPrimaryNodeId)));
105+
var currentPrimaryNodeVersion = getNodeVersion(currentPrimaryNodeId);
106+
assertTrue(isOldClusterVersion(currentPrimaryNodeVersion.version(), currentPrimaryNodeVersion.buildHash()));
106107
}
107108
} else {
108109
logger.info("--> not in first upgrade round, removing exclusions for [{}]", indexName);
@@ -137,17 +138,24 @@ private List<String> getUpgradedNodeIds() throws IOException {
137138
List<String> upgradedNodes = new ArrayList<>();
138139
for (Map.Entry<String, Map<String, Object>> nodeInfoEntry : nodes.entrySet()) {
139140
String nodeVersion = extractValue(nodeInfoEntry.getValue(), "version");
140-
if (isOldClusterVersion(nodeVersion) == false) {
141+
String nodeBuildHash = extractValue(nodeInfoEntry.getValue(), "build_hash");
142+
if (isOldClusterVersion(nodeVersion, nodeBuildHash) == false) {
141143
upgradedNodes.add(nodeInfoEntry.getKey());
142144
}
143145
}
144146
return upgradedNodes;
145147
}
146148

147-
private String getNodeVersion(String primaryNodeId) throws IOException {
149+
private record NodeVersion(String version, String buildHash) {}
150+
151+
private NodeVersion getNodeVersion(String primaryNodeId) throws IOException {
148152
Request request = new Request(HttpGet.METHOD_NAME, "_nodes/" + primaryNodeId);
149153
Response response = client().performRequest(request);
150-
return extractValue(responseAsMap(response), "nodes." + primaryNodeId + ".version");
154+
Map<String, Object> responseAsMap = responseAsMap(response);
155+
return new NodeVersion(
156+
extractValue(responseAsMap, "nodes." + primaryNodeId + ".version"),
157+
extractValue(responseAsMap, "nodes." + primaryNodeId + ".build_hash")
158+
);
151159
}
152160

153161
private String getPrimaryNodeIdOfShard(String indexName, int shard) throws Exception {

‎qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/SourceModeRollingUpgradeIT.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public SourceModeRollingUpgradeIT(@Name("upgradedNodes") int upgradedNodes) {
2727
}
2828

2929
public void testConfigureStoredSourceBeforeIndexCreationLegacy() throws IOException {
30-
assumeTrue("testing deprecation warnings and deprecation migrations", getOldClusterTestVersion().before("9.0.0"));
30+
assumeFalse("testing deprecation warnings and deprecation migrations", oldClusterHasFeature("gte_v9.0.0"));
3131
String templateName = "logs@custom";
3232
if (isOldCluster()) {
3333
var storedSourceMapping = """
@@ -56,7 +56,7 @@ public void testConfigureStoredSourceBeforeIndexCreationLegacy() throws IOExcept
5656
}
5757

5858
public void testConfigureStoredSourceWhenIndexIsCreatedLegacy() throws IOException {
59-
assumeTrue("testing deprecation warnings and deprecation migrations", getOldClusterTestVersion().before("9.0.0"));
59+
assumeFalse("testing deprecation warnings and deprecation migrations", oldClusterHasFeature("gte_v9.0.0"));
6060
String templateName = "logs@custom";
6161
if (isOldCluster()) {
6262
var storedSourceMapping = """

‎qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/VectorSearchIT.java‎

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,16 @@ public VectorSearchIT(@Name("upgradedNodes") int upgradedNodes) {
3737
private static final String BBQ_INDEX_NAME = "bbq_vector_index";
3838
private static final String FLAT_QUANTIZED_INDEX_NAME = "flat_quantized_vector_index";
3939
private static final String FLAT_BBQ_INDEX_NAME = "flat_bbq_vector_index";
40-
private static final String FLOAT_VECTOR_SEARCH_VERSION = "8.4.0";
41-
private static final String BYTE_VECTOR_SEARCH_VERSION = "8.6.0";
42-
private static final String QUANTIZED_VECTOR_SEARCH_VERSION = "8.12.1";
43-
private static final String FLAT_QUANTIZED_VECTOR_SEARCH_VERSION = "8.13.0";
44-
private static final String BBQ_VECTOR_SEARCH_VERSION = "8.18.0";
40+
41+
// TODO: replace these with proper test features
42+
private static final String FLOAT_VECTOR_SEARCH_TEST_FEATURE = "gte_v8.4.0";
43+
private static final String BYTE_VECTOR_SEARCH_TEST_FEATURE = "gte_v8.6.0";
44+
private static final String QUANTIZED_VECTOR_SEARCH_TEST_FEATURE = "gte_v8.12.1";
45+
private static final String FLAT_QUANTIZED_VECTOR_SEARCH_TEST_FEATURE = "gte_v8.13.0";
46+
private static final String BBQ_VECTOR_SEARCH_TEST_FEATURE = "gte_v8.18.0";
4547

4648
public void testScriptByteVectorSearch() throws Exception {
47-
assumeTrue("byte vector search is not supported on this version", getOldClusterTestVersion().onOrAfter(BYTE_VECTOR_SEARCH_VERSION));
49+
assumeTrue("byte vector search is not supported on this version", oldClusterHasFeature(BYTE_VECTOR_SEARCH_TEST_FEATURE));
4850
if (isOldCluster()) {
4951
// create index and index 10 random floating point vectors
5052
String mapping = """
@@ -91,10 +93,7 @@ public void testScriptByteVectorSearch() throws Exception {
9193
}
9294

9395
public void testScriptVectorSearch() throws Exception {
94-
assumeTrue(
95-
"Float vector search is not supported on this version",
96-
getOldClusterTestVersion().onOrAfter(FLOAT_VECTOR_SEARCH_VERSION)
97-
);
96+
assumeTrue("Float vector search is not supported on this version", oldClusterHasFeature(FLOAT_VECTOR_SEARCH_TEST_FEATURE));
9897
if (isOldCluster()) {
9998
// create index and index 10 random floating point vectors
10099
String mapping = """
@@ -140,10 +139,7 @@ public void testScriptVectorSearch() throws Exception {
140139
}
141140

142141
public void testFloatVectorSearch() throws Exception {
143-
assumeTrue(
144-
"Float vector search is not supported on this version",
145-
getOldClusterTestVersion().onOrAfter(FLOAT_VECTOR_SEARCH_VERSION)
146-
);
142+
assumeTrue("Float vector search is not supported on this version", oldClusterHasFeature(FLOAT_VECTOR_SEARCH_TEST_FEATURE));
147143
if (isOldCluster()) {
148144
String mapping = """
149145
{
@@ -215,7 +211,7 @@ public void testFloatVectorSearch() throws Exception {
215211
}
216212

217213
public void testByteVectorSearch() throws Exception {
218-
assumeTrue("Byte vector search is not supported on this version", getOldClusterTestVersion().onOrAfter(BYTE_VECTOR_SEARCH_VERSION));
214+
assumeTrue("Byte vector search is not supported on this version", oldClusterHasFeature(BYTE_VECTOR_SEARCH_TEST_FEATURE));
219215
if (isOldCluster()) {
220216
String mapping = """
221217
{
@@ -288,10 +284,7 @@ public void testByteVectorSearch() throws Exception {
288284
}
289285

290286
public void testQuantizedVectorSearch() throws Exception {
291-
assumeTrue(
292-
"Quantized vector search is not supported on this version",
293-
getOldClusterTestVersion().onOrAfter(QUANTIZED_VECTOR_SEARCH_VERSION)
294-
);
287+
assumeTrue("Quantized vector search is not supported on this version", oldClusterHasFeature(QUANTIZED_VECTOR_SEARCH_TEST_FEATURE));
295288
if (isOldCluster()) {
296289
String mapping = """
297290
{
@@ -364,7 +357,7 @@ public void testQuantizedVectorSearch() throws Exception {
364357
public void testFlatQuantizedVectorSearch() throws Exception {
365358
assumeTrue(
366359
"Quantized vector search is not supported on this version",
367-
getOldClusterTestVersion().onOrAfter(FLAT_QUANTIZED_VECTOR_SEARCH_VERSION)
360+
oldClusterHasFeature(FLAT_QUANTIZED_VECTOR_SEARCH_TEST_FEATURE)
368361
);
369362
if (isOldCluster()) {
370363
String mapping = """
@@ -434,10 +427,7 @@ public void testFlatQuantizedVectorSearch() throws Exception {
434427
}
435428

436429
public void testBBQVectorSearch() throws Exception {
437-
assumeTrue(
438-
"Quantized vector search is not supported on this version",
439-
getOldClusterTestVersion().onOrAfter(BBQ_VECTOR_SEARCH_VERSION)
440-
);
430+
assumeTrue("Quantized vector search is not supported on this version", oldClusterHasFeature(BBQ_VECTOR_SEARCH_TEST_FEATURE));
441431
if (isOldCluster()) {
442432
String mapping = """
443433
{
@@ -518,10 +508,7 @@ public void testBBQVectorSearch() throws Exception {
518508
}
519509

520510
public void testFlatBBQVectorSearch() throws Exception {
521-
assumeTrue(
522-
"Quantized vector search is not supported on this version",
523-
getOldClusterTestVersion().onOrAfter(BBQ_VECTOR_SEARCH_VERSION)
524-
);
511+
assumeTrue("Quantized vector search is not supported on this version", oldClusterHasFeature(BBQ_VECTOR_SEARCH_TEST_FEATURE));
525512
if (isOldCluster()) {
526513
String mapping = """
527514
{

0 commit comments

Comments
 (0)