Fix tests involving getBestDowngradeVersion

Author: prdoyle

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

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
+     * <p>
+     * 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.)
      *
-     * <p>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

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")
         );
     }