From cd73065794db8aab0f93f07e5c61bb03670af1c7 Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Fri, 2 May 2025 11:29:23 -0400 Subject: [PATCH 1/3] Fix tests involving getBestDowngradeVersion --- muted-tests.yml | 6 -- .../elasticsearch/env/NodeEnvironment.java | 12 +++- .../env/NodeEnvironmentTests.java | 56 +++++++++++-------- 3 files changed, 41 insertions(+), 33 deletions(-) diff --git a/muted-tests.yml b/muted-tests.yml index b45b9f7840d85..df71fb4312572 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -185,9 +185,6 @@ tests: issue: https://github.com/elastic/elasticsearch/issues/121165 - class: org.elasticsearch.xpack.security.authc.ldap.ActiveDirectorySessionFactoryTests issue: https://github.com/elastic/elasticsearch/issues/121285 -- class: org.elasticsearch.env.NodeEnvironmentTests - method: testGetBestDowngradeVersion - issue: https://github.com/elastic/elasticsearch/issues/121316 - class: org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT issue: https://github.com/elastic/elasticsearch/issues/121407 - class: org.elasticsearch.analysis.common.CommonAnalysisClientYamlTestSuiteIT @@ -234,9 +231,6 @@ tests: - class: org.elasticsearch.smoketest.MlWithSecurityIT method: test {yaml=ml/3rd_party_deployment/Test start and stop multiple deployments} issue: https://github.com/elastic/elasticsearch/issues/124315 -- class: org.elasticsearch.env.NodeEnvironmentTests - method: testIndexCompatibilityChecks - issue: https://github.com/elastic/elasticsearch/issues/124388 - class: org.elasticsearch.multiproject.test.CoreWithMultipleProjectsClientYamlTestSuiteIT method: test {yaml=data_stream/190_failure_store_redirection/Redirect ingest failure in data stream to failure store} issue: https://github.com/elastic/elasticsearch/issues/124518 diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index febde6b6a69ac..5a9ec019dcaed 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -23,6 +23,7 @@ import org.apache.lucene.store.NativeFSLockFactory; import org.elasticsearch.Build; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodeRole; @@ -1501,10 +1502,15 @@ private static void tryWriteTempFile(Path path) throws IOException { /** * Get a useful version string to direct a user's downgrade operation + *

+ * Assuming that the index was compatible with {@code previousNodeVersion}, + * the user should downgrade to that {@code previousNodeVersion}, + * unless it's prior to the minimum compatible version, + * in which case the user should downgrade to that instead. + * (If the index version is so old that the minimum compatible version is incompatible with the index, + * then the cluster was already borked before the node upgrade began, + * and we can't probably help them without more info than we have here.) * - *

