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

Apply system properties after all arguemnts are parsed in BootstrapCLIParser 

One of our tests leaked a system property here since we failed after appling some
system properties in BootstrapCLIParser. This is not a huge deal in production since
we exit the JVM if we fail on that. Yet for correctnes we should only apply them if
we manage to parse them all.
This also caused a test failure lately on CI but on an unrelated test:
  https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+periodic/314/console
This commit is contained in:
Simon Willnauer 2016-02-05 11:21:00 +01:00
parent 89a5eadfea
commit 1e18c9a7b6
2 changed files with 20 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java
View File

@ -32,10 +32,12 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.monitor.jvm.JvmInfo;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Locale;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd;
import static org.elasticsearch.common.cli.CliToolConfig.Builder.optionBuilder;
@ -131,6 +133,7 @@ final class BootstrapCLIParser extends CliTool {
// hacky way to extract all the fancy extra args, there is no CLI tool helper for this
Iterator<String> iterator = cli.getArgList().iterator();
final Map<String, String> properties = new HashMap<>();
while (iterator.hasNext()) {
String arg = iterator.next();
if (!arg.startsWith("--")) {
@ -148,20 +151,22 @@ final class BootstrapCLIParser extends CliTool {
String[] splitArg = arg.split("=", 2);
String key = splitArg[0];
String value = splitArg[1];
System.setProperty("es." + key, value);
properties.put("es." + key, value);
} else {
if (iterator.hasNext()) {
String value = iterator.next();
if (value.startsWith("--")) {
throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "] needs value");
}
System.setProperty("es." + arg, value);
properties.put("es." + arg, value);
} else {
throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "] needs value");
}
}
}
for (Map.Entry<String, String> entry : properties.entrySet()) {
System.setProperty(entry.getKey(), entry.getValue());
}
return new Start(terminal);
}

12
qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java
View File

@ -29,11 +29,15 @@ import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.monitor.jvm.JvmInfo;
import org.hamcrest.Matcher;
import org.junit.After;
import org.junit.Before;
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 java.util.Properties;
import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK;
import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK_AND_EXIT;
@ -48,6 +52,12 @@ public class BootstrapCliParserTests extends CliToolTestCase {
private CaptureOutputTerminal terminal = new CaptureOutputTerminal();
private List<String> propertiesToClear = new ArrayList<>();
private Map<Object, Object> properties;
@Before
public void before() {
this.properties = new HashMap<>(System.getProperties());
}
@After
public void clearProperties() {
@ -55,6 +65,7 @@ public class BootstrapCliParserTests extends CliToolTestCase {
System.clearProperty(property);
}
propertiesToClear.clear();
assertEquals("properties leaked", properties, new HashMap<>(System.getProperties()));
}
public void testThatVersionIsReturned() throws Exception {
@ -235,6 +246,7 @@ public class BootstrapCliParserTests extends CliToolTestCase {
parser.parse("start", new String[]{"--foo=bar", "-Dbaz=qux"});
});
assertThat(e.getMessage(), containsString("must be before any parameters starting with --"));
assertNull(System.getProperty("es.foo"));
}
private void registerProperties(String ... systemProperties) {