Check config convergence before restarting services

Controlled by feature flag

6 files changed, 124 insertions, 9 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 3464a31a669..bde5e73d59d 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
@@ -218,6 +218,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         private Metric metric = new NullMetric();
         private SecretStoreValidator secretStoreValidator = new SecretStoreValidator(new SecretStoreProvider().get());
         private FlagSource flagSource = new InMemoryFlagSource();
+        private ConfigConvergenceChecker configConvergenceChecker = new ConfigConvergenceChecker();
 
         public Builder withTenantRepository(TenantRepository tenantRepository) {
             this.tenantRepository = tenantRepository;
@@ -281,11 +282,16 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
             return this;
         }
 
+        public Builder withConfigConvergenceChecker(ConfigConvergenceChecker configConvergenceChecker) {
+            this.configConvergenceChecker = configConvergenceChecker;
+            return this;
+        }
+
         public ApplicationRepository build() {
             return new ApplicationRepository(tenantRepository,
                                              hostProvisioner,
                                              InfraDeployerProvider.empty().getInfraDeployer(),
-                                             new ConfigConvergenceChecker(),
+                                             configConvergenceChecker,
                                              httpProxy,
                                              configserverConfig,
                                              orchestrator,
@@ -1044,6 +1050,12 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return getTenant(appId).getSessionRepository().getActiveApplicationSet(appId);
     }
 
+    public Application getActiveApplication(ApplicationId applicationId) {
+        return getActiveApplicationSet(applicationId)
+                .map(a -> a.getForVersionOrLatest(Optional.empty(), clock.instant()))
+                .orElseThrow(() -> new RuntimeException("Found no active application for " + applicationId));
+    }
+
     private File decompressApplication(InputStream in, String contentType, File tempDir) {
         try (CompressedApplicationInputStream application =
                      CompressedApplicationInputStream.createFromCompressedStream(in, contentType)) {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
index 4b7e39f48a4..be5867d132f 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
@@ -19,15 +19,20 @@ import com.yahoo.vespa.config.server.ApplicationRepository;
 import com.yahoo.vespa.config.server.ApplicationRepository.ActionTimer;
 import com.yahoo.vespa.config.server.ApplicationRepository.Activation;
 import com.yahoo.vespa.config.server.TimeoutBudget;
+import com.yahoo.vespa.config.server.application.Application;
+import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker;
 import com.yahoo.vespa.config.server.configchange.ConfigChangeActions;
 import com.yahoo.vespa.config.server.configchange.ReindexActions;
 import com.yahoo.vespa.config.server.configchange.RestartActions;
 import com.yahoo.vespa.config.server.session.PrepareParams;
 import com.yahoo.vespa.config.server.session.Session;
 import com.yahoo.vespa.config.server.tenant.Tenant;
+import com.yahoo.vespa.flags.BooleanFlag;
+import com.yahoo.vespa.flags.Flags;
 
 import java.time.Clock;
 import java.time.Duration;
+import java.time.Instant;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
@@ -35,6 +40,8 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
+import static com.yahoo.vespa.config.server.application.ConfigConvergenceChecker.ServiceListResponse;
+
 /**
  * The process of deploying an application.
  * Deployments are created by an {@link ApplicationRepository}.
@@ -149,6 +156,9 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
         RestartActions restartActions = configChangeActions.getRestartActions().useForInternalRestart(internalRedeploy);
 
         if ( ! restartActions.isEmpty()) {
+
+            waitForConfigToConverge(applicationId);
+
             Set<String> hostnames = restartActions.getEntries().stream()
                                                   .flatMap(entry -> entry.getServices().stream())
                                                   .map(ServiceInfo::getHostName)
@@ -165,6 +175,31 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
         }
     }
 
+    private void waitForConfigToConverge(ApplicationId applicationId) {
+        BooleanFlag verify = Flags.CHECK_CONFIG_CONVERGENCE_BEFORE_RESTARTING.bindTo(applicationRepository.flagSource());
+        if ( ! verify.value()) return;
+
+        Instant end = clock.instant().plus(Duration.ofMinutes(10));
+        // Timeout per service when getting config generations
+        Duration timeout = Duration.ofSeconds(10);
+        do {
+            Application app = applicationRepository.getActiveApplication(applicationId);
+            log.info("Wait for services in " + applicationId + " to converge on new generation before restarting");
+            ConfigConvergenceChecker convergenceChecker = applicationRepository.configConvergenceChecker();
+            ServiceListResponse response = convergenceChecker.getConfigGenerationsForAllServices(app, timeout);
+            if (response.converged) {
+                log.info("services converged on new generation " + response.currentGeneration);
+                return;
+            } else {
+                log.info("services not converged on new generation, wanted generation: " + response.wantedGeneration +
+                                 ", current generation: " + response.currentGeneration + ", will retry");
+                try { Thread.sleep(10_000); } catch (InterruptedException e) { /* ignore */ }
+            }
+        } while (clock.instant().isBefore(end));
+
+        throw new RuntimeException("Config has not converged");
+    }
+
     private void storeReindexing(ApplicationId applicationId, long requiredSession) {
         applicationRepository.modifyReindexing(applicationId, reindexing -> {
             if (configChangeActions != null)
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
index 7baad75ebc5..d3ddcc95468 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
@@ -103,6 +103,7 @@ public class ApplicationRepositoryTest {
     private OrchestratorMock orchestrator;
     private TimeoutBudget timeoutBudget;
     private Curator curator;
+    private ConfigserverConfig configserverConfig;
 
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
@@ -114,7 +115,7 @@ public class ApplicationRepositoryTest {
     @Before
     public void setup() throws IOException {
         curator = new MockCurator();
-        ConfigserverConfig configserverConfig = new ConfigserverConfig.Builder()
+        configserverConfig = new ConfigserverConfig.Builder()
                 .payloadCompressionType(ConfigserverConfig.PayloadCompressionType.Enum.UNCOMPRESSED)
                 .configServerDBDir(temporaryFolder.newFolder().getAbsolutePath())
                 .configDefinitionsDir(temporaryFolder.newFolder().getAbsolutePath())
@@ -187,6 +188,16 @@ public class ApplicationRepositoryTest {
 
     @Test
     public void prepareAndActivateWithRestart() {
+        applicationRepository = new ApplicationRepository.Builder()
+                .withTenantRepository(tenantRepository)
+                .withProvisioner(provisioner)
+                .withConfigserverConfig(configserverConfig)
+                .withOrchestrator(orchestrator)
+                .withLogRetriever(new MockLogRetriever())
+                .withClock(clock)
+                .withConfigConvergenceChecker(new MockConfigConvergenceChecker(2))
+                .build();
+
         prepareAndActivate(testAppJdiscOnly);
         PrepareResult result = prepareAndActivate(testAppJdiscOnlyRestart);
         assertTrue(result.configChangeActions().getRefeedActions().isEmpty());
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/MockConfigConvergenceChecker.java b/configserver/src/test/java/com/yahoo/vespa/config/server/MockConfigConvergenceChecker.java
new file mode 100644
index 00000000000..b21d89fa626
--- /dev/null
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/MockConfigConvergenceChecker.java
@@ -0,0 +1,33 @@
+package com.yahoo.vespa.config.server;
+
+import com.yahoo.config.model.api.ServiceInfo;
+import com.yahoo.vespa.config.server.application.Application;
+import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker;
+
+import java.time.Duration;
+import java.util.Map;
+
+public class MockConfigConvergenceChecker extends ConfigConvergenceChecker {
+
+    private final long wantedGeneration;
+
+    public MockConfigConvergenceChecker(long wantedGeneration) {
+        this.wantedGeneration = wantedGeneration;
+    }
+
+    @Override
+    public Map<ServiceInfo, Long> getServiceConfigGenerations(Application application, Duration timeoutPerService) {
+        return Map.of();
+    }
+
+    @Override
+    public ServiceListResponse getConfigGenerationsForAllServices(Application application, Duration timeoutPerService) {
+        return new ServiceListResponse(Map.of(), wantedGeneration, wantedGeneration);
+    }
+
+    @Override
+    public ServiceResponse getServiceConfigGeneration(Application application, String hostAndPortToCheck, Duration timeout) {
+        return new ServiceResponse(ServiceResponse.Status.ok, wantedGeneration);
+    }
+
+}
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 f16b1102f2a..7f223fbce6f 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
@@ -24,6 +24,7 @@ import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.config.server.ApplicationRepository;
 import com.yahoo.vespa.config.server.MockProvisioner;
 import com.yahoo.vespa.config.server.TimeoutBudget;
+import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker;
 import com.yahoo.vespa.config.server.application.OrchestratorMock;
 import com.yahoo.vespa.config.server.filedistribution.MockFileDistributionFactory;
 import com.yahoo.vespa.config.server.http.v2.PrepareResult;
@@ -37,9 +38,10 @@ import com.yahoo.vespa.config.server.tenant.TenantRepository;
 import com.yahoo.vespa.config.server.tenant.TestTenantRepository;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
+import com.yahoo.vespa.flags.FlagSource;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.VespaModelFactory;
-import com.yahoo.vespa.orchestrator.Orchestrator;
 
 import java.io.File;
 import java.nio.file.Files;
@@ -253,7 +255,8 @@ public class DeployTester {
         private Curator curator = new MockCurator();
         private Metrics metrics;
         private List<ModelFactory> modelFactories;
-        private Orchestrator orchestrator;
+        private ConfigConvergenceChecker configConvergenceChecker = new ConfigConvergenceChecker();
+        private FlagSource flagSource = new InMemoryFlagSource();
 
         public DeployTester build() {
             Clock clock = Optional.ofNullable(this.clock).orElseGet(Clock::systemUTC);
@@ -285,9 +288,11 @@ public class DeployTester {
             ApplicationRepository applicationRepository = new ApplicationRepository.Builder()
                     .withTenantRepository(tenantRepository)
                     .withConfigserverConfig(configserverConfig)
-                    .withOrchestrator(Optional.ofNullable(orchestrator).orElseGet(OrchestratorMock::new))
+                    .withOrchestrator(new OrchestratorMock())
                     .withClock(clock)
                     .withProvisioner(provisioner)
+                    .withConfigConvergenceChecker(configConvergenceChecker)
+                    .withFlagSource(flagSource)
                     .build();
 
             return new DeployTester(clock, tenantRepository, applicationRepository);
@@ -336,10 +341,16 @@ public class DeployTester {
             return this;
         }
 
-        public Builder orchestrator(Orchestrator orchestrator) {
-            this.orchestrator = orchestrator;
+        public Builder configConvergenceChecker(ConfigConvergenceChecker configConvergenceChecker) {
+            this.configConvergenceChecker = configConvergenceChecker;
             return this;
         }
+
+        public Builder flagSource(FlagSource flagSource) {
+            this.flagSource = flagSource;
+            return this;
+        }
+
     }
 
 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java
index 33e18843738..143f9a8e80d 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java
@@ -22,6 +22,7 @@ import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.test.ManualClock;
+import com.yahoo.vespa.config.server.MockConfigConvergenceChecker;
 import com.yahoo.vespa.config.server.application.ApplicationReindexing;
 import com.yahoo.vespa.config.server.http.InternalServerException;
 import com.yahoo.vespa.config.server.http.InvalidApplicationException;
@@ -29,6 +30,8 @@ import com.yahoo.vespa.config.server.http.UnknownVespaVersionException;
 import com.yahoo.vespa.config.server.http.v2.PrepareResult;
 import com.yahoo.vespa.config.server.model.TestModelFactory;
 import com.yahoo.vespa.config.server.session.PrepareParams;
+import com.yahoo.vespa.flags.Flags;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.model.application.validation.change.VespaReindexAction;
 import com.yahoo.vespa.model.application.validation.change.VespaRestartAction;
 import org.junit.Rule;
@@ -432,7 +435,15 @@ public class HostedDeployTest {
                                                                           "reindex please", services, "music"),
                                                     new VespaRestartAction(ClusterSpec.Id.from("test"), "change", services)));
 
-        DeployTester tester = createTester(hosts, modelFactories, prodZone, clock);
+        DeployTester tester = new DeployTester.Builder()
+                .modelFactories(modelFactories)
+                .configserverConfig(createConfigserverConfig(prodZone))
+                .clock(clock)
+                .zone(prodZone)
+                .hostProvisioner(new InMemoryProvisioner(new Hosts(hosts), true, false))
+                .configConvergenceChecker(new MockConfigConvergenceChecker(2))
+                .flagSource(new InMemoryFlagSource().withBooleanFlag(Flags.CHECK_CONFIG_CONVERGENCE_BEFORE_RESTARTING.id(), true))
+                .build();
         PrepareResult prepareResult = tester.deployApp("src/test/apps/hosted/", "6.1.0");
 
         assertEquals(7, tester.getAllocatedHostsOf(tester.applicationId()).getHosts().size());
@@ -480,7 +491,9 @@ public class HostedDeployTest {
                 .configserverConfig(createConfigserverConfig(prodZone))
                 .clock(clock)
                 .zone(prodZone)
-                .hostProvisioner(new InMemoryProvisioner(new Hosts(hosts), true, false)).build();
+                .hostProvisioner(new InMemoryProvisioner(new Hosts(hosts), true, false))
+                .configConvergenceChecker(new MockConfigConvergenceChecker(2))
+                .build();
     }
 
     private static class ConfigChangeActionsModelFactory extends TestModelFactory {