YARN-5266. Wrong exit code while trying to get app logs using regex via CLI. Contributed by Xuan Gong

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/LogsCLI.java

@@ -371,7 +371,7 @@ private List<String> getContainerLogFiles(Configuration conf,
 
   @Private
   @VisibleForTesting
-  public void printContainerLogsFromRunningApplication(Configuration conf,
+  public int printContainerLogsFromRunningApplication(Configuration conf,
       ContainerLogsRequest request, LogCLIHelpers logCliHelper)
       throws IOException {
     String containerIdStr = request.getContainerId().toString();
@@ -385,10 +385,12 @@ public void printContainerLogsFromRunningApplication(Configuration conf,
       // filter the log files based on the given --logFiles pattern
       List<String> allLogs=
           getContainerLogFiles(getConf(), containerIdStr, nodeHttpAddress);
-      List<String> matchedFiles = getMatchedLogFiles(
+      List<String> matchedFiles = getMatchedLogFiles(request, allLogs);
-          request, allLogs, true);
       if (matchedFiles.isEmpty()) {
-        return;
+        System.err.println("Can not find any log file matching the pattern: "
+            + request.getLogTypes() + " for the container: " + containerIdStr
+            + " within the application: " + request.getAppId());
+        return -1;
       }
       ContainerLogsRequest newOptions = new ContainerLogsRequest(request);
       newOptions.setLogTypes(matchedFiles);
@@ -398,7 +400,7 @@ public void printContainerLogsFromRunningApplication(Configuration conf,
           + nodeId;
       out.println(containerString);
       out.println(StringUtils.repeat("=", containerString.length()));
-
+      boolean foundAnyLogs = false;
       for (String logFile : newOptions.getLogTypes()) {
         out.println("LogType:" + logFile);
         out.println("Log Upload Time:"
@@ -418,6 +420,7 @@ public void printContainerLogsFromRunningApplication(Configuration conf,
               + " to a running container (" + containerIdStr + ") and so may"
               + " not be complete.");
           out.flush();
+          foundAnyLogs = true;
         } catch (ClientHandlerException | UniformInterfaceException ex) {
           System.err.println("Can not find the log file:" + logFile
               + " for the container:" + containerIdStr + " in NodeManager:"
@@ -425,7 +428,13 @@ public void printContainerLogsFromRunningApplication(Configuration conf,
         }
       }
       // for the case, we have already uploaded partial logs in HDFS
-      logCliHelper.dumpAContainersLogsForALogType(newOptions, false);
+      int result = logCliHelper.dumpAContainerLogsForLogType(
+          newOptions, false);
+      if (result == 0 || foundAnyLogs) {
+        return 0;
+      } else {
+        return -1;
+      }
     } finally {
       logCliHelper.closePrintStream(out);
     }
@@ -437,9 +446,13 @@ private int printContainerLogsForFinishedApplication(
     ContainerLogsRequest newOptions = getMatchedLogOptions(
         request, logCliHelper);
     if (newOptions == null) {
+      System.err.println("Can not find any log file matching the pattern: "
+          + request.getLogTypes() + " for the container: "
+          + request.getContainerId() + " within the application: "
+          + request.getAppId());
       return -1;
     }
-    return logCliHelper.dumpAContainersLogsForALogType(newOptions);
+    return logCliHelper.dumpAContainerLogsForLogType(newOptions);
   }
 
   private int printContainerLogsForFinishedApplicationWithoutNodeId(
@@ -448,9 +461,13 @@ private int printContainerLogsForFinishedApplicationWithoutNodeId(
     ContainerLogsRequest newOptions = getMatchedLogOptions(
         request, logCliHelper);
     if (newOptions == null) {
+      System.err.println("Can not find any log file matching the pattern: "
+          + request.getLogTypes() + " for the container: "
+          + request.getContainerId() + " within the application: "
+          + request.getAppId());
       return -1;
     }
-    return logCliHelper.dumpAContainersLogsForALogTypeWithoutNodeId(
+    return logCliHelper.dumpAContainerLogsForLogTypeWithoutNodeId(
         newOptions);
   }
 
@@ -635,7 +652,7 @@ private Options createCommandOpts() {
     amOption.setArgName("AM Containers");
     opts.addOption(amOption);
     Option logFileOpt = new Option(CONTAINER_LOG_FILES, true,
-        "Work with -am/-containerId and specify comma-separated value "
+        "Specify comma-separated value "
         + "to get specified container log files. Use \"ALL\" to fetch all the "
         + "log files for the container. It also supports Java Regex.");
     logFileOpt.setValueSeparator(',');
@@ -813,7 +830,7 @@ private int fetchContainerLogs(ContainerLogsRequest request,
         logFiles = Arrays.asList("syslog");
       }
       request.setLogTypes(logFiles);
-      printContainerLogsFromRunningApplication(getConf(), request,
+      resultCode = printContainerLogsFromRunningApplication(getConf(), request,
           logCliHelper);
     } else {
       // If the application is in the final state, we will directly
@@ -831,12 +848,14 @@ private int fetchApplicationLogs(ContainerLogsRequest options,
     // If the application is still running, we would get the full
     // list of the containers first, then fetch the logs for each
     // container from NM.
-    int resultCode = 0;
+    int resultCode = -1;
     if (options.isAppFinished()) {
       ContainerLogsRequest newOptions = getMatchedLogOptions(
           options, logCliHelper);
       if (newOptions == null) {
-        resultCode = -1;
+        System.err.println("Can not find any log file matching the pattern: "
+            + options.getLogTypes() + " for the application: "
+            + options.getAppId());
       } else {
         resultCode =
             logCliHelper.dumpAllContainersLogs(newOptions);
@@ -845,8 +864,11 @@ private int fetchApplicationLogs(ContainerLogsRequest options,
       List<ContainerLogsRequest> containerLogRequests =
           getContainersLogRequestForRunningApplication(options);
       for (ContainerLogsRequest container : containerLogRequests) {
-        printContainerLogsFromRunningApplication(getConf(), container,
+        int result = printContainerLogsFromRunningApplication(getConf(),
-            logCliHelper);
+            container, logCliHelper);
+        if (result == 0) {
+          resultCode = 0;
+        }
       }
     }
     if (resultCode == -1) {
@@ -879,8 +901,7 @@ private ContainerLogsRequest getMatchedLogOptions(
       List<String> matchedFiles = new ArrayList<String>();
       if (!request.getLogTypes().contains(".*")) {
         Set<String> files = logCliHelper.listContainerLogs(request);
-        matchedFiles = getMatchedLogFiles(
+        matchedFiles = getMatchedLogFiles(request, files);
-            request, files, true);
         if (matchedFiles.isEmpty()) {
           return null;
         }
@@ -891,7 +912,7 @@ private ContainerLogsRequest getMatchedLogOptions(
   }
 
   private List<String> getMatchedLogFiles(ContainerLogsRequest options,
-      Collection<String> candidate, boolean printError) throws IOException {
+      Collection<String> candidate) throws IOException {
     List<String> matchedFiles = new ArrayList<String>();
     List<String> filePattern = options.getLogTypes();
     for (String file : candidate) {
@@ -899,13 +920,6 @@ private List<String> getMatchedLogFiles(ContainerLogsRequest options,
         matchedFiles.add(file);
       }
     }
-    if (matchedFiles.isEmpty()) {
-      if (printError) {
-        System.err.println("Can not find any log file matching the pattern: "
-            + options.getLogTypes() + " for the application: "
-            + options.getAppId());
-      }
-    }
     return matchedFiles;
   }
 

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestLogsCLI.java

@@ -22,7 +22,6 @@
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
-import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
@@ -208,11 +207,10 @@ public void testHelpMessage() throws Exception {
     pw.println(" -list_nodes                     Show the list of nodes that successfully");
     pw.println("                                 aggregated logs. This option can only be");
     pw.println("                                 used with finished applications.");
-    pw.println(" -logFiles <Log File Name>       Work with -am/-containerId and specify");
+    pw.println(" -logFiles <Log File Name>       Specify comma-separated value to get");
-    pw.println("                                 comma-separated value to get specified");
+    pw.println("                                 specified container log files. Use \"ALL\"");
-    pw.println("                                 container log files. Use \"ALL\" to fetch");
+    pw.println("                                 to fetch all the log files for the");
-    pw.println("                                 all the log files for the container. It");
+    pw.println("                                 container. It also supports Java Regex.");
-    pw.println("                                 also supports Java Regex.");
     pw.println(" -nodeAddress <Node Address>     NodeAddress in the format nodename:port");
     pw.println(" -out <Local Directory>          Local directory for storing individual");
     pw.println("                                 container logs. The container logs will");
@@ -364,7 +362,8 @@ public ContainerReport getContainerReport(String containerIdStr)
         "-logFiles", "123"});
     assertTrue(exitCode == -1);
     assertTrue(sysErrStream.toString().contains(
-        "Can not find any log file matching the pattern: [123]"));
+        "Can not find any log file matching the pattern: [123] "
+        + "for the application: " + appId.toString()));
     sysErrStream.reset();
 
     // specify the bytes which is larger than the actual file size,
@@ -391,6 +390,15 @@ public ContainerReport getContainerReport(String containerIdStr)
           + " are not present in this log-file."));
     sysOutStream.reset();
 
+    exitCode = cli.run(new String[] {"-applicationId", appId.toString(),
+        "-containerId", containerId3.toString(), "-logFiles", "123" });
+    assertTrue(exitCode == -1);
+    assertTrue(sysErrStream.toString().contains(
+        "Can not find any log file matching the pattern: [123] "
+        + "for the container: " + containerId3
+        + " within the application: " + appId.toString()));
+    sysErrStream.reset();
+
     exitCode = cli.run(new String[] {"-applicationId", appId.toString(),
         "-containerId", containerId3.toString(), "-logFiles", "stdout" });
     assertTrue(exitCode == 0);
@@ -530,7 +538,7 @@ public void testFetchRunningApplicationLogs() throws Exception {
         YarnApplicationState.RUNNING, ugi.getShortUserName(), true,
         attemptReports, containerReports);
     LogsCLI cli = spy(new LogsCLIForTest(mockYarnClient));
-    doNothing().when(cli).printContainerLogsFromRunningApplication(
+    doReturn(0).when(cli).printContainerLogsFromRunningApplication(
         any(Configuration.class), any(ContainerLogsRequest.class),
         any(LogCLIHelpers.class));
 

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/LogCLIHelpers.java

@@ -46,8 +46,6 @@
 import org.apache.hadoop.yarn.conf.YarnConfiguration;
 import org.apache.hadoop.yarn.logaggregation.AggregatedLogFormat.LogKey;
 import org.apache.hadoop.yarn.logaggregation.AggregatedLogFormat.LogReader;
-import org.apache.hadoop.yarn.util.ConverterUtils;
-
 import com.google.common.annotations.VisibleForTesting;
 
 public class LogCLIHelpers implements Configurable {
@@ -66,7 +64,7 @@ public int dumpAContainersLogs(String appId, String containerId,
     List<String> logs = new ArrayList<String>();
     options.setLogTypes(logs);
     options.setBytes(Long.MAX_VALUE);
-    return dumpAContainersLogsForALogType(options, false);
+    return dumpAContainerLogsForLogType(options, false);
   }
 
   @Private
@@ -118,14 +116,14 @@ public static String getOwnerForAppIdOrNull(
 
   @Private
   @VisibleForTesting
-  public int dumpAContainersLogsForALogType(ContainerLogsRequest options)
+  public int dumpAContainerLogsForLogType(ContainerLogsRequest options)
       throws IOException {
-    return dumpAContainersLogsForALogType(options, true);
+    return dumpAContainerLogsForLogType(options, true);
   }
 
   @Private
   @VisibleForTesting
-  public int dumpAContainersLogsForALogType(ContainerLogsRequest options,
+  public int dumpAContainerLogsForLogType(ContainerLogsRequest options,
       boolean outputFailure) throws IOException {
     ApplicationId applicationId = options.getAppId();
     String jobOwner = options.getAppOwner();
@@ -190,7 +188,7 @@ public int dumpAContainersLogsForALogType(ContainerLogsRequest options,
   }
 
   @Private
-  public int dumpAContainersLogsForALogTypeWithoutNodeId(
+  public int dumpAContainerLogsForLogTypeWithoutNodeId(
       ContainerLogsRequest options) throws IOException {
     ApplicationId applicationId = options.getAppId();
     String jobOwner = options.getAppOwner();