From 496f0ffe9017b11d0d7c071bad259d132687c656 Mon Sep 17 00:00:00 2001 From: Arpit Agarwal Date: Mon, 29 Oct 2018 17:14:15 -0700 Subject: [PATCH] HDDS-620. ozone.scm.client.address should be an optional setting. Contributed by chencan and Arpit Agarwal. --- .../org/apache/hadoop/hdds/HddsUtils.java | 40 ++++- .../hadoop/hdds/scm/HddsServerUtil.java | 16 +- .../hadoop/hdds/scm/TestHddsServerUtils.java | 153 ++++++++++++++++++ .../ozone/client/TestHddsClientUtils.java | 137 ++++++++++++++-- 4 files changed, 325 insertions(+), 21 deletions(-) create mode 100644 hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java index 7a42a10303..09fc75b333 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java @@ -87,9 +87,21 @@ private HddsUtils() { * @return Target InetSocketAddress for the SCM client endpoint. */ public static InetSocketAddress getScmAddressForClients(Configuration conf) { - final Optional host = getHostNameFromConfigKeys(conf, + Optional host = getHostNameFromConfigKeys(conf, ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY); + if (!host.isPresent()) { + // Fallback to Ozone SCM names. + Collection scmAddresses = getSCMAddresses(conf); + if (scmAddresses.size() > 1) { + throw new IllegalArgumentException( + ScmConfigKeys.OZONE_SCM_NAMES + + " must contain a single hostname. Multiple SCM hosts are " + + "currently unsupported"); + } + host = Optional.of(scmAddresses.iterator().next().getHostName()); + } + if (!host.isPresent()) { throw new IllegalArgumentException( ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY + " must be defined. See" @@ -109,7 +121,8 @@ public static InetSocketAddress getScmAddressForClients(Configuration conf) { * Retrieve the socket address that should be used by clients to connect * to the SCM for block service. If * {@link ScmConfigKeys#OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY} is not defined - * then {@link ScmConfigKeys#OZONE_SCM_CLIENT_ADDRESS_KEY} is used. + * then {@link ScmConfigKeys#OZONE_SCM_CLIENT_ADDRESS_KEY} is used. If neither + * is defined then {@link ScmConfigKeys#OZONE_SCM_NAMES} is used. * * @param conf * @return Target InetSocketAddress for the SCM block client endpoint. @@ -123,13 +136,26 @@ public static InetSocketAddress getScmAddressForBlockClients( if (!host.isPresent()) { host = getHostNameFromConfigKeys(conf, ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY); - if (!host.isPresent()) { + } + + if (!host.isPresent()) { + // Fallback to Ozone SCM names. + Collection scmAddresses = getSCMAddresses(conf); + if (scmAddresses.size() > 1) { throw new IllegalArgumentException( - ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY - + " must be defined. See" - + " https://wiki.apache.org/hadoop/Ozone#Configuration" - + " for details on configuring Ozone."); + ScmConfigKeys.OZONE_SCM_NAMES + + " must contain a single hostname. Multiple SCM hosts are " + + "currently unsupported"); } + host = Optional.of(scmAddresses.iterator().next().getHostName()); + } + + if (!host.isPresent()) { + throw new IllegalArgumentException( + ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY + + " must be defined. See" + + " https://wiki.apache.org/hadoop/Ozone#Configuration" + + " for details on configuring Ozone."); } final Optional port = getPortNumberFromConfigKeys(conf, diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/hdds/scm/HddsServerUtil.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/hdds/scm/HddsServerUtil.java index d505be379c..395a77d4be 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/hdds/scm/HddsServerUtil.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/hdds/scm/HddsServerUtil.java @@ -28,6 +28,7 @@ import java.io.File; import java.net.InetSocketAddress; +import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -81,11 +82,24 @@ public static InetSocketAddress getScmAddressForDataNodes( // target host. // - OZONE_SCM_DATANODE_ADDRESS_KEY // - OZONE_SCM_CLIENT_ADDRESS_KEY + // - OZONE_SCM_NAMES // - final Optional host = getHostNameFromConfigKeys(conf, + Optional host = getHostNameFromConfigKeys(conf, ScmConfigKeys.OZONE_SCM_DATANODE_ADDRESS_KEY, ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY); + if (!host.isPresent()) { + // Fallback to Ozone SCM names. + Collection scmAddresses = getSCMAddresses(conf); + if (scmAddresses.size() > 1) { + throw new IllegalArgumentException( + ScmConfigKeys.OZONE_SCM_NAMES + + " must contain a single hostname. Multiple SCM hosts are " + + "currently unsupported"); + } + host = Optional.of(scmAddresses.iterator().next().getHostName()); + } + if (!host.isPresent()) { throw new IllegalArgumentException( ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY + diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java new file mode 100644 index 0000000000..21acda8543 --- /dev/null +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java @@ -0,0 +1,153 @@ +/** + * 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.hadoop.hdds.scm; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.rules.Timeout; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.net.InetSocketAddress; + +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_ADDRESS_KEY; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_PORT_DEFAULT; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_NAMES; +import static org.junit.Assert.assertEquals; + +/** + * Unit tests for {@link HddsServerUtil} + */ +public class TestHddsServerUtils { + public static final Logger LOG = LoggerFactory.getLogger( + TestHddsServerUtils.class); + + @Rule + public Timeout timeout = new Timeout(300_000); + + @Rule + public ExpectedException thrown= ExpectedException.none(); + + /** + * Test getting OZONE_SCM_DATANODE_ADDRESS_KEY with port. + */ + @Test + public void testGetDatanodeAddressWithPort() { + final String scmHost = "host123:100"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_DATANODE_ADDRESS_KEY, scmHost); + final InetSocketAddress address = + HddsServerUtil.getScmAddressForDataNodes(conf); + assertEquals(address.getHostName(), scmHost.split(":")[0]); + assertEquals(address.getPort(), Integer.parseInt(scmHost.split(":")[1])); + } + + /** + * Test getting OZONE_SCM_DATANODE_ADDRESS_KEY without port. + */ + @Test + public void testGetDatanodeAddressWithoutPort() { + final String scmHost = "host123"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_DATANODE_ADDRESS_KEY, scmHost); + final InetSocketAddress address = + HddsServerUtil.getScmAddressForDataNodes(conf); + assertEquals(address.getHostName(), scmHost); + assertEquals(address.getPort(), OZONE_SCM_DATANODE_PORT_DEFAULT); + } + + /** + * When OZONE_SCM_DATANODE_ADDRESS_KEY is undefined, test fallback to + * OZONE_SCM_CLIENT_ADDRESS_KEY. + */ + @Test + public void testDatanodeAddressFallbackToClientNoPort() { + final String scmHost = "host123"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, scmHost); + final InetSocketAddress address = + HddsServerUtil.getScmAddressForDataNodes(conf); + assertEquals(address.getHostName(), scmHost); + assertEquals(address.getPort(), OZONE_SCM_DATANODE_PORT_DEFAULT); + } + + /** + * When OZONE_SCM_DATANODE_ADDRESS_KEY is undefined, test fallback to + * OZONE_SCM_CLIENT_ADDRESS_KEY. Port number defined by + * OZONE_SCM_CLIENT_ADDRESS_KEY should be ignored. + */ + @Test + public void testDatanodeAddressFallbackToClientWithPort() { + final String scmHost = "host123:100"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, scmHost); + final InetSocketAddress address = + HddsServerUtil.getScmAddressForDataNodes(conf); + assertEquals(address.getHostName(), scmHost.split(":")[0]); + assertEquals(address.getPort(), OZONE_SCM_DATANODE_PORT_DEFAULT); + } + + /** + * When OZONE_SCM_DATANODE_ADDRESS_KEY and OZONE_SCM_CLIENT_ADDRESS_KEY + * are undefined, test fallback to OZONE_SCM_NAMES. + */ + @Test + public void testDatanodeAddressFallbackToScmNamesNoPort() { + final String scmHost = "host123"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, scmHost); + final InetSocketAddress address = + HddsServerUtil.getScmAddressForDataNodes(conf); + assertEquals(address.getHostName(), scmHost); + assertEquals(address.getPort(), OZONE_SCM_DATANODE_PORT_DEFAULT); + } + + /** + * When OZONE_SCM_DATANODE_ADDRESS_KEY and OZONE_SCM_CLIENT_ADDRESS_KEY + * are undefined, test fallback to OZONE_SCM_NAMES. Port number + * defined by OZONE_SCM_NAMES should be ignored. + */ + @Test + public void testDatanodeAddressFallbackToScmNamesWithPort() { + final String scmHost = "host123:100"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, scmHost); + final InetSocketAddress address = + HddsServerUtil.getScmAddressForDataNodes(conf); + assertEquals(address.getHostName(), scmHost.split(":")[0]); + assertEquals(address.getPort(), OZONE_SCM_DATANODE_PORT_DEFAULT); + } + + /** + * getScmAddressForDataNodes should fail when OZONE_SCM_NAMES has + * multiple addresses. + */ + @Test + public void testClientFailsWithMultipleScmNames() { + final String scmHost = "host123,host456"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, scmHost); + thrown.expect(IllegalArgumentException.class); + HddsServerUtil.getScmAddressForDataNodes(conf); + } +} diff --git a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestHddsClientUtils.java b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestHddsClientUtils.java index 3aefe8ac23..9850778516 100644 --- a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestHddsClientUtils.java +++ b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestHddsClientUtils.java @@ -19,9 +19,10 @@ package org.apache.hadoop.ozone.client; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.om.OMConfigKeys; -import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -29,9 +30,12 @@ import java.net.InetSocketAddress; -import static org.apache.hadoop.hdds.HddsUtils.getScmAddressForClients; -import static org.apache.hadoop.ozone.OmUtils.getOmAddress; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_PORT_DEFAULT; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_NAMES; import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; /** @@ -40,7 +44,7 @@ */ public class TestHddsClientUtils { @Rule - public Timeout timeout = new Timeout(300000); + public Timeout timeout = new Timeout(300_000); @Rule public ExpectedException thrown= ExpectedException.none(); @@ -52,7 +56,7 @@ public class TestHddsClientUtils { public void testMissingScmClientAddress() { final Configuration conf = new OzoneConfiguration(); thrown.expect(IllegalArgumentException.class); - getScmAddressForClients(conf); + HddsUtils.getScmAddressForClients(conf); } /** @@ -65,15 +69,15 @@ public void testGetScmClientAddress() { // First try a client address with just a host name. Verify it falls // back to the default port. - conf.set(ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY, "1.2.3.4"); - InetSocketAddress addr = getScmAddressForClients(conf); + conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, "1.2.3.4"); + InetSocketAddress addr = HddsUtils.getScmAddressForClients(conf); assertThat(addr.getHostString(), is("1.2.3.4")); - assertThat(addr.getPort(), is(ScmConfigKeys.OZONE_SCM_CLIENT_PORT_DEFAULT)); + assertThat(addr.getPort(), is(OZONE_SCM_CLIENT_PORT_DEFAULT)); // Next try a client address with a host name and port. Verify both // are used correctly. - conf.set(ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY, "1.2.3.4:100"); - addr = getScmAddressForClients(conf); + conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, "1.2.3.4:100"); + addr = HddsUtils.getScmAddressForClients(conf); assertThat(addr.getHostString(), is("1.2.3.4")); assertThat(addr.getPort(), is(100)); } @@ -85,21 +89,128 @@ public void testGetOmAddress() { // First try a client address with just a host name. Verify it falls // back to the default port. conf.set(OMConfigKeys.OZONE_OM_ADDRESS_KEY, "1.2.3.4"); - InetSocketAddress addr = getOmAddress(conf); + InetSocketAddress addr = OmUtils.getOmAddress(conf); assertThat(addr.getHostString(), is("1.2.3.4")); assertThat(addr.getPort(), is(OMConfigKeys.OZONE_OM_PORT_DEFAULT)); // Next try a client address with just a host name and port. Verify the port // is ignored and the default OM port is used. conf.set(OMConfigKeys.OZONE_OM_ADDRESS_KEY, "1.2.3.4:100"); - addr = getOmAddress(conf); + addr = OmUtils.getOmAddress(conf); assertThat(addr.getHostString(), is("1.2.3.4")); assertThat(addr.getPort(), is(100)); // Assert the we are able to use default configs if no value is specified. conf.set(OMConfigKeys.OZONE_OM_ADDRESS_KEY, ""); - addr = getOmAddress(conf); + addr = OmUtils.getOmAddress(conf); assertThat(addr.getHostString(), is("0.0.0.0")); assertThat(addr.getPort(), is(OMConfigKeys.OZONE_OM_PORT_DEFAULT)); } + + @Test + public void testBlockClientFallbackToClientNoPort() { + // When OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY is undefined it should + // fallback to OZONE_SCM_CLIENT_ADDRESS_KEY. + final String scmHost = "host123"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, scmHost); + final InetSocketAddress address =HddsUtils.getScmAddressForBlockClients( + conf); + assertEquals(address.getHostName(), scmHost); + assertEquals(address.getPort(), OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT); + } + + @Test + public void testBlockClientFallbackToClientWithPort() { + // When OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY is undefined it should + // fallback to OZONE_SCM_CLIENT_ADDRESS_KEY. + // + // Verify that the OZONE_SCM_CLIENT_ADDRESS_KEY port number is ignored, + // if present. Instead we should use OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT. + final String scmHost = "host123:100"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, scmHost); + final InetSocketAddress address =HddsUtils.getScmAddressForBlockClients( + conf); + assertEquals(address.getHostName(), scmHost.split(":")[0]); + assertEquals(address.getPort(), OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT); + } + + @Test + public void testBlockClientFallbackToScmNamesNoPort() { + // When OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY and OZONE_SCM_CLIENT_ADDRESS_KEY + // are undefined it should fallback to OZONE_SCM_NAMES. + final String scmHost = "host456"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, scmHost); + final InetSocketAddress address =HddsUtils.getScmAddressForBlockClients( + conf); + assertEquals(address.getHostName(), scmHost); + assertEquals(address.getPort(), OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT); + } + + @Test + public void testBlockClientFallbackToScmNamesWithPort() { + // When OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY and OZONE_SCM_CLIENT_ADDRESS_KEY + // are undefined it should fallback to OZONE_SCM_NAMES. + // + // Verify that the OZONE_SCM_NAMES port number is ignored, if present. + // Instead we should use OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT. + final String scmHost = "host456:200"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, scmHost); + final InetSocketAddress address =HddsUtils.getScmAddressForBlockClients( + conf); + assertEquals(address.getHostName(), scmHost.split(":")[0]); + assertEquals(address.getPort(), OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT); + } + + @Test + public void testClientFallbackToScmNamesNoPort() { + // When OZONE_SCM_CLIENT_ADDRESS_KEY is undefined, it should fallback + // to OZONE_SCM_NAMES. + final String scmHost = "host456"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, scmHost); + final InetSocketAddress address =HddsUtils.getScmAddressForClients(conf); + assertEquals(address.getHostName(), scmHost); + assertEquals(address.getPort(), OZONE_SCM_CLIENT_PORT_DEFAULT); + } + + @Test + public void testClientFallbackToScmNamesWithPort() { + // When OZONE_SCM_CLIENT_ADDRESS_KEY is undefined, it should fallback + // to OZONE_SCM_NAMES. + // + // Verify that the OZONE_SCM_NAMES port number is ignored, if present. + // Instead we should use OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT. + final String scmHost = "host456:300"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, scmHost); + final InetSocketAddress address =HddsUtils.getScmAddressForClients(conf); + assertEquals(address.getHostName(), scmHost.split(":")[0]); + assertEquals(address.getPort(), OZONE_SCM_CLIENT_PORT_DEFAULT); + } + + @Test + public void testBlockClientFailsWithMultipleScmNames() { + // When OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY and OZONE_SCM_CLIENT_ADDRESS_KEY + // are undefined, fail if OZONE_SCM_NAMES has multiple SCMs. + final String scmHost = "host123,host456"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, scmHost); + thrown.expect(IllegalArgumentException.class); + HddsUtils.getScmAddressForBlockClients(conf); + } + + @Test + public void testClientFailsWithMultipleScmNames() { + // When OZONE_SCM_CLIENT_ADDRESS_KEY is undefined, fail if OZONE_SCM_NAMES + // has multiple SCMs. + final String scmHost = "host123,host456"; + final Configuration conf = new OzoneConfiguration(); + conf.set(OZONE_SCM_NAMES, scmHost); + thrown.expect(IllegalArgumentException.class); + HddsUtils.getScmAddressForClients(conf); + } }