OOZIE-2872 Address backward compatibility issue introduced by OOZIE-2748 (pbacsko)
(cherry picked from commit e0b7cde711b1b9e1a03660ec635041eeb9755049) core/src/main/resources/oozie-default.xml Project: http://git-wip-us.apache.org/repos/asf/oozie/repo Commit: http://git-wip-us.apache.org/repos/asf/oozie/commit/7ea8f19a Tree: http://git-wip-us.apache.org/repos/asf/oozie/tree/7ea8f19a Diff: http://git-wip-us.apache.org/repos/asf/oozie/diff/7ea8f19a Branch: refs/heads/branch-4.3 Commit: 7ea8f19a756fd8775671bd9bd71b210666547e6d Parents: f553c4e Author: Peter Bacsko <pbac...@cloudera.com> Authored: Mon May 15 12:50:28 2017 +0200 Committer: satishsaley <satishsa...@apache.org> Committed: Fri Dec 8 16:34:55 2017 -0800 ---------------------------------------------------------------------- .../oozie/action/hadoop/JavaActionExecutor.java | 6 ++++ core/src/main/resources/oozie-default.xml | 8 +++++ .../action/hadoop/TestJavaActionExecutor.java | 17 +++++++-- release-log.txt | 1 + .../oozie/action/hadoop/LauncherMapper.java | 35 ++++++++++++------ .../oozie/action/hadoop/TestLauncherMapper.java | 37 +++++++++++++++++--- 6 files changed, 87 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/oozie/blob/7ea8f19a/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java b/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java index 8a403dc..50577fd 100644 --- a/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java +++ b/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java @@ -993,6 +993,12 @@ public class JavaActionExecutor extends ActionExecutor { args[i] = list.get(i).getTextTrim(); } LauncherMapperHelper.setupMainArguments(launcherJobConf, args); + // backward compatibility flag - see OOZIE-2872 + if (ConfigurationService.getBoolean(LauncherMapper.CONF_OOZIE_NULL_ARGS_ALLOWED)) { + launcherJobConf.setBoolean(LauncherMapper.CONF_OOZIE_NULL_ARGS_ALLOWED, true); + } else { + launcherJobConf.setBoolean(LauncherMapper.CONF_OOZIE_NULL_ARGS_ALLOWED, false); + } // Make mapred.child.java.opts and mapreduce.map.java.opts equal, but give values from the latter priority; also append // <java-opt> and <java-opts> and give those highest priority http://git-wip-us.apache.org/repos/asf/oozie/blob/7ea8f19a/core/src/main/resources/oozie-default.xml ---------------------------------------------------------------------- diff --git a/core/src/main/resources/oozie-default.xml b/core/src/main/resources/oozie-default.xml index 8c55a0d..dbab18b 100644 --- a/core/src/main/resources/oozie-default.xml +++ b/core/src/main/resources/oozie-default.xml @@ -2850,4 +2850,12 @@ will be the requeue interval for the actions which are waiting for a long time w </description> </property> + <property> + <name>oozie.actions.null.args.allowed</name> + <value>true</value> + <description> + When set to true, empty arguments (like <arg></arg>) will be passed as "null" to the main method of a + given action. That is, the args[] array will contain "null" elements. When set to false, then "nulls" are removed. + </description> + </property> </configuration> http://git-wip-us.apache.org/repos/asf/oozie/blob/7ea8f19a/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java b/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java index 1c4b429..a6e1d50 100644 --- a/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java +++ b/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java @@ -2941,7 +2941,18 @@ public class TestJavaActionExecutor extends ActionExecutorTestCase { assertEquals("DEBUG", conf.get(oozieActionHiveRootLogger)); } - public void testEmptyArgs() throws Exception { + public void testEmptyArgsWithNullArgsNotAllowed() throws Exception { + testEmptyArgs(false, "SUCCEEDED", WorkflowAction.Status.OK); + } + + public void testEmptyArgsWithNullArgsAllowed() throws Exception { + testEmptyArgs(true, "FAILED/KILLED", WorkflowAction.Status.ERROR); + } + + private void testEmptyArgs(boolean nullArgsAllowed, String expectedExternalStatus, WorkflowAction.Status expectedStatus) + throws Exception { + ConfigurationService.setBoolean(LauncherMapper.CONF_OOZIE_NULL_ARGS_ALLOWED, nullArgsAllowed); + String actionXml = "<java>" + "<job-tracker>" + getJobTrackerUri() + "</job-tracker>" + "<name-node>" + getNameNodeUri() + "</name-node>" + @@ -2961,10 +2972,10 @@ public class TestJavaActionExecutor extends ActionExecutorTestCase { ActionExecutor ae = new JavaActionExecutor(); ae.check(context, context.getAction()); assertTrue(ae.isCompleted(context.getAction().getExternalStatus())); - assertEquals("SUCCEEDED", context.getAction().getExternalStatus()); + assertEquals(expectedExternalStatus, context.getAction().getExternalStatus()); assertNull(context.getAction().getData()); ae.end(context, context.getAction()); - assertEquals(WorkflowAction.Status.OK, context.getAction().getStatus()); + assertEquals(expectedStatus, context.getAction().getStatus()); } } http://git-wip-us.apache.org/repos/asf/oozie/blob/7ea8f19a/release-log.txt ---------------------------------------------------------------------- diff --git a/release-log.txt b/release-log.txt index 2b36ec0..a79d8c2 100644 --- a/release-log.txt +++ b/release-log.txt @@ -1,5 +1,6 @@ -- Oozie 4.3.1 release +OOZIE-2872 Address backward compatibility issue introduced by OOZIE-2748 (pbacsko) OOZIE-2863 SLACalculatorMemory.loadOnRestart causing delay in server start (puru via satishsaley) OOZIE-2862 Coord change command doesn't change job to running if job was killed without creating any actions (puru) OOZIE-2811 amend Add support for filtering out properties from SparkConfigurationService http://git-wip-us.apache.org/repos/asf/oozie/blob/7ea8f19a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java ---------------------------------------------------------------------- diff --git a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java b/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java index 8edebac..8657c67 100644 --- a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java +++ b/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java @@ -63,6 +63,7 @@ public class LauncherMapper<K1, V1, K2, V2> implements Mapper<K1, V1, K2, V2>, R static final String CONF_OOZIE_EXTERNAL_STATS_MAX_SIZE = "oozie.external.stats.max.size"; static final String OOZIE_ACTION_CONFIG_CLASS = ACTION_PREFIX + "config.class"; static final String CONF_OOZIE_ACTION_FS_GLOB_MAX = ACTION_PREFIX + "fs.glob.max"; + static final String CONF_OOZIE_NULL_ARGS_ALLOWED = ACTION_PREFIX + "null.args.allowed"; static final String COUNTER_GROUP = "oozie.launcher"; static final String COUNTER_LAUNCHER_ERROR = "oozie.launcher.error"; @@ -497,18 +498,28 @@ public class LauncherMapper<K1, V1, K2, V2> implements Mapper<K1, V1, K2, V2>, R public static String[] getMainArguments(Configuration conf) { String[] args = new String[conf.getInt(CONF_OOZIE_ACTION_MAIN_ARG_COUNT, 0)]; - int pos = 0; - for (int i = 0; i < args.length; i++) { - String arg = conf.get(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + i); - if (!Strings.isNullOrEmpty(arg)) { - args[pos++] = conf.get(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + i); + String[] retArray; + + if (conf.getBoolean(CONF_OOZIE_NULL_ARGS_ALLOWED, true)) { + for (int i = 0; i < args.length; i++) { + args[i] = conf.get(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + i); + } + + retArray = args; + } else { + int pos = 0; + for (int i = 0; i < args.length; i++) { + String arg = conf.get(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + i); + if (!Strings.isNullOrEmpty(arg)) { + args[pos++] = conf.get(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + i); + } } - } - // this is to skip null args, that is <arg></arg> in the workflow XML -- in this case, - // args[] might look like {"arg1", "arg2", null, null} at this point - String[] retArray = new String[pos]; - System.arraycopy(args, 0, retArray, 0, pos); + // this is to skip null args, that is <arg></arg> in the workflow XML -- in this case, + // args[] might look like {"arg1", "arg2", null, null} at this point + retArray = new String[pos]; + System.arraycopy(args, 0, retArray, 0, pos); + } return retArray; } @@ -632,6 +643,10 @@ public class LauncherMapper<K1, V1, K2, V2> implements Mapper<K1, V1, K2, V2>, R System.out.println(banner); boolean maskNextArg = false; for (String arg : args) { + if (arg == null) { + arg = "null"; // prevent NPE in pwd masking + } + if (maskNextArg) { System.out.println(" " + "********"); maskNextArg = false; http://git-wip-us.apache.org/repos/asf/oozie/blob/7ea8f19a/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherMapper.java ---------------------------------------------------------------------- diff --git a/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherMapper.java b/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherMapper.java index 1dd8002..51b1d6f 100644 --- a/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherMapper.java +++ b/sharelib/oozie/src/test/java/org/apache/oozie/action/hadoop/TestLauncherMapper.java @@ -23,6 +23,7 @@ import static org.apache.oozie.action.hadoop.LauncherMapper.CONF_OOZIE_ACTION_MA import static org.junit.Assert.assertTrue; import static org.mockito.BDDMockito.given; import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.anyBoolean; import java.util.Arrays; import java.util.List; @@ -41,8 +42,9 @@ public class TestLauncherMapper { private Configuration conf; // we have to use mock, because conf.set(null) throws exception @Test - public void testLauncherMapperArgsHandlingWithoutNulls() { + public void testArgsHandlingWithoutNullsAndNullsNotAllowed() { setupConf(Lists.newArrayList("a", "b", "c")); + setEnableNullArgsAllowed(false); String args[] = LauncherMapper.getMainArguments(conf); @@ -50,8 +52,9 @@ public class TestLauncherMapper { } @Test - public void testLauncherMapperArgsHandlingWhenArgsContainNulls() { + public void testHandlingWhenArgsContainNullsAndNullsNotAllowed() { setupConf(Lists.newArrayList("a", null, "b", null, "c")); + setEnableNullArgsAllowed(false); String args[] = LauncherMapper.getMainArguments(conf); @@ -59,8 +62,9 @@ public class TestLauncherMapper { } @Test - public void testLauncherMapperArgsHandlingWhenArgsContainsNullsOnly() { + public void testArgsHandlingWhenArgsContainsNullsOnlyAndNullsNotAllowed() { setupConf(Lists.<String>newArrayList(null, null, null)); + setEnableNullArgsAllowed(false); String args[] = LauncherMapper.getMainArguments(conf); @@ -68,14 +72,35 @@ public class TestLauncherMapper { } @Test - public void testLauncherMapperArgsHandlingWhenArgsContainsOneNull() { + public void testArgsHandlingWhenArgsContainsOneNullAndNullsNotAllowed() { setupConf(Lists.<String>newArrayList((String) null)); + setEnableNullArgsAllowed(false); String args[] = LauncherMapper.getMainArguments(conf); assertTrue(Arrays.equals(new String[] {}, args)); } + @Test + public void testHandlingWhenArgsContainNullsAndNullAllowed() { + setupConf(Lists.newArrayList("a", null, "b", null, "c")); + setEnableNullArgsAllowed(true); + + String args[] = LauncherMapper.getMainArguments(conf); + + assertTrue(Arrays.equals(new String[] { "a", null, "b", null, "c"}, args)); + } + + @Test + public void testArgsHandlingWhenArgsContainsOneNullAndNullsAllowed() { + setupConf(Lists.<String>newArrayList((String) null)); + setEnableNullArgsAllowed(true); + + String args[] = LauncherMapper.getMainArguments(conf); + + assertTrue(Arrays.equals(new String[] { null }, args)); + } + private void setupConf(List<String> argList) { int argCount = argList.size(); @@ -85,4 +110,8 @@ public class TestLauncherMapper { given(conf.get(eq(CONF_OOZIE_ACTION_MAIN_ARG_PREFIX + i))).willReturn(argList.get(i)); } } + + private void setEnableNullArgsAllowed(boolean nullArgsAllowed) { + given(conf.getBoolean(eq(LauncherMapper.CONF_OOZIE_NULL_ARGS_ALLOWED), anyBoolean())).willReturn(nullArgsAllowed); + } }