SONAR-5687 refactor configuration of cluster

8 files changed, 71 insertions, 54 deletions

diff --git a/server/sonar-process/src/main/java/org/sonar/process/ProcessConstants.java b/server/sonar-process/src/main/java/org/sonar/process/ProcessConstants.java
index 57aeb2bd622..0f7952a66c2 100644
--- a/server/sonar-process/src/main/java/org/sonar/process/ProcessConstants.java
+++ b/server/sonar-process/src/main/java/org/sonar/process/ProcessConstants.java
@@ -25,8 +25,9 @@ package org.sonar.process;
  */
 public interface ProcessConstants {
 
-  String CLUSTER_ACTIVATION = "sonar.cluster.activation";
-  String CLUSTER_MASTER_HOSTS = "sonar.cluster.master";
+  String CLUSTER_ACTIVATE = "sonar.cluster.activate";
+  String CLUSTER_MASTER = "sonar.cluster.master";
+  String CLUSTER_MASTER_HOST = "sonar.cluster.masterHost";
   String CLUSTER_NAME = "sonar.cluster.name";
   String CLUSTER_NODE_NAME = "sonar.node.name";
 
diff --git a/server/sonar-search/src/main/java/org/sonar/search/SearchSettings.java b/server/sonar-search/src/main/java/org/sonar/search/SearchSettings.java
index e82efd85365..3e72423bd01 100644
--- a/server/sonar-search/src/main/java/org/sonar/search/SearchSettings.java
+++ b/server/sonar-search/src/main/java/org/sonar/search/SearchSettings.java
@@ -30,7 +30,6 @@ import org.sonar.process.Props;
 import org.sonar.search.script.ListUpdate;
 
 import java.io.File;
-import java.net.InetAddress;
 import java.util.Arrays;
 import java.util.LinkedHashSet;
 import java.util.Set;
@@ -44,13 +43,13 @@ class SearchSettings {
   public static final String PROP_MARVEL_HOSTS = "sonar.search.marvelHosts";
 
   private final Props props;
-  private final Set<String> clusterNodes = new LinkedHashSet<String>();
+  private final Set<String> masterHosts = new LinkedHashSet<String>();
   private final String clusterName;
   private final int tcpPort;
 
   SearchSettings(Props props) {
     this.props = props;
-    clusterNodes.addAll(Arrays.asList(StringUtils.split(props.value(ProcessConstants.CLUSTER_MASTER_HOSTS, ""), ",")));
+    masterHosts.addAll(Arrays.asList(StringUtils.split(props.value(ProcessConstants.CLUSTER_MASTER_HOST, ""), ",")));
     clusterName = props.value(ProcessConstants.CLUSTER_NAME);
     Integer port = props.valueAsInt(ProcessConstants.SEARCH_PORT);
     if (port == null) {
@@ -60,7 +59,7 @@ class SearchSettings {
   }
 
   boolean inCluster() {
-    return !clusterNodes.isEmpty();
+    return props.valueAsBoolean(ProcessConstants.CLUSTER_ACTIVATE, false);
   }
 
   String clusterName() {
@@ -150,34 +149,30 @@ class SearchSettings {
   }
 
   private void configureCluster(ImmutableSettings.Builder builder) {
-    if (!clusterNodes.isEmpty()) {
-      LoggerFactory.getLogger(SearchServer.class).info("Joining ES cluster with master: {}", clusterNodes);
-      builder.put("discovery.zen.ping.unicast.hosts", StringUtils.join(clusterNodes, ","));
-      builder.put("node.master", false);
-
-      // Enforce a N/2+1 number of masters in cluster
-      builder.put("discovery.zen.minimum_master_nodes", 1);
+    int replicationFactor = 0;
+    if (inCluster()) {
+      replicationFactor = 1;
+      if (props.valueAsBoolean(ProcessConstants.CLUSTER_MASTER, false)) {
+        // master node
+        builder.put("node.master", true);
+      } else if (!masterHosts.isEmpty()) {
+        LoggerFactory.getLogger(SearchServer.class).info("Joining ES cluster with master: {}", masterHosts);
+        builder.put("discovery.zen.ping.unicast.hosts", StringUtils.join(masterHosts, ","));
+        builder.put("node.master", false);
+
+        // Enforce a N/2+1 number of masters in cluster
+        builder.put("discovery.zen.minimum_master_nodes", 1);
+      } else {
+        throw new MessageException(String.format("Not a master nor a slave. Please check properties %s and %s",
+          ProcessConstants.CLUSTER_MASTER, ProcessConstants.CLUSTER_MASTER_HOST));
+      }
     }
 
-    // When SQ is ran as a cluster
-    // see https://jira.codehaus.org/browse/SONAR-5687
-    int replicationFactor = props.valueAsBoolean(ProcessConstants.CLUSTER_ACTIVATION, false) ? 1 : 0;
     builder.put("index.number_of_replicas", replicationFactor);
-
-    // Set cluster coordinates
     builder.put("cluster.name", clusterName);
     builder.put("cluster.routing.allocation.awareness.attributes", "rack_id");
     builder.put("node.rack_id", props.value(ProcessConstants.CLUSTER_NODE_NAME, "unknown"));
-    if (props.contains(ProcessConstants.CLUSTER_NODE_NAME)) {
-      builder.put("node.name", props.value(ProcessConstants.CLUSTER_NODE_NAME));
-    } else {
-      try {
-        builder.put("node.name", InetAddress.getLocalHost().getHostName());
-      } catch (Exception e) {
-        LoggerFactory.getLogger(SearchServer.class).warn("Could not determine hostname", e);
-        builder.put("node.name", "sq-" + System.currentTimeMillis());
-      }
-    }
+    builder.put("node.name", props.value(ProcessConstants.CLUSTER_NODE_NAME));
   }
 
   private void configureMarvel(ImmutableSettings.Builder builder) {
diff --git a/server/sonar-search/src/test/java/org/sonar/search/SearchServerTest.java b/server/sonar-search/src/test/java/org/sonar/search/SearchServerTest.java
index d91f11a853e..042bcf97805 100644
--- a/server/sonar-search/src/test/java/org/sonar/search/SearchServerTest.java
+++ b/server/sonar-search/src/test/java/org/sonar/search/SearchServerTest.java
@@ -70,6 +70,7 @@ public class SearchServerTest {
     Props props = new Props(new Properties());
     props.set(ProcessConstants.SEARCH_PORT, String.valueOf(port));
     props.set(ProcessConstants.CLUSTER_NAME, CLUSTER_NAME);
+    props.set(ProcessConstants.CLUSTER_NODE_NAME, "test");
     props.set(ProcessConstants.PATH_HOME, temp.newFolder().getAbsolutePath());
 
     searchServer = new SearchServer(props);
@@ -90,10 +91,10 @@ public class SearchServerTest {
 
   @Test
   public void slave_success_replication() throws Exception {
-
     Props props = new Props(new Properties());
-    props.set(ProcessConstants.CLUSTER_ACTIVATION, "true");
     props.set(ProcessConstants.SEARCH_PORT, String.valueOf(port));
+    props.set(ProcessConstants.CLUSTER_ACTIVATE, "true");
+    props.set(ProcessConstants.CLUSTER_MASTER, "true");
     props.set(ProcessConstants.CLUSTER_NODE_NAME, "MASTER");
     props.set(ProcessConstants.CLUSTER_NAME, CLUSTER_NAME);
     props.set(ProcessConstants.PATH_HOME, temp.newFolder().getAbsolutePath());
@@ -108,10 +109,11 @@ public class SearchServerTest {
 
     // start a slave
     props = new Props(new Properties());
-    props.set(ProcessConstants.CLUSTER_MASTER_HOSTS, "localhost:" + port);
+    props.set(ProcessConstants.CLUSTER_ACTIVATE, "true");
+    props.set(ProcessConstants.CLUSTER_MASTER_HOST, "localhost:" + port);
+    props.set(ProcessConstants.CLUSTER_NAME, CLUSTER_NAME);
     props.set(ProcessConstants.CLUSTER_NODE_NAME, "SLAVE");
     props.set(ProcessConstants.SEARCH_PORT, String.valueOf(NetworkUtils.freePort()));
-    props.set(ProcessConstants.CLUSTER_NAME, CLUSTER_NAME);
     props.set(ProcessConstants.PATH_HOME, temp.newFolder().getAbsolutePath());
     SearchServer slaveServer = new SearchServer(props);
     assertThat(slaveServer).isNotNull();
@@ -131,10 +133,10 @@ public class SearchServerTest {
   @Test
   public void slave_failed_replication() throws Exception {
     Props props = new Props(new Properties());
-    props.set(ProcessConstants.CLUSTER_ACTIVATION, "false");
     props.set(ProcessConstants.SEARCH_PORT, String.valueOf(port));
-    props.set(ProcessConstants.CLUSTER_NODE_NAME, "MASTER");
+    props.set(ProcessConstants.CLUSTER_ACTIVATE, "false");
     props.set(ProcessConstants.CLUSTER_NAME, CLUSTER_NAME);
+    props.set(ProcessConstants.CLUSTER_NODE_NAME, "NOT_MASTER");
     props.set(ProcessConstants.PATH_HOME, temp.newFolder().getAbsolutePath());
     searchServer = new SearchServer(props);
     assertThat(searchServer).isNotNull();
@@ -147,10 +149,12 @@ public class SearchServerTest {
 
     // start a slave
     props = new Props(new Properties());
-    props.set(ProcessConstants.CLUSTER_MASTER_HOSTS, "localhost:" + port);
+    props.set(ProcessConstants.CLUSTER_ACTIVATE, "true");
+    props.set(ProcessConstants.CLUSTER_MASTER, "false");
+    props.set(ProcessConstants.CLUSTER_MASTER_HOST, "localhost:" + port);
+    props.set(ProcessConstants.CLUSTER_NAME, CLUSTER_NAME);
     props.set(ProcessConstants.CLUSTER_NODE_NAME, "SLAVE");
     props.set(ProcessConstants.SEARCH_PORT, String.valueOf(NetworkUtils.freePort()));
-    props.set(ProcessConstants.CLUSTER_NAME, CLUSTER_NAME);
     props.set(ProcessConstants.PATH_HOME, temp.newFolder().getAbsolutePath());
     SearchServer slaveServer = new SearchServer(props);
     assertThat(slaveServer).isNotNull();
diff --git a/server/sonar-search/src/test/java/org/sonar/search/SearchSettingsTest.java b/server/sonar-search/src/test/java/org/sonar/search/SearchSettingsTest.java
index cece4e0f000..ee24ef492a0 100644
--- a/server/sonar-search/src/test/java/org/sonar/search/SearchSettingsTest.java
+++ b/server/sonar-search/src/test/java/org/sonar/search/SearchSettingsTest.java
@@ -55,16 +55,19 @@ public class SearchSettingsTest {
     Props props = new Props(new Properties());
     props.set(ProcessConstants.SEARCH_PORT, "1234");
     props.set(ProcessConstants.PATH_HOME, homeDir.getAbsolutePath());
-    props.set(ProcessConstants.CLUSTER_NAME, "test");
+    props.set(ProcessConstants.CLUSTER_NAME, "tests");
+    props.set(ProcessConstants.CLUSTER_NODE_NAME, "test");
 
     SearchSettings searchSettings = new SearchSettings(props);
     assertThat(searchSettings.inCluster()).isFalse();
-    assertThat(searchSettings.clusterName()).isEqualTo("test");
+    assertThat(searchSettings.clusterName()).isEqualTo("tests");
     assertThat(searchSettings.tcpPort()).isEqualTo(1234);
 
     Settings generated = searchSettings.build();
     assertThat(generated.get("transport.tcp.port")).isEqualTo("1234");
-    assertThat(generated.get("cluster.name")).isEqualTo("test");
+    assertThat(generated.get("cluster.name")).isEqualTo("tests");
+    assertThat(generated.get("node.name")).isEqualTo("test");
+
     assertThat(generated.get("path.data")).isNotNull();
     assertThat(generated.get("path.logs")).isNotNull();
     assertThat(generated.get("path.work")).isNotNull();
@@ -75,19 +78,15 @@ public class SearchSettingsTest {
     // no cluster, but node name is set though
     assertThat(generated.get("index.number_of_replicas")).isEqualTo("0");
     assertThat(generated.get("discovery.zen.ping.unicast.hosts")).isNull();
-    assertThat(generated.get("node.name")).isNotEmpty();
   }
 
   @Test
   public void override_dirs() throws Exception {
-    File homeDir = temp.newFolder(), dataDir = temp.newFolder(), logDir = temp.newFolder(), tempDir = temp.newFolder();
-    Props props = new Props(new Properties());
-    props.set(ProcessConstants.SEARCH_PORT, "1234");
-    props.set(ProcessConstants.PATH_HOME, homeDir.getAbsolutePath());
+    File dataDir = temp.newFolder(), logDir = temp.newFolder(), tempDir = temp.newFolder();
+    Props props = minProps();
     props.set(ProcessConstants.PATH_DATA, dataDir.getAbsolutePath());
     props.set(ProcessConstants.PATH_LOGS, logDir.getAbsolutePath());
     props.set(ProcessConstants.PATH_TEMP, tempDir.getAbsolutePath());
-    props.set(ProcessConstants.CLUSTER_NAME, "test");
 
     Settings settings = new SearchSettings(props).build();
 
@@ -99,18 +98,20 @@ public class SearchSettingsTest {
   @Test
   public void test_cluster_master() throws Exception {
     Props props = minProps();
-    props.set(ProcessConstants.CLUSTER_ACTIVATION, "true");
+    props.set(ProcessConstants.CLUSTER_ACTIVATE, "true");
+    props.set(ProcessConstants.CLUSTER_MASTER, "true");
     Settings settings = new SearchSettings(props).build();
 
     assertThat(settings.get("index.number_of_replicas")).isEqualTo("1");
     assertThat(settings.get("discovery.zen.ping.unicast.hosts")).isNull();
-    // TODO set node.master=true ?
+    assertThat(settings.get("node.master")).isEqualTo("true");
   }
 
   @Test
   public void test_cluster_slave() throws Exception {
     Props props = minProps();
-    props.set(ProcessConstants.CLUSTER_MASTER_HOSTS, "127.0.0.2,127.0.0.3");
+    props.set(ProcessConstants.CLUSTER_ACTIVATE, "true");
+    props.set(ProcessConstants.CLUSTER_MASTER_HOST, "127.0.0.2,127.0.0.3");
     Settings settings = new SearchSettings(props).build();
 
     assertThat(settings.get("discovery.zen.ping.unicast.hosts")).isEqualTo("127.0.0.2,127.0.0.3");
@@ -118,6 +119,17 @@ public class SearchSettingsTest {
   }
 
   @Test
+  public void bad_cluster_configuration() throws Exception {
+    Props props = minProps();
+    props.set(ProcessConstants.CLUSTER_ACTIVATE, "true");
+    try {
+      new SearchSettings(props).build();
+      fail();
+    } catch (MessageException e) {
+    }
+  }
+
+  @Test
   public void enable_marvel() throws Exception {
     Props props = minProps();
     props.set(SearchSettings.PROP_MARVEL_HOSTS, "127.0.0.2,127.0.0.3");
@@ -142,7 +154,8 @@ public class SearchSettingsTest {
     Props props = new Props(new Properties());
     props.set(ProcessConstants.SEARCH_PORT, "1234");
     props.set(ProcessConstants.PATH_HOME, homeDir.getAbsolutePath());
-    props.set(ProcessConstants.CLUSTER_NAME, "test");
+    props.set(ProcessConstants.CLUSTER_NAME, "tests");
+    props.set(ProcessConstants.CLUSTER_NODE_NAME, "test");
     return props;
   }
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/search/BaseIndexTest.java b/server/sonar-server/src/test/java/org/sonar/server/search/BaseIndexTest.java
index f9d971e20b1..74073a45029 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/search/BaseIndexTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/search/BaseIndexTest.java
@@ -58,6 +58,7 @@ public class BaseIndexTest {
     clusterPort = NetworkUtils.freePort();
     Properties properties = new Properties();
     properties.setProperty(ProcessConstants.CLUSTER_NAME, clusterName);
+    properties.setProperty(ProcessConstants.CLUSTER_NODE_NAME, "test");
     properties.setProperty(ProcessConstants.SEARCH_PORT, clusterPort.toString());
     properties.setProperty(ProcessConstants.PATH_HOME, temp.getRoot().getAbsolutePath());
     try {
@@ -77,8 +78,9 @@ public class BaseIndexTest {
   public void setup() throws IOException {
     File dataDir = temp.newFolder();
     Settings settings = new Settings();
-    settings.setProperty(ProcessConstants.CLUSTER_ACTIVATION, false);
+    settings.setProperty(ProcessConstants.CLUSTER_ACTIVATE, false);
     settings.setProperty(ProcessConstants.CLUSTER_NAME, clusterName);
+    settings.setProperty(ProcessConstants.CLUSTER_NODE_NAME, "test");
     settings.setProperty(ProcessConstants.SEARCH_PORT, clusterPort.toString());
     settings.setProperty(ProcessConstants.PATH_HOME, dataDir.getAbsolutePath());
     searchClient = new SearchClient(settings);
diff --git a/server/sonar-server/src/test/java/org/sonar/server/tester/ServerTester.java b/server/sonar-server/src/test/java/org/sonar/server/tester/ServerTester.java
index 067b80abfb3..0ea43649cf4 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/tester/ServerTester.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/tester/ServerTester.java
@@ -71,6 +71,7 @@ public class ServerTester extends ExternalResource {
     clusterPort = NetworkUtils.freePort();
     Properties properties = new Properties();
     properties.setProperty(ProcessConstants.CLUSTER_NAME, clusterName);
+    properties.setProperty(ProcessConstants.CLUSTER_NODE_NAME, "test");
     properties.setProperty(ProcessConstants.SEARCH_PORT, clusterPort.toString());
     properties.setProperty(ProcessConstants.PATH_HOME, homeDir.getAbsolutePath());
     searchServer = new SearchServer(new Props(properties));
@@ -93,6 +94,7 @@ public class ServerTester extends ExternalResource {
     Properties properties = new Properties();
     properties.putAll(initialProps);
     properties.setProperty(ProcessConstants.CLUSTER_NAME, clusterName);
+    properties.setProperty(ProcessConstants.CLUSTER_NODE_NAME, "test");
     properties.setProperty(ProcessConstants.SEARCH_PORT, clusterPort.toString());
     properties.setProperty(ProcessConstants.PATH_HOME, homeDir.getAbsolutePath());
     properties.setProperty(DatabaseProperties.PROP_URL, "jdbc:h2:" + homeDir.getAbsolutePath() + "/h2");
diff --git a/sonar-application/src/main/java/org/sonar/application/App.java b/sonar-application/src/main/java/org/sonar/application/App.java
index f0a859435ef..b70b2666de1 100644
--- a/sonar-application/src/main/java/org/sonar/application/App.java
+++ b/sonar-application/src/main/java/org/sonar/application/App.java
@@ -81,8 +81,8 @@ public class App implements Stoppable {
       .addClasspath("./lib/search/*");
     commands.add(elasticsearch);
 
-    // do not yet start SQ in cluster mode. See SONAR-5483 & SONAR-5391
-    if (StringUtils.isEmpty(props.value(ProcessConstants.CLUSTER_MASTER_HOSTS))) {
+    // do not yet start SQ on elasticsearch slaves
+    if (StringUtils.isBlank(props.value(ProcessConstants.CLUSTER_MASTER_HOST))) {
       JavaCommand webServer = new JavaCommand("web")
         .setWorkDir(homeDir)
         .addJavaOptions("-Djava.awt.headless=true -Dfile.encoding=UTF-8 -Djruby.management.enabled=false")
diff --git a/sonar-application/src/test/java/org/sonar/application/AppTest.java b/sonar-application/src/test/java/org/sonar/application/AppTest.java
index c419e846ac1..9d5c0e48826 100644
--- a/sonar-application/src/test/java/org/sonar/application/AppTest.java
+++ b/sonar-application/src/test/java/org/sonar/application/AppTest.java
@@ -91,11 +91,11 @@ public class AppTest {
   }
 
   @Test
-  public void do_not_start_tomcat_if_elasticsearch_single_node() throws Exception {
+  public void do_not_start_tomcat_if_elasticsearch_slave() throws Exception {
     Monitor monitor = mock(Monitor.class);
     App app = new App(monitor);
     Props props = initDefaultProps();
-    props.set("sonar.cluster.master", "x.y.z");
+    props.set("sonar.cluster.masterHost", "1.2.3.4");
     app.start(props);
 
     Class<List<JavaCommand>> listClass = (Class<List<JavaCommand>>)(Class)List.class;