Merge pull request #5945 from vespa-engine/jvenstad/deblock-system-app-order-across-parallel-zones

Jvenstad/deblock system app order across parallel zones

2 files changed, 23 insertions, 18 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 9965d88c64e..516cd52d710 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
@@ -50,11 +50,16 @@ public class SystemUpgrader extends Maintainer {
     private void deploy(List<SystemApplication> applications, Version target) {
         for (List<ZoneId> zones : controller().zoneRegistry().upgradePolicy().asList()) {
             boolean converged = true;
-            for (SystemApplication application : applications) {
-                if (application.dependencies().stream().allMatch(dep -> convergedOn(target, dep, zones))) {
-                    deploy(zones, application, target);
+            for (ZoneId zone : zones) {
+                for (SystemApplication application : applications) {
+                    boolean dependenciesConverged = application.dependencies().stream()
+                                                               .filter(applications::contains) // TODO: Remove when all() is used.
+                                                               .allMatch(dependency -> currentVersion(zone, dependency.id()).equals(target));
+                    if (dependenciesConverged) {
+                        deploy(target, application, zone);
+                    }
+                    converged &= currentVersion(zone, application.id()).equals(target);
                 }
-                converged &= convergedOn(target, application, zones);
             }
             if (!converged) {
                 break;
@@ -63,25 +68,19 @@ public class SystemUpgrader extends Maintainer {
     }
 
     /** Deploy application on given version idempotently */
-    private void deploy(List<ZoneId> zones, SystemApplication application, Version target) {
-        for (ZoneId zone : zones) {
-            if (!wantedVersion(zone, application.id(), target).equals(target)) {
-                log.info(String.format("Deploying %s version %s in %s", application.id(), target, zone));
-                controller().applications().deploy(application, zone, target);
-            }
+    private void deploy(Version target, SystemApplication application, ZoneId zone) {
+        if (!wantedVersion(zone, application.id(), target).equals(target)) {
+            log.info(String.format("Deploying %s version %s in %s", application.id(), target, zone));
+            controller().applications().deploy(application, zone, target);
         }
     }
 
-    private boolean convergedOn(Version target, SystemApplication application, List<ZoneId> zones) {
-        return zones.stream().allMatch(zone -> currentVersion(zone, application.id(), target).equals(target));
-    }
-
     private Version wantedVersion(ZoneId zone, ApplicationId application, Version defaultVersion) {
         return minVersion(zone, application, Node::wantedVersion).orElse(defaultVersion);
     }
 
-    private Version currentVersion(ZoneId zone, ApplicationId application, Version defaultVersion) {
-        return minVersion(zone, application, Node::currentVersion).orElse(defaultVersion);
+    private Version currentVersion(ZoneId zone, ApplicationId application) {
+        return minVersion(zone, application, Node::currentVersion).orElse(Version.emptyVersion);
     }
 
     private Optional<Version> minVersion(ZoneId zone, ApplicationId application, Function<Node, Version> versionField) {
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 3fbda16cb4a..1ec64f8d478 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
@@ -79,12 +79,18 @@ public class SystemUpgraderTest {
         assertWantedVersion(SystemApplication.configServer, version1, zone2, zone3, zone4);
         assertWantedVersion(SystemApplication.zone, version1, zone2, zone3, zone4);
 
-        // zone 2 and 3: zone-config-server upgrades in parallel
+        // 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);
         assertWantedVersion(SystemApplication.configServer, version1, zone4);
         assertWantedVersion(SystemApplication.zone, version1, zone2, zone3, zone4);
-        completeUpgrade(SystemApplication.configServer, version2, zone2, zone3);
+        completeUpgrade(SystemApplication.configServer, version2, zone2);
+
+        // zone-application starts upgrading in zone 2, while zone-config-server completes upgrade in zone 3
+        tester.systemUpgrader().maintain();
+        assertWantedVersion(SystemApplication.zone, version2, zone2);
+        assertWantedVersion(SystemApplication.zone, version1, zone3);
+        completeUpgrade(SystemApplication.configServer, version2, zone3);
 
         // zone 2 and 3: zone-application upgrades in parallel
         tester.systemUpgrader().maintain();