Use not() for negated conditions in ApplicationList

3 files changed, 28 insertions, 58 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java
index 8978af78574..3fe8e9f52c3 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java
@@ -12,7 +12,6 @@ import com.yahoo.vespa.hosted.controller.Instance;
 
 import java.time.Instant;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
@@ -59,7 +58,12 @@ public class ApplicationList extends AbstractFilteringList<Application, Applicat
 
     /** Returns the subset of applications which are currently upgrading to the given version */
     public ApplicationList upgradingTo(Version version) {
-        return matching(application -> isUpgradingTo(version, application));
+        return upgradingTo(List.of(version));
+    }
+
+    /** Returns the subset of applications which are currently upgrading to the given version */
+    public ApplicationList upgradingTo(Collection<Version> versions) {
+        return matching(application -> versions.stream().anyMatch(version -> isUpgradingTo(version, application)));
     }
 
     /** Returns the subset of applications which are not pinned to a certain Vespa version. */
@@ -67,47 +71,22 @@ public class ApplicationList extends AbstractFilteringList<Application, Applicat
         return matching(application -> ! application.change().isPinned());
     }
 
-    /** Returns the subset of applications which are currently not upgrading to the given version */
-    public ApplicationList notUpgradingTo(Version version) {
-        return notUpgradingTo(Collections.singletonList(version));
-    }
-
-    public ApplicationList notFailingUpgrade() {
-        return matching(application -> application.instances().values().stream()
+    public ApplicationList failingUpgrade() {
+        return matching(application -> ! application.instances().values().stream()
                                                     .allMatch(instance -> JobList.from(instance)
                                                                                  .failing()
                                                                                  .not().failingApplicationChange()
                                                                                  .isEmpty()));
     }
 
-    /** Returns the subset of applications which are currently not upgrading to any of the given versions */
-    public ApplicationList notUpgradingTo(Collection<Version> versions) {
-        return matching(application -> versions.stream().noneMatch(version -> isUpgradingTo(version, application)));
-    }
-
-    /**
-     * Returns the subset of applications which are currently not upgrading to the given version,
-     * or returns all if no version is specified
-     */
-    public ApplicationList notUpgradingTo(Optional<Version> version) {
-        if (version.isEmpty()) return this;
-        return notUpgradingTo(version.get());
-    }
-
     /** Returns the subset of applications which have changes left to deploy; blocked, or deploying */
     public ApplicationList withChanges() {
         return matching(application -> application.change().hasTargets() || application.outstandingChange().hasTargets());
     }
 
-    /** Returns the subset of applications which are currently not deploying a change */
-    public ApplicationList notDeploying() {
-        return matching(application -> ! application.change().hasTargets());
-    }
-
-    /** Returns the subset of applications which currently does not have any failing jobs */
-    public ApplicationList notFailing() {
-        return matching(application -> application.instances().values().stream()
-                                                    .noneMatch(instance -> instance.deploymentJobs().hasFailures()));
+    /** Returns the subset of applications which are currently deploying a change */
+    public ApplicationList deploying() {
+        return matching(application -> application.change().hasTargets());
     }
 
     /** Returns the subset of applications which currently have failing jobs */
@@ -119,19 +98,19 @@ public class ApplicationList extends AbstractFilteringList<Application, Applicat
     /** Returns the subset of applications which have been failing an upgrade to the given version since the given instant */
     public ApplicationList failingUpgradeToVersionSince(Version version, Instant threshold) {
         return matching(application -> application.instances().values().stream()
-                                                    .anyMatch(instance -> failingUpgradeToVersionSince(instance, version, threshold)));
+                                                  .anyMatch(instance -> failingUpgradeToVersionSince(instance, version, threshold)));
     }
 
     /** Returns the subset of applications which have been failing an application change since the given instant */
     public ApplicationList failingApplicationChangeSince(Instant threshold) {
         return matching(application -> application.instances().values().stream()
-                          .anyMatch(instance -> failingApplicationChangeSince(instance, threshold)));
+                                                  .anyMatch(instance -> failingApplicationChangeSince(instance, threshold)));
     }
 
-    /** Returns the subset of applications which currently does not have any failing jobs on the given version */
-    public ApplicationList notFailingOn(Version version) {
+    /** Returns the subset of applications which currently have failing jobs on the given version */
+    public ApplicationList failingOn(Version version) {
         return matching(application -> application.instances().values().stream()
-                          .noneMatch(instance -> failingOn(version, instance)));
+                                                  .anyMatch(instance -> failingOn(version, instance)));
     }
 
     /** Returns the subset of applications which have at least one production deployment */
@@ -153,13 +132,6 @@ public class ApplicationList extends AbstractFilteringList<Application, Applicat
                                                      .anyMatch(instance -> instance.upgradePolicy() == policy));
     }
 
