MAPREDUCE-6110. JobHistoryServer CLI throws NullPointerException with job ids that do not exist. (Kai Sasaki via gtcarrera9)

hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/tools/CLI.java

@@ -402,8 +402,13 @@ public class CLI extends Configured implements Tool {
           }
         }
       } else if (listEvents) {
-        listEvents(getJob(JobID.forName(jobid)), fromEvent, nEvents);
+        Job job = getJob(JobID.forName(jobid));
+        if (job == null) {
+          System.out.println("Could not find job " + jobid);
+        } else {
+          listEvents(job, fromEvent, nEvents);
           exitCode = 0;
+        }
       } else if (listJobs) {
         listJobs(cluster);
         exitCode = 0;
@@ -417,8 +422,13 @@ public class CLI extends Configured implements Tool {
         listBlacklistedTrackers(cluster);
         exitCode = 0;
       } else if (displayTasks) {
+        Job job = getJob(JobID.forName(jobid));
+        if (job == null) {
+          System.out.println("Could not find job " + jobid);
+        } else {
           displayTasks(getJob(JobID.forName(jobid)), taskType, taskState);
           exitCode = 0;
+        }
       } else if(killTask) {
         TaskAttemptID taskID = TaskAttemptID.forName(taskid);
         Job job = getJob(taskID.getJobID());
@@ -444,15 +454,18 @@ public class CLI extends Configured implements Tool {
           exitCode = -1;
         }
       } else if (logs) {
-        try {
         JobID jobID = JobID.forName(jobid);
+        if (getJob(jobID) == null) {
+          System.out.println("Could not find job " + jobid);
+        } else {
+          try {
             TaskAttemptID taskAttemptID = TaskAttemptID.forName(taskid);
             LogParams logParams = cluster.getLogParams(jobID, taskAttemptID);
             LogCLIHelpers logDumper = new LogCLIHelpers();
             logDumper.setConf(getConf());
-        exitCode = logDumper.dumpAContainersLogs(logParams.getApplicationId(),
+            exitCode = logDumper.dumpAContainersLogs(
-            logParams.getContainerId(), logParams.getNodeId(),
+                    logParams.getApplicationId(), logParams.getContainerId(),
-            logParams.getOwner());
+                    logParams.getNodeId(), logParams.getOwner());
           } catch (IOException e) {
             if (e instanceof RemoteException) {
               throw e;
@@ -460,6 +473,7 @@ public class CLI extends Configured implements Tool {
             System.out.println(e.getMessage());
           }
         }
+      }
     } catch (RemoteException re) {
       IOException unwrappedException = re.unwrapRemoteException();
       if (unwrappedException instanceof AccessControlException) {

hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/tools/TestCLI.java

@@ -87,7 +87,7 @@ public class TestCLI {
     JobID jobId = JobID.forName(jobIdStr);
     Cluster mockCluster = mock(Cluster.class);
     Job job = mock(Job.class);
-    CLI cli = spy(new CLI());
+    CLI cli = spy(new CLI(new Configuration()));
 
     doReturn(mockCluster).when(cli).createCluster();
     when(mockCluster.getJob(jobId)).thenReturn(job);
@@ -101,12 +101,18 @@ public class TestCLI {
     int retCode_invalidTaskState = cli.run(new String[] { "-list-attempt-ids",
         jobIdStr, "REDUCE", "complete" });
 
+    String jobIdStr2 = "job_1015298225799_0016";
+    int retCode_invalidJobId = cli.run(new String[] { "-list-attempt-ids",
+        jobIdStr2, "MAP", "running" });
+
     assertEquals("JOB_SETUP is an invalid input,exit code should be -1", -1,
         retCode_JOB_SETUP);
     assertEquals("JOB_CLEANUP is an invalid input,exit code should be -1", -1,
         retCode_JOB_CLEANUP);
     assertEquals("complete is an invalid input,exit code should be -1", -1,
         retCode_invalidTaskState);
+    assertEquals("Non existing job id should be skippted with -1", -1,
+        retCode_invalidJobId);
 
   }
 
@@ -176,4 +182,34 @@ public class TestCLI {
       Assert.assertTrue(end - start < ((i + 1) * sleepTime));
     }
   }
+
+  @Test
+  public void testListEvents() throws Exception {
+    Cluster mockCluster = mock(Cluster.class);
+    CLI cli = spy(new CLI(new Configuration()));
+    doReturn(mockCluster).when(cli).createCluster();
+    String jobId1 = "job_1234654654_001";
+    String jobId2 = "job_1234654656_002";
+
+    Job mockJob1 = mockJob(mockCluster, jobId1, State.RUNNING);
+
+    // Check exiting with non existing job
+    int exitCode = cli.run(new String[]{"-events", jobId2, "0", "10"});
+    assertEquals(-1, exitCode);
+  }
+
+  @Test
+  public void testLogs() throws Exception {
+    Cluster mockCluster = mock(Cluster.class);
+    CLI cli = spy(new CLI(new Configuration()));
+    doReturn(mockCluster).when(cli).createCluster();
+    String jobId1 = "job_1234654654_001";
+    String jobId2 = "job_1234654656_002";
+
+    Job mockJob1 = mockJob(mockCluster, jobId1, State.SUCCEEDED);
+
+    // Check exiting with non existing job
+    int exitCode = cli.run(new String[]{"-logs", jobId2});
+    assertEquals(-1, exitCode);
+  }
 }