From 8a05c2a2bef6b5980df286d885ae732657872d15 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 13 Mar 2016 09:10:56 -0400 Subject: [PATCH 1/8] Bootstrap does not set system properties Today, certain bootstrap properties are set and read via system properties. This action-at-distance way of managing these properties is rather confusing, and completely unnecessary. But another problem exists with setting these as system properties. Namely, these system properties are interpreted as Elasticsearch settings, not all of which are registered. This leads to Elasticsearch failing to startup if any of these special properties are set. Instead, these properties should be kept as local as possible, and passed around as method parameters where needed. This eliminates the action-at-distance way of handling these properties, and eliminates the need to register these non-setting properties. This commit does exactly that. Additionally, today we use the "-D" command line flag to set the properties, but this is confusing because "-D" is a special flag to the JVM for setting system properties. This creates confusion because some "-D" properties should be passed via arguments to the JVM (so via ES_JAVA_OPTS), and some should be passed as arguments to Elasticsearch. This commit changes the "-D" flag for Elasticsearch settings to "-E". --- .../elasticsearch/gradle/BuildPlugin.groovy | 1 + .../gradle/test/ClusterConfiguration.groovy | 7 + .../elasticsearch/gradle/test/NodeInfo.groovy | 10 +- .../resources/checkstyle_suppressions.xml | 2 - .../elasticsearch/bootstrap/Bootstrap.java | 60 +++--- .../bootstrap/BootstrapCliParser.java | 95 --------- .../bootstrap/Elasticsearch.java | 85 +++++++- .../common/logging/LogConfigurator.java | 4 +- .../common/settings/Settings.java | 25 +-- .../internal/InternalSettingsPreparer.java | 12 +- .../ElasticsearchCommandLineParsingTests.java | 195 ++++++++++++++++++ .../common/logging/config/logging.yml | 2 +- .../src/main/packaging/init.d/elasticsearch | 2 +- .../src/main/packaging/init.d/elasticsearch | 2 +- .../packaging/systemd/elasticsearch.service | 10 +- .../src/main/resources/bin/elasticsearch | 8 +- .../resources/bin/elasticsearch-plugin.bat | 2 +- .../main/resources/bin/elasticsearch.in.bat | 2 +- .../src/main/resources/config/logging.yml | 2 +- docs/plugins/plugin-script.asciidoc | 2 +- docs/reference/getting-started.asciidoc | 2 +- .../allocation/filtering.asciidoc | 2 +- .../migration/migrate_5_0/settings.asciidoc | 17 +- .../cluster/allocation_awareness.asciidoc | 2 +- docs/reference/modules/node.asciidoc | 2 +- docs/reference/setup.asciidoc | 7 +- docs/reference/setup/configuration.asciidoc | 4 +- docs/reference/setup/rolling_upgrade.asciidoc | 2 +- modules/lang-groovy/build.gradle | 4 +- modules/lang-mustache/build.gradle | 4 +- plugins/lang-javascript/build.gradle | 4 +- plugins/lang-python/build.gradle | 4 +- .../bootstrap/BootstrapCliParserTests.java | 164 --------------- qa/smoke-test-ingest-disabled/build.gradle | 2 +- .../build.gradle | 2 +- .../scripts/packaging_test_utils.bash | 2 +- .../packaging/scripts/plugin_test_cases.bash | 4 +- .../common/cli/CliToolTestCase.java | 65 ------ 38 files changed, 392 insertions(+), 429 deletions(-) delete mode 100644 core/src/main/java/org/elasticsearch/bootstrap/BootstrapCliParser.java create mode 100644 core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCommandLineParsingTests.java delete mode 100644 qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java delete mode 100644 test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index ca78157bcf2..a63d31e9085 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -407,6 +407,7 @@ class BuildPlugin implements Plugin { systemProperty 'jna.nosys', 'true' // default test sysprop values systemProperty 'tests.ifNoTests', 'fail' + // TODO: remove setting logging level via system property systemProperty 'es.logger.level', 'WARN' for (Map.Entry property : System.properties.entrySet()) { if (property.getKey().startsWith('tests.') || diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy index 3e8b6225329..2adc59e9e9d 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy @@ -73,6 +73,8 @@ class ClusterConfiguration { return tmpFile.exists() } + Map esSettings = new HashMap<>(); + Map systemProperties = new HashMap<>() Map settings = new HashMap<>() @@ -86,6 +88,11 @@ class ClusterConfiguration { LinkedHashMap setupCommands = new LinkedHashMap<>() + @Input + void esSetting(String setting, String value) { + esSettings.put(setting, value); + } + @Input void systemProperty(String property, String value) { systemProperties.put(property, value) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy index b41b1822000..168a67a4728 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy @@ -129,14 +129,16 @@ class NodeInfo { 'JAVA_HOME' : project.javaHome, 'ES_GC_OPTS': config.jvmArgs // we pass these with the undocumented gc opts so the argline can set gc, etc ] - args.add("-Des.node.portsfile=true") - args.addAll(config.systemProperties.collect { key, value -> "-D${key}=${value}" }) + args.addAll("-E", "es.node.portsfile=true") + args.addAll(config.esSettings.collectMany { key, value -> ["-E", "${key}=${value}" ] }) + env.put('ES_JAVA_OPTS', config.systemProperties.collect { key, value -> "-D${key}=${value}" }.join(" ")) for (Map.Entry property : System.properties.entrySet()) { if (property.getKey().startsWith('es.')) { - args.add("-D${property.getKey()}=${property.getValue()}") + args.add("-E") + args.add("${property.getKey()}=${property.getValue()}") } } - args.add("-Des.path.conf=${confDir}") + args.addAll("-E", "es.path.conf=${confDir}") if (Os.isFamily(Os.FAMILY_WINDOWS)) { args.add('"') // end the entire command, quoted } diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index cbe612e5358..0f5a59abea6 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -259,7 +259,6 @@ - @@ -1597,7 +1596,6 @@ - diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index 6cd2b4d80fe..3ad592af635 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -19,21 +19,14 @@ package org.elasticsearch.bootstrap; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.PrintStream; -import java.nio.file.Path; -import java.util.Locale; -import java.util.concurrent.CountDownLatch; - import org.apache.lucene.util.Constants; import org.apache.lucene.util.IOUtils; import org.apache.lucene.util.StringHelper; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; -import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.Terminal; import org.elasticsearch.common.PidFile; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.inject.CreationException; import org.elasticsearch.common.logging.ESLogger; @@ -47,7 +40,13 @@ import org.elasticsearch.monitor.process.ProcessProbe; import org.elasticsearch.node.Node; import org.elasticsearch.node.internal.InternalSettingsPreparer; -import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.PrintStream; +import java.nio.file.Path; +import java.util.Locale; +import java.util.Map; +import java.util.concurrent.CountDownLatch; /** * Internal startup code. @@ -189,9 +188,14 @@ final class Bootstrap { node = new Node(nodeSettings); } - private static Environment initialSettings(boolean foreground) { - Terminal terminal = foreground ? Terminal.DEFAULT : null; - return InternalSettingsPreparer.prepareEnvironment(EMPTY_SETTINGS, terminal); + private static Environment initialSettings(boolean daemonize, String pathHome, String pidFile) { + Terminal terminal = daemonize ? null : Terminal.DEFAULT; + Settings.Builder builder = Settings.builder(); + builder.put(Environment.PATH_HOME_SETTING.getKey(), pathHome); + if (Strings.hasLength(pidFile)) { + builder.put(Environment.PIDFILE_SETTING.getKey(), pidFile); + } + return InternalSettingsPreparer.prepareEnvironment(builder.build(), terminal); } private void start() { @@ -218,22 +222,19 @@ final class Bootstrap { * This method is invoked by {@link Elasticsearch#main(String[])} * to startup elasticsearch. */ - static void init(String[] args) throws Throwable { + static void init( + final boolean daemonize, + final String pathHome, + final String pidFile, + final Map esSettings) throws Throwable { // Set the system property before anything has a chance to trigger its use initLoggerPrefix(); - BootstrapCliParser parser = new BootstrapCliParser(); - int status = parser.main(args, Terminal.DEFAULT); - - if (parser.shouldRun() == false || status != ExitCodes.OK) { - exit(status); - } + elasticsearchSettings(esSettings); INSTANCE = new Bootstrap(); - boolean foreground = !"false".equals(System.getProperty("es.foreground", System.getProperty("es-foreground"))); - - Environment environment = initialSettings(foreground); + Environment environment = initialSettings(daemonize, pathHome, pidFile); Settings settings = environment.settings(); LogConfigurator.configure(settings, true); checkForCustomConfFile(); @@ -249,7 +250,7 @@ final class Bootstrap { } try { - if (!foreground) { + if (daemonize) { Loggers.disableConsoleLogging(); closeSystOut(); } @@ -264,12 +265,12 @@ final class Bootstrap { INSTANCE.start(); - if (!foreground) { + if (daemonize) { closeSysError(); } } catch (Throwable e) { // disable console logging, so user does not see the exception twice (jvm will show it already) - if (foreground) { + if (!daemonize) { Loggers.disableConsoleLogging(); } ESLogger logger = Loggers.getLogger(Bootstrap.class); @@ -289,7 +290,7 @@ final class Bootstrap { logger.error("Exception", e); } // re-enable it if appropriate, so they can see any logging during the shutdown process - if (foreground) { + if (!daemonize) { Loggers.enableConsoleLogging(); } @@ -297,6 +298,13 @@ final class Bootstrap { } } + @SuppressForbidden(reason = "Sets system properties passed as CLI parameters") + private static void elasticsearchSettings(Map esSettings) { + for (Map.Entry esSetting : esSettings.entrySet()) { + System.setProperty(esSetting.getKey(), esSetting.getValue()); + } + } + @SuppressForbidden(reason = "System#out") private static void closeSystOut() { System.out.close(); diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCliParser.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCliParser.java deleted file mode 100644 index 5c927305f14..00000000000 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapCliParser.java +++ /dev/null @@ -1,95 +0,0 @@ -/* - * 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.bootstrap; - -import java.util.Arrays; - -import joptsimple.OptionSet; -import joptsimple.OptionSpec; -import org.elasticsearch.Build; -import org.elasticsearch.cli.Command; -import org.elasticsearch.cli.ExitCodes; -import org.elasticsearch.cli.UserError; -import org.elasticsearch.common.Strings; -import org.elasticsearch.cli.Terminal; -import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.monitor.jvm.JvmInfo; - -final class BootstrapCliParser extends Command { - - private final OptionSpec versionOption; - private final OptionSpec daemonizeOption; - private final OptionSpec pidfileOption; - private final OptionSpec propertyOption; - private boolean shouldRun = false; - - BootstrapCliParser() { - super("Starts elasticsearch"); - // TODO: in jopt-simple 5.0, make this mutually exclusive with all other options - versionOption = parser.acceptsAll(Arrays.asList("V", "version"), - "Prints elasticsearch version information and exits"); - daemonizeOption = parser.acceptsAll(Arrays.asList("d", "daemonize"), - "Starts Elasticsearch in the background"); - // TODO: in jopt-simple 5.0 this option type can be a Path - pidfileOption = parser.acceptsAll(Arrays.asList("p", "pidfile"), - "Creates a pid file in the specified path on start") - .withRequiredArg(); - propertyOption = parser.accepts("D", "Configures an Elasticsearch setting") - .withRequiredArg(); - } - - // TODO: don't use system properties as a way to do this, its horrible... - @SuppressForbidden(reason = "Sets system properties passed as CLI parameters") - @Override - protected void execute(Terminal terminal, OptionSet options) throws Exception { - if (options.has(versionOption)) { - terminal.println("Version: " + org.elasticsearch.Version.CURRENT - + ", Build: " + Build.CURRENT.shortHash() + "/" + Build.CURRENT.date() - + ", JVM: " + JvmInfo.jvmInfo().version()); - return; - } - - // TODO: don't use sysprops for any of these! pass the args through to bootstrap... - if (options.has(daemonizeOption)) { - System.setProperty("es.foreground", "false"); - } - String pidFile = pidfileOption.value(options); - if (Strings.isNullOrEmpty(pidFile) == false) { - System.setProperty("es.pidfile", pidFile); - } - - for (String property : propertyOption.values(options)) { - String[] keyValue = property.split("=", 2); - if (keyValue.length != 2) { - throw new UserError(ExitCodes.USAGE, "Malformed elasticsearch setting, must be of the form key=value"); - } - String key = keyValue[0]; - if (key.startsWith("es.") == false) { - key = "es." + key; - } - System.setProperty(key, keyValue[1]); - } - shouldRun = true; - } - - boolean shouldRun() { - return shouldRun; - } -} diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index 3b95c3f4a6f..dfe49c52e98 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -19,23 +19,98 @@ package org.elasticsearch.bootstrap; +import joptsimple.OptionSet; +import joptsimple.OptionSpec; +import joptsimple.util.KeyValuePair; +import org.elasticsearch.Build; +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserError; +import org.elasticsearch.monitor.jvm.JvmInfo; + import java.io.IOException; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; /** * This class starts elasticsearch. */ -public final class Elasticsearch { +class Elasticsearch extends Command { + + private final OptionSpec versionOption; + private final OptionSpec daemonizeOption; + private final OptionSpec pathHomeOption; + private final OptionSpec pidfileOption; + private final OptionSpec propertyOption; /** no instantiation */ - private Elasticsearch() {} + Elasticsearch() { + super("starts elasticsearch"); + // TODO: in jopt-simple 5.0, make this mutually exclusive with all other options + versionOption = parser.acceptsAll(Arrays.asList("V", "version"), + "Prints elasticsearch version information and exits"); + daemonizeOption = parser.acceptsAll(Arrays.asList("d", "daemonize"), + "Starts Elasticsearch in the background"); + // TODO: in jopt-simple 5.0 this option type can be a Path + pathHomeOption = parser.acceptsAll(Arrays.asList("H", "path.home"), "").withRequiredArg(); + // TODO: in jopt-simple 5.0 this option type can be a Path + pidfileOption = parser.acceptsAll(Arrays.asList("p", "pidfile"), + "Creates a pid file in the specified path on start") + .withRequiredArg(); + propertyOption = parser.accepts("E", "Configure an Elasticsearch setting").withRequiredArg().ofType(KeyValuePair.class); + } /** * Main entry point for starting elasticsearch */ - public static void main(String[] args) throws Exception { + public static void main(final String[] args) throws Exception { + final Elasticsearch elasticsearch = new Elasticsearch(); + int status = main(args, elasticsearch, Terminal.DEFAULT); + if (status != ExitCodes.OK) { + exit(status); + } + } + + static int main(final String[] args, final Elasticsearch elasticsearch, final Terminal terminal) throws Exception { + return elasticsearch.main(args, terminal); + } + + @Override + protected void execute(Terminal terminal, OptionSet options) throws Exception { + if (options.has(versionOption)) { + if (options.has(daemonizeOption) || options.has(pathHomeOption) || options.has(pidfileOption)) { + throw new UserError(ExitCodes.USAGE, "Elasticsearch version option is mutually exclusive with any other option"); + } + terminal.println("Version: " + org.elasticsearch.Version.CURRENT + + ", Build: " + Build.CURRENT.shortHash() + "/" + Build.CURRENT.date() + + ", JVM: " + JvmInfo.jvmInfo().version()); + return; + } + + final boolean daemonize = options.has(daemonizeOption); + final String pathHome = pathHomeOption.value(options); + final String pidFile = pidfileOption.value(options); + + final Map esSettings = new HashMap<>(); + for (final KeyValuePair kvp : propertyOption.values(options)) { + if (!kvp.key.startsWith("es.")) { + throw new UserError(ExitCodes.USAGE, "Elasticsearch settings must be prefixed with [es.] but was [" + kvp.key + "]"); + } + if (kvp.value.isEmpty()) { + throw new UserError(ExitCodes.USAGE, "Elasticsearch setting [" + kvp.key + "] must not be empty"); + } + esSettings.put(kvp.key, kvp.value); + } + + init(daemonize, pathHome, pidFile, esSettings); + } + + void init(final boolean daemonize, final String pathHome, final String pidFile, final Map esSettings) { try { - Bootstrap.init(args); - } catch (Throwable t) { + Bootstrap.init(daemonize, pathHome, pidFile, esSettings); + } catch (final Throwable t) { // format exceptions to the console in a special way // to avoid 2MB stacktraces from guice, etc. throw new StartupError(t); diff --git a/core/src/main/java/org/elasticsearch/common/logging/LogConfigurator.java b/core/src/main/java/org/elasticsearch/common/logging/LogConfigurator.java index 28feca13c02..da628b09d2b 100644 --- a/core/src/main/java/org/elasticsearch/common/logging/LogConfigurator.java +++ b/core/src/main/java/org/elasticsearch/common/logging/LogConfigurator.java @@ -110,9 +110,7 @@ public class LogConfigurator { if (resolveConfig) { resolveConfig(environment, settingsBuilder); } - settingsBuilder - .putProperties("elasticsearch.", BootstrapInfo.getSystemProperties()) - .putProperties("es.", BootstrapInfo.getSystemProperties()); + settingsBuilder.putProperties("es.", BootstrapInfo.getSystemProperties()); // add custom settings after config was added so that they are not overwritten by config settingsBuilder.put(settings); settingsBuilder.replacePropertyPlaceholders(); diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java index aafaff3e9d7..e06e4ad893b 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -1136,10 +1136,10 @@ public final class Settings implements ToXContent { * @param properties The properties to put * @return The builder */ - public Builder putProperties(String prefix, Dictionary properties) { - for (Object key1 : Collections.list(properties.keys())) { - String key = Objects.toString(key1); - String value = Objects.toString(properties.get(key)); + public Builder putProperties(String prefix, Dictionary properties) { + for (Object property : Collections.list(properties.keys())) { + String key = Objects.toString(property); + String value = Objects.toString(properties.get(property)); if (key.startsWith(prefix)) { map.put(key.substring(prefix.length()), value); } @@ -1154,19 +1154,12 @@ public final class Settings implements ToXContent { * @param properties The properties to put * @return The builder */ - public Builder putProperties(String prefix, Dictionary properties, String[] ignorePrefixes) { - for (Object key1 : Collections.list(properties.keys())) { - String key = Objects.toString(key1); - String value = Objects.toString(properties.get(key)); + public Builder putProperties(String prefix, Dictionary properties, String ignorePrefix) { + for (Object property : Collections.list(properties.keys())) { + String key = Objects.toString(property); + String value = Objects.toString(properties.get(property)); if (key.startsWith(prefix)) { - boolean ignore = false; - for (String ignorePrefix : ignorePrefixes) { - if (key.startsWith(ignorePrefix)) { - ignore = true; - break; - } - } - if (!ignore) { + if (!key.startsWith(ignorePrefix)) { map.put(key.substring(prefix.length()), value); } } diff --git a/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java b/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java index b1ad3a3239f..8864a70ccdc 100644 --- a/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java +++ b/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java @@ -53,8 +53,8 @@ import static org.elasticsearch.common.settings.Settings.settingsBuilder; public class InternalSettingsPreparer { private static final String[] ALLOWED_SUFFIXES = {".yml", ".yaml", ".json", ".properties"}; - static final String[] PROPERTY_PREFIXES = {"es.", "elasticsearch."}; - static final String[] PROPERTY_DEFAULTS_PREFIXES = {"es.default.", "elasticsearch.default."}; + static final String PROPERTY_PREFIX = "es."; + static final String PROPERTY_DEFAULTS_PREFIX = "es.default."; public static final String SECRET_PROMPT_VALUE = "${prompt.secret}"; public static final String TEXT_PROMPT_VALUE = "${prompt.text}"; @@ -126,13 +126,9 @@ public class InternalSettingsPreparer { output.put(input); if (useSystemProperties(input)) { if (loadDefaults) { - for (String prefix : PROPERTY_DEFAULTS_PREFIXES) { - output.putProperties(prefix, BootstrapInfo.getSystemProperties()); - } - } - for (String prefix : PROPERTY_PREFIXES) { - output.putProperties(prefix, BootstrapInfo.getSystemProperties(), PROPERTY_DEFAULTS_PREFIXES); + output.putProperties(PROPERTY_DEFAULTS_PREFIX, BootstrapInfo.getSystemProperties()); } + output.putProperties(PROPERTY_PREFIX, BootstrapInfo.getSystemProperties(), PROPERTY_DEFAULTS_PREFIX); } output.replacePropertyPlaceholders(); } diff --git a/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCommandLineParsingTests.java b/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCommandLineParsingTests.java new file mode 100644 index 00000000000..0d70cb8fba5 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCommandLineParsingTests.java @@ -0,0 +1,195 @@ +/* + * 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.bootstrap; + +import org.elasticsearch.Build; +import org.elasticsearch.Version; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.MockTerminal; +import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.monitor.jvm.JvmInfo; +import org.elasticsearch.test.ESTestCase; +import org.junit.After; +import org.junit.Before; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.Matchers.hasEntry; + +public class ElasticsearchCommandLineParsingTests extends ESTestCase { + + public void testVersion() throws Exception { + runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "-d"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "--daemonize"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "-H", "/tmp/home"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "--path.home", "/tmp/home"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "-p", "/tmp/pid"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "--pidfile", "/tmp/pid"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-d"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--daemonize"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-H", "/tmp/home"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--path.home", "/tmp/home"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-p", "/tmp/pid"); + runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--pidfile", "/tmp/pid"); + runTestThatVersionIsReturned("-V"); + runTestThatVersionIsReturned("--version"); + } + + private void runTestThatVersionIsMutuallyExclusiveToOtherOptions(String... args) throws Exception { + runTestVersion( + ExitCodes.USAGE, + output -> assertThat( + output, + containsString("ERROR: Elasticsearch version option is mutually exclusive with any other option")), + args); + } + + private void runTestThatVersionIsReturned(String... args) throws Exception { + runTestVersion(ExitCodes.OK, output -> { + assertThat(output, containsString("Version: " + Version.CURRENT.toString())); + assertThat(output, containsString("Build: " + Build.CURRENT.shortHash() + "/" + Build.CURRENT.date())); + assertThat(output, containsString("JVM: " + JvmInfo.jvmInfo().version())); + }, args); + } + + private void runTestVersion(int expectedStatus, Consumer outputConsumer, String... args) throws Exception { + runTest(expectedStatus, false, outputConsumer, (daemonize, pathHome, pidFile, esSettings) -> {}, args); + } + + public void testThatPidFileCanBeConfigured() throws Exception { + runPidFileTest(ExitCodes.USAGE, false, output -> assertThat(output, containsString("Option p/pidfile requires an argument")), "-p"); + runPidFileTest(ExitCodes.OK, true, output -> {}, "-p", "/tmp/pid"); + runPidFileTest(ExitCodes.OK, true, output -> {}, "--pidfile", "/tmp/pid"); + } + + private void runPidFileTest(final int expectedStatus, final boolean expectedInit, Consumer outputConsumer, final String... args) + throws Exception { + runTest( + expectedStatus, + expectedInit, + outputConsumer, + (daemonize, pathHome, pidFile, esSettings) -> assertThat(pidFile, equalTo("/tmp/pid")), + args); + } + + public void testThatParsingDaemonizeWorks() throws Exception { + runDaemonizeTest(true, "-d"); + runDaemonizeTest(true, "--daemonize"); + runDaemonizeTest(false); + } + + private void runDaemonizeTest(final boolean expectedDaemonize, final String... args) throws Exception { + runTest( + ExitCodes.OK, + true, + output -> {}, + (daemonize, pathHome, pidFile, esSettings) -> assertThat(daemonize, equalTo(expectedDaemonize)), + args); + } + + public void testElasticsearchSettings() throws Exception { + runTest( + ExitCodes.OK, + true, + output -> {}, + (daemonize, pathHome, pidFile, esSettings) -> { + assertThat(esSettings.size(), equalTo(2)); + assertThat(esSettings, hasEntry("es.foo", "bar")); + assertThat(esSettings, hasEntry("es.baz", "qux")); + }, + "-Ees.foo=bar", "-E", "es.baz=qux" + ); + } + + public void testElasticsearchSettingPrefix() throws Exception { + runElasticsearchSettingPrefixTest("-E", "foo"); + runElasticsearchSettingPrefixTest("-E", "foo=bar"); + runElasticsearchSettingPrefixTest("-E", "=bar"); + } + + private void runElasticsearchSettingPrefixTest(String... args) throws Exception { + runTest( + ExitCodes.USAGE, + false, + output -> assertThat(output, containsString("Elasticsearch settings must be prefixed with [es.] but was [")), + (daemonize, pathHome, pidFile, esSettings) -> {}, + args + ); + } + + public void testElasticsearchSettingCanNotBeEmpty() throws Exception { + runTest( + ExitCodes.USAGE, + false, + output -> assertThat(output, containsString("Elasticsearch setting [es.foo] must not be empty")), + (daemonize, pathHome, pidFile, esSettings) -> {}, + "-E", "es.foo=" + ); + } + + public void testUnknownOption() throws Exception { + runTest( + ExitCodes.USAGE, + false, + output -> assertThat(output, containsString("network.host is not a recognized option")), + (daemonize, pathHome, pidFile, esSettings) -> {}, + "--network.host"); + } + + private interface InitConsumer { + void accept(final boolean daemonize, final String pathHome, final String pidFile, final Map esSettings); + } + + private void runTest( + final int expectedStatus, + final boolean expectedInit, + final Consumer outputConsumer, + final InitConsumer initConsumer, + String... args) throws Exception { + final MockTerminal terminal = new MockTerminal(); + try { + final AtomicBoolean init = new AtomicBoolean(); + final int status = Elasticsearch.main(args, new Elasticsearch() { + @Override + void init(final boolean daemonize, final String pathHome, final String pidFile, final Map esSettings) { + init.set(true); + initConsumer.accept(daemonize, pathHome, pidFile, esSettings); + } + }, terminal); + assertThat(status, equalTo(expectedStatus)); + assertThat(init.get(), equalTo(expectedInit)); + outputConsumer.accept(terminal.getOutput()); + } catch (Throwable t) { + // if an unexpected exception is thrown, we log + // terminal output to aid debugging + logger.info(terminal.getOutput()); + // rethrow so the test fails + throw t; + } + } + +} diff --git a/core/src/test/resources/org/elasticsearch/common/logging/config/logging.yml b/core/src/test/resources/org/elasticsearch/common/logging/config/logging.yml index bd7a15f4434..515e4320fd2 100644 --- a/core/src/test/resources/org/elasticsearch/common/logging/config/logging.yml +++ b/core/src/test/resources/org/elasticsearch/common/logging/config/logging.yml @@ -1,4 +1,4 @@ -# you can override this using by setting a system property, for example -Des.logger.level=DEBUG +# you can override this using by setting a system property, for example -Ees.logger.level=DEBUG es.logger.level: INFO rootLogger: ${es.logger.level}, console logger: diff --git a/distribution/deb/src/main/packaging/init.d/elasticsearch b/distribution/deb/src/main/packaging/init.d/elasticsearch index 078e79a92d1..e2d857a7ffe 100755 --- a/distribution/deb/src/main/packaging/init.d/elasticsearch +++ b/distribution/deb/src/main/packaging/init.d/elasticsearch @@ -99,7 +99,7 @@ fi # Define other required variables PID_FILE="$PID_DIR/$NAME.pid" DAEMON=$ES_HOME/bin/elasticsearch -DAEMON_OPTS="-d -p $PID_FILE -D es.default.path.home=$ES_HOME -D es.default.path.logs=$LOG_DIR -D es.default.path.data=$DATA_DIR -D es.default.path.conf=$CONF_DIR" +DAEMON_OPTS="-d -p $PID_FILE -Ees.default.path.home=$ES_HOME -Ees.default.path.logs=$LOG_DIR -Ees.default.path.data=$DATA_DIR -Ees.default.path.conf=$CONF_DIR" export ES_HEAP_SIZE export ES_HEAP_NEWSIZE diff --git a/distribution/rpm/src/main/packaging/init.d/elasticsearch b/distribution/rpm/src/main/packaging/init.d/elasticsearch index 1132fca4f9e..c68a5b65f3f 100644 --- a/distribution/rpm/src/main/packaging/init.d/elasticsearch +++ b/distribution/rpm/src/main/packaging/init.d/elasticsearch @@ -117,7 +117,7 @@ start() { cd $ES_HOME echo -n $"Starting $prog: " # if not running, start it up here, usually something like "daemon $exec" - daemon --user $ES_USER --pidfile $pidfile $exec -p $pidfile -d -D es.default.path.home=$ES_HOME -D es.default.path.logs=$LOG_DIR -D es.default.path.data=$DATA_DIR -D es.default.path.conf=$CONF_DIR + daemon --user $ES_USER --pidfile $pidfile $exec -p $pidfile -d -Ees.default.path.home=$ES_HOME -Ees.default.path.logs=$LOG_DIR -Ees.default.path.data=$DATA_DIR -Ees.default.path.conf=$CONF_DIR retval=$? echo [ $retval -eq 0 ] && touch $lockfile diff --git a/distribution/src/main/packaging/systemd/elasticsearch.service b/distribution/src/main/packaging/systemd/elasticsearch.service index 301586c1038..4f643f6a4a4 100644 --- a/distribution/src/main/packaging/systemd/elasticsearch.service +++ b/distribution/src/main/packaging/systemd/elasticsearch.service @@ -20,11 +20,11 @@ Group=elasticsearch ExecStartPre=/usr/share/elasticsearch/bin/elasticsearch-systemd-pre-exec ExecStart=/usr/share/elasticsearch/bin/elasticsearch \ - -Des.pidfile=${PID_DIR}/elasticsearch.pid \ - -Des.default.path.home=${ES_HOME} \ - -Des.default.path.logs=${LOG_DIR} \ - -Des.default.path.data=${DATA_DIR} \ - -Des.default.path.conf=${CONF_DIR} + -Ees.pidfile=${PID_DIR}/elasticsearch.pid \ + -Ees.default.path.home=${ES_HOME} \ + -Ees.default.path.logs=${LOG_DIR} \ + -Ees.default.path.data=${DATA_DIR} \ + -Ees.default.path.conf=${CONF_DIR} StandardOutput=journal StandardError=inherit diff --git a/distribution/src/main/resources/bin/elasticsearch b/distribution/src/main/resources/bin/elasticsearch index b15105f1854..0d0e0069ae2 100755 --- a/distribution/src/main/resources/bin/elasticsearch +++ b/distribution/src/main/resources/bin/elasticsearch @@ -126,11 +126,11 @@ export HOSTNAME # manual parsing to find out, if process should be detached daemonized=`echo $* | egrep -- '(^-d |-d$| -d |--daemonize$|--daemonize )'` if [ -z "$daemonized" ] ; then - exec "$JAVA" $JAVA_OPTS $ES_JAVA_OPTS -Des.path.home="$ES_HOME" -cp "$ES_CLASSPATH" \ - org.elasticsearch.bootstrap.Elasticsearch start "$@" + exec "$JAVA" $JAVA_OPTS $ES_JAVA_OPTS -cp "$ES_CLASSPATH" \ + org.elasticsearch.bootstrap.Elasticsearch --path.home "$ES_HOME" "$@" else - exec "$JAVA" $JAVA_OPTS $ES_JAVA_OPTS -Des.path.home="$ES_HOME" -cp "$ES_CLASSPATH" \ - org.elasticsearch.bootstrap.Elasticsearch start "$@" <&- & + exec "$JAVA" $JAVA_OPTS $ES_JAVA_OPTS -cp "$ES_CLASSPATH" \ + org.elasticsearch.bootstrap.Elasticsearch --path.home "$ES_HOME" "$@" <&- & retval=$? pid=$! [ $retval -eq 0 ] || exit $retval diff --git a/distribution/src/main/resources/bin/elasticsearch-plugin.bat b/distribution/src/main/resources/bin/elasticsearch-plugin.bat index 6c6be019fc6..9ed797e6308 100644 --- a/distribution/src/main/resources/bin/elasticsearch-plugin.bat +++ b/distribution/src/main/resources/bin/elasticsearch-plugin.bat @@ -48,7 +48,7 @@ GOTO loop SET HOSTNAME=%COMPUTERNAME% -"%JAVA_HOME%\bin\java" -client -Des.path.home="%ES_HOME%" !properties! -cp "%ES_HOME%/lib/*;" "org.elasticsearch.plugins.PluginCli" !args! +"%JAVA_HOME%\bin\java" -client -Ees.path.home="%ES_HOME%" !properties! -cp "%ES_HOME%/lib/*;" "org.elasticsearch.plugins.PluginCli" !args! goto finally diff --git a/distribution/src/main/resources/bin/elasticsearch.in.bat b/distribution/src/main/resources/bin/elasticsearch.in.bat index b909a464952..80ed7894316 100644 --- a/distribution/src/main/resources/bin/elasticsearch.in.bat +++ b/distribution/src/main/resources/bin/elasticsearch.in.bat @@ -104,4 +104,4 @@ ECHO additional elements via the plugin mechanism, or if code must really be 1>& ECHO added to the main classpath, add jars to lib\, unsupported 1>&2 EXIT /B 1 ) -set ES_PARAMS=-Delasticsearch -Des-foreground=yes -Des.path.home="%ES_HOME%" +set ES_PARAMS=-Delasticsearch -Ees.path.home="%ES_HOME%" diff --git a/distribution/src/main/resources/config/logging.yml b/distribution/src/main/resources/config/logging.yml index 939aa1eed0e..187e79cffa0 100644 --- a/distribution/src/main/resources/config/logging.yml +++ b/distribution/src/main/resources/config/logging.yml @@ -1,4 +1,4 @@ -# you can override this using by setting a system property, for example -Des.logger.level=DEBUG +# you can override this using by setting a system property, for example -Ees.logger.level=DEBUG es.logger.level: INFO rootLogger: ${es.logger.level}, console, file logger: diff --git a/docs/plugins/plugin-script.asciidoc b/docs/plugins/plugin-script.asciidoc index d2e57b2efc8..fba4704ab97 100644 --- a/docs/plugins/plugin-script.asciidoc +++ b/docs/plugins/plugin-script.asciidoc @@ -167,7 +167,7 @@ can do this as follows: [source,sh] --------------------- -sudo bin/elasticsearch-plugin -Des.path.conf=/path/to/custom/config/dir install +sudo bin/elasticsearch-plugin -Ees.path.conf=/path/to/custom/config/dir install --------------------- You can also set the `CONF_DIR` environment variable to the custom config diff --git a/docs/reference/getting-started.asciidoc b/docs/reference/getting-started.asciidoc index 8ff832c673f..47bcb3031ff 100755 --- a/docs/reference/getting-started.asciidoc +++ b/docs/reference/getting-started.asciidoc @@ -163,7 +163,7 @@ As mentioned previously, we can override either the cluster or node name. This c [source,sh] -------------------------------------------------- -./elasticsearch --cluster.name my_cluster_name --node.name my_node_name +./elasticsearch -Ees.cluster.name=my_cluster_name -Ees.node.name=my_node_name -------------------------------------------------- Also note the line marked http with information about the HTTP address (`192.168.8.112`) and port (`9200`) that our node is reachable from. By default, Elasticsearch uses port `9200` to provide access to its REST API. This port is configurable if necessary. diff --git a/docs/reference/index-modules/allocation/filtering.asciidoc b/docs/reference/index-modules/allocation/filtering.asciidoc index 784fa1af24c..44c9b1a712c 100644 --- a/docs/reference/index-modules/allocation/filtering.asciidoc +++ b/docs/reference/index-modules/allocation/filtering.asciidoc @@ -14,7 +14,7 @@ attribute as follows: [source,sh] ------------------------ -bin/elasticsearch --node.rack rack1 --node.size big <1> +bin/elasticsearch -Ees.node.rack=rack1 -Ees.node.size=big <1> ------------------------ <1> These attribute settings can also be specified in the `elasticsearch.yml` config file. diff --git a/docs/reference/migration/migrate_5_0/settings.asciidoc b/docs/reference/migration/migrate_5_0/settings.asciidoc index 002d6cf05df..5d39f87773d 100644 --- a/docs/reference/migration/migrate_5_0/settings.asciidoc +++ b/docs/reference/migration/migrate_5_0/settings.asciidoc @@ -153,7 +153,7 @@ on startup if it is set too low. ==== Removed es.netty.gathering -Disabling Netty from using NIO gathering could be done via the escape +Disabling Netty from using NIO gathring could be done via the escape hatch of setting the system property "es.netty.gathering" to "false". Time has proven enabling gathering by default is a non-issue and this non-documented setting has been removed. @@ -172,3 +172,18 @@ Two cache concurrency level settings `indices.fielddata.cache.concurrency_level` because they no longer apply to the cache implementation used for the request cache and the field data cache. +==== Using system properties to configure Elasticsearch + +Elasticsearch can be configured by setting system properties on the +command line via `-Des.name.of.property=value.of.property`. This will be +removed in a future version of Elasticsearch. Instead, use +`-E es.name.of.setting=value.of.setting`. Note that in all cases the +name of the setting must be prefixed with `es.`. + +==== Removed using double-dashes to configure Elasticsearch + +Elasticsearch could previously be configured on the command line by +setting settings via `--name.of.setting value.of.setting`. This feature +has been removed. Instead, use +`-Ees.name.of.setting=value.of.setting`. Note that in all cases the +name of the setting must be prefixed with `es.`. diff --git a/docs/reference/modules/cluster/allocation_awareness.asciidoc b/docs/reference/modules/cluster/allocation_awareness.asciidoc index ee3cbc17f5f..5735b52a1a8 100644 --- a/docs/reference/modules/cluster/allocation_awareness.asciidoc +++ b/docs/reference/modules/cluster/allocation_awareness.asciidoc @@ -21,7 +21,7 @@ attribute called `rack_id` -- we could use any attribute name. For example: [source,sh] ---------------------- -./bin/elasticsearch --node.rack_id rack_one <1> +./bin/elasticsearch -Ees.node.rack_id=rack_one <1> ---------------------- <1> This setting could also be specified in the `elasticsearch.yml` config file. diff --git a/docs/reference/modules/node.asciidoc b/docs/reference/modules/node.asciidoc index 0117d193043..f3da7787667 100644 --- a/docs/reference/modules/node.asciidoc +++ b/docs/reference/modules/node.asciidoc @@ -233,7 +233,7 @@ Like all node settings, it can also be specified on the command line as: [source,sh] ----------------------- -./bin/elasticsearch --path.data /var/elasticsearch/data +./bin/elasticsearch -Ees.path.data=/var/elasticsearch/data ----------------------- TIP: When using the `.zip` or `.tar.gz` distributions, the `path.data` setting diff --git a/docs/reference/setup.asciidoc b/docs/reference/setup.asciidoc index 15f23e6fe1e..f0bf03985b0 100644 --- a/docs/reference/setup.asciidoc +++ b/docs/reference/setup.asciidoc @@ -67,13 +67,12 @@ There are added features when using the `elasticsearch` shell script. The first, which was explained earlier, is the ability to easily run the process either in the foreground or the background. -Another feature is the ability to pass `-D` or getopt long style -configuration parameters directly to the script. When set, all override -anything set using either `JAVA_OPTS` or `ES_JAVA_OPTS`. For example: +Another feature is the ability to pass `-E` configuration parameters +directly to the script. For example: [source,sh] -------------------------------------------------- -$ bin/elasticsearch -Des.index.refresh_interval=5s --node.name=my-node +$ bin/elasticsearch -Ees.index.refresh_interval=5s -Ees.node.name=my-node -------------------------------------------------- ************************************************************************* diff --git a/docs/reference/setup/configuration.asciidoc b/docs/reference/setup/configuration.asciidoc index bef563cd965..1a687f15fb9 100644 --- a/docs/reference/setup/configuration.asciidoc +++ b/docs/reference/setup/configuration.asciidoc @@ -259,7 +259,7 @@ command, for example: [source,sh] -------------------------------------------------- -$ elasticsearch -Des.network.host=10.0.0.4 +$ elasticsearch -Ees.network.host=10.0.0.4 -------------------------------------------------- Another option is to set `es.default.` prefix instead of `es.` prefix, @@ -336,7 +336,7 @@ course, the above can also be set as a "collapsed" setting, for example: [source,sh] -------------------------------------------------- -$ elasticsearch -Des.index.refresh_interval=5s +$ elasticsearch -Ees.index.refresh_interval=5s -------------------------------------------------- All of the index level configuration can be found within each diff --git a/docs/reference/setup/rolling_upgrade.asciidoc b/docs/reference/setup/rolling_upgrade.asciidoc index b3c00d337f8..cb9073b558e 100644 --- a/docs/reference/setup/rolling_upgrade.asciidoc +++ b/docs/reference/setup/rolling_upgrade.asciidoc @@ -80,7 +80,7 @@ To upgrade using a zip or compressed tarball: overwrite the `config` or `data` directories. * Either copy the files in the `config` directory from your old installation - to your new installation, or use the `--path.conf` option on the command + to your new installation, or use the `-E path.conf=` option on the command line to point to an external config directory. * Either copy the files in the `data` directory from your old installation diff --git a/modules/lang-groovy/build.gradle b/modules/lang-groovy/build.gradle index 2160210ba73..340dd620ca6 100644 --- a/modules/lang-groovy/build.gradle +++ b/modules/lang-groovy/build.gradle @@ -28,8 +28,8 @@ dependencies { integTest { cluster { - systemProperty 'es.script.inline', 'true' - systemProperty 'es.script.indexed', 'true' + esSetting 'es.script.inline', 'true' + esSetting 'es.script.indexed', 'true' } } diff --git a/modules/lang-mustache/build.gradle b/modules/lang-mustache/build.gradle index f41ffb90128..36b58792d86 100644 --- a/modules/lang-mustache/build.gradle +++ b/modules/lang-mustache/build.gradle @@ -28,7 +28,7 @@ dependencies { integTest { cluster { - systemProperty 'es.script.inline', 'true' - systemProperty 'es.script.indexed', 'true' + esSetting 'es.script.inline', 'true' + esSetting 'es.script.indexed', 'true' } } diff --git a/plugins/lang-javascript/build.gradle b/plugins/lang-javascript/build.gradle index dae5204db20..41d85824318 100644 --- a/plugins/lang-javascript/build.gradle +++ b/plugins/lang-javascript/build.gradle @@ -28,7 +28,7 @@ dependencies { integTest { cluster { - systemProperty 'es.script.inline', 'true' - systemProperty 'es.script.indexed', 'true' + esSetting 'es.script.inline', 'true' + esSetting 'es.script.indexed', 'true' } } diff --git a/plugins/lang-python/build.gradle b/plugins/lang-python/build.gradle index 0980d7f62c9..bc9db2a20c2 100644 --- a/plugins/lang-python/build.gradle +++ b/plugins/lang-python/build.gradle @@ -28,8 +28,8 @@ dependencies { integTest { cluster { - systemProperty 'es.script.inline', 'true' - systemProperty 'es.script.indexed', 'true' + esSetting 'es.script.inline', 'true' + esSetting 'es.script.indexed', 'true' } } diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java deleted file mode 100644 index fc7504fc97f..00000000000 --- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java +++ /dev/null @@ -1,164 +0,0 @@ -/* - * 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.bootstrap; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Locale; -import java.util.Map; - -import joptsimple.OptionException; -import org.elasticsearch.Build; -import org.elasticsearch.Version; -import org.elasticsearch.cli.Command; -import org.elasticsearch.cli.CommandTestCase; -import org.elasticsearch.cli.UserError; -import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.monitor.jvm.JvmInfo; -import org.junit.After; -import org.junit.Before; - -import static org.hamcrest.Matchers.is; - -@SuppressForbidden(reason = "modifies system properties intentionally") -public class BootstrapCliParserTests extends CommandTestCase { - - @Override - protected Command newCommand() { - return new BootstrapCliParser(); - } - - private List propertiesToClear = new ArrayList<>(); - private Map properties; - - @Before - public void before() { - this.properties = new HashMap<>(System.getProperties()); - } - - @After - public void clearProperties() { - for (String property : propertiesToClear) { - System.clearProperty(property); - } - propertiesToClear.clear(); - assertEquals("properties leaked", properties, new HashMap<>(System.getProperties())); - } - - void assertShouldRun(boolean shouldRun) { - BootstrapCliParser parser = (BootstrapCliParser)command; - assertEquals(shouldRun, parser.shouldRun()); - } - - public void testVersion() throws Exception { - String output = execute("-V"); - assertTrue(output, output.contains(Version.CURRENT.toString())); - assertTrue(output, output.contains(Build.CURRENT.shortHash())); - assertTrue(output, output.contains(Build.CURRENT.date())); - assertTrue(output, output.contains(JvmInfo.jvmInfo().version())); - assertShouldRun(false); - - terminal.reset(); - output = execute("--version"); - assertTrue(output, output.contains(Version.CURRENT.toString())); - assertTrue(output, output.contains(Build.CURRENT.shortHash())); - assertTrue(output, output.contains(Build.CURRENT.date())); - assertTrue(output, output.contains(JvmInfo.jvmInfo().version())); - assertShouldRun(false); - } - - public void testPidfile() throws Exception { - registerProperties("es.pidfile"); - - // missing argument - OptionException e = expectThrows(OptionException.class, () -> { - execute("-p"); - }); - assertEquals("Option p/pidfile requires an argument", e.getMessage()); - assertShouldRun(false); - - // good cases - terminal.reset(); - execute("--pidfile", "/tmp/pid"); - assertSystemProperty("es.pidfile", "/tmp/pid"); - assertShouldRun(true); - - System.clearProperty("es.pidfile"); - terminal.reset(); - execute("-p", "/tmp/pid"); - assertSystemProperty("es.pidfile", "/tmp/pid"); - assertShouldRun(true); - } - - public void testNoDaemonize() throws Exception { - registerProperties("es.foreground"); - - execute(); - assertSystemProperty("es.foreground", null); - assertShouldRun(true); - } - - public void testDaemonize() throws Exception { - registerProperties("es.foreground"); - - execute("-d"); - assertSystemProperty("es.foreground", "false"); - assertShouldRun(true); - - System.clearProperty("es.foreground"); - execute("--daemonize"); - assertSystemProperty("es.foreground", "false"); - assertShouldRun(true); - } - - public void testConfig() throws Exception { - registerProperties("es.foo", "es.spam"); - - execute("-Dfoo=bar", "-Dspam=eggs"); - assertSystemProperty("es.foo", "bar"); - assertSystemProperty("es.spam", "eggs"); - assertShouldRun(true); - } - - public void testConfigMalformed() throws Exception { - UserError e = expectThrows(UserError.class, () -> { - execute("-Dfoo"); - }); - assertTrue(e.getMessage(), e.getMessage().contains("Malformed elasticsearch setting")); - } - - public void testUnknownOption() throws Exception { - OptionException e = expectThrows(OptionException.class, () -> { - execute("--network.host"); - }); - assertTrue(e.getMessage(), e.getMessage().contains("network.host is not a recognized option")); - } - - private void registerProperties(String ... systemProperties) { - propertiesToClear.addAll(Arrays.asList(systemProperties)); - } - - private void assertSystemProperty(String name, String expectedValue) throws Exception { - String msg = String.format(Locale.ROOT, "Expected property %s to be %s, terminal output was %s", name, expectedValue, terminal.getOutput()); - assertThat(msg, System.getProperty(name), is(expectedValue)); - } -} diff --git a/qa/smoke-test-ingest-disabled/build.gradle b/qa/smoke-test-ingest-disabled/build.gradle index ca71697a7b4..f8ebd631786 100644 --- a/qa/smoke-test-ingest-disabled/build.gradle +++ b/qa/smoke-test-ingest-disabled/build.gradle @@ -21,6 +21,6 @@ apply plugin: 'elasticsearch.rest-test' integTest { cluster { - systemProperty 'es.node.ingest', 'false' + esSetting 'es.node.ingest', 'false' } } diff --git a/qa/smoke-test-reindex-with-groovy/build.gradle b/qa/smoke-test-reindex-with-groovy/build.gradle index a42f5e708a2..749f5c1237c 100644 --- a/qa/smoke-test-reindex-with-groovy/build.gradle +++ b/qa/smoke-test-reindex-with-groovy/build.gradle @@ -21,6 +21,6 @@ apply plugin: 'elasticsearch.rest-test' integTest { cluster { - systemProperty 'es.script.inline', 'true' + esSetting 'es.script.inline', 'true' } } diff --git a/qa/vagrant/src/test/resources/packaging/scripts/packaging_test_utils.bash b/qa/vagrant/src/test/resources/packaging/scripts/packaging_test_utils.bash index 11961e06921..852d03ea6f6 100644 --- a/qa/vagrant/src/test/resources/packaging/scripts/packaging_test_utils.bash +++ b/qa/vagrant/src/test/resources/packaging/scripts/packaging_test_utils.bash @@ -303,7 +303,7 @@ run_elasticsearch_service() { # This line is attempting to emulate the on login behavior of /usr/share/upstart/sessions/jayatana.conf [ -f /usr/share/java/jayatanaag.jar ] && export JAVA_TOOL_OPTIONS="-javaagent:/usr/share/java/jayatanaag.jar" # And now we can start Elasticsearch normally, in the background (-d) and with a pidfile (-p). -$timeoutCommand/tmp/elasticsearch/bin/elasticsearch $background -p /tmp/elasticsearch/elasticsearch.pid -Des.path.conf=$CONF_DIR $commandLineArgs +$timeoutCommand/tmp/elasticsearch/bin/elasticsearch $background -p /tmp/elasticsearch/elasticsearch.pid -Ees.path.conf=$CONF_DIR $commandLineArgs BASH [ "$status" -eq "$expectedStatus" ] elif is_systemd; then diff --git a/qa/vagrant/src/test/resources/packaging/scripts/plugin_test_cases.bash b/qa/vagrant/src/test/resources/packaging/scripts/plugin_test_cases.bash index c81d850d94d..e829141def0 100644 --- a/qa/vagrant/src/test/resources/packaging/scripts/plugin_test_cases.bash +++ b/qa/vagrant/src/test/resources/packaging/scripts/plugin_test_cases.bash @@ -102,7 +102,7 @@ fi echo "CONF_FILE=$CONF_FILE" >> /etc/sysconfig/elasticsearch; fi - run_elasticsearch_service 1 -Des.default.config="$CONF_FILE" + run_elasticsearch_service 1 -Ees.default.config="$CONF_FILE" # remove settings again otherwise cleaning up before next testrun will fail if is_dpkg ; then @@ -408,7 +408,7 @@ fi remove_jvm_example local relativePath=${1:-$(readlink -m jvm-example-*.zip)} - sudo -E -u $ESPLUGIN_COMMAND_USER "$ESHOME/bin/elasticsearch-plugin" install "file://$relativePath" -Des.logger.level=DEBUG > /tmp/plugin-cli-output + sudo -E -u $ESPLUGIN_COMMAND_USER "$ESHOME/bin/elasticsearch-plugin" install "file://$relativePath" -Ees.logger.level=DEBUG > /tmp/plugin-cli-output local loglines=$(cat /tmp/plugin-cli-output | wc -l) if [ "$GROUP" == "TAR PLUGINS" ]; then [ "$loglines" -gt "7" ] || { diff --git a/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java b/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java deleted file mode 100644 index 576ecf2d1ee..00000000000 --- a/test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java +++ /dev/null @@ -1,65 +0,0 @@ -/* - * 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.common.cli; - -import java.io.IOException; - -import org.elasticsearch.cli.MockTerminal; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.SuppressForbidden; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.StreamsUtils; -import org.junit.After; -import org.junit.Before; - -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.isEmptyString; -import static org.hamcrest.Matchers.not; - -public abstract class CliToolTestCase extends ESTestCase { - - @Before - @SuppressForbidden(reason = "sets es.default.path.home during tests") - public void setPathHome() { - System.setProperty("es.default.path.home", createTempDir().toString()); - } - - @After - @SuppressForbidden(reason = "clears es.default.path.home during tests") - public void clearPathHome() { - System.clearProperty("es.default.path.home"); - } - - public static String[] args(String command) { - if (!Strings.hasLength(command)) { - return Strings.EMPTY_ARRAY; - } - return command.split("\\s+"); - } - - public static void assertTerminalOutputContainsHelpFile(MockTerminal terminal, String classPath) throws IOException { - String output = terminal.getOutput(); - assertThat(output, not(isEmptyString())); - String expectedDocs = StreamsUtils.copyToStringFromClasspath(classPath); - // convert to *nix newlines as MockTerminal used for tests also uses *nix newlines - expectedDocs = expectedDocs.replace("\r\n", "\n"); - assertThat(output, containsString(expectedDocs)); - } -} From fac0f990fcbea296871ac4e6092502fcb62e09b4 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 15 Mar 2016 09:27:01 -0400 Subject: [PATCH 2/8] Rename "daemonize" to "foreground" This commit renames the Bootstrap#init parameter "daemonize" to "foreground" for clarity. --- .../org/elasticsearch/bootstrap/Bootstrap.java | 16 ++++++++-------- .../elasticsearch/bootstrap/Elasticsearch.java | 2 +- .../ElasticsearchCommandLineParsingTests.java | 18 +++++++++--------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index 3ad592af635..a89bee0f4ac 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -188,8 +188,8 @@ final class Bootstrap { node = new Node(nodeSettings); } - private static Environment initialSettings(boolean daemonize, String pathHome, String pidFile) { - Terminal terminal = daemonize ? null : Terminal.DEFAULT; + private static Environment initialSettings(boolean foreground, String pathHome, String pidFile) { + Terminal terminal = foreground ? Terminal.DEFAULT : null; Settings.Builder builder = Settings.builder(); builder.put(Environment.PATH_HOME_SETTING.getKey(), pathHome); if (Strings.hasLength(pidFile)) { @@ -223,7 +223,7 @@ final class Bootstrap { * to startup elasticsearch. */ static void init( - final boolean daemonize, + final boolean foreground, final String pathHome, final String pidFile, final Map esSettings) throws Throwable { @@ -234,7 +234,7 @@ final class Bootstrap { INSTANCE = new Bootstrap(); - Environment environment = initialSettings(daemonize, pathHome, pidFile); + Environment environment = initialSettings(foreground, pathHome, pidFile); Settings settings = environment.settings(); LogConfigurator.configure(settings, true); checkForCustomConfFile(); @@ -250,7 +250,7 @@ final class Bootstrap { } try { - if (daemonize) { + if (!foreground) { Loggers.disableConsoleLogging(); closeSystOut(); } @@ -265,12 +265,12 @@ final class Bootstrap { INSTANCE.start(); - if (daemonize) { + if (!foreground) { closeSysError(); } } catch (Throwable e) { // disable console logging, so user does not see the exception twice (jvm will show it already) - if (!daemonize) { + if (foreground) { Loggers.disableConsoleLogging(); } ESLogger logger = Loggers.getLogger(Bootstrap.class); @@ -290,7 +290,7 @@ final class Bootstrap { logger.error("Exception", e); } // re-enable it if appropriate, so they can see any logging during the shutdown process - if (!daemonize) { + if (foreground) { Loggers.enableConsoleLogging(); } diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index dfe49c52e98..f492f06cd5a 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -109,7 +109,7 @@ class Elasticsearch extends Command { void init(final boolean daemonize, final String pathHome, final String pidFile, final Map esSettings) { try { - Bootstrap.init(daemonize, pathHome, pidFile, esSettings); + Bootstrap.init(!daemonize, pathHome, pidFile, esSettings); } catch (final Throwable t) { // 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/ElasticsearchCommandLineParsingTests.java b/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCommandLineParsingTests.java index 0d70cb8fba5..62603412a2c 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCommandLineParsingTests.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCommandLineParsingTests.java @@ -77,7 +77,7 @@ public class ElasticsearchCommandLineParsingTests extends ESTestCase { } private void runTestVersion(int expectedStatus, Consumer outputConsumer, String... args) throws Exception { - runTest(expectedStatus, false, outputConsumer, (daemonize, pathHome, pidFile, esSettings) -> {}, args); + runTest(expectedStatus, false, outputConsumer, (foreground, pathHome, pidFile, esSettings) -> {}, args); } public void testThatPidFileCanBeConfigured() throws Exception { @@ -92,7 +92,7 @@ public class ElasticsearchCommandLineParsingTests extends ESTestCase { expectedStatus, expectedInit, outputConsumer, - (daemonize, pathHome, pidFile, esSettings) -> assertThat(pidFile, equalTo("/tmp/pid")), + (foreground, pathHome, pidFile, esSettings) -> assertThat(pidFile, equalTo("/tmp/pid")), args); } @@ -107,7 +107,7 @@ public class ElasticsearchCommandLineParsingTests extends ESTestCase { ExitCodes.OK, true, output -> {}, - (daemonize, pathHome, pidFile, esSettings) -> assertThat(daemonize, equalTo(expectedDaemonize)), + (foreground, pathHome, pidFile, esSettings) -> assertThat(foreground, equalTo(!expectedDaemonize)), args); } @@ -116,7 +116,7 @@ public class ElasticsearchCommandLineParsingTests extends ESTestCase { ExitCodes.OK, true, output -> {}, - (daemonize, pathHome, pidFile, esSettings) -> { + (foreground, pathHome, pidFile, esSettings) -> { assertThat(esSettings.size(), equalTo(2)); assertThat(esSettings, hasEntry("es.foo", "bar")); assertThat(esSettings, hasEntry("es.baz", "qux")); @@ -136,7 +136,7 @@ public class ElasticsearchCommandLineParsingTests extends ESTestCase { ExitCodes.USAGE, false, output -> assertThat(output, containsString("Elasticsearch settings must be prefixed with [es.] but was [")), - (daemonize, pathHome, pidFile, esSettings) -> {}, + (foreground, pathHome, pidFile, esSettings) -> {}, args ); } @@ -146,7 +146,7 @@ public class ElasticsearchCommandLineParsingTests extends ESTestCase { ExitCodes.USAGE, false, output -> assertThat(output, containsString("Elasticsearch setting [es.foo] must not be empty")), - (daemonize, pathHome, pidFile, esSettings) -> {}, + (foreground, pathHome, pidFile, esSettings) -> {}, "-E", "es.foo=" ); } @@ -156,12 +156,12 @@ public class ElasticsearchCommandLineParsingTests extends ESTestCase { ExitCodes.USAGE, false, output -> assertThat(output, containsString("network.host is not a recognized option")), - (daemonize, pathHome, pidFile, esSettings) -> {}, + (foreground, pathHome, pidFile, esSettings) -> {}, "--network.host"); } private interface InitConsumer { - void accept(final boolean daemonize, final String pathHome, final String pidFile, final Map esSettings); + void accept(final boolean foreground, final String pathHome, final String pidFile, final Map esSettings); } private void runTest( @@ -177,7 +177,7 @@ public class ElasticsearchCommandLineParsingTests extends ESTestCase { @Override void init(final boolean daemonize, final String pathHome, final String pidFile, final Map esSettings) { init.set(true); - initConsumer.accept(daemonize, pathHome, pidFile, esSettings); + initConsumer.accept(!daemonize, pathHome, pidFile, esSettings); } }, terminal); assertThat(status, equalTo(expectedStatus)); From 088dea8863339639710431a44cd7f7b15f8e79e8 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 15 Mar 2016 09:35:17 -0400 Subject: [PATCH 3/8] Fix javadoc comment on Elasticsearch#init This commit fixes a stale javadoc comment on Elasticsearch#init; the method is now visible for testing. --- .../main/java/org/elasticsearch/bootstrap/Elasticsearch.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index f492f06cd5a..d15f72e8655 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -45,7 +45,7 @@ class Elasticsearch extends Command { private final OptionSpec pidfileOption; private final OptionSpec propertyOption; - /** no instantiation */ + // visible for testing Elasticsearch() { super("starts elasticsearch"); // TODO: in jopt-simple 5.0, make this mutually exclusive with all other options From 2f7e181318192966ca7ebab8c6e36ba942533d3f Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 15 Mar 2016 10:05:28 -0400 Subject: [PATCH 4/8] Fix typo inadvertently introduced --- docs/reference/migration/migrate_5_0/settings.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/migration/migrate_5_0/settings.asciidoc b/docs/reference/migration/migrate_5_0/settings.asciidoc index 5d39f87773d..87ea356ec7a 100644 --- a/docs/reference/migration/migrate_5_0/settings.asciidoc +++ b/docs/reference/migration/migrate_5_0/settings.asciidoc @@ -153,7 +153,7 @@ on startup if it is set too low. ==== Removed es.netty.gathering -Disabling Netty from using NIO gathring could be done via the escape +Disabling Netty from using NIO gathering could be done via the escape hatch of setting the system property "es.netty.gathering" to "false". Time has proven enabling gathering by default is a non-issue and this non-documented setting has been removed. From d1b85f69ef469812693b26393c1519618793cf2f Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 15 Mar 2016 11:22:53 -0400 Subject: [PATCH 5/8] Shorter name for test class This commit renames the ElasticsearchCommandLineParsingTests to ElasticsearchCliTests. --- ...hCommandLineParsingTests.java => ElasticsearchCliTests.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename core/src/test/java/org/elasticsearch/bootstrap/{ElasticsearchCommandLineParsingTests.java => ElasticsearchCliTests.java} (99%) diff --git a/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCommandLineParsingTests.java b/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java similarity index 99% rename from core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCommandLineParsingTests.java rename to core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java index 62603412a2c..fe1eeb8b5a6 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCommandLineParsingTests.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java @@ -40,7 +40,7 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.Matchers.hasEntry; -public class ElasticsearchCommandLineParsingTests extends ESTestCase { +public class ElasticsearchCliTests extends ESTestCase { public void testVersion() throws Exception { runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "-d"); From 5994e91b084c99fea415713133adeed13d865cad Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 15 Mar 2016 15:47:27 -0400 Subject: [PATCH 6/8] Fix systemd pidfile setting This commit fixes the pidfile setting on systems that used systemd. The issue is that the pidfile can only be set via the command line arguments -p or --pidfile, and is no longer settable via a setting. --- distribution/src/main/packaging/systemd/elasticsearch.service | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/src/main/packaging/systemd/elasticsearch.service b/distribution/src/main/packaging/systemd/elasticsearch.service index 4f643f6a4a4..6aa6efeadde 100644 --- a/distribution/src/main/packaging/systemd/elasticsearch.service +++ b/distribution/src/main/packaging/systemd/elasticsearch.service @@ -20,7 +20,7 @@ Group=elasticsearch ExecStartPre=/usr/share/elasticsearch/bin/elasticsearch-systemd-pre-exec ExecStart=/usr/share/elasticsearch/bin/elasticsearch \ - -Ees.pidfile=${PID_DIR}/elasticsearch.pid \ + -p ${PID_DIR}/elasticsearch.pid \ -Ees.default.path.home=${ES_HOME} \ -Ees.default.path.logs=${LOG_DIR} \ -Ees.default.path.data=${DATA_DIR} \ From 4ee90db13dc54744a9eb7b5d56df2ea4238014d8 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 15 Mar 2016 16:29:29 -0400 Subject: [PATCH 7/8] Remove path.home command-line setting --- .../elasticsearch/bootstrap/Bootstrap.java | 6 ++--- .../bootstrap/Elasticsearch.java | 12 ++++------ .../elasticsearch/bootstrap/security.policy | 3 --- .../bootstrap/ElasticsearchCliTests.java | 24 ++++++++----------- .../src/main/packaging/init.d/elasticsearch | 2 +- .../packaging/systemd/elasticsearch.service | 1 - .../src/main/resources/bin/elasticsearch | 8 +++---- .../src/main/resources/bin/service.bat | 2 +- 8 files changed, 22 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index a89bee0f4ac..2a8984e59d4 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -188,10 +188,9 @@ final class Bootstrap { node = new Node(nodeSettings); } - private static Environment initialSettings(boolean foreground, String pathHome, String pidFile) { + private static Environment initialSettings(boolean foreground, String pidFile) { Terminal terminal = foreground ? Terminal.DEFAULT : null; Settings.Builder builder = Settings.builder(); - builder.put(Environment.PATH_HOME_SETTING.getKey(), pathHome); if (Strings.hasLength(pidFile)) { builder.put(Environment.PIDFILE_SETTING.getKey(), pidFile); } @@ -224,7 +223,6 @@ final class Bootstrap { */ static void init( final boolean foreground, - final String pathHome, final String pidFile, final Map esSettings) throws Throwable { // Set the system property before anything has a chance to trigger its use @@ -234,7 +232,7 @@ final class Bootstrap { INSTANCE = new Bootstrap(); - Environment environment = initialSettings(foreground, pathHome, pidFile); + Environment environment = initialSettings(foreground, pidFile); Settings settings = environment.settings(); LogConfigurator.configure(settings, true); checkForCustomConfFile(); diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index d15f72e8655..0cc952907c0 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -41,7 +41,6 @@ class Elasticsearch extends Command { private final OptionSpec versionOption; private final OptionSpec daemonizeOption; - private final OptionSpec pathHomeOption; private final OptionSpec pidfileOption; private final OptionSpec propertyOption; @@ -54,8 +53,6 @@ class Elasticsearch extends Command { daemonizeOption = parser.acceptsAll(Arrays.asList("d", "daemonize"), "Starts Elasticsearch in the background"); // TODO: in jopt-simple 5.0 this option type can be a Path - pathHomeOption = parser.acceptsAll(Arrays.asList("H", "path.home"), "").withRequiredArg(); - // TODO: in jopt-simple 5.0 this option type can be a Path pidfileOption = parser.acceptsAll(Arrays.asList("p", "pidfile"), "Creates a pid file in the specified path on start") .withRequiredArg(); @@ -80,7 +77,7 @@ class Elasticsearch extends Command { @Override protected void execute(Terminal terminal, OptionSet options) throws Exception { if (options.has(versionOption)) { - if (options.has(daemonizeOption) || options.has(pathHomeOption) || options.has(pidfileOption)) { + if (options.has(daemonizeOption) || options.has(pidfileOption)) { throw new UserError(ExitCodes.USAGE, "Elasticsearch version option is mutually exclusive with any other option"); } terminal.println("Version: " + org.elasticsearch.Version.CURRENT @@ -90,7 +87,6 @@ class Elasticsearch extends Command { } final boolean daemonize = options.has(daemonizeOption); - final String pathHome = pathHomeOption.value(options); final String pidFile = pidfileOption.value(options); final Map esSettings = new HashMap<>(); @@ -104,12 +100,12 @@ class Elasticsearch extends Command { esSettings.put(kvp.key, kvp.value); } - init(daemonize, pathHome, pidFile, esSettings); + init(daemonize, pidFile, esSettings); } - void init(final boolean daemonize, final String pathHome, final String pidFile, final Map esSettings) { + void init(final boolean daemonize, final String pidFile, final Map esSettings) { try { - Bootstrap.init(!daemonize, pathHome, pidFile, esSettings); + Bootstrap.init(!daemonize, pidFile, esSettings); } catch (final Throwable t) { // format exceptions to the console in a special way // to avoid 2MB stacktraces from guice, etc. diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy index 4909959015b..10bf8b2a158 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/security.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/security.policy @@ -72,9 +72,6 @@ grant { // set by ESTestCase to improve test reproducibility // TODO: set this with gradle or some other way that repros with seed? permission java.util.PropertyPermission "es.processors.override", "write"; - // set by CLIToolTestCase - // TODO: do this differently? or test commandline tools differently? - permission java.util.PropertyPermission "es.default.path.home", "write"; // TODO: these simply trigger a noisy warning if its unable to clear the properties // fix that in randomizedtesting diff --git a/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java b/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java index fe1eeb8b5a6..51274af9a01 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java @@ -45,14 +45,10 @@ public class ElasticsearchCliTests extends ESTestCase { public void testVersion() throws Exception { runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "-d"); runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "--daemonize"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "-H", "/tmp/home"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "--path.home", "/tmp/home"); runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "-p", "/tmp/pid"); runTestThatVersionIsMutuallyExclusiveToOtherOptions("-V", "--pidfile", "/tmp/pid"); runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-d"); runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--daemonize"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-H", "/tmp/home"); - runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--path.home", "/tmp/home"); runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "-p", "/tmp/pid"); runTestThatVersionIsMutuallyExclusiveToOtherOptions("--version", "--pidfile", "/tmp/pid"); runTestThatVersionIsReturned("-V"); @@ -77,7 +73,7 @@ public class ElasticsearchCliTests extends ESTestCase { } private void runTestVersion(int expectedStatus, Consumer outputConsumer, String... args) throws Exception { - runTest(expectedStatus, false, outputConsumer, (foreground, pathHome, pidFile, esSettings) -> {}, args); + runTest(expectedStatus, false, outputConsumer, (foreground, pidFile, esSettings) -> {}, args); } public void testThatPidFileCanBeConfigured() throws Exception { @@ -92,7 +88,7 @@ public class ElasticsearchCliTests extends ESTestCase { expectedStatus, expectedInit, outputConsumer, - (foreground, pathHome, pidFile, esSettings) -> assertThat(pidFile, equalTo("/tmp/pid")), + (foreground, pidFile, esSettings) -> assertThat(pidFile, equalTo("/tmp/pid")), args); } @@ -107,7 +103,7 @@ public class ElasticsearchCliTests extends ESTestCase { ExitCodes.OK, true, output -> {}, - (foreground, pathHome, pidFile, esSettings) -> assertThat(foreground, equalTo(!expectedDaemonize)), + (foreground, pidFile, esSettings) -> assertThat(foreground, equalTo(!expectedDaemonize)), args); } @@ -116,7 +112,7 @@ public class ElasticsearchCliTests extends ESTestCase { ExitCodes.OK, true, output -> {}, - (foreground, pathHome, pidFile, esSettings) -> { + (foreground, pidFile, esSettings) -> { assertThat(esSettings.size(), equalTo(2)); assertThat(esSettings, hasEntry("es.foo", "bar")); assertThat(esSettings, hasEntry("es.baz", "qux")); @@ -136,7 +132,7 @@ public class ElasticsearchCliTests extends ESTestCase { ExitCodes.USAGE, false, output -> assertThat(output, containsString("Elasticsearch settings must be prefixed with [es.] but was [")), - (foreground, pathHome, pidFile, esSettings) -> {}, + (foreground, pidFile, esSettings) -> {}, args ); } @@ -146,7 +142,7 @@ public class ElasticsearchCliTests extends ESTestCase { ExitCodes.USAGE, false, output -> assertThat(output, containsString("Elasticsearch setting [es.foo] must not be empty")), - (foreground, pathHome, pidFile, esSettings) -> {}, + (foreground, pidFile, esSettings) -> {}, "-E", "es.foo=" ); } @@ -156,12 +152,12 @@ public class ElasticsearchCliTests extends ESTestCase { ExitCodes.USAGE, false, output -> assertThat(output, containsString("network.host is not a recognized option")), - (foreground, pathHome, pidFile, esSettings) -> {}, + (foreground, pidFile, esSettings) -> {}, "--network.host"); } private interface InitConsumer { - void accept(final boolean foreground, final String pathHome, final String pidFile, final Map esSettings); + void accept(final boolean foreground, final String pidFile, final Map esSettings); } private void runTest( @@ -175,9 +171,9 @@ public class ElasticsearchCliTests extends ESTestCase { final AtomicBoolean init = new AtomicBoolean(); final int status = Elasticsearch.main(args, new Elasticsearch() { @Override - void init(final boolean daemonize, final String pathHome, final String pidFile, final Map esSettings) { + void init(final boolean daemonize, final String pidFile, final Map esSettings) { init.set(true); - initConsumer.accept(!daemonize, pathHome, pidFile, esSettings); + initConsumer.accept(!daemonize, pidFile, esSettings); } }, terminal); assertThat(status, equalTo(expectedStatus)); diff --git a/distribution/deb/src/main/packaging/init.d/elasticsearch b/distribution/deb/src/main/packaging/init.d/elasticsearch index e2d857a7ffe..1476a520c1d 100755 --- a/distribution/deb/src/main/packaging/init.d/elasticsearch +++ b/distribution/deb/src/main/packaging/init.d/elasticsearch @@ -99,7 +99,7 @@ fi # Define other required variables PID_FILE="$PID_DIR/$NAME.pid" DAEMON=$ES_HOME/bin/elasticsearch -DAEMON_OPTS="-d -p $PID_FILE -Ees.default.path.home=$ES_HOME -Ees.default.path.logs=$LOG_DIR -Ees.default.path.data=$DATA_DIR -Ees.default.path.conf=$CONF_DIR" +DAEMON_OPTS="-d -p $PID_FILE -Ees.default.path.logs=$LOG_DIR -Ees.default.path.data=$DATA_DIR -Ees.default.path.conf=$CONF_DIR" export ES_HEAP_SIZE export ES_HEAP_NEWSIZE diff --git a/distribution/src/main/packaging/systemd/elasticsearch.service b/distribution/src/main/packaging/systemd/elasticsearch.service index 6aa6efeadde..1aed30ac968 100644 --- a/distribution/src/main/packaging/systemd/elasticsearch.service +++ b/distribution/src/main/packaging/systemd/elasticsearch.service @@ -21,7 +21,6 @@ ExecStartPre=/usr/share/elasticsearch/bin/elasticsearch-systemd-pre-exec ExecStart=/usr/share/elasticsearch/bin/elasticsearch \ -p ${PID_DIR}/elasticsearch.pid \ - -Ees.default.path.home=${ES_HOME} \ -Ees.default.path.logs=${LOG_DIR} \ -Ees.default.path.data=${DATA_DIR} \ -Ees.default.path.conf=${CONF_DIR} diff --git a/distribution/src/main/resources/bin/elasticsearch b/distribution/src/main/resources/bin/elasticsearch index 0d0e0069ae2..253ee1ee1f5 100755 --- a/distribution/src/main/resources/bin/elasticsearch +++ b/distribution/src/main/resources/bin/elasticsearch @@ -126,11 +126,11 @@ export HOSTNAME # manual parsing to find out, if process should be detached daemonized=`echo $* | egrep -- '(^-d |-d$| -d |--daemonize$|--daemonize )'` if [ -z "$daemonized" ] ; then - exec "$JAVA" $JAVA_OPTS $ES_JAVA_OPTS -cp "$ES_CLASSPATH" \ - org.elasticsearch.bootstrap.Elasticsearch --path.home "$ES_HOME" "$@" + exec "$JAVA" $JAVA_OPTS $ES_JAVA_OPTS -Des.path.home="$ES_HOME" -cp "$ES_CLASSPATH" \ + org.elasticsearch.bootstrap.Elasticsearch "$@" else - exec "$JAVA" $JAVA_OPTS $ES_JAVA_OPTS -cp "$ES_CLASSPATH" \ - org.elasticsearch.bootstrap.Elasticsearch --path.home "$ES_HOME" "$@" <&- & + exec "$JAVA" $JAVA_OPTS $ES_JAVA_OPTS -Des.path.home="$ES_HOME" -cp "$ES_CLASSPATH" \ + org.elasticsearch.bootstrap.Elasticsearch "$@" <&- & retval=$? pid=$! [ $retval -eq 0 ] || exit $retval diff --git a/distribution/src/main/resources/bin/service.bat b/distribution/src/main/resources/bin/service.bat index 22242e36ff9..2786c87a634 100644 --- a/distribution/src/main/resources/bin/service.bat +++ b/distribution/src/main/resources/bin/service.bat @@ -152,7 +152,7 @@ if "%DATA_DIR%" == "" set DATA_DIR=%ES_HOME%\data if "%CONF_DIR%" == "" set CONF_DIR=%ES_HOME%\config -set ES_PARAMS=-Delasticsearch;-Des.path.home="%ES_HOME%";-Des.default.path.home="%ES_HOME%";-Des.default.path.logs="%LOG_DIR%";-Des.default.path.data="%DATA_DIR%";-Des.default.path.conf="%CONF_DIR%" +set ES_PARAMS=-Delasticsearch;-Des.path.home="%ES_HOME%";-Des.default.path.logs="%LOG_DIR%";-Des.default.path.data="%DATA_DIR%";-Des.default.path.conf="%CONF_DIR%" set JVM_OPTS=%JAVA_OPTS: =;% From 66ba044ec569f76b6e2b03bfe7ba0912c44707df Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 15 Mar 2016 17:45:17 -0400 Subject: [PATCH 8/8] Use setting in integration test cluster config --- .../elasticsearch/gradle/test/ClusterConfiguration.groovy | 7 ------- .../groovy/org/elasticsearch/gradle/test/NodeInfo.groovy | 1 - modules/lang-groovy/build.gradle | 4 ++-- modules/lang-mustache/build.gradle | 4 ++-- plugins/lang-javascript/build.gradle | 4 ++-- plugins/lang-python/build.gradle | 4 ++-- qa/smoke-test-ingest-disabled/build.gradle | 2 +- qa/smoke-test-reindex-with-groovy/build.gradle | 2 +- 8 files changed, 10 insertions(+), 18 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy index 2adc59e9e9d..3e8b6225329 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy @@ -73,8 +73,6 @@ class ClusterConfiguration { return tmpFile.exists() } - Map esSettings = new HashMap<>(); - Map systemProperties = new HashMap<>() Map settings = new HashMap<>() @@ -88,11 +86,6 @@ class ClusterConfiguration { LinkedHashMap setupCommands = new LinkedHashMap<>() - @Input - void esSetting(String setting, String value) { - esSettings.put(setting, value); - } - @Input void systemProperty(String property, String value) { systemProperties.put(property, value) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy index 168a67a4728..ebeb3d53895 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/NodeInfo.groovy @@ -130,7 +130,6 @@ class NodeInfo { 'ES_GC_OPTS': config.jvmArgs // we pass these with the undocumented gc opts so the argline can set gc, etc ] args.addAll("-E", "es.node.portsfile=true") - args.addAll(config.esSettings.collectMany { key, value -> ["-E", "${key}=${value}" ] }) env.put('ES_JAVA_OPTS', config.systemProperties.collect { key, value -> "-D${key}=${value}" }.join(" ")) for (Map.Entry property : System.properties.entrySet()) { if (property.getKey().startsWith('es.')) { diff --git a/modules/lang-groovy/build.gradle b/modules/lang-groovy/build.gradle index 340dd620ca6..884fe8b65ba 100644 --- a/modules/lang-groovy/build.gradle +++ b/modules/lang-groovy/build.gradle @@ -28,8 +28,8 @@ dependencies { integTest { cluster { - esSetting 'es.script.inline', 'true' - esSetting 'es.script.indexed', 'true' + setting 'script.inline', 'true' + setting 'script.indexed', 'true' } } diff --git a/modules/lang-mustache/build.gradle b/modules/lang-mustache/build.gradle index 36b58792d86..8eed31dd668 100644 --- a/modules/lang-mustache/build.gradle +++ b/modules/lang-mustache/build.gradle @@ -28,7 +28,7 @@ dependencies { integTest { cluster { - esSetting 'es.script.inline', 'true' - esSetting 'es.script.indexed', 'true' + setting 'script.inline', 'true' + setting 'script.indexed', 'true' } } diff --git a/plugins/lang-javascript/build.gradle b/plugins/lang-javascript/build.gradle index 41d85824318..1f431241838 100644 --- a/plugins/lang-javascript/build.gradle +++ b/plugins/lang-javascript/build.gradle @@ -28,7 +28,7 @@ dependencies { integTest { cluster { - esSetting 'es.script.inline', 'true' - esSetting 'es.script.indexed', 'true' + setting 'script.inline', 'true' + setting 'script.indexed', 'true' } } diff --git a/plugins/lang-python/build.gradle b/plugins/lang-python/build.gradle index bc9db2a20c2..c7466316806 100644 --- a/plugins/lang-python/build.gradle +++ b/plugins/lang-python/build.gradle @@ -28,8 +28,8 @@ dependencies { integTest { cluster { - esSetting 'es.script.inline', 'true' - esSetting 'es.script.indexed', 'true' + setting 'script.inline', 'true' + setting 'script.indexed', 'true' } } diff --git a/qa/smoke-test-ingest-disabled/build.gradle b/qa/smoke-test-ingest-disabled/build.gradle index f8ebd631786..09b2d1409a1 100644 --- a/qa/smoke-test-ingest-disabled/build.gradle +++ b/qa/smoke-test-ingest-disabled/build.gradle @@ -21,6 +21,6 @@ apply plugin: 'elasticsearch.rest-test' integTest { cluster { - esSetting 'es.node.ingest', 'false' + setting 'node.ingest', 'false' } } diff --git a/qa/smoke-test-reindex-with-groovy/build.gradle b/qa/smoke-test-reindex-with-groovy/build.gradle index 749f5c1237c..c4b462ce45a 100644 --- a/qa/smoke-test-reindex-with-groovy/build.gradle +++ b/qa/smoke-test-reindex-with-groovy/build.gradle @@ -21,6 +21,6 @@ apply plugin: 'elasticsearch.rest-test' integTest { cluster { - esSetting 'es.script.inline', 'true' + setting 'script.inline', 'true' } }