Repository: hadoop
Updated Branches:
  refs/heads/trunk 7f083ed86 -> d45a0b7d7


YARN-8141.  Removed YARN_CONTAINER_RUNTIME_DOCKER_LOCAL_RESOURCE_MOUNTS flag.
            Contributed by Chandni Singh


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/d45a0b7d
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/d45a0b7d
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/d45a0b7d

Branch: refs/heads/trunk
Commit: d45a0b7d73519acb78cd94ac3186bd8481f6c13e
Parents: 7f083ed
Author: Eric Yang <ey...@apache.org>
Authored: Thu May 17 17:29:34 2018 -0400
Committer: Eric Yang <ey...@apache.org>
Committed: Thu May 17 17:29:34 2018 -0400

----------------------------------------------------------------------
 .../containerlaunch/AbstractLauncher.java       | 26 ++++++++++-----
 .../runtime/DockerLinuxContainerRuntime.java    | 35 +++-----------------
 .../runtime/TestDockerContainerRuntime.java     | 31 ++++-------------
 .../src/site/markdown/DockerContainers.md       |  1 -
 4 files changed, 28 insertions(+), 65 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/d45a0b7d/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/containerlaunch/AbstractLauncher.java
----------------------------------------------------------------------
diff --git 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/containerlaunch/AbstractLauncher.java
 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/containerlaunch/AbstractLauncher.java
