You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by je...@apache.org on 2020/09/03 13:16:05 UTC
[geode] branch develop updated: GEODE-8462: Make geode server
startup fail if geode redis server has a port conflict (#5483)
This is an automated email from the ASF dual-hosted git repository.
jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 1076193 GEODE-8462: Make geode server startup fail if geode redis server has a port conflict (#5483)
1076193 is described below
commit 10761930ab59cd3168c1334e882c9256f03d0055
Author: John Hutchison <jh...@gmail.com>
AuthorDate: Thu Sep 3 09:15:15 2020 -0400
GEODE-8462: Make geode server startup fail if geode redis server has a port conflict (#5483)
Co-authored-by: Sarah Abbey <sa...@vmware.com>
Co-authored-by: john Hutchison <hu...@vmware.com>
Co-authored-by: Ray Ingles <ri...@vmware.com>
---
.../geode/test/junit/rules/gfsh/GfshExecution.java | 32 +++++---
geode-redis/build.gradle | 4 +
.../GeodeRedisServerStartUpAcceptanceTest.java | 96 ++++++++++++++++++++++
.../key/ExpireAtNativeRedisAcceptanceTest.java | 6 +-
.../redis/GeodeRedisServerStartupDUnitTest.java | 15 +++-
.../redis/internal/netty/NettyRedisServer.java | 22 +++--
6 files changed, 158 insertions(+), 17 deletions(-)
diff --git a/geode-junit/src/main/java/org/apache/geode/test/junit/rules/gfsh/GfshExecution.java b/geode-junit/src/main/java/org/apache/geode/test/junit/rules/gfsh/GfshExecution.java
index 7ef3150..1225bba 100644
--- a/geode-junit/src/main/java/org/apache/geode/test/junit/rules/gfsh/GfshExecution.java
+++ b/geode-junit/src/main/java/org/apache/geode/test/junit/rules/gfsh/GfshExecution.java
@@ -46,10 +46,20 @@ public class GfshExecution {
private static final String SCRIPT_EXIT_VALUE_DESCRIPTION =
"Exit value from process started by [%s]";
+ private static final Predicate<File> IS_RUNNING_SERVER_DIRECTORY =
+ directory -> stream(directory.list())
+ .anyMatch(filename -> filename.endsWith("server.pid"));
+
private static final Predicate<File> IS_SERVER_DIRECTORY = directory -> stream(directory.list())
- .anyMatch(filename -> filename.endsWith("server.pid"));
+ .anyMatch(filename -> filename.endsWith("server.pid") || filename.contains("server.status"));
+
+ private static final Predicate<File> IS_RUNNING_LOCATOR_DIRECTORY =
+ directory -> stream(directory.list())
+ .anyMatch(filename -> filename.endsWith("locator.pid"));
+
private static final Predicate<File> IS_LOCATOR_DIRECTORY = directory -> stream(directory.list())
- .anyMatch(filename -> filename.endsWith("locator.pid"));
+ .anyMatch(
+ filename -> filename.endsWith("locator.pid") || filename.contains("locator.status"));
private final Process process;
private final File workingDir;
@@ -121,10 +131,10 @@ public class GfshExecution {
}
String[] getStopMemberCommands() {
- Stream<String> stopServers = getServerDirs()
+ Stream<String> stopServers = getServerDirs(true)
.stream()
.map(f -> "stop server --dir=" + quoteArgument(f.toString()));
- Stream<String> stopLocators = getLocatorDirs()
+ Stream<String> stopLocators = getLocatorDirs(true)
.stream()
.map(f -> "stop locator --dir=" + quoteArgument(f.toString()));
return concat(stopServers, stopLocators)
@@ -150,17 +160,19 @@ public class GfshExecution {
}
}
- private List<File> getServerDirs() {
+ private List<File> getServerDirs(boolean isRunning) {
+ Predicate<File> predicate = isRunning ? IS_RUNNING_SERVER_DIRECTORY : IS_SERVER_DIRECTORY;
return findPotentialMemberDirectories()
.stream()
- .filter(IS_SERVER_DIRECTORY)
+ .filter(predicate)
.collect(toList());
}
- private List<File> getLocatorDirs() {
+ private List<File> getLocatorDirs(boolean isRunning) {
+ Predicate<File> predicate = isRunning ? IS_RUNNING_LOCATOR_DIRECTORY : IS_LOCATOR_DIRECTORY;
return findPotentialMemberDirectories()
.stream()
- .filter(IS_LOCATOR_DIRECTORY)
+ .filter(predicate)
.collect(toList());
}
@@ -178,8 +190,8 @@ public class GfshExecution {
}
private List<File> findLogFiles() {
- List<File> servers = getServerDirs();
- List<File> locators = getLocatorDirs();
+ List<File> servers = getServerDirs(false);
+ List<File> locators = getLocatorDirs(false);
return concat(servers.stream(), locators.stream())
.flatMap(GfshExecution::findLogFiles)
diff --git a/geode-redis/build.gradle b/geode-redis/build.gradle
index 0d9a3f8..bf23fd1 100644
--- a/geode-redis/build.gradle
+++ b/geode-redis/build.gradle
@@ -92,6 +92,10 @@ configurations{
}
}
+acceptanceTest {
+ environment 'GEODE_HOME', "$buildDir/../../geode-assembly/build/install/apache-geode"
+}
+
tasks.register("redisAPITest") {
dependsOn ':geode-assembly:installDist'
doLast {
diff --git a/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/GeodeRedisServerStartUpAcceptanceTest.java b/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/GeodeRedisServerStartUpAcceptanceTest.java
new file mode 100644
index 0000000..5d1eca2
--- /dev/null
+++ b/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/GeodeRedisServerStartUpAcceptanceTest.java
@@ -0,0 +1,96 @@
+/*
+ * 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.redis.internal.executor;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.junit.rules.gfsh.GfshExecution;
+import org.apache.geode.test.junit.rules.gfsh.GfshRule;
+import org.apache.geode.test.junit.rules.gfsh.GfshScript;
+
+public class GeodeRedisServerStartUpAcceptanceTest {
+
+ @Rule
+ public GfshRule gfshRule = new GfshRule();
+
+ @Test
+ public void shouldReturnErrorMessage_givenSamePortAndAddress() throws IOException {
+
+ int port = AvailablePortHelper.getRandomAvailableTCPPort();
+
+ String startServerCommand = String.join(" ",
+ "start server",
+ "--name", "same-port-and-address-server",
+ "--redis-bind-address", "localhost",
+ "--redis-port", String.valueOf(port));
+ GfshExecution execution;
+
+ try (Socket interferingSocket = new Socket()) {
+ interferingSocket.bind(new InetSocketAddress("localhost", port));
+ execution = GfshScript.of(startServerCommand)
+ .expectFailure()
+ .execute(gfshRule);
+ }
+
+ assertThat(execution.getOutputText()).containsIgnoringCase("address already in use");
+ }
+
+ @Test
+ public void shouldReturnErrorMessage_givenSamePortAndAllAddresses() throws IOException {
+
+ int port = AvailablePortHelper.getRandomAvailableTCPPort();
+
+ String startServerCommand = String.join(" ",
+ "start server",
+ "--name", "same-port-all-addresses-server",
+ "--redis-port", String.valueOf(port));
+ GfshExecution execution;
+
+ try (Socket interferingSocket = new Socket()) {
+ interferingSocket.bind(new InetSocketAddress("0.0.0.0", port));
+ execution = GfshScript.of(startServerCommand)
+ .expectFailure()
+ .execute(gfshRule);
+ }
+
+ assertThat(execution.getOutputText()).containsIgnoringCase("address already in use");
+ }
+
+ @Test
+ public void shouldReturnErrorMessage_givenInvalidBindAddress() {
+
+ String startServerCommand = String.join(" ",
+ "start server",
+ "--name", "invalid-bind-server",
+ "--redis-bind-address", "1.1.1.1");
+ GfshExecution execution;
+
+ execution = GfshScript.of(startServerCommand)
+ .expectFailure()
+ .execute(gfshRule);
+
+ assertThat(execution.getOutputText()).containsIgnoringCase(
+ "The redis-bind-address 1.1.1.1 is not a valid address for this machine");
+ }
+}
diff --git a/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/key/ExpireAtNativeRedisAcceptanceTest.java b/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/key/ExpireAtNativeRedisAcceptanceTest.java
index 3b9a40e..c3aeee7 100644
--- a/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/key/ExpireAtNativeRedisAcceptanceTest.java
+++ b/geode-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/key/ExpireAtNativeRedisAcceptanceTest.java
@@ -34,16 +34,20 @@ public class ExpireAtNativeRedisAcceptanceTest extends ExpireAtIntegrationTest {
@ClassRule
public static TestRule ignoreOnWindowsRule = new IgnoreOnWindowsRule();
+ private static GenericContainer redisContainer;
+
@BeforeClass
public static void setUp() {
- GenericContainer redisContainer = new GenericContainer<>("redis:5.0.6").withExposedPorts(6379);
+ redisContainer = new GenericContainer<>("redis:5.0.6").withExposedPorts(6379);
redisContainer.start();
+
jedis = new Jedis("localhost", redisContainer.getFirstMappedPort(), REDIS_CLIENT_TIMEOUT);
}
@AfterClass
public static void classLevelTearDown() {
jedis.close();
+ redisContainer.stop();
}
}
diff --git a/geode-redis/src/distributedTest/java/org/apache/geode/redis/GeodeRedisServerStartupDUnitTest.java b/geode-redis/src/distributedTest/java/org/apache/geode/redis/GeodeRedisServerStartupDUnitTest.java
index 409e2da..31aa860 100644
--- a/geode-redis/src/distributedTest/java/org/apache/geode/redis/GeodeRedisServerStartupDUnitTest.java
+++ b/geode-redis/src/distributedTest/java/org/apache/geode/redis/GeodeRedisServerStartupDUnitTest.java
@@ -226,7 +226,7 @@ public class GeodeRedisServerStartupDUnitTest {
public void startupFailsGivenPortAlreadyInUse() throws Exception {
int port = AvailablePortHelper.getRandomAvailableTCPPort();
- addIgnoredException("Could not start Server");
+ addIgnoredException("Could not start Redis Server");
try (Socket interferingSocket = new Socket()) {
interferingSocket.bind(new InetSocketAddress("localhost", port));
assertThatThrownBy(() -> cluster.startServerVM(0, s -> s
@@ -238,6 +238,19 @@ public class GeodeRedisServerStartupDUnitTest {
}
@Test
+ public void startupFailsGivenInvalidBindAddress() {
+ int port = AvailablePortHelper.getRandomAvailableTCPPort();
+
+ addIgnoredException("Could not start Redis Server");
+ assertThatThrownBy(() -> cluster.startServerVM(0, s -> s
+ .withProperty(REDIS_PORT, "" + port)
+ .withProperty(REDIS_BIND_ADDRESS, "1.1.1.1")
+ .withProperty(REDIS_ENABLED, "true")))
+ .hasStackTraceContaining(
+ "The redis-bind-address 1.1.1.1 is not a valid address for this machine");
+ }
+
+ @Test
public void startupWorksGivenAnyLocalAddress() {
String anyLocal = LocalHostUtil.getAnyLocalAddress().getHostAddress();
MemberVM server = cluster.startServerVM(0, s -> s
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/NettyRedisServer.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/NettyRedisServer.java
index bdcc80b..a991c61 100644
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/NettyRedisServer.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/netty/NettyRedisServer.java
@@ -101,11 +101,16 @@ public class NettyRedisServer {
if (port < RANDOM_PORT_INDICATOR) {
throw new IllegalArgumentException("Redis port cannot be less than 0");
}
- this.bindAddress = getBindAddress(requestedAddress);
- selectorGroup = createEventLoopGroup("Selector", false, 1);
+ selectorGroup = createEventLoopGroup("Selector", true, 1);
workerGroup = createEventLoopGroup("Worker", true, 0);
subscriberGroup = createEventLoopGroup("Subscriber", true, 0);
- serverChannel = createChannel(port);
+ try {
+ this.bindAddress = getBindAddress(requestedAddress);
+ serverChannel = createChannel(port);
+ } catch (ManagementException e) {
+ stop();
+ throw e;
+ }
serverPort = getActualPort();
logStartupMessage();
}
@@ -207,7 +212,11 @@ public class NettyRedisServer {
try {
channelFuture.syncUninterruptibly();
} catch (Exception ex) {
- throw new ManagementException("Could not start Server", ex);
+ throw new ManagementException(
+ "Could not start Redis Server using bind address: " + bindAddress +
+ " and port: " + port + ". " +
+ "Please make sure nothing else is running on this address/port combination.",
+ ex);
}
return channelFuture.channel();
}
@@ -243,7 +252,10 @@ public class NettyRedisServer {
try {
return InetAddress.getByName(requestedAddress);
} catch (UnknownHostException e) {
- throw new ManagementException("Could not start Server", e);
+ throw new ManagementException(
+ "Could not start Redis Server using bind address: '" + requestedAddress
+ + "'. Please make sure that this is a valid address for this host.",
+ e);
}
}
}