Set timeout for internal deployment in one place

Use barrier timeout as basis for timeout, in the same way we do for external
deployments.

9 files changed, 41 insertions, 8 deletions

diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java
index 77faa45ebe5..111073aca77 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java
@@ -11,8 +11,20 @@ import java.util.Optional;
  */
 public interface Deployer {
 
+
+    /**
+     * Creates a new deployment from the active application, if available. Will use the default timeout for deployment.
+     *
+     * @param application the active application to be redeployed
+     * @return a new deployment from the local active, or empty if a local active application
+     * was not present for this id (meaning it either is not active or active on another
+     * node in the config server cluster)
+     */
+    Optional<Deployment> deployFromLocalActive(ApplicationId application);
+
     /**
-     * Creates a new deployment from the active application, if available.
+     * Creates a new deployment from the active application, if available. Prefer {@link #deployFromLocalActive(ApplicationId)}
+     * if possible, this method is for testing and will override the default timeout for deployment.
      *
      * @param application the active application to be redeployed
      * @param timeout the timeout to use for each individual deployment operation
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 13401d4a4a6..a4dd943aa47 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
@@ -112,6 +112,18 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
      * Creates a new deployment from the active application, if available.
      *
      * @param application the active application to be redeployed
+     * @return a new deployment from the local active, or empty if a local active application
+     *         was not present for this id (meaning it either is not active or active on another
+     *         node in the config server cluster)
+     */
+    public Optional<com.yahoo.config.provision.Deployment> deployFromLocalActive(ApplicationId application) {
+        return deployFromLocalActive(application, Duration.ofSeconds(configserverConfig.zookeeper().barrierTimeout()).plus(Duration.ofSeconds(5)));
+    }
+
+    /**
+     * Creates a new deployment from the active application, if available.
+     *
+     * @param application the active application to be redeployed
      * @param timeout the timeout to use for each individual deployment operation
      * @return a new deployment from the local active, or empty if a local active application
      *         was not present for this id (meaning it either is not active or active on another
@@ -352,7 +364,7 @@ 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, Duration.ofMinutes(30))
+        deployer.deployFromLocalActive(applicationId)
                 .ifPresent(deployment -> deploymentExecutor.execute(() -> {
                     try {
                         deployment.activate();
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 384ae0853d8..5d573323bb6 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
@@ -29,7 +29,6 @@ import java.io.File;
 import java.io.FileReader;
 import java.time.Clock;
 import java.util.ArrayList;
-import java.util.Optional;
 
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertFalse;
@@ -71,7 +70,7 @@ public class ConfigServerBootstrapTest extends TestWithTenant {
         VersionState versionState = new VersionState(versionFile);
         assertTrue(versionState.isUpgraded());
         ConfigServerBootstrap bootstrap =
-                new ConfigServerBootstrap(applicationRepository, rpc,  (application, timeout) -> Optional.empty(), versionState,
+                new ConfigServerBootstrap(applicationRepository, rpc,  new MockDeployer(), versionState,
                                           new StateMonitor(new HealthMonitorConfig(new HealthMonitorConfig.Builder()), new SystemTimer()));
         waitUntilStarted(rpc, 60000);
         assertFalse(versionState.isUpgraded());
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/MockDeployer.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/MockDeployer.java
index 4fc54943a79..051e7c9a8f9 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/MockDeployer.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/MockDeployer.java
@@ -15,6 +15,11 @@ public class MockDeployer implements com.yahoo.config.provision.Deployer {
     public ApplicationId lastDeployed;
 
     @Override
+    public Optional<Deployment> deployFromLocalActive(ApplicationId application) {
+        return deployFromLocalActive(application, Duration.ofSeconds(60));
+    }
+
+    @Override
     public Optional<Deployment> deployFromLocalActive(ApplicationId application, Duration timeout) {
         lastDeployed = application;
         return Optional.empty();
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java
index 500861ad0d2..6a2376d748b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java
@@ -75,7 +75,7 @@ public abstract class ApplicationMaintainer extends Maintainer {
         // Lock is acquired with a low timeout to reduce the chance of colliding with an external deployment.
         try (Mutex lock = nodeRepository().lock(application, Duration.ofSeconds(1))) {
             if ( ! isActive(application)) return; // became inactive since deployment was requested
-            Optional<Deployment> deployment = deployer.deployFromLocalActive(application, Duration.ofMinutes(30));
+            Optional<Deployment> deployment = deployer.deployFromLocalActive(application);
             if ( ! deployment.isPresent()) return; // this will be done at another config server
 
             deployment.get().activate();
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirer.java
index 12b63f66d3f..30b5f6f737d 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRetirer.java
@@ -123,7 +123,7 @@ public class NodeRetirer extends Maintainer {
                             entry -> filterRetireableNodes(entry.getValue())));
             if (retireableNodesByCluster.values().stream().mapToInt(Set::size).sum() == 0) continue;
 
-            Optional<Deployment> deployment = deployer.deployFromLocalActive(applicationId, Duration.ofMinutes(30));
+            Optional<Deployment> deployment = deployer.deployFromLocalActive(applicationId);
             if ( ! deployment.isPresent()) continue; // this will be done at another config server
 
             Set<Node> replaceableNodes = retireableNodesByCluster.entrySet().stream()
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredEarlyExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredEarlyExpirer.java
index cb2fcb89284..ca0594e346c 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredEarlyExpirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredEarlyExpirer.java
@@ -51,7 +51,7 @@ public class RetiredEarlyExpirer extends Maintainer {
             List<Node> retiredNodes = entry.getValue();
 
             try {
-                Optional<Deployment> deployment = deployer.deployFromLocalActive(application, Duration.ofMinutes(30));
+                Optional<Deployment> deployment = deployer.deployFromLocalActive(application);
                 if ( ! deployment.isPresent()) continue; // this will be done at another config server
 
                 List<Node> nodesToRemove = new ArrayList<>();
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java
index 9ef09858bfa..4c5c8adf576 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java
@@ -50,7 +50,7 @@ public class RetiredExpirer extends Expirer {
             ApplicationId application = entry.getKey();
             List<Node> nodesToRemove = entry.getValue();
             try {
-                Optional<Deployment> deployment = deployer.deployFromLocalActive(application, Duration.ofMinutes(30));
+                Optional<Deployment> deployment = deployer.deployFromLocalActive(application);
                 if ( ! deployment.isPresent()) continue; // this will be done at another config server
 
                 nodeRepository.setRemovable(application, nodesToRemove);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
index a05b0be344d..c880c66abc7 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
@@ -46,6 +46,11 @@ public class MockDeployer implements Deployer {
     }
 
     @Override
+    public Optional<Deployment> deployFromLocalActive(ApplicationId application) {
+        return deployFromLocalActive(application, Duration.ofSeconds(60));
+    }
+
+    @Override
     public Optional<Deployment> deployFromLocalActive(ApplicationId id, Duration timeout) {
         return Optional.of(new MockDeployment(provisioner, applications.get(id)));
     }