YARN-5301. NM mount cpu cgroups failed on some systems
(Contributed by Miklos Szegedi via Daniel Templeton)


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

Branch: refs/heads/HDFS-10467
Commit: a2f680493f040704e2b85108e286731ee3860a52
Parents: 7dd258d
Author: Daniel Templeton <templ...@apache.org>
Authored: Tue May 9 11:18:21 2017 -0700
Committer: Daniel Templeton <templ...@apache.org>
Committed: Tue May 9 12:05:46 2017 -0700

----------------------------------------------------------------------
 .../linux/resources/CGroupsHandler.java         |   6 +-
 .../linux/resources/CGroupsHandlerImpl.java     | 238 ++++++-----
 .../util/CgroupsLCEResourcesHandler.java        |  19 +-
 .../impl/container-executor.c                   |  22 +-
 .../linux/resources/TestCGroupsHandlerImpl.java | 392 ++++++++++++-------
 .../resources/TestResourceHandlerModule.java    |   3 -
 .../util/TestCgroupsLCEResourcesHandler.java    |  49 ++-
 7 files changed, 452 insertions(+), 277 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2f68049/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsHandler.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/resources/CGroupsHandler.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/resources/CGroupsHandler.java
index d09a25d..8fc35a8 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/resources/CGroupsHandler.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/resources/CGroupsHandler.java
@@ -39,7 +39,11 @@ public interface CGroupsHandler {
     CPU("cpu"),
     NET_CLS("net_cls"),
     BLKIO("blkio"),
-    MEMORY("memory");
+    MEMORY("memory"),
+    CPUACCT("cpuacct"),
+    CPUSET("cpuset"),
+    FREEZER("freezer"),
+    DEVICES("devices");
 
     private final String name;
 

http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2f68049/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsHandlerImpl.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/resources/CGroupsHandlerImpl.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/resources/CGroupsHandlerImpl.java
index 0f4c17e..85b01cd 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/resources/CGroupsHandlerImpl.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/resources/CGroupsHandlerImpl.java
@@ -21,13 +21,16 @@
 package 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Joiner;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileUtil;
+import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.io.IOUtils;
+import org.apache.hadoop.util.Shell;
 import org.apache.hadoop.yarn.conf.YarnConfiguration;
 import 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperation;
 import 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperationException;
@@ -40,8 +43,9 @@ import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.HashMap;
-import java.util.List;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.regex.Matcher;
@@ -66,6 +70,7 @@ class CGroupsHandlerImpl implements CGroupsHandler {
   private final long deleteCGroupTimeout;
   private final long deleteCGroupDelay;
   private Map<CGroupController, String> controllerPaths;
+  private Map<String, Set<String>> parsedMtab;
   private final ReadWriteLock rwLock;
   private final PrivilegedOperationExecutor privilegedOperationExecutor;
   private final Clock clock;
@@ -95,6 +100,7 @@ class CGroupsHandlerImpl implements CGroupsHandler {
         conf.getLong(YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_DELETE_DELAY,
             YarnConfiguration.DEFAULT_NM_LINUX_CONTAINER_CGROUPS_DELETE_DELAY);
     this.controllerPaths = new HashMap<>();
+    this.parsedMtab = new HashMap<>();
     this.rwLock = new ReentrantReadWriteLock();
     this.privilegedOperationExecutor = privilegedOperationExecutor;
     this.clock = SystemClock.getInstance();
@@ -128,51 +134,53 @@ class CGroupsHandlerImpl implements CGroupsHandler {
   }
 
   private void initializeControllerPaths() throws ResourceHandlerException {
-    if (enableCGroupMount) {
-      // nothing to do here - we support 'deferred' mounting of specific
-      // controllers - we'll populate the path for a given controller when an
-      // explicit mountCGroupController request is issued.
-      LOG.info("CGroup controller mounting enabled.");
-    } else {
-      // cluster admins are expected to have mounted controllers in specific
-      // locations - we'll attempt to figure out mount points
+    // Cluster admins may have some subsystems mounted in specific locations
+    // We'll attempt to figure out mount points. We do this even if we plan
+    // to mount cgroups into our own tree to control the path permissions or
+    // to mount subsystems that are not mounted previously.
+    // The subsystems for new and existing mount points have to match, and
+    // the same hierarchy will be mounted at each mount point with the same
+    // subsystem set.
+
+    Map<String, Set<String>> newMtab;
+    Map<CGroupController, String> cPaths;
+    try {
+      // parse mtab
+      newMtab = parseMtab(mtabFile);
 
-      Map<CGroupController, String> cPaths =
-          initializeControllerPathsFromMtab(mtabFile, this.cGroupPrefix);
-      // we want to do a bulk update without the paths changing concurrently
-      try {
-        rwLock.writeLock().lock();
-        controllerPaths = cPaths;
-      } finally {
-        rwLock.writeLock().unlock();
-      }
+      // find cgroup controller paths
+      cPaths = initializeControllerPathsFromMtab(newMtab);
+    } catch (IOException e) {
+      LOG.warn("Failed to initialize controller paths! Exception: " + e);
+      throw new ResourceHandlerException(
+          "Failed to initialize controller paths!");
+    }
+
+    // we want to do a bulk update without the paths changing concurrently
+    try {
+      rwLock.writeLock().lock();
+      controllerPaths = cPaths;
+      parsedMtab = newMtab;
+    } finally {
+      rwLock.writeLock().unlock();
     }
   }
 
   @VisibleForTesting
   static Map<CGroupController, String> initializeControllerPathsFromMtab(
-      String mtab, String cGroupPrefix) throws ResourceHandlerException {
-    try {
-      Map<String, List<String>> parsedMtab = parseMtab(mtab);
-      Map<CGroupController, String> ret = new HashMap<>();
+      Map<String, Set<String>> parsedMtab)
+      throws ResourceHandlerException {
+    Map<CGroupController, String> ret = new HashMap<>();
 
-      for (CGroupController controller : CGroupController.values()) {
-        String subsystemName = controller.getName();
-        String controllerPath = findControllerInMtab(subsystemName, 
parsedMtab);
+    for (CGroupController controller : CGroupController.values()) {
+      String subsystemName = controller.getName();
+      String controllerPath = findControllerInMtab(subsystemName, parsedMtab);
 
-        if (controllerPath != null) {
-          ret.put(controller, controllerPath);
-        } else {
-          LOG.warn("Controller not mounted but automount disabled: " +
-              subsystemName);
-        }
+      if (controllerPath != null) {
+        ret.put(controller, controllerPath);
       }
-      return ret;
-    } catch (IOException e) {
-      LOG.warn("Failed to initialize controller paths! Exception: " + e);
-      throw new ResourceHandlerException(
-        "Failed to initialize controller paths!");
     }
+    return ret;
   }
 
   /* We are looking for entries of the form:
@@ -190,10 +198,15 @@ class CGroupsHandlerImpl implements CGroupsHandler {
    * for mounts with type "cgroup". Cgroup controllers will
    * appear in the list of options for a path.
    */
-  private static Map<String, List<String>> parseMtab(String mtab)
+  @VisibleForTesting
+  static Map<String, Set<String>> parseMtab(String mtab)
       throws IOException {
-    Map<String, List<String>> ret = new HashMap<String, List<String>>();
+    Map<String, Set<String>> ret = new HashMap<>();
     BufferedReader in = null;
+    HashSet<String> validCgroups = new HashSet<>();
+    for (CGroupController controller : CGroupController.values()) {
+      validCgroups.add(controller.getName());
+    }
 
     try {
       FileInputStream fis = new FileInputStream(new File(mtab));
@@ -209,13 +222,21 @@ class CGroupsHandlerImpl implements CGroupsHandler {
           String options = m.group(3);
 
           if (type.equals(CGROUPS_FSTYPE)) {
-            List<String> value = Arrays.asList(options.split(","));
-            ret.put(path, value);
+            Set<String> cgroupList =
+                new HashSet<>(Arrays.asList(options.split(",")));
+            // Collect the valid subsystem names
+            cgroupList.retainAll(validCgroups);
+            ret.put(path, cgroupList);
           }
         }
       }
     } catch (IOException e) {
-      throw new IOException("Error while reading " + mtab, e);
+      if (Shell.LINUX) {
+        throw new IOException("Error while reading " + mtab, e);
+      } else {
+        // Ignore the error, if we are running on an os other than Linux
+        LOG.warn("Error while reading " + mtab, e);
+      }
     } finally {
       IOUtils.cleanup(LOG, in);
     }
@@ -234,8 +255,8 @@ class CGroupsHandlerImpl implements CGroupsHandler {
    */
   @VisibleForTesting
   static String findControllerInMtab(String controller,
-      Map<String, List<String>> entries) {
-    for (Map.Entry<String, List<String>> e : entries.entrySet()) {
+      Map<String, Set<String>> entries) {
+    for (Map.Entry<String, Set<String>> e : entries.entrySet()) {
       if (e.getValue().contains(controller)) {
         if (new File(e.getKey()).canRead()) {
           return e.getKey();
@@ -251,31 +272,45 @@ class CGroupsHandlerImpl implements CGroupsHandler {
 
   private void mountCGroupController(CGroupController controller)
       throws ResourceHandlerException {
-    String path = getControllerPath(controller);
+    if (cGroupMountPath == null) {
+      throw new ResourceHandlerException(
+          String.format("Cgroups mount path not specified in %s.",
+              YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_MOUNT_PATH));
+    }
+    String existingMountPath = getControllerPath(controller);
+    String requestedMountPath =
+        new File(cGroupMountPath, controller.getName()).getAbsolutePath();
 
-    if (path == null) {
+    if (existingMountPath == null ||
+        !requestedMountPath.equals(existingMountPath)) {
       try {
         //lock out other readers/writers till we are done
         rwLock.writeLock().lock();
 
-        String hierarchy = cGroupPrefix;
-        StringBuffer controllerPath = new StringBuffer()
-            .append(cGroupMountPath).append('/').append(controller.getName());
-        StringBuffer cGroupKV = new StringBuffer()
-            .append(controller.getName()).append('=').append(controllerPath);
+        // If the controller was already mounted we have to mount it
+        // with the same options to clone the mount point otherwise
+        // the operation will fail
+        String mountOptions;
+        if (existingMountPath != null) {
+          mountOptions = Joiner.on(',')
+              .join(parsedMtab.get(existingMountPath));
+        } else {
+          mountOptions = controller.getName();
+        }
+
+        String cGroupKV =
+            mountOptions + "=" + requestedMountPath;
         PrivilegedOperation.OperationType opType = PrivilegedOperation
             .OperationType.MOUNT_CGROUPS;
         PrivilegedOperation op = new PrivilegedOperation(opType);
 
-        op.appendArgs(hierarchy, cGroupKV.toString());
+        op.appendArgs(cGroupPrefix, cGroupKV);
         LOG.info("Mounting controller " + controller.getName() + " at " +
-              controllerPath);
+              requestedMountPath);
         privilegedOperationExecutor.executePrivilegedOperation(op, false);
 
         //if privileged operation succeeds, update controller paths
-        controllerPaths.put(controller, controllerPath.toString());
-
-        return;
+        controllerPaths.put(controller, requestedMountPath);
       } catch (PrivilegedOperationException e) {
         LOG.error("Failed to mount controller: " + controller.getName());
         throw new ResourceHandlerException("Failed to mount controller: "
@@ -284,37 +319,34 @@ class CGroupsHandlerImpl implements CGroupsHandler {
         rwLock.writeLock().unlock();
       }
     } else {
-      LOG.info("CGroup controller already mounted at: " + path);
-      return;
+      LOG.info("CGroup controller already mounted at: " + existingMountPath);
     }
   }
 
   @Override
   public String getRelativePathForCGroup(String cGroupId) {
-    return new StringBuffer(cGroupPrefix).append("/")
-        .append(cGroupId).toString();
+    return cGroupPrefix + Path.SEPARATOR + cGroupId;
   }
 
   @Override
   public String getPathForCGroup(CGroupController controller, String cGroupId) 
{
-    return new StringBuffer(getControllerPath(controller))
-        .append('/').append(cGroupPrefix).append("/")
-        .append(cGroupId).toString();
+    return getControllerPath(controller) + Path.SEPARATOR + cGroupPrefix
+        + Path.SEPARATOR + cGroupId;
   }
 
   @Override
   public String getPathForCGroupTasks(CGroupController controller,
       String cGroupId) {
-    return new StringBuffer(getPathForCGroup(controller, cGroupId))
-        .append('/').append(CGROUP_FILE_TASKS).toString();
+    return getPathForCGroup(controller, cGroupId)
+        + Path.SEPARATOR + CGROUP_FILE_TASKS;
   }
 
   @Override
   public String getPathForCGroupParam(CGroupController controller,
       String cGroupId, String param) {
-    return new StringBuffer(getPathForCGroup(controller, cGroupId))
-        .append('/').append(controller.getName()).append('.')
-        .append(param).toString();
+    return getPathForCGroup(controller, cGroupId)
+        + Path.SEPARATOR + controller.getName()
+        + "." + param;
   }
 
   /**
@@ -330,10 +362,21 @@ class CGroupsHandlerImpl implements CGroupsHandler {
       // We have a controller that needs to be mounted
       mountCGroupController(controller);
     } else {
-      // We are working with a pre-mounted contoller
-      // Make sure that Yarn cgroup hierarchy path exists
-      initializePreMountedCGroupController(controller);
+      String controllerPath = getControllerPath(controller);
+
+      if (controllerPath == null) {
+        throw new ResourceHandlerException(
+            String.format("Controller %s not mounted."
+                + " You either need to mount it with %s"
+                + " or mount cgroups before launching Yarn",
+                controller.getName(), YarnConfiguration.
+                NM_LINUX_CONTAINER_CGROUPS_MOUNT));
+      }
     }
+
+    // We are working with a pre-mounted contoller
+    // Make sure that Yarn cgroup hierarchy path exists
+    initializePreMountedCGroupController(controller);
   }
 
   /**
@@ -347,11 +390,22 @@ class CGroupsHandlerImpl implements CGroupsHandler {
    * @throws ResourceHandlerException yarn hierarchy cannot be created or
    *   accessed for any reason
    */
-  public void initializePreMountedCGroupController(CGroupController controller)
+  private void initializePreMountedCGroupController(CGroupController 
controller)
       throws ResourceHandlerException {
     // Check permissions to cgroup hierarchy and
     // create YARN cgroup if it does not exist, yet
-    File rootHierarchy = new File(getControllerPath(controller));
+    String controllerPath = getControllerPath(controller);
+
+    if (controllerPath == null) {
+      throw new ResourceHandlerException(
+          String.format("Controller %s not mounted."
+                  + " You either need to mount it with %s"
+                  + " or mount cgroups before launching Yarn",
+              controller.getName(), YarnConfiguration.
+                  NM_LINUX_CONTAINER_CGROUPS_MOUNT));
+    }
+
+    File rootHierarchy = new File(controllerPath);
     File yarnHierarchy = new File(rootHierarchy, cGroupPrefix);
     String subsystemName = controller.getName();
 
@@ -403,17 +457,9 @@ class CGroupsHandlerImpl implements CGroupsHandler {
       String errorMessage,
       String subsystemName,
       String yarnCgroupPath) {
-    return new StringBuilder()
-        .append(errorMessage)
-        .append(" Subsystem:")
-        .append(subsystemName)
-        .append(" Mount points:")
-        .append(mtabFile)
-        .append(" User:")
-        .append(System.getProperty("user.name"))
-        .append(" Path: ")
-        .append(yarnCgroupPath)
-        .toString();
+    return String.format("%s Subsystem:%s Mount points:%s User:%s Path:%s ",
+        errorMessage, subsystemName, mtabFile, System.getProperty("user.name"),
+        yarnCgroupPath);
   }
 
   @Override
@@ -456,7 +502,7 @@ class CGroupsHandlerImpl implements CGroupsHandler {
    * @param cgf object referring to the cgroup to be deleted
    * @return Boolean indicating whether cgroup was deleted
    */
-  boolean checkAndDeleteCgroup(File cgf) throws InterruptedException {
+  private boolean checkAndDeleteCgroup(File cgf) throws InterruptedException {
     boolean deleted = false;
     // FileInputStream in = null;
     try (FileInputStream in = new FileInputStream(cgf + "/tasks")) {
@@ -504,8 +550,8 @@ class CGroupsHandlerImpl implements CGroupsHandler {
     } while (!deleted && (clock.getTime() - start) < deleteCGroupTimeout);
 
     if (!deleted) {
-      LOG.warn("Unable to delete  " + cGroupPath +
-          ", tried to delete for " + deleteCGroupTimeout + "ms");
+      LOG.warn(String.format("Unable to delete  %s, tried to delete for %d ms",
+          cGroupPath, deleteCGroupTimeout));
     }
   }
 
@@ -517,8 +563,8 @@ class CGroupsHandlerImpl implements CGroupsHandler {
 
     if (LOG.isDebugEnabled()) {
       LOG.debug(
-          "updateCGroupParam for path: " + cGroupParamPath + " with value " +
-              value);
+          String.format("updateCGroupParam for path: %s with value %s",
+              cGroupParamPath, value));
     }
 
     try {
@@ -527,22 +573,22 @@ class CGroupsHandlerImpl implements CGroupsHandler {
       pw = new PrintWriter(w);
       pw.write(value);
     } catch (IOException e) {
-      throw new ResourceHandlerException(new StringBuffer("Unable to write to 
")
-          .append(cGroupParamPath).append(" with value: ").append(value)
-          .toString(), e);
+      throw new ResourceHandlerException(
+          String.format("Unable to write to %s with value: %s",
+              cGroupParamPath, value), e);
     } finally {
       if (pw != null) {
         boolean hasError = pw.checkError();
         pw.close();
         if (hasError) {
           throw new ResourceHandlerException(
-              new StringBuffer("Unable to write to ")
-                  .append(cGroupParamPath).append(" with value: 
").append(value)
-                  .toString());
+              String.format("PrintWriter unable to write to %s with value: %s",
+                  cGroupParamPath, value));
         }
         if (pw.checkError()) {
-          throw new ResourceHandlerException("Error while closing cgroup file" 
+
-              " " + cGroupParamPath);
+          throw new ResourceHandlerException(
+              String.format("Error while closing cgroup file %s",
+                  cGroupParamPath));
         }
       }
     }

http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2f68049/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/util/CgroupsLCEResourcesHandler.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/util/CgroupsLCEResourcesHandler.java
 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/util/CgroupsLCEResourcesHandler.java
index cb4dcf6..bca4fdc 100644
--- 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/util/CgroupsLCEResourcesHandler.java
+++ 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/util/CgroupsLCEResourcesHandler.java
@@ -28,17 +28,18 @@ import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.io.Writer;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import com.google.common.annotations.VisibleForTesting;
 
+import com.google.common.collect.Sets;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
@@ -262,7 +263,7 @@ public class CgroupsLCEResourcesHandler implements 
LCEResourcesHandler {
   /**
    * If tasks file is empty, delete the cgroup.
    *
-   * @param file object referring to the cgroup to be deleted
+   * @param cgf object referring to the cgroup to be deleted
    * @return Boolean indicating whether cgroup was deleted
    */
   @VisibleForTesting
@@ -396,8 +397,8 @@ public class CgroupsLCEResourcesHandler implements 
LCEResourcesHandler {
    * for mounts with type "cgroup". Cgroup controllers will
    * appear in the list of options for a path.
    */
-  private Map<String, List<String>> parseMtab() throws IOException {
-    Map<String, List<String>> ret = new HashMap<String, List<String>>();
+  private Map<String, Set<String>> parseMtab() throws IOException {
+    Map<String, Set<String>> ret = new HashMap<String, Set<String>>();
     BufferedReader in = null;
 
     try {
@@ -414,7 +415,7 @@ public class CgroupsLCEResourcesHandler implements 
LCEResourcesHandler {
           String options = m.group(3);
 
           if (type.equals(CGROUPS_FSTYPE)) {
-            List<String> value = Arrays.asList(options.split(","));
+            HashSet<String> value = Sets.newHashSet(options.split(","));
             ret.put(path, value);
           }
         }
@@ -430,8 +431,8 @@ public class CgroupsLCEResourcesHandler implements 
LCEResourcesHandler {
 
   @VisibleForTesting
   String findControllerInMtab(String controller,
-                                      Map<String, List<String>> entries) {
-    for (Entry<String, List<String>> e : entries.entrySet()) {
+                                      Map<String, Set<String>> entries) {
+    for (Entry<String, Set<String>> e : entries.entrySet()) {
       if (e.getValue().contains(controller)) {
         if (new File(e.getKey()).canRead()) {
           return e.getKey();
@@ -447,7 +448,7 @@ public class CgroupsLCEResourcesHandler implements 
LCEResourcesHandler {
 
   private void initializeControllerPaths() throws IOException {
     String controllerPath;
-    Map<String, List<String>> parsedMtab = parseMtab();
+    Map<String, Set<String>> parsedMtab = parseMtab();
 
     // CPU
 

http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2f68049/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
----------------------------------------------------------------------
diff --git 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
index bf08035..f11272e 100644
--- 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
+++ 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
@@ -2050,17 +2050,31 @@ int mount_cgroup(const char *pair, const char 
*hierarchy) {
   fprintf(LOGFILE, "Failed to mount cgroup controller, not supported\n");
   return -1;
 #else
-  char *controller = malloc(strlen(pair));
-  char *mount_path = malloc(strlen(pair));
+  size_t len = strlen(pair);
+  char *controller = malloc(len);
+  char *mount_path = malloc(len);
   char hier_path[EXECUTOR_PATH_MAX];
   int result = 0;
+  struct stat sb;
 
-  if (get_kv_key(pair, controller, strlen(pair)) < 0 ||
-      get_kv_value(pair, mount_path, strlen(pair)) < 0) {
+  if (controller == NULL || mount_path == NULL) {
+    fprintf(LOGFILE, "Failed to mount cgroup controller; not enough memory\n");
+    result = OUT_OF_MEMORY;
+  }
+  if (get_kv_key(pair, controller, len) < 0 ||
+      get_kv_value(pair, mount_path, len) < 0) {
     fprintf(LOGFILE, "Failed to mount cgroup controller; invalid option: %s\n",
               pair);
     result = -1;
   } else {
+    if (stat(mount_path, &sb) != 0) {
+      // Create mount point, if it does not exist
+      const mode_t mount_perms = S_IRWXU | S_IRGRP | S_IXGRP;
+      if (mkdirs(mount_path, mount_perms) == 0) {
+        fprintf(LOGFILE, "Failed to create cgroup mount point %s at %s\n",
+          controller, mount_path);
+      }
+    }
     if (mount("none", mount_path, "cgroup", 0, controller) == 0) {
       char *buf = stpncpy(hier_path, mount_path, strlen(mount_path));
       *buf++ = '/';

http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2f68049/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/TestCGroupsHandlerImpl.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/resources/TestCGroupsHandlerImpl.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/resources/TestCGroupsHandlerImpl.java
index 804f8e7..dd8e338 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/resources/TestCGroupsHandlerImpl.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/resources/TestCGroupsHandlerImpl.java
@@ -25,6 +25,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileUtil;
+import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.yarn.conf.YarnConfiguration;
 import 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperation;
 import 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperationException;
@@ -42,10 +43,11 @@ import java.nio.file.Files;
 import java.security.Permission;
 import java.util.Collections;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.UUID;
 
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -60,7 +62,6 @@ public class TestCGroupsHandlerImpl {
       LogFactory.getLog(TestCGroupsHandlerImpl.class);
 
   private PrivilegedOperationExecutor privilegedOperationExecutorMock;
-  private Configuration conf;
   private String tmpPath;
   private String hierarchy;
   private CGroupsHandler.CGroupController controller;
@@ -69,36 +70,143 @@ public class TestCGroupsHandlerImpl {
   @Before
   public void setup() {
     privilegedOperationExecutorMock = mock(PrivilegedOperationExecutor.class);
-    conf = new YarnConfiguration();
+
+    // Prepare test directory
     tmpPath = System.getProperty("test.build.data") + "/cgroups";
+    File tmpDir = new File(tmpPath);
+    FileUtils.deleteQuietly(tmpDir);
+    assertTrue(tmpDir.mkdirs());
+
     //no leading or trailing slashes here
     hierarchy = "test-hadoop-yarn";
 
+    // Sample subsystem. Not used by all the tests
+    controller = CGroupsHandler.CGroupController.NET_CLS;
+    controllerPath =
+        new File(new File(tmpPath, controller.getName()), hierarchy)
+            .getAbsolutePath();
+  }
+
+  @After
+  public void teardown() {
+    FileUtil.fullyDelete(new File(tmpPath));
+  }
+
+  /**
+   * Security manager simulating access denied.
+   */
+  private class MockSecurityManagerDenyWrite extends SecurityManager {
+    @Override
+    public void checkPermission(Permission perm) {
+      if(perm.getActions().equals("write")) {
+        throw new SecurityException("Mock not allowed");
+      }
+    }
+  }
+
+  /**
+   * Create configuration to mount cgroups that do not exist.
+   * @return configuration object
+   */
+  private YarnConfiguration createMountConfiguration() {
+    YarnConfiguration conf = new YarnConfiguration();
     conf.set(YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_HIERARCHY, 
hierarchy);
     conf.setBoolean(YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_MOUNT, true);
     conf.set(YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_MOUNT_PATH, tmpPath);
-    controller = CGroupsHandler.CGroupController.NET_CLS;
-    controllerPath = new StringBuffer(tmpPath).append('/')
-        .append(controller.getName()).append('/').append(hierarchy).toString();
+    return conf;
+  }
+
+  /**
+   * Create configuration where the cgroups are premounted.
+   * @param myHierarchy Yarn cgroup
+   * @return configuration object
+   */
+  private Configuration createNoMountConfiguration(String myHierarchy) {
+    Configuration confNoMount = new Configuration();
+    confNoMount.set(YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_HIERARCHY,
+        myHierarchy);
+    confNoMount.setBoolean(YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_MOUNT,
+        false);
+    return confNoMount;
+  }
+
+  /**
+   * Create an empty mtab file. No cgroups are premounted
+   * @return mtab file
+   * @throws IOException could not create file
+   */
+  private File createEmptyCgroups() throws IOException {
+    File emptyMtab = new File(tmpPath, "mtab");
+    assertTrue("New file should have been created", emptyMtab.createNewFile());
+    return emptyMtab;
+  }
+
+  /**
+   * Create simulated cgroups mount point.
+   * @param parentDir cgroups mount point
+   * @param cpuAcct simulate newer Linux behavior by mounting cpu with cpuacct
+   * @return simulated mtab file location
+   * @throws IOException mtab file was not created
+   */
+  public static File createPremountedCgroups(File parentDir, boolean cpuAcct)
+      throws IOException {
+    // Mark an empty directory called 'cp' cgroup. It is processed before 'cpu'
+    String cpuMtabContentMissing =
+        "none " + parentDir.getAbsolutePath()
+            + "/cp cgroup rw,relatime,cpu 0 0\n";
+
+    File cpuCgroup = new File(parentDir, "cpu");
+    String cpuMtabContent =
+        "none " + cpuCgroup.getAbsolutePath()
+            + " cgroup rw,relatime,cpu"
+            + (cpuAcct ? ",cpuacct" :"")
+            + " 0 0\n";
+    assertTrue("Directory should be created", cpuCgroup.mkdirs());
+
+    File blkioCgroup = new File(parentDir, "blkio");
+    String blkioMtabContent =
+        "none " + blkioCgroup.getAbsolutePath()
+            + " cgroup rw,relatime,blkio 0 0\n";
+    assertTrue("Directory should be created", blkioCgroup.mkdirs());
+
+    File mockMtab = new File(parentDir, UUID.randomUUID().toString());
+    if (!mockMtab.exists()) {
+      if (!mockMtab.createNewFile()) {
+        String message = "Could not create file " + mockMtab.getAbsolutePath();
+        throw new IOException(message);
+      }
+    }
+    FileWriter mtabWriter = new FileWriter(mockMtab.getAbsoluteFile());
+    mtabWriter.write(cpuMtabContentMissing);
+    mtabWriter.write(cpuMtabContent);
+    mtabWriter.write(blkioMtabContent);
+    mtabWriter.close();
+    mockMtab.deleteOnExit();
+    return mockMtab;
   }
 
   @Test
-  public void testMountController() {
-    CGroupsHandler cGroupsHandler = null;
+  public void testMountController() throws IOException {
+    File parentDir = new File(tmpPath);
+    File cgroup = new File(parentDir, controller.getName());
+    assertTrue("cgroup dir should be cerated", cgroup.mkdirs());
     //Since we enabled (deferred) cgroup controller mounting, no interactions
     //should have occurred, with this mock
     verifyZeroInteractions(privilegedOperationExecutorMock);
+    File emptyMtab = createEmptyCgroups();
 
     try {
-      cGroupsHandler = new CGroupsHandlerImpl(conf,
-          privilegedOperationExecutorMock);
+      CGroupsHandler cGroupsHandler = new CGroupsHandlerImpl(
+          createMountConfiguration(),
+          privilegedOperationExecutorMock,
+          emptyMtab.getAbsolutePath());
       PrivilegedOperation expectedOp = new PrivilegedOperation(
           PrivilegedOperation.OperationType.MOUNT_CGROUPS);
       //This is expected to be of the form :
       //net_cls=<mount_path>/net_cls
-      StringBuffer controllerKV = new StringBuffer(controller.getName())
-          
.append('=').append(tmpPath).append('/').append(controller.getName());
-      expectedOp.appendArgs(hierarchy, controllerKV.toString());
+      String controllerKV = controller.getName() + "=" + tmpPath
+          + Path.SEPARATOR + controller.getName();
+      expectedOp.appendArgs(hierarchy, controllerKV);
 
       cGroupsHandler.initializeCGroupController(controller);
       try {
@@ -117,79 +225,90 @@ public class TestCGroupsHandlerImpl {
         verifyNoMoreInteractions(privilegedOperationExecutorMock);
       } catch (PrivilegedOperationException e) {
         LOG.error("Caught exception: " + e);
-        Assert.assertTrue("Unexpected PrivilegedOperationException from mock!",
+        assertTrue("Unexpected PrivilegedOperationException from mock!",
             false);
       }
     } catch (ResourceHandlerException e) {
       LOG.error("Caught exception: " + e);
-      Assert.assertTrue("Unexpected ResourceHandler Exception!", false);
+      assertTrue("Unexpected ResourceHandler Exception!", false);
     }
   }
 
   @Test
-  public void testCGroupPaths() {
+  public void testCGroupPaths() throws IOException {
     //As per junit behavior, we expect a new mock object to be available
     //in this test.
     verifyZeroInteractions(privilegedOperationExecutorMock);
     CGroupsHandler cGroupsHandler = null;
+    File mtab = createEmptyCgroups();
+
+    // Lets manually create a path to (partially) simulate a controller mounted
+    // later in the test. This is required because the handler uses a mocked
+    // privileged operation executor
+    assertTrue("Sample subsystem should be created",
+        new File(controllerPath).mkdirs());
+
     try {
-      cGroupsHandler = new CGroupsHandlerImpl(conf,
-          privilegedOperationExecutorMock);
+      cGroupsHandler = new CGroupsHandlerImpl(createMountConfiguration(),
+          privilegedOperationExecutorMock, mtab.getAbsolutePath());
       cGroupsHandler.initializeCGroupController(controller);
     } catch (ResourceHandlerException e) {
       LOG.error("Caught exception: " + e);
-      Assert.assertTrue(
+      assertTrue(
           "Unexpected ResourceHandlerException when mounting controller!",
           false);
     }
 
     String testCGroup = "container_01";
-    String expectedPath = new StringBuffer(controllerPath).append('/')
-        .append(testCGroup).toString();
+    String expectedPath =
+        controllerPath + Path.SEPARATOR + testCGroup;
     String path = cGroupsHandler.getPathForCGroup(controller, testCGroup);
     Assert.assertEquals(expectedPath, path);
 
-    String expectedPathTasks = new StringBuffer(expectedPath).append('/')
-        .append(CGroupsHandler.CGROUP_FILE_TASKS).toString();
+    String expectedPathTasks = expectedPath + Path.SEPARATOR
+        + CGroupsHandler.CGROUP_FILE_TASKS;
     path = cGroupsHandler.getPathForCGroupTasks(controller, testCGroup);
     Assert.assertEquals(expectedPathTasks, path);
 
     String param = CGroupsHandler.CGROUP_PARAM_CLASSID;
-    String expectedPathParam = new StringBuffer(expectedPath).append('/')
-        .append(controller.getName()).append('.').append(param).toString();
+    String expectedPathParam = expectedPath + Path.SEPARATOR
+        + controller.getName() + "." + param;
     path = cGroupsHandler.getPathForCGroupParam(controller, testCGroup, param);
     Assert.assertEquals(expectedPathParam, path);
   }
 
   @Test
-  public void testCGroupOperations() {
+  public void testCGroupOperations() throws IOException {
     //As per junit behavior, we expect a new mock object to be available
     //in this test.
     verifyZeroInteractions(privilegedOperationExecutorMock);
     CGroupsHandler cGroupsHandler = null;
+    File mtab = createEmptyCgroups();
+
+    // Lets manually create a path to (partially) simulate a controller mounted
+    // later in the test. This is required because the handler uses a mocked
+    // privileged operation executor
+    assertTrue("Sample subsystem should be created",
+        new File(controllerPath).mkdirs());
 
     try {
-      cGroupsHandler = new CGroupsHandlerImpl(conf,
-          privilegedOperationExecutorMock);
+      cGroupsHandler = new CGroupsHandlerImpl(createMountConfiguration(),
+          privilegedOperationExecutorMock, mtab.getAbsolutePath());
       cGroupsHandler.initializeCGroupController(controller);
     } catch (ResourceHandlerException e) {
       LOG.error("Caught exception: " + e);
-      Assert.assertTrue(
+      assertTrue(
           "Unexpected ResourceHandlerException when mounting controller!",
           false);
     }
-    //Lets manually create a path to (partially) simulate a mounted controller
-    //this is required because the handler uses a mocked privileged operation
-    //executor
-    new File(controllerPath).mkdirs();
 
     String testCGroup = "container_01";
-    String expectedPath = new StringBuffer(controllerPath).append('/')
-        .append(testCGroup).toString();
+    String expectedPath = controllerPath
+        + Path.SEPARATOR + testCGroup;
     try {
       String path = cGroupsHandler.createCGroup(controller, testCGroup);
 
-      Assert.assertTrue(new File(expectedPath).exists());
+      assertTrue(new File(expectedPath).exists());
       Assert.assertEquals(expectedPath, path);
 
       //update param and read param tests.
@@ -202,11 +321,12 @@ public class TestCGroupsHandlerImpl {
 
       cGroupsHandler
           .updateCGroupParam(controller, testCGroup, param, paramValue);
-      String paramPath = new StringBuffer(expectedPath).append('/')
-          .append(controller.getName()).append('.').append(param).toString();
+      String paramPath = expectedPath
+          + Path.SEPARATOR + controller.getName()
+          + "." + param;
       File paramFile = new File(paramPath);
 
-      Assert.assertTrue(paramFile.exists());
+      assertTrue(paramFile.exists());
       try {
         Assert.assertEquals(paramValue, new String(Files.readAllBytes(
             paramFile.toPath())));
@@ -233,52 +353,6 @@ public class TestCGroupsHandlerImpl {
     }
   }
 
-  public static File createMockCgroupMount(File parentDir, String type)
-      throws IOException {
-    return createMockCgroupMount(parentDir, type, "hadoop-yarn");
-  }
-
-  private static File createMockCgroupMount(File parentDir, String type,
-      String hierarchy) throws IOException {
-    File cgroupMountDir =
-        new File(parentDir.getAbsolutePath(), type + "/" + hierarchy);
-    FileUtils.deleteQuietly(cgroupMountDir);
-    if (!cgroupMountDir.mkdirs()) {
-      String message =
-          "Could not create dir " + cgroupMountDir.getAbsolutePath();
-      throw new IOException(message);
-    }
-    return cgroupMountDir;
-  }
-
-  public static File createMockMTab(File parentDir) throws IOException {
-    String cpuMtabContent =
-        "none " + parentDir.getAbsolutePath()
-            + "/cpu cgroup rw,relatime,cpu 0 0\n";
-    // Mark an empty directory called 'cp' cgroup. It is processed before 'cpu'
-    String cpuMtabContentMissing =
-        "none " + parentDir.getAbsolutePath()
-            + "/cp cgroup rw,relatime,cpu 0 0\n";
-    String blkioMtabContent =
-        "none " + parentDir.getAbsolutePath()
-            + "/blkio cgroup rw,relatime,blkio 0 0\n";
-
-    File mockMtab = new File(parentDir, UUID.randomUUID().toString());
-    if (!mockMtab.exists()) {
-      if (!mockMtab.createNewFile()) {
-        String message = "Could not create file " + mockMtab.getAbsolutePath();
-        throw new IOException(message);
-      }
-    }
-    FileWriter mtabWriter = new FileWriter(mockMtab.getAbsoluteFile());
-    mtabWriter.write(cpuMtabContentMissing);
-    mtabWriter.write(cpuMtabContent);
-    mtabWriter.write(blkioMtabContent);
-    mtabWriter.close();
-    mockMtab.deleteOnExit();
-    return mockMtab;
-  }
-
   /**
    * Tests whether mtab parsing works as expected with a valid hierarchy set.
    * @throws Exception the test will fail
@@ -288,24 +362,20 @@ public class TestCGroupsHandlerImpl {
     // Initialize mtab and cgroup dir
     File parentDir = new File(tmpPath);
     // create mock cgroup
-    File cpuCgroupMountDir = createMockCgroupMount(parentDir, "cpu",
-        hierarchy);
-    Assert.assertTrue(cpuCgroupMountDir.exists());
-    File blkioCgroupMountDir = createMockCgroupMount(parentDir,
-        "blkio", hierarchy);
-    Assert.assertTrue(blkioCgroupMountDir.exists());
-    File mockMtabFile = createMockMTab(parentDir);
+    File mockMtabFile = createPremountedCgroups(parentDir, false);
 
     // Run mtabs parsing
+    Map<String, Set<String>> newMtab =
+        CGroupsHandlerImpl.parseMtab(mockMtabFile.getAbsolutePath());
     Map<CGroupsHandler.CGroupController, String> controllerPaths =
         CGroupsHandlerImpl.initializeControllerPathsFromMtab(
-            mockMtabFile.getAbsolutePath(), hierarchy);
+            newMtab);
 
     // Verify
     Assert.assertEquals(2, controllerPaths.size());
-    Assert.assertTrue(controllerPaths
+    assertTrue(controllerPaths
         .containsKey(CGroupsHandler.CGroupController.CPU));
-    Assert.assertTrue(controllerPaths
+    assertTrue(controllerPaths
         .containsKey(CGroupsHandler.CGroupController.BLKIO));
     String cpuDir = controllerPaths.get(CGroupsHandler.CGroupController.CPU);
     String blkioDir =
@@ -315,17 +385,6 @@ public class TestCGroupsHandlerImpl {
   }
 
   /**
-   * Tests whether mtab parsing works as expected with an empty hierarchy set.
-   * @throws Exception the test will fail
-   */
-  @Test
-  public void testPreMountedController() throws Exception {
-    testPreMountedControllerInitialization("hadoop-yarn");
-    testPreMountedControllerInitialization("");
-    testPreMountedControllerInitialization("/");
-  }
-
-  /**
    * Tests whether mtab parsing works as expected with the specified hierarchy.
    * @param myHierarchy path to local cgroup hierarchy
    * @throws Exception the test will fail
@@ -334,64 +393,57 @@ public class TestCGroupsHandlerImpl {
       throws Exception {
     // Initialize mount point
     File parentDir = new File(tmpPath);
-    FileUtils.deleteQuietly(parentDir);
-    Assert.assertTrue("Could not create dirs", parentDir.mkdirs());
-    File mtab = createMockMTab(parentDir);
+    File mtab = createPremountedCgroups(parentDir, false);
     File mountPoint = new File(parentDir, "cpu");
-    File cpuCgroupMountDir = createMockCgroupMount(
-        parentDir, "cpu", myHierarchy);
 
     // Initialize Yarn classes
-    Configuration confNoMount = new Configuration();
-    confNoMount.set(YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_HIERARCHY,
-        myHierarchy);
-    confNoMount.setBoolean(YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_MOUNT,
-        false);
+    Configuration confNoMount = createNoMountConfiguration(myHierarchy);
     CGroupsHandlerImpl cGroupsHandler = new CGroupsHandlerImpl(confNoMount,
         privilegedOperationExecutorMock, mtab.getAbsolutePath());
 
-
+    File cpuCgroupMountDir = new File(
+        cGroupsHandler.getPathForCGroup(CGroupsHandler.CGroupController.CPU,
+            ""));
     // Test that a missing yarn hierarchy will be created automatically
     if (!cpuCgroupMountDir.equals(mountPoint)) {
-      Assert.assertTrue("Could not delete cgroups", 
cpuCgroupMountDir.delete());
-      Assert.assertTrue("Directory should be deleted",
+      assertTrue("Directory should be deleted",
           !cpuCgroupMountDir.exists());
     }
     cGroupsHandler.initializeCGroupController(
         CGroupsHandler.CGroupController.CPU);
-    Assert.assertTrue("Cgroups not writable", cpuCgroupMountDir.exists() &&
+    assertTrue("Cgroups not writable", cpuCgroupMountDir.exists() &&
         cpuCgroupMountDir.canWrite());
 
     // Test that an inaccessible yarn hierarchy results in an exception
-    Assert.assertTrue(cpuCgroupMountDir.setWritable(false));
+    assertTrue(cpuCgroupMountDir.setWritable(false));
     try {
       cGroupsHandler.initializeCGroupController(
           CGroupsHandler.CGroupController.CPU);
       Assert.fail("An inaccessible path should result in an exception");
     } catch (Exception e) {
-      Assert.assertTrue("Unexpected exception " + e.getClass().toString(),
+      assertTrue("Unexpected exception " + e.getClass().toString(),
           e instanceof ResourceHandlerException);
     } finally {
-      Assert.assertTrue("Could not revert writable permission",
+      assertTrue("Could not revert writable permission",
           cpuCgroupMountDir.setWritable(true));
     }
 
     // Test that a non-accessible mount directory results in an exception
     if (!cpuCgroupMountDir.equals(mountPoint)) {
-      Assert.assertTrue("Could not delete cgroups", 
cpuCgroupMountDir.delete());
-      Assert.assertTrue("Directory should be deleted",
+      assertTrue("Could not delete cgroups", cpuCgroupMountDir.delete());
+      assertTrue("Directory should be deleted",
           !cpuCgroupMountDir.exists());
     }
-    Assert.assertTrue(mountPoint.setWritable(false));
+    assertTrue(mountPoint.setWritable(false));
     try {
       cGroupsHandler.initializeCGroupController(
           CGroupsHandler.CGroupController.CPU);
       Assert.fail("An inaccessible path should result in an exception");
     } catch (Exception e) {
-      Assert.assertTrue("Unexpected exception " + e.getClass().toString(),
+      assertTrue("Unexpected exception " + e.getClass().toString(),
           e instanceof ResourceHandlerException);
     } finally {
-      Assert.assertTrue("Could not revert writable permission",
+      assertTrue("Could not revert writable permission",
           mountPoint.setWritable(true));
     }
 
@@ -399,7 +451,7 @@ public class TestCGroupsHandlerImpl {
     if (!cpuCgroupMountDir.equals(mountPoint)) {
       Assert.assertFalse("Could not delete cgroups",
           cpuCgroupMountDir.delete());
-      Assert.assertTrue("Directory should be deleted",
+      assertTrue("Directory should be deleted",
           !cpuCgroupMountDir.exists());
       SecurityManager manager = System.getSecurityManager();
       System.setSecurityManager(new MockSecurityManagerDenyWrite());
@@ -408,7 +460,7 @@ public class TestCGroupsHandlerImpl {
             CGroupsHandler.CGroupController.CPU);
         Assert.fail("An inaccessible path should result in an exception");
       } catch (Exception e) {
-        Assert.assertTrue("Unexpected exception " + e.getClass().toString(),
+        assertTrue("Unexpected exception " + e.getClass().toString(),
             e instanceof ResourceHandlerException);
       } finally {
         System.setSecurityManager(manager);
@@ -419,18 +471,18 @@ public class TestCGroupsHandlerImpl {
     if (!cpuCgroupMountDir.equals(mountPoint)) {
       Assert.assertFalse("Could not delete cgroups",
           cpuCgroupMountDir.delete());
-      Assert.assertTrue("Directory should be deleted",
+      assertTrue("Directory should be deleted",
           !cpuCgroupMountDir.exists());
     }
     FileUtils.deleteQuietly(mountPoint);
-    Assert.assertTrue("cgroups mount point should be deleted",
+    assertTrue("cgroups mount point should be deleted",
         !mountPoint.exists());
     try {
       cGroupsHandler.initializeCGroupController(
           CGroupsHandler.CGroupController.CPU);
       Assert.fail("An inaccessible path should result in an exception");
     } catch (Exception e) {
-      Assert.assertTrue("Unexpected exception " + e.getClass().toString(),
+      assertTrue("Unexpected exception " + e.getClass().toString(),
           e instanceof ResourceHandlerException);
     }
   }
@@ -442,19 +494,19 @@ public class TestCGroupsHandlerImpl {
     File memory = new File(tmpPath, "memory");
     try {
       CGroupsHandlerImpl handler = new CGroupsHandlerImpl(
-          conf,
+          createNoMountConfiguration(tmpPath),
           privilegedOperationExecutorMock);
-      Map<String, List<String>> cgroups = new LinkedHashMap<>();
+      Map<String, Set<String>> cgroups = new LinkedHashMap<>();
 
       Assert.assertTrue("temp dir should be created", cpu.mkdirs());
       Assert.assertTrue("temp dir should be created", memory.mkdirs());
       Assert.assertFalse("temp dir should not be created", 
cpuNoExist.exists());
 
       cgroups.put(
-          memory.getAbsolutePath(), Collections.singletonList("memory"));
+          memory.getAbsolutePath(), Collections.singleton("memory"));
       cgroups.put(
-          cpuNoExist.getAbsolutePath(), Collections.singletonList("cpu"));
-      cgroups.put(cpu.getAbsolutePath(), Collections.singletonList("cpu"));
+          cpuNoExist.getAbsolutePath(), Collections.singleton("cpu"));
+      cgroups.put(cpu.getAbsolutePath(), Collections.singleton("cpu"));
       String selectedCPU = handler.findControllerInMtab("cpu", cgroups);
       Assert.assertEquals("Wrong CPU mount point selected",
           cpu.getAbsolutePath(), selectedCPU);
@@ -464,17 +516,61 @@ public class TestCGroupsHandlerImpl {
     }
   }
 
-  @After
-  public void teardown() {
-    FileUtil.fullyDelete(new File(tmpPath));
+  /**
+   * Tests whether mtab parsing works as expected with an empty hierarchy set.
+   * @throws Exception the test will fail
+   */
+  @Test
+  public void testPreMountedControllerEmpty() throws Exception {
+    testPreMountedControllerInitialization("");
   }
 
-  private class MockSecurityManagerDenyWrite extends SecurityManager {
-    @Override
-    public void checkPermission(Permission perm) {
-      if(perm.getActions().equals("write")) {
-        throw new SecurityException("Mock not allowed");
-      }
-    }
+  /**
+   * Tests whether mtab parsing works as expected with a / hierarchy set.
+   * @throws Exception the test will fail
+   */
+  @Test
+  public void testPreMountedControllerRoot() throws Exception {
+    testPreMountedControllerInitialization("/");
+  }
+
+  /**
+   * Tests whether mtab parsing works as expected with the specified hierarchy.
+   * @throws Exception the test will fail
+   */
+  @Test
+  public void testRemount()
+      throws Exception {
+    // Initialize mount point
+    File parentDir = new File(tmpPath);
+
+    final String oldMountPointDir = "oldmount";
+    final String newMountPointDir = "newmount";
+
+    File oldMountPoint = new File(parentDir, oldMountPointDir);
+    File mtab = createPremountedCgroups(
+        oldMountPoint, true);
+
+    File newMountPoint = new File(parentDir, newMountPointDir);
+    assertTrue("Could not create dirs",
+        new File(newMountPoint, "cpu").mkdirs());
+
+    // Initialize Yarn classes
+    Configuration confMount = createMountConfiguration();
+    confMount.set(YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_MOUNT_PATH,
+        parentDir.getAbsolutePath() + Path.SEPARATOR + newMountPointDir);
+    CGroupsHandlerImpl cGroupsHandler = new CGroupsHandlerImpl(confMount,
+        privilegedOperationExecutorMock, mtab.getAbsolutePath());
+
+    cGroupsHandler.initializeCGroupController(
+        CGroupsHandler.CGroupController.CPU);
+
+    ArgumentCaptor<PrivilegedOperation> opCaptor = ArgumentCaptor.forClass(
+        PrivilegedOperation.class);
+    verify(privilegedOperationExecutorMock)
+        .executePrivilegedOperation(opCaptor.capture(), eq(false));
+    File hierarchyFile =
+        new File(new File(newMountPoint, "cpu"), this.hierarchy);
+    assertTrue("Yarn cgroup should exist", hierarchyFile.exists());
   }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2f68049/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/TestResourceHandlerModule.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/resources/TestResourceHandlerModule.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/resources/TestResourceHandlerModule.java
index 69479d1..9da37dd 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/resources/TestResourceHandlerModule.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/resources/TestResourceHandlerModule.java
@@ -43,9 +43,6 @@ public class TestResourceHandlerModule {
 
     
networkEnabledConf.setBoolean(YarnConfiguration.NM_NETWORK_RESOURCE_ENABLED,
         true);
-    //We need to bypass mtab parsing for figuring out cgroups mount locations
-    networkEnabledConf.setBoolean(YarnConfiguration
-        .NM_LINUX_CONTAINER_CGROUPS_MOUNT, true);
     ResourceHandlerModule.nullifyResourceHandlerChain();
   }
 

http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2f68049/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/util/TestCgroupsLCEResourcesHandler.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/util/TestCgroupsLCEResourcesHandler.java
 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/util/TestCgroupsLCEResourcesHandler.java
index 83c8d5d..b562133 100644
--- 
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/util/TestCgroupsLCEResourcesHandler.java
+++ 
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/util/TestCgroupsLCEResourcesHandler.java
@@ -38,11 +38,12 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Scanner;
+import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 
 @Deprecated
 public class TestCgroupsLCEResourcesHandler {
-  static File cgroupDir = null;
+  private static File cgroupDir = null;
 
   @Before
   public void setUp() throws Exception {
@@ -132,9 +133,9 @@ public class TestCgroupsLCEResourcesHandler {
   static class CustomCgroupsLCEResourceHandler extends
       CgroupsLCEResourcesHandler {
 
-    String mtabFile;
-    int[] limits = new int[2];
-    boolean generateLimitsMode = false;
+    private String mtabFile;
+    private int[] limits = new int[2];
+    private boolean generateLimitsMode = false;
 
     @Override
     int[] getOverallLimits(float x) {
@@ -154,6 +155,20 @@ public class TestCgroupsLCEResourcesHandler {
     }
   }
 
+  private static File createMockCgroupMount(File parentDir,
+                                            String type)
+      throws IOException {
+    File cgroupMountDir =
+        new File(parentDir.getAbsolutePath(), type + "/hadoop-yarn");
+    FileUtils.deleteQuietly(cgroupMountDir);
+    if (!cgroupMountDir.mkdirs()) {
+      String message =
+          "Could not create dir " + cgroupMountDir.getAbsolutePath();
+      throw new IOException(message);
+    }
+    return cgroupMountDir;
+  }
+
   @Test
   public void testInit() throws IOException {
     LinuxContainerExecutor mockLCE = new MockLinuxContainerExecutor();
@@ -168,13 +183,14 @@ public class TestCgroupsLCEResourcesHandler {
     handler.setConf(conf);
     handler.initConfig();
 
+    // create mock mtab
+    File mockMtab =
+        TestCGroupsHandlerImpl.createPremountedCgroups(cgroupDir, false);
+
     // create mock cgroup
-    File cpuCgroupMountDir = TestCGroupsHandlerImpl.createMockCgroupMount(
+    File cpuCgroupMountDir = createMockCgroupMount(
         cgroupDir, "cpu");
 
-    // create mock mtab
-    File mockMtab = TestCGroupsHandlerImpl.createMockMTab(cgroupDir);
-
     // setup our handler and call init()
     handler.setMtabFile(mockMtab.getAbsolutePath());
 
@@ -265,13 +281,14 @@ public class TestCgroupsLCEResourcesHandler {
     handler.setConf(conf);
     handler.initConfig();
 
+    // create mock mtab
+    File mockMtab =
+        TestCGroupsHandlerImpl.createPremountedCgroups(cgroupDir, false);
+
     // create mock cgroup
-    File cpuCgroupMountDir = TestCGroupsHandlerImpl.createMockCgroupMount(
+    File cpuCgroupMountDir = createMockCgroupMount(
         cgroupDir, "cpu");
 
-    // create mock mtab
-    File mockMtab = TestCGroupsHandlerImpl.createMockMTab(cgroupDir);
-
     // setup our handler and call init()
     handler.setMtabFile(mockMtab.getAbsolutePath());
     handler.init(mockLCE, plugin);
@@ -352,17 +369,17 @@ public class TestCgroupsLCEResourcesHandler {
     File memory = new File(cgroupDir, "memory");
     try {
       CgroupsLCEResourcesHandler handler = new CgroupsLCEResourcesHandler();
-      Map<String, List<String>> cgroups = new LinkedHashMap<>();
+      Map<String, Set<String>> cgroups = new LinkedHashMap<>();
 
       Assert.assertTrue("temp dir should be created", cpu.mkdirs());
       Assert.assertTrue("temp dir should be created", memory.mkdirs());
       Assert.assertFalse("temp dir should not be created", 
cpuNoExist.exists());
 
       cgroups.put(
-          memory.getAbsolutePath(), Collections.singletonList("memory"));
+          memory.getAbsolutePath(), Collections.singleton("memory"));
       cgroups.put(
-          cpuNoExist.getAbsolutePath(), Collections.singletonList("cpu"));
-      cgroups.put(cpu.getAbsolutePath(), Collections.singletonList("cpu"));
+          cpuNoExist.getAbsolutePath(), Collections.singleton("cpu"));
+      cgroups.put(cpu.getAbsolutePath(), Collections.singleton("cpu"));
       String selectedCPU = handler.findControllerInMtab("cpu", cgroups);
       Assert.assertEquals("Wrong CPU mount point selected",
           cpu.getAbsolutePath(), selectedCPU);


---------------------------------------------------------------------
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