You are viewing a plain text version of this content. The canonical link for it is here.
Posted to yarn-issues@hadoop.apache.org by "Varun Vasudev (JIRA)" <ji...@apache.org> on 2016/08/03 15:47:20 UTC
[jira] [Comment Edited] (YARN-5430) Get container's ip and host
from NM
[ https://issues.apache.org/jira/browse/YARN-5430?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15406094#comment-15406094 ]
Varun Vasudev edited comment on YARN-5430 at 8/3/16 3:46 PM:
-------------------------------------------------------------
Thanks for the patch [~jianhe]!
1)
{code}
+ @Private
+ @Unstable
+ public abstract void setIp(String ip);
+
{code}
We can’t return just one ip. In the non-docker scenario, multiple ips for a host are common and Docker itself has support for multiple ips for a container. We don’t currently support that but it’ll probably be required for us to support soon. Similarly we should change getIpAndHost and split them up into getIps and getHost calls.
2)
{code}
+ optional string ip = 7;
+ optional string host = 8;
{code}
Instead of named variables - can we make this a bit more generic and add support for a simple list of string->list? Then the next time we need to send a new variable, we don’t need to mess around with the protobuf pieces. Something like
{code}
message StringListMapProto {
required string key = 1;
optional repeated string value = 2;
}
{code}
and in ContainerStatusProto
{code}
optional repeated StringListMapProto attributes_map = 7;
{code}
3)
{code}
+ public String[] getIpAndHost(Container container) {
+ return null;
+ }
{code}
The default implementation shouldn’t return null. We should return the host name and ip.
4)
{code}
+ public void setIpAndHost(String[] ipAndHost) {
+ this.ip = ipAndHost[0];
+ this.host = ipAndHost[1];
+ }
{code}
This may become a problem if the size of the array is more than 2(multiple ips)
5)
{code}
+ @Override
+ public String[] getIpAndHost(Container container) {
+ String[] ipAndHost = new String[2];
+ try {
+ InetAddress address = InetAddress.getLocalHost();
+ ipAndHost[0] = address.getHostAddress();
+ ipAndHost[1] = address.getHostName();
+ } catch (UnknownHostException e) {
+ LOG.error("Unable to get Local hostname and ip for " + container
+ .getContainerId());
+ }
+ return ipAndHost;
+ }
}
{code}
Instead of this, we should just use NetUtils.getLocalHostname() and NetUtils.getIPs. we’ll have to modify getIps() because today it expects a subnet and we want all ips irrespective of subnet.
6)
{code}
+ LOG.info("Docker inspect output for " + containerId + ": " + output);
{code}
Change log level to debug?
7)
{code}
+ // Be sure to not use space in the argument, otherwise the
+ // extract_values_delim method in container-executor binary
+ // cannot parse the arguments correctly.
+ super.addCommandArguments("--format='{{range.NetworkSettings.Networks}}"
+ + "{{.IPAddress}}{{end}},{{.Config.Hostname}}'");
+ super.addCommandArguments(containerName);
+ return this;
{code}
Ugh. This is an ugly one. I thought there needed to be a space between range and NetworkSettings because the code passed is meant to be a go template. Use this instead - {code}--format='{{range(.NetworkSettings.Networks)}}{code}
8)
{code}
+ if (docker_binary == NULL) {
+ docker_binary = "docker";
+ }
{code}
Can we make this a common function - the same code is in launch_docker_container_as_user?
was (Author: vvasudev):
Thanks for the patch [~jianhe]!
1)
{code}
+ @Private
+ @Unstable
+ public abstract void setIp(String ip);
+
{code}
We can’t return just one ip. In the non-docker scenario, multiple ips for a host are common and Docker itself has support for multiple ips for a container. We don’t currently support that but it’ll probably be required for us to support soon. Similarly we should change getIpAndHost and split them up into getIps and getHost calls.
2)
{code}
+ optional string ip = 7;
+ optional string host = 8;
{code}
Instead of named variables - can we make this a bit more generic and add support for a simple list of string->list? Then the next time we need to send a new variable, we don’t need to mess around with the protobuf pieces. Something like
{code}
message StringListMapProto {
required string key = 1;
optional repeated string value = 2;
}
and in ContainerStatusProto
{code}
optional repeated StringListMapProto attributes_map = 7;
{code}
3)
{code}
+ public String[] getIpAndHost(Container container) {
+ return null;
+ }
{code}
The default implementation shouldn’t return null. We should return the host name and ip.
4)
{code}
+ public void setIpAndHost(String[] ipAndHost) {
+ this.ip = ipAndHost[0];
+ this.host = ipAndHost[1];
+ }
{code}
This may become a problem if the size of the array is more than 2(multiple ips)
5)
{code}
+ @Override
+ public String[] getIpAndHost(Container container) {
+ String[] ipAndHost = new String[2];
+ try {
+ InetAddress address = InetAddress.getLocalHost();
+ ipAndHost[0] = address.getHostAddress();
+ ipAndHost[1] = address.getHostName();
+ } catch (UnknownHostException e) {
+ LOG.error("Unable to get Local hostname and ip for " + container
+ .getContainerId());
+ }
+ return ipAndHost;
+ }
}
{code}
Instead of this, we should just use NetUtils.getLocalHostname() and NetUtils.getIPs. we’ll have to modify getIps() because today it expects a subnet and we want all ips irrespective of subnet.
6)
{code}
+ LOG.info("Docker inspect output for " + containerId + ": " + output);
{code}
Change log level to debug?
7)
{code}
+ // Be sure to not use space in the argument, otherwise the
+ // extract_values_delim method in container-executor binary
+ // cannot parse the arguments correctly.
+ super.addCommandArguments("--format='{{range.NetworkSettings.Networks}}"
+ + "{{.IPAddress}}{{end}},{{.Config.Hostname}}'");
+ super.addCommandArguments(containerName);
+ return this;
{code}
Ugh. This is an ugly one. I thought there needed to be a space between range and NetworkSettings because the code passed is meant to be a go template. Use this instead - --format='{{range(.NetworkSettings.Networks)}}
8)
{code}
+ if (docker_binary == NULL) {
+ docker_binary = "docker";
+ }
{code}
Can we make this a common function - the same code is in launch_docker_container_as_user?
> Get container's ip and host from NM
> -----------------------------------
>
> Key: YARN-5430
> URL: https://issues.apache.org/jira/browse/YARN-5430
> Project: Hadoop YARN
> Issue Type: Bug
> Reporter: Jian He
> Assignee: Jian He
> Attachments: YARN-5430.1.patch, YARN-5430.2.patch, YARN-5430.3.patch
>
>
> In YARN-4757, we introduced a DNS mechanism for containers. That's based on the assumption that we can get the container's ip and host information and store it in the registry-service. This jira aims to get the container's ip and host from the NM, primarily docker container
--
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