Orchestrate controller OS upgrade as any other zone

10 files changed, 39 insertions, 18 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 1a1b6988a96..b742c45bf09 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
@@ -11,6 +11,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.ServiceCon
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
 
 import java.util.Arrays;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.Optional;
 
@@ -22,6 +23,7 @@ import java.util.Optional;
  */
 public enum SystemApplication {
 
+    controllerHost(ApplicationId.from("hosted-vespa", "controller-host", "default"), NodeType.controllerhost),
     configServerHost(ApplicationId.from("hosted-vespa", "configserver-host", "default"), NodeType.confighost),
     configServer(ApplicationId.from("hosted-vespa", "zone-config-servers", "default"), NodeType.config),
     proxyHost(ApplicationId.from("hosted-vespa", "proxy-host", "default"), NodeType.proxyhost),
@@ -81,7 +83,12 @@ public enum SystemApplication {
         return Optional.of(Endpoint.of(this, zone, zoneRegistry.getConfigServerVipUri(zone)));
     }
 
-    /** All known system applications */
+    /** All system applications that are not the controller */
+    public static List<SystemApplication> notController() {
+        return List.copyOf(EnumSet.complementOf(EnumSet.of(SystemApplication.controllerHost)));
+    }
+
+    /** All system applications */
     public static List<SystemApplication> all() {
         return List.of(values());
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java
index f61ae5ec077..9859d12510a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/InfrastructureUpgrader.java
@@ -14,6 +14,7 @@ import java.time.Duration;
 import java.util.Comparator;
 import java.util.EnumSet;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Function;
 import java.util.logging.Logger;
@@ -28,15 +29,18 @@ public abstract class InfrastructureUpgrader<VERSION> extends ControllerMaintain
     private static final Logger log = Logger.getLogger(InfrastructureUpgrader.class.getName());
 
     protected final UpgradePolicy upgradePolicy;
+    private final List<SystemApplication> managedApplications;
 
-    public InfrastructureUpgrader(Controller controller, Duration interval, UpgradePolicy upgradePolicy, String name) {
+    public InfrastructureUpgrader(Controller controller, Duration interval, UpgradePolicy upgradePolicy,
+                                  List<SystemApplication> managedApplications, String name) {
         super(controller, interval, name, EnumSet.allOf(SystemName.class));
         this.upgradePolicy = upgradePolicy;
+        this.managedApplications = List.copyOf(Objects.requireNonNull(managedApplications));
     }
 
     @Override
     protected boolean maintain() {
-        targetVersion().ifPresent(target -> upgradeAll(target, SystemApplication.all()));
+        targetVersion().ifPresent(target -> upgradeAll(target, managedApplications));
         return true;
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java
index 43e9ce51040..92ebf4ee4b6 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java
@@ -37,7 +37,7 @@ public class OsUpgrader extends InfrastructureUpgrader<OsVersionTarget> {
     private final CloudName cloud;
 
     public OsUpgrader(Controller controller, Duration interval, CloudName cloud) {
-        super(controller, interval, controller.zoneRegistry().osUpgradePolicy(cloud), name(cloud));
+        super(controller, interval, controller.zoneRegistry().osUpgradePolicy(cloud), SystemApplication.all(), name(cloud));
         this.cloud = cloud;
     }
 
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 b84e77a1d85..e286db5882b 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
@@ -26,7 +26,7 @@ public class SystemUpgrader extends InfrastructureUpgrader<Version> {
     private static final Set<Node.State> upgradableNodeStates = Set.of(Node.State.active, Node.State.reserved);
 
     public SystemUpgrader(Controller controller, Duration interval) {
-        super(controller, interval, controller.zoneRegistry().upgradePolicy(), null);
+        super(controller, interval, controller.zoneRegistry().upgradePolicy(), SystemApplication.notController(), null);
     }
 
     @Override
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 a30409dfa80..625154693da 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
@@ -152,7 +152,7 @@ public class VersionStatus {
     private static NodeVersions findSystemApplicationVersions(Controller controller, VersionStatus versionStatus) {
         var nodeVersions = new LinkedHashMap<HostName, NodeVersion>();
         for (var zone : controller.zoneRegistry().zones().controllerUpgraded().zones()) {
-            for (var application : SystemApplication.all()) {
+            for (var application : SystemApplication.notController()) {
                 var nodes = controller.serviceRegistry().configServer().nodeRepository()
                                       .list(zone.getId(), application.id()).stream()
                                       .filter(SystemUpgrader::eligibleForUpgrade)
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
index f8645139244..03487163936 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
@@ -214,7 +214,7 @@ public final class ControllerTester {
 
     /** Upgrade system applications in all zones to given version */
     public void upgradeSystemApplications(Version version) {
-        upgradeSystemApplications(version, SystemApplication.all());
+        upgradeSystemApplications(version, SystemApplication.notController());
     }
 
     /** Upgrade given system applications in all zones to version */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
index a7a99f286df..6c43c4e120d 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
@@ -12,7 +12,6 @@ import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
-import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.flags.json.FlagData;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics;
@@ -101,7 +100,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
 
     @Inject
     public ConfigServerMock(ZoneRegistryMock zoneRegistry) {
-        bootstrap(zoneRegistry.zones().all().ids(), SystemApplication.all());
+        bootstrap(zoneRegistry.zones().all().ids(), SystemApplication.notController());
     }
 
     /** Sets the ConfigChangeActions that will be returned on next deployment. */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java
index e3830e274c9..ccccf4f24d9 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java
@@ -37,12 +37,14 @@ public class OsUpgraderTest {
     public void upgrade_os() {
         CloudName cloud1 = CloudName.from("c1");
         CloudName cloud2 = CloudName.from("c2");
+        ZoneApi zone0 = zone("prod.controller", cloud1);
         ZoneApi zone1 = zone("prod.eu-west-1", cloud1);
         ZoneApi zone2 = zone("prod.us-west-1", cloud1);
         ZoneApi zone3 = zone("prod.us-central-1", cloud1);
         ZoneApi zone4 = zone("prod.us-east-3", cloud1);
         ZoneApi zone5 = zone("prod.us-north-1", cloud2);
         UpgradePolicy upgradePolicy = UpgradePolicy.create()
+                                                   .upgrade(zone0)
                                                    .upgrade(zone1)
                                                    .upgradeInParallel(zone2, zone3)
                                                    .upgrade(zone5) // Belongs to a different cloud and is ignored by this upgrader
@@ -52,8 +54,9 @@ public class OsUpgraderTest {
         // Bootstrap system
         tester.configServer().bootstrap(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId(), zone5.getId()),
                                         List.of(SystemApplication.tenantHost));
+        tester.configServer().addNodes(List.of(zone0.getId()), List.of(SystemApplication.controllerHost));
 
-        // Add system applications that exist in a real system, but isn't upgraded
+        // Add system application that exists in a real system, but isn't eligible for OS upgrades
         tester.configServer().addNodes(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId(), zone5.getId()),
                                        List.of(SystemApplication.configServer));
 
@@ -68,7 +71,15 @@ public class OsUpgraderTest {
         assertEquals(1, tester.controller().osVersionTargets().size()); // Only allows one version per cloud
         statusUpdater.maintain();
 
+        // zone 0: controllers upgrade first
+        osUpgrader.maintain();
+        assertWanted(version1, SystemApplication.controllerHost, zone0.getId());
+        completeUpgrade(version1, SystemApplication.controllerHost, zone0.getId());
+        statusUpdater.maintain();
+        assertEquals(3, nodesOn(version1).size());
+
         // zone 1: begins upgrading
+        assertWanted(Version.emptyVersion, SystemApplication.tenantHost, zone1.getId());
         osUpgrader.maintain();
         assertWanted(version1, SystemApplication.tenantHost, zone1.getId());
 
@@ -78,7 +89,7 @@ public class OsUpgraderTest {
         // zone 1: completes upgrade
         completeUpgrade(version1, SystemApplication.tenantHost, zone1.getId());
         statusUpdater.maintain();
-        assertEquals(2, nodesOn(version1).size());
+        assertEquals(5, nodesOn(version1).size());
         assertEquals(11, nodesOn(Version.emptyVersion).size());
 
         // zone 2 and 3: begins upgrading
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 6370cfedc41..db2353860ae 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
@@ -179,10 +179,10 @@ public class SystemUpgraderTest {
         );
 
         Version version1 = Version.fromString("6.5");
-        tester.configServer().bootstrap(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId()), SystemApplication.all());
+        tester.configServer().bootstrap(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId()), SystemApplication.notController());
         tester.upgradeSystem(version1);
         systemUpgrader.maintain();
-        assertCurrentVersion(SystemApplication.all(), version1, zone1, zone2, zone3, zone4);
+        assertCurrentVersion(SystemApplication.notController(), version1, zone1, zone2, zone3, zone4);
 
         // Controller upgrades
         Version version2 = Version.fromString("6.6");
@@ -199,7 +199,7 @@ public class SystemUpgraderTest {
         systemUpgrader.maintain();
         completeUpgrade(SystemApplication.proxy, version2, zone1);
         convergeServices(SystemApplication.proxy, zone1);
-        assertWantedVersion(SystemApplication.all(), version1, zone2, zone3, zone4);
+        assertWantedVersion(SystemApplication.notController(), version1, zone2, zone3, zone4);
 
         // zone 2 and 3:
         systemUpgrader.maintain();
@@ -207,7 +207,7 @@ public class SystemUpgraderTest {
         systemUpgrader.maintain();
         completeUpgrade(SystemApplication.proxy, version2, zone2, zone3);
         convergeServices(SystemApplication.proxy, zone2, zone3);
-        assertWantedVersion(SystemApplication.all(), version1, zone4);
+        assertWantedVersion(SystemApplication.notController(), version1, zone4);
 
         // zone 4:
         systemUpgrader.maintain();
@@ -217,7 +217,7 @@ public class SystemUpgraderTest {
 
         // All done
         systemUpgrader.maintain();
-        assertWantedVersion(SystemApplication.all(), version2, zone1, zone2, zone3, zone4);
+        assertWantedVersion(SystemApplication.notController(), version2, zone1, zone2, zone3, zone4);
     }
 
     @Test
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
index 38f5a60cf8a..e96af475216 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
@@ -584,7 +584,7 @@ public class RoutingPoliciesTest {
               .setZones(zones)
               .setRoutingMethod(zones, RoutingMethod.exclusive);
         tester.controllerTester().configServer().bootstrap(List.of(prodZone, stagingZone, testZone),
-                                                           SystemApplication.all());
+                                                           SystemApplication.notController());
 
         var context = tester.tester.newDeploymentContext();
         var endpointId = EndpointId.of("r0");
@@ -750,7 +750,7 @@ public class RoutingPoliciesTest {
                   tester.controllerTester().zoneRegistry().exclusiveRoutingIn(zones);
             }
             tester.controllerTester().configServer().bootstrap(tester.controllerTester().zoneRegistry().zones().all().ids(),
-                                                               SystemApplication.all());
+                                                               SystemApplication.notController());
         }
 
         private void provisionLoadBalancers(int clustersPerZone, ApplicationId application, boolean shared, ZoneId... zones) {