From 6ab5aa1c1f82f81726c6daa38b3db90d8c3ad856 Mon Sep 17 00:00:00 2001 From: Xuan Date: Wed, 22 Jun 2016 21:48:49 -0700 Subject: [PATCH] YARN-5266. Wrong exit code while trying to get app logs using regex via CLI. Contributed by Xuan Gong --- .../hadoop/yarn/client/cli/LogsCLI.java | 62 ++++++++++++------- .../hadoop/yarn/client/cli/TestLogsCLI.java | 24 ++++--- .../yarn/logaggregation/LogCLIHelpers.java | 12 ++-- 3 files changed, 59 insertions(+), 39 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/LogsCLI.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/LogsCLI.java index 411d0bc8711..3cd3bcff90f 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/LogsCLI.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/LogsCLI.java @@ -371,7 +371,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(); @@ -385,10 +385,12 @@ public class LogsCLI extends Configured implements Tool { // filter the log files based on the given --logFiles pattern List allLogs= getContainerLogFiles(getConf(), containerIdStr, nodeHttpAddress); - List matchedFiles = getMatchedLogFiles( - request, allLogs, true); + List 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); @@ -398,7 +400,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:" @@ -418,6 +420,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:" @@ -425,7 +428,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); } @@ -437,9 +446,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( @@ -448,9 +461,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); } @@ -635,7 +652,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(','); @@ -813,7 +830,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 @@ -831,12 +848,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); @@ -845,8 +864,11 @@ public class LogsCLI extends Configured implements Tool { List 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) { @@ -879,8 +901,7 @@ public class LogsCLI extends Configured implements Tool { List matchedFiles = new ArrayList(); if (!request.getLogTypes().contains(".*")) { Set files = logCliHelper.listContainerLogs(request); - matchedFiles = getMatchedLogFiles( - request, files, true); + matchedFiles = getMatchedLogFiles(request, files); if (matchedFiles.isEmpty()) { return null; } @@ -891,7 +912,7 @@ public class LogsCLI extends Configured implements Tool { } private List getMatchedLogFiles(ContainerLogsRequest options, - Collection candidate, boolean printError) throws IOException { + Collection candidate) throws IOException { List matchedFiles = new ArrayList(); List filePattern = options.getLogTypes(); for (String file : candidate) { @@ -899,13 +920,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; } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestLogsCLI.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestLogsCLI.java index 6683fc75c1a..85287ea2bd4 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestLogsCLI.java +++ b/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 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 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 NodeAddress in the format nodename:port"); pw.println(" -out 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)); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/LogCLIHelpers.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/LogCLIHelpers.java index e2e3b34d801..4de11a5df1d 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/LogCLIHelpers.java +++ b/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 logs = new ArrayList(); 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();