Address comments

Signed-off-by: Lakshya Taragi <[email protected]>
ltaragi · Apr 18, 2024 · 4b0d9ce · 4b0d9ce
1 parent 838d60a
commit 4b0d9ce
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 28 deletions.
diff --git a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java
@@ -521,24 +521,22 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod
                     );
                 }
             }
-        } else if (remoteStoreCompatibilityMode == CompatibilityMode.MIXED) {
-            Version joiningNodeVersion = joiningNode.getVersion();
-            if (joiningNodeVersion.after(currentNodes.getMaxNodeVersion()) || joiningNodeVersion.before(currentNodes.getMinNodeVersion())) {
-                boolean isAfter = joiningNodeVersion.after(currentNodes.getMaxNodeVersion());
-                String reason = String.format(
-                    Locale.ROOT,
-                    "mixed mode: a %s version [%s] node [%s] is not allowed to join cluster with %s version [%s]",
-                    isAfter ? "higher" : "lower",
-                    joiningNode.getVersion(),
-                    joiningNode,
-                    isAfter ? "maximum" : "minimum",
-                    isAfter ? currentNodes.getMaxNodeVersion() : currentNodes.getMinNodeVersion()
-                );
-                throw new IllegalStateException(reason);
-            }
-            if (joiningNode.isRemoteStoreNode()) {
-                Optional<DiscoveryNode> remoteDN = existingNodes.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst();
-                remoteDN.ifPresent(discoveryNode -> ensureRemoteStoreNodesCompatibility(joiningNode, discoveryNode));
+        } else {
+            if (remoteStoreCompatibilityMode == CompatibilityMode.MIXED) {
+                if (joiningNode.getVersion().after(currentNodes.getMaxNodeVersion())) {
+                    String reason = String.format(
+                        Locale.ROOT,
+                        "remote migration : a node [%s] of higher version [%s] is not allowed to join a cluster with maximum version [%s]",
+                        joiningNode,
+                        joiningNode.getVersion(),
+                        currentNodes.getMaxNodeVersion()
+                    );
+                    throw new IllegalStateException(reason);
+                }
+                if (joiningNode.isRemoteStoreNode()) {
+                    Optional<DiscoveryNode> remoteDN = existingNodes.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst();
+                    remoteDN.ifPresent(discoveryNode -> ensureRemoteStoreNodesCompatibility(joiningNode, discoveryNode));
+                }
             }
         }
     }

diff --git a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java
@@ -891,8 +891,7 @@ public void testNodeJoinInMixedMode() {
         FeatureFlags.initializeFeatureFlags(nodeSettings);
 
         List<Version> versions = allOpenSearchVersions();
-        assert versions.size() >= 3 : "test requires at least three open search versions";
-        Version lowerVersion = versions.get(versions.size() - 3);
+        assert versions.size() >= 2 : "test requires at least two open search versions";
         Version baseVersion = versions.get(versions.size() - 2);
         Version higherVersion = versions.get(versions.size() - 1);
 
@@ -911,28 +910,24 @@ public void testNodeJoinInMixedMode() {
 
         Metadata metadata = Metadata.builder().persistentSettings(mixedModeCompatibilitySettings).build();
 
-        boolean joiningNodeIsHigher = randomBoolean();
-
-        // joining node of a different version (higher or lower) than the current nodes
+        // joining node of a higher version than the current nodes
         DiscoveryNode joiningNode1 = new DiscoveryNode(
             randomAlphaOfLength(10),
             randomAlphaOfLength(10),
             buildNewFakeTransportAddress(),
             remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO),
             Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE),
-            joiningNodeIsHigher ? higherVersion : lowerVersion
+            higherVersion
         );
         final IllegalStateException exception = expectThrows(
             IllegalStateException.class,
             () -> JoinTaskExecutor.ensureNodesCompatibility(joiningNode1, currentNodes, metadata)
         );
         String reason = String.format(
             Locale.ROOT,
-            "mixed mode: a %s version [%s] node [%s] is not allowed to join cluster with %s version [%s]",
-            joiningNodeIsHigher ? "higher" : "lower",
-            joiningNode1.getVersion(),
+            "remote migration : a node [%s] of higher version [%s] is not allowed to join a cluster with maximum version [%s]",
             joiningNode1,
-            joiningNodeIsHigher ? "maximum" : "minimum",
+            joiningNode1.getVersion(),
             currentNodes.getMaxNodeVersion()
         );
         assertEquals(reason, exception.getMessage());