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

(cherry picked from commit 6ab5aa1c1f82f81726c6daa38b3db90d8c3ad856)

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

@@ -370,7 +370,7 @@ public class LogsCLI extends Configured implements Tool {
 
   @Private
   @VisibleForTesting
-  public void printContainerLogsFromRunningApplication(Configuration conf,
+  public int printContainerLogsFromRunningApplication(Configuration conf,
       ContainerLogsRequest request, LogCLIHelpers logCliHelper)
       throws IOException {
     String containerIdStr = request.getContainerId().toString();
@@ -384,10 +384,12 @@ public class LogsCLI extends Configured implements Tool {
       // filter the log files based on the given --logFiles pattern
       List<String> allLogs=
           getContainerLogFiles(getConf(), containerIdStr, nodeHttpAddress);
-      List<String> matchedFiles = getMatchedLogFiles(
-          request, allLogs, true);
+      List<String> matchedFiles = getMatchedLogFiles(request, allLogs);
       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);
@@ -397,7 +399,7 @@ public class LogsCLI extends Configured implements Tool {
           + 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:"
@@ -417,6 +419,7 @@ public class LogsCLI extends Configured implements Tool {
               + " 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:"
@@ -424,7 +427,13 @@ public class LogsCLI extends Configured implements Tool {
         }
       }
       // 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);
     }
@@ -436,9 +445,13 @@ public class LogsCLI extends Configured implements Tool {
     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(
@@ -447,9 +460,13 @@ public class LogsCLI extends Configured implements Tool {
     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);
   }
 
@@ -634,7 +651,7 @@ public class LogsCLI extends Configured implements Tool {
     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(',');
@@ -812,7 +829,7 @@ public class LogsCLI extends Configured implements Tool {
         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
@@ -830,12 +847,14 @@ public class LogsCLI extends Configured implements Tool {
     // 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);
@@ -844,8 +863,11 @@ public class LogsCLI extends Configured implements Tool {
       List<ContainerLogsRequest> containerLogRequests =
           getContainersLogRequestForRunningApplication(options);
       for (ContainerLogsRequest container : containerLogRequests) {
-        printContainerLogsFromRunningApplication(getConf(), container,
-            logCliHelper);
+        int result = printContainerLogsFromRunningApplication(getConf(),
+            container, logCliHelper);
+        if (result == 0) {
+          resultCode = 0;
+        }
       }
     }
     if (resultCode == -1) {
@@ -878,8 +900,7 @@ public class LogsCLI extends Configured implements Tool {
       List<String> matchedFiles = new ArrayList<String>();
       if (!request.getLogTypes().contains(".*")) {
         Set<String> files = logCliHelper.listContainerLogs(request);
-        matchedFiles = getMatchedLogFiles(
-            request, files, true);
+        matchedFiles = getMatchedLogFiles(request, files);
         if (matchedFiles.isEmpty()) {
           return null;
         }
@@ -890,7 +911,7 @@ public class LogsCLI extends Configured implements Tool {
   }
 
   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) {
@@ -898,13 +919,6 @@ public class LogsCLI extends Configured implements Tool {
         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.assertFalse;
 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 class TestLogsCLI {
     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("                                 comma-separated value to get specified");
-    pw.println("                                 container log files. Use \"ALL\" to fetch");
-    pw.println("                                 all the log files for the container. It");
-    pw.println("                                 also supports Java Regex.");
+    pw.println(" -logFiles <Log File Name>       Specify comma-separated value to get");
+    pw.println("                                 specified container log files. Use \"ALL\"");
+    pw.println("                                 to fetch all the log files for the");
+    pw.println("                                 container. It 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 class TestLogsCLI {
         "-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 class TestLogsCLI {
           + " 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 class TestLogsCLI {
         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.api.records.ApplicationId;
 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 class LogCLIHelpers implements Configurable {
     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 class LogCLIHelpers implements Configurable {
 
   @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 class LogCLIHelpers implements Configurable {
   }
 
   @Private
-  public int dumpAContainersLogsForALogTypeWithoutNodeId(
+  public int dumpAContainerLogsForLogTypeWithoutNodeId(
       ContainerLogsRequest options) throws IOException {
     ApplicationId applicationId = options.getAppId();
     String jobOwner = options.getAppOwner();