Repository: incubator-geode Updated Branches: refs/heads/develop 80026a7d6 -> 2777aec05
GEODE-1955: fix flaky tests by properly closing the gfsh instance in ConnectToLocatorSSLDUnitTest GEODE-1955: fix flaky tests by properly closing the gfsh instance in ConnectToLocatorSSLDUnitTest GEODE-1955: fix flaky tests by properly closing the gfsh instance in ConnectToLocatorSSLDUnitTest Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/2777aec0 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/2777aec0 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/2777aec0 Branch: refs/heads/develop Commit: 2777aec05f1af920bf065d3f1117e525ed246a20 Parents: 80026a7 Author: Jinmei Liao <jil...@pivotal.io> Authored: Fri Oct 28 14:46:10 2016 -0700 Committer: Jinmei Liao <jil...@pivotal.io> Committed: Fri Nov 4 08:47:53 2016 -0700 ---------------------------------------------------------------------- .../ConnectToLocatorSSLDUnitTest.java | 106 ++++++++----------- .../security/GfshCommandsPostProcessorTest.java | 7 +- .../security/GfshCommandsSecurityTest.java | 11 +- .../dunit/rules/GfshShellConnectionRule.java | 81 +++++++------- .../dunit/rules/LocatorServerStartupRule.java | 8 +- .../GfshCommandsOverHttpSecurityTest.java | 2 +- 6 files changed, 101 insertions(+), 114 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/2777aec0/geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java index 8426c6f..1c8079a 100644 --- a/geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java @@ -15,16 +15,35 @@ package org.apache.geode.management; -import static org.apache.geode.distributed.ConfigurationProperties.*; +import static org.apache.geode.distributed.ConfigurationProperties.CLUSTER_SSL_ENABLED; +import static org.apache.geode.distributed.ConfigurationProperties.CLUSTER_SSL_KEYSTORE; +import static org.apache.geode.distributed.ConfigurationProperties.CLUSTER_SSL_KEYSTORE_PASSWORD; +import static org.apache.geode.distributed.ConfigurationProperties.CLUSTER_SSL_KEYSTORE_TYPE; +import static org.apache.geode.distributed.ConfigurationProperties.CLUSTER_SSL_TRUSTSTORE; +import static org.apache.geode.distributed.ConfigurationProperties.CLUSTER_SSL_TRUSTSTORE_PASSWORD; +import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_SSL_ENABLED; +import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_SSL_KEYSTORE; +import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_SSL_KEYSTORE_PASSWORD; +import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_SSL_KEYSTORE_TYPE; +import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_SSL_TRUSTSTORE; +import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_SSL_TRUSTSTORE_PASSWORD; +import static org.apache.geode.distributed.ConfigurationProperties.SSL_ENABLED_COMPONENTS; +import static org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE; +import static org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE_PASSWORD; +import static org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE_TYPE; +import static org.apache.geode.distributed.ConfigurationProperties.SSL_PROTOCOLS; +import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE; +import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_PASSWORD; import static org.apache.geode.internal.Assert.assertTrue; -import static org.apache.geode.util.test.TestUtil.*; -import static org.junit.Assert.*; - -import java.io.File; -import java.io.FileOutputStream; -import java.io.OutputStream; -import java.util.Properties; +import static org.apache.geode.util.test.TestUtil.getResourcePath; +import org.apache.geode.internal.security.SecurableCommunicationChannel; +import org.apache.geode.management.internal.cli.i18n.CliStrings; +import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase; +import org.apache.geode.test.dunit.rules.GfshShellConnectionRule; +import org.apache.geode.test.dunit.rules.LocatorServerStartupRule; +import org.apache.geode.test.junit.categories.DistributedTest; +import org.apache.geode.test.junit.rules.serializable.SerializableTemporaryFolder; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -32,47 +51,36 @@ import org.junit.Test; import org.junit.experimental.categories.Category; import org.junit.rules.TemporaryFolder; -import org.apache.geode.distributed.Locator; -import org.apache.geode.internal.AvailablePortHelper; -import org.apache.geode.internal.security.SecurableCommunicationChannel; -import org.apache.geode.management.cli.Result.Status; -import org.apache.geode.management.internal.cli.CliUtil; -import org.apache.geode.management.internal.cli.HeadlessGfsh; -import org.apache.geode.management.internal.cli.i18n.CliStrings; -import org.apache.geode.management.internal.cli.result.CommandResult; -import org.apache.geode.management.internal.cli.util.CommandStringBuilder; -import org.apache.geode.test.dunit.Host; -import org.apache.geode.test.dunit.VM; -import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase; -import org.apache.geode.test.junit.categories.DistributedTest; -import org.apache.geode.test.junit.rules.serializable.SerializableTemporaryFolder; +import java.io.File; +import java.io.FileOutputStream; +import java.io.OutputStream; +import java.util.Properties; @Category(DistributedTest.class) public class ConnectToLocatorSSLDUnitTest extends JUnit4DistributedTestCase { - protected VM locator = null; - protected File jks = null; - protected File securityPropsFile = null; - @Rule public TemporaryFolder folder = new SerializableTemporaryFolder(); + @Rule + public LocatorServerStartupRule lsRule = new LocatorServerStartupRule(); + + protected File jks = null; + protected File securityPropsFile = null; + protected Properties securityProps; @Before public void before() throws Exception { - final Host host = Host.getHost(0); - this.locator = host.getVM(0); this.jks = new File(getResourcePath(getClass(), "/ssl/trusted.keystore")); securityPropsFile = folder.newFile("security.properties"); + securityProps = new Properties(); } @After public void after() throws Exception { securityPropsFile.delete(); - CliUtil.isGfshVM = false; } @Test public void testConnectToLocatorWithSSL() throws Exception { - Properties securityProps = new Properties(); securityProps.setProperty(SSL_ENABLED_COMPONENTS, SecurableCommunicationChannel.LOCATOR.getConstant()); securityProps.setProperty(SSL_KEYSTORE, jks.getCanonicalPath()); @@ -82,12 +90,12 @@ public class ConnectToLocatorSSLDUnitTest extends JUnit4DistributedTestCase { securityProps.setProperty(SSL_TRUSTSTORE_PASSWORD, "password"); securityProps.setProperty(SSL_PROTOCOLS, "TLSv1.2,TLSv1.1"); + setUpLocatorAndConnect(securityProps); } @Test public void testConnectToLocatorWithLegacyClusterSSL() throws Exception { - Properties securityProps = new Properties(); securityProps.setProperty(CLUSTER_SSL_ENABLED, "true"); securityProps.setProperty(CLUSTER_SSL_KEYSTORE, jks.getCanonicalPath()); securityProps.setProperty(CLUSTER_SSL_KEYSTORE_PASSWORD, "password"); @@ -100,7 +108,6 @@ public class ConnectToLocatorSSLDUnitTest extends JUnit4DistributedTestCase { @Test public void testConnectToLocatorWithLegacyJMXSSL() throws Exception { - Properties securityProps = new Properties(); securityProps.setProperty(JMX_MANAGER_SSL_ENABLED, "true"); securityProps.setProperty(JMX_MANAGER_SSL_KEYSTORE, jks.getCanonicalPath()); securityProps.setProperty(JMX_MANAGER_SSL_KEYSTORE_PASSWORD, "password"); @@ -112,41 +119,18 @@ public class ConnectToLocatorSSLDUnitTest extends JUnit4DistributedTestCase { } public void setUpLocatorAndConnect(Properties securityProps) throws Exception { - // set up locator with cluster-ssl-* - int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2); - int locatorPort = ports[0]; - int jmxPort = ports[1]; - - locator.invoke(() -> { - Properties props = new Properties(); - props.setProperty(MCAST_PORT, "0"); - props.put(JMX_MANAGER, "true"); - props.put(JMX_MANAGER_START, "true"); - props.put(JMX_MANAGER_PORT, jmxPort + ""); - props.putAll(securityProps); - Locator.startLocatorAndDS(locatorPort, folder.newFile("locator.log"), props); - }); + lsRule.getLocatorVM(0, securityProps); // saving the securityProps to a file OutputStream out = new FileOutputStream(securityPropsFile); securityProps.store(out, ""); - // run gfsh connect command in this vm - CliUtil.isGfshVM = true; - String shellId = getClass().getSimpleName(); - HeadlessGfsh gfsh = - new HeadlessGfsh(shellId, 30, folder.newFolder("gfsh_files").getCanonicalPath()); - - // connect to the locator with the saved property file - final CommandStringBuilder command = new CommandStringBuilder(CliStrings.CONNECT); - command.addOption(CliStrings.CONNECT__LOCATOR, "localhost[" + locatorPort + "]"); - command.addOption(CliStrings.CONNECT__SECURITY_PROPERTIES, + GfshShellConnectionRule gfshConnector = + new GfshShellConnectionRule(lsRule.getPort(0), GfshShellConnectionRule.PortType.locator); + gfshConnector.connect(CliStrings.CONNECT__SECURITY_PROPERTIES, securityPropsFile.getCanonicalPath()); - - gfsh.executeCommand(command.toString()); - CommandResult result = (CommandResult) gfsh.getResult(); - assertEquals(Status.OK, result.getStatus()); - assertTrue(result.getContent().toString().contains("Successfully connected to")); + assertTrue(gfshConnector.isConnected()); + gfshConnector.close(); } } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/2777aec0/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java index d3390ba..37d1795 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java @@ -50,7 +50,9 @@ public class GfshCommandsPostProcessorTest { "org/apache/geode/management/internal/security/cacheServer.json"); } }; - + @Rule + public GfshShellConnectionRule gfshConnection = + new GfshShellConnectionRule(jmxPort, GfshShellConnectionRule.PortType.jmxManger); private HeadlessGfsh gfsh = null; @BeforeClass @@ -60,9 +62,6 @@ public class GfshCommandsPostProcessorTest { serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create("region1"); } - @Rule - public GfshShellConnectionRule gfshConnection = new GfshShellConnectionRule(jmxPort); - @Before public void before() { gfsh = gfshConnection.getGfsh(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/2777aec0/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java index b2dc6fe..9480a03 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java @@ -74,7 +74,9 @@ public class GfshCommandsSecurityTest { "org/apache/geode/management/internal/security/cacheServer.json"); } }; - + @Rule + public GfshShellConnectionRule gfshConnection = + new GfshShellConnectionRule(jmxPort, GfshShellConnectionRule.PortType.jmxManger); private HeadlessGfsh gfsh = null; @BeforeClass @@ -84,9 +86,6 @@ public class GfshCommandsSecurityTest { serverStarter.cache.createRegionFactory(RegionShortcut.REPLICATE).create("region1"); } - @Rule - public GfshShellConnectionRule gfshConnection = new GfshShellConnectionRule(jmxPort); - @Before public void before() { gfsh = gfshConnection.getGfsh(); @@ -95,13 +94,13 @@ public class GfshCommandsSecurityTest { @Test @ConnectionConfiguration(user = "data-admin", password = "wrongPwd") public void testInvalidCredentials() throws Exception { - assertFalse(gfshConnection.isAuthenticated()); + assertFalse(gfshConnection.isConnected()); } @Test @ConnectionConfiguration(user = "data-admin", password = "1234567") public void testValidCredentials() throws Exception { - assertTrue(gfshConnection.isAuthenticated()); + assertTrue(gfshConnection.isConnected()); } @Test http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/2777aec0/geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java index 25780e6..44da08b 100644 --- a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java +++ b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java @@ -14,12 +14,11 @@ */ package org.apache.geode.test.dunit.rules; +import org.apache.geode.management.cli.Result; import org.apache.geode.management.internal.cli.CliUtil; import org.apache.geode.management.internal.cli.HeadlessGfsh; import org.apache.geode.management.internal.cli.i18n.CliStrings; import org.apache.geode.management.internal.cli.result.CommandResult; -import org.apache.geode.management.internal.cli.result.ErrorResultData; -import org.apache.geode.management.internal.cli.result.ResultBuilder; import org.apache.geode.management.internal.cli.util.CommandStringBuilder; import org.apache.geode.test.junit.rules.DescribedExternalResource; import org.junit.runner.Description; @@ -32,39 +31,41 @@ import org.junit.runner.Description; public class GfshShellConnectionRule extends DescribedExternalResource { private int port = 0; - private boolean useHttp = false; + private PortType portType = null; private HeadlessGfsh gfsh; - private boolean authenticated; + private boolean connected; - /** - * Rule constructor - */ - - public GfshShellConnectionRule(int port) { - this.useHttp = false; - this.port = port; - } - - public GfshShellConnectionRule(int port, boolean useHttp) { - this.useHttp = useHttp; + public GfshShellConnectionRule(int port, PortType portType) { + this.portType = portType; this.port = port; + try { + this.gfsh = new HeadlessGfsh(getClass().getName(), 30, "gfsh_files"); + } catch (Exception e) { + e.printStackTrace(); + } + this.connected = false; } protected void before(Description description) throws Throwable { - CliUtil.isGfshVM = true; - String shellId = getClass().getSimpleName() + "_" + description.getMethodName(); - gfsh = new HeadlessGfsh(shellId, 30, "gfsh_files"); // TODO: move to TemporaryFolder - - final CommandStringBuilder connectCommand = new CommandStringBuilder(CliStrings.CONNECT); - ConnectionConfiguration config = description.getAnnotation(ConnectionConfiguration.class); if (config != null) { - connectCommand.addOption(CliStrings.CONNECT__USERNAME, config.user()); - connectCommand.addOption(CliStrings.CONNECT__PASSWORD, config.password()); + connect(CliStrings.CONNECT__USERNAME, config.user(), CliStrings.CONNECT__PASSWORD, + config.password()); + } else { + connect(); } + } + + public void connect(String... options) throws Exception { + CliUtil.isGfshVM = true; + final CommandStringBuilder connectCommand = new CommandStringBuilder(CliStrings.CONNECT); String endpoint; - if (useHttp) { + if (portType == PortType.locator) { + // port is the locator port + endpoint = "localhost[" + port + "]"; + connectCommand.addOption(CliStrings.CONNECT__LOCATOR, endpoint); + } else if (portType == PortType.http) { endpoint = "http://localhost:"; + port + "/gemfire/v1"; connectCommand.addOption(CliStrings.CONNECT__USE_HTTP, Boolean.TRUE.toString()); connectCommand.addOption(CliStrings.CONNECT__URL, endpoint); @@ -72,30 +73,32 @@ public class GfshShellConnectionRule extends DescribedExternalResource { endpoint = "localhost[" + port + "]"; connectCommand.addOption(CliStrings.CONNECT__JMX_MANAGER, endpoint); } - System.out.println(getClass().getSimpleName() + " using endpoint: " + endpoint); + + // add the extra options + if (options != null) { + for (int i = 0; i < options.length; i += 2) { + connectCommand.addOption(options[i], options[i + 1]); + } + } gfsh.executeCommand(connectCommand.toString()); CommandResult result = (CommandResult) gfsh.getResult(); - if (result.getResultData() instanceof ErrorResultData) { - ErrorResultData errorResultData = (ErrorResultData) result.getResultData(); - this.authenticated = - !(errorResultData.getErrorCode() == ResultBuilder.ERRORCODE_CONNECTION_ERROR); - } else { - this.authenticated = true; - } + connected = (result.getStatus() == Result.Status.OK); } /** * Override to tear down your specific external resource. */ protected void after(Description description) throws Throwable { + close(); + } + + public void close() throws Exception { if (gfsh != null) { - gfsh.clearEvents(); - gfsh.executeCommand("disconnect"); + gfsh.clear(); gfsh.executeCommand("exit"); gfsh.terminate(); - gfsh.setThreadLocalInstance(); gfsh = null; } CliUtil.isGfshVM = false; @@ -105,7 +108,11 @@ public class GfshShellConnectionRule extends DescribedExternalResource { return gfsh; } - public boolean isAuthenticated() { - return authenticated; + public boolean isConnected() { + return connected; + } + + public enum PortType { + locator, jmxManger, http } } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/2777aec0/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java index 4e10b0a..6053e1e 100644 --- a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java +++ b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java @@ -36,14 +36,11 @@ import java.util.Properties; */ public class LocatorServerStartupRule extends ExternalResource implements Serializable { - private Host host = getHost(0); - - public int[] ports = new int[4]; - - // these are only avaialbe in each VM public static ServerStarterRule serverStarter; public static LocatorStarterRule locatorStarter; + public int[] ports = new int[4]; + private Host host = getHost(0); @Before public void before() { @@ -67,6 +64,7 @@ public class LocatorServerStartupRule extends ExternalResource implements Serial */ public VM getLocatorVM(int index, Properties locatorProperties) throws IOException { VM locatorVM = host.getVM(index); + locatorProperties.setProperty(NAME, "locator-" + index); int locatorPort = locatorVM.invoke(() -> { locatorStarter = new LocatorStarterRule(locatorProperties); locatorStarter.startLocator(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/2777aec0/geode-web/src/test/java/org/apache/geode/management/internal/security/GfshCommandsOverHttpSecurityTest.java ---------------------------------------------------------------------- diff --git a/geode-web/src/test/java/org/apache/geode/management/internal/security/GfshCommandsOverHttpSecurityTest.java b/geode-web/src/test/java/org/apache/geode/management/internal/security/GfshCommandsOverHttpSecurityTest.java index d477565..5449568 100644 --- a/geode-web/src/test/java/org/apache/geode/management/internal/security/GfshCommandsOverHttpSecurityTest.java +++ b/geode-web/src/test/java/org/apache/geode/management/internal/security/GfshCommandsOverHttpSecurityTest.java @@ -24,6 +24,6 @@ import org.junit.experimental.categories.Category; @Category({IntegrationTest.class, SecurityTest.class}) public class GfshCommandsOverHttpSecurityTest extends GfshCommandsSecurityTest { public GfshCommandsOverHttpSecurityTest() { - gfshConnection = new GfshShellConnectionRule(httpPort, true); + gfshConnection = new GfshShellConnectionRule(httpPort, GfshShellConnectionRule.PortType.http); } }
