You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@oozie.apache.org by pb...@apache.org on 2017/05/08 10:40:08 UTC

[02/37] oozie git commit: OOZIE-2816 Strip out the first command word from Sqoop action if its "sqoop" (harsh)

OOZIE-2816 Strip out the first command word from Sqoop action if its "sqoop" (harsh)


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

Branch: refs/heads/oya
Commit: 2453e6c7fade5396ec22945c8325e11a660936c4
Parents: 59ff7ff
Author: Harsh J <ha...@cloudera.com>
Authored: Thu Mar 9 18:30:17 2017 +0530
Committer: Harsh J <ha...@cloudera.com>
Committed: Tue Mar 21 22:08:38 2017 +0530

----------------------------------------------------------------------
 .../action/hadoop/SqoopActionExecutor.java      |  29 +++--
 release-log.txt                                 |   1 +
 .../action/hadoop/TestSqoopActionExecutor.java  | 114 +++++++++++++++++--
 3 files changed, 124 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/oozie/blob/2453e6c7/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java b/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
index 6cee32a..22e2874 100644
--- a/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
+++ b/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
@@ -44,9 +44,10 @@ public class SqoopActionExecutor extends JavaActionExecutor {
   public static final String OOZIE_ACTION_EXTERNAL_STATS_WRITE = "oozie.action.external.stats.write";
   private static final String SQOOP_MAIN_CLASS_NAME = "org.apache.oozie.action.hadoop.SqoopMain";
   static final String SQOOP_ARGS = "oozie.sqoop.args";
+  private static final String SQOOP = "sqoop";
 
     public SqoopActionExecutor() {
-        super("sqoop");
+        super(SQOOP);
     }
 
     @Override
@@ -85,25 +86,35 @@ public class SqoopActionExecutor extends JavaActionExecutor {
             throw convertException(ex);
         }
 
-        String[] args;
+        final List<String> argList = new ArrayList<>();
+        // Build a list of arguments from either a tokenized <command> string or a list of <arg>
         if (actionXml.getChild("command", ns) != null) {
             String command = actionXml.getChild("command", ns).getTextTrim();
             StringTokenizer st = new StringTokenizer(command, " ");
-            List<String> l = new ArrayList<String>();
             while (st.hasMoreTokens()) {
-                l.add(st.nextToken());
+                argList.add(st.nextToken());
             }
-            args = l.toArray(new String[l.size()]);
         }
         else {
             List<Element> eArgs = (List<Element>) actionXml.getChildren("arg", ns);
-            args = new String[eArgs.size()];
-            for (int i = 0; i < eArgs.size(); i++) {
-                args[i] = eArgs.get(i).getTextTrim();
+            for (Element elem : eArgs) {
+                argList.add(elem.getTextTrim());
             }
         }
+        // If the command is given accidentally as "sqoop import --option"
+        // instead of "import --option" we can make a user's life easier
+        // by removing away the unnecessary "sqoop" token.
+        // However, we do not do this if the command looks like
+        // "sqoop --option", as that's entirely invalid.
+        if (argList.size() > 1 &&
+                argList.get(0).equalsIgnoreCase(SQOOP) &&
+                !argList.get(1).startsWith("-")) {
+            XLog.getLog(getClass()).info(
+                    "Found a redundant 'sqoop' prefixing the command. Removing it.");
+            argList.remove(0);
+        }
 
-        setSqoopCommand(actionConf, args);
+        setSqoopCommand(actionConf, argList.toArray(new String[argList.size()]));
         return actionConf;
     }
 

http://git-wip-us.apache.org/repos/asf/oozie/blob/2453e6c7/release-log.txt
----------------------------------------------------------------------
diff --git a/release-log.txt b/release-log.txt
index f461753..ef3c33d 100644
--- a/release-log.txt
+++ b/release-log.txt
@@ -1,5 +1,6 @@
 -- Oozie 4.4.0 release (trunk - unreleased)
 
+OOZIE-2816 Strip out the first command word from Sqoop action if its "sqoop" (harsh)
 OOZIE-2813 Remove tabs and trailing whitespaces from oozie-defaul.xml (gezapeti)
 OOZIE-2830 Use tarLongFileMode with 'gnu' in the assembly plugin's configuration (asasvari via gezapeti)
 OOZIE-2819 Make Oozie REST API accept multibyte characters for script Actions (asasvari via rkanter)

http://git-wip-us.apache.org/repos/asf/oozie/blob/2453e6c7/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
----------------------------------------------------------------------
diff --git a/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java b/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
index 166d939..3dfd606 100644
--- a/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
+++ b/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
@@ -52,11 +52,11 @@ import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
-import java.util.Properties;
 
 public class TestSqoopActionExecutor extends ActionExecutorTestCase {
 
-    private static final String SQOOP_COMMAND = "import --connect {0} --table TT --target-dir {1} -m 1";
+    private static final String SQOOP_IMPORT_COMMAND =
+            "import --connect {0} --table TT --target-dir {1} -m 1";
 
     private static final String SQOOP_ACTION_COMMAND_XML =
             "<sqoop xmlns=\"uri:oozie:sqoop-action:0.1\">" +
@@ -81,18 +81,18 @@ public class TestSqoopActionExecutor extends ActionExecutorTestCase {
             "<value>INFO</value>" +
             "</property>" +
             "</configuration>" +
-            "<arg>import</arg>" +
+            "{2}" +
             "<arg>--connect</arg>" +
-            "<arg>{2}</arg>" +
+            "<arg>{3}</arg>" +
             "<arg>--username</arg>" +
             "<arg>sa</arg>" +
             "<arg>--password</arg>" +
             "<arg></arg>" +
             "<arg>--verbose</arg>" +
             "<arg>--query</arg>" +
-            "<arg>{3}</arg>" +
-            "<arg>--target-dir</arg>" +
             "<arg>{4}</arg>" +
+            "<arg>--target-dir</arg>" +
+            "<arg>{5}</arg>" +
             "<arg>--split-by</arg>" +
             "<arg>I</arg>" +
             "</sqoop>";
@@ -146,23 +146,45 @@ public class TestSqoopActionExecutor extends ActionExecutorTestCase {
     }
 
     private String getActionXml() {
-        String command = MessageFormat.format(SQOOP_COMMAND, getActionJdbcUri(), getSqoopOutputDir());
+        String command = MessageFormat.format(SQOOP_IMPORT_COMMAND, getActionJdbcUri(), getSqoopOutputDir());
         return MessageFormat.format(SQOOP_ACTION_COMMAND_XML, getJobTrackerUri(), getNameNodeUri(),
                                     "dummy", "dummyValue", command);
     }
 
+    private String getRedundantCommandActionXml() {
+        String command = "sqoop " +
+                MessageFormat.format(SQOOP_IMPORT_COMMAND, getActionJdbcUri(), getSqoopOutputDir());
+        return MessageFormat.format(SQOOP_ACTION_COMMAND_XML, getJobTrackerUri(), getNameNodeUri(),
+                "dummy", "dummyValue", command);
+    }
+
+    private String getBadCommandActionXml() {
+        String command = MessageFormat.format(SQOOP_IMPORT_COMMAND,
+                getActionJdbcUri(), getSqoopOutputDir()).replace("import", "sqoop");
+        return MessageFormat.format(SQOOP_ACTION_COMMAND_XML, getJobTrackerUri(), getNameNodeUri(),
+                "dummy", "dummyValue", command);
+    }
+
     private String getActionXmlEval() {
       String query = "select TT.I, TT.S from TT";
       return MessageFormat.format(SQOOP_ACTION_EVAL_XML, getJobTrackerUri(), getNameNodeUri(),
         getActionJdbcUri(), query);
     }
 
-    private String getActionXmlFreeFromQuery() {
+    private String getArgsActionXmlFreeFromQuery(boolean redundant) {
         String query = "select TT.I, TT.S from TT where $CONDITIONS";
         return MessageFormat.format(SQOOP_ACTION_ARGS_XML, getJobTrackerUri(), getNameNodeUri(),
+                                    (redundant ? "<arg>sqoop</arg>" : "") + "<arg>import</arg>",
                                     getActionJdbcUri(), query, getSqoopOutputDir());
     }
 
+    private String getBadArgsActionXml() {
+        String query = "select TT.I, TT.S from TT where $CONDITIONS";
+        return MessageFormat.format(SQOOP_ACTION_ARGS_XML, getJobTrackerUri(), getNameNodeUri(),
+                "<arg>sqoop</arg>",
+                getActionJdbcUri(), query, getSqoopOutputDir());
+    }
+
     private void createDB() throws Exception {
         Class.forName("org.hsqldb.jdbcDriver");
         Connection conn = DriverManager.getConnection(getLocalJdbcUri(), "sa", "");
@@ -175,10 +197,57 @@ public class TestSqoopActionExecutor extends ActionExecutorTestCase {
         conn.close();
     }
 
+    /**
+     * Tests a bad command of 'sqoop --username ...' style.
+     * Test asserts that the job will fail.
+     */
+    public void testSqoopActionWithBadCommand() throws Exception {
+        runSqoopActionWithBadCommand(getBadCommandActionXml());
+    }
+
+    private void runSqoopActionWithBadCommand(String actionXml) throws Exception {
+        createDB();
+
+        Context context = createContext(actionXml);
+        final RunningJob launcherJob = submitAction(context);
+        String launcherId = context.getAction().getExternalId();
+        waitFor(120 * 1000, new Predicate() {
+            public boolean evaluate() throws Exception {
+                return launcherJob.isComplete();
+            }
+        });
+        assertTrue(launcherJob.isSuccessful());
+        Map<String, String> actionData = LauncherMapperHelper.getActionData(getFileSystem(), context.getActionDir(),
+                context.getProtoActionConf());
+        assertFalse(LauncherMapperHelper.hasIdSwap(actionData));
+
+        SqoopActionExecutor ae = new SqoopActionExecutor();
+        ae.check(context, context.getAction());
+        assertTrue(launcherId.equals(context.getAction().getExternalId()));
+        assertEquals("FAILED/KILLED", context.getAction().getExternalStatus());
+        ae.end(context, context.getAction());
+        assertEquals(WorkflowAction.Status.ERROR, context.getAction().getStatus());
+    }
+
+    /**
+     * Tests a normal command of 'import --username ...'.
+     */
     public void testSqoopAction() throws Exception {
+        runSqoopAction(getActionXml());
+    }
+
+    /**
+     * Tests a redundant command of 'sqoop import --username ...'.
+     * The test guarantees a success, since the redundant 'sqoop' must get removed.
+     */
+    public void testSqoopActionWithRedundantPrefix() throws Exception {
+        runSqoopAction(getRedundantCommandActionXml());
+    }
+
+    private void runSqoopAction(String actionXml) throws Exception {
         createDB();
 
-        Context context = createContext(getActionXml());
+        Context context = createContext(actionXml);
         final RunningJob launcherJob = submitAction(context);
         String launcherId = context.getAction().getExternalId();
         waitFor(120 * 1000, new Predicate() {
@@ -245,10 +314,33 @@ public class TestSqoopActionExecutor extends ActionExecutorTestCase {
         assertTrue(hadoopCounters.isEmpty());
     }
 
-    public void testSqoopActionFreeFormQuery() throws Exception {
+    /**
+     * Runs a job with arg-style command of 'sqoop --username ...' form that's invalid.
+     * The test ensures it fails.
+     */
+    public void testSqoopActionWithBadRedundantArgsAndFreeFormQuery() throws Exception {
+        runSqoopActionWithBadCommand(getBadArgsActionXml());
+    }
+
+    /**
+     * Runs a job with the arg-style command of 'sqoop import --username ...'.
+     * The test guarantees that the redundant 'sqoop' is auto-removed (job passes).
+     */
+    public void testSqoopActionWithRedundantArgsAndFreeFormQuery() throws Exception {
+        runSqoopActionFreeFormQuery(getArgsActionXmlFreeFromQuery(true));
+    }
+
+    /**
+     * Runs a job with the normal arg-style command of 'import --username ...'.
+     */
+    public void testSqoopActionWithArgsAndFreeFormQuery() throws Exception {
+        runSqoopActionFreeFormQuery(getArgsActionXmlFreeFromQuery(false));
+    }
+
+    private void runSqoopActionFreeFormQuery(String actionXml) throws Exception {
         createDB();
 
-        Context context = createContext(getActionXmlFreeFromQuery());
+        Context context = createContext(actionXml);
         final RunningJob launcherJob = submitAction(context);
         String launcherId = context.getAction().getExternalId();
         waitFor(120 * 1000, new Predicate() {