Merge pull request #23612 from vespa-engine/mpolden/simplify

Consider all nodes when deciding whether to change target

3 files changed, 5 insertions, 6 deletions

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 1454d78ce33..b051590ac5a 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
@@ -104,7 +104,7 @@ public abstract class InfrastructureUpgrader<TARGET extends VersionTarget> exten
             Set<SystemApplication> dependencies = kv.getValue();
             boolean allConverged = dependencies.stream().allMatch(app -> convergedOn(target, app, zone, nodeSlice));
             if (allConverged) {
-                if (changeTargetTo(target, application, zone, nodeSlice)) {
+                if (changeTargetTo(target, application, zone)) {
                     upgrade(target, application, zone);
                 }
                 converged &= convergedOn(target, application, zone, nodeSlice);
@@ -114,7 +114,7 @@ public abstract class InfrastructureUpgrader<TARGET extends VersionTarget> exten
     }
 
     /** Returns whether target version for application in zone should be changed */
-    protected abstract boolean changeTargetTo(TARGET target, SystemApplication application, ZoneApi zone, NodeSlice nodeSlice);
+    protected abstract boolean changeTargetTo(TARGET target, SystemApplication application, ZoneApi zone);
 
     /** Upgrade component to target version. Implementation should be idempotent */
     protected abstract void upgrade(TARGET target, SystemApplication application, ZoneApi zone);
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 46b504cadff..f4dcf7f6088 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
@@ -76,7 +76,7 @@ public class OsUpgrader extends InfrastructureUpgrader<OsVersionTarget> {
     }
 
     @Override
-    protected boolean changeTargetTo(OsVersionTarget target, SystemApplication application, ZoneApi zone, NodeSlice nodeSlice) {
+    protected boolean changeTargetTo(OsVersionTarget target, SystemApplication application, ZoneApi zone) {
         if (!application.shouldUpgradeOs()) return false;
         return controller().serviceRegistry().configServer().nodeRepository()
                            .targetVersionsOf(zone.getVirtualId())
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 86587c8e9f7..8e74ef9a983 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
@@ -74,16 +74,15 @@ public class SystemUpgrader extends InfrastructureUpgrader<VespaVersionTarget> {
     }
 
     @Override
-    protected boolean changeTargetTo(VespaVersionTarget target, SystemApplication application, ZoneApi zone, NodeSlice nodeSlice) {
+    protected boolean changeTargetTo(VespaVersionTarget target, SystemApplication application, ZoneApi zone) {
         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 versionOf(nodeSlice, zone, application, Node::wantedVersion)
+            return versionOf(NodeSlice.ALL, zone, application, Node::wantedVersion)
                     .map(wantedVersion -> !wantedVersion.equals(target.version()))
                     .orElse(zoneHasSharedRouting); // Always upgrade if zone uses shared routing, but has no nodes allocated yet
-
         }
         return controller().serviceRegistry().configServer().nodeRepository()
                            .targetVersionsOf(zone.getId())