diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java index f4279a3b09e..d9f2ea3d86c 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java @@ -361,7 +361,8 @@ public abstract class ContainerExecutor implements Configurable { if (environment != null) { sb.echo("Setting up env variables"); - for (Map.Entry env : environment.entrySet()) { + for (Map.Entry env : + sb.orderEnvByDependencies(environment).entrySet()) { sb.env(env.getKey(), env.getValue()); } } 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/launcher/ContainerLaunch.java b/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 index 112f54a537a..b89cf6f878f 100644 --- a/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 +++ b/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 @@ -30,11 +30,16 @@ import java.io.PrintStream; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.atomic.AtomicBoolean; @@ -1121,8 +1126,14 @@ public class ContainerLaunch implements Callable { public static abstract class ShellScriptBuilder { public static ShellScriptBuilder create() { - return Shell.WINDOWS ? new WindowsShellScriptBuilder() : - new UnixShellScriptBuilder(); + return create(Shell.osType); + } + + @VisibleForTesting + public static ShellScriptBuilder create(Shell.OSType osType) { + return (osType == Shell.OSType.OS_TYPE_WIN) ? + new WindowsShellScriptBuilder() : + new UnixShellScriptBuilder(); } private static final String LINE_SEPARATOR = @@ -1248,6 +1259,72 @@ public class ContainerLaunch implements Callable { return redirectStdErr; } + /** + * Parse an environment value and returns all environment keys it uses. + * @param envVal an environment variable's value + * @return all environment variable names used in envVal. + */ + public Set getEnvDependencies(final String envVal) { + return Collections.emptySet(); + } + + /** + * Returns a dependency ordered version of envs. Does not alter + * input envs map. + * @param envs environment map + * @return a dependency ordered version of envs + */ + public final Map orderEnvByDependencies( + Map envs) { + if (envs == null || envs.size() < 2) { + return envs; + } + final Map ordered = new LinkedHashMap(); + class Env { + private boolean resolved = false; + private final Collection deps = new ArrayList<>(); + private final String name; + private final String value; + Env(String name, String value) { + this.name = name; + this.value = value; + } + void resolve() { + resolved = true; + for (Env dep : deps) { + if (!dep.resolved) { + dep.resolve(); + } + } + ordered.put(name, value); + } + } + final Map singletons = new HashMap<>(); + for (Map.Entry e : envs.entrySet()) { + Env env = singletons.get(e.getKey()); + if (env == null) { + env = new Env(e.getKey(), e.getValue()); + singletons.put(env.name, env); + } + for (String depStr : getEnvDependencies(env.value)) { + if (!envs.containsKey(depStr)) { + continue; + } + Env depEnv = singletons.get(depStr); + if (depEnv == null) { + depEnv = new Env(depStr, envs.get(depStr)); + singletons.put(depStr, depEnv); + } + env.deps.add(depEnv); + } + } + for (Env env : singletons.values()) { + if (!env.resolved) { + env.resolve(); + } + } + return ordered; + } } private static final class UnixShellScriptBuilder extends ShellScriptBuilder { @@ -1339,6 +1416,79 @@ public class ContainerLaunch implements Callable { public void setExitOnFailure() { line("set -o pipefail -e"); } + + /** + * Parse envVal using bash-like syntax to extract env variables + * it depends on. + */ + @Override + public Set getEnvDependencies(final String envVal) { + if (envVal == null || envVal.isEmpty()) { + return Collections.emptySet(); + } + final Set deps = new HashSet<>(); + // env/whitelistedEnv dump values inside double quotes + boolean inDoubleQuotes = true; + char c; + int i = 0; + final int len = envVal.length(); + while (i < len) { + c = envVal.charAt(i); + if (c == '"') { + inDoubleQuotes = !inDoubleQuotes; + } else if (c == '\'' && !inDoubleQuotes) { + i++; + // eat until closing simple quote + while (i < len) { + c = envVal.charAt(i); + if (c == '\\') { + i++; + } + if (c == '\'') { + break; + } + i++; + } + } else if (c == '\\') { + i++; + } else if (c == '$') { + i++; + if (i >= len) { + break; + } + c = envVal.charAt(i); + if (c == '{') { // for ${... bash like syntax + i++; + if (i >= len) { + break; + } + c = envVal.charAt(i); + if (c == '#') { // for ${#... bash array syntax + i++; + if (i >= len) { + break; + } + } + } + final int start = i; + while (i < len) { + c = envVal.charAt(i); + if (c != '$' && ( + (i == start && Character.isJavaIdentifierStart(c)) || + (i > start && Character.isJavaIdentifierPart(c)))) { + i++; + } else { + break; + } + } + if (i > start) { + deps.add(envVal.substring(start, i)); + } + } + i++; + } + return deps; + } } private static final class WindowsShellScriptBuilder @@ -1420,6 +1570,46 @@ public class ContainerLaunch implements Callable { String.format("@echo \"dir:\" > \"%s\"", output.toString())); lineWithLenCheck(String.format("dir >> \"%s\"", output.toString())); } + + /** + * Parse envVal using cmd/bat-like syntax to extract env + * variables it depends on. + */ + public Set getEnvDependencies(final String envVal) { + if (envVal == null || envVal.isEmpty()) { + return Collections.emptySet(); + } + final Set deps = new HashSet<>(); + final int len = envVal.length(); + int i = 0; + while (i < len) { + i = envVal.indexOf('%', i); // find beginning of variable + if (i < 0 || i == (len - 1)) { + break; + } + i++; + // 3 cases: %var%, %var:...% or %% + final int j = envVal.indexOf('%', i); // find end of variable + if (j == i) { + // %% case, just skip it + i++; + continue; + } + if (j < 0) { + break; // even %var:...% syntax ends with a %, so j cannot be negative + } + final int k = envVal.indexOf(':', i); + if (k >= 0 && k < j) { + // %var:...% syntax + deps.add(envVal.substring(i, k)); + } else { + // %var% syntax + deps.add(envVal.substring(i, j)); + } + i = j + 1; + } + return deps; + } } private static void putEnvIfNotNull( 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/launcher/TestContainerLaunch.java b/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 index 5923f8ef055..b993b6c82d2 100644 --- a/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 +++ b/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 @@ -40,9 +40,12 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.StringTokenizer; import java.util.jar.JarFile; import java.util.jar.Manifest; @@ -1824,4 +1827,338 @@ public class TestContainerLaunch extends BaseContainerManagerTest { } } } + + + private static void assertOrderEnvByDependencies( + final Map env, + final ContainerLaunch.ShellScriptBuilder sb) { + LinkedHashMap copy = new LinkedHashMap<>(); + copy.putAll(env); + Map ordered = sb.orderEnvByDependencies(env); + // 1st, check that env and copy are the same + Assert.assertEquals( + "Input env map has been altered because its size changed", + copy.size(), env.size() + ); + final Iterator> ai = env.entrySet().iterator(); + for (Map.Entry e : copy.entrySet()) { + Map.Entry a = ai.next(); + Assert.assertTrue( + "Keys have been reordered in input env map", + // env must not be altered at all, so we don't use String.equals + // copy and env must use the same String refs + e.getKey() == a.getKey() + ); + Assert.assertTrue( + "Key "+e.getKey()+" does not longer points to its " + +"original value have been reordered in input env map", + // env must be altered at all, so we don't use String.equals + // copy and env must use the same String refs + e.getValue() == a.getValue() + ); + } + // 2nd, check the ordered version as the expected ordering + // and did not altered values + Assert.assertEquals( + "Input env map and ordered env map must have the same size, env="+env+ + ", ordered="+ordered, env.size(), ordered.size() + ); + int iA = -1; + int iB = -1; + int iC = -1; + int iD = -1; + int icA = -1; + int icB = -1; + int icC = -1; + int i=0; + for (Map.Entry e: ordered.entrySet()) { + if ("A".equals(e.getKey())) { + iA = i++; + } else if ("B".equals(e.getKey())) { + iB = i++; + } else if ("C".equals(e.getKey())) { + iC = i++; + } else if ("D".equals(e.getKey())) { + iD = i++; + } else if ("cyclic_A".equals(e.getKey())) { + icA = i++; + } else if ("cyclic_B".equals(e.getKey())) { + icB = i++; + } else if ("cyclic_C".equals(e.getKey())) { + icC = i++; + } else { + Assert.fail("Test need to ne fixed, got an unexpected env entry "+ + e.getKey()); + } + } + // expected order : AA and C>B + // but there is no assertion about C>A because B might be missing in some + // broken envs + Assert.assertTrue("when reordering "+env+" into "+ordered+ + ", B should be after A", iA<0 || iB<0 || iA=0 || + icA<0 || icB<0 || icB=0 || + icB<0 || icC<0 || icC=0 || + icC<0 || icA<0 || icA expected = new HashSet<>(); + final ContainerLaunch.ShellScriptBuilder bash = + ContainerLaunch.ShellScriptBuilder.create(Shell.OSType.OS_TYPE_LINUX); + String s; + + s = null; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = ""; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "A"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "\\$A"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "$$"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "$1"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "handle \"'$A'\" simple quotes"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "$ crash test for StringArrayOutOfBoundException"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "${ crash test for StringArrayOutOfBoundException"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "${# crash test for StringArrayOutOfBoundException"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "crash test for StringArrayOutOfBoundException $"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "crash test for StringArrayOutOfBoundException ${"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "crash test for StringArrayOutOfBoundException ${#"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + + expected.add("A"); + s = "$A"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "${A}"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "${#A[*]}"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "in the $A midlle"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + + expected.add("B"); + s = "${A:-$B} var in var"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "${A}$B var outside var"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + + expected.add("C"); + s = "$A:$B:$C:pathlist var"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + s = "${A}/foo/bar:$B:${C}:pathlist var"; + Assert.assertEquals("failed to parse " + s, expected, + bash.getEnvDependencies(s)); + + ContainerLaunch.ShellScriptBuilder win = + ContainerLaunch.ShellScriptBuilder.create(Shell.OSType.OS_TYPE_WIN); + expected.clear(); + s = null; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = ""; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = "A"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = "%%%%%%"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = "%%A%"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = "%A"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = "%A:"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + + expected.add("A"); + s = "%A%"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = "%%%A%"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = "%%C%A%"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = "%A:~-1%"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = "%A%B%"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = "%A%%%%%B%"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + + expected.add("B"); + s = "%A%%B%"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = "%A%%%%B%"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + + expected.add("C"); + s = "%A%:%B%:%C%:pathlist var"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + s = "%A%\\foo\\bar:%B%:%C%:pathlist var"; + Assert.assertEquals("failed to parse " + s, expected, + win.getEnvDependencies(s)); + } + + private Set asSet(String...str) { + final Set set = new HashSet<>(); + Collections.addAll(set, str); + return set; + } + + @Test(timeout = 5000) + public void testOrderEnvByDependencies() { + final Map> fakeDeps = new HashMap<>(); + fakeDeps.put("Aval", Collections.emptySet()); // A has no dependencies + fakeDeps.put("Bval", asSet("A")); // B depends on A + fakeDeps.put("Cval", asSet("B")); // C depends on B + fakeDeps.put("Dval", asSet("A", "B")); // C depends on B + fakeDeps.put("cyclic_Aval", asSet("cyclic_B")); + fakeDeps.put("cyclic_Bval", asSet("cyclic_C")); + fakeDeps.put("cyclic_Cval", asSet("cyclic_A", "C")); + + final ContainerLaunch.ShellScriptBuilder sb = + new ContainerLaunch.ShellScriptBuilder() { + @Override public Set getEnvDependencies(final String envVal) { + return fakeDeps.get(envVal); + } + @Override protected void mkdir(Path path) throws IOException {} + @Override public void listDebugInformation(Path output) + throws IOException {} + @Override protected void link(Path src, Path dst) + throws IOException {} + @Override public void env(String key, String value) + throws IOException {} + @Override public void copyDebugInformation(Path src, Path dst) + throws IOException {} + @Override public void command(List command) + throws IOException {} + @Override public void setStdOut(Path stdout) + throws IOException {}; + @Override public void setStdErr(Path stdout) + throws IOException {}; + @Override public void echo(String echoStr) + throws IOException {}; + }; + + try { + Assert.assertNull("Ordering a null env map must return a null value.", + sb.orderEnvByDependencies(null)); + } catch (Exception e) { + Assert.fail("null value is to be supported"); + } + + try { + Assert.assertEquals( + "Ordering an empty env map must return an empty map.", + 0, sb.orderEnvByDependencies(Collections.emptyMap()).size() + ); + } catch (Exception e) { + Assert.fail("Empty map is to be supported"); + } + + final Map combination = new LinkedHashMap<>(); + // to test all possible cases, we create all possible combinations and test + // each of them + class TestEnv { + private final String key; + private final String value; + private boolean used=false; + TestEnv(String key, String value) { + this.key = key; + this.value = value; + } + void generateCombinationAndTest(int nbItems, + final ArrayList keylist) { + used = true; + combination.put(key, value); + try { + if (nbItems == 0) { + //LOG.info("Combo : " + combination); + assertOrderEnvByDependencies(combination, sb); + return; + } + for (TestEnv localEnv: keylist) { + if (!localEnv.used) { + localEnv.generateCombinationAndTest(nbItems - 1, keylist); + } + } + } finally { + combination.remove(key); + used=false; + } + } + } + final ArrayList keys = new ArrayList<>(); + for (String key : new String[] {"A", "B", "C", "D", + "cyclic_A", "cyclic_B", "cyclic_C"}) { + keys.add(new TestEnv(key, key+"val")); + } + for (int count=keys.size(); count > 0; count--) { + for (TestEnv env : keys) { + env.generateCombinationAndTest(count, keys); + } + } + } }