From: Sébastien Lesaint Date: Mon, 11 Sep 2017 14:57:18 +0000 (+0200) Subject: SONAR-9773 add warning when ES_JVM_OPTIONS is set X-Git-Tag: 6.6-RC1~283 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=ee9750c2fafc4e1e0f6d8e5e115ae759da4453c4;p=sonarqube.git SONAR-9773 add warning when ES_JVM_OPTIONS is set --- diff --git a/server/sonar-process/src/main/java/org/sonar/process/command/CommandFactoryImpl.java b/server/sonar-process/src/main/java/org/sonar/process/command/CommandFactoryImpl.java index abeabc77819..b6d8dedaa40 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/command/CommandFactoryImpl.java +++ b/server/sonar-process/src/main/java/org/sonar/process/command/CommandFactoryImpl.java @@ -76,7 +76,7 @@ public class CommandFactoryImpl implements CommandFactory { if (!esFileSystem.getExecutable().exists()) { throw new IllegalStateException("Cannot find elasticsearch binary"); } - Map settingsMap = new EsSettings(props, esFileSystem).build(); + Map settingsMap = new EsSettings(props, esFileSystem, System2.INSTANCE).build(); return new EsCommand(ProcessId.ELASTICSEARCH, esFileSystem.getHomeDirectory()) .setFileSystem(esFileSystem) diff --git a/server/sonar-process/src/main/java/org/sonar/process/es/EsSettings.java b/server/sonar-process/src/main/java/org/sonar/process/es/EsSettings.java index f06380774ee..223bae8c239 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/es/EsSettings.java +++ b/server/sonar-process/src/main/java/org/sonar/process/es/EsSettings.java @@ -32,6 +32,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.process.ProcessProperties; import org.sonar.process.Props; +import org.sonar.process.System2; import static java.lang.String.valueOf; import static org.sonar.cluster.ClusterProperties.CLUSTER_ENABLED; @@ -52,7 +53,7 @@ public class EsSettings { private final String clusterName; private final String nodeName; - public EsSettings(Props props, EsFileSystem fileSystem) { + public EsSettings(Props props, EsFileSystem fileSystem, System2 system2) { this.props = props; this.fileSystem = fileSystem; @@ -63,6 +64,11 @@ public class EsSettings { } else { this.nodeName = STANDALONE_NODE_NAME; } + String esJvmOptions = system2.getenv("ES_JVM_OPTIONS"); + if (esJvmOptions != null && !esJvmOptions.trim().isEmpty()) { + LOGGER.warn("ES_JVM_OPTIONS is defined but will be ignored. " + + "Use sonar.search.javaOpts and/or sonar.search.javaAdditionalOpts in sonar.properties to specify jvm options for Elasticsearch"); + } } public Map build() { diff --git a/server/sonar-process/src/test/java/org/sonar/process/command/CommandFactoryImplTest.java b/server/sonar-process/src/test/java/org/sonar/process/command/CommandFactoryImplTest.java index 5c87e00c6b4..c6b2abc4e09 100644 --- a/server/sonar-process/src/test/java/org/sonar/process/command/CommandFactoryImplTest.java +++ b/server/sonar-process/src/test/java/org/sonar/process/command/CommandFactoryImplTest.java @@ -20,11 +20,8 @@ package org.sonar.process.command; import ch.qos.logback.classic.spi.ILoggingEvent; -import ch.qos.logback.core.AppenderBase; import java.io.File; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; import java.util.Properties; import org.apache.commons.io.FileUtils; import org.junit.After; @@ -38,7 +35,7 @@ import org.sonar.process.ProcessId; import org.sonar.process.ProcessProperties; import org.sonar.process.Props; import org.sonar.process.System2; -import org.sonar.process.logging.LogbackHelper; +import org.sonar.process.logging.ListAppender; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; @@ -47,7 +44,6 @@ import static org.mockito.Mockito.when; public class CommandFactoryImplTest { - private static final String MEMORY_APPENDER_NAME = "MEMORY_APPENDER"; @Rule public ExpectedException expectedException = ExpectedException.none(); @Rule @@ -68,9 +64,7 @@ public class CommandFactoryImplTest { @After public void tearDown() throws Exception { if (listAppender != null) { - listAppender.stop(); - new LogbackHelper().getRootContext().getLogger(CommandFactoryImpl.class) - .detachAppender(listAppender); + ListAppender.detachMemoryAppenderToLoggerOf(CommandFactoryImpl.class, listAppender); } } @@ -239,27 +233,7 @@ public class CommandFactoryImplTest { } private void attachMemoryAppenderToLoggerOf(Class loggerClass) { - this.listAppender = new ListAppender(); - new LogbackHelper().getRootContext().getLogger(loggerClass) - .addAppender(listAppender); - listAppender.start(); + this.listAppender = ListAppender.attachMemoryAppenderToLoggerOf(loggerClass); } - private static final class ListAppender extends AppenderBase { - private final List logs = new ArrayList<>(); - - @Override - public String getName() { - return MEMORY_APPENDER_NAME; - } - - @Override - protected void append(ILoggingEvent eventObject) { - logs.add(eventObject); - } - - public List getLogs() { - return logs; - } - } } diff --git a/server/sonar-process/src/test/java/org/sonar/process/es/EsSettingsTest.java b/server/sonar-process/src/test/java/org/sonar/process/es/EsSettingsTest.java index c11fc2589a0..2185051df69 100644 --- a/server/sonar-process/src/test/java/org/sonar/process/es/EsSettingsTest.java +++ b/server/sonar-process/src/test/java/org/sonar/process/es/EsSettingsTest.java @@ -19,11 +19,13 @@ */ package org.sonar.process.es; +import ch.qos.logback.classic.spi.ILoggingEvent; import java.io.File; import java.io.IOException; import java.util.Map; import java.util.Properties; import java.util.Random; +import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -31,7 +33,10 @@ import org.junit.rules.TemporaryFolder; import org.sonar.cluster.ClusterProperties; import org.sonar.process.ProcessProperties; import org.sonar.process.Props; +import org.sonar.process.System2; +import org.sonar.process.logging.ListAppender; +import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -45,9 +50,60 @@ public class EsSettingsTest { @Rule public TemporaryFolder temp = new TemporaryFolder(); - @Rule public ExpectedException expectedException = ExpectedException.none(); + private ListAppender listAppender; + + @After + public void tearDown() throws Exception { + if (listAppender != null) { + ListAppender.detachMemoryAppenderToLoggerOf(EsSettings.class, listAppender); + } + } + + @Test + public void constructor_does_not_logs_warning_if_env_variable_ES_JVM_OPTIONS_is_not_set() { + this.listAppender = ListAppender.attachMemoryAppenderToLoggerOf(EsSettings.class); + Props props = minimalProps(); + System2 system2 = mock(System2.class); + new EsSettings(props, new EsFileSystem(props), system2); + + assertThat(listAppender.getLogs()).isEmpty(); + } + + @Test + public void constructor_does_not_logs_warning_if_env_variable_ES_JVM_OPTIONS_is_set_and_empty() { + this.listAppender = ListAppender.attachMemoryAppenderToLoggerOf(EsSettings.class); + Props props = minimalProps(); + System2 system2 = mock(System2.class); + when(system2.getenv("ES_JVM_OPTIONS")).thenReturn(" "); + new EsSettings(props, new EsFileSystem(props), system2); + + assertThat(listAppender.getLogs()).isEmpty(); + } + + @Test + public void constructor_logs_warning_if_env_variable_ES_JVM_OPTIONS_is_set_and_non_empty() throws IOException { + this.listAppender = ListAppender.attachMemoryAppenderToLoggerOf(EsSettings.class); + Props props = minimalProps(); + System2 system2 = mock(System2.class); + when(system2.getenv("ES_JVM_OPTIONS")).thenReturn(randomAlphanumeric(2)); + new EsSettings(props, new EsFileSystem(props), system2); + + assertThat(listAppender.getLogs()) + .extracting(ILoggingEvent::getMessage) + .containsOnly("ES_JVM_OPTIONS is defined but will be ignored. " + + "Use sonar.search.javaOpts and/or sonar.search.javaAdditionalOpts in sonar.properties to specify jvm options for Elasticsearch"); + } + + private Props minimalProps() { + Props props = new Props(new Properties()); + props.set(ProcessProperties.PATH_HOME, randomAlphanumeric(12)); + props.set(ProcessProperties.PATH_TEMP, randomAlphanumeric(12)); + props.set(ProcessProperties.PATH_LOGS, randomAlphanumeric(12)); + props.set(CLUSTER_NAME, randomAlphanumeric(12)); + return props; + } @Test public void test_default_settings_for_standalone_mode() throws Exception { @@ -60,7 +116,7 @@ public class EsSettingsTest { props.set(ProcessProperties.PATH_LOGS, temp.newFolder().getAbsolutePath()); props.set(CLUSTER_NAME, "sonarqube"); - EsSettings esSettings = new EsSettings(props, new EsFileSystem(props)); + EsSettings esSettings = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE); Map generated = esSettings.build(); assertThat(generated.get("transport.tcp.port")).isEqualTo("1234"); @@ -98,7 +154,7 @@ public class EsSettingsTest { props.set(ClusterProperties.CLUSTER_ENABLED, "true"); props.set(ClusterProperties.CLUSTER_NODE_NAME, "node-1"); - EsSettings esSettings = new EsSettings(props, new EsFileSystem(props)); + EsSettings esSettings = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE); Map generated = esSettings.build(); assertThat(generated.get("cluster.name")).isEqualTo("sonarqube-1"); @@ -116,7 +172,7 @@ public class EsSettingsTest { props.set(ProcessProperties.PATH_HOME, homeDir.getAbsolutePath()); props.set(ProcessProperties.PATH_TEMP, temp.newFolder().getAbsolutePath()); props.set(ProcessProperties.PATH_LOGS, temp.newFolder().getAbsolutePath()); - EsSettings esSettings = new EsSettings(props, new EsFileSystem(props)); + EsSettings esSettings = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE); Map generated = esSettings.build(); assertThat(generated.get("node.name")).startsWith("sonarqube-"); } @@ -132,7 +188,7 @@ public class EsSettingsTest { props.set(ProcessProperties.PATH_HOME, homeDir.getAbsolutePath()); props.set(ProcessProperties.PATH_TEMP, temp.newFolder().getAbsolutePath()); props.set(ProcessProperties.PATH_LOGS, temp.newFolder().getAbsolutePath()); - EsSettings esSettings = new EsSettings(props, new EsFileSystem(props)); + EsSettings esSettings = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE); Map generated = esSettings.build(); assertThat(generated.get("node.name")).isEqualTo("sonarqube"); } @@ -145,7 +201,7 @@ public class EsSettingsTest { when(mockedEsFileSystem.getLogDirectory()).thenReturn(new File("/foo/log")); when(mockedEsFileSystem.getDataDirectory()).thenReturn(new File("/foo/data")); - EsSettings underTest = new EsSettings(minProps(new Random().nextBoolean()), mockedEsFileSystem); + EsSettings underTest = new EsSettings(minProps(new Random().nextBoolean()), mockedEsFileSystem, System2.INSTANCE); Map generated = underTest.build(); assertThat(generated.get("path.data")).isEqualTo("/foo/data"); @@ -157,7 +213,7 @@ public class EsSettingsTest { public void set_discovery_settings_if_cluster_is_enabled() throws Exception { Props props = minProps(CLUSTER_ENABLED); props.set(CLUSTER_SEARCH_HOSTS, "1.2.3.4:9000,1.2.3.5:8080"); - Map settings = new EsSettings(props, new EsFileSystem(props)).build(); + Map settings = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE).build(); assertThat(settings.get("discovery.zen.ping.unicast.hosts")).isEqualTo("1.2.3.4:9000,1.2.3.5:8080"); assertThat(settings.get("discovery.zen.minimum_master_nodes")).isEqualTo("2"); @@ -169,7 +225,7 @@ public class EsSettingsTest { Props props = minProps(CLUSTER_ENABLED); props.set(ProcessProperties.SEARCH_MINIMUM_MASTER_NODES, "ꝱꝲꝳପ"); - EsSettings underTest = new EsSettings(props, new EsFileSystem(props)); + EsSettings underTest = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE); expectedException.expect(IllegalStateException.class); expectedException.expectMessage("Value of property sonar.search.minimumMasterNodes is not an integer:"); @@ -180,7 +236,7 @@ public class EsSettingsTest { public void cluster_is_enabled_with_defined_minimum_master_nodes() throws Exception { Props props = minProps(CLUSTER_ENABLED); props.set(ProcessProperties.SEARCH_MINIMUM_MASTER_NODES, "5"); - Map settings = new EsSettings(props, new EsFileSystem(props)).build(); + Map settings = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE).build(); assertThat(settings.get("discovery.zen.minimum_master_nodes")).isEqualTo("5"); } @@ -189,7 +245,7 @@ public class EsSettingsTest { public void cluster_is_enabled_with_defined_initialTimeout() throws Exception { Props props = minProps(CLUSTER_ENABLED); props.set(ProcessProperties.SEARCH_INITIAL_STATE_TIMEOUT, "10s"); - Map settings = new EsSettings(props, new EsFileSystem(props)).build(); + Map settings = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE).build(); assertThat(settings.get("discovery.initial_state_timeout")).isEqualTo("10s"); } @@ -198,7 +254,7 @@ public class EsSettingsTest { public void in_standalone_initialTimeout_is_not_overridable() throws Exception { Props props = minProps(CLUSTER_DISABLED); props.set(ProcessProperties.SEARCH_INITIAL_STATE_TIMEOUT, "10s"); - Map settings = new EsSettings(props, new EsFileSystem(props)).build(); + Map settings = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE).build(); assertThat(settings.get("discovery.initial_state_timeout")).isEqualTo("30s"); } @@ -207,7 +263,7 @@ public class EsSettingsTest { public void in_standalone_minimumMasterNodes_is_not_overridable() throws Exception { Props props = minProps(CLUSTER_DISABLED); props.set(ProcessProperties.SEARCH_MINIMUM_MASTER_NODES, "5"); - Map settings = new EsSettings(props, new EsFileSystem(props)).build(); + Map settings = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE).build(); assertThat(settings.get("discovery.zen.minimum_master_nodes")).isEqualTo("1"); } @@ -216,7 +272,7 @@ public class EsSettingsTest { public void enable_marvel() throws Exception { Props props = minProps(CLUSTER_DISABLED); props.set("sonar.search.marvelHosts", "127.0.0.2,127.0.0.3"); - Map settings = new EsSettings(props, new EsFileSystem(props)).build(); + Map settings = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE).build(); assertThat(settings.get("marvel.agent.exporter.es.hosts")).isEqualTo("127.0.0.2,127.0.0.3"); } @@ -225,7 +281,7 @@ public class EsSettingsTest { public void enable_http_connector() throws Exception { Props props = minProps(CLUSTER_DISABLED); props.set(ProcessProperties.SEARCH_HTTP_PORT, "9010"); - Map settings = new EsSettings(props, new EsFileSystem(props)).build(); + Map settings = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE).build(); assertThat(settings.get("http.port")).isEqualTo("9010"); assertThat(settings.get("http.host")).isEqualTo("127.0.0.1"); @@ -237,7 +293,7 @@ public class EsSettingsTest { Props props = minProps(CLUSTER_DISABLED); props.set(ProcessProperties.SEARCH_HTTP_PORT, "9010"); props.set(ProcessProperties.SEARCH_HOST, "127.0.0.2"); - Map settings = new EsSettings(props, new EsFileSystem(props)).build(); + Map settings = new EsSettings(props, new EsFileSystem(props), System2.INSTANCE).build(); assertThat(settings.get("http.port")).isEqualTo("9010"); assertThat(settings.get("http.host")).isEqualTo("127.0.0.2"); diff --git a/server/sonar-process/src/test/java/org/sonar/process/logging/ListAppender.java b/server/sonar-process/src/test/java/org/sonar/process/logging/ListAppender.java new file mode 100644 index 00000000000..fec800706c2 --- /dev/null +++ b/server/sonar-process/src/test/java/org/sonar/process/logging/ListAppender.java @@ -0,0 +1,52 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.process.logging; + +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.AppenderBase; +import java.util.ArrayList; +import java.util.List; + +public final class ListAppender extends AppenderBase { + private final List logs = new ArrayList<>(); + + @Override + protected void append(ILoggingEvent eventObject) { + logs.add(eventObject); + } + + public List getLogs() { + return logs; + } + + public static ListAppender attachMemoryAppenderToLoggerOf(Class loggerClass) { + ListAppender listAppender = new ListAppender(); + new LogbackHelper().getRootContext().getLogger(loggerClass) + .addAppender(listAppender); + listAppender.start(); + return listAppender; + } + + public static void detachMemoryAppenderToLoggerOf(Class loggerClass, ListAppender listAppender) { + listAppender.stop(); + new LogbackHelper().getRootContext().getLogger(loggerClass) + .detachAppender(listAppender); + } +}