This is an automated email from the ASF dual-hosted git repository.
jinmeiliao pushed a commit to branch support/1.12
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.12 by this push:
new 3d4512a GEODE-8331: allow GFSH to connect to other versions of
cluster (#5375)
3d4512a is described below
commit 3d4512a361f6eede01e6208e9a08c45638d3b8d6
Author: Jinmei Liao <[email protected]>
AuthorDate: Thu Jul 16 08:47:34 2020 -0700
GEODE-8331: allow GFSH to connect to other versions of cluster (#5375)
---
geode-gfsh/build.gradle | 9 +++
.../internal/cli/commands/ConnectCommand.java | 31 +++-----
.../geode/management/internal/cli/help/Helper.java | 23 ++++++
.../cli/remote/OnlineCommandProcessor.java | 7 +-
.../geode/management/internal/cli/shell/Gfsh.java | 1 -
.../internal/cli/shell/GfshExecutionStrategy.java | 20 ++++-
.../internal/cli/commands/ConnectCommandTest.java | 51 ++++---------
.../cli/remote/OnlineCommandProcessorTest.java | 5 +-
.../cli/shell/GfshExecutionStrategyTest.java | 14 ++++
.../geode/management/GfshCompatibilityTest.java | 89 ++++++++++++++++++++++
10 files changed, 191 insertions(+), 59 deletions(-)
diff --git a/geode-gfsh/build.gradle b/geode-gfsh/build.gradle
index 72e6b37..fd0dc6a 100644
--- a/geode-gfsh/build.gradle
+++ b/geode-gfsh/build.gradle
@@ -43,6 +43,15 @@ dependencies {
distributedTestRuntime('org.apache.derby:derby')
+ upgradeTestImplementation(project(':geode-junit'))
+ upgradeTestImplementation(project(':geode-dunit'))
+
+ upgradeTestImplementation('org.awaitility:awaitility')
+ upgradeTestImplementation('org.assertj:assertj-core')
+ upgradeTestImplementation('junit:junit')
+ upgradeTestImplementation('xml-apis:xml-apis:2.0.2')
+ upgradeTestRuntimeOnly(project(path: ':geode-old-versions', configuration:
'testOutput'))
+
implementation('net.sf.jopt-simple:jopt-simple')
//Log4j is used everywhere
diff --git
a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
index cd58f79..ae8aaea 100644
---
a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
+++
b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
@@ -156,22 +156,22 @@ public class ConnectCommand extends OfflineGfshCommand {
return result;
}
- String gfshVersion = gfsh.getVersion();
+ // since 1.14, only allow gfsh to connect to cluster that's older than 1.10
String remoteVersion = null;
+ String gfshVersion = gfsh.getVersion();
try {
- String gfshGeodeSerializationVersion =
gfsh.getGeodeSerializationVersion();
- String remoteGeodeSerializationVersion =
invoker.getRemoteGeodeSerializationVersion();
- if (hasSameMajorMinor(gfshGeodeSerializationVersion,
remoteGeodeSerializationVersion)) {
+ remoteVersion = invoker.getRemoteVersion();
+ int minorVersion = Integer.parseInt(versionComponent(remoteVersion,
VERSION_MINOR));
+ if (versionComponent(remoteVersion, VERSION_MAJOR).equals("1") &&
minorVersion >= 10 ||
+ versionComponent(remoteVersion, VERSION_MAJOR).equals("9") &&
minorVersion >= 9) {
+ InfoResultModel versionInfo = result.addInfo("versionInfo");
+ versionInfo.addLine("You are connected to a cluster of version: " +
remoteVersion);
return result;
}
- } catch (Exception e) {
- // we failed to get the remote geode serialization version; get remote
product version for
- // error message
- try {
- remoteVersion = invoker.getRemoteVersion();
- } catch (Exception ex) {
- gfsh.logInfo("failed to get the the remote version.", ex);
- }
+ } catch (Exception ex) {
+ // if unable to get the remote version, we are certainly talking to
+ // a pre-1.5 cluster
+ gfsh.logInfo("failed to get the the remote version.", ex);
}
// will reach here only when remoteVersion is not available or does not
match
@@ -185,13 +185,6 @@ public class ConnectCommand extends OfflineGfshCommand {
}
}
- private static boolean hasSameMajorMinor(String gfshVersion, String
remoteVersion) {
- return versionComponent(remoteVersion, VERSION_MAJOR)
- .equalsIgnoreCase(versionComponent(gfshVersion, VERSION_MAJOR))
- && versionComponent(remoteVersion, VERSION_MINOR)
- .equalsIgnoreCase(versionComponent(gfshVersion, VERSION_MINOR));
- }
-
private static String versionComponent(String version, int component) {
String[] versionComponents = StringUtils.split(version, '.');
return versionComponents.length >= component + 1 ?
versionComponents[component] : "";
diff --git
a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java
b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java
index 9dc1c3f..33161f7 100644
---
a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java
+++
b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java
@@ -24,6 +24,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -465,4 +466,26 @@ public class Helper {
public Set<String> getCommands() {
return commands.keySet();
}
+
+ public Method getCommandMethod(String command) {
+ return commands.get(command);
+
+ }
+
+ // methods added for future option comparison
+ public Set<String> getOptions(String command) {
+ Method method = getCommandMethod(command);
+ Set<String> optionList = new HashSet<>();
+ Annotation[][] annotations = method.getParameterAnnotations();
+ if (annotations == null || annotations.length == 0) {
+ // can't validate arguments if command doesn't have any
+ return optionList;
+ }
+
+ for (Annotation[] annotation : annotations) {
+ CliOption cliOption = getAnnotation(annotation, CliOption.class);
+ optionList.add(getPrimaryKey(cliOption));
+ }
+ return optionList;
+ }
}
diff --git
a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
index cbeea88..6261209 100644
---
a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
+++
b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
@@ -27,6 +27,7 @@ import org.springframework.util.StringUtils;
import org.apache.geode.annotations.VisibleForTesting;
import org.apache.geode.cache.Cache;
import org.apache.geode.cache.internal.CommandProcessor;
+import org.apache.geode.internal.GemFireVersion;
import org.apache.geode.internal.cache.CacheService;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.security.SecurityService;
@@ -104,7 +105,11 @@ public class OnlineCommandProcessor implements
CommandProcessor {
ParseResult parseResult = parseCommand(commentLessLine);
if (parseResult == null) {
- return ResultModel.createError("Could not parse command string. " +
command);
+ String version = GemFireVersion.getGemFireVersion();
+ String message = "Could not parse command string. " + command + "." +
System.lineSeparator() +
+ "The command or some options in this command may not be supported by
this locator(" +
+ version + ") gfsh is connected with.";
+ return ResultModel.createError(message);
}
Method method = parseResult.getMethod();
diff --git
a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
index 852b28f..3da0ea3 100755
---
a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
+++
b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
@@ -673,7 +673,6 @@ public class Gfsh extends JLineShell {
if (full) {
return GemFireVersion.asString();
} else {
-
return GemFireVersion.getGemFireVersion();
}
}
diff --git
a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java
b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java
index d78b267..f05da28 100755
---
a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java
+++
b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java
@@ -26,6 +26,7 @@ import org.springframework.shell.event.ParseResult;
import org.springframework.util.Assert;
import org.apache.geode.internal.ClassPathLoader;
+import org.apache.geode.internal.GemFireVersion;
import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.Result.Status;
import org.apache.geode.management.internal.cli.CliAroundInterceptor;
@@ -203,8 +204,23 @@ public class GfshExecutionStrategy implements
ExecutionStrategy {
// it can also be a Path to a temp file downloaded from the rest http
request
ResultModel commandResult = null;
if (response instanceof String) {
- commandResult = ResultModel.fromJson((String) response);
-
+ try {
+ commandResult = ResultModel.fromJson((String) response);
+ } catch (Exception e) {
+ logWrapper.severe("Unable to parse the remote response.", e);
+ String clientVersion = GemFireVersion.getGemFireVersion();
+ String remoteVersion = null;
+ try {
+ remoteVersion = shell.getOperationInvoker().getRemoteVersion();
+ } catch (Exception exception) {
+ // unable to get the remote version (pre-1.5.0 manager does not have
this capability)
+ // ignore
+ }
+ String message = "Unable to parse the remote response. This might due
to gfsh client "
+ + "version(" + clientVersion + ") mismatch with the remote cluster
version"
+ + ((remoteVersion == null) ? "." : "(" + remoteVersion + ").");
+ return ResultModel.createError(message);
+ }
} else if (response instanceof Path) {
tempFile = (Path) response;
}
diff --git
a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
index 017d51e..7e78ce7 100644
---
a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
+++
b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
@@ -39,7 +39,6 @@ import org.junit.ClassRule;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
-import org.apache.geode.management.cli.Result;
import org.apache.geode.management.internal.cli.result.CommandResult;
import org.apache.geode.management.internal.cli.result.model.ResultModel;
import org.apache.geode.management.internal.cli.shell.Gfsh;
@@ -322,57 +321,39 @@ public class ConnectCommandTest {
}
@Test
- public void connectToManagerWithGreaterPatchVersion() {
- when(gfsh.getGeodeSerializationVersion()).thenReturn("1.5.1");
-
when(operationInvoker.getRemoteGeodeSerializationVersion()).thenReturn("1.5.2");
+ public void connectToManagerOlderThan1_10() {
+ when(operationInvoker.getRemoteVersion()).thenReturn("1.10");
when(operationInvoker.isConnected()).thenReturn(true);
- when(resultModel.getStatus()).thenReturn(Result.Status.OK);
- gfshParserRule.executeAndAssertThat(connectCommand, "connect
--locator=localhost:4040")
- .statusIsSuccess();
- }
-
- @Test
- public void connectToManagerWithNoPatchVersion() {
- when(gfsh.getGeodeSerializationVersion()).thenReturn("1.5.1");
-
when(operationInvoker.getRemoteGeodeSerializationVersion()).thenReturn("1.5");
- when(operationInvoker.isConnected()).thenReturn(true);
- when(resultModel.getStatus()).thenReturn(Result.Status.OK);
-
- gfshParserRule.executeAndAssertThat(connectCommand, "connect
--locator=localhost:4040")
- .statusIsSuccess();
- }
-
- @Test
- public void connectToManagerWithLessorPatchVersion() {
- when(gfsh.getGeodeSerializationVersion()).thenReturn("1.5.1");
-
when(operationInvoker.getRemoteGeodeSerializationVersion()).thenReturn("1.5.0");
- when(operationInvoker.isConnected()).thenReturn(true);
- when(resultModel.getStatus()).thenReturn(Result.Status.OK);
+ ResultModel resultModel = new ResultModel();
+ when(connectCommand.jmxConnect(any(), anyBoolean(), any(), any(),
anyBoolean()))
+ .thenReturn(resultModel);
gfshParserRule.executeAndAssertThat(connectCommand, "connect
--locator=localhost:4040")
- .statusIsSuccess();
+ .statusIsSuccess()
+ .containsOutput("You are connected to a cluster of version: 1.10");
}
@Test
- public void connectToOlderManagerWithNewerGfsh() {
- when(gfsh.getVersion()).thenReturn("1.5");
+ public void connectToOlderManagerWithNoRemoteVersion() {
+ when(gfsh.getVersion()).thenReturn("1.14");
when(operationInvoker.getRemoteVersion())
.thenThrow(new RuntimeException("release version not available"));
when(operationInvoker.isConnected()).thenReturn(true);
gfshParserRule.executeAndAssertThat(connectCommand, "connect
--locator=localhost:4040")
- .statusIsError().containsOutput("Cannot use a 1.5 gfsh client to
connect to this cluster.");
+ .statusIsError()
+ .containsOutput("Cannot use a 1.14 gfsh client to connect to this
cluster.");
}
@Test
- public void connectToAValidManager() {
- when(gfsh.getGeodeSerializationVersion()).thenReturn("1.5");
-
when(operationInvoker.getRemoteGeodeSerializationVersion()).thenReturn("1.5");
+ public void connectToManagerBefore1_10() {
+ when(gfsh.getVersion()).thenReturn("1.14");
+ when(operationInvoker.getRemoteVersion()).thenReturn("1.9");
when(operationInvoker.isConnected()).thenReturn(true);
- when(resultModel.getStatus()).thenReturn(Result.Status.OK);
gfshParserRule.executeAndAssertThat(connectCommand, "connect
--locator=localhost:4040")
- .statusIsSuccess();
+ .statusIsError()
+ .containsOutput("Cannot use a 1.14 gfsh client to connect to a 1.9
cluster");
}
}
diff --git
a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
index b2f3193..3f6ffc6 100644
---
a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
+++
b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
@@ -89,6 +89,9 @@ public class OnlineCommandProcessorTest {
public void handlesParsingError() throws Exception {
ResultModel commandResult = onlineCommandProcessor.executeCommand("foo
--bar");
assertThat(commandResult).isInstanceOf(ResultModel.class);
- assertThat(commandResult.toString()).contains("Could not parse command
string. foo --bar");
+ assertThat(commandResult.toString())
+ .contains("Could not parse command string. foo --bar")
+ .contains(
+ "The command or some options in this command may not be supported
by this locator");
}
}
diff --git
a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategyTest.java
b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategyTest.java
index dc2e8b7..f682f7b 100644
---
a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategyTest.java
+++
b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategyTest.java
@@ -103,6 +103,20 @@ public class GfshExecutionStrategyTest {
}
@Test
+ public void testOnLineCommandWhenGfshReceivesInvalidJson() throws Exception {
+
when(parsedCommand.getMethod()).thenReturn(Commands.class.getDeclaredMethod("onlineCommand"));
+ when(parsedCommand.getInstance()).thenReturn(new Commands());
+ when(gfsh.isConnectedAndReady()).thenReturn(true);
+ OperationInvoker invoker = mock(OperationInvoker.class);
+
+
when(invoker.processCommand(any(CommandRequest.class))).thenReturn("invalid-json");
+ when(gfsh.getOperationInvoker()).thenReturn(invoker);
+ Result result = (Result) gfshExecutionStrategy.execute(parsedCommand);
+ assertThat(result.getStatus()).isEqualTo(Result.Status.ERROR);
+ assertThat(result.nextLine().trim()).contains("Unable to parse the remote
response.");
+ }
+
+ @Test
public void resolveInterceptorClassName() throws Exception {
when(parsedCommand.getMethod())
.thenReturn(Commands.class.getDeclaredMethod("interceptedCommand"));
diff --git
a/geode-gfsh/src/upgradeTest/java/org/apache/geode/management/GfshCompatibilityTest.java
b/geode-gfsh/src/upgradeTest/java/org/apache/geode/management/GfshCompatibilityTest.java
new file mode 100644
index 0000000..07690ec
--- /dev/null
+++
b/geode-gfsh/src/upgradeTest/java/org/apache/geode/management/GfshCompatibilityTest.java
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Collection;
+import java.util.List;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.BackwardCompatibilityTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import
org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
+import org.apache.geode.test.version.TestVersion;
+import org.apache.geode.test.version.VersionManager;
+
+@Category({BackwardCompatibilityTest.class})
+@RunWith(Parameterized.class)
[email protected](CategoryWithParameterizedRunnerFactory.class)
+public class GfshCompatibilityTest {
+ private final String oldVersion;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static Collection<String> data() {
+ List<String> result =
VersionManager.getInstance().getVersionsWithoutCurrent();
+ return result;
+ }
+
+ public GfshCompatibilityTest(String oldVersion) {
+ this.oldVersion = oldVersion;
+ }
+
+ private MemberVM oldLocator;
+
+ @Rule
+ public GfshCommandRule gfsh = new GfshCommandRule();
+
+ @Rule
+ public ClusterStartupRule cluster = new ClusterStartupRule();
+
+ @Test
+ public void currentGfshConnectToOlderVersionsOfLocator() throws Exception {
+ oldLocator = cluster.startLocatorVM(0, oldVersion);
+ int locatorPort = oldLocator.getPort();
+ cluster.startServerVM(1, oldVersion,
+ s -> s.withConnectionToLocator(locatorPort));
+ // pre 1.10, it should not be able to connect
+ if (TestVersion.compare(oldVersion, "1.5.0") < 0) {
+ gfsh.connect(oldLocator.getPort(), GfshCommandRule.PortType.locator);
+ assertThat(gfsh.isConnected()).isFalse();
+ assertThat(gfsh.getGfshOutput()).contains("Cannot use a")
+ .contains("gfsh client to connect to this cluster.");
+ } else if (TestVersion.compare(oldVersion, "1.10.0") < 0) {
+ gfsh.connect(oldLocator.getPort(), GfshCommandRule.PortType.locator);
+ assertThat(gfsh.isConnected()).isFalse();
+ assertThat(gfsh.getGfshOutput()).contains("Cannot use a")
+ .contains("gfsh client to connect to a " + oldVersion + " cluster.");
+ }
+ // post 1.10 (including) should connect and be able to execute command
+ else {
+ gfsh.connectAndVerify(oldLocator);
+ assertThat(gfsh.getGfshOutput())
+ .contains("You are connected to a cluster of version: " +
oldVersion);
+ gfsh.executeAndAssertThat("list members")
+ .statusIsSuccess();
+ }
+ }
+
+}
