diff --git a/core/cli/src/main/java/org/elasticsearch/cli/Command.java b/core/cli/src/main/java/org/elasticsearch/cli/Command.java index 78a9f31283d00..34ede7ccf9423 100644 --- a/core/cli/src/main/java/org/elasticsearch/cli/Command.java +++ b/core/cli/src/main/java/org/elasticsearch/cli/Command.java @@ -38,6 +38,8 @@ public abstract class Command implements Closeable { /** A description of the command, used in the help output. */ protected final String description; + private final Runnable beforeMain; + /** The option parser for this command. */ protected final OptionParser parser = new OptionParser(); @@ -46,8 +48,15 @@ public abstract class Command implements Closeable { private final OptionSpec verboseOption = parser.acceptsAll(Arrays.asList("v", "verbose"), "show verbose output").availableUnless(silentOption); - public Command(String description) { + /** + * Construct the command with the specified command description and runnable to execute before main is invoked. + * + * @param description the command description + * @param beforeMain the before-main runnable + */ + public Command(final String description, final Runnable beforeMain) { this.description = description; + this.beforeMain = beforeMain; } private Thread shutdownHookThread; @@ -75,7 +84,7 @@ public final int main(String[] args, Terminal terminal) throws Exception { Runtime.getRuntime().addShutdownHook(shutdownHookThread); } - beforeExecute(); + beforeMain.run(); try { mainWithoutErrorHandling(args, terminal); @@ -93,12 +102,6 @@ public final int main(String[] args, Terminal terminal) throws Exception { return ExitCodes.OK; } - /** - * Setup method to be executed before parsing or execution of the command being run. Any exceptions thrown by the - * method will not be cleanly caught by the parser. - */ - protected void beforeExecute() {} - /** * Executes the command, but all errors are thrown. */ diff --git a/core/cli/src/main/java/org/elasticsearch/cli/MultiCommand.java b/core/cli/src/main/java/org/elasticsearch/cli/MultiCommand.java index 16754cd7bf12e..ba6b447792aa1 100644 --- a/core/cli/src/main/java/org/elasticsearch/cli/MultiCommand.java +++ b/core/cli/src/main/java/org/elasticsearch/cli/MultiCommand.java @@ -35,8 +35,14 @@ public class MultiCommand extends Command { private final NonOptionArgumentSpec arguments = parser.nonOptions("command"); - public MultiCommand(String description) { - super(description); + /** + * Construct the multi-command with the specified command description and runnable to execute before main is invoked. + * + * @param description the multi-command description + * @param beforeMain the before-main runnable + */ + public MultiCommand(final String description, final Runnable beforeMain) { + super(description, beforeMain); parser.posixlyCorrect(true); } diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index cfe73459a05af..1538f0cdf0003 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -51,7 +51,7 @@ class Elasticsearch extends EnvironmentAwareCommand { // visible for testing Elasticsearch() { - super("starts elasticsearch"); + super("starts elasticsearch", () -> {}); // we configure logging later so we override the base class from configuring logging versionOption = parser.acceptsAll(Arrays.asList("V", "version"), "Prints elasticsearch version information and exits"); daemonizeOption = parser.acceptsAll(Arrays.asList("d", "daemonize"), @@ -92,15 +92,6 @@ static int main(final String[] args, final Elasticsearch elasticsearch, final Te return elasticsearch.main(args, terminal); } - @Override - protected boolean shouldConfigureLoggingWithoutConfig() { - /* - * If we allow logging to be configured without a config before we are ready to read the log4j2.properties file, then we will fail - * to detect uses of logging before it is properly configured. - */ - return false; - } - @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws UserException { if (options.nonOptionArguments().isEmpty() == false) { diff --git a/core/src/main/java/org/elasticsearch/cli/CommandLoggingConfigurator.java b/core/src/main/java/org/elasticsearch/cli/CommandLoggingConfigurator.java new file mode 100644 index 0000000000000..406c362dd724a --- /dev/null +++ b/core/src/main/java/org/elasticsearch/cli/CommandLoggingConfigurator.java @@ -0,0 +1,43 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.cli; + +import org.apache.logging.log4j.Level; +import org.elasticsearch.common.logging.LogConfigurator; +import org.elasticsearch.common.settings.Settings; + +/** + * Holder class for method to configure logging without Elasticsearch configuration files for use in CLI tools that will not read such + * files. + */ +final class CommandLoggingConfigurator { + + /** + * Configures logging without Elasticsearch configuration files based on the system property "es.logger.level" only. As such, any + * logging will be written to the console. + */ + static void configureLoggingWithoutConfig() { + // initialize default for es.logger.level because we will not read the log4j2.properties + final String loggerLevel = System.getProperty("es.logger.level", Level.INFO.name()); + final Settings settings = Settings.builder().put("logger.level", loggerLevel).build(); + LogConfigurator.configureWithoutConfig(settings); + } + +} diff --git a/core/src/main/java/org/elasticsearch/cli/EnvironmentAwareCommand.java b/core/src/main/java/org/elasticsearch/cli/EnvironmentAwareCommand.java index b2bd887e0f6ec..7d9636559571e 100644 --- a/core/src/main/java/org/elasticsearch/cli/EnvironmentAwareCommand.java +++ b/core/src/main/java/org/elasticsearch/cli/EnvironmentAwareCommand.java @@ -22,9 +22,7 @@ import joptsimple.OptionSet; import joptsimple.OptionSpec; import joptsimple.util.KeyValuePair; -import org.apache.logging.log4j.Level; import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.common.logging.LogConfigurator; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.node.InternalSettingsPreparer; @@ -40,8 +38,25 @@ public abstract class EnvironmentAwareCommand extends Command { private final OptionSpec settingOption; - public EnvironmentAwareCommand(String description) { - super(description); + /** + * Construct the command with the specified command description. This command will have logging configured without reading Elasticsearch + * configuration files. + * + * @param description the command description + */ + public EnvironmentAwareCommand(final String description) { + this(description, CommandLoggingConfigurator::configureLoggingWithoutConfig); + } + + /** + * Construct the command with the specified command description and runnable to execute before main is invoked. Commands constructed + * with this constructor must take ownership of configuring logging. + * + * @param description the command description + * @param beforeMain the before-main runnable + */ + public EnvironmentAwareCommand(final String description, final Runnable beforeMain) { + super(description, beforeMain); this.settingOption = parser.accepts("E", "Configure a setting").withRequiredArg().ofType(KeyValuePair.class); } @@ -104,26 +119,6 @@ private static void putSystemPropertyIfSettingIsMissing(final Map {}); } @Override @@ -46,7 +46,7 @@ protected boolean addShutdownHook() { static class UsageErrorCommand extends Command { UsageErrorCommand() { - super("Throws a usage error"); + super("Throws a usage error", () -> {}); } @Override @@ -66,7 +66,7 @@ static class NoopCommand extends Command { boolean executed = false; NoopCommand() { - super("Does nothing"); + super("Does nothing", () -> {}); } @Override diff --git a/core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java b/core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java index f468049202808..f4448bbedfef5 100644 --- a/core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java +++ b/core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java @@ -26,13 +26,13 @@ public class MultiCommandTests extends CommandTestCase { static class DummyMultiCommand extends MultiCommand { DummyMultiCommand() { - super("A dummy multi command"); + super("A dummy multi command", () -> {}); } } static class DummySubCommand extends Command { DummySubCommand() { - super("A dummy subcommand"); + super("A dummy subcommand", () -> {}); } @Override protected void execute(Terminal terminal, OptionSet options) throws Exception { diff --git a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/PluginCli.java b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/PluginCli.java index ccc96c94eb7f8..aac22302d3be4 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/PluginCli.java +++ b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/PluginCli.java @@ -21,6 +21,7 @@ import org.apache.lucene.util.IOUtils; import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.LoggingAwareMultiCommand; import org.elasticsearch.cli.MultiCommand; import org.elasticsearch.cli.Terminal; @@ -31,7 +32,7 @@ /** * A cli tool for adding, removing and listing plugins for elasticsearch. */ -public class PluginCli extends MultiCommand { +public class PluginCli extends LoggingAwareMultiCommand { private final Collection commands; diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/cli/EvilCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/cli/EvilCommandTests.java index 7c51f8afe69b1..2990101134fb2 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/cli/EvilCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/cli/EvilCommandTests.java @@ -33,7 +33,7 @@ public class EvilCommandTests extends ESTestCase { public void testCommandShutdownHook() throws Exception { final AtomicBoolean closed = new AtomicBoolean(); final boolean shouldThrow = randomBoolean(); - final Command command = new Command("test-command-shutdown-hook") { + final Command command = new Command("test-command-shutdown-hook", () -> {}) { @Override protected void execute(Terminal terminal, OptionSet options) throws Exception {