Merge pull request #24298 from vespa-engine/hmusum/refactor-bootstrap

Hmusum/refactor bootstrap [run-systemtest]

4 files changed, 61 insertions, 83 deletions

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 f76c7939d8d..5eb0f830ef6 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
@@ -1,27 +1,23 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server;
 
-import com.yahoo.component.annotation.Inject;
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.component.AbstractComponent;
+import com.yahoo.component.annotation.Inject;
 import com.yahoo.concurrent.DaemonThreadFactory;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Deployment;
 import com.yahoo.config.provision.TransientException;
 import com.yahoo.container.handler.VipStatus;
 import com.yahoo.container.jdisc.state.StateMonitor;
-import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker;
 import com.yahoo.vespa.config.server.maintenance.ConfigServerMaintenance;
 import com.yahoo.vespa.config.server.rpc.RpcServer;
 import com.yahoo.vespa.config.server.version.VersionState;
-import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.yolean.Exceptions;
-
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashSet;
@@ -37,9 +33,6 @@ import java.util.concurrent.TimeoutException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import static com.yahoo.vespa.config.server.ConfigServerBootstrap.Mode.BOOTSTRAP_IN_CONSTRUCTOR;
-import static com.yahoo.vespa.config.server.ConfigServerBootstrap.Mode.FOR_TESTING_NO_BOOTSTRAP_OF_APPS;
-import static com.yahoo.vespa.config.server.ConfigServerBootstrap.RedeployingApplicationsFails.CONTINUE;
 import static com.yahoo.vespa.config.server.ConfigServerBootstrap.RedeployingApplicationsFails.EXIT_JVM;
 
 /**
@@ -59,7 +52,6 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
 
     private static final Logger log = Logger.getLogger(ConfigServerBootstrap.class.getName());
 
-    enum Mode { BOOTSTRAP_IN_CONSTRUCTOR, FOR_TESTING_NO_BOOTSTRAP_OF_APPS}
     enum RedeployingApplicationsFails { EXIT_JVM, CONTINUE }
     enum VipStatusMode { VIP_STATUS_FILE, VIP_STATUS_PROGRAMMATICALLY }
 
@@ -79,28 +71,14 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
     @SuppressWarnings("unused") //  Injected component
     @Inject
     public ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server,
-                                 VersionState versionState, StateMonitor stateMonitor, VipStatus vipStatus,
-                                 FlagSource flagSource, ConfigConvergenceChecker convergence) {
-        this(applicationRepository, server, versionState, stateMonitor, vipStatus, BOOTSTRAP_IN_CONSTRUCTOR, EXIT_JVM,
-             applicationRepository.configserverConfig().hostedVespa()
-                     ? VipStatusMode.VIP_STATUS_FILE
-                     : VipStatusMode.VIP_STATUS_PROGRAMMATICALLY,
-             flagSource, convergence, Clock.systemUTC());
+                                 VersionState versionState, StateMonitor stateMonitor, VipStatus vipStatus) {
+        this(applicationRepository, server, versionState, stateMonitor, vipStatus, EXIT_JVM, vipStatusMode(applicationRepository));
     }
 
-    // For testing only
-    ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server, VersionState versionState,
-                          StateMonitor stateMonitor, VipStatus vipStatus, VipStatusMode vipStatusMode,
-                          FlagSource flagSource, ConfigConvergenceChecker convergence, Clock clock) {
-        this(applicationRepository, server, versionState, stateMonitor, vipStatus,
-             FOR_TESTING_NO_BOOTSTRAP_OF_APPS, CONTINUE, vipStatusMode, flagSource, convergence, clock);
-    }
-
-    private ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server,
-                                  VersionState versionState, StateMonitor stateMonitor, VipStatus vipStatus,
-                                  Mode mode, RedeployingApplicationsFails exitIfRedeployingApplicationsFails,
-                                  VipStatusMode vipStatusMode, FlagSource flagSource, ConfigConvergenceChecker convergence,
-                                  Clock clock) {
+    protected ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server,
+                                    VersionState versionState, StateMonitor stateMonitor, VipStatus vipStatus,
+                                    RedeployingApplicationsFails exitIfRedeployingApplicationsFails,
+                                    VipStatusMode vipStatusMode) {
         this.applicationRepository = applicationRepository;
         this.server = server;
         this.versionState = versionState;
@@ -110,27 +88,13 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
         this.maxDurationOfRedeployment = Duration.ofSeconds(configserverConfig.maxDurationOfBootstrap());
         this.sleepTimeWhenRedeployingFails = Duration.ofSeconds(configserverConfig.sleepTimeWhenRedeployingFails());
         this.exitIfRedeployingApplicationsFails = exitIfRedeployingApplicationsFails;
-        this.clock = clock;
+        this.clock = applicationRepository.clock();
         rpcServerExecutor = Executors.newSingleThreadExecutor(new DaemonThreadFactory("config server RPC server"));
-
-        configServerMaintenance = new ConfigServerMaintenance(configserverConfig,
-                                                              applicationRepository,
-                                                              applicationRepository.tenantRepository().getCurator(),
-                                                              flagSource,
-                                                              convergence);
+        configServerMaintenance = new ConfigServerMaintenance(applicationRepository);
         configServerMaintenance.startBeforeBootstrap();
-        log.log(Level.FINE, () -> "Bootstrap mode: " + mode + ", VIP status mode: " + vipStatusMode);
+        log.log(Level.FINE, () -> "VIP status mode: " + vipStatusMode);
         initializing(vipStatusMode);
-
-        switch (mode) {
-            case BOOTSTRAP_IN_CONSTRUCTOR:
-                start();
-                break;
-            case FOR_TESTING_NO_BOOTSTRAP_OF_APPS:
-                break;
-            default:
-                throw new IllegalArgumentException("Unknown bootstrap mode " + mode + ", legal values: " + Arrays.toString(Mode.values()));
-        }
+        start();
     }
 
     @Override
@@ -325,6 +289,12 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
         }
     }
 
+    private static VipStatusMode vipStatusMode(ApplicationRepository applicationRepository) {
+        return applicationRepository.configserverConfig().hostedVespa()
+                ? VipStatusMode.VIP_STATUS_FILE
+                : VipStatusMode.VIP_STATUS_PROGRAMMATICALLY;
+    }
+
     private static class LogState {
 
         private final int applicationCount;
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java
index a6516ac361b..a4cc11bd22e 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java
@@ -31,16 +31,12 @@ public class ConfigServerMaintenance {
     private final FlagSource flagSource;
     private final ConfigConvergenceChecker convergenceChecker;
 
-    public ConfigServerMaintenance(ConfigserverConfig configserverConfig,
-                                   ApplicationRepository applicationRepository,
-                                   Curator curator,
-                                   FlagSource flagSource,
-                                   ConfigConvergenceChecker convergenceChecker) {
-        this.configserverConfig = configserverConfig;
+    public ConfigServerMaintenance(ApplicationRepository applicationRepository) {
+        this.configserverConfig = applicationRepository.configserverConfig();
         this.applicationRepository = applicationRepository;
-        this.curator = curator;
-        this.flagSource = flagSource;
-        this.convergenceChecker = convergenceChecker;
+        this.curator = applicationRepository.tenantRepository().getCurator();
+        this.flagSource = applicationRepository.flagSource();
+        this.convergenceChecker = applicationRepository.configConvergenceChecker();
     }
 
     public void startBeforeBootstrap() {
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 9a90d517ca5..9b6ef7ce197 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
@@ -18,19 +18,15 @@ import com.yahoo.container.handler.VipStatus;
 import com.yahoo.container.jdisc.state.StateMonitor;
 import com.yahoo.docproc.jdisc.metric.NullMetric;
 import com.yahoo.path.Path;
-import com.yahoo.test.ManualClock;
 import com.yahoo.text.Utf8;
-import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker;
 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 com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
-import com.yahoo.vespa.flags.InMemoryFlagSource;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
-
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Paths;
@@ -43,6 +39,7 @@ import java.util.Optional;
 import java.util.function.BooleanSupplier;
 import java.util.stream.Collectors;
 
+import static com.yahoo.vespa.config.server.ConfigServerBootstrap.RedeployingApplicationsFails.CONTINUE;
 import static com.yahoo.vespa.config.server.ConfigServerBootstrap.VipStatusMode;
 import static com.yahoo.vespa.config.server.ConfigServerBootstrap.VipStatusMode.VIP_STATUS_FILE;
 import static com.yahoo.vespa.config.server.ConfigServerBootstrap.VipStatusMode.VIP_STATUS_PROGRAMMATICALLY;
@@ -57,9 +54,6 @@ import static org.junit.Assert.assertTrue;
  */
 public class ConfigServerBootstrapTest {
 
-    private final MockCurator curator = new MockCurator();
-    private final ManualClock clock = new ManualClock();
-
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
@@ -75,14 +69,14 @@ public class ConfigServerBootstrapTest {
         RpcServer rpcServer = createRpcServer(configserverConfig);
         // Take a host away so that there are too few for the application, to verify we can still bootstrap
         provisioner.allocations().values().iterator().next().remove(0);
-        ConfigServerBootstrap bootstrap = createBootstrap(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY);
+        Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY);
         assertEquals(List.of("ApplicationPackageMaintainer", "TenantsMaintainer"),
                      bootstrap.configServerMaintenance().maintainers().stream()
                               .map(Maintainer::name)
                               .sorted().collect(Collectors.toList()));
         assertFalse(bootstrap.vipStatus().isInRotation());
 
-        bootstrap.start();
+        bootstrap.doStart();
         waitUntil(rpcServer::isRunning, "failed waiting for Rpc server running");
         assertTrue(rpcServer.isServingConfigRequests());
         waitUntil(() -> bootstrap.status() == StateMonitor.Status.up, "failed waiting for status 'up'");
@@ -110,10 +104,10 @@ public class ConfigServerBootstrapTest {
         tester.deployApp("src/test/apps/hosted/");
 
         RpcServer rpcServer = createRpcServer(configserverConfig);
-        ConfigServerBootstrap bootstrap = createBootstrap(tester, rpcServer, VIP_STATUS_FILE);
+        Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_FILE);
         assertTrue(bootstrap.vipStatus().isInRotation()); // default is in rotation when using status file
 
-        bootstrap.start();
+        bootstrap.doStart();
         waitUntil(rpcServer::isRunning, "failed waiting for Rpc server running");
         assertTrue(rpcServer.isServingConfigRequests());
         waitUntil(() -> bootstrap.status() == StateMonitor.Status.up, "failed waiting for status 'up'");
@@ -135,10 +129,10 @@ public class ConfigServerBootstrapTest {
                                            .resolve("sessions/2/services.xml"));
 
         RpcServer rpcServer = createRpcServer(configserverConfig);
-        ConfigServerBootstrap bootstrap = createBootstrap(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY);
+        Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY);
         assertFalse(bootstrap.vipStatus().isInRotation());
         // Call method directly, to be sure that it is finished redeploying all applications and we can check status
-        bootstrap.start();
+        bootstrap.doStart();
         // App is invalid, bootstrapping was unsuccessful. Status should be 'initializing',
         // rpc server should not be running and it should be out of rotation
         assertEquals(StateMonitor.Status.initializing, bootstrap.status());
@@ -173,29 +167,21 @@ public class ConfigServerBootstrapTest {
         curator.set(Path.fromString("/config/v2/tenants/" + applicationId.tenant().value() + "/sessions/2/version"), Utf8.toBytes("1.2.2"));
 
         RpcServer rpcServer = createRpcServer(configserverConfig);
-        ConfigServerBootstrap bootstrap = createBootstrap(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY);
-        bootstrap.start();
+        Bootstrapper bootstrap = createBootstrapper(tester, rpcServer, VIP_STATUS_PROGRAMMATICALLY);
+        bootstrap.doStart();
         waitUntil(rpcServer::isRunning, "failed waiting for Rpc server running");
         assertTrue(rpcServer.isServingConfigRequests());
         waitUntil(() -> bootstrap.status() == StateMonitor.Status.up, "failed waiting for status 'up'");
         waitUntil(() -> bootstrap.vipStatus().isInRotation(), "failed waiting for server to be in rotation");
     }
 
-    private ConfigServerBootstrap createBootstrap(DeployTester tester, RpcServer rpcServer, VipStatusMode vipStatusMode) throws IOException {
-        VersionState versionState = createVersionState();
+    private Bootstrapper createBootstrapper(DeployTester tester, RpcServer rpcServer, VipStatusMode vipStatusMode) throws IOException {
+        VersionState versionState = createVersionState(tester.curator());
         assertTrue(versionState.isUpgraded());
 
         StateMonitor stateMonitor = StateMonitor.createForTesting();
         VipStatus vipStatus = createVipStatus(stateMonitor);
-        return new ConfigServerBootstrap(tester.applicationRepository(),
-                                         rpcServer,
-                                         versionState,
-                                         stateMonitor,
-                                         vipStatus,
-                                         vipStatusMode,
-                                         new InMemoryFlagSource(),
-                                         new ConfigConvergenceChecker(),
-                                         clock);
+        return new Bootstrapper(tester.applicationRepository(), rpcServer, versionState, stateMonitor, vipStatus, vipStatusMode);
     }
 
     private void waitUntil(BooleanSupplier booleanSupplier, String messageIfWaitingFails) throws InterruptedException {
@@ -248,7 +234,7 @@ public class ConfigServerBootstrapTest {
                              new NullMetric());
     }
 
-    private VersionState createVersionState() throws IOException {
+    private VersionState createVersionState(Curator curator) throws IOException {
         return new VersionState(temporaryFolder.newFile(), curator);
     }
 
@@ -276,4 +262,26 @@ public class ConfigServerBootstrapTest {
         }
     }
 
+    private static class Bootstrapper extends ConfigServerBootstrap {
+
+        public Bootstrapper(ApplicationRepository applicationRepository,
+                            RpcServer server,
+                            VersionState versionState,
+                            StateMonitor stateMonitor,
+                            VipStatus vipStatus,
+                            VipStatusMode vipStatusMode) {
+            super(applicationRepository, server, versionState, stateMonitor, vipStatus, CONTINUE, vipStatusMode);
+        }
+
+        @Override
+        public void start() {
+            // Do nothing, avoids bootstrapping apps in constructor, use doBootstrap() below to really bootstrap apps
+        }
+
+        public void doStart() {
+            super.start();
+        }
+
+    }
+
 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java
index 4b03847571f..62bb4a77916 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java
@@ -172,6 +172,10 @@ public class DeployTester {
         return applicationRepository;
     }
 
+    public Curator curator() {
+        return tenantRepository.getCurator();
+    }
+
     private static HostProvisioner createProvisioner() {
         return new InMemoryProvisioner(9, false);
     }