SONAR-9773 add warning when ES_JVM_OPTIONS is set

5 files changed, 134 insertions, 46 deletions

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<String, String> settingsMap = new EsSettings(props, esFileSystem).build();
+    Map<String, String> 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<String, String> 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 <T> void attachMemoryAppenderToLoggerOf(Class<T> 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<ILoggingEvent> {
-    private final List<ILoggingEvent> logs = new ArrayList<>();
-
-    @Override
-    public String getName() {
-      return MEMORY_APPENDER_NAME;
-    }
-
-    @Override
-    protected void append(ILoggingEvent eventObject) {
-      logs.add(eventObject);
-    }
-
-    public List<ILoggingEvent> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> settings = new EsSettings(props, new EsFileSystem(props)).build();
+    Map<String, String> 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<String, String> settings = new EsSettings(props, new EsFileSystem(props)).build();
+    Map<String, String> 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<String, String> settings = new EsSettings(props, new EsFileSystem(props)).build();
+    Map<String, String> 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<String, String> settings = new EsSettings(props, new EsFileSystem(props)).build();
+    Map<String, String> 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<String, String> settings = new EsSettings(props, new EsFileSystem(props)).build();
+    Map<String, String> 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<String, String> settings = new EsSettings(props, new EsFileSystem(props)).build();
+    Map<String, String> 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<String, String> settings = new EsSettings(props, new EsFileSystem(props)).build();
+    Map<String, String> 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<String, String> settings = new EsSettings(props, new EsFileSystem(props)).build();
+    Map<String, String> 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<ILoggingEvent> {
+  private final List<ILoggingEvent> logs = new ArrayList<>();
+
+  @Override
+  protected void append(ILoggingEvent eventObject) {
+    logs.add(eventObject);
+  }
+
+  public List<ILoggingEvent> getLogs() {
+    return logs;
+  }
+
+  public static <T> ListAppender attachMemoryAppenderToLoggerOf(Class<T> loggerClass) {
+    ListAppender listAppender = new ListAppender();
+    new LogbackHelper().getRootContext().getLogger(loggerClass)
+      .addAppender(listAppender);
+    listAppender.start();
+    return listAppender;
+  }
+
+  public static <T> void detachMemoryAppenderToLoggerOf(Class<T> loggerClass, ListAppender listAppender) {
+    listAppender.stop();
+    new LogbackHelper().getRootContext().getLogger(loggerClass)
+      .detachAppender(listAppender);
+  }
+}