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

Jim Brennan commented on YARN-9560:
-----------------------------------

[~ebadger] Thanks for the patch. Overall this looks like a good restructuring 
to enable the addition of the oci runtime. Some comments:

OCIContainerRuntime.java
 * comment at line 70 - maybe change “Docker containers” to “OCI-compliant 
containers”, or something like that. There are other references to Docker that 
might need to be genericized as well. If you are planning to make these types 
of changes as part of actually adding the oci runtime, that is OK with me.
 * comment: YARN_CONTAINER_RUNTIME_DOCKER_RUN_OVERRIDE_DISABLE comment was 
removed from DockerLinuxContainerRuntime.java, but was not added to 
OCIContainerRuntime.java.
 * Looks like declaration of ENV_DOCKER_CONTAINER_RUN_OVERRIDE_DISABLE was 
removed from DockerLinuxContainerRuntime.java, and was not added to 
OCIContainerRuntime.java.
 * YARN_SYSFS_PATH also seems to be dropped.
 * Seems like isDockerContainerRequested() will ultimately need to be pushed up 
to OCIContainerRuntime.
 * Removed protected scope from mountReadOnlyPath() should it be private?

DockerLinuxContainerRuntime
 * Removed private scope from addCgroupParentIfRequired()?
 * handleContainerStop(), handleContainerKill(), and handleContainerRemove() 
should be protected?

> Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
> ---------------------------------------------------------------------------
>
>                 Key: YARN-9560
>                 URL: https://issues.apache.org/jira/browse/YARN-9560
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Eric Badger
>            Assignee: Eric Badger
>            Priority: Major
>         Attachments: YARN-9560.001.patch
>
>
> Since the new OCI/squashFS/runc runtime will be using a lot of the same code 
> as DockerLinuxContainerRuntime, it would be good to move a bunch of the 
> DockerLinuxContainerRuntime code up a level to an abstract class that both of 
> the runtimes can extend. 
> The new structure will look like:
> {noformat}
> OCIContainerRuntime (abstract class)
>   - DockerLinuxContainerRuntime
>   - FSImageContainerRuntime (name negotiable)
> {noformat}
> This JIRA should only change the structure of the code, not the actual 
> semantics



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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

Reply via email to