Merge pull request #13868 from vespa-engine/mpolden/todo

Default to deploying proxy application in zones with shared routing

2 files changed, 9 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 bf44c796f34..d9c78c8a442 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
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.yahoo.component.Version;
+import com.yahoo.config.provision.zone.RoutingMethod;
 import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
@@ -62,15 +63,12 @@ public class SystemUpgrader extends InfrastructureUpgrader<Version> {
         if (application.hasApplicationPackage()) {
             // 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.
+            boolean zoneHasSharedRouting = controller().zoneRegistry().routingMethods(zone.getId()).stream()
+                                                       .anyMatch(RoutingMethod::isShared);
             return minVersion(zone, application, Node::wantedVersion)
-                    // 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);
+                    .map(target::isAfter)          // Upgrade if target is after any wanted version
+                    .orElse(zoneHasSharedRouting); // Always upgrade if zone uses shared routing, but has no nodes allocated yet
+
         }
         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 d5c26408b23..98412b8147b 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
@@ -275,11 +275,12 @@ public class SystemUpgraderTest {
     }
 
     @Test
-    public void does_not_deploy_proxy_app_in_zones_without_proxy() {
+    public void does_not_deploy_proxy_app_in_zone_without_shared_routing() {
         var applications = List.of(SystemApplication.configServerHost, SystemApplication.configServer,
                                    SystemApplication.tenantHost);
         tester.configServer().bootstrap(List.of(zone1.getId()), applications);
         tester.configServer().disallowConvergenceCheck(SystemApplication.proxy.id());
+        tester.zoneRegistry().exclusiveRoutingIn(zone1);
         var systemUpgrader = systemUpgrader(UpgradePolicy.create().upgrade(zone1));
 
         // System begins upgrade
@@ -382,8 +383,7 @@ public class SystemUpgraderTest {
 
     private SystemUpgrader systemUpgrader(UpgradePolicy upgradePolicy) {
         tester.zoneRegistry().setUpgradePolicy(upgradePolicy);
-        return new SystemUpgrader(tester.controller(), Duration.ofDays(1)
-        );
+        return new SystemUpgrader(tester.controller(), Duration.ofDays(1));
     }
 
 }