Skip to content

Commit 64568ee

Browse files
prdoyleelasticsearchmachine
andauthored
Fix tests involving getBestDowngradeVersion (elastic#127646)
* Fix tests involving getBestDowngradeVersion * [CI] Auto commit changes from spotless * Typo in assume message --------- Co-authored-by: elasticsearchmachine <[email protected]>
1 parent 5df5cb8 commit 64568ee

File tree

3 files changed

+40
-34
lines changed

3 files changed

+40
-34
lines changed

muted-tests.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,6 @@ tests:
185185
issue: https://github.com/elastic/elasticsearch/issues/121165
186186
- class: org.elasticsearch.xpack.security.authc.ldap.ActiveDirectorySessionFactoryTests
187187
issue: https://github.com/elastic/elasticsearch/issues/121285
188-
- class: org.elasticsearch.env.NodeEnvironmentTests
189-
method: testGetBestDowngradeVersion
190-
issue: https://github.com/elastic/elasticsearch/issues/121316
191188
- class: org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT
192189
issue: https://github.com/elastic/elasticsearch/issues/121407
193190
- class: org.elasticsearch.analysis.common.CommonAnalysisClientYamlTestSuiteIT
@@ -234,9 +231,6 @@ tests:
234231
- class: org.elasticsearch.smoketest.MlWithSecurityIT
235232
method: test {yaml=ml/3rd_party_deployment/Test start and stop multiple deployments}
236233
issue: https://github.com/elastic/elasticsearch/issues/124315
237-
- class: org.elasticsearch.env.NodeEnvironmentTests
238-
method: testIndexCompatibilityChecks
239-
issue: https://github.com/elastic/elasticsearch/issues/124388
240234
- class: org.elasticsearch.multiproject.test.CoreWithMultipleProjectsClientYamlTestSuiteIT
241235
method: test {yaml=data_stream/190_failure_store_redirection/Redirect ingest failure in data stream to failure store}
242236
issue: https://github.com/elastic/elasticsearch/issues/124518

server/src/main/java/org/elasticsearch/env/NodeEnvironment.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1501,10 +1501,15 @@ private static void tryWriteTempFile(Path path) throws IOException {
15011501

15021502
/**
15031503
* Get a useful version string to direct a user's downgrade operation
1504+
* <p>
1505+
* Assuming that the index was compatible with {@code previousNodeVersion},
1506+
* the user should downgrade to that {@code previousNodeVersion},
1507+
* unless it's prior to the minimum compatible version,
1508+
* in which case the user should downgrade to that instead.
1509+
* (If the index version is so old that the minimum compatible version is incompatible with the index,
1510+
* then the cluster was already borked before the node upgrade began,
1511+
* and we can't probably help them without more info than we have here.)
15041512
*
1505-
* <p>If a user is trying to install current major N but has incompatible indices, the user should
1506-
* downgrade to last minor of the previous major (N-1).last. We return (N-1).last, unless the user is trying to upgrade from
1507-
* a (N-1).last.x release, in which case we return the last installed version.
15081513
* @return Version to downgrade to
15091514
*/
15101515
// visible for testing

server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import org.elasticsearch.test.MockLog;
4343
import org.elasticsearch.test.NodeRoles;
4444
import org.elasticsearch.test.junit.annotations.TestLogging;
45-
import org.hamcrest.Matchers;
4645
import org.junit.AssumptionViolatedException;
4746

4847
import java.io.IOException;
@@ -582,9 +581,9 @@ public void testIndexCompatibilityChecks() throws IOException {
582581
containsString("it holds metadata for indices with version [" + oldIndexVersion.toReleaseVersion() + "]"),
583582
containsString(
584583
"Revert this node to version ["
585-
+ (previousNodeVersion.major == Version.CURRENT.major
586-
? Version.CURRENT.minimumCompatibilityVersion()
587-
: previousNodeVersion)
584+
+ (previousNodeVersion.onOrAfter(Version.CURRENT.minimumCompatibilityVersion())
585+
? previousNodeVersion
586+
: Version.CURRENT.minimumCompatibilityVersion())
588587
+ "]"
589588
)
590589
)
@@ -639,29 +638,37 @@ public void testSymlinkDataDirectory() throws Exception {
639638
}
640639

641640
public void testGetBestDowngradeVersion() {
642-
assertThat(
643-
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.18.0")),
644-
Matchers.equalTo(BuildVersion.fromString("8.18.0"))
645-
);
646-
assertThat(
647-
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.18.5")),
648-
Matchers.equalTo(BuildVersion.fromString("8.18.5"))
649-
);
650-
assertThat(
651-
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.18.12")),
652-
Matchers.equalTo(BuildVersion.fromString("8.18.12"))
653-
);
654-
assertThat(
655-
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.19.0")),
656-
Matchers.equalTo(BuildVersion.fromString("8.19.0"))
641+
int prev = Version.CURRENT.minimumCompatibilityVersion().major;
642+
int last = Version.CURRENT.minimumCompatibilityVersion().minor;
643+
int old = prev - 1;
644+
645+
assumeTrue("The current compatibility rules are active only from 8.x onward", prev >= 7);
646+
assertEquals(Version.CURRENT.major - 1, prev);
647+
648+
assertEquals(
649+
"From an old major, recommend prev.last",
650+
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString(old + ".0.0")),
651+
BuildVersion.fromString(prev + "." + last + ".0")
657652
);
658-
assertThat(
659-
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.17.0")),
660-
Matchers.equalTo(BuildVersion.fromString("8.18.0"))
653+
654+
if (last >= 1) {
655+
assertEquals(
656+
"From an old minor of the previous major, recommend prev.last",
657+
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString(prev + "." + (last - 1) + ".0")),
658+
BuildVersion.fromString(prev + "." + last + ".0")
659+
);
660+
}
661+
662+
assertEquals(
663+
"From an old patch of prev.last, return that version itself",
664+
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString(prev + "." + last + ".1")),
665+
BuildVersion.fromString(prev + "." + last + ".1")
661666
);
662-
assertThat(
663-
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("7.17.0")),
664-
Matchers.equalTo(BuildVersion.fromString("8.18.0"))
667+
668+
assertEquals(
669+
"From the first version of this major, return that version itself",
670+
NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString(Version.CURRENT.major + ".0.0")),
671+
BuildVersion.fromString(Version.CURRENT.major + ".0.0")
665672
);
666673
}
667674

0 commit comments

Comments
 (0)