hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Varun Vasudev (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-5404) Add the ability to split reverse zone subnets
Date Tue, 26 Jul 2016 09:06:20 GMT

    [ https://issues.apache.org/jira/browse/YARN-5404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15393473#comment-15393473
] 

Varun Vasudev commented on YARN-5404:
-------------------------------------

Thanks for the patch [~shanekumpf@gmail.com]. Some feedback -

1)
{code}
+    Boolean shouldSplitReverseZone =
+        conf.getBoolean(KEY_DNS_SPLIT_REVERSE_ZONE, false);
{code}

Instead of false , create a variable called DEFAULT_DNS_SPLIT_REVERSE_ZONE in RegistryConstants,
set it to false and use it in the conf.getBoolean function

2)
{code}
+    String subnet = conf.get(KEY_DNS_ZONE_SUBNET);
+    String mask = conf.get(KEY_DNS_ZONE_MASK);
+    String range = conf.get(KEY_DNS_SPLIT_REVERSE_ZONE_RANGE);
+    SubnetUtils subnetUtils = new SubnetUtils(subnet, mask);
+    subnetUtils.setInclusiveHostCount(true);
+    int ipCount = subnetUtils.getInfo().getAddressCount();
+    return ipCount / Integer.parseInt(range);
{code}
SubnetUtils throws an IllegalArgumentException if the subnet, mask are messed up. Similarly
Integer.parseInt will throw an exception if a non integer is provided. We should catch them
and log a meaningful message. Can we also add a check that range is greater than 0?

3)
{code}
+  public String getReverseZoneNetworkAddress(String baseIp, int range, int
+      index) throws
+      ValidatorException {
{code}

Make all functions in ReverseZoneUtils static and add javadoc for getReverseZoneNetworkAddress.
Also, the formatting for that function seems off.

4)
{code}
+    // Set 256^x vars
+    long pow3 = (long) Math.pow(256, 3);
+    long pow2 = (long) Math.pow(256, 2);
+    long pow1 = (long) Math.pow(256, 1);
{code}

pow\{1,2,3} can be calculated statically and don’t need to be calculated every time we call
the function.

5)
{code}
+    Iterator<Long> iter = ipPartsOut.iterator();
+    StringBuilder sb = new StringBuilder();
+    if(iter.hasNext()) {
+      sb.append(iter.next());
+      while (iter.hasNext()) {
+        sb.append(".").append(iter.next());
+      }
+    }
+    return sb.toString();
{code}

You can use StringUtils.join instead of the loop.

6)
{code}
+  @VisibleForTesting
+  public String[] splitIp(String baseIp) {
+    return baseIp.split("\\.");
+  }
{code}

Instead of split - use InetAddress.getByName(baseIp) - it’ll do the validation of the ip
for you and will handle ipv6 if we ever need to. Then check if the InetAddress is of type
Inet4Address and if it is use the getAddress function to get the octets. Throw an exception
for ipv6. Also, if the function could probably be made protected.

7)
{code}
+  @Test
+  public void testGetReverseZoneNetworkAddress() throws Exception {
+    assertEquals("172.17.4.0",
+        reverseZoneUtils.getReverseZoneNetworkAddress(NET, RANGE, INDEX));
+  }
{code}

Can you please add some more test cases with different values for range and index?

8)
{code}
+    conf.set(RegistryConstants.KEY_DNS_DOMAIN, "hwx.test");
{code}
Change “hwx.test” to “example.com"


> Add the ability to split reverse zone subnets
> ---------------------------------------------
>
>                 Key: YARN-5404
>                 URL: https://issues.apache.org/jira/browse/YARN-5404
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Shane Kumpf
>            Assignee: Shane Kumpf
>         Attachments: YARN-5404-YARN-4757.001.patch, YARN-5404-YARN-4757.001.patch, YARN-5404-YARN-4757.001.patch,
YARN-5404.001.patch
>
>
> In some environments, the entire container subnet may not be used exclusively by containers
(ie the YARN nodemanager host IPs may also be part of the larger subnet). 
> As a result, the reverse lookup zones created by the YARN Registry DNS server may not
match those created on the forwarders.
> For example:
> Network: 172.27.0.0
> Subnet: 255.255.248.0
> Hosts:
> 0.27.172.in-addr.arpa
> 1.27.172.in-addr.arpa
> 2.27.172.in-addr.arpa
> 3.27.172.in-addr.arpa
> Containers
> 4.27.172.in-addr.arpa
> 5.27.172.in-addr.arpa
> 6.27.172.in-addr.arpa
> 7.27.172.in-addr.arpa
> YARN Registry DNS only allows for creating (as the total IP count is greater than 256):
> 27.172.in-addr.arpa
> Provide configuration to further subdivide the subnets.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message