-    /** Returns the subset of applications which does not have the given upgrade policy */
-    // TODO jonmv: Make this instance based when instances are orchestrated, and deployments reported per instance.
-    public ApplicationList without(UpgradePolicy policy) {
-        return matching(application ->  application.deploymentSpec().instances().stream()
-                                                     .allMatch(instance -> instance.upgradePolicy() != policy));
-    }
-
     /** Returns the subset of applications which have at least one deployment on a lower version than the given one */
     public ApplicationList onLowerVersionThan(Version version) {
         return matching(application -> application.instances().values().stream()
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
index 08fa3abcd9f..28e276c1497 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
@@ -49,29 +49,29 @@ public class Upgrader extends Maintainer {
     @Override
     public void maintain() {
         // Determine target versions for each upgrade policy
-        Optional<Version> canaryTarget = controller().versionStatus().systemVersion().map(VespaVersion::versionNumber);
+        Version canaryTarget = controller().systemVersion();
         Collection<Version> defaultTargets = targetVersions(Confidence.normal);
         Collection<Version> conservativeTargets = targetVersions(Confidence.high);
 
         // Cancel upgrades to broken targets (let other ongoing upgrades complete to avoid starvation)
         for (VespaVersion version : controller().versionStatus().versions()) {
             if (version.confidence() == Confidence.broken)
-                cancelUpgradesOf(applications().without(UpgradePolicy.canary).upgradingTo(version.versionNumber()),
+                cancelUpgradesOf(applications().not().with(UpgradePolicy.canary).upgradingTo(version.versionNumber()),
                                  version.versionNumber() + " is broken");
         }
 
         // Canaries should always try the canary target
-        cancelUpgradesOf(applications().with(UpgradePolicy.canary).upgrading().notUpgradingTo(canaryTarget),
+        cancelUpgradesOf(applications().with(UpgradePolicy.canary).upgrading().not().upgradingTo(canaryTarget),
                          "Outdated target version for Canaries");
 
         // Cancel *failed* upgrades to earlier versions, as the new version may fix it
         String reason = "Failing on outdated version";
-        cancelUpgradesOf(applications().with(UpgradePolicy.defaultPolicy).upgrading().failing().notUpgradingTo(defaultTargets), reason);
-        cancelUpgradesOf(applications().with(UpgradePolicy.conservative).upgrading().failing().notUpgradingTo(conservativeTargets), reason);
+        cancelUpgradesOf(applications().with(UpgradePolicy.defaultPolicy).upgrading().failing().not().upgradingTo(defaultTargets), reason);
+        cancelUpgradesOf(applications().with(UpgradePolicy.conservative).upgrading().failing().not().upgradingTo(conservativeTargets), reason);
 
         // Schedule the right upgrades
         ApplicationList applications = applications();
-        canaryTarget.ifPresent(target -> upgrade(applications.with(UpgradePolicy.canary), target));
+        upgrade(applications.with(UpgradePolicy.canary), canaryTarget);
         defaultTargets.forEach(target -> upgrade(applications.with(UpgradePolicy.defaultPolicy), target));
         conservativeTargets.forEach(target -> upgrade(applications.with(UpgradePolicy.conservative), target));
     }
@@ -98,13 +98,13 @@ public class Upgrader extends Maintainer {
         applications = applications.withProductionDeployment();
         applications = applications.onLowerVersionThan(version);
         applications = applications.allowMajorVersion(version.getMajor(), targetMajorVersion().orElse(version.getMajor()));
-        applications = applications.notDeploying(); // wait with applications deploying an application change or already upgrading
-        applications = applications.notFailingOn(version); // try to upgrade only if it hasn't failed on this version
+        applications = applications.not().deploying(); // wait with applications deploying an application change or already upgrading
+        applications = applications.not().failingOn(version); // try to upgrade only if it hasn't failed on this version
         applications = applications.canUpgradeAt(controller().clock().instant()); // wait with applications that are currently blocking upgrades
         applications = applications.byIncreasingDeployedVersion(); // start with lowest versions
         for (Application application : applications.with(UpgradePolicy.canary).asList())
             controller().applications().deploymentTrigger().triggerChange(application.id(), Change.of(version));
-        for (Application application : applications.without(UpgradePolicy.canary).first(numberOfApplicationsToUpgrade()).asList())
+        for (Application application : applications.not().with(UpgradePolicy.canary).first(numberOfApplicationsToUpgrade()).asList())
             controller().applications().deploymentTrigger().triggerChange(application.id(), Change.of(version));
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java
index d0cf5aac2d9..296639245a3 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java
@@ -45,9 +45,7 @@ public class VespaVersion implements Comparable<VespaVersion> {
 
     public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller) {
         // 'production on this': All deployment jobs upgrading to this version have completed without failure
-        ApplicationList productionOnThis = ApplicationList.from(statistics.production(), controller.applications())
-                                                          .notUpgradingTo(statistics.version())
-                                                          .notFailingUpgrade();
+        ApplicationList productionOnThis = ApplicationList.from(statistics.production(), controller.applications()).not().upgradingTo(statistics.version()).not().failingUpgrade();
         ApplicationList failingOnThis = ApplicationList.from(statistics.failing(), controller.applications());
         ApplicationList all = ApplicationList.from(controller.applications().asList())
                                              .withProductionDeployment();
@@ -162,8 +160,8 @@ public class VespaVersion implements Comparable<VespaVersion> {
     private static boolean nonCanaryApplicationsBroken(Version version,
                                                        ApplicationList failingOnThis,
                                                        ApplicationList productionOnThis) {
-        ApplicationList failingNonCanaries = failingOnThis.without(UpgradePolicy.canary).startedFailingOn(version);
-        ApplicationList productionNonCanaries = productionOnThis.without(UpgradePolicy.canary);
+        ApplicationList failingNonCanaries = failingOnThis.not().with(UpgradePolicy.canary).startedFailingOn(version);
+        ApplicationList productionNonCanaries = productionOnThis.not().with(UpgradePolicy.canary);
 
         if (productionNonCanaries.size() + failingNonCanaries.size() == 0) return false;