hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Varun Vasudev (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-5366) Add support for toggling the removal of completed and failed docker containers
Date Thu, 01 Sep 2016 16:33:20 GMT

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

Varun Vasudev commented on YARN-5366:

Thanks for the patch [~shanekumpf@gmail.com]!

+      // Validate the configured value
+      if (!keepContainer.equalsIgnoreCase("true") &&
+          !keepContainer.equalsIgnoreCase("false")) {
+        throw new IllegalArgumentException("Only true and false are valid for"
+      }

We should validate the value on submission - not when we signal the container

We should add a configuration option to let admins allow/disallow this behavior - users shouldn’t
be allowed to keep containers around because they feel like it

+        LOG.debug("Docker container is not being removed due to user request. "
+            + "ContainerId: " + containerId);

I think for now this should be logged at info level. What do you think?

+      String msg =
+          "Liveliness check failed for PID: " + ctx.getExecutionAttribute(PID)
+              + ". Container may have already completed.";
+      LOG.warn(msg);
I think this ends up double logging the message because the some one in the caller chain also
logs the same message?

+  public enum StatusState {

Rename to either DockerContainerStatus or DockerContainerState

+      if (currentContainerStatus == null) {
+        return StatusState.UNKNOWN;
+      } else if (currentContainerStatus.equals(StatusState.RUNNING.getName())) {
+        return StatusState.RUNNING;
+      } else if (currentContainerStatus.equals(StatusState.STOPPED.getName())) {
+        return StatusState.STOPPED;
+      } else if (currentContainerStatus.equals(StatusState.EXITED.getName())) {
+        return StatusState.EXITED;
+      } else {
+        return StatusState.UNKNOWN;
+      }

Minor nit - but maybe change to switch/case?

+    // Allow for injecting the container's status for testing.
+    if (statusState != null) {
+      status = statusState.getName();
+    }

Remove this. What you can do in your testing use cases is to create a class that inherits
from DockerContainerStatusHandler and returns the status that you’ve set, but the the code
snippet above shouldn’t be in DockerContainerStatusHandler

Does MockContainerExecutorBinary need to be it’s own class - the pattern is used in other
places so we should either get everyone else to use this class or move it into TestDockerContainerRuntime.

+    File f = new File("./src/test/resources/mock-container-executor");

The path of the file looks incorrect. It really should be in the target directory. You should
create a directory in the target directory and create the file in that directory. It also
looks like you create the mock executor but don’t clean it up?

Rename PrivilegedOperationCaptor to MockPrivilegedOperationCaptor? I would like the name to
reflect that it only works in the testing case.

> Add support for toggling the removal of completed and failed docker containers
> ------------------------------------------------------------------------------
>                 Key: YARN-5366
>                 URL: https://issues.apache.org/jira/browse/YARN-5366
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: yarn
>            Reporter: Shane Kumpf
>            Assignee: Shane Kumpf
>         Attachments: YARN-5366.001.patch, YARN-5366.002.patch
> Currently, completed and failed docker containers are removed by container-executor.
Add a job level environment variable to DockerLinuxContainerRuntime to allow the user to toggle
whether they want the container deleted or not and remove the logic from container-executor.

This message was sent by Atlassian JIRA

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

View raw message