Do not deploy routing application if no nodes are allocated

2 files changed, 25 insertions, 11 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 9b1bb300354..72c5639735d 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
@@ -40,8 +40,8 @@ public class SystemUpgrader extends InfrastructureUpgrader {
         // Skip application convergence check if there are no nodes belonging to the application in the zone
         if (minVersion.isEmpty()) return true;
 
-        return     minVersion.get().equals(target)
-                && application.configConvergedIn(zone.getId(), controller(), Optional.of(target));
+        return minVersion.get().equals(target) &&
+               application.configConvergedIn(zone.getId(), controller(), Optional.of(target));
     }
 
     @Override
@@ -63,8 +63,14 @@ public class SystemUpgrader extends InfrastructureUpgrader {
             // For applications with package we do not have a zone-wide version target. This means that we must check
             // the wanted version of each node.
             return minVersion(zone, application, Node::wantedVersion)
-                    .map(target::isAfter)                                     // Upgrade if target is after any wanted version
-                    .orElse(true);                                            // Upgrade if there are no nodes allocated
+                    // Upgrade if target is after any wanted version
+                    .map(target::isAfter)
+                    // Skip upgrade if there are no nodes allocated. This is overloaded to mean that the zone is not
+                    // expected to have a deployment of this application.
+                    // TODO(mpolden): Once all zones are either directly routed or not: Change this to
+                    //                always deploy proxy app and wait for convergence in zones that are not directly
+                    //                routed.
+                    .orElse(false);
         }
         return controller().serviceRegistry().configServer().nodeRepository()
                            .targetVersionsOf(zone.getId())
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 dbcd9b7f6c3..140c37cdd94 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
@@ -276,17 +276,25 @@ public class SystemUpgraderTest {
 
     @Test
     public void does_not_deploy_proxy_app_in_zones_without_proxy() {
-        List<SystemApplication> applications = List.of(
-                SystemApplication.configServerHost, SystemApplication.configServer, SystemApplication.tenantHost);
+        var applications = List.of(SystemApplication.configServerHost, SystemApplication.configServer,
+                                   SystemApplication.tenantHost);
         tester.configServer().bootstrap(List.of(zone1.getId()), applications);
         tester.configServer().disallowConvergenceCheck(SystemApplication.proxy.id());
+        var systemUpgrader = systemUpgrader(UpgradePolicy.create().upgrade(zone1));
 
-        SystemUpgrader systemUpgrader = systemUpgrader(UpgradePolicy.create().upgrade(zone1));
+        // System begins upgrade
+        var version1 = Version.fromString("6.5");
+        tester.upgradeController(version1);
+        systemUpgrader.maintain();
+        assertWantedVersion(applications, version1, zone1);
+        assertWantedVersion(SystemApplication.proxy, Version.emptyVersion, zone1);
 
-        Version version1 = Version.fromString("6.5");
-        tester.upgradeSystem(version1);
+        // System completes upgrade. Wanted version is not raised for proxy as it's is never deployed
+        completeUpgrade(applications, version1, zone1);
         systemUpgrader.maintain();
-        assertCurrentVersion(applications, version1, zone1);
+        assertWantedVersion(SystemApplication.proxy, Version.emptyVersion, zone1);
+        tester.computeVersionStatus();
+        assertEquals(version1, tester.controller().systemVersion());
     }
 
     /** Simulate upgrade of nodes allocated to given application. In a real system this is done by the node itself */
@@ -357,7 +365,7 @@ public class SystemUpgraderTest {
                                ZoneApi first, ZoneApi... rest) {
         Stream.concat(Stream.of(first), Stream.of(rest)).forEach(zone -> {
             for (Node node : listNodes(zone, application)) {
-                assertEquals(application + " version", version, versionField.apply(node));
+                assertEquals("Version of " + application.id() + " in " + zone.getId(), version, versionField.apply(node));
             }
         });
     }