[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875252#comment-16875252
]
Eric Badger commented on YARN-9560:
---
Thanks [~eyang], [~Jim_Brennan], [~ccondit], [~shaneku...@gmail.com] for the
patience and help with this patch! I'll put up an initial patch for YARN-9562
soon
> 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
> Labels: Docker
> Fix For: 3.3.0
>
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch,
> YARN-9560.012.patch, YARN-9560.013.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875235#comment-16875235
]
Hudson commented on YARN-9560:
--
FAILURE: Integrated in Jenkins build Hadoop-trunk-Commit #16838 (See
[https://builds.apache.org/job/Hadoop-trunk-Commit/16838/])
YARN-9560. Restructure DockerLinuxContainerRuntime to extend (eyang: rev
29465bf169a7e348a4f32265083450faf66d5631)
* (edit)
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java
* (edit)
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/gpu/GpuResourceHandlerImpl.java
* (edit)
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerCleanup.java
* (edit)
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DockerLinuxContainerRuntime.java
* (add)
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/OCIContainerRuntime.java
* (edit)
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/TestDockerContainerRuntime.java
* (edit)
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/resourceplugin/deviceframework/DeviceResourceHandlerImpl.java
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch,
> YARN-9560.012.patch, YARN-9560.013.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875217#comment-16875217
]
Eric Yang commented on YARN-9560:
-
+1 on patch 013. Will commit shortly.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch,
> YARN-9560.012.patch, YARN-9560.013.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875124#comment-16875124
]
Jim Brennan commented on YARN-9560:
---
Thanks for all the updates [~ebadger]! I am also +1 on patch 013 (non-binding).
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch,
> YARN-9560.012.patch, YARN-9560.013.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875089#comment-16875089
]
Shane Kumpf commented on YARN-9560:
---
Thanks for the patch and explanation, [~ebadger]. It is a similar pattern to
what we do in the delegating runtime. I tested out the patch and it looks good
to me. The unit test failing looks to be unrelated. I'm +1 on patch 013.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch,
> YARN-9560.012.patch, YARN-9560.013.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875081#comment-16875081
]
Hadoop QA commented on YARN-9560:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
17s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 1 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m
5s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
15s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
27s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
43s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 51s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
58s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
25s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
59s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
59s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
18s{color} | {color:green}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 0 new + 22 unchanged - 2 fixed = 22 total (was 24) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 31s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
6s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
22s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 21m 4s{color}
| {color:red} hadoop-yarn-server-nodemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
29s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 72m 5s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests |
hadoop.yarn.server.nodemanager.amrmproxy.TestFederationInterceptor |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:bdbca0e |
| JIRA Issue | YARN-9560 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12973188/YARN-9560.013.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 8d2acf393598 4.4.0-139-generic #165-Ubuntu SMP Wed Oct 24
10:58:50 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / cbae241 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_212 |
| findbugs | v3.1.0-RC1 |
| unit |
https://builds.apache.org/job/PreCommit-YARN-Build/24334/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/24334/testReport/ |
| Max. process+thread count | 413 (vs. ulimit of 1) |
| modules | C:
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875020#comment-16875020
]
Craig Condit commented on YARN-9560:
+1 on patch 013 (non-binding).
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch,
> YARN-9560.012.patch, YARN-9560.013.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875018#comment-16875018
]
Eric Badger commented on YARN-9560:
---
Patch 013 addresses checkstyle in the meantime
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch,
> YARN-9560.012.patch, YARN-9560.013.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16875010#comment-16875010
]
Eric Badger commented on YARN-9560:
---
The reason for having the {{isDockerContainerRequested()}} in
{{OCIContainerRuntime}} is so that we can tell if either a Docker container
_or_ a Runc container was requested. The classes that call
{{isOCICompliantContainerRequested()}} are calling it from a static context.
They don't know whether the container is docker, runc, or something else. For
now since there are only Docker containers, the logic of
{{isDockerContainerRequested()}} is identical to
{{isOCICompliantContainerRequested()}}. However, once Runc is added, the logic
of {{isOCICompliantContainerRequsted()}} will be a logical OR of
{{isDockerContainerRequsted()}} and {{isRuncContainerRequested()}}. I thought
this would be cleaner than changing all of the invocations of
{{isDockerContainerRequested()}} and making those logical ORs. And to me it
makes more sense to let the subclasses define the logic around whether a docker
container or runc container is requested. If you have a betterr idea, let me
know.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch,
> YARN-9560.012.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16874687#comment-16874687
]
Eric Yang commented on YARN-9560:
-
[~ebadger], formatOciEnvKey function is helpful in this case. Thank you.
Another question for OCIContainerRuntime imports static method
DockerLinuxContainerRuntime.isDockerContainerRequested; This looks a bit odd
that base class imports static method of its extended class. It seems like
both classes will reply the same answer. How is JVM identify if it should use
RuncRuntime vs DockerLinuxContainerRuntime? Does OCIContainerRuntime needs to
import another static class from RuncRuntime to combine that logic in
isOCICompliantContainerRequested? Can we push isDockerContainerReuqested logic
to OCIContainerRuntime to avoid the base class imports method from extended
class?
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch,
> YARN-9560.012.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16874670#comment-16874670
]
Hadoop QA commented on YARN-9560:
-
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
32s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 1 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m
57s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
7s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
24s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
43s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 31s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
2s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
28s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
38s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
1s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m
1s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 25s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 2 new + 22 unchanged - 2 fixed = 24 total (was 24) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
40s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
13m 14s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
11s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
23s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 20m
49s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
25s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 75m 34s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=18.09.5 Server=18.09.5 Image:yetus/hadoop:bdbca0e53b4 |
| JIRA Issue | YARN-9560 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12973132/YARN-9560.012.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux bf8150d01bf7 4.15.0-48-generic #51-Ubuntu SMP Wed Apr 3
08:28:49 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 4a21224 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_212 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
https://builds.apache.org/job/PreCommit-YARN-Build/24332/artifact/out/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/24332/testReport/ |
| Max. process+thread count | 307 (vs. ulimit of 5500) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
U:
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16874630#comment-16874630
]
Eric Badger commented on YARN-9560:
---
Patch 012 keeps the environment variables as static in OCIContainerRuntime and
moves the formatting to the subclasses where the runtime type is known and
static. The formatting of the strings is a static function in
OCIContainerRuntime. That method is called from DockerLinuxContainerRuntime to
get the actual keys of the environment variables. These results are passed back
to OCIContainerRuntime via abstract methods to get the environment variables.
I don't know if this is what was envisioned or not, but it works and leaves the
variables as static. I'm not too particular about exactly how this piece of
code is done. I'd just like to get something we can all agree on so that we can
move to the next phase of patches with the actual runtime.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch,
> YARN-9560.012.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16874288#comment-16874288
]
Eric Yang commented on YARN-9560:
-
[~Jim_Brennan] From OCIRuntime and DockerLinuxContainerRuntime perspective,
they are static string. The config key strings do not change when used in those
classes. They will only change when a third implementation say RuncRuntime is
implemented and allowing DockerLinuxContainerRuntime configuration keys to work
in RuncRuntime. Instance variables only make sense to handle the config key
overload in RuncRuntime because config keys becomes variables base on
additional cluster config, and not static strings. This makes it more clear to
the developers that they need to handle config key shading in RuncRuntime with
care, while keeping DockerLinuxContainerRuntime logic as closely to how it was
written as possible. This helps the community to maintaining the existing code
base with least amount of risk to configuration fragmentation into multiple
runtimes. Disruptive retrofitting to Docker runtime requires human parser to
figure out if code sharing is put in the right place. Without ability to
statically define Docker configuration key in DockerLinuxContainerRuntime,
there is a risk to have Docker config key pop up in RuncRuntime or Runc config
pop up in Docker. This goes back to my original concern that config key
fragmentation issue hasn't been addressed correctly in patch 11.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16874203#comment-16874203
]
Jim Brennan commented on YARN-9560:
---
I think since these are not static strings - they are determined at runtime by
calling a method getRuntimeType() - that the current camelCase naming is
appropriate - it makes it clear to the developer that these strings are NOT
static - they depend on the class of the runtime. I think going to extra
effort to make them static doesn't really make things clearer.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16873714#comment-16873714
]
Eric Yang commented on YARN-9560:
-
[~ccondit] I thought I was arguing for code clarity to let developers use them
as static strings without over thinking this. I am +0 on patch 11.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872799#comment-16872799
]
Craig Condit commented on YARN-9560:
[~eyang], overloading static methods, especially in a context such as this is
extremely bad form. Given these strings will be initialized only on object
instantiation, these seems like an extreme case of over optimization at the
expense of code clarity. We’ve got a good patch here — lets get it merged and
move on. I’m +1 (non binding) on patch 011. It’s not worth going around again
IMO.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872786#comment-16872786
]
Eric Yang commented on YARN-9560:
-
{quote}Eric Yang, making getRuntimeType() static would require the base class
to be aware of all possible subclasses. This is extremely ugly.{quote}
Static method can be overloaded, but can not be overridden in Java. If you
declare, another static method with same signature in derived class than static
method of super class will be hidden, and any call to that static method in sub
class will go to static method declared in that class itself. I simply defined
static String getRuntimeType() in both OCIContainerRuntime and
DockerLinuxContainerRuntime.
All the keys can be optimized by compiler, it would be better to keep them as
static String instead of having some as instance variables imho.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872775#comment-16872775
]
Hadoop QA commented on YARN-9560:
-
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
37s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 1 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m
2s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
6s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
29s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
44s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 45s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
6s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
25s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
41s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
4s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m
4s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
20s{color} | {color:green}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 0 new + 22 unchanged - 2 fixed = 22 total (was 24) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
40s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
13m 10s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
10s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
25s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 21m
25s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
26s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 75m 33s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=18.09.5 Server=18.09.5 Image:yetus/hadoop:bdbca0e53b4 |
| JIRA Issue | YARN-9560 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12972905/YARN-9560.011.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 943fbf54ed0d 4.15.0-48-generic #51-Ubuntu SMP Wed Apr 3
08:28:49 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 4b50981 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_212 |
| findbugs | v3.1.0-RC1 |
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/24324/testReport/ |
| Max. process+thread count | 342 (vs. ulimit of 5500) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
U:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
|
| Console output |
https://builds.apache.org/job/PreCommit-YARN-Build/24324/console |
| Powered by | Apache Yetus 0.8.0
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872726#comment-16872726
]
Jim Brennan commented on YARN-9560:
---
+1 (non-binding) on patch 011.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872716#comment-16872716
]
Eric Badger commented on YARN-9560:
---
Patch 011 changes the names of the non static keys from all caps to camel case
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch, YARN-9560.011.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872711#comment-16872711
]
Jim Brennan commented on YARN-9560:
---
{quote}
I'm not sure about the checkstyle warnings. The variables are all capitalized
since they are still being used as keys. But They are no longer static strings.
I can fix them if you think that they should be camel case instead.
{quote}
I think we should just fix the names.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872696#comment-16872696
]
Craig Condit commented on YARN-9560:
[~eyang], making getRuntimeType() static would require the base class to be
aware of all possible subclasses. This is extremely ugly.
Might I suggest we table further changes to this patch and focus on the next
one (the RunC patch itself)? I think getting something (even if not 100%
perfect) in place so that we can iterate on functionality rather than form
would be much more useful.
I'm +1 as is on patch 10. LGTM.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872678#comment-16872678
]
Eric Yang commented on YARN-9560:
-
[~ebadger] I think this can be fixed by making those variables static, and also
getRuntimeType() static. OCIContainerRuntime would have a real implementation
of getRuntimeType(). I tested this approach, and it works fine.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872669#comment-16872669
]
Eric Badger commented on YARN-9560:
---
I'm not sure about the checkstyle warnings. The variables are all capitalized
since they are still being used as keys. But They are no longer static strings.
I can fix them if you think that they should be camel case instead.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16872467#comment-16872467
]
Jim Brennan commented on YARN-9560:
---
Thanks for the update [~ebadger]! I am +1 (non-binding) on patch 010.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.patch
>
>
> Since the new RuncContainerRuntime 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
> - RuncContainerRuntime
> {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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16871761#comment-16871761
]
Hadoop QA commented on YARN-9560:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
37s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 1 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 17m
39s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
0s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
24s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
39s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 22s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
58s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
26s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 21s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 4 new + 22 unchanged - 2 fixed = 26 total (was 24) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
13m 8s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
7s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
22s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 21m 1s{color}
| {color:red} hadoop-yarn-server-nodemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
27s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 72m 46s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests |
hadoop.yarn.server.nodemanager.amrmproxy.TestFederationInterceptor |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=18.09.5 Server=18.09.5 Image:yetus/hadoop:bdbca0e53b4 |
| JIRA Issue | YARN-9560 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12972772/YARN-9560.010.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 548f7ce08834 4.15.0-48-generic #51-Ubuntu SMP Wed Apr 3
08:28:49 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 129576f |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_212 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
https://builds.apache.org/job/PreCommit-YARN-Build/24318/artifact/out/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
|
| unit |
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16871712#comment-16871712
]
Eric Badger commented on YARN-9560:
---
Attaching patch 010 to fix some checkstyle issues
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.patch, YARN-9560.010.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16871711#comment-16871711
]
Eric Badger commented on YARN-9560:
---
Thanks for the reviews [~Jim_Brennan], [~eyang]! Comments on your review
comments inline.
bq. privilegedOperationExecutor is defined and set in OCIContainerRuntime, but
it is never used. Do we need it?
Probably not. I had it defined since it is passed into the constructor. But I
can remove it since it's never actually used.
bq. Do we need the allowedNetworks and allowedRuntimes variables in
OCIContainerRuntime? Seems like we could just call the getter functions when we
need to access them
Fixed
bq. Also, not sure we need the OCIContainerRuntime.initialize() function
anymore.
I'd like to keep the initialize function even if it's empty. That way the
subclasses can still call {{super.initialize()}}. I don't want to get in the
scenario where we add intialize back into the super class and then the
subclasses forget to call super on it.
bq. I feel like isOCICompliantContainerRequested() should be calling
isRuntimeRequested(), which are implemented in the docker/runc runtimes. I
guess for now we are still using the runtime type docker for both?
I changed {{isOCICompliantContainerRequested()}} to call
{{isDockerContainerRequested()}}. When we add the new runtime we can OR this
with {{isRunContainerRequested()}}. Since
{{isOCICompliantContainerRequested()}} is a static, we can't call
{{IsRuntimeRequested()}}
bq. We should probably fix the copy/paste error in
validateContainerRuntimeType() - "Allowed networks"
Good catch, fixed.
bq. (nit) for the *_FMT Strings in OCIContainerRuntime, I think we should have
a corresponding non-_FMT string, and resolve that in the constructor, (or
statically if that works)
Fixed this in latest patch.
bq. DockerLinuxContainerRuntime Javadoc removed
YARN_CONTAINER_RUNTIME_DOCKER_RUN_OVERRIDE_DISABLE and
YARN_CONTAINER_RUNTIME_DOCKER_CONTAINER_NETWORK without listing them in OCI or
Docker runtime. Is this correct? If yes, what is the reason to remove them?
This is a mistake. I've added them back. Thanks for catching this.
bq. OCIRuntime list environment variable
YARN_CONTAINER_RUNTIME_OCI_CONTAINER_PID_NAMESPACE_FMT. This is a formatter
string, not the actual environment variable. User might get confused what
exactly to specify because inconsistency between Javadoc for DockerRuntime and
OCIRuntime. How about document this in DockerRuntime, then link to OCIRuntime
code that Jim suggested?
That sounds good. I've fixed this up. We'll just need to make sure to document
the same variables in the runc runtime as well, even though they're common and
used in OCIContainerRuntime.
bq. How about getEnvConfigType() rename to getRuntimeType()? Does this more
accurately reflect the static values in runtime classes?
Yea that's probably more clear. Fixed.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.patch,
> YARN-9560.009.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16869960#comment-16869960
]
Eric Yang commented on YARN-9560:
-
[~ebadger] Thanks for the update. A few questions:
# DockerLinuxContainerRuntime Javadoc removed
YARN_CONTAINER_RUNTIME_DOCKER_RUN_OVERRIDE_DISABLE and
YARN_CONTAINER_RUNTIME_DOCKER_CONTAINER_NETWORK without listing them in OCI or
Docker runtime. Is this correct? If yes, what is the reason to remove them?
# OCIRuntime list environment variable
YARN_CONTAINER_RUNTIME_OCI_CONTAINER_PID_NAMESPACE_FMT. This is a formatter
string, not the actual environment variable. User might get confused what
exactly to specify because inconsistency between Javadoc for DockerRuntime and
OCIRuntime. How about document this in DockerRuntime, then link to OCIRuntime
code that Jim suggested?
Maybe some way to shorten the long FMT variables?
{code}
static final String RUNTIME_PREFIX = "YARN_CONTAINER_RUNTIME_%s_%s";
String.format(RUNTIME_PREFIX, getEnvConfigType(), key);
{code}
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16869869#comment-16869869
]
Jim Brennan commented on YARN-9560:
---
Thanks for the updates [~ebadger]!
Comments on patch 008:
privilegedOperationExecutor is defined and set in OCIContainerRuntime, but it
is never used. Do we need it?
Do we need the allowedNetworks and allowedRuntimes variables in
OCIContainerRuntime? Seems like we could just call the getter functions when
we need to access them. Also, not sure we need the
OCIContainerRuntime.initialize() function anymore.
I feel like isOCICompliantContainerRequested() should be calling
isRuntimeRequested(), which are implemented in the docker/runc runtimes. I
guess for now we are still using the runtime type docker for both?
We should probably fix the copy/paste error in validateContainerRuntimeType() -
"Allowed networks"
(nit) for the *_FMT Strings in OCIContainerRuntime, I think we should have a
corresponding non-_FMT string, and resolve that in the constructor, (or
statically if that works) e.g.:
{code}
private static String ENV_OCI_CONTAINER_PID_NAMESPACE;
...
ENV_OCI_CONTAINER_PID_NAMESPACE =
String.format(ENV_OCI_CONTAINER_PID_NAMESPACE_FMT, envConfigTypeUpper);
{code}
Then you can just reference ENV_OCI_CONTAINER_PID_NAMESPACE in code. Only do
the formatting once.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16869654#comment-16869654
]
Craig Condit commented on YARN-9560:
[~ebadger], I am +1 on patch 8. [~eyang], can we get a final signoff and merge?
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16868803#comment-16868803
]
Hadoop QA commented on YARN-9560:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
22s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 1 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m
30s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
5s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
27s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
41s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 20s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
0s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
28s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
0s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m
0s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 23s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 1 new + 22 unchanged - 3 fixed = 23 total (was 25) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
37s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
13m 4s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
7s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
24s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 20m 40s{color}
| {color:red} hadoop-yarn-server-nodemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
33s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 74m 21s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests |
hadoop.yarn.server.nodemanager.amrmproxy.TestFederationInterceptor |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:bdbca0e |
| JIRA Issue | YARN-9560 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12972357/YARN-9560.008.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux f668e73c3747 4.4.0-144-generic #170~14.04.1-Ubuntu SMP Mon Mar
18 15:02:05 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 9c4b15d |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_212 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
https://builds.apache.org/job/PreCommit-YARN-Build/24296/artifact/out/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
|
| unit |
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16868664#comment-16868664
]
Eric Badger commented on YARN-9560:
---
Attaching path 008 to address checkstyle. There will still be 1 checkstyle
warning from OCIContainerRuntime.java:88, but I don't know how to fix that
error without splitting up the config, which doesn't seem right.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.patch, YARN-9560.008.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16868240#comment-16868240
]
Hadoop QA commented on YARN-9560:
-
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
33s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 1 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 18m
7s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
8s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
31s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
45s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
13m 23s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
6s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
28s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
41s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
8s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m
8s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 23s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 4 new + 21 unchanged - 3 fixed = 25 total (was 24) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
39s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
13m 56s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
12s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
30s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 21m
56s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
31s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 77m 2s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=18.09.5 Server=18.09.5 Image:yetus/hadoop:bdbca0e53b4 |
| JIRA Issue | YARN-9560 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12972280/YARN-9560.007.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 7b9362145dfb 4.15.0-48-generic #51-Ubuntu SMP Wed Apr 3
08:28:49 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 5bfdf62 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_212 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
https://builds.apache.org/job/PreCommit-YARN-Build/24290/artifact/out/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/24290/testReport/ |
| Max. process+thread count | 307 (vs. ulimit of 5500) |
| modules | C:
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
U:
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16868189#comment-16868189
]
Eric Badger commented on YARN-9560:
---
Attached patch 007 that moves a bunch of the configs back into
{{DockerLinuxContainerRuntime}} since we are choosing to have separate configs
for Docker and Runc. My ideal final product would be to have the subclassed
runtimes define variables (such as {{allowedRuntimes}}, and then have
{{OCIContainerRuntime}} define the implementation of the methods that use them.
However, at this time that isn't possible, since there will be enough
difference between Docker and Runc. So for the time being, I've put the
Docker-related configs in Docker if the implementation of the methods that use
them in their current state cannot be shared between the runtimes.
I also added a formatted string type for the shared ENV variables in
{{OCIContainerRuntime}} that will be defined by {{envConfigType}} which is
defined by the subclassed runtimes.
I went back and forth on a few different approaches before landing on this
implementation. So your feedback is appreciated.
cc [~Jim_Brennan], [~eyang], [~ccondit]
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.patch, YARN-9560.007.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16862386#comment-16862386
]
Eric Badger commented on YARN-9560:
---
bq. Perhaps we could have the OCI interface class expose a getEnvConfigPrefix()
method, which for now would return "DOCKER", but in the RunC runtime could
return "RUNC"?
That sounds like a good approach. I'll add that into the next version of the
patch. Thanks for the suggestion, [~ccondit]!
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16862348#comment-16862348
]
Eric Yang commented on YARN-9560:
-
[~ccondit] {quote}Perhaps we could have the OCI interface class expose a
getEnvConfigPrefix() method, which for now would return "DOCKER", but in the
RunC runtime could return "RUNC"?{quote}
I agree this is a cleaner approach to provide separated name space to avoid
splitting hair in future config name changes.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16862343#comment-16862343
]
Eric Yang commented on YARN-9560:
-
[~ebadger] Does this mean runc specific config will use prefix
YARN_CONTAINER_RUNTIME_RUNC_* to make distinction?
This means end user may expose to:
YARN_CONTAINER_RUNTIME_DOCKER_*
YARN_CONTAINER_RUNTIME_RUNC_*
When using runc runtime.
What happen if Docker needs to change YARN_CONTAINER_RUNTIME_DOCKER_* which is
moved to OCIContainerRuntime, are we allowed to make the change that apply
globally or we need to make a override? There will become two implementations
of YARN_CONTAINER_RUNTIME_DOCKER_* settings, which may be confusing to maintain.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16862337#comment-16862337
]
Craig Condit commented on YARN-9560:
Perhaps we could have the OCI interface class expose a getEnvConfigPrefix()
method, which for now would return "DOCKER", but in the RunC runtime could
return "RUNC"? The base class could use this to dynamically namespace the
configs which are supported by both. I think it will be very confusing to users
to have DOCKER_* props refer to the runc runtime.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16862329#comment-16862329
]
Eric Badger commented on YARN-9560:
---
{quote}
At this time, the end user configuration YARN_CONTAINER_RUNTIME_DOCKER_*
applies to OCIContainerRuntime as well. I am not sure this is a good idea to
allow YARN Docker configuration key name used by other container runtime. It
may create schematic overloading problem if not careful. It would be good to
have a mechanism to prefix them differently. In case future evolution need to
separate out runc specific port mapping mechanism from Docker supported port
mapping mechanism. We would be setup to have distinct namespace.
{quote}
In the future we might have a reason to have distinct conf values, since those
are NM-specific instead of task-specific. The environment variables should be
fine since you can't simultaneously run both the runc runtime as well as the
docker runtime. In the short term I would like to use as many of the docker
configs as possible. Then if we need to make a distinction between runc and
docker then we can define them each separately at the leaf runtimes (runc,
docker) and remove them from OCIContainerRuntime. I'd rather not have
duplicates of every configs, one for runc and one for docker. I'd be fine with
creating OCI configs that support both docker and runc, but we would also need
to support the current docker configs to make sure that we don't break
compatibility.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16859029#comment-16859029
]
Eric Yang commented on YARN-9560:
-
Ping [~ebadger], any thought on how to resolve distinct name space for config
flags?
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16851429#comment-16851429
]
Eric Yang commented on YARN-9560:
-
[~ebadger] Tested patch 006, and it works as written. At this time, the end
user configuration YARN_CONTAINER_RUNTIME_DOCKER_* applies to
OCIContainerRuntime as well. I am not sure this is a good idea to allow YARN
Docker configuration key name used by other container runtime. It may create
schematic overloading problem if not careful. It would be good to have a
mechanism to prefix them differently. In case future evolution need to
separate out runc specific port mapping mechanism from Docker supported port
mapping mechanism. We would be setup to have distinct namespace.
> 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
> Labels: Docker
> Attachments: YARN-9560.001.patch, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847833#comment-16847833
]
Eric Badger commented on YARN-9560:
---
I've opened YARN-9582 to port the yarn sysfs feature to the new runtime
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847804#comment-16847804
]
Eric Yang commented on YARN-9560:
-
[~ebadger] Sounds reasonable to me.
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847782#comment-16847782
]
Eric Badger commented on YARN-9560:
---
bq. Are there two JSON formats used in OCIContainerRuntime? One for passing
information between Java and C, and another passed to runc for execution?
In the patch for YARN-9562 there will be 2 formats, 1 for Java to C and one for
C to runc.
bq. If there is already two types of JSON messages setup for communication
between Java-container-executor and container-executor-runc, then it would be
better to have sysfs included for communication between Java and
container-executor. Container-executor binary needs to handle how to translate
the flag into meaningful mount operations for runc.
Agreed that this is necessary for the yarn sysfs feature to work. However, we
can make that change in a followup JIRA. I don't want to conflate this
restructuring JIRA with features that will need extra code changes to support
such as changing the {{setYarnSysFS()}} method. The new runtime won't be using
{{DockerRunCommand}}, since that is Docker specific. So to make way for the
yarn sysfs feature in {{OCIContainerRuntime}}, I'd need to change
{{setYarnSysFS} to something more general. This is something I'd like to avoid
so that I can keep the changes as minimal as possible here and then make any
non-trivial changes in followup JIRAs. That way we can minimize the patch size
and the number of things that we're changing.
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847723#comment-16847723
]
Hadoop QA commented on YARN-9560:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
17s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 17m
27s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
4s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
26s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
43s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 48s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
0s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
30s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
59s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
59s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 22s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 1 new + 10 unchanged - 2 fixed = 11 total (was 12) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 59s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
4s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
26s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 21m
22s{color} | {color:green} hadoop-yarn-server-nodemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
29s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 71m 18s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:bdbca0e |
| JIRA Issue | YARN-9560 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12969658/YARN-9560.006.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 9abb7eaccc63 4.4.0-139-generic #165-Ubuntu SMP Wed Oct 24
10:58:50 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 460ba7f |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_212 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
https://builds.apache.org/job/PreCommit-YARN-Build/24147/artifact/out/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-YARN-Build/24147/testReport/ |
| Max. process+thread count | 447 (vs.
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847712#comment-16847712
]
Eric Yang commented on YARN-9560:
-
[~ebadger] Are there two JSON formats used in OCIContainerRuntime? One for
passing information between Java and C, and another passed to runc for
execution? If there is only one format that the one passing from Java is
consumed by runc, then I agree with you that it is not easy to pass this flag
and follow up JIRA make sense to further develop communication filtering
between Java, container-executor and runc. If there is already two types of
JSON messages setup for communication between Java <-> container-executor and
container-executor <-> runc, then it would be better to have sysfs included for
communication between Java and container-executor. Container-executor binary
needs to handle how to translate the flag into meaningful mount operations for
runc.
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847658#comment-16847658
]
Eric Badger commented on YARN-9560:
---
Patch 006 adds the Private and Unstable flags to {{OCIContainerRuntime}}
bq. It is basically a forward and pass operation to make sure that downstream C
side of code receives this flag, and perform the necessary operation to setup
the sysfs directory in the container working directory. Sysfs directory will be
populated through async rest api call with a json file that contains the
application structure, i.e. ip address and host names of the containers. In
this case, by passing the flag as part of json to container-executor is
sufficient.
I still think we should add this in a follow-up JIRA. The current code is
docker specific since it is a method of {{DockerRunCommand}}. If you agree I
can file a followup JIRA
bq. I will do tests, and probably try with and without ENTRYPOINT to make sure
it's well covered.
Thanks! I appreciate it
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.patch,
> YARN-9560.006.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847143#comment-16847143
]
Eric Yang commented on YARN-9560:
-
{quote}Looking at the code for setYarnSysFS I can't tell where this argument is
ever getting used, so I'm not sure how to add it correctly.{quote}
It is basically a forward and pass operation to make sure that downstream C
side of code receives this flag, and perform the necessary operation to setup
the sysfs directory in the container working directory. Sysfs directory will
be populated through async rest api call with a json file that contains the
application structure, i.e. ip address and host names of the containers. In
this case, by passing the flag as part of json to container-executor is
sufficient.
{quote}For testing, I have started up a single node cluster using
DockerLinuxContainerRuntime and run a few types of jobs (sleep, pi) using a few
different images (rhel6, rhel7).{quote}
I will do tests, and probably try with and without ENTRYPOINT to make sure it's
well covered.
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847093#comment-16847093
]
Eric Badger commented on YARN-9560:
---
bq. The current change will generate OCIContainerRuntime as public API. This is
a big commitment change, and I am not sure that YARN is ready to expose this
abstract class as a reference implementation.
This is a valid concern. I will add the Private and Unstable tags to the new
OCIContainerRuntime
{quote}
. It would be better to have YARN sysfs logic as part of the
OCIContainerRuntime to ensure that we remind developer to implement YARN sysfs
API for their runtime to expose cluster runtime configuration inside container.
{quote}
Looking at the code for {{setYarnSysFS}} I can't tell where this argument is
ever getting used, so I'm not sure how to add it correctly. But even so, the
new runtime isn't using the same type of command file as
{{DockerLinuxContainerRuntime}} is so this change isn't super trivial. I think
that we should file followup JIRAs for this and any other features that are not
implemented in the initial pass of the new runtime. If I try and port all of
the features from Docker, I will certainly mess some of them up. So I would
rather keep the initial implementation small and focused and then add on
features to it in followup JIRAs just like we did with Docker.
{quote}
2. We don't have method by method comprehensive test for
DockerContainerRuntime. It is impossible to detect if the code is using the
latest version of DockerContainerRuntime for the refactoring, or if the
ordering of the statement has any side effect to the running code. What manual
test have been done on your side for DockerContainerRuntime? I need some time
to repeat the tests and add my own tests to support the coverages.
3. Without unit test case, more thorough test needs to be conducted which will
delay the commit and get into repeated iterations to test the refactoring
matches latest code. This is something that I like to avoid as well.
{quote}
I don't think we need many additional unit tests. There should be no docker
code changes here. All of the tests from {{TestDockerContainerRuntime}} should
still be valid. Since all of the unit tests are passing, that indicates that
all of the Docker code works as expected (unless the tests are incomplete). If
I didn't choose the most up to date version of the
{{DockerLinuxContainerRuntime}} then the unit test(s) should fail to indicate
that functionality is missing or broken. For testing, I have started up a
single node cluster using {{DockerLinuxContainerRuntime}} and run a few types
of jobs (sleep, pi) using a few different images (rhel6, rhel7).
I can fix up the {{TestDelegatingLinuxContainerRuntime}} tests in YARN-9562 to
make sure that the new runtime is correctly picked. But they wouldn't be
relevant in this JIRA since there is still only
{{DockerLinuxContainerRuntime}}.
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16846310#comment-16846310
]
Eric Yang commented on YARN-9560:
-
# All of node manager logic are marked with:
{code:java}
@InterfaceAudience.Private
@InterfaceStability.Unstable
{code}
To prevent javadoc generation for private API. The current change will generate
OCIContainerRuntime as public API. This is a big commitment change, and I am
not sure that YARN is ready to expose this abstract class as a reference
implementation.
# . It would be better to have YARN sysfs logic as part of the
OCIContainerRuntime to ensure that we remind developer to implement YARN sysfs
API for their runtime to expose cluster runtime configuration inside container.
# We don't have method by method comprehensive test for
DockerContainerRuntime. It is impossible to detect if the code is using the
latest version of DockerContainerRuntime for the refactoring, or if the
ordering of the statement has any side effect to the running code. What manual
test have been done on your side for DockerContainerRuntime? I need some time
to repeat the tests and add my own tests to support the coverages.
# Without unit test case, more thorough test needs to be conducted which will
delay the commit and get into repeated iterations to test the refactoring
matches latest code. This is something that I like to avoid as well.
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16846226#comment-16846226
]
Eric Badger commented on YARN-9560:
---
Hey [~eyang], if {{YARN_CONTAINER_RUNTIME_TYPE}} is undefined, the code will
act just like it did before this patch. So it will choose the default runtime
since there isn't a specific runtime specified.
We could add some test cases, but I'm not sure that's within the scope of this
restructuring. This code shouldn't be changing how anything works, just
changing the structure of it. If it is changing how things work, then that's
something that I need to fix.
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16846209#comment-16846209
]
Eric Yang commented on YARN-9560:
-
[~ebadger] If YARN_CONTAINER_RUNTIME_TYPE is undefined, job submission will
trigger OCIContainerRuntime? Is there a distinction between OCIContainer and
regular java task based container? How is that controlled? Can we include
some test cases to show how to toggle between each runtime?
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16846015#comment-16846015
]
Hadoop QA commented on YARN-9560:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
13s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 17m
26s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
0s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
22s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
39s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 22s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
57s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
26s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
54s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
54s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 20s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 1 new + 10 unchanged - 2 fixed = 11 total (was 12) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 5s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
7s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
23s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 20m 58s{color}
| {color:red} hadoop-yarn-server-nodemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
24s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 68m 54s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | hadoop.yarn.server.nodemanager.webapp.TestNMWebServices |
| | hadoop.yarn.server.nodemanager.amrmproxy.TestFederationInterceptor |
| |
hadoop.yarn.server.nodemanager.containermanager.logaggregation.TestLogAggregationService
|
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:bdbca0e |
| JIRA Issue | YARN-9560 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12969405/YARN-9560.005.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 3d8e8a6a41a3 4.4.0-139-generic #165-Ubuntu SMP Wed Oct 24
10:58:50 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / a315913 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_212 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16846011#comment-16846011
]
Craig Condit commented on YARN-9560:
I'm +1 (no-binding) on patch 005 as well.
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16846007#comment-16846007
]
Eric Badger commented on YARN-9560:
---
[~Jim_Brennan], thanks for the thorough reviews!
[~eyang], [~billie.rinaldi], [~shaneku...@gmail.com], as a committer would one
of you mind reviewing this?
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16845999#comment-16845999
]
Jim Brennan commented on YARN-9560:
---
[~ebadger] thanks for the update! I am +1 (non-binding) on patch 005.
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16845957#comment-16845957
]
Eric Badger commented on YARN-9560:
---
Thanks [~Jim_Brennan]. Patch 5 moves the OCIContainerRuntime import down and
removes all instances of super.getXXX from DockerLinuxContainerRuntime
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.patch, YARN-9560.005.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16845917#comment-16845917
]
Jim Brennan commented on YARN-9560:
---
[~ebadger] thanks for updating the patch and addressing the checkstyle issues.
A couple comments on patch 004.
* GpuResourceHandlerImpl - (nit) move import for OCIContainerRuntime down.
* DockerLinuxContainerRuntime - I think you should only use super.getXXX in
places where you need to differentiate from a potentially overridden method in
the subclass and the one in the super. In most cases, you will want the
overridden method if it exists, so you should not use super (I realize
currently none of these are overridden). I think the only cases where you need
to use super is in the constructor and initialize method.
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16845515#comment-16845515
]
Hadoop QA commented on YARN-9560:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
20s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m
28s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
2s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
24s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
41s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 30s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
6s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
25s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
33s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
59s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
59s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 19s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 1 new + 10 unchanged - 2 fixed = 11 total (was 12) {color}
|
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 40s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
7s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
22s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 21m 5s{color}
| {color:red} hadoop-yarn-server-nodemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
33s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 72m 6s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests |
hadoop.yarn.server.nodemanager.containermanager.logaggregation.TestLogAggregationService
|
| | hadoop.yarn.server.nodemanager.webapp.TestNMWebServices |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:bdbca0e |
| JIRA Issue | YARN-9560 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12969341/YARN-9560.004.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 3f0f6e6d5fa1 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2
17:16:02 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 77c49f2 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_212 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16845439#comment-16845439
]
Eric Badger commented on YARN-9560:
---
Uploading patch 004 to fix the indentation checkstyle errors. I don't think I
should fix the method length error for {{launchContainer()}} since that would
be a more in-depth change and is unrelated to this patch
> 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, YARN-9560.002.patch,
> YARN-9560.003.patch, YARN-9560.004.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16845425#comment-16845425
]
Hadoop QA commented on YARN-9560:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
18s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 18m
12s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
6s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
28s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
41s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 14s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
58s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
22s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
57s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
57s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 19s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 10 new + 11 unchanged - 2 fixed = 21 total (was 13)
{color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 29s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
7s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
21s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 20m 58s{color}
| {color:red} hadoop-yarn-server-nodemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
25s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 69m 59s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests |
hadoop.yarn.server.nodemanager.containermanager.logaggregation.TestLogAggregationService
|
| | hadoop.yarn.server.nodemanager.webapp.TestNMWebServices |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:bdbca0e |
| JIRA Issue | YARN-9560 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12969322/YARN-9560.003.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux dab2d3fb8cd9 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2
17:16:02 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / ef1cc72 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_212 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16845273#comment-16845273
]
Eric Badger commented on YARN-9560:
---
[~Jim_Brennan], fixed up everything according to your comments in patch 003.
For member variables that will be shared between
{{DockerLinuxContainerRuntime}} and {{FSImageContainerRuntime}} I made them
private in {{OCIContainerRuntime}} and then added getters so that the child
classes can access them that way. It caused a little bit more of a code diff,
but should keep things better encapsulated.
The unit tests from the previous HadoopQA are being tracked in YARN-9558. I ran
all of the unit tests locally and the only ones that failed were the ones
tracked in that JIRA.
> 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, YARN-9560.002.patch,
> YARN-9560.003.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16844919#comment-16844919
]
Jim Brennan commented on YARN-9560:
---
Thanks for the updates [~ebadger]!
Some comments on the new patch:
* Need to fix checkstyle, javadoc, and double-check junit failures
* ContainerCleanup - remove import for DockerLinuxContainerRuntime
* GpuResourceHandlerImpl - remove import for DockerLinuxContainerRuntime and
(nit) move import for OCIContainerRuntime down.
* DockerLinuxContainerRuntime (nit) - comment for signalContainer references
OCI-compliant instead of docker.
* DeviceResourceHandlerImpl - (nit) move import for OCIContainerRuntime down.
> 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, YARN-9560.002.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16844355#comment-16844355
]
Hadoop QA commented on YARN-9560:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 13m
53s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 23m
47s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
25s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
35s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
51s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
13m 59s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
19s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
33s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
44s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
20s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m
20s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 25s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 33 new + 11 unchanged - 2 fixed = 44 total (was 13)
{color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
45s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
1s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
14m 49s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
21s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
31s{color} | {color:red}
hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager
generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 22m 23s{color}
| {color:red} hadoop-yarn-server-nodemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
39s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 99m 23s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests |
hadoop.yarn.server.nodemanager.containermanager.logaggregation.TestLogAggregationService
|
| | hadoop.yarn.server.nodemanager.webapp.TestNMWebServices |
| | hadoop.yarn.server.nodemanager.amrmproxy.TestFederationInterceptor |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:bdbca0e |
| JIRA Issue | YARN-9560 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12969201/YARN-9560.002.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 7dfe6da8e2c4 4.4.0-143-generic #169~14.04.2-Ubuntu SMP Wed Feb
13 15:00:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 05db2a5 |
| maven | version:
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16844282#comment-16844282
]
Eric Badger commented on YARN-9560:
---
Thanks for the review, [~Jim_Brennan]! I uploaded patch 002 to address your
comments. However, there are a few things that I didn't fix.
I removed {{ENV_DOCKER_CONTAINER_RUN_OVERRIDE_DISABLE}} and the associated
{{YARN_CONTAINER_RUNTIME_DOCKER_RUN_OVERRIDE_DISABLE}} comment because
{{ENV_DOCKER_CONTAINER_RUN_OVERRIDE_DISABLE}} was never referenced anywhere in
the code. Same thing with {{YARN_SYSFS_PATH}}.
bq. Seems like isDockerContainerRequested() will ultimately need to be pushed
up to OCIContainerRuntime.
For this, I added a method in {{OCIContainerRuntime}} named
{{isOCICompliantContainerRequested()}}. I believe that we need to have both
separation between bare-metal lxc-based containers as well as docker vs. runc
containers.
bq. handleContainerStop(), handleContainerKill(), and handleContainerRemove()
should be protected?
I removed the {{signalContainer()}} definition from {{OCIContainerRuntime}} and
moved it back into {{DockerLinuxContainerRuntime}}. In the Docker case we need
to worry about privileged containers and can't just signal the container.
However, in the OCI-based containers, we can signal the container directly. So
the functions can be defined differently.
As for the protected vs package private, I'm not quite sure what the convention
is here. Strictly speaking they don't need to be protected right now since they
are only referenced within the same package. But, if it's better style to make
them protected, then I'm fine with that as well. For now, I've made all of the
package-private functions protected in {{OCIContainerRuntime}}
> 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, YARN-9560.002.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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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
[jira] [Commented] (YARN-9560) Restructure DockerLinuxContainerRuntime to extend a new OCIContainerRuntime
[
https://issues.apache.org/jira/browse/YARN-9560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16841751#comment-16841751
]
Hadoop QA commented on YARN-9560:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
13s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 17m
4s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
4s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
25s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
42s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
12m 16s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
0s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
29s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 22s{color} | {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:
The patch generated 25 new + 5 unchanged - 2 fixed = 30 total (was 7) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
11m 54s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
4s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
25s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 21m 21s{color}
| {color:red} hadoop-yarn-server-nodemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
29s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 71m 12s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | hadoop.yarn.server.nodemanager.webapp.TestNMWebServices |
| |
hadoop.yarn.server.nodemanager.containermanager.logaggregation.TestLogAggregationService
|
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:bdbca0e |
| JIRA Issue | YARN-9560 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12968959/YARN-9560.001.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 65caff154974 4.4.0-139-generic #165-Ubuntu SMP Wed Oct 24
10:58:50 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / fab5b80 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_212 |
| findbugs | v3.1.0-RC1 |
| checkstyle |
