Skip to content

Commit

Permalink
Addressing more Version usages in tests (#102803)
Browse files Browse the repository at this point in the history
  • Loading branch information
ldematte committed Dec 12, 2023
1 parent 2f01a37 commit 93bd1ab
Show file tree
Hide file tree
Showing 11 changed files with 80 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import java.util.List;
import java.util.Map;

import static org.elasticsearch.cluster.ClusterState.VERSION_INTRODUCING_TRANSPORT_VERSIONS;
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;

/**
Expand Down Expand Up @@ -251,9 +252,9 @@ public void testQueryBuilderBWC() throws Exception {
StreamInput input = new NamedWriteableAwareStreamInput(new InputStreamStreamInput(in), registry)
) {

@UpdateForV9 // always true
@UpdateForV9 // condition will always be true
var originalClusterHasTransportVersion = parseLegacyVersion(getOldClusterVersion()).map(
v -> v.onOrAfter(Version.V_8_8_0)
v -> v.onOrAfter(VERSION_INTRODUCING_TRANSPORT_VERSIONS)
).orElse(true);

final TransportVersion transportVersion;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1031,9 +1031,11 @@ public static ClusterState readFrom(StreamInput in, DiscoveryNode localNode) thr
*/
public static final TransportVersion INFERRED_TRANSPORT_VERSION = TransportVersions.V_8_8_0;

public static final Version VERSION_INTRODUCING_TRANSPORT_VERSIONS = Version.V_8_8_0;

private static TransportVersion inferTransportVersion(DiscoveryNode node) {
TransportVersion tv;
if (node.getVersion().before(Version.V_8_8_0)) {
if (node.getVersion().before(VERSION_INTRODUCING_TRANSPORT_VERSIONS)) {
// 1-to-1 mapping between Version and TransportVersion
tv = TransportVersion.fromId(node.getPre811VersionId().getAsInt());
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@
import static java.util.Collections.sort;
import static java.util.Collections.unmodifiableList;
import static org.elasticsearch.client.RestClient.IGNORE_RESPONSE_CODES_PARAM;
import static org.elasticsearch.cluster.ClusterState.VERSION_INTRODUCING_TRANSPORT_VERSIONS;
import static org.elasticsearch.core.Strings.format;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.containsString;
Expand Down Expand Up @@ -2149,7 +2150,7 @@ protected static TransportVersion getTransportVersionWithFallback(
// In that case the transport_version field won't exist. Use version, but only for <8.8.0: after that versions diverge.
var version = parseLegacyVersion(versionField);
assert version.isPresent();
if (version.get().before(Version.V_8_8_0)) {
if (version.get().before(VERSION_INTRODUCING_TRANSPORT_VERSIONS)) {
return TransportVersion.fromId(version.get().id);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.Map;

import static java.util.Map.entry;
import static org.elasticsearch.cluster.ClusterState.VERSION_INTRODUCING_TRANSPORT_VERSIONS;

/**
* This class groups historical features that have been removed from the production codebase, but are still used by the test
Expand All @@ -37,6 +38,19 @@ public class RestTestLegacyFeatures implements FeatureSpecification {
public static final NodeFeature DELETE_TEMPLATE_MULTIPLE_NAMES_SUPPORTED = new NodeFeature(
"indices.delete_template_multiple_names_supported"
);
public static final NodeFeature ML_NEW_MEMORY_FORMAT = new NodeFeature("ml.new_memory_format");

/** These are "pure test" features: normally we would not need them, and test for TransportVersion/fallback to Version (see for example
* {@code ESRestTestCase#minimumTransportVersion()}. However, some tests explicitly check and validate the content of a response, so
* we need these features to support them.
*/
public static final NodeFeature TRANSPORT_VERSION_SUPPORTED = new NodeFeature("transport_version_supported");
public static final NodeFeature STATE_REPLACED_TRANSPORT_VERSION_WITH_NODES_VERSION = new NodeFeature(
"state.transport_version_to_nodes_version"
);

// Ref: https://github.com/elastic/elasticsearch/pull/86416
public static final NodeFeature ML_MEMORY_OVERHEAD_FIXED = new NodeFeature("ml.memory_overhead_fixed");

// QA - rolling upgrade tests
public static final NodeFeature SECURITY_UPDATE_API_KEY = new NodeFeature("security.api_key_update");
Expand Down Expand Up @@ -73,6 +87,10 @@ public Map<NodeFeature, Version> getHistoricalFeatures() {
entry(ML_STATE_RESET_FALLBACK_ON_DISABLED, Version.V_8_7_0),
entry(SECURITY_UPDATE_API_KEY, Version.V_8_4_0),
entry(SECURITY_BULK_UPDATE_API_KEY, Version.V_8_5_0),
entry(ML_NEW_MEMORY_FORMAT, Version.V_8_11_0),
entry(TRANSPORT_VERSION_SUPPORTED, VERSION_INTRODUCING_TRANSPORT_VERSIONS),
entry(STATE_REPLACED_TRANSPORT_VERSION_WITH_NODES_VERSION, Version.V_8_11_0),
entry(ML_MEMORY_OVERHEAD_FIXED, Version.V_8_2_1),
entry(WATCHES_VERSION_IN_META, Version.V_7_13_0),
entry(SECURITY_ROLE_DESCRIPTORS_OPTIONAL, Version.V_7_3_0),
entry(SEARCH_AGGREGATIONS_FORCE_INTERVAL_SELECTION_DATE_HISTOGRAM, Version.V_7_2_0),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import java.util.zip.ZipEntry;

import static java.util.Collections.emptyMap;
import static org.elasticsearch.cluster.ClusterState.VERSION_INTRODUCING_TRANSPORT_VERSIONS;
import static org.elasticsearch.test.ESTestCase.between;
import static org.elasticsearch.test.ESTestCase.randomAlphaOfLength;
import static org.elasticsearch.test.ESTestCase.randomBoolean;
Expand Down Expand Up @@ -313,7 +314,7 @@ public static TestNodes buildNodeAndVersions(RestClient client, String bwcNodesV
// this json might be from a node <8.8.0, but about a node >=8.8.0
// In that case the transport_version field won't exist. Just ignore it for now.
Version version = Version.fromString(nodeVersion);
if (version.before(Version.V_8_8_0)) {
if (version.before(VERSION_INTRODUCING_TRANSPORT_VERSIONS)) {
transportVersion = TransportVersion.fromId(version.id);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package org.elasticsearch.upgrades;

import org.elasticsearch.Build;
import org.elasticsearch.Version;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
Expand Down Expand Up @@ -38,6 +39,15 @@ protected static boolean isOriginalCluster(String clusterVersion) {
return UPGRADE_FROM_VERSION.equals(clusterVersion);
}

/**
* Upgrade tests by design are also executed with the same version. We might want to skip some checks if that's the case, see
* for example gh#39102.
* @return true if the cluster version is the current version.
*/
protected static boolean isOriginalClusterCurrent() {
return UPGRADE_FROM_VERSION.equals(Build.current().version());
}

@Deprecated(forRemoval = true)
@UpdateForV9
// Tests should be reworked to rely on features from the current cluster (old, mixed or upgraded).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.logging.LogManager;
import org.elasticsearch.logging.Logger;
import org.elasticsearch.test.rest.RestTestLegacyFeatures;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -86,7 +87,7 @@ public void testMlAssignmentPlannerUpgrade() throws Exception {

// assert correct memory format is used
assertOldMemoryFormat("old_memory_format");
if (isOriginalClusterVersionAtLeast(Version.V_8_11_0)) {
if (clusterHasFeature(RestTestLegacyFeatures.ML_NEW_MEMORY_FORMAT)) {
assertNewMemoryFormat("new_memory_format");
} else {
assertOldMemoryFormat("new_memory_format");
Expand All @@ -102,7 +103,7 @@ public void testMlAssignmentPlannerUpgrade() throws Exception {

// assert correct memory format is used
assertOldMemoryFormat("old_memory_format");
if (isOriginalClusterVersionAtLeast(Version.V_8_11_0)) {
if (clusterHasFeature(RestTestLegacyFeatures.ML_NEW_MEMORY_FORMAT)) {
assertNewMemoryFormat("new_memory_format");
} else {
assertOldMemoryFormat("new_memory_format");
Expand Down Expand Up @@ -141,7 +142,7 @@ private void waitForDeploymentStarted(String modelId) throws Exception {
@SuppressWarnings("unchecked")
private void assertOldMemoryFormat(String modelId) throws Exception {
// There was a change in the MEMORY_OVERHEAD value in 8.3.0, see #86416
long memoryOverheadMb = Version.fromString(UPGRADE_FROM_VERSION).onOrAfter(Version.V_8_2_1) ? 240 : 270;
long memoryOverheadMb = clusterHasFeature(RestTestLegacyFeatures.ML_MEMORY_OVERHEAD_FIXED) ? 240 : 270;
var response = getTrainedModelStats(modelId);
Map<String, Object> map = entityAsMap(response);
List<Map<String, Object>> stats = (List<Map<String, Object>>) map.get("trained_model_stats");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
package org.elasticsearch.upgrades;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.Response;
Expand Down Expand Up @@ -76,10 +75,7 @@ public void testSnapshotUpgrader() throws Exception {
switch (CLUSTER_TYPE) {
case OLD -> createJobAndSnapshots();
case MIXED -> {
assumeTrue(
"We should only test if old cluster is before new cluster",
isOriginalClusterVersionAtLeast(Version.CURRENT) == false
);
assumeTrue("We should only test if old cluster is before new cluster", isOriginalClusterCurrent() == false);
ensureHealth((request -> {
request.addParameter("timeout", "70s");
request.addParameter("wait_for_nodes", "3");
Expand All @@ -88,10 +84,7 @@ public void testSnapshotUpgrader() throws Exception {
testSnapshotUpgradeFailsOnMixedCluster();
}
case UPGRADED -> {
assumeTrue(
"We should only test if old cluster is before new cluster",
isOriginalClusterVersionAtLeast(Version.CURRENT) == false
);
assumeTrue("We should only test if old cluster is before new cluster", isOriginalClusterCurrent() == false);
ensureHealth((request -> {
request.addParameter("timeout", "70s");
request.addParameter("wait_for_nodes", "3");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import org.apache.http.HttpHost;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.elasticsearch.Build;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.RestClient;
Expand Down Expand Up @@ -235,7 +234,7 @@ private void verifyContinuousTransformHandlesData(long expectedLastCheckpoint) t

private void verifyUpgradeFailsIfMixedCluster() {
// upgrade tests by design are also executed with the same version, this check must be skipped in this case, see gh#39102.
if (isOriginalCluster(Build.current().version())) {
if (isOriginalClusterCurrent()) {
return;
}
final Request upgradeTransformRequest = new Request("POST", getTransformEndpoint() + "_upgrade");
Expand Down

0 comments on commit 93bd1ab

Please sign in to comment.