diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/OzoneClientUtils.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/OzoneClientUtils.java index 46b1d66b281..ec2b1dae8d5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/OzoneClientUtils.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/OzoneClientUtils.java @@ -29,7 +29,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.net.InetSocketAddress; +import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -82,6 +84,44 @@ public final class OzoneClientUtils { // Never constructed } + /** + * Retrieve the socket addresses of all storage container managers. + * + * @param conf + * @return A collection of SCM addresses + * @throws IllegalArgumentException If the configuration is invalid + */ + public static Collection getSCMAddresses( + Configuration conf) throws IllegalArgumentException { + Collection addresses = + new HashSet(); + Collection names = + conf.getTrimmedStringCollection(OzoneConfigKeys.OZONE_SCM_NAMES); + if (names == null || names.isEmpty()) { + throw new IllegalArgumentException(OzoneConfigKeys.OZONE_SCM_NAMES + + " need to be a set of valid DNS names or IP addresses." + + " Null or empty address list found."); + } + + final com.google.common.base.Optional + defaultPort = com.google.common.base.Optional.of(OzoneConfigKeys + .OZONE_SCM_DEFAULT_PORT); + for (String address : names) { + com.google.common.base.Optional hostname = + OzoneClientUtils.getHostName(address); + if (!hostname.isPresent()) { + throw new IllegalArgumentException("Invalid hostname for SCM: " + + hostname); + } + com.google.common.base.Optional port = + OzoneClientUtils.getHostPort(address); + InetSocketAddress addr = NetUtils.createSocketAddr(hostname.get(), + port.or(defaultPort.get())); + addresses.add(addr); + } + return addresses; + } + /** * Retrieve the socket address that should be used by clients to connect * to the SCM. diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/states/datanode/InitDatanodeState.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/states/datanode/InitDatanodeState.java index 233cac19348..9e95f533958 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/states/datanode/InitDatanodeState.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/container/common/states/datanode/InitDatanodeState.java @@ -16,11 +16,9 @@ */ package org.apache.hadoop.ozone.container.common.states.datanode; -import com.google.common.base.Optional; +import com.google.common.base.Strings; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.net.NetUtils; import org.apache.hadoop.ozone.OzoneClientUtils; -import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine; import org.apache.hadoop.ozone.container.common.statemachine.SCMConnectionManager; import org.apache.hadoop.ozone.container.common.statemachine.StateContext; @@ -29,6 +27,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.net.InetSocketAddress; +import java.util.Collection; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -70,26 +69,23 @@ public class InitDatanodeState implements DatanodeState, */ @Override public DatanodeStateMachine.DatanodeStates call() throws Exception { - String[] addresses = conf.getStrings(OzoneConfigKeys.OZONE_SCM_NAMES); - final Optional defaultPort = Optional.of(OzoneConfigKeys - .OZONE_SCM_DEFAULT_PORT); - - if (addresses == null || addresses.length <= 0) { - LOG.error("SCM addresses need to be a set of valid DNS names " + - "or IP addresses. Null or empty address list found. Aborting " + - "containers."); + Collection addresses = null; + try { + addresses = OzoneClientUtils.getSCMAddresses(conf); + } catch (IllegalArgumentException e) { + if(!Strings.isNullOrEmpty(e.getMessage())) { + LOG.error("Failed to get SCM addresses: " + e.getMessage()); + } return DatanodeStateMachine.DatanodeStates.SHUTDOWN; } - for (String address : addresses) { - Optional hostname = OzoneClientUtils.getHostName(address); - if (!hostname.isPresent()) { - LOG.error("Invalid hostname for SCM."); - return DatanodeStateMachine.DatanodeStates.SHUTDOWN; + + if (addresses == null || addresses.isEmpty()) { + LOG.error("Null or empty SCM address list found."); + return DatanodeStateMachine.DatanodeStates.SHUTDOWN; + } else { + for (InetSocketAddress addr : addresses) { + connectionManager.addSCMServer(addr); } - Optional port = OzoneClientUtils.getHostPort(address); - InetSocketAddress addr = NetUtils.createSocketAddr(hostname.get(), - port.or(defaultPort.get())); - connectionManager.addSCMServer(addr); } return this.context.getState().getNextState(); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/TestOzoneClientUtils.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/TestOzoneClientUtils.java index fec0121c78f..33bed4207b4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/TestOzoneClientUtils.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/TestOzoneClientUtils.java @@ -26,11 +26,15 @@ import org.junit.Rule; import org.junit.rules.Timeout; import java.net.InetSocketAddress; +import java.util.Collection; +import java.util.HashMap; +import java.util.Iterator; import static org.apache.hadoop.ozone.OzoneConfigKeys.*; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertThat; - +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * This test class verifies the parsing of SCM endpoint config settings. @@ -217,4 +221,104 @@ public class TestOzoneClientUtils { assertThat(addr.getHostString(), is("5.6.7.8")); assertThat(addr.getPort(), is(200)); } + + @Test + public void testGetSCMAddresses() { + final Configuration conf = new OzoneConfiguration(); + Collection addresses = null; + InetSocketAddress addr = null; + Iterator it = null; + + // Verify valid IP address setup + conf.setStrings(OZONE_SCM_NAMES, "1.2.3.4"); + addresses = OzoneClientUtils.getSCMAddresses(conf); + assertThat(addresses.size(), is(1)); + addr = addresses.iterator().next(); + assertThat(addr.getHostName(), is("1.2.3.4")); + assertThat(addr.getPort(), is(OZONE_SCM_DEFAULT_PORT)); + + // Verify valid hostname setup + conf.setStrings(OZONE_SCM_NAMES, "scm1"); + addresses = OzoneClientUtils.getSCMAddresses(conf); + assertThat(addresses.size(), is(1)); + addr = addresses.iterator().next(); + assertThat(addr.getHostName(), is("scm1")); + assertThat(addr.getPort(), is(OZONE_SCM_DEFAULT_PORT)); + + // Verify valid hostname and port + conf.setStrings(OZONE_SCM_NAMES, "scm1:1234"); + addresses = OzoneClientUtils.getSCMAddresses(conf); + assertThat(addresses.size(), is(1)); + addr = addresses.iterator().next(); + assertThat(addr.getHostName(), is("scm1")); + assertThat(addr.getPort(), is(1234)); + + final HashMap hostsAndPorts = + new HashMap(); + hostsAndPorts.put("scm1", 1234); + hostsAndPorts.put("scm2", 2345); + hostsAndPorts.put("scm3", 3456); + + // Verify multiple hosts and port + conf.setStrings(OZONE_SCM_NAMES, "scm1:1234,scm2:2345,scm3:3456"); + addresses = OzoneClientUtils.getSCMAddresses(conf); + assertThat(addresses.size(), is(3)); + it = addresses.iterator(); + HashMap expected1 = new HashMap<>(hostsAndPorts); + while(it.hasNext()) { + InetSocketAddress current = it.next(); + assertTrue(expected1.remove(current.getHostName(), + current.getPort())); + } + assertTrue(expected1.isEmpty()); + + // Verify names with spaces + conf.setStrings(OZONE_SCM_NAMES, " scm1:1234, scm2:2345 , scm3:3456 "); + addresses = OzoneClientUtils.getSCMAddresses(conf); + assertThat(addresses.size(), is(3)); + it = addresses.iterator(); + HashMap expected2 = new HashMap<>(hostsAndPorts); + while(it.hasNext()) { + InetSocketAddress current = it.next(); + assertTrue(expected2.remove(current.getHostName(), + current.getPort())); + } + assertTrue(expected2.isEmpty()); + + // Verify empty value + conf.setStrings(OZONE_SCM_NAMES, ""); + try { + addresses = OzoneClientUtils.getSCMAddresses(conf); + fail("Empty value should cause an IllegalArgumentException"); + } catch (Exception e) { + assertTrue(e instanceof IllegalArgumentException); + } + + // Verify invalid hostname + conf.setStrings(OZONE_SCM_NAMES, "s..x..:1234"); + try { + addresses = OzoneClientUtils.getSCMAddresses(conf); + fail("An invalid hostname should cause an IllegalArgumentException"); + } catch (Exception e) { + assertTrue(e instanceof IllegalArgumentException); + } + + // Verify invalid port + conf.setStrings(OZONE_SCM_NAMES, "scm:xyz"); + try { + addresses = OzoneClientUtils.getSCMAddresses(conf); + fail("An invalid port should cause an IllegalArgumentException"); + } catch (Exception e) { + assertTrue(e instanceof IllegalArgumentException); + } + + // Verify a mixed case (valid and invalid value both appears) + conf.setStrings(OZONE_SCM_NAMES, "scm1:1234, scm:xyz"); + try { + addresses = OzoneClientUtils.getSCMAddresses(conf); + fail("An invalid value should cause an IllegalArgumentException"); + } catch (Exception e) { + assertTrue(e instanceof IllegalArgumentException); + } + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java index 19edb6ca8ec..bd671ed44d1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java @@ -261,4 +261,36 @@ public class TestDatanodeStateMachine { Assert.assertEquals(1, mock.getHeartbeatCount()); } } + + /** + * Test state transition with a list of invalid SCM names, + * and verify the state transits to SHUTDOWN each time. + */ + @Test + public void testDatanodeStateMachineWithInvalidSCMNames() + throws Exception { + for (String name : new String[] { + "", // Empty + "x..y", // Invalid schema + "scm:", // Missing port + "scm:xyz", // Invalid port + "scm:123456" // Port out of range + }) { + conf.setStrings(OzoneConfigKeys.OZONE_SCM_NAMES, name); + final DatanodeStateMachine stateMachine = + new DatanodeStateMachine(conf); + DatanodeStateMachine.DatanodeStates currentState = + stateMachine.getContext().getState(); + Assert.assertEquals(DatanodeStateMachine.DatanodeStates.INIT, + currentState); + + DatanodeState task = + stateMachine.getContext().getTask(); + task.execute(executorService); + DatanodeStateMachine.DatanodeStates newState = + task.await(2, TimeUnit.SECONDS); + Assert.assertEquals(DatanodeStateMachine.DatanodeStates.SHUTDOWN, + newState); + } + } }