Upgrade host applications

4 files changed, 103 insertions, 26 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java
index d3daa68741e..f264251361a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/SystemApplication.java
@@ -12,8 +12,11 @@ import java.util.List;
  *
  * @author mpolden
  */
-public enum  SystemApplication {
+public enum SystemApplication {
 
+    // Note that the enum declaration order decides the upgrade order
+    configServerHost(ApplicationId.from("hosted-vespa", "configserver-host", "default"), NodeType.confighost),
+    proxyHost(ApplicationId.from("hosted-vespa", "proxy-host", "default"), NodeType.proxyhost),
     configServer(ApplicationId.from("hosted-vespa", "zone-config-servers", "default"), NodeType.config),
     zone(ApplicationId.from("hosted-vespa", "routing", "default"), NodeType.proxy);
 
@@ -34,11 +37,16 @@ public enum  SystemApplication {
         return nodeType;
     }
 
-    /** Returns whether this system application has its own application package */
+    /** Returns whether this system application has an application package */
     public boolean hasApplicationPackage() {
         return nodeType == NodeType.proxy;
     }
 
+    /** Returns whether this system application must be upgraded in the declared order */
+    public boolean upgradeInOrder() {
+        return nodeType != NodeType.confighost && nodeType != NodeType.proxyhost;
+    }
+
     /** All known system applications */
     public static List<SystemApplication> all() {
         return Arrays.asList(values());
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 d7fc71e4b08..d6addf19b44 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
@@ -37,51 +37,53 @@ public class SystemUpgrader extends Maintainer {
         if (!target.isPresent()) {
             return;
         }
+        deploy(SystemApplication.all(), target.get());
+    }
+
+    /** Deploy a list of system applications on given version */
+    private void deploy(List<SystemApplication> applications, Version target) {
         for (List<ZoneId> zones : controller().zoneRegistry().upgradePolicy().asList()) {
-            // The order here is important. Config servers should always upgrade first
-            if (!deploy(zones, SystemApplication.configServer, target.get())) {
-                break;
-            }
-            if (!deploy(zones, SystemApplication.zone, target.get())) {
-                break;
+            for (SystemApplication application : applications) {
+                if (!deploy(zones, application, target) && application.upgradeInOrder()) {
+                    return;
+                }
             }
         }
     }
 
     /** Deploy application on given version. Returns true when all allocated nodes are on requested version */
-    private boolean deploy(List<ZoneId> zones, SystemApplication application, Version version) {
+    private boolean deploy(List<ZoneId> zones, SystemApplication application, Version target) {
         boolean completed = true;
         for (ZoneId zone : zones) {
-            if (!wantedVersion(zone, application.id()).equals(version)) {
-                log.info(String.format("Deploying %s version %s in %s", application.id(), version, zone));
-                controller().applications().deploy(application, zone, version);
+            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);
             }
-            completed = completed && currentVersion(zone, application.id()).equals(version);
+            completed = completed && currentVersion(zone, application.id(), target).equals(target);
         }
         return completed;
     }
 
-    private Version wantedVersion(ZoneId zone, ApplicationId application) {
-        return minVersion(zone, application, Node::wantedVersion);
+    private Version wantedVersion(ZoneId zone, ApplicationId application, Version defaultVersion) {
+        return minVersion(zone, application, Node::wantedVersion).orElse(defaultVersion);
     }
 
-    private Version currentVersion(ZoneId zone, ApplicationId application) {
-        return minVersion(zone, application, Node::currentVersion);
+    private Version currentVersion(ZoneId zone, ApplicationId application, Version defaultVersion) {
+        return minVersion(zone, application, Node::currentVersion).orElse(defaultVersion);
     }
 
-    private Version minVersion(ZoneId zone, ApplicationId application, Function<Node, Version> versionField) {
+    private Optional<Version> minVersion(ZoneId zone, ApplicationId application, Function<Node, Version> versionField) {
         try {
             return controller().configServer()
                                .nodeRepository()
                                .listOperational(zone, application)
                                .stream()
                                .map(versionField)
-                               .min(Comparator.naturalOrder())
-                               .orElse(Version.emptyVersion);
+                               .min(Comparator.naturalOrder());
         } catch (Exception e) {
             log.log(Level.WARNING, String.format("Failed to get version for %s in %s: %s", application, zone,
                                                  Exceptions.toMessageString(e)));
-            return Version.emptyVersion;
+            return Optional.empty();
         }
     }
 
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 e9eec0682f3..99b913709df 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
@@ -54,13 +54,17 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
 
     @Inject
     public ConfigServerMock(ZoneRegistryMock zoneRegistry) {
-        bootstrap(zoneRegistry.zones().all().ids());
+        bootstrap(zoneRegistry.zones().all().ids(), SystemApplication.all());
     }
 
-    public void bootstrap(List<ZoneId> zones) {
+    public void bootstrap(List<ZoneId> zones, SystemApplication... applications) {
+        bootstrap(zones, Arrays.asList(applications));
+    }
+
+    public void bootstrap(List<ZoneId> zones, List<SystemApplication> applications) {
         nodeRepository().clear();
         for (ZoneId zone : zones) {
-            for (SystemApplication application : SystemApplication.all()) {
+            for (SystemApplication application : applications) {
                 List<Node> nodes = IntStream.rangeClosed(1, 3)
                                             .mapToObj(i -> new Node(
                                                     HostName.from("node-" + i + "-" + application.id().application()
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 5790ab2a0c8..54e9ace1142 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
@@ -39,7 +39,9 @@ public class SystemUpgraderTest {
         );
 
         Version version1 = Version.fromString("6.5");
-        tester.configServer().bootstrap(Arrays.asList(zone1, zone2, zone3, zone4));
+        // Bootstrap a system without host applications
+        tester.configServer().bootstrap(Arrays.asList(zone1, zone2, zone3, zone4), SystemApplication.configServer,
+                                        SystemApplication.zone);
         // Fail a few nodes. Failed nodes should not affect versions
         failNodeIn(zone1, SystemApplication.configServer);
         failNodeIn(zone3, SystemApplication.zone);
@@ -109,6 +111,54 @@ public class SystemUpgraderTest {
     }
 
     @Test
+    public void upgrade_system_containing_host_applications() {
+        tester.controllerTester().zoneRegistry().setUpgradePolicy(
+                UpgradePolicy.create()
+                             .upgrade(zone1)
+                             .upgradeInParallel(zone2, zone3)
+                             .upgrade(zone4)
+        );
+
+        Version version1 = Version.fromString("6.5");
+        tester.configServer().bootstrap(Arrays.asList(zone1, zone2, zone3, zone4), SystemApplication.all());
+        tester.upgradeSystem(version1);
+        tester.systemUpgrader().maintain();
+        assertCurrentVersion(SystemApplication.all(), version1, zone1, zone2, zone3, zone4);
+
+        // Controller upgrades
+        Version version2 = Version.fromString("6.6");
+        tester.upgradeController(version2);
+        assertEquals(version2, tester.controller().versionStatus().controllerVersion().get().versionNumber());
+
+        // System upgrades in zone 1:
+        tester.systemUpgrader().maintain();
+        List<SystemApplication> allExceptZone = Arrays.asList(SystemApplication.configServerHost,
+                                                              SystemApplication.proxyHost,
+                                                              SystemApplication.configServer);
+        completeUpgrade(allExceptZone, version2, zone1);
+        tester.systemUpgrader().maintain();
+        completeUpgrade(SystemApplication.zone, version2, zone1);
+        assertWantedVersion(SystemApplication.all(), version1, zone2, zone3, zone4);
+
+        // zone 2 and 3:
+        tester.systemUpgrader().maintain();
+        completeUpgrade(allExceptZone, version2, zone2, zone3);
+        tester.systemUpgrader().maintain();
+        completeUpgrade(SystemApplication.zone, version2, zone2, zone3);
+        assertWantedVersion(SystemApplication.all(), version1, zone4);
+
+        // zone 4:
+        tester.systemUpgrader().maintain();
+        completeUpgrade(allExceptZone, version2, zone4);
+        tester.systemUpgrader().maintain();
+        completeUpgrade(SystemApplication.zone, version2, zone4);
+
+        // All done
+        tester.systemUpgrader().maintain();
+        assertWantedVersion(SystemApplication.all(), version2, zone1, zone2, zone3, zone4);
+    }
+
+    @Test
     public void never_downgrades_system() {
         ZoneId zone = ZoneId.from("prod", "eu-west-1");
         tester.controllerTester().zoneRegistry().setUpgradePolicy(UpgradePolicy.create().upgrade(zone));
@@ -130,6 +180,7 @@ public class SystemUpgraderTest {
 
     /** Simulate upgrade of nodes allocated to given application. In a real system this is done by the node itself */
     private void completeUpgrade(SystemApplication application, Version version, ZoneId... zones) {
+        assertWantedVersion(application, version, zones);
         for (ZoneId zone : zones) {
             for (Node node : nodeRepository().listOperational(zone, application.id())) {
                 nodeRepository().add(zone, new Node(node.hostname(), node.state(), node.type(), node.owner(),
@@ -139,6 +190,10 @@ public class SystemUpgraderTest {
         }
     }
 
+    private void completeUpgrade(List<SystemApplication> applications, Version version, ZoneId... zones) {
+        applications.forEach(application -> completeUpgrade(application, version, zones));
+    }
+
     private void failNodeIn(ZoneId zone, SystemApplication application) {
         List<Node> nodes = nodeRepository().list(zone, application.id());
         if (nodes.isEmpty()) {
@@ -157,11 +212,19 @@ public class SystemUpgraderTest {
         assertVersion(application.id(), version, Node::currentVersion, zones);
     }
 
+    private void assertWantedVersion(List<SystemApplication> applications, Version version, ZoneId... zones) {
+        applications.forEach(application -> assertVersion(application.id(), version, Node::wantedVersion, zones));
+    }
+
+    private void assertCurrentVersion(List<SystemApplication> applications, Version version, ZoneId... zones) {
+        applications.forEach(application -> assertVersion(application.id(), version, Node::currentVersion, zones));
+    }
+
     private void assertVersion(ApplicationId application, Version version, Function<Node, Version> versionField,
                                ZoneId... zones) {
         for (ZoneId zone : zones) {
             for (Node node : nodeRepository().listOperational(zone, application)) {
-                assertEquals(version, versionField.apply(node));
+                assertEquals(application + " version", version, versionField.apply(node));
             }
         }
     }