Fix failing test caused by versioning change. (opensearch-project#598)

The change in 0ba0e7c, introduced the issue where randomly selecting an incompatible version fails the test. It caused the filtering logic to incorrectly identify all ES 7.*.* versions as bad versions for joining which should not be the case. Additionally, split the test into two separate tests where earlier only one of them was run at random. Signed-off-by: Rabi Panda <[email protected]>
sashashura · Apr 23, 2021 · 821c44c · 821c44c
1 parent 6e04778
commit 821c44c
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 9 deletions.
diff --git a/server/src/main/java/org/opensearch/Version.java b/server/src/main/java/org/opensearch/Version.java
@@ -234,6 +234,13 @@ public boolean onOrBefore(Version version) {
         return version.id >= id;
     }
 
+    // LegacyESVersion major 7 is equivalent to Version major 1
+    public int compareMajor(Version other) {
+        int m = major == 1 ? 7 : major;
+        int om = other.major == 1 ? 7 : other.major;
+        return Integer.compare(m, om);
+    }
+
     @Override
     public int compareTo(Version other) {
         return Integer.compare(this.id, other.id);

diff --git a/server/src/test/java/org/opensearch/VersionTests.java b/server/src/test/java/org/opensearch/VersionTests.java
@@ -88,6 +88,10 @@ public void testVersionComparison() throws Exception {
         assertThat(V_6_3_0, is(lessThan(V_7_0_0)));
         assertThat(V_6_3_0.compareTo(V_6_3_0), is(0));
         assertThat(V_7_0_0, is(greaterThan(V_6_3_0)));
+
+        assertThat(Version.V_1_0_0.compareMajor(LegacyESVersion.V_7_0_0), is(0));
+        assertThat(Version.V_1_0_0.compareMajor(LegacyESVersion.V_6_3_0), is(1));
+        assertThat(LegacyESVersion.V_6_3_0.compareMajor(Version.V_1_0_0), is(-1));
     }
 
     public void testMin() {

diff --git a/server/src/test/java/org/opensearch/discovery/zen/NodeJoinControllerTests.java b/server/src/test/java/org/opensearch/discovery/zen/NodeJoinControllerTests.java
@@ -600,20 +600,24 @@ public void testRejectingRestartedNodeJoinsBeforeProcessingNodeLeft() throws Int
         assertThat(e.getMessage(), containsString("found existing node"));
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/issues/577")
-    public void testRejectingJoinWithIncompatibleVersion() throws InterruptedException, ExecutionException {
+    public void testRejectingJoinWithBeforeMinCompatibleVersion() throws ExecutionException, InterruptedException {
+        final Version badVersion = getPreviousVersion(Version.CURRENT.minimumCompatibilityVersion());
+        assertRejectingJoinWithIncompatibleVersion(badVersion);
+    }
+
+    public void testRejectingJoinWithPreviousMajorVersion() throws ExecutionException, InterruptedException {
+        final Version badVersion =
+            randomFrom(allVersions().stream().filter(v -> v.compareMajor(Version.CURRENT) < 0).collect(Collectors.toList()));
+        assertRejectingJoinWithIncompatibleVersion(badVersion);
+    }
+
+    private void assertRejectingJoinWithIncompatibleVersion(final Version badVersion) throws InterruptedException, ExecutionException {
         addNodes(randomInt(5));
-        final Version badVersion;
-        if (randomBoolean()) {
-            badVersion = getPreviousVersion(Version.CURRENT.minimumCompatibilityVersion());
-        } else {
-            badVersion = randomFrom(allVersions().stream().filter(v -> v.before(Version.CURRENT)).collect(Collectors.toList()));
-        }
         final DiscoveryNode badNode = new DiscoveryNode("badNode", buildNewFakeTransportAddress(), emptyMap(),
             new HashSet<>(randomSubsetOf(DiscoveryNodeRole.BUILT_IN_ROLES)), badVersion);
 
         final Version goodVersion =
-            randomFrom(allVersions().stream().filter(v -> v.onOrAfter(Version.CURRENT)).collect(Collectors.toList()));
+            randomFrom(allVersions().stream().filter(v -> v.compareMajor(Version.CURRENT) >= 0).collect(Collectors.toList()));
         final DiscoveryNode goodNode = new DiscoveryNode("goodNode", buildNewFakeTransportAddress(), emptyMap(),
             new HashSet<>(randomSubsetOf(DiscoveryNodeRole.BUILT_IN_ROLES)), goodVersion);