If a user is trying to install current major N but has incompatible indices, the user should - * downgrade to last minor of the previous major (N-1).last. We return (N-1).last, unless the user is trying to upgrade from - * a (N-1).last.x release, in which case we return the last installed version. * @return Version to downgrade to */ // visible for testing diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index 497795ec4d2c5..6caf29f836834 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -582,9 +582,9 @@ public void testIndexCompatibilityChecks() throws IOException { containsString("it holds metadata for indices with version [" + oldIndexVersion.toReleaseVersion() + "]"), containsString( "Revert this node to version [" - + (previousNodeVersion.major == Version.CURRENT.major - ? Version.CURRENT.minimumCompatibilityVersion() - : previousNodeVersion) + + (previousNodeVersion.onOrAfter(Version.CURRENT.minimumCompatibilityVersion()) + ? previousNodeVersion + : Version.CURRENT.minimumCompatibilityVersion()) + "]" ) ) @@ -639,29 +639,37 @@ public void testSymlinkDataDirectory() throws Exception { } public void testGetBestDowngradeVersion() { - assertThat( - NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.18.0")), - Matchers.equalTo(BuildVersion.fromString("8.18.0")) - ); - assertThat( - NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.18.5")), - Matchers.equalTo(BuildVersion.fromString("8.18.5")) - ); - assertThat( - NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.18.12")), - Matchers.equalTo(BuildVersion.fromString("8.18.12")) - ); - assertThat( - NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.19.0")), - Matchers.equalTo(BuildVersion.fromString("8.19.0")) + int prev = Version.CURRENT.minimumCompatibilityVersion().major; + int last = Version.CURRENT.minimumCompatibilityVersion().minor; + int old = prev-1; + + assumeTrue("The current compatibility rules are active only from 9.x onward", prev >= 7); + assertEquals(Version.CURRENT.major-1, prev); + + assertEquals( + "From an old major, recommend prev.last", + NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString(old + ".0.0")), + BuildVersion.fromString(prev + "." + last + ".0") ); - assertThat( - NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.17.0")), - Matchers.equalTo(BuildVersion.fromString("8.18.0")) + + if (last >= 1) { + assertEquals( + "From an old minor of the previous major, recommend prev.last", + NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString(prev + "." + (last - 1) + ".0")), + BuildVersion.fromString(prev + "." + last + ".0") + ); + } + + assertEquals( + "From an old patch of prev.last, return that version itself", + NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString(prev + "." + last + ".1")), + BuildVersion.fromString(prev + "." + last + ".1") ); - assertThat( - NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("7.17.0")), - Matchers.equalTo(BuildVersion.fromString("8.18.0")) + + assertEquals( + "From the first version of this major, return that version itself", + NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString(Version.CURRENT.major + ".0.0")), + BuildVersion.fromString(Version.CURRENT.major + ".0.0") ); } From 230132406420b6fb7ad6037bc4fac0c3ccd26e64 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Fri, 2 May 2025 15:42:11 +0000 Subject: [PATCH 2/3] [CI] Auto commit changes from spotless --- .../src/main/java/org/elasticsearch/env/NodeEnvironment.java | 1 - .../java/org/elasticsearch/env/NodeEnvironmentTests.java | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index 5a9ec019dcaed..ddc36cc81dda1 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -23,7 +23,6 @@ import org.apache.lucene.store.NativeFSLockFactory; import org.elasticsearch.Build; import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodeRole; diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index 6caf29f836834..1fb84c865801d 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -42,7 +42,6 @@ import org.elasticsearch.test.MockLog; import org.elasticsearch.test.NodeRoles; import org.elasticsearch.test.junit.annotations.TestLogging; -import org.hamcrest.Matchers; import org.junit.AssumptionViolatedException; import java.io.IOException; @@ -641,10 +640,10 @@ public void testSymlinkDataDirectory() throws Exception { public void testGetBestDowngradeVersion() { int prev = Version.CURRENT.minimumCompatibilityVersion().major; int last = Version.CURRENT.minimumCompatibilityVersion().minor; - int old = prev-1; + int old = prev - 1; assumeTrue("The current compatibility rules are active only from 9.x onward", prev >= 7); - assertEquals(Version.CURRENT.major-1, prev); + assertEquals(Version.CURRENT.major - 1, prev); assertEquals( "From an old major, recommend prev.last", From 9c3d55f3a98130b981025d828e4233858a930bc9 Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Fri, 2 May 2025 11:51:39 -0400 Subject: [PATCH 3/3] Typo in assume message --- .../test/java/org/elasticsearch/env/NodeEnvironmentTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index 1fb84c865801d..9a7fadad33a53 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -642,7 +642,7 @@ public void testGetBestDowngradeVersion() { int last = Version.CURRENT.minimumCompatibilityVersion().minor; int old = prev - 1; - assumeTrue("The current compatibility rules are active only from 9.x onward", prev >= 7); + assumeTrue("The current compatibility rules are active only from 8.x onward", prev >= 7); assertEquals(Version.CURRENT.major - 1, prev); assertEquals(