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

Ravi Prakash commented on YARN-1964:
------------------------------------

Hi Abin! Thanks a lot for your work on this.

Here are my comments:
1. Nit: Please document DOCKER_IMAGE_PATTERN . What is that regex for?
2. Please document toMount()
3. Remove @VisibleForTesting for public static abstract class ShellScriptBuilder
4. Please explicitly specify your imports. Not import <package>.*;
5. If you made writeLaunchEnv static, you wouldn't have to unnecessarily create 
a DefaultContainerExecutor in TestContainerLaunch
6. In DockerContainerExecutor:
 a. In init() throw an exception if DOCKER_CONTAINER_EXECUTOR_SCRIPT doesn't 
exists. That way the NodeManager won't come up.
 b. The comment "// create log dir under app" seems misplaced
 c. You are missing shExec.close() in the finally block (not that it does 
anything, but still
 d. In writeLaunchEnv, you can remove exclusionSet and call super.writeLaunchEnv
 e. out.close(); could throw an Exception in the finally block.
 f. What's the story on signalContainer(), deleteAsUser() and 
isContainerProcessAlive() (false is probably the wrong value to return here)? 
I'm not sure we can commit this patch without those functions.


> Create Docker analog of the LinuxContainerExecutor in YARN
> ----------------------------------------------------------
>
>                 Key: YARN-1964
>                 URL: https://issues.apache.org/jira/browse/YARN-1964
>             Project: Hadoop YARN
>          Issue Type: New Feature
>    Affects Versions: 2.2.0
>            Reporter: Arun C Murthy
>            Assignee: Abin Shahab
>         Attachments: YARN-1964.patch, YARN-1964.patch, YARN-1964.patch, 
> YARN-1964.patch, YARN-1964.patch, YARN-1964.patch, YARN-1964.patch, 
> yarn-1964-branch-2.2.0-docker.patch, yarn-1964-branch-2.2.0-docker.patch, 
> yarn-1964-docker.patch, yarn-1964-docker.patch, yarn-1964-docker.patch, 
> yarn-1964-docker.patch, yarn-1964-docker.patch
>
>
> Docker (https://www.docker.io/) is, increasingly, a very popular container 
> technology.
> In context of YARN, the support for Docker will provide a very elegant 
> solution to allow applications to *package* their software into a Docker 
> container (entire Linux file system incl. custom versions of perl, python 
> etc.) and use it as a blueprint to launch all their YARN containers with 
> requisite software environment. This provides both consistency (all YARN 
> containers will have the same software environment) and isolation (no 
> interference with whatever is installed on the physical machine).



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

Reply via email to