[
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 [[email protected]]. 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: [email protected]
For additional commands, e-mail: [email protected]