index dc51b25..da5a8d6 100644
--- 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/containerlaunch/AbstractLauncher.java
+++ 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/containerlaunch/AbstractLauncher.java
@@ -46,6 +46,8 @@ public class AbstractLauncher {
   private static final Logger log =
     LoggerFactory.getLogger(AbstractLauncher.class);
   public static final String CLASSPATH = "CLASSPATH";
+  public static final String ENV_DOCKER_CONTAINER_MOUNTS =
+      "YARN_CONTAINER_RUNTIME_DOCKER_MOUNTS";
   /**
    * Env vars; set up at final launch stage
    */
@@ -153,17 +155,23 @@ public class AbstractLauncher {
         env.put("YARN_CONTAINER_RUNTIME_DOCKER_RUN_PRIVILEGED_CONTAINER",
             "true");
       }
-      StringBuilder sb = new StringBuilder();
-      for (Entry<String,String> mount : mountPaths.entrySet()) {
-        if (sb.length() > 0) {
-          sb.append(",");
+      if (!mountPaths.isEmpty()) {
+        StringBuilder sb = new StringBuilder();
+        if (env.get(ENV_DOCKER_CONTAINER_MOUNTS) != null) {
+          // user specified mounts in the spec
+          sb.append(env.get(ENV_DOCKER_CONTAINER_MOUNTS));
         }
-        sb.append(mount.getKey());
-        sb.append(":");
-        sb.append(mount.getValue());
+        for (Entry<String, String> mount : mountPaths.entrySet()) {
+          if (sb.length() > 0) {
+            sb.append(",");
+          }
+          sb.append(mount.getKey()).append(":");
+          sb.append(mount.getValue()).append(":ro");
+        }
+        env.put(ENV_DOCKER_CONTAINER_MOUNTS, sb.toString());
       }
-      env.put("YARN_CONTAINER_RUNTIME_DOCKER_LOCAL_RESOURCE_MOUNTS", 
sb.toString());
-      log.info("yarn docker env var has been set {}", 
containerLaunchContext.getEnvironment().toString());
+      log.info("yarn docker env var has been set {}",
+          containerLaunchContext.getEnvironment().toString());
     }
 
     return containerLaunchContext;

http://git-wip-us.apache.org/repos/asf/hadoop/blob/d45a0b7d/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
----------------------------------------------------------------------
diff --git 
a/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
 
b/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
index a14b085..40cb031 100644
--- 
a/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
+++ 
b/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
@@ -153,14 +153,6 @@ import static 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.r
  *     setting it to false.
  *   </li>
  *   <li>
- *     {@code YARN_CONTAINER_RUNTIME_DOCKER_LOCAL_RESOURCE_MOUNTS} adds
- *     additional volume mounts to the Docker container. The value of the
- *     environment variable should be a comma-separated list of mounts.
- *     All such mounts must be given as {@code source:dest}, where the
- *     source is an absolute path that is not a symlink and that points to a
- *     localized resource.
- *   </li>
- *   <li>
  *     {@code YARN_CONTAINER_RUNTIME_DOCKER_MOUNTS} allows users to specify
  +     additional volume mounts for the Docker container. The value of the
  *     environment variable should be a comma-separated list of mounts.
@@ -227,9 +219,6 @@ public class DockerLinuxContainerRuntime implements 
LinuxContainerRuntime {
   public static final String ENV_DOCKER_CONTAINER_RUN_ENABLE_USER_REMAPPING =
       "YARN_CONTAINER_RUNTIME_DOCKER_RUN_ENABLE_USER_REMAPPING";
   @InterfaceAudience.Private
-  public static final String ENV_DOCKER_CONTAINER_LOCAL_RESOURCE_MOUNTS =
-      "YARN_CONTAINER_RUNTIME_DOCKER_LOCAL_RESOURCE_MOUNTS";
-  @InterfaceAudience.Private
   public static final String ENV_DOCKER_CONTAINER_MOUNTS =
       "YARN_CONTAINER_RUNTIME_DOCKER_MOUNTS";
   @InterfaceAudience.Private
@@ -680,8 +669,7 @@ public class DockerLinuxContainerRuntime implements 
LinuxContainerRuntime {
     return true;
   }
 
-  @VisibleForTesting
-  protected String validateMount(String mount,
+  private String mountReadOnlyPath(String mount,
       Map<Path, List<String>> localizedResources)
       throws ContainerExecutionException {
     for (Entry<Path, List<String>> resource : localizedResources.entrySet()) {
@@ -817,23 +805,6 @@ public class DockerLinuxContainerRuntime implements 
LinuxContainerRuntime {
     runCommand.addAllReadOnlyMountLocations(filecacheDirs);
     runCommand.addAllReadOnlyMountLocations(userFilecacheDirs);
 
-    if (environment.containsKey(ENV_DOCKER_CONTAINER_LOCAL_RESOURCE_MOUNTS)) {
-      String mounts = environment.get(
-          ENV_DOCKER_CONTAINER_LOCAL_RESOURCE_MOUNTS);
-      if (!mounts.isEmpty()) {
-        for (String mount : StringUtils.split(mounts)) {
-          String[] dir = StringUtils.split(mount, ':');
-          if (dir.length != 2) {
-            throw new ContainerExecutionException("Invalid mount : " +
-                mount);
-          }
-          String src = validateMount(dir[0], localizedResources);
-          String dst = dir[1];
-          runCommand.addReadOnlyMountLocation(src, dst, true);
-        }
-      }
-    }
-
     if (environment.containsKey(ENV_DOCKER_CONTAINER_MOUNTS)) {
       Matcher parsedMounts = USER_MOUNT_PATTERN.matcher(
           environment.get(ENV_DOCKER_CONTAINER_MOUNTS));
@@ -845,6 +816,10 @@ public class DockerLinuxContainerRuntime implements 
LinuxContainerRuntime {
       parsedMounts.reset();
       while (parsedMounts.find()) {
         String src = parsedMounts.group(1);
+        java.nio.file.Path srcPath = java.nio.file.Paths.get(src);
+        if (!srcPath.isAbsolute()) {
+          src = mountReadOnlyPath(src, localizedResources);
+        }
         String dst = parsedMounts.group(2);
         String mode = parsedMounts.group(3);
         if (!mode.equals("ro") && !mode.equals("rw")) {

http://git-wip-us.apache.org/repos/asf/hadoop/blob/d45a0b7d/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
----------------------------------------------------------------------
diff --git 
a/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
 
b/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
index 6ad35b2..af69e22 100644
--- 
a/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
+++ 
b/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
@@ -26,7 +26,6 @@ import org.apache.hadoop.fs.FileUtil;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.io.DataOutputBuffer;
 import org.apache.hadoop.registry.client.api.RegistryConstants;
-import org.apache.hadoop.registry.client.binding.RegistryPathUtils;
 import org.apache.hadoop.security.Credentials;
 import org.apache.hadoop.util.Shell;
 import org.apache.hadoop.util.StringUtils;
@@ -1098,7 +1097,7 @@ public class TestDockerContainerRuntime {
     runtime.initialize(conf, nmContext);
 
     env.put(
-        DockerLinuxContainerRuntime.ENV_DOCKER_CONTAINER_LOCAL_RESOURCE_MOUNTS,
+        DockerLinuxContainerRuntime.ENV_DOCKER_CONTAINER_MOUNTS,
         "source");
 
     try {
@@ -1118,8 +1117,8 @@ public class TestDockerContainerRuntime {
     runtime.initialize(conf, nmContext);
 
     env.put(
-        DockerLinuxContainerRuntime.ENV_DOCKER_CONTAINER_LOCAL_RESOURCE_MOUNTS,
-        "test_dir/test_resource_file:test_mount");
+        DockerLinuxContainerRuntime.ENV_DOCKER_CONTAINER_MOUNTS,
+        "test_dir/test_resource_file:test_mount:ro");
 
     runtime.launchContainer(builder.build());
     PrivilegedOperation op = capturePrivilegedOperationAndVerifyArgs();
@@ -1165,24 +1164,6 @@ public class TestDockerContainerRuntime {
   }
 
   @Test
-  public void testMountInvalid() throws ContainerExecutionException {
-    DockerLinuxContainerRuntime runtime = new DockerLinuxContainerRuntime(
-        mockExecutor, mockCGroupsHandler);
-    runtime.initialize(conf, nmContext);
-
-    env.put(
-        DockerLinuxContainerRuntime.ENV_DOCKER_CONTAINER_LOCAL_RESOURCE_MOUNTS,
-        "source:target:other");
-
-    try {
-      runtime.launchContainer(builder.build());
-      Assert.fail("Expected a launch container failure due to invalid mount.");
-    } catch (ContainerExecutionException e) {
-      LOG.info("Caught expected exception : " + e);
-    }
-  }
-
-  @Test
   public void testMountMultiple()
       throws ContainerExecutionException, PrivilegedOperationException,
       IOException {
@@ -1191,9 +1172,9 @@ public class TestDockerContainerRuntime {
     runtime.initialize(conf, nmContext);
 
     env.put(
-        DockerLinuxContainerRuntime.ENV_DOCKER_CONTAINER_LOCAL_RESOURCE_MOUNTS,
-        "test_dir/test_resource_file:test_mount1," +
-            "test_dir/test_resource_file:test_mount2");
+        DockerLinuxContainerRuntime.ENV_DOCKER_CONTAINER_MOUNTS,
+        "test_dir/test_resource_file:test_mount1:ro," +
+            "test_dir/test_resource_file:test_mount2:ro");
 
     runtime.launchContainer(builder.build());
     PrivilegedOperation op = capturePrivilegedOperationAndVerifyArgs();

http://git-wip-us.apache.org/repos/asf/hadoop/blob/d45a0b7d/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/DockerContainers.md
----------------------------------------------------------------------
diff --git 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/DockerContainers.md
 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/DockerContainers.md
index 423f1da..3c39291 100644
--- 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/DockerContainers.md
+++ 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/DockerContainers.md
@@ -303,7 +303,6 @@ environment variables in the application's environment:
 | `YARN_CONTAINER_RUNTIME_DOCKER_CONTAINER_NETWORK` | Sets the network type to 
be used by the Docker container. It must be a valid value as determined by the 
yarn.nodemanager.runtime.linux.docker.allowed-container-networks property. |
 | `YARN_CONTAINER_RUNTIME_DOCKER_CONTAINER_PID_NAMESPACE` | Controls which PID 
namespace will be used by the Docker container. By default, each Docker 
container has its own PID namespace. To share the namespace of the host, the 
yarn.nodemanager.runtime.linux.docker.host-pid-namespace.allowed property must 
be set to true. If the host PID namespace is allowed and this environment 
variable is set to host, the Docker container will share the host's PID 
namespace. No other value is allowed. |
 | `YARN_CONTAINER_RUNTIME_DOCKER_RUN_PRIVILEGED_CONTAINER` | Controls whether 
the Docker container is a privileged container. In order to use privileged 
containers, the 
yarn.nodemanager.runtime.linux.docker.privileged-containers.allowed property 
must be set to true, and the application owner must appear in the value of the 
yarn.nodemanager.runtime.linux.docker.privileged-containers.acl property. If 
this environment variable is set to true, a privileged Docker container will be 
used if allowed. No other value is allowed, so the environment variable should 
be left unset rather than setting it to false. |
-| `YARN_CONTAINER_RUNTIME_DOCKER_LOCAL_RESOURCE_MOUNTS` | Adds additional 
volume mounts to the Docker container. The value of the environment variable 
should be a comma-separated list of mounts. All such mounts must be given as 
"source:dest", where the source is an absolute path that is not a symlink and 
that points to a localized resource. Note that as of YARN-5298, localized 
directories are automatically mounted into the container as volumes. |
 | `YARN_CONTAINER_RUNTIME_DOCKER_MOUNTS` | Adds additional volume mounts to 
the Docker container. The value of the environment variable should be a 
comma-separated list of mounts. All such mounts must be given as 
"source:dest:mode" and the mode must be "ro" (read-only) or "rw" (read-write) 
to specify the type of access being requested. The requested mounts will be 
validated by container-executor based on the values set in 
container-executor.cfg for docker.allowed.ro-mounts and 
docker.allowed.rw-mounts. |
 | `YARN_CONTAINER_RUNTIME_DOCKER_DELAYED_REMOVAL` | Allows a user to request 
delayed deletion of the Docker container on a per container basis. If true, 
Docker containers will not be removed until the duration defined by 
yarn.nodemanager.delete.debug-delay-sec has elapsed. Administrators can disable 
this feature through the yarn-site property 
yarn.nodemanager.runtime.linux.docker.delayed-removal.allowed. This feature is 
disabled by default. When this feature is disabled or set to false, the 
container will be removed as soon as it exits. |
 


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

Reply via email to