MAPREDUCE-6494. Permission issue when running archive-logs tool as different users (rkanter)

hadoop-mapreduce-project/CHANGES.txt

@@ -585,6 +585,9 @@ Release 2.8.0 - UNRELEASED
 
    MAPREDUCE-6480. archive-logs tool may miss applications (rkanter)
 
+   MAPREDUCE-6494. Permission issue when running archive-logs tool as
+   different users (rkanter)
+
 Release 2.7.2 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-tools/hadoop-archive-logs/src/main/java/org/apache/hadoop/tools/HadoopArchiveLogs.java

@@ -76,6 +76,7 @@ public class HadoopArchiveLogs implements Tool {
   private static final String MAX_TOTAL_LOGS_SIZE_OPTION = "maxTotalLogsSize";
   private static final String MEMORY_OPTION = "memory";
   private static final String VERBOSE_OPTION = "verbose";
+  private static final String FORCE_OPTION = "force";
 
   private static final int DEFAULT_MAX_ELIGIBLE = -1;
   private static final int DEFAULT_MIN_NUM_LOG_FILES = 20;
@@ -91,6 +92,8 @@ public class HadoopArchiveLogs implements Tool {
   @VisibleForTesting
   long memory = DEFAULT_MEMORY;
   private boolean verbose = false;
+  @VisibleForTesting
+  boolean force = false;
 
   @VisibleForTesting
   Set<AppInfo> eligibleApplications;
@@ -126,6 +129,8 @@ public static void main(String[] args) {
 
   @Override
   public int run(String[] args) throws Exception {
+    int exitCode = 1;
+
     handleOpts(args);
 
     FileSystem fs = null;
@@ -141,20 +146,9 @@ public int run(String[] args) throws Exception {
     }
     try {
       fs = FileSystem.get(conf);
-      checkFilesAndSeedApps(fs, remoteRootLogDir, suffix);
+      if (prepareWorkingDir(fs, workingDir)) {
 
-      // Prepare working directory
+        checkFilesAndSeedApps(fs, remoteRootLogDir, suffix);
-      if (fs.exists(workingDir)) {
-        fs.delete(workingDir, true);
-      }
-      fs.mkdirs(workingDir);
-      fs.setPermission(workingDir,
-          new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.NONE));
-    } finally {
-      if (fs != null) {
-        fs.close();
-      }
-    }
 
         filterAppsByAggregatedStatus();
 
@@ -162,9 +156,8 @@ public int run(String[] args) throws Exception {
 
         if (eligibleApplications.isEmpty()) {
           LOG.info("No eligible applications to process");
-      System.exit(0);
+          exitCode = 0;
-    }
+        } else {
-
           StringBuilder sb =
               new StringBuilder("Will process the following applications:");
           for (AppInfo app : eligibleApplications) {
@@ -175,10 +168,19 @@ public int run(String[] args) throws Exception {
           File localScript = File.createTempFile("hadoop-archive-logs-", ".sh");
           generateScript(localScript, workingDir, remoteRootLogDir, suffix);
 
-    if (runDistributedShell(localScript)) {
+          exitCode = runDistributedShell(localScript) ? 0 : 1;
-      return 0;
         }
-    return -1;
+      }
+    } finally {
+      if (fs != null) {
+        // Cleanup working directory
+        if (fs.exists(workingDir)) {
+          fs.delete(workingDir, true);
+        }
+        fs.close();
+      }
+    }
+    return exitCode;
   }
 
   private void handleOpts(String[] args) throws ParseException {
@@ -202,12 +204,17 @@ private void handleOpts(String[] args) throws ParseException {
     memoryOpt.setArgName("megabytes");
     Option verboseOpt = new Option(VERBOSE_OPTION, false,
         "Print more details.");
+    Option forceOpt = new Option(FORCE_OPTION, false,
+        "Force recreating the working directory if an existing one is found. " +
+            "This should only be used if you know that another instance is " +
+            "not currently running");
     opts.addOption(helpOpt);
     opts.addOption(maxEligibleOpt);
     opts.addOption(minNumLogFilesOpt);
     opts.addOption(maxTotalLogsSizeOpt);
     opts.addOption(memoryOpt);
     opts.addOption(verboseOpt);
+    opts.addOption(forceOpt);
 
     try {
       CommandLineParser parser = new GnuParser();
@@ -242,6 +249,9 @@ private void handleOpts(String[] args) throws ParseException {
       if (commandLine.hasOption(VERBOSE_OPTION)) {
         verbose = true;
       }
+      if (commandLine.hasOption(FORCE_OPTION)) {
+        force = true;
+      }
     } catch (ParseException pe) {
       HelpFormatter formatter = new HelpFormatter();
       formatter.printHelp("yarn archive-logs", opts);
@@ -249,6 +259,25 @@ private void handleOpts(String[] args) throws ParseException {
     }
   }
 
+  @VisibleForTesting
+  boolean prepareWorkingDir(FileSystem fs, Path workingDir) throws IOException {
+    if (fs.exists(workingDir)) {
+      if (force) {
+        LOG.info("Existing Working Dir detected: -" + FORCE_OPTION +
+            " specified -> recreating Working Dir");
+        fs.delete(workingDir, true);
+      } else {
+        LOG.info("Existing Working Dir detected: -" + FORCE_OPTION +
+            " not specified -> exiting");
+        return false;
+      }
+    }
+    fs.mkdirs(workingDir);
+    fs.setPermission(workingDir,
+        new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.NONE));
+    return true;
+  }
+
   @VisibleForTesting
   void filterAppsByAggregatedStatus() throws IOException, YarnException {
     YarnClient client = YarnClient.createYarnClient();

hadoop-tools/hadoop-archive-logs/src/test/java/org/apache/hadoop/tools/TestHadoopArchiveLogs.java

@@ -23,6 +23,8 @@
 import org.apache.hadoop.fs.FSDataOutputStream;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.permission.FsAction;
+import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.yarn.api.records.ApplicationId;
 import org.apache.hadoop.yarn.api.records.ApplicationReport;
 import org.apache.hadoop.yarn.api.records.ApplicationSubmissionContext;
@@ -309,6 +311,44 @@ public void testStatuses() throws Exception {
     Assert.assertArrayEquals(statuses, LogAggregationStatus.values());
   }
 
+  @Test(timeout = 5000)
+  public void testPrepareWorkingDir() throws Exception {
+    Configuration conf = new Configuration();
+    HadoopArchiveLogs hal = new HadoopArchiveLogs(conf);
+    FileSystem fs = FileSystem.getLocal(conf);
+    Path workingDir = new Path("target", "testPrepareWorkingDir");
+    fs.delete(workingDir, true);
+    Assert.assertFalse(fs.exists(workingDir));
+    // -force is false and the dir doesn't exist so it will create one
+    hal.force = false;
+    boolean dirPrepared = hal.prepareWorkingDir(fs, workingDir);
+    Assert.assertTrue(dirPrepared);
+    Assert.assertTrue(fs.exists(workingDir));
+    Assert.assertEquals(
+        new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.NONE),
+        fs.getFileStatus(workingDir).getPermission());
+    // Throw a file in the dir
+    Path dummyFile = new Path(workingDir, "dummy.txt");
+    fs.createNewFile(dummyFile);
+    Assert.assertTrue(fs.exists(dummyFile));
+    // -force is false and the dir exists, so nothing will happen and the dummy
+    // still exists
+    dirPrepared = hal.prepareWorkingDir(fs, workingDir);
+    Assert.assertFalse(dirPrepared);
+    Assert.assertTrue(fs.exists(workingDir));
+    Assert.assertTrue(fs.exists(dummyFile));
+    // -force is true and the dir exists, so it will recreate it and the dummy
+    // won't exist anymore
+    hal.force = true;
+    dirPrepared = hal.prepareWorkingDir(fs, workingDir);
+    Assert.assertTrue(dirPrepared);
+    Assert.assertTrue(fs.exists(workingDir));
+    Assert.assertEquals(
+        new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.NONE),
+        fs.getFileStatus(workingDir).getPermission());
+    Assert.assertFalse(fs.exists(dummyFile));
+  }
+
   private static void createFile(FileSystem fs, Path p, long sizeMultiple)
       throws IOException {
     FSDataOutputStream out = null;