Require zone-application config to converge when upgrading

4 files changed, 43 insertions, 3 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java
index 8603698d498..e9dd093fae8 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java
@@ -3,7 +3,9 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.yahoo.component.Version;
 import com.yahoo.vespa.hosted.controller.Controller;
+import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
+import com.yahoo.vespa.hosted.controller.api.integration.configserver.ServiceConvergence;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
@@ -52,7 +54,8 @@ public class SystemUpgrader extends Maintainer {
                     log.log(Level.WARNING, e.getMessage() + ". Continuing to next parallel deployed zone");
                 } catch (Exception e) {
                     converged = false;
-                    log.log(Level.WARNING, "Failed to upgrade " + zone + ". Continuing to next parallel deployed zone", e);
+                    log.log(Level.WARNING, "Failed to upgrade " + zone +
+                                           ". Continuing to next parallel deployed zone", e);
                 }
             }
             if (!converged) {
@@ -68,11 +71,21 @@ public class SystemUpgrader extends Maintainer {
             if (convergedOn(target, application.dependencies(), zone)) {
                 deploy(target, application, zone);
             }
-            converged &= convergedOn(target, application, zone);
+            converged &= convergedOn(target, application, zone) & configConverged(application, zone);
         }
         return converged;
     }
 
+    /** Returns whether config for given application has converged */
+    private boolean configConverged(SystemApplication application, ZoneId zone) {
+        if (!application.hasApplicationPackage()) {
+            return true;
+        }
+        return controller().configServer().serviceConvergence(new DeploymentId(application.id(), zone))
+                           .map(ServiceConvergence::converged)
+                           .orElse(false);
+    }
+
     /** Deploy application on given version idempotently */
     private void deploy(Version target, SystemApplication application, ZoneId zone) {
         if (!wantedVersion(zone, application, target).equals(target)) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
index 86950f9a61c..dcb7947728c 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
@@ -8,8 +8,8 @@ import com.yahoo.component.Vtag;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
-import com.yahoo.vespa.hosted.controller.api.integration.github.GitSha;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
+import com.yahoo.vespa.hosted.controller.api.integration.github.GitSha;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.ApplicationList;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerMock.java
index ac9afd9752a..a023abd1a1d 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerMock.java
@@ -16,6 +16,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServ
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse;
+import com.yahoo.vespa.hosted.controller.api.integration.configserver.ServiceConvergence;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
 import com.yahoo.vespa.serviceview.bindings.ApplicationView;
@@ -44,6 +45,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
     private final Map<String, EndpointStatus> endpoints = new HashMap<>();
     private final Map<URI, Version> versions = new HashMap<>();
     private final NodeRepositoryMock nodeRepository = new NodeRepositoryMock();
+    private final Map<DeploymentId, ServiceConvergence> serviceStatus = new HashMap<>();
 
     private Version initialVersion = new Version(6, 1, 0);
     private Version lastPrepareVersion = null;
@@ -77,6 +79,11 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
         }
     }
 
+    /** Converge all services belonging to the given application */
+    public void convergeServices(ApplicationId application, ZoneId zone) {
+        serviceStatus.put(new DeploymentId(application, zone), new ServiceConvergence(application, zone, true));
+    }
+
     /** The version given in the previous prepare call, or empty if no call has been made */
     public Optional<Version> lastPrepareVersion() {
         return Optional.ofNullable(lastPrepareVersion);
@@ -121,6 +128,11 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
     }
 
     @Override
+    public Optional<ServiceConvergence> serviceConvergence(DeploymentId deployment) {
+        return Optional.ofNullable(serviceStatus.get(deployment));
+    }
+
+    @Override
     public PreparedApplication deploy(DeploymentId deployment, DeployOptions deployOptions, Set<String> rotationCnames,
                                        Set<String> rotationNames, byte[] content) {
         lastPrepareVersion = deployOptions.vespaVersion.map(Version::fromString).orElse(null);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java
index 5bfda19863f..cddc0f5603e 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java
@@ -77,6 +77,11 @@ public class SystemUpgraderTest {
         assertWantedVersion(SystemApplication.configServer, version1, zone2, zone3, zone4);
         assertWantedVersion(SystemApplication.zone, version1, zone2, zone3, zone4);
 
+        // zone 2 and 3: upgrade does not start until zone 1 zone-application config converges
+        tester.systemUpgrader().maintain();
+        assertWantedVersion(SystemApplication.configServer, version1, zone2, zone3);
+        convergeServices(SystemApplication.zone, zone1);
+
         // zone 2 and 3: zone-config-server upgrades, first in zone 2, then in zone 3
         tester.systemUpgrader().maintain();
         assertWantedVersion(SystemApplication.configServer, version2, zone2, zone3);
@@ -94,6 +99,7 @@ public class SystemUpgraderTest {
         tester.systemUpgrader().maintain();
         assertWantedVersion(SystemApplication.zone, version2, zone2, zone3);
         completeUpgrade(SystemApplication.zone, version2, zone2, zone3);
+        convergeServices(SystemApplication.zone, zone2, zone3);
 
         // zone 4: zone-config-server upgrades
         tester.systemUpgrader().maintain();
@@ -146,6 +152,7 @@ public class SystemUpgraderTest {
         completeUpgrade(allExceptZone, version2, zone1);
         tester.systemUpgrader().maintain();
         completeUpgrade(SystemApplication.zone, version2, zone1);
+        convergeServices(SystemApplication.zone, zone1);
         assertWantedVersion(SystemApplication.all(), version1, zone2, zone3, zone4);
 
         // zone 2 and 3:
@@ -153,6 +160,7 @@ public class SystemUpgraderTest {
         completeUpgrade(allExceptZone, version2, zone2, zone3);
         tester.systemUpgrader().maintain();
         completeUpgrade(SystemApplication.zone, version2, zone2, zone3);
+        convergeServices(SystemApplication.zone, zone2, zone3);
         assertWantedVersion(SystemApplication.all(), version1, zone4);
 
         // zone 4:
@@ -194,10 +202,17 @@ public class SystemUpgraderTest {
                 nodeRepository().add(zone, new Node(node.hostname(), node.state(), node.type(), node.owner(),
                                                     node.wantedVersion(), node.wantedVersion()));
             }
+
             assertCurrentVersion(application, version, zone);
         }
     }
 
+    private void convergeServices(SystemApplication application, ZoneId... zones) {
+        for (ZoneId zone : zones) {
+            tester.controllerTester().configServer().convergeServices(application.id(), zone);
+        }
+    }
+
     private void completeUpgrade(List<SystemApplication> applications, Version version, ZoneId... zones) {
         applications.forEach(application -> completeUpgrade(application, version, zones));
     }