Skip to content

YARN-11386. Fix issue with classpath resolution #5183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
GauthamBanasandra merged 3 commits into from
Dec 6, 2022
Merged

YARN-11386. Fix issue with classpath resolution #5183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e467176
Fix classpath expansion
GauthamBanasandra Dec 4, 2022
92741d0
Fix unit tests
GauthamBanasandra Dec 4, 2022
ed17d08
Fix Javadoc
GauthamBanasandra Dec 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 35 additions & 29 deletions .../org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ public Integer call() {
launchContext.setCommands(newCmds);

// The actual expansion of environment variables happens after calling
// sanitizeEnv. This allows variables specified in NM_ADMIN_USER_ENV
// addConfigsToEnv. This allows variables specified in NM_ADMIN_USER_ENV
// to reference user or container-defined variables.
Map<String, String> environment = launchContext.getEnvironment();
// /////////////////////////// End of variable expansion
Expand Down Expand Up @@ -340,13 +340,15 @@ public Integer call() {
try (DataOutputStream containerScriptOutStream =
lfs.create(nmPrivateContainerScriptPath,
EnumSet.of(CREATE, OVERWRITE))) {
addConfigsToEnv(environment);

expandAllEnvironmentVars(environment, containerLogDir);

// Sanitize the container's environment
sanitizeEnv(environment, containerWorkDir, appDirs, userLocalDirs,
containerLogDirs, localResources, nmPrivateClasspathJarDir,
nmEnvVars);

expandAllEnvironmentVars(environment, containerLogDir);

// Add these if needed after expanding so we don't expand key values.
if (keystore != null) {
addKeystoreVars(environment, containerWorkDir);
Expand Down Expand Up @@ -1641,13 +1643,35 @@ public void sanitizeEnv(Map<String, String> environment, Path pwd,
addToEnvMap(environment, nmVars, "JVM_PID", "$$");
}

// TODO: Remove Windows check and use this approach on all platforms after
// additional testing. See YARN-358.
if (Shell.WINDOWS) {
sanitizeWindowsEnv(environment, pwd,
resources, nmPrivateClasspathJarDir);
}

// put AuxiliaryService data to environment
for (Map.Entry<String, ByteBuffer> meta : containerManager
.getAuxServiceMetaData().entrySet()) {
AuxiliaryServiceHelper.setServiceDataIntoEnv(
meta.getKey(), meta.getValue(), environment);
nmVars.add(AuxiliaryServiceHelper.getPrefixServiceName(meta.getKey()));
}
}

/**
* There are some configurations (such as {@value YarnConfiguration#NM_ADMIN_USER_ENV}) whose
* values need to be added to the environment variables.
*
* @param environment The environment variables map to add the configuration values to.
*/
public void addConfigsToEnv(Map<String, String> environment) {
// variables here will be forced in, even if the container has
// specified them. Note: we do not track these in nmVars, to
// allow them to be ordered properly if they reference variables
// defined by the user.
String defEnvStr = conf.get(YarnConfiguration.DEFAULT_NM_ADMIN_USER_ENV);
Apps.setEnvFromInputProperty(environment,
YarnConfiguration.NM_ADMIN_USER_ENV, defEnvStr, conf,
Apps.setEnvFromInputProperty(environment, YarnConfiguration.NM_ADMIN_USER_ENV, defEnvStr, conf,
File.pathSeparator);

if (!Shell.WINDOWS) {
Expand All @@ -1658,39 +1682,21 @@ public void sanitizeEnv(Map<String, String> environment, Path pwd,
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);
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);
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) {

sanitizeWindowsEnv(environment, pwd,
resources, nmPrivateClasspathJarDir);
}
// put AuxiliaryService data to environment
for (Map.Entry<String, ByteBuffer> meta : containerManager
.getAuxServiceMetaData().entrySet()) {
AuxiliaryServiceHelper.setServiceDataIntoEnv(
meta.getKey(), meta.getValue(), environment);
nmVars.add(AuxiliaryServiceHelper.getPrefixServiceName(meta.getKey()));
}
}

private void sanitizeWindowsEnv(Map<String, String> environment, Path pwd,
Map<Path, List<String>> resources, Path nmPrivateClasspathJarDir)
throws IOException {

String inputClassPath = environment.get(Environment.CLASSPATH.name());

if (inputClassPath != null && !inputClassPath.isEmpty()) {
Expand Down
3 changes: 3 additions & 0 deletions .../apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,7 @@ public void handle(Event event) {
resources.put(userjar, lpaths);
Path nmp = new Path(testDir);

launch.addConfigsToEnv(userSetEnv);
launch.sanitizeEnv(userSetEnv, pwd, appDirs, userLocalDirs, containerLogs,
resources, nmp, nmEnvTrack);
Assert.assertTrue(userSetEnv.containsKey("MALLOC_ARENA_MAX"));
Expand Down Expand Up @@ -864,6 +865,7 @@ public void handle(Event event) {

ContainerLaunch launch = new ContainerLaunch(distContext, conf,
dispatcher, exec, null, container, dirsHandler, containerManager);
launch.addConfigsToEnv(userSetEnv);
launch.sanitizeEnv(userSetEnv, pwd, appDirs, userLocalDirs, containerLogs,
resources, nmp, nmEnvTrack);

Expand All @@ -876,6 +878,7 @@ public void handle(Event event) {
containerLaunchContext.setEnvironment(userSetEnv);
when(container.getLaunchContext()).thenReturn(containerLaunchContext);

launch.addConfigsToEnv(userSetEnv);
launch.sanitizeEnv(userSetEnv, pwd, appDirs, userLocalDirs, containerLogs,
resources, nmp, nmEnvTrack);

Expand Down