honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

do not prompt for node name twice 

We allow setting the node's name a few different ways: the `name` system
property, the setting `name`, and the setting `node.name`. There is an order
of preference to these settings that gets applied, which can copy values from the
system property or `node.name` setting to the `name` setting. When setting
only `node.name` to one of the prompt placeholders, the user would be
prompted twice as the value of `node.name` is copied to `name` prior to
prompting for input. Additionally, the value entered by the user for `node.name`
would not be used and only the value entered for `name` would be used.

This fix changes the behavior to only prompt once when `node.name is set` and
`name` is not set. This is accomplished by waiting until all values have been
prompted and replaced, then the logic for determining the node's name is
executed.

Closes #11564
This commit is contained in:
jaymode 2015-06-15 08:15:53 -04:00
parent 2213a5aa81
commit ae1ed34355
2 changed files with 98 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java
View File

@ -50,8 +50,8 @@ public class InternalSettingsPreparer {
/**
* Prepares the settings by gathering all elasticsearch system properties, optionally loading the configuration settings,
* and then replacing all property placeholders. This method will not work with settings that have <code>__prompt__</code>
* as their value unless they have been resolved previously.
* and then replacing all property placeholders. This method will not work with settings that have <code>${prompt.text}</code>
* or <code>${prompt.secret}</code> as their value unless they have been resolved previously.
* @param pSettings The initial settings to use
* @param loadConfigSettings flag to indicate whether to load settings from the configuration directory/file
* @return the {@link Settings} and {@link Environment} as a {@link Tuple}
@ -63,7 +63,8 @@ public class InternalSettingsPreparer {
/**
* Prepares the settings by gathering all elasticsearch system properties, optionally loading the configuration settings,
* and then replacing all property placeholders. If a {@link Terminal} is provided and configuration settings are loaded,
* settings with the <code>__prompt__</code> value will result in a prompt for the setting to the user.
* settings with a value of <code>${prompt.text}</code> or <code>${prompt.secret}</code> will result in a prompt for
* the setting to the user.
* @param pSettings The initial settings to use
* @param loadConfigSettings flag to indicate whether to load settings from the configuration directory/file
* @param terminal the Terminal to use for input/output
@ -131,16 +132,9 @@ public class InternalSettingsPreparer {
}
settingsBuilder.replacePropertyPlaceholders();
// generate the name
// check if name is set in settings, if not look for system property and set it
if (settingsBuilder.get("name") == null) {
String name = System.getProperty("name");
if (name == null || name.isEmpty()) {
name = settingsBuilder.get("node.name");
if (name == null || name.isEmpty()) {
name = Names.randomNodeName(environment.resolveConfig("names.txt"));
}
}
if (name != null) {
settingsBuilder.put("name", name);
}
@ -155,17 +149,33 @@ public class InternalSettingsPreparer {
if (v != null) {
Settings.setSettingsRequireUnits(Booleans.parseBoolean(v, true));
}
Settings v1 = replacePromptPlaceholders(settingsBuilder.build(), terminal);
environment = new Environment(v1);
Settings settings = replacePromptPlaceholders(settingsBuilder.build(), terminal);
// all settings placeholders have been resolved. resolve the value for the name setting by checking for name,
// then looking for node.name, and finally generate one if needed
if (settings.get("name") == null) {
final String name = settings.get("node.name");
if (name == null || name.isEmpty()) {
settings = settingsBuilder().put(settings)
.put("name", Names.randomNodeName(environment.resolveConfig("names.txt")))
.build();
} else {
settings = settingsBuilder().put(settings)
.put("name", name)
.build();
}
}
environment = new Environment(settings);
// put back the env settings
settingsBuilder = settingsBuilder().put(v1);
settingsBuilder = settingsBuilder().put(settings);
// we put back the path.logs so we can use it in the logging configuration file
settingsBuilder.put("path.logs", cleanPath(environment.logsFile().toAbsolutePath().toString()));
v1 = settingsBuilder.build();
settings = settingsBuilder.build();
return new Tuple<>(v1, environment);
return new Tuple<>(settings, environment);
}
static Settings replacePromptPlaceholders(Settings settings, Terminal terminal) {

75
core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java
View File

@ -31,22 +31,23 @@ import org.junit.Test;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import static org.elasticsearch.common.settings.Settings.settingsBuilder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.*;
public class InternalSettingsPreparerTests extends ElasticsearchTestCase {
@Before
public void setupSystemProperties() {
System.setProperty("es.node.zone", "foo");
System.setProperty("name", "sys-prop-name");
}
@After
public void cleanupSystemProperties() {
System.clearProperty("es.node.zone");
System.clearProperty("name");
}
@Test
@ -151,4 +152,72 @@ public class InternalSettingsPreparerTests extends ElasticsearchTestCase {
assertThat(e.getMessage(), containsString("with value [" + InternalSettingsPreparer.TEXT_PROMPT_VALUE + "]"));
}
}
@Test
public void testNameSettingsPreference() {
// Test system property overrides node.name
Settings settings = settingsBuilder()
.put("node.name", "node-name")
.put("path.home", createTempDir().toString())
.build();
Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettings(settings, true);
assertThat(tuple.v1().get("name"), equalTo("sys-prop-name"));
// test name in settings overrides sys prop and node.name
settings = settingsBuilder()
.put("name", "name-in-settings")
.put("node.name", "node-name")
.put("path.home", createTempDir().toString())
.build();
tuple = InternalSettingsPreparer.prepareSettings(settings, true);
assertThat(tuple.v1().get("name"), equalTo("name-in-settings"));
// test only node.name in settings
System.clearProperty("name");
settings = settingsBuilder()
.put("node.name", "node-name")
.put("path.home", createTempDir().toString())
.build();
tuple = InternalSettingsPreparer.prepareSettings(settings, true);
assertThat(tuple.v1().get("name"), equalTo("node-name"));
// test no name at all results in name being set
settings = settingsBuilder()
.put("path.home", createTempDir().toString())
.build();
tuple = InternalSettingsPreparer.prepareSettings(settings, true);
assertThat(tuple.v1().get("name"), not("name-in-settings"));
assertThat(tuple.v1().get("name"), not("sys-prop-name"));
assertThat(tuple.v1().get("name"), not("node-name"));
assertThat(tuple.v1().get("name"), notNullValue());
}
@Test
public void testPromptForNodeNameOnlyPromptsOnce() {
final AtomicInteger counter = new AtomicInteger();
final Terminal terminal = new CliToolTestCase.MockTerminal() {
@Override
public char[] readSecret(String message, Object... args) {
fail("readSecret should never be called by this test");
return null;
}
@Override
public String readText(String message, Object... args) {
int count = counter.getAndIncrement();
return "prompted name " + count;
}
};
System.clearProperty("name");
Settings settings = Settings.builder()
.put("path.home", createTempDir())
.put("node.name", InternalSettingsPreparer.TEXT_PROMPT_VALUE)
.build();
Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettings(settings, false, terminal);
settings = tuple.v1();
assertThat(counter.intValue(), is(1));
assertThat(settings.get("name"), is("prompted name 0"));
assertThat(settings.get("node.name"), is("prompted name 0"));
}
}