From dbecd3c5e27fa0216790635791dabb2b720499bf Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Tue, 4 Oct 2022 12:31:10 +0200 Subject: Simplify by moving some test code into test --- .../vespa/config/server/ConfigServerBootstrap.java | 53 +++++++-------------- .../config/server/ConfigServerBootstrapTest.java | 55 ++++++++++++++-------- 2 files changed, 52 insertions(+), 56 deletions(-) (limited to 'configserver') 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..4ba27d91deb 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,9 +1,9 @@ // 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; @@ -16,12 +16,10 @@ 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 +35,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 +54,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 } @@ -81,26 +75,15 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable 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()); + this(applicationRepository, server, versionState, stateMonitor, vipStatus, EXIT_JVM, + vipStatusMode(applicationRepository), flagSource, convergence, Clock.systemUTC()); } - // 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, FlagSource flagSource, ConfigConvergenceChecker convergence, + Clock clock) { this.applicationRepository = applicationRepository; this.server = server; this.versionState = versionState; @@ -112,25 +95,15 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable this.exitIfRedeployingApplicationsFails = exitIfRedeployingApplicationsFails; this.clock = clock; rpcServerExecutor = Executors.newSingleThreadExecutor(new DaemonThreadFactory("config server RPC server")); - configServerMaintenance = new ConfigServerMaintenance(configserverConfig, applicationRepository, applicationRepository.tenantRepository().getCurator(), flagSource, convergence); 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 +298,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/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java index 9a90d517ca5..c8c5bd8cf48 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 @@ -30,10 +30,10 @@ 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; +import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.Arrays; @@ -43,6 +43,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; @@ -75,14 +76,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 +111,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 +136,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 +174,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 { + private Bootstrapper createBootstrapper(DeployTester tester, RpcServer rpcServer, VipStatusMode vipStatusMode) throws IOException { VersionState versionState = createVersionState(); 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, clock); } private void waitUntil(BooleanSupplier booleanSupplier, String messageIfWaitingFails) throws InterruptedException { @@ -276,4 +269,28 @@ public class ConfigServerBootstrapTest { } } + private static class Bootstrapper extends ConfigServerBootstrap { + + public Bootstrapper(ApplicationRepository applicationRepository, + RpcServer server, + VersionState versionState, + StateMonitor stateMonitor, + VipStatus vipStatus, + VipStatusMode vipStatusMode, + Clock clock) { + super(applicationRepository, server, versionState, stateMonitor, vipStatus, CONTINUE, vipStatusMode, + new InMemoryFlagSource(), new ConfigConvergenceChecker(), clock); + } + + @Override + public void start() { + // Do nothing, avoids bootstrapping apps in constructor, use doBootstrap() below to really bootstrap apps + } + + public void doStart() { + super.start(); + } + + } + } -- cgit v1.2.3