[YARN-10607] User environment is unable to prepend PATH when mapreduce.admin.user.env also sets PATH. Contributed by Eric Badger.

Author: jbrennan333

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java

@@ -1245,6 +1245,15 @@ public static boolean isAclEnabled(Configuration conf) {
   public static final String NM_ADMIN_USER_ENV = NM_PREFIX + "admin-env";
   public static final String DEFAULT_NM_ADMIN_USER_ENV = "MALLOC_ARENA_MAX=$MALLOC_ARENA_MAX";
 
+  /**
+   * PATH components that will be prepended to the user's path.
+   * If this is defined and the user does not define PATH, NM will also
+   * append ":$PATH" to prevent this from eclipsing the PATH defined in
+   * the container. This feature is only available for Linux.
+   * */
+  public static final String NM_ADMIN_FORCE_PATH = NM_PREFIX + "force.path";
+  public static final String DEFAULT_NM_ADMIN_FORCE_PATH = "";
+
   /** Environment variables that containers may override rather than use NodeManager's default.*/
   public static final String NM_ENV_WHITELIST = NM_PREFIX + "env-whitelist";
   public static final String DEFAULT_NM_ENV_WHITELIST = StringUtils.join(",",

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml

@@ -1195,6 +1195,17 @@
     <value>MALLOC_ARENA_MAX=$MALLOC_ARENA_MAX</value>
   </property>
 
+  <property>
+    <description>
+      * PATH components that will be prepended to the user's path.
+      * If this is defined and the user does not define PATH, NM will also
+      * append ":$PATH" to prevent this from eclipsing the PATH defined in
+      * the container. This feature is only available for Linux.
+    </description>
+    <name>yarn.nodemanager.force.path</name>
+    <value></value>
+  </property>
+
   <property>
     <description>Environment variables that containers may override rather than use NodeManager's default.</description>
     <name>yarn.nodemanager.env-whitelist</name>

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java

@@ -1636,6 +1636,27 @@ public void sanitizeEnv(Map<String, String> environment, Path pwd,
     nmVars.addAll(Apps.getEnvVarsFromInputProperty(
         YarnConfiguration.NM_ADMIN_USER_ENV, defEnvStr, conf));
 
+    if (!Shell.WINDOWS) {
+      // maybe force path components
+      String forcePath = conf.get(YarnConfiguration.NM_ADMIN_FORCE_PATH,
+          YarnConfiguration.DEFAULT_NM_ADMIN_FORCE_PATH);
+      if (!forcePath.isEmpty()) {
+        String userPath = environment.get(Environment.PATH.name());
+        environment.remove(Environment.PATH.name());
+        if (userPath == null || userPath.isEmpty()) {
+          Apps.addToEnvironment(environment, Environment.PATH.name(),
+              forcePath, File.pathSeparator);
+          Apps.addToEnvironment(environment, Environment.PATH.name(),
+              "$PATH", File.pathSeparator);
+        } else {
+          Apps.addToEnvironment(environment, Environment.PATH.name(),
+              forcePath, File.pathSeparator);
+          Apps.addToEnvironment(environment, Environment.PATH.name(),
+              userPath, File.pathSeparator);
+        }
+      }
+    }
+
     // TODO: Remove Windows check and use this approach on all platforms after
     // additional testing.  See YARN-358.
     if (Shell.WINDOWS) {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java

@@ -823,6 +823,69 @@ public void handle(Event event) {
     Assert.assertEquals(testVal3, userSetEnv.get(testKey3));
   }
 
+  @Test
+  public void testNmForcePath() throws Exception {
+    // Valid only for unix
+    assumeNotWindows();
+    ContainerLaunchContext containerLaunchContext =
+        recordFactory.newRecordInstance(ContainerLaunchContext.class);
+    ApplicationId appId = ApplicationId.newInstance(0, 0);
+    ApplicationAttemptId appAttemptId =
+        ApplicationAttemptId.newInstance(appId, 1);
+    ContainerId cId = ContainerId.newContainerId(appAttemptId, 0);
+    Map<String, String> userSetEnv = new HashMap<>();
+    Set<String> nmEnvTrack = new LinkedHashSet<>();
+    containerLaunchContext.setEnvironment(userSetEnv);
+    Container container = mock(Container.class);
+    when(container.getContainerId()).thenReturn(cId);
+    when(container.getLaunchContext()).thenReturn(containerLaunchContext);
+    when(container.getLocalizedResources()).thenReturn(null);
+    Dispatcher dispatcher = mock(Dispatcher.class);
+    EventHandler<Event> eventHandler = new EventHandler<Event>() {
+      public void handle(Event event) {
+        Assert.assertTrue(event instanceof ContainerExitEvent);
+        ContainerExitEvent exitEvent = (ContainerExitEvent) event;
+        Assert.assertEquals(ContainerEventType.CONTAINER_EXITED_WITH_FAILURE,
+            exitEvent.getType());
+      }
+    };
+    when(dispatcher.getEventHandler()).thenReturn(eventHandler);
+
+    String testDir = System.getProperty("test.build.data",
+        "target/test-dir");
+    Path pwd = new Path(testDir);
+    List<Path> appDirs = new ArrayList<>();
+    List<String> userLocalDirs = new ArrayList<>();
+    List<String> containerLogs = new ArrayList<>();
+    Map<Path, List<String>> resources = new HashMap<>();
+    Path nmp = new Path(testDir);
+
+    YarnConfiguration conf = new YarnConfiguration();
+    String forcePath = "./force-path";
+    conf.set("yarn.nodemanager.force.path", forcePath);
+
+    ContainerLaunch launch = new ContainerLaunch(distContext, conf,
+        dispatcher, exec, null, container, dirsHandler, containerManager);
+    launch.sanitizeEnv(userSetEnv, pwd, appDirs, userLocalDirs, containerLogs,
+        resources, nmp, nmEnvTrack);
+
+    Assert.assertTrue(userSetEnv.containsKey(Environment.PATH.name()));
+    Assert.assertEquals(forcePath + ":$PATH",
+        userSetEnv.get(Environment.PATH.name()));
+
+    String userPath = "/usr/bin:/usr/local/bin";
+    userSetEnv.put(Environment.PATH.name(), userPath);
+    containerLaunchContext.setEnvironment(userSetEnv);
+    when(container.getLaunchContext()).thenReturn(containerLaunchContext);
+
+    launch.sanitizeEnv(userSetEnv, pwd, appDirs, userLocalDirs, containerLogs,
+        resources, nmp, nmEnvTrack);
+
+    Assert.assertTrue(userSetEnv.containsKey(Environment.PATH.name()));
+    Assert.assertEquals(forcePath + ":" + userPath,
+        userSetEnv.get(Environment.PATH.name()));
+  }
+
   @Test
   public void testErrorLogOnContainerExit() throws Exception {
     verifyTailErrorLogOnContainerExit(new Configuration(), "/stderr", false);