From 6090c51fc5531691cb3babaa5dca456126bfb281 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Mon, 12 Sep 2016 16:56:13 +0200 Subject: [PATCH] Add quiet option to disable console logging (#20422) This commit adds a -q/--quiet option to Elasticsearch so that it does not log anything in the console and closes stdout & stderr streams. This is useful for SystemD to avoid duplicate logs in both journalctl and /var/log/elasticsearch/elasticsearch.log while still allows the JVM to print error messages in stdout/stderr if needed. closes #17220 --- TESTING.asciidoc | 4 +-- .../elasticsearch/bootstrap/Bootstrap.java | 9 ++--- .../bootstrap/BootstrapException.java | 2 +- .../bootstrap/Elasticsearch.java | 13 +++++-- .../bootstrap/ElasticsearchCliTests.java | 36 ++++++++++++++----- .../packaging/systemd/elasticsearch.service | 7 ++++ docs/reference/setup/install/systemd.asciidoc | 29 +++++++++++++-- .../setup/install/zip-targz.asciidoc | 7 ++-- .../bootstrap/EvilElasticsearchCliTests.java | 4 +-- .../packaging/scripts/60_systemd.bats | 15 ++++++++ .../bootstrap/ESElasticsearchCliTestCase.java | 6 ++-- 11 files changed, 103 insertions(+), 29 deletions(-) diff --git a/TESTING.asciidoc b/TESTING.asciidoc index 5cdc99c9be3..dd6c093047a 100644 --- a/TESTING.asciidoc +++ b/TESTING.asciidoc @@ -368,7 +368,8 @@ These are the linux flavors the Vagrantfile currently supports: * debian-8 aka jessie, the current debian stable distribution * centos-6 * centos-7 -* fedora-22 +* fedora-24 +* oel-6 aka Oracle Enterprise Linux 6 * oel-7 aka Oracle Enterprise Linux 7 * sles-12 * opensuse-13 @@ -377,7 +378,6 @@ We're missing the following from the support matrix because there aren't high quality boxes available in vagrant atlas: * sles-11 -* oel-6 We're missing the follow because our tests are very linux/bash centric: diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index d2e22c013b5..348fd213e9b 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -227,12 +227,12 @@ final class Bootstrap { } /** - * This method is invoked by {@link Elasticsearch#main(String[])} - * to startup elasticsearch. + * This method is invoked by {@link Elasticsearch#main(String[])} to startup elasticsearch. */ static void init( final boolean foreground, final Path pidFile, + final boolean quiet, final Map esSettings) throws BootstrapException, NodeValidationException { // Set the system property before anything has a chance to trigger its use initLoggerPrefix(); @@ -259,8 +259,9 @@ final class Bootstrap { } } + final boolean closeStandardStreams = (foreground == false) || quiet; try { - if (!foreground) { + if (closeStandardStreams) { final Logger rootLogger = ESLoggerFactory.getRootLogger(); final Appender maybeConsoleAppender = Loggers.findAppender(rootLogger, ConsoleAppender.class); if (maybeConsoleAppender != null) { @@ -285,7 +286,7 @@ final class Bootstrap { INSTANCE.start(); - if (!foreground) { + if (closeStandardStreams) { closeSysError(); } } catch (NodeValidationException | RuntimeException e) { diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java index d28abb9f6ac..540a732dfae 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java @@ -26,7 +26,7 @@ import java.util.Map; * Wrapper exception for checked exceptions thrown during the bootstrap process. Methods invoked * during bootstrap should explicitly declare the checked exceptions that they can throw, rather * than declaring the top-level checked exception {@link Exception}. This exception exists to wrap - * these checked exceptions so that {@link Bootstrap#init(boolean, Path, Map)} does not have to + * these checked exceptions so that {@link Bootstrap#init(boolean, Path, boolean, Map)} does not have to * declare all of these checked exceptions. */ class BootstrapException extends Exception { diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index 64e7350b5fa..43160ee8c9b 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -44,6 +44,7 @@ class Elasticsearch extends SettingCommand { private final OptionSpecBuilder versionOption; private final OptionSpecBuilder daemonizeOption; private final OptionSpec pidfileOption; + private final OptionSpecBuilder quietOption; // visible for testing Elasticsearch() { @@ -58,6 +59,10 @@ class Elasticsearch extends SettingCommand { .availableUnless(versionOption) .withRequiredArg() .withValuesConvertedBy(new PathConverter()); + quietOption = parser.acceptsAll(Arrays.asList("q", "quiet"), + "Turns off standard ouput/error streams logging in console") + .availableUnless(versionOption) + .availableUnless(daemonizeOption); } /** @@ -92,17 +97,19 @@ class Elasticsearch extends SettingCommand { final boolean daemonize = options.has(daemonizeOption); final Path pidFile = pidfileOption.value(options); + final boolean quiet = options.has(quietOption); try { - init(daemonize, pidFile, settings); + init(daemonize, pidFile, quiet, settings); } catch (NodeValidationException e) { throw new UserException(ExitCodes.CONFIG, e.getMessage()); } } - void init(final boolean daemonize, final Path pidFile, final Map esSettings) throws NodeValidationException { + void init(final boolean daemonize, final Path pidFile, final boolean quiet, final Map esSettings) + throws NodeValidationException { try { - Bootstrap.init(!daemonize, pidFile, esSettings); + Bootstrap.init(!daemonize, pidFile, quiet, esSettings); } catch (BootstrapException | RuntimeException e) { // format exceptions to the console in a special way // to avoid 2MB stacktraces from guice, etc. diff --git a/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java b/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java index f8bdf244999..9a1417bdfa6 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java @@ -43,6 +43,9 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--daemonize"); runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-p", "/tmp/pid"); runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--pidfile", "/tmp/pid"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-q"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--quiet"); + runTestThatVersionIsReturned("-V"); runTestThatVersionIsReturned("--version"); } @@ -66,7 +69,7 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { } private void runTestVersion(int expectedStatus, Consumer outputConsumer, String... args) throws Exception { - runTest(expectedStatus, false, outputConsumer, (foreground, pidFile, esSettings) -> {}, args); + runTest(expectedStatus, false, outputConsumer, (foreground, pidFile, quiet, esSettings) -> {}, args); } public void testPositionalArgs() throws Exception { @@ -74,21 +77,21 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { ExitCodes.USAGE, false, output -> assertThat(output, containsString("Positional arguments not allowed, found [foo]")), - (foreground, pidFile, esSettings) -> {}, + (foreground, pidFile, quiet, esSettings) -> {}, "foo" ); runTest( ExitCodes.USAGE, false, output -> assertThat(output, containsString("Positional arguments not allowed, found [foo, bar]")), - (foreground, pidFile, esSettings) -> {}, + (foreground, pidFile, quiet, esSettings) -> {}, "foo", "bar" ); runTest( ExitCodes.USAGE, false, output -> assertThat(output, containsString("Positional arguments not allowed, found [foo]")), - (foreground, pidFile, esSettings) -> {}, + (foreground, pidFile, quiet, esSettings) -> {}, "-E", "foo=bar", "foo", "-E", "baz=qux" ); } @@ -109,7 +112,7 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { expectedStatus, expectedInit, outputConsumer, - (foreground, pidFile, esSettings) -> assertThat(pidFile.toString(), equalTo(expectedPidFile.toString())), + (foreground, pidFile, quiet, esSettings) -> assertThat(pidFile.toString(), equalTo(expectedPidFile.toString())), args); } @@ -124,7 +127,22 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { ExitCodes.OK, true, output -> {}, - (foreground, pidFile, esSettings) -> assertThat(foreground, equalTo(!expectedDaemonize)), + (foreground, pidFile, quiet, esSettings) -> assertThat(foreground, equalTo(!expectedDaemonize)), + args); + } + + public void testThatParsingQuietOptionWorks() throws Exception { + runQuietTest(true, "-q"); + runQuietTest(true, "--quiet"); + runQuietTest(false); + } + + private void runQuietTest(final boolean expectedQuiet, final String... args) throws Exception { + runTest( + ExitCodes.OK, + true, + output -> {}, + (foreground, pidFile, quiet, esSettings) -> assertThat(quiet, equalTo(expectedQuiet)), args); } @@ -133,7 +151,7 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { ExitCodes.OK, true, output -> {}, - (foreground, pidFile, esSettings) -> { + (foreground, pidFile, quiet, esSettings) -> { assertThat(esSettings.size(), equalTo(2)); assertThat(esSettings, hasEntry("foo", "bar")); assertThat(esSettings, hasEntry("baz", "qux")); @@ -147,7 +165,7 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { ExitCodes.USAGE, false, output -> assertThat(output, containsString("Setting [foo] must not be empty")), - (foreground, pidFile, esSettings) -> {}, + (foreground, pidFile, quiet, esSettings) -> {}, "-E", "foo=" ); } @@ -157,7 +175,7 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { ExitCodes.USAGE, false, output -> assertThat(output, containsString("network.host is not a recognized option")), - (foreground, pidFile, esSettings) -> {}, + (foreground, pidFile, quiet, esSettings) -> {}, "--network.host"); } diff --git a/distribution/src/main/packaging/systemd/elasticsearch.service b/distribution/src/main/packaging/systemd/elasticsearch.service index 8971bc2bc19..0554371a1f9 100644 --- a/distribution/src/main/packaging/systemd/elasticsearch.service +++ b/distribution/src/main/packaging/systemd/elasticsearch.service @@ -21,10 +21,17 @@ ExecStartPre=/usr/share/elasticsearch/bin/elasticsearch-systemd-pre-exec ExecStart=/usr/share/elasticsearch/bin/elasticsearch \ -p ${PID_DIR}/elasticsearch.pid \ + --quiet \ -Edefault.path.logs=${LOG_DIR} \ -Edefault.path.data=${DATA_DIR} \ -Edefault.path.conf=${CONF_DIR} +# StandardOutput is configured to redirect to journalctl since +# some error messages may be logged in standard output before +# elasticsearch logging system is initialized. Elasticsearch +# stores its logs in /var/log/elasticsearch and does not use +# journalctl by default. If you also want to enable journalctl +# logging, you can simply remove the "quiet" option from ExecStart. StandardOutput=journal StandardError=inherit diff --git a/docs/reference/setup/install/systemd.asciidoc b/docs/reference/setup/install/systemd.asciidoc index 035932a83f8..bf94e95fb63 100644 --- a/docs/reference/setup/install/systemd.asciidoc +++ b/docs/reference/setup/install/systemd.asciidoc @@ -18,13 +18,36 @@ sudo systemctl stop elasticsearch.service -------------------------------------------- These commands provide no feedback as to whether Elasticsearch was started -successfully or not. Instead, this information will be written to the -`systemd` journal, which can be tailed as follows: +successfully or not. Instead, this information will be written in the log +files located in `/var/log/elasticsearch/`. + +By default the Elasticsearch service doesn't log information in the `systemd` +journal. To enable `journalctl` logging, the `--quiet` option must be removed + from the `ExecStart` command line in the `elasticsearch.service` file. + +When `systemd` logging is enabled, the logging information are available using +the `journalctl` commands: + +To tail the journal: [source,sh] -------------------------------------------- sudo journalctl -f -------------------------------------------- -Log files can be found in `/var/log/elasticsearch/`. +To list journal entries for the elasticsearch service: +[source,sh] +-------------------------------------------- +sudo journalctl --unit elasticsearch +-------------------------------------------- + +To list journal entries for the elasticsearch service starting from a given time: + +[source,sh] +-------------------------------------------- +sudo journalctl --unit elasticsearch --since "2016-10-30 18:17:16" +-------------------------------------------- + +Check `man journalctl` or https://www.freedesktop.org/software/systemd/man/journalctl.html for +more command line options. diff --git a/docs/reference/setup/install/zip-targz.asciidoc b/docs/reference/setup/install/zip-targz.asciidoc index 5f85447bdcd..14421795e05 100644 --- a/docs/reference/setup/install/zip-targz.asciidoc +++ b/docs/reference/setup/install/zip-targz.asciidoc @@ -52,11 +52,14 @@ Elasticsearch can be started from the command line as follows: ./bin/elasticsearch -------------------------------------------- -By default, Elasticsearch runs in the foreground, prints its logs to `STDOUT`, -and can be stopped by pressing `Ctrl-C`. +By default, Elasticsearch runs in the foreground, prints its logs to the +standard output (`stdout`), and can be stopped by pressing `Ctrl-C`. include::check-running.asciidoc[] +Log printing to `stdout` can be disabled using the `-q` or `--quiet` +option on the command line. + [[setup-installation-daemon]] ==== Running as a daemon diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java index 8bd2451da57..23da3a99df5 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java @@ -38,7 +38,7 @@ public class EvilElasticsearchCliTests extends ESElasticsearchCliTestCase { ExitCodes.OK, true, output -> {}, - (foreground, pidFile, esSettings) -> { + (foreground, pidFile, quiet, esSettings) -> { assertThat(esSettings.size(), equalTo(1)); assertThat(esSettings, hasEntry("path.home", value)); }); @@ -49,7 +49,7 @@ public class EvilElasticsearchCliTests extends ESElasticsearchCliTestCase { ExitCodes.OK, true, output -> {}, - (foreground, pidFile, esSettings) -> { + (foreground, pidFile, quiet, esSettings) -> { assertThat(esSettings.size(), equalTo(1)); assertThat(esSettings, hasEntry("path.home", commandLineValue)); }, diff --git a/qa/vagrant/src/test/resources/packaging/scripts/60_systemd.bats b/qa/vagrant/src/test/resources/packaging/scripts/60_systemd.bats index da7b6a180f1..bfd06b530f0 100644 --- a/qa/vagrant/src/test/resources/packaging/scripts/60_systemd.bats +++ b/qa/vagrant/src/test/resources/packaging/scripts/60_systemd.bats @@ -68,9 +68,24 @@ setup() { # starting Elasticsearch so we don't have to wait for elasticsearch to scan for # them. install_elasticsearch_test_scripts + + # Capture the current epoch in millis + run date +%s + epoch="$output" + systemctl start elasticsearch.service wait_for_elasticsearch_status assert_file_exist "/var/run/elasticsearch/elasticsearch.pid" + assert_file_exist "/var/log/elasticsearch/elasticsearch.log" + + # Converts the epoch back in a human readable format + run date --date=@$epoch "+%Y-%m-%d %H:%M:%S" + since="$output" + + # Verifies that no new entries in journald have been added + # since the last start + run journalctl _SYSTEMD_UNIT=elasticsearch.service --since "$since" + [ "$status" -eq 1 ] } @test "[SYSTEMD] start (running)" { diff --git a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java index 2a04a5be97f..b3bbd5a9a43 100644 --- a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java @@ -32,7 +32,7 @@ import static org.hamcrest.CoreMatchers.equalTo; abstract class ESElasticsearchCliTestCase extends ESTestCase { interface InitConsumer { - void accept(final boolean foreground, final Path pidFile, final Map esSettings); + void accept(final boolean foreground, final Path pidFile, final boolean quiet, final Map esSettings); } void runTest( @@ -46,9 +46,9 @@ abstract class ESElasticsearchCliTestCase extends ESTestCase { final AtomicBoolean init = new AtomicBoolean(); final int status = Elasticsearch.main(args, new Elasticsearch() { @Override - void init(final boolean daemonize, final Path pidFile, final Map esSettings) { + void init(final boolean daemonize, final Path pidFile, final boolean quiet, final Map esSettings) { init.set(true); - initConsumer.accept(!daemonize, pidFile, esSettings); + initConsumer.accept(!daemonize, pidFile, quiet, esSettings); } }, terminal); assertThat(status, equalTo(expectedStatus));