You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2020/07/23 23:55:24 UTC
[geode] branch support/1.13 updated: Revert "GEODE-8331: allow GFSH
to connect to other versions of cluster (#5375)"
This is an automated email from the ASF dual-hosted git repository.
jinmeiliao pushed a commit to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.13 by this push:
new 8ed3da3 Revert "GEODE-8331: allow GFSH to connect to other versions of cluster (#5375)"
8ed3da3 is described below
commit 8ed3da354299253c30efd03fca3d7b55dcfc7a9c
Author: Jinmei Liao <ji...@pivotal.io>
AuthorDate: Thu Jul 23 16:54:32 2020 -0700
Revert "GEODE-8331: allow GFSH to connect to other versions of cluster (#5375)"
This reverts commit 903eff97
---
geode-gfsh/build.gradle | 9 ---
.../internal/cli/commands/ConnectCommand.java | 31 +++++---
.../geode/management/internal/cli/help/Helper.java | 23 ------
.../cli/remote/OnlineCommandProcessor.java | 6 +-
.../geode/management/internal/cli/shell/Gfsh.java | 1 +
.../internal/cli/shell/GfshExecutionStrategy.java | 20 +----
.../internal/cli/commands/ConnectCommandTest.java | 50 ++++++++----
.../cli/remote/OnlineCommandProcessorTest.java | 5 +-
.../cli/shell/GfshExecutionStrategyTest.java | 14 ----
.../geode/management/GfshCompatibilityTest.java | 89 ----------------------
10 files changed, 58 insertions(+), 190 deletions(-)
diff --git a/geode-gfsh/build.gradle b/geode-gfsh/build.gradle
index d82854f..212b9d9 100644
--- a/geode-gfsh/build.gradle
+++ b/geode-gfsh/build.gradle
@@ -53,15 +53,6 @@ dependencies {
testCompileOnly(platform(project(':boms:geode-all-bom')))
testCompileOnly('io.swagger:swagger-annotations')
- 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 2531b80..eeb4f6b 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
@@ -168,22 +168,22 @@ public class ConnectCommand extends OfflineGfshCommand {
return result;
}
- // since 1.14, only allow gfsh to connect to cluster that's older than 1.10
- String remoteVersion = null;
String gfshVersion = gfsh.getVersion();
+ String remoteVersion = null;
try {
- 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);
+ String gfshGeodeSerializationVersion = gfsh.getGeodeSerializationVersion();
+ String remoteGeodeSerializationVersion = invoker.getRemoteGeodeSerializationVersion();
+ if (hasSameMajorMinor(gfshGeodeSerializationVersion, remoteGeodeSerializationVersion)) {
return result;
}
- } 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);
+ } 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);
+ }
}
// will reach here only when remoteVersion is not available or does not match
@@ -197,6 +197,13 @@ 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 33161f7..9dc1c3f 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,7 +24,6 @@ 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;
@@ -466,26 +465,4 @@ 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 fa975f8..cbeea88 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
@@ -104,11 +104,7 @@ public class OnlineCommandProcessor implements CommandProcessor {
ParseResult parseResult = parseCommand(commentLessLine);
if (parseResult == null) {
- 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);
+ return ResultModel.createError("Could not parse command string. " + command);
}
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 b954c1e..ea35931 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
@@ -675,6 +675,7 @@ 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 f05da28..d78b267 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,7 +26,6 @@ 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;
@@ -204,23 +203,8 @@ 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) {
- 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);
- }
+ commandResult = ResultModel.fromJson((String) response);
+
} 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 329efde..0d7a0db 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
@@ -382,39 +382,57 @@ public class ConnectCommandTest {
}
@Test
- public void connectToManagerOlderThan1_10() {
- when(operationInvoker.getRemoteVersion()).thenReturn("1.10");
+ public void connectToManagerWithGreaterPatchVersion() {
+ when(gfsh.getGeodeSerializationVersion()).thenReturn("1.5.1");
+ when(operationInvoker.getRemoteGeodeSerializationVersion()).thenReturn("1.5.2");
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();
+ }
+
+ @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()
- .containsOutput("You are connected to a cluster of version: 1.10");
+ .statusIsSuccess();
}
@Test
- public void connectToOlderManagerWithNoRemoteVersion() {
- when(gfsh.getVersion()).thenReturn("1.14");
+ 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);
+
+ gfshParserRule.executeAndAssertThat(connectCommand, "connect --locator=localhost:4040")
+ .statusIsSuccess();
+ }
+
+ @Test
+ public void connectToOlderManagerWithNewerGfsh() {
+ when(gfsh.getVersion()).thenReturn("1.5");
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.14 gfsh client to connect to this cluster.");
+ .statusIsError().containsOutput("Cannot use a 1.5 gfsh client to connect to this cluster.");
}
@Test
- public void connectToManagerBefore1_10() {
- when(gfsh.getVersion()).thenReturn("1.14");
- when(operationInvoker.getRemoteVersion()).thenReturn("1.9");
+ public void connectToAValidManager() {
+ when(gfsh.getGeodeSerializationVersion()).thenReturn("1.5");
+ 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")
- .statusIsError()
- .containsOutput("Cannot use a 1.14 gfsh client to connect to a 1.9 cluster");
+ .statusIsSuccess();
}
}
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 6c04a5b..472f4ca 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,9 +89,6 @@ public class OnlineCommandProcessorTest {
public void handlesParsingError() {
ResultModel commandResult = onlineCommandProcessor.executeCommand("foo --bar");
assertThat(commandResult).isInstanceOf(ResultModel.class);
- 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");
+ assertThat(commandResult.toString()).contains("Could not parse command string. foo --bar");
}
}
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 7ffb205..287a462 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,20 +103,6 @@ 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
deleted file mode 100644
index 07690ec..0000000
--- a/geode-gfsh/src/upgradeTest/java/org/apache/geode/management/GfshCompatibilityTest.java
+++ /dev/null
@@ -1,89 +0,0 @@
-/*
- * 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)
-@Parameterized.UseParametersRunnerFactory(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();
- }
- }
-
-}