Repository: oozie
Updated Branches:
  refs/heads/master 901616b21 -> 7770d1e83


OOZIE-2504 Create a log4j.properties under HADOOP_CONF_DIR in Shell Action 
(harsh)


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

Branch: refs/heads/master
Commit: 7770d1e831b023cd98c3f64f542aff19d82834fa
Parents: 901616b
Author: Harsh J <[email protected]>
Authored: Tue Jun 14 10:44:20 2016 +0530
Committer: Harsh J <[email protected]>
Committed: Tue Jun 14 10:44:20 2016 +0530

----------------------------------------------------------------------
 .../action/hadoop/ShellActionExecutor.java      | 10 +++
 core/src/main/resources/oozie-default.xml       | 30 +++++++++
 .../action/hadoop/TestShellActionExecutor.java  | 67 ++++++++++++++++++--
 .../site/twiki/DG_ShellActionExtension.twiki    | 10 +++
 release-log.txt                                 |  1 +
 .../apache/oozie/action/hadoop/ShellMain.java   | 39 +++++++++++-
 6 files changed, 150 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/oozie/blob/7770d1e8/core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java 
b/core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java
index 4fdd3ff..b9ffa7a 100644
--- a/core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java
+++ b/core/src/main/java/org/apache/oozie/action/hadoop/ShellActionExecutor.java
@@ -74,6 +74,16 @@ public class ShellActionExecutor extends JavaActionExecutor {
         boolean setupHadoopConfDir = 
actionConf.getBoolean(ShellMain.CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR,
                 
ConfigurationService.getBoolean(ShellMain.CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR));
         
actionConf.setBoolean(ShellMain.CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR, 
setupHadoopConfDir);
+        // Setting to control if ShellMain should write log4j.properties
+        boolean writeL4J = 
actionConf.getBoolean(ShellMain.CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR_WRITE_LOG4J_PROPERTIES,
+                
ConfigurationService.getBoolean(ShellMain.CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR_WRITE_LOG4J_PROPERTIES));
+        
actionConf.setBoolean(ShellMain.CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR_WRITE_LOG4J_PROPERTIES,
 writeL4J);
+        // Setting of content of log4j.properties, if to be written
+        if (writeL4J) {
+            String l4jContent = 
actionConf.get(ShellMain.CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR_LOG4J_CONTENT,
+                    
ConfigurationService.get(ShellMain.CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR_LOG4J_CONTENT));
+            
actionConf.set(ShellMain.CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR_LOG4J_CONTENT, 
l4jContent);
+        }
 
         return actionConf;
     }

http://git-wip-us.apache.org/repos/asf/oozie/blob/7770d1e8/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 6c2f7d8..13984f9 100644
--- a/core/src/main/resources/oozie-default.xml
+++ b/core/src/main/resources/oozie-default.xml
@@ -1821,6 +1821,36 @@ will be the requeue interval for the actions which are 
waiting for a long time w
     </property>
 
     <property>
+        
<name>oozie.action.shell.setup.hadoop.conf.dir.write.log4j.properties</name>
+        <value>true</value>
+        <description>
+            Toggle to control if a log4j.properties file should be written 
into the configuration directory prepared when
+            oozie.action.shell.setup.hadoop.conf.dir is enabled. This is used 
to control logging behavior of log4j using commands
+            run within the shell action script, and to ensure logging does not 
impact output data capture if leaked to stdout.
+            Content of the written file is determined by the value of 
oozie.action.shell.setup.hadoop.conf.dir.log4j.content.
+        </description>
+    </property>
+
+    <property>
+        <name>oozie.action.shell.setup.hadoop.conf.dir.log4j.content</name>
+        <value>
+            log4j.rootLogger=${hadoop.root.logger}
+            hadoop.root.logger=INFO,console
+            log4j.appender.console=org.apache.log4j.ConsoleAppender
+            log4j.appender.console.target=System.err
+            log4j.appender.console.layout=org.apache.log4j.PatternLayout
+            log4j.appender.console.layout.ConversionPattern=%d{yy/MM/dd 
HH:mm:ss} %p %c{2}: %m%n
+        </value>
+        <description>
+            The value to write into a log4j.properties file under the config 
directory created when
+            oozie.action.shell.setup.hadoop.conf.dir and 
oozie.action.shell.setup.hadoop.conf.dir.write.log4j.properties
+            properties are both enabled. The values must be properly newline 
separated and in format expected by Log4J.
+            Trailing and preceding whitespaces will be trimmed when reading 
this property.
+            This is used to control logging behavior of log4j using commands 
run within the shell action script.
+        </description>
+    </property>
+
+    <property>
         <name>oozie.action.launcher.yarn.timeline-service.enabled</name>
         <value>false</value>
         <description>

http://git-wip-us.apache.org/repos/asf/oozie/blob/7770d1e8/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java
----------------------------------------------------------------------
diff --git 
a/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java
 
b/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java
index 6a962a1..1531ed9 100644
--- 
a/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java
+++ 
b/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java
@@ -63,8 +63,15 @@ public class TestShellActionExecutor extends 
ActionExecutorTestCase {
             ? "echo OOZIE_ACTION_CONF_XML=%OOZIE_ACTION_CONF_XML%\necho 
HADOOP_CONF_DIR=%HADOOP_CONF_DIR%\n"
             : "echo OOZIE_ACTION_CONF_XML=$OOZIE_ACTION_CONF_XML\necho 
HADOOP_CONF_DIR=$HADOOP_CONF_DIR\n";
     private static final String SHELL_SCRIPT_YARN_CONF_DIR_CONTENT = 
Shell.WINDOWS
-            ? "echo OOZIE_ACTION_CONF_XML=%OOZIE_ACTION_CONF_XML%\necho 
YARN_CONF_DIR=%YARN_CONF_DIR%\n"
-            : "echo OOZIE_ACTION_CONF_XML=$OOZIE_ACTION_CONF_XML\necho 
YARN_CONF_DIR=$YARN_CONF_DIR\n";
+            ? "echo YARN_CONF_DIR=%YARN_CONF_DIR%\n"
+            : "echo YARN_CONF_DIR=$YARN_CONF_DIR\n";
+    private static final String SHELL_SCRIPT_LOG4J_EXISTENCE_CHECKER = 
Shell.WINDOWS
+            ? "IF EXIST %HADOOP_CONF_DIR%\\log4j.properties echo 
L4J_EXISTS=yes\n"
+            : "if [ -f $HADOOP_CONF_DIR/log4j.properties ]; then echo 
L4J_EXISTS=yes; fi\n";
+    private static final String SHELL_SCRIPT_LOG4J_CONTENT_COUNTER = 
Shell.WINDOWS
+            ? "SET COMMAND=\"type %HADOOP_CONF_DIR%\\log4j.properties | FIND 
/c /v \"~DOESNOTMATCH~\"\"\n" +
+              "FOR /f %%i IN (' %COMMAND% ') DO SET L4J_LC=%%i\necho 
L4J_WC=%L4J_LC%\n"
+            : "echo L4J_LC=$(cat $HADOOP_CONF_DIR/log4j.properties | wc -l)\n";
 
     /**
      * Verify if the ShellActionExecutor indeed setups the basic stuffs
@@ -94,7 +101,12 @@ public class TestShellActionExecutor extends 
ActionExecutorTestCase {
         assertEquals("2", conf.get("oozie.shell.args.size"));
         assertEquals("a=A", conf.get("oozie.shell.args.0"));
         assertEquals("b=B", conf.get("oozie.shell.args.1"));
-        assertEquals("false", 
conf.get("oozie.action.shell.setup.hadoop.conf.dir"));
+        assertEquals("Expected HADOOP_CONF_DIR setup switch to be disabled",
+                "false", conf.get("oozie.action.shell.setup.hadoop.conf.dir"));
+        assertEquals("Expected log4j.properties write switch to be enabled",
+                "true", 
conf.get("oozie.action.shell.setup.hadoop.conf.dir.write.log4j.properties"));
+        assertNotNull("Expected a default config to exist for 
log4j.properties",
+                
conf.get("oozie.action.shell.setup.hadoop.conf.dir.log4j.content"));
     }
 
     /**
@@ -121,7 +133,7 @@ public class TestShellActionExecutor extends 
ActionExecutorTestCase {
     }
 
     /**
-     * test if a sample shell script could run successfully
+     * Test if a shell script could run successfully with {@link 
ShellMain#CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR} enabled.
      *
      * @throws Exception
      */
@@ -132,6 +144,8 @@ public class TestShellActionExecutor extends 
ActionExecutorTestCase {
         Writer w = new OutputStreamWriter(fs.create(script));
         w.write(SHELL_SCRIPT_HADOOP_CONF_DIR_CONTENT);
         w.write(SHELL_SCRIPT_YARN_CONF_DIR_CONTENT);
+        w.write(SHELL_SCRIPT_LOG4J_EXISTENCE_CHECKER);
+        w.write(SHELL_SCRIPT_LOG4J_CONTENT_COUNTER);
         w.close();
 
         // Create sample Shell action xml
@@ -146,11 +160,52 @@ public class TestShellActionExecutor extends 
ActionExecutorTestCase {
         String oozieActionConfXml = 
PropertiesUtils.stringToProperties(action.getData()).getProperty("OOZIE_ACTION_CONF_XML");
         String hadoopConfDir = 
PropertiesUtils.stringToProperties(action.getData()).getProperty("HADOOP_CONF_DIR");
         String yarnConfDir = 
PropertiesUtils.stringToProperties(action.getData()).getProperty("YARN_CONF_DIR");
+        String log4jExists = 
PropertiesUtils.stringToProperties(action.getData()).getProperty("L4J_EXISTS");
+        String log4jFileLineCount = 
PropertiesUtils.stringToProperties(action.getData()).getProperty("L4J_LC");
         assertNotNull(oozieActionConfXml);
         assertNotNull(hadoopConfDir);
         String s = new File(oozieActionConfXml).getParent() + File.separator + 
"oozie-hadoop-conf-";
-        Assert.assertTrue("Expected HADOOP_CONF_DIR to start with " + s + " 
but was " + hadoopConfDir, hadoopConfDir.startsWith(s));
-        Assert.assertTrue("Expected YARN_CONF_DIR to start with " + s + " but 
was " + yarnConfDir, yarnConfDir.startsWith(s));
+        Assert.assertTrue(
+                "Expected HADOOP_CONF_DIR to start with " + s + " but was " + 
hadoopConfDir,
+                hadoopConfDir.startsWith(s));
+        Assert.assertTrue(
+                "Expected YARN_CONF_DIR to start with " + s + " but was " + 
yarnConfDir,
+                yarnConfDir.startsWith(s));
+        Assert.assertEquals(
+                "Expected log4j.properties file to exist", log4jExists, "yes");
+        Assert.assertTrue(
+                "Expected log4j.properties to have non-zero line count, but 
has: " + log4jFileLineCount,
+                Integer.parseInt(log4jFileLineCount) > 0);
+    }
+
+    /**
+     * Test if a shell script could run successfully with {@link 
ShellMain#CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR} enabled.
+     * Run with {@link 
ShellMain#CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR_WRITE_LOG4J_PROPERTIES} 
disabled.
+     *
+     * @throws Exception
+     */
+    public void testShellScriptHadoopConfDirWithNoL4J() throws Exception {
+        FileSystem fs = getFileSystem();
+        // Create the script file with canned shell command
+        Path script = new Path(getAppPath(), SHELL_SCRIPTNAME);
+        Writer w = new OutputStreamWriter(fs.create(script));
+        w.write(SHELL_SCRIPT_LOG4J_EXISTENCE_CHECKER);
+        w.close();
+
+        // Create sample Shell action xml
+        String actionXml = "<shell>" + "<job-tracker>" + getJobTrackerUri() + 
"</job-tracker>" + "<name-node>"
+                + getNameNodeUri() + "</name-node>" + "<configuration>"
+                + 
"<property><name>oozie.action.shell.setup.hadoop.conf.dir</name><value>true</value></property>"
+                + 
"<property><name>oozie.action.shell.setup.hadoop.conf.dir.write.log4j.properties"
+                + "</name><value>false</value></property>"
+                + "</configuration>" + "<exec>" + SHELL_EXEC + "</exec>" + 
"<argument>" + SHELL_PARAM + "</argument>"
+                + "<argument>" + SHELL_SCRIPTNAME + "</argument>" + "<file>" + 
script.toString()
+                + "#" + script.getName() + "</file>" + "<capture-output/>" + 
"</shell>";
+        // Submit and verify the job's status
+        WorkflowAction action = _testSubmit(actionXml, true, "");
+        String log4jExists = 
PropertiesUtils.stringToProperties(action.getData()).getProperty("L4J_EXISTS");
+        Assert.assertNull(
+                "Expected no log4j.properties file to exist", log4jExists);
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/oozie/blob/7770d1e8/docs/src/site/twiki/DG_ShellActionExtension.twiki
----------------------------------------------------------------------
diff --git a/docs/src/site/twiki/DG_ShellActionExtension.twiki 
b/docs/src/site/twiki/DG_ShellActionExtension.twiki
index 025a94d..5a5759a 100644
--- a/docs/src/site/twiki/DG_ShellActionExtension.twiki
+++ b/docs/src/site/twiki/DG_ShellActionExtension.twiki
@@ -222,6 +222,16 @@ queueName=default
 ---+++ Shell Action Configuration
 
 =oozie.action.shell.setup.hadoop.conf.dir= - Generates a config directory with 
various core/hdfs/yarn/mapred-site.xml files and points =HADOOP_CONF_DIR= and 
=YARN_CONF_DIR= env-vars to it, before the Script is invoked. XML is sourced 
from the action configuration. Useful when the Shell script passed uses various 
=hadoop= commands. Default is false.
+=oozie.action.shell.setup.hadoop.conf.dir.write.log4j.properties= - When 
=oozie.action.shell.setup.hadoop.conf.dir= is enabled, toggle if a 
log4j.properties file should also be written under the configuration files 
directory. Default is true.
+=oozie.action.shell.setup.hadoop.conf.dir.log4j.content= - When 
=oozie.action.shell.setup.hadoop.conf.dir.write.log4j.properties= is enabled, 
the content to write into the log4j.properties file under the configuration 
files directory. Default is a simple console based stderr logger, as presented 
below:
+<verbatim>
+log4j.rootLogger=${hadoop.root.logger}
+hadoop.root.logger=INFO,console
+log4j.appender.console=org.apache.log4j.ConsoleAppender
+log4j.appender.console.target=System.err
+log4j.appender.console.layout=org.apache.log4j.PatternLayout
+log4j.appender.console.layout.ConversionPattern=%d{yy/MM/dd HH:mm:ss} %p 
%c{2}: %m%n
+</verbatim>
 
 ---+++ Shell Action Logging
 

http://git-wip-us.apache.org/repos/asf/oozie/blob/7770d1e8/release-log.txt
----------------------------------------------------------------------
diff --git a/release-log.txt b/release-log.txt
index fba302b..b585a27 100644
--- a/release-log.txt
+++ b/release-log.txt
@@ -1,5 +1,6 @@
 -- Oozie 4.3.0 release (trunk - unreleased)
 
+OOZIE-2504 Create a log4j.properties under HADOOP_CONF_DIR in Shell Action 
(harsh)
 OOZIE-2567 HCat connection is not closed while getting hcat cred (puru)
 OOZIE-2547 Add mapreduce.job.cache.files to spark action (satishsaley via 
rohini)
 OOZIE-2550 Flaky tests in TestZKUUIDService.java (pbacsko via rkanter)

http://git-wip-us.apache.org/repos/asf/oozie/blob/7770d1e8/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java
----------------------------------------------------------------------
diff --git 
a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java 
b/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java
index e5d8eef..c4a6e9b 100644
--- a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java
+++ b/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java
@@ -22,7 +22,10 @@ package org.apache.oozie.action.hadoop;
 import java.io.BufferedReader;
 import java.io.BufferedWriter;
 import java.io.File;
+import java.io.FileOutputStream;
 import java.io.FileWriter;
+import java.io.PrintWriter;
+import java.io.StringReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.util.ArrayList;
@@ -39,11 +42,16 @@ public class ShellMain extends LauncherMain {
     public static final String CONF_OOZIE_SHELL_ENVS = "oozie.shell.envs";
     public static final String CONF_OOZIE_SHELL_CAPTURE_OUTPUT = 
"oozie.shell.capture-output";
     public static final String CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR = 
"oozie.action.shell.setup.hadoop.conf.dir";
+    public static final String 
CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR_WRITE_LOG4J_PROPERTIES =
+            "oozie.action.shell.setup.hadoop.conf.dir.write.log4j.properties";
+    public static final String 
CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR_LOG4J_CONTENT =
+            "oozie.action.shell.setup.hadoop.conf.dir.log4j.content";
     public static final String OOZIE_ACTION_CONF_XML = "OOZIE_ACTION_CONF_XML";
     private static final String HADOOP_CONF_DIR = "HADOOP_CONF_DIR";
     private static final String YARN_CONF_DIR = "YARN_CONF_DIR";
 
     private static String[] HADOOP_SITE_FILES = new String[] {"core-site.xml", 
"hdfs-site.xml", "mapred-site.xml", "yarn-site.xml"};
+    private static String LOG4J_PROPERTIES = "log4j.properties";
 
     /**
      * @param args Invoked from LauncherMapper:map()
@@ -121,7 +129,7 @@ public class ShellMain extends LauncherMain {
      * HADOOP/YARN_CONF_DIR to point there.  This should allow most Hadoop 
ecosystem CLI programs to have the proper configuration,
      * propagated from Oozie's copy and including anything set in the 
Workflow's configuration section as well.  Otherwise,
      * HADOOP/YARN_CONF_DIR points to the NodeManager's *-site files, which 
are likely not suitable for client programs.
-     * It will only do this if {@link CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR} 
is set to true.
+     * It will only do this if {@link #CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR} 
is set to true.
      *
      * @param actionConf The action configuration
      * @param envp The environment for the Shell process
@@ -141,6 +149,10 @@ public class ShellMain extends LauncherMain {
                     dstFiles[i] = new File(confDir, HADOOP_SITE_FILES[i]);
                 }
                 copyFileMultiplex(actionXmlFile, dstFiles);
+                if 
(actionConf.getBoolean(CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR_WRITE_LOG4J_PROPERTIES,
 true)) {
+                    System.out.println("Writing " + LOG4J_PROPERTIES + " to " 
+ confDir);
+                    writeLoggerProperties(actionConf, confDir);
+                }
                 System.out.println("Setting " + HADOOP_CONF_DIR + " and " + 
YARN_CONF_DIR
                     + " to " + confDir.getAbsolutePath());
                 envp.put(HADOOP_CONF_DIR, confDir.getAbsolutePath());
@@ -150,6 +162,31 @@ public class ShellMain extends LauncherMain {
     }
 
     /**
+     * Write a {@link #LOG4J_PROPERTIES} file into the provided directory.
+     * Content of the log4j.properties sourced from the default value of 
property
+     * {@link #CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR_LOG4J_CONTENT}, defined 
in oozie-default.xml.
+     * This is required for properly redirecting command outputs to stderr.
+     * Otherwise, logging from commands may go into stdout and make it to the 
Shell's captured output.
+     * @param actionConf the action's configuration, to source log4j contents 
from
+     * @param confDir the directory to write a {@link #LOG4J_PROPERTIES} file 
under
+     */
+    private static void writeLoggerProperties(Configuration actionConf, File 
confDir) throws IOException {
+        String log4jContents = actionConf.get(
+                CONF_OOZIE_SHELL_SETUP_HADOOP_CONF_DIR_LOG4J_CONTENT);
+        File log4jPropertiesFile = new File(confDir, LOG4J_PROPERTIES);
+        FileOutputStream log4jFileOutputStream = new 
FileOutputStream(log4jPropertiesFile, false);
+        PrintWriter log4jWriter = new PrintWriter(log4jFileOutputStream);
+        BufferedReader lineReader = new BufferedReader(new 
StringReader(log4jContents));
+        String line = lineReader.readLine();
+        while (line != null) {
+            // Trim the line (both preceding and trailing whitespaces) before 
writing it as a line in file
+            log4jWriter.println(line.trim());
+            line = lineReader.readLine();
+        }
+        log4jWriter.close();
+    }
+
+    /**
      * Return the environment variable to pass to in shell command execution.
      *
      */

Reply via email to