Handle failing app redeployment at statup and do not set health status to 'up'

5 files changed, 147 insertions, 111 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
index cd6a451ecf8..f9d5e9ba9b9 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
@@ -12,7 +12,6 @@ import com.yahoo.config.application.api.ApplicationMetaData;
 import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
-import com.yahoo.config.provision.Deployer;
 import com.yahoo.config.provision.HostFilter;
 import com.yahoo.config.provision.Provisioner;
 import com.yahoo.config.provision.TenantName;
@@ -56,10 +55,15 @@ import java.net.URI;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -413,15 +417,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return session.getSessionId();
     }
 
-    void redeployAllApplications(Deployer deployer) throws InterruptedException {
-        ExecutorService deploymentExecutor = Executors.newFixedThreadPool(configserverConfig.numParallelTenantLoaders(),
-                                                                          new DaemonThreadFactory("redeploy apps"));
-        tenants.getAllTenants().forEach(tenant -> listApplicationIds(tenant)
-                .forEach(applicationId -> redeployApplication(applicationId, deployer, deploymentExecutor)));
-        deploymentExecutor.shutdown();
-        deploymentExecutor.awaitTermination(365, TimeUnit.DAYS); // Timeout should never happen
-    }
-
     private static void cleanupApplicationDirectory(File tempDir, DeployLogger logger) {
         logger.log(LogLevel.DEBUG, "Deleting tmp dir '" + tempDir + "'");
         if (!IOUtils.recursiveDeleteDir(tempDir)) {
@@ -429,16 +424,24 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         }
     }
 
-    private void redeployApplication(ApplicationId applicationId, Deployer deployer, ExecutorService deploymentExecutor) {
-        log.log(LogLevel.DEBUG, () -> "Redeploying " + applicationId);
-        deployer.deployFromLocalActive(applicationId)
-                .ifPresent(deployment -> deploymentExecutor.execute(() -> {
-                    try {
-                        deployment.activate();
-                    } catch (RuntimeException e) {
-                        log.log(LogLevel.ERROR, "Redeploying " + applicationId + " failed", e);
-                    }
-                }));
+    void redeployAllApplications() throws InterruptedException {
+        ExecutorService executor = Executors.newFixedThreadPool(configserverConfig.numParallelTenantLoaders(),
+                                                                new DaemonThreadFactory("redeploy apps"));
+        // Keep track of deployment per application
+        Map<ApplicationId, Future<?>> futures = new HashMap<>();
+        tenants.getAllTenants()
+                .forEach(tenant -> listApplicationIds(tenant)
+                        .forEach(appId -> deployFromLocalActive(appId).ifPresent(
+                                deployment -> futures.put(appId,executor.submit(deployment::activate)))));
+        for (Map.Entry<ApplicationId, Future<?>> f : futures.entrySet()) {
+            try {
+                f.getValue().get();
+            } catch (ExecutionException e) {
+                throw new RuntimeException("Redeploying of " + f.getKey() + " failed", e);
+            }
+        }
+        executor.shutdown();
+        executor.awaitTermination(365, TimeUnit.DAYS); // Timeout should never happen
     }
 
     public ApplicationFile getApplicationFileFromSession(TenantName tenantName, long sessionId, String path, LocalSession.Mode mode) {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java
index 6ab98f5af1c..9793a441355 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.config.server;
 
 import com.google.inject.Inject;
 import com.yahoo.component.AbstractComponent;
-import com.yahoo.config.provision.Deployer;
 import com.yahoo.container.jdisc.state.StateMonitor;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.config.server.rpc.RpcServer;
@@ -22,7 +21,6 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
     private final ApplicationRepository applicationRepository;
     private final RpcServer server;
     private final Thread serverThread;
-    private final Deployer deployer;
     private final VersionState versionState;
     private final StateMonitor stateMonitor;
 
@@ -31,13 +29,23 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
     @SuppressWarnings("WeakerAccess")
     @Inject
     public ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server,
-                                 Deployer deployer, VersionState versionState, StateMonitor stateMonitor) {
+                                 VersionState versionState, StateMonitor stateMonitor) {
+        this(applicationRepository, server, versionState, stateMonitor, true);
+    }
+
+    // For testing only
+    ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server,
+                          VersionState versionState, StateMonitor stateMonitor, boolean startMainThread) {
         this.applicationRepository = applicationRepository;
         this.server = server;
-        this.deployer = deployer;
         this.versionState = versionState;
         this.stateMonitor = stateMonitor;
         this.serverThread = new Thread(this, "configserver main");
+        if (startMainThread)
+            start();
+    }
+
+    private void start() {
         serverThread.start();
     }
 
@@ -55,22 +63,35 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
     @Override
     public void run() {
         if (versionState.isUpgraded()) {
-            log.log(LogLevel.INFO, "Configserver upgraded from " + versionState.storedVersion() + " to "
+            log.log(LogLevel.INFO, "Configserver upgrading from " + versionState.storedVersion() + " to "
                     + versionState.currentVersion() + ". Redeploying all applications");
             try {
-                applicationRepository.redeployAllApplications(deployer);
-            } catch (InterruptedException e) {
-                throw new RuntimeException("Redeploying applications failed", e);
+                applicationRepository.redeployAllApplications();
+                versionState.saveNewVersion();
+                log.log(LogLevel.INFO, "All applications redeployed successfully");
+            } catch (Exception e) {
+                log.log(LogLevel.ERROR, "Redeployment of applications failed", e);
+                return; // Status will not be set to 'up' since we return here
             }
-            log.log(LogLevel.INFO, "All applications redeployed");
         }
-        versionState.saveNewVersion();
         stateMonitor.status(StateMonitor.Status.up);
-        log.log(LogLevel.DEBUG, "Starting RPC server");
+        log.log(LogLevel.INFO, "Starting RPC server");
         server.run();
-        log.log(LogLevel.DEBUG, "RPC server stopped");
+        do {
+            try {
+                Thread.sleep(1000);
+            } catch (InterruptedException e) {
+                log.log(LogLevel.ERROR, "Got interrupted", e);
+                break;
+            }
+        } while (server.isRunning());
+        log.log(LogLevel.INFO, "RPC server stopped");
         stateMonitor.status(StateMonitor.Status.down);
     }
 
+    StateMonitor.Status status() {
+        return stateMonitor.status();
+    }
+
 }
 
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java
index a5f288bf254..d42468ec8fd 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java
@@ -100,6 +100,7 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener {
     private final ThreadPoolExecutor executorService;
     private final FileDownloader downloader;
     private volatile boolean allTenantsLoaded = false;
+    private boolean isRunning = false;
 
     /**
      * Creates an RpcServer listening on the specified <code>port</code>.
@@ -168,6 +169,7 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener {
         log.log(LogLevel.INFO, "Rpc server listening on port " + spec.port());
         try {
             Acceptor acceptor = supervisor.listen(spec);
+            isRunning = true;
             supervisor.transport().join();
             acceptor.shutdown().join();
         } catch (ListenFailedException e) {
@@ -185,6 +187,11 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener {
         }
         delayedConfigResponses.stop();
         supervisor.transport().shutdown().join();
+        isRunning = false;
+    }
+
+    public boolean isRunning() {
+        return isRunning;
     }
 
     /**
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
index 5d573323bb6..b8efc4f8692 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
@@ -1,132 +1,133 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server;
 
+import com.google.common.io.Files;
 import com.yahoo.cloud.config.ConfigserverConfig;
-import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.ApplicationName;
-import com.yahoo.config.provision.InstanceName;
-import com.yahoo.config.provision.TenantName;
 import com.yahoo.container.jdisc.config.HealthMonitorConfig;
 import com.yahoo.container.jdisc.state.StateMonitor;
-import com.yahoo.io.IOUtils;
 import com.yahoo.jdisc.core.SystemTimer;
-import com.yahoo.vespa.config.server.deploy.MockDeployer;
-import com.yahoo.vespa.config.server.host.HostRegistries;
-import com.yahoo.vespa.config.server.http.SessionHandlerTest;
-import com.yahoo.vespa.config.server.monitoring.Metrics;
-import com.yahoo.vespa.config.server.rpc.UncompressedConfigResponseFactory;
-import com.yahoo.vespa.config.server.tenant.Tenant;
-import com.yahoo.vespa.config.server.tenant.TenantRequestHandler;
-import com.yahoo.vespa.config.server.tenant.TestWithTenant;
+import com.yahoo.vespa.config.server.deploy.DeployTester;
+import com.yahoo.vespa.config.server.rpc.RpcServer;
 import com.yahoo.vespa.config.server.version.VersionState;
-import org.hamcrest.core.Is;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
 import java.io.File;
-import java.io.FileReader;
-import java.time.Clock;
-import java.util.ArrayList;
+import java.nio.file.Paths;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.function.BooleanSupplier;
 
-import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 /**
- * @author lulf
- * @since 5.1
+ * @author Ulf Lilleengen
+ * @author Harald Musum
  */
-public class ConfigServerBootstrapTest extends TestWithTenant {
-    private final TenantName tenant1 = TenantName.from("tenant1");
-    private final TenantName tenant2 = TenantName.from("tenant2");
-
-    private ApplicationRepository applicationRepository;
+public class ConfigServerBootstrapTest {
 
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
-    @Before
-    public void setup() throws Exception {
-        tenants.addTenant(tenant1);
-        tenants.addTenant(tenant2);
-
-        applicationRepository = new ApplicationRepository(tenants,
-                                                          new SessionHandlerTest.MockProvisioner(),
-                                                          Clock.systemUTC());
-    }
-
     @Test
-    public void testConfigServerBootstrap() throws Exception {
-        File versionFile = temporaryFolder.newFile();
-        ConfigserverConfig.Builder config = new ConfigserverConfig.Builder();
-        MockTenantRequestHandler myServer = new MockTenantRequestHandler(Metrics.createTestMetrics());
-        MockRpc rpc = new MockRpc(new ConfigserverConfig(config).rpcport());
+    public void testBootStrap() throws Exception {
+        ConfigserverConfig configserverConfig = createConfigserverConfig();
+        DeployTester tester = new DeployTester("src/test/apps/hosted/", configserverConfig);
+        tester.deployApp("myApp", "4.5.6", Instant.now());
 
-        assertFalse(myServer.started);
-        assertFalse(myServer.stopped);
+        File versionFile = temporaryFolder.newFile();
         VersionState versionState = new VersionState(versionFile);
         assertTrue(versionState.isUpgraded());
-        ConfigServerBootstrap bootstrap =
-                new ConfigServerBootstrap(applicationRepository, rpc,  new MockDeployer(), versionState,
-                                          new StateMonitor(new HealthMonitorConfig(new HealthMonitorConfig.Builder()), new SystemTimer()));
-        waitUntilStarted(rpc, 60000);
-        assertFalse(versionState.isUpgraded());
-        assertThat(versionState.currentVersion(), is(versionState.storedVersion()));
-        assertThat(IOUtils.readAll(new FileReader(versionFile)), is(versionState.currentVersion().toSerializedForm()));
-        assertTrue(rpc.started);
-        assertFalse(rpc.stopped);
+
+        RpcServer rpcServer = createRpcServer(configserverConfig);
+        ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState, createStateMonitor());
+        waitUntil(() -> bootstrap.status() == StateMonitor.Status.up, "failed waiting for status 'up'");
+        waitUntil(rpcServer::isRunning, "failed waiting for Rpc server running");
+
         bootstrap.deconstruct();
-        assertTrue(rpc.started);
-        assertTrue(rpc.stopped);
+        assertEquals(StateMonitor.Status.down, bootstrap.status());
+        assertFalse(rpcServer.isRunning());
     }
 
     @Test
-    public void testTenantRedeployment() throws Exception {
-        MockDeployer deployer = new MockDeployer();
-        Tenant tenant = tenants.getTenant(tenant1);
-        ApplicationId id = ApplicationId.from(tenant1, ApplicationName.defaultName(), InstanceName.defaultName());
-        tenant.getApplicationRepo().createPutApplicationTransaction(id, 3).commit();
-        applicationRepository.redeployAllApplications(deployer);
-        assertThat(deployer.lastDeployed, Is.is(id));
+    public void testBootStrapWhenRedeploymentFails() throws Exception {
+        ConfigserverConfig configserverConfig = createConfigserverConfig();
+        DeployTester tester = new DeployTester("src/test/apps/hosted/", configserverConfig);
+        tester.deployApp("myApp", "4.5.6", Instant.now());
+
+        File versionFile = temporaryFolder.newFile();
+        VersionState versionState = new VersionState(versionFile);
+        assertTrue(versionState.isUpgraded());
+
+        // Manipulate application package so that it will fail deployment when config server starts
+        java.nio.file.Files.delete(Paths.get(configserverConfig.configServerDBDir())
+                                           .resolve("tenants/")
+                                           .resolve(tester.tenant().getName().value())
+                                           .resolve("sessions/2/services.xml"));
+
+        RpcServer rpcServer = createRpcServer(configserverConfig);
+        ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState,
+                                                                    createStateMonitor(), false /* do not call run method */);
+        // Call method directly, to be sure that it is finished redeploying all applications and we can check status
+        bootstrap.run();
+        // App is invalid, so bootstrapping was unsuccessful. Status should be 'initializing' and rpc server should not be running
+        assertEquals(StateMonitor.Status.initializing, bootstrap.status());
+        assertFalse(rpcServer.isRunning());
     }
 
-    private void waitUntilStarted(MockRpc server, long timeout) throws InterruptedException {
-        long start = System.currentTimeMillis();
-        while ((System.currentTimeMillis() - start) < timeout) {
-            if (server.started)
+    private void waitUntil(BooleanSupplier booleanSupplier, String messageIfWaitingFails) throws InterruptedException {
+        Duration timeout = Duration.ofSeconds(60);
+        Instant endTime = Instant.now().plus(timeout);
+        while (Instant.now().isBefore(endTime)) {
+            if (booleanSupplier.getAsBoolean())
                 return;
             Thread.sleep(10);
         }
+        throw new RuntimeException(messageIfWaitingFails);
+    }
+
+    private MockRpc createRpcServer(ConfigserverConfig configserverConfig) {
+        return new MockRpc(configserverConfig.rpcport());
     }
 
-    public static class MockTenantRequestHandler extends TenantRequestHandler {
-        public volatile boolean started = false;
-        public volatile boolean stopped = false;
+    private StateMonitor createStateMonitor() {
+        return new StateMonitor(new HealthMonitorConfig(new HealthMonitorConfig.Builder().initialStatus("initializing")),
+                                new SystemTimer());
+    }
 
-        public MockTenantRequestHandler(Metrics statistics) {
-            super(statistics, TenantName.from("testTenant"), new ArrayList<>(), new UncompressedConfigResponseFactory(), new HostRegistries());
-        }
+    private static ConfigserverConfig createConfigserverConfig() {
+        return new ConfigserverConfig(new ConfigserverConfig.Builder()
+                                              .configServerDBDir(Files.createTempDir().getAbsolutePath())
+                                              .configDefinitionsDir(Files.createTempDir().getAbsolutePath())
+                                              .hostedVespa(true)
+                                              .multitenant(true));
     }
 
     public static class MockRpc extends com.yahoo.vespa.config.server.rpc.MockRpc {
-        public volatile boolean started = false;
-        public volatile boolean stopped = false;
 
-        public MockRpc(int port) {
+        volatile boolean isRunning = false;
+
+        MockRpc(int port) {
             super(port);
         }
 
         @Override
         public void run() {
-            started = true;
+            isRunning = true;
         }
 
         @Override
         public void stop() {
-            stopped = true;
+            isRunning = false;
+        }
+
+        @Override
+        public boolean isRunning() {
+            return isRunning;
         }
     }
+
 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java
index 4c2a4b56751..5a9735f774a 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java
@@ -20,8 +20,7 @@ import java.util.concurrent.CompletionService;
 /**
  * Test utility mocking an RPC server.
  *
- * @author lulf
- * @since 5.25
+ * @author Ulf Lilleengen
  */
 public class MockRpc extends RpcServer {
 
@@ -111,4 +110,9 @@ public class MockRpc extends RpcServer {
     @Override
     public boolean allTenantsLoaded() { return true; }
 
+    @Override
+    public boolean isRunning() {
+        return true;
+    }
+
 }