SONAR-7937 restart reloads sonar.properties

but changes of FileSystem related properties are not supported and will make restart fail

3 files changed, 170 insertions, 22 deletions

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 d6c3b843a85..8fc3ba6f5b3 100644
--- a/sonar-application/src/main/java/org/sonar/application/App.java
+++ b/sonar-application/src/main/java/org/sonar/application/App.java
@@ -19,10 +19,14 @@
  */
 package org.sonar.application;
 
+import com.google.common.annotations.VisibleForTesting;
 import java.io.File;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Properties;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.function.Supplier;
 import org.apache.commons.io.FilenameUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -41,27 +45,46 @@ import static org.sonar.process.ProcessId.APP;
  */
 public class App implements Stoppable {
 
+  private final Properties commandLineArguments;
+  private final Function<Properties, Props> propsSupplier;
   private final JavaCommandFactory javaCommandFactory;
   private final Monitor monitor;
+  private final Supplier<List<JavaCommand>> javaCommandSupplier;
 
-  public App(AppFileSystem appFileSystem, boolean watchForHardStop) {
-    this(Monitor.newMonitorBuilder()
+  private App(Properties commandLineArguments) {
+    this.commandLineArguments = commandLineArguments;
+    this.propsSupplier = properties -> new PropsBuilder(properties, new JdbcSettings()).build();
+    this.javaCommandFactory = new JavaCommandFactoryImpl();
+    Props props = propsSupplier.apply(commandLineArguments);
+
+    AppFileSystem appFileSystem = new AppFileSystem(props);
+    appFileSystem.verifyProps();
+    AppLogging logging = new AppLogging();
+    logging.configure(props);
+
+    // used by orchestrator
+    boolean watchForHardStop = props.valueAsBoolean(ProcessProperties.ENABLE_STOP_COMMAND, false);
+    this.monitor = Monitor.newMonitorBuilder()
       .setProcessNumber(APP.getIpcIndex())
       .setFileSystem(appFileSystem)
       .setWatchForHardStop(watchForHardStop)
       .setWaitForOperational()
       .addListener(new AppLifecycleListener())
-      .build(),
-      new JavaCommandFactoryImpl());
+      .build();
+    this.javaCommandSupplier = new ReloadableCommandSupplier(props, appFileSystem::ensureUnchangedConfiguration);
   }
 
-  App(Monitor monitor, JavaCommandFactory javaCommandFactory) {
+  @VisibleForTesting
+  App(Properties commandLineArguments, Function<Properties, Props> propsSupplier, Monitor monitor, CheckFSConfigOnReload checkFsConfigOnReload, JavaCommandFactory javaCommandFactory) {
+    this.commandLineArguments = commandLineArguments;
+    this.propsSupplier = propsSupplier;
     this.javaCommandFactory = javaCommandFactory;
     this.monitor = monitor;
+    this.javaCommandSupplier = new ReloadableCommandSupplier(propsSupplier.apply(commandLineArguments), checkFsConfigOnReload);
   }
 
-  public void start(Props props) throws InterruptedException {
-    monitor.start(() -> createCommands(props));
+  public void start() throws InterruptedException {
+    monitor.start(javaCommandSupplier);
     monitor.awaitTermination();
   }
 
@@ -96,21 +119,16 @@ public class App implements Stoppable {
   public static void main(String[] args) throws InterruptedException {
     CommandLineParser cli = new CommandLineParser();
     Properties rawProperties = cli.parseArguments(args);
-    Props props = new PropsBuilder(rawProperties, new JdbcSettings()).build();
-    AppFileSystem appFileSystem = new AppFileSystem(props);
-    appFileSystem.verifyProps();
-    AppLogging logging = new AppLogging();
-    logging.configure(props);
 
-    // used by orchestrator
-    boolean watchForHardStop = props.valueAsBoolean(ProcessProperties.ENABLE_STOP_COMMAND, false);
-    App app = new App(appFileSystem, watchForHardStop);
-    app.start(props);
+    App app = new App(rawProperties);
+    app.start();
   }
 
   @Override
   public void stopAsync() {
-    monitor.stop();
+    if (monitor != null) {
+      monitor.stop();
+    }
   }
 
   private static class AppLifecycleListener implements Lifecycle.LifecycleListener {
@@ -123,4 +141,40 @@ public class App implements Stoppable {
       }
     }
   }
+
+  @FunctionalInterface
+  interface CheckFSConfigOnReload extends Consumer<Props> {
+
+  }
+
+  private class ReloadableCommandSupplier implements Supplier<List<JavaCommand>> {
+    private final Props initialProps;
+    private final CheckFSConfigOnReload checkFsConfigOnReload;
+    private boolean initialPropsConsumed = false;
+
+    ReloadableCommandSupplier(Props initialProps, CheckFSConfigOnReload checkFsConfigOnReload) {
+      this.initialProps = initialProps;
+      this.checkFsConfigOnReload = checkFsConfigOnReload;
+    }
+
+    @Override
+    public List<JavaCommand> get() {
+      if (!initialPropsConsumed) {
+        initialPropsConsumed = true;
+        return createCommands(this.initialProps);
+      }
+      return recreateCommands();
+    }
+
+    private List<JavaCommand> recreateCommands() {
+      Props reloadedProps = propsSupplier.apply(commandLineArguments);
+      AppFileSystem appFileSystem = new AppFileSystem(reloadedProps);
+      appFileSystem.verifyProps();
+      checkFsConfigOnReload.accept(reloadedProps);
+      AppLogging logging = new AppLogging();
+      logging.configure(reloadedProps);
+
+      return createCommands(reloadedProps);
+    }
+  }
 }
diff --git a/sonar-application/src/main/java/org/sonar/application/AppFileSystem.java b/sonar-application/src/main/java/org/sonar/application/AppFileSystem.java
index d8b0497d504..9b07583314e 100644
--- a/sonar-application/src/main/java/org/sonar/application/AppFileSystem.java
+++ b/sonar-application/src/main/java/org/sonar/application/AppFileSystem.java
@@ -29,12 +29,14 @@ import java.nio.file.Paths;
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.EnumSet;
+import java.util.Objects;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.process.AllProcessesCommands;
 import org.sonar.process.Props;
 import org.sonar.process.monitor.FileSystem;
 
+import static java.lang.String.format;
 import static java.nio.file.FileVisitResult.CONTINUE;
 import static org.apache.commons.io.FileUtils.forceMkdir;
 import static org.sonar.process.FileUtils.deleteDirectory;
@@ -118,7 +120,7 @@ public class AppFileSystem implements FileSystem {
 
   private static void ensureIsNotAFile(String propKey, File dir) {
     if (!dir.isDirectory()) {
-      throw new IllegalStateException(String.format("Property '%s' is not valid, not a directory: %s",
+      throw new IllegalStateException(format("Property '%s' is not valid, not a directory: %s",
         propKey, dir.getAbsolutePath()));
     }
   }
@@ -132,6 +134,21 @@ public class AppFileSystem implements FileSystem {
     return dir;
   }
 
+  public void ensureUnchangedConfiguration(Props newProps) {
+    verifyUnchanged(newProps, PATH_DATA, DEFAULT_DATA_DIRECTORY_NAME);
+    verifyUnchanged(newProps, PATH_WEB, DEFAULT_WEB_DIRECTORY_NAME);
+    verifyUnchanged(newProps, PATH_LOGS, DEFAULT_LOGS_DIRECTORY_NAME);
+    verifyUnchanged(newProps, PATH_TEMP, DEFAULT_TEMP_DIRECTORY_NAME);
+  }
+
+  private void verifyUnchanged(Props newProps, String propKey, String defaultRelativePath) {
+    String initialValue = props.value(propKey, defaultRelativePath);
+    String newValue = newProps.value(propKey, defaultRelativePath);
+    if (!Objects.equals(newValue, initialValue)) {
+      throw new IllegalStateException(format("Change of property '%s' is not supported ('%s'=> '%s')", propKey, initialValue, newValue));
+    }
+  }
+
   private static class CleanTempDirFileVisitor extends SimpleFileVisitor<Path> {
     private static final Path SHAREDMEMORY_FILE = Paths.get("sharedmemory");
     public static final int VISIT_MAX_DEPTH = 1;
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 70bec0bb1d1..5d8e3b88270 100644
--- a/sonar-application/src/test/java/org/sonar/application/AppTest.java
+++ b/sonar-application/src/test/java/org/sonar/application/AppTest.java
@@ -21,12 +21,16 @@ package org.sonar.application;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.Iterator;
 import java.util.List;
+import java.util.NoSuchElementException;
 import java.util.Properties;
 import java.util.function.Supplier;
 import org.apache.commons.io.FilenameUtils;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
 import org.mockito.ArgumentCaptor;
 import org.sonar.process.ProcessProperties;
@@ -35,13 +39,18 @@ import org.sonar.process.monitor.JavaCommand;
 import org.sonar.process.monitor.Monitor;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
 
 public class AppTest {
 
   @Rule
   public TemporaryFolder temp = new TemporaryFolder();
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
 
   private final JavaCommand esCommand = mock(JavaCommand.class);
   private final JavaCommand webCommand = mock(JavaCommand.class);
@@ -63,6 +72,7 @@ public class AppTest {
       return AppTest.this.ceCommand;
     }
   };
+  private App.CheckFSConfigOnReload checkFsConfigOnReload = mock(App.CheckFSConfigOnReload.class);
 
   @Test
   public void starPath() throws IOException {
@@ -77,8 +87,8 @@ public class AppTest {
   public void start_all_processes_if_cluster_mode_is_disabled() throws Exception {
     Props props = initDefaultProps();
     Monitor monitor = mock(Monitor.class);
-    App app = new App(monitor, javaCommandFactory);
-    app.start(props);
+    App app = new App(props.rawProperties(), properties -> props, monitor, checkFsConfigOnReload, javaCommandFactory);
+    app.start();
 
     ArgumentCaptor<Supplier<List<JavaCommand>>> argument = newJavaCommandArgumentCaptor();
     verify(monitor).start(argument.capture());
@@ -125,6 +135,73 @@ public class AppTest {
     assertThat(commands).containsOnly(esCommand);
   }
 
+  @Test
+  public void javaCommands_supplier_reloads_properties_and_ensure_filesystem_props_have_not_changed() throws Exception {
+    // initial props is not cluster => all three processes must be created
+    Props initialProps = initDefaultProps();
+    // second props is cluster with only ES and CE enabled => only two processes must be created
+    Props props = initDefaultProps();
+    props.set(ProcessProperties.CLUSTER_ENABLED, "true");
+    props.set(ProcessProperties.CLUSTER_WEB_DISABLED, "true");
+    // setup an App that emulate reloading of props returning different configuration
+    Iterator<Props> propsIterator = Arrays.asList(initialProps, props).iterator();
+    Monitor monitor = mock(Monitor.class);
+    App app = new App(initialProps.rawProperties(), properties -> propsIterator.next(), monitor, checkFsConfigOnReload, javaCommandFactory);
+    // start App and capture the JavaCommand supplier it provides to the Monitor
+    app.start();
+    Supplier<List<JavaCommand>> supplier = captureJavaCommandsSupplier(monitor);
+
+    // first call, consumes initial props
+    assertThat(supplier.get()).containsExactly(esCommand, webCommand, ceCommand);
+    verifyZeroInteractions(checkFsConfigOnReload);
+
+    // second call, consumes second props
+    assertThat(supplier.get()).containsExactly(esCommand, ceCommand);
+    verify(checkFsConfigOnReload).accept(props);
+
+    // third call will trigger error from iterator
+    expectedException.expect(NoSuchElementException.class);
+    supplier.get();
+  }
+
+  @Test
+  public void javaCommands_supplier_propagate_errors_from_CheckFsConfigOnReload_instance() throws Exception {
+    // initial props is not cluster => all three processes must be created
+    Props initialProps = initDefaultProps();
+    // second props is cluster with only ES and CE enabled => only two processes must be created
+    Props props = initDefaultProps();
+    props.set(ProcessProperties.CLUSTER_ENABLED, "true");
+    props.set(ProcessProperties.CLUSTER_WEB_DISABLED, "true");
+    // setup an App that emulate reloading of props returning different configuration
+    Iterator<Props> propsIterator = Arrays.asList(initialProps, props).iterator();
+    Monitor monitor = mock(Monitor.class);
+    App app = new App(initialProps.rawProperties(), properties -> propsIterator.next(), monitor, checkFsConfigOnReload, javaCommandFactory);
+    // start App and capture the JavaCommand supplier it provides to the Monitor
+    app.start();
+    Supplier<List<JavaCommand>> supplier = captureJavaCommandsSupplier(monitor);
+
+    // first call, consumes initial props
+    assertThat(supplier.get()).containsExactly(esCommand, webCommand, ceCommand);
+    verifyZeroInteractions(checkFsConfigOnReload);
+
+    // second call, consumes second props and propagates exception thrown by checkFsConfigOnReload
+    IllegalStateException expected = new IllegalStateException("emulating change of FS properties is not supported exception");
+    doThrow(expected).when(checkFsConfigOnReload).accept(props);
+    try {
+      supplier.get();
+      fail("Supplier should have propagated exception from checkFsConfigOnReload");
+    } catch (IllegalStateException e) {
+      assertThat(e).isSameAs(expected);
+    }
+  }
+
+  private Supplier<List<JavaCommand>> captureJavaCommandsSupplier(Monitor monitor) throws InterruptedException {
+    ArgumentCaptor<Supplier<List<JavaCommand>>> argument = newJavaCommandArgumentCaptor();
+    verify(monitor).start(argument.capture());
+
+    return argument.getValue();
+  }
+
   private Props initDefaultProps() throws IOException {
     Props props = new Props(new Properties());
     ProcessProperties.completeDefaults(props);
@@ -136,8 +213,8 @@ public class AppTest {
 
   private List<JavaCommand> start(Props props) throws Exception {
     Monitor monitor = mock(Monitor.class);
-    App app = new App(monitor, javaCommandFactory);
-    app.start(props);
+    App app = new App(props.rawProperties(), properties -> props, monitor, checkFsConfigOnReload, javaCommandFactory);
+    app.start();
     ArgumentCaptor<Supplier<List<JavaCommand>>> argument = newJavaCommandArgumentCaptor();
     verify(monitor).start(argument.capture());
     return argument.getValue().get();