Moved subset selection code to ApplicationList

2 files changed, 62 insertions, 41 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 785aeefc57f..c7aa631b8b6 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
@@ -5,13 +5,13 @@ import com.google.common.collect.ImmutableList;
 import com.yahoo.component.Version;
 import com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.Environment;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.ApplicationController;
 
 import java.time.Instant;
 import java.util.Comparator;
 import java.util.List;
+import java.util.function.Predicate;
 import java.util.stream.Stream;
 
 /**
@@ -73,6 +73,12 @@ public class ApplicationList {
         return listOf(list.stream().filter(application -> ! application.deploymentJobs().hasFailures()));
     }
 
+    /** Returns the subset of applications which have deployment jobs matching all the given conditions */
+    @SafeVarargs
+    public final ApplicationList withDeploymentJobs(Predicate<JobStatus>... filters) {
+        return listOf(list.stream().filter(application -> hasJobsMatching(filters, application)));
+    }
+
     /** Returns the subset of applications which currently does not have any failing jobs on the given version */
     public ApplicationList notFailingOn(Version version) {
         return listOf(list.stream().filter(application -> ! failingOn(version, application)));
@@ -145,6 +151,31 @@ public class ApplicationList {
         return listOf(list.stream().filter(a -> !currentlyUpgrading(change, a, jobTimeoutLimit)));
     }
 
+    // ----------------------------------- JobStatus filters
+
+    public static final Predicate<JobStatus> failingUpgrade = job -> {
+        if (   job.isSuccess()) return false;
+        if ( ! job.lastSuccess().isPresent()) return false; // An application which never succeeded is surely bad.
+        if ( ! job.firstFailing().get().revision().equals(job.lastSuccess().get().revision())) return false; // Application change may be to blame.
+        if ( ! job.lastSuccess().get().revision().isPresent()) return false; // Indicates the component job, which is not an upgrade.
+        return ! job.firstFailing().get().version().equals(job.lastSuccess().get().version()); // Version change may be to blame.
+    };
+
+    public static final Predicate<JobStatus> failingApplicationChange = job -> {
+        if (   job.isSuccess()) return false;
+        if ( ! job.lastSuccess().isPresent()) return true; // An application which never succeeded is surely bad.
+        if ( ! job.firstFailing().get().version().equals(job.lastSuccess().get().version())) return false; // Version change may be to blame.
+        if ( ! job.lastSuccess().get().revision().isPresent()) return true; // Indicates the component job, which is always an application change.
+        return ! job.firstFailing().get().revision().equals(job.lastSuccess().get().revision()); // Return whether there is an application change.
+    };
+
+    public static Predicate<JobStatus> failingSince(Instant threshold) {
+        return job -> {
+            if (job.isSuccess()) return false;
+            return job.firstFailing().get().at().isBefore(threshold);
+        };
+    }
+
     // ----------------------------------- Internal helpers
     
     private static boolean isUpgradingTo(Version version, Application application) {
@@ -177,7 +208,15 @@ public class ApplicationList {
                 .map(status -> status.lastTriggered().get())
                 .anyMatch(jobRun -> jobRun.version().equals(change.version()));
     }
-    
+
+    private static boolean hasJobsMatching(Predicate<JobStatus>[] filters, Application application) {
+        Stream<JobStatus> statuses = application.deploymentJobs().jobStatus().values().stream();
+        for (Predicate<JobStatus> filter : filters)
+            statuses = statuses.filter(filter);
+        return statuses.findAny().isPresent();
+    }
+
+
     /** Convenience converter from a stream to an ApplicationList */
     private static ApplicationList listOf(Stream<Application> applications) {
         ImmutableList.Builder<Application> b = new ImmutableList.Builder<>();
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java
index 031809ce699..b70db0f6ced 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentIssueReporter.java
@@ -13,20 +13,20 @@ import com.yahoo.vespa.hosted.controller.api.integration.organization.Deployment
 import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.User;
 import com.yahoo.vespa.hosted.controller.application.ApplicationList;
-import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
-import com.yahoo.vespa.hosted.controller.application.JobStatus;
-import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 
 import java.time.Duration;
-import java.time.Instant;
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Comparator;
 import java.util.List;
 import java.util.NoSuchElementException;
 import java.util.Optional;
-import java.util.function.Predicate;
+import java.util.Set;
 import java.util.logging.Level;
+import java.util.stream.Collectors;
+
+import static com.yahoo.vespa.hosted.controller.application.ApplicationList.failingApplicationChange;
+import static com.yahoo.vespa.hosted.controller.application.ApplicationList.failingSince;
+import static com.yahoo.vespa.hosted.controller.application.ApplicationList.failingUpgrade;
+import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.broken;
 
 /**
  * Maintenance job which files issues for tenants when they have jobs which fails continuously
@@ -60,15 +60,17 @@ public class DeploymentIssueReporter extends Maintainer {
      * where deployment has not failed for this amount of time.
      */
     private void maintainDeploymentIssues(List<Application> applications) {
-        List<ApplicationId> failingApplications = new ArrayList<>();
+        Set<ApplicationId> failingApplications = ApplicationList.from(applications)
+                .withDeploymentJobs(failingApplicationChange, failingSince(controller().clock().instant().minus(maxFailureAge)))
+                .asList().stream()
+                .map(Application::id)
+                .collect(Collectors.toSet());
+
         for (Application application : applications)
-            if (oldFailuresIn(application.deploymentJobs(), this::causedByApplicationChange, maxFailureAge))
-                failingApplications.add(application.id());
+            if (failingApplications.contains(application.id()))
+                fileDeploymentIssueFor(application.id());
             else
                 storeIssueId(application.id(), null);
-
-        failingApplications.forEach(this::fileDeploymentIssueFor);
-
     }
 
     /**
@@ -77,39 +79,19 @@ public class DeploymentIssueReporter extends Maintainer {
      * longer than the set grace period, or update this list if the issue already exists.
      */
     private void maintainPlatformIssue(List<Application> applications) {
-        if ( ! (controller().versionStatus().version(controller().systemVersion()).confidence() == VespaVersion.Confidence.broken))
+        if ( ! (controller().versionStatus().version(controller().systemVersion()).confidence() == broken))
             return;
 
-        List<ApplicationId> failingApplications = new ArrayList<>();
-        for (Application application : ApplicationList.from(applications).upgradingTo(controller().systemVersion()).asList())
-            if (oldFailuresIn(application.deploymentJobs(), job -> ! causedByApplicationChange(job), upgradeGracePeriod))
-                failingApplications.add(application.id());
+        List<ApplicationId> failingApplications = ApplicationList.from(applications)
+                .withDeploymentJobs(failingUpgrade, failingSince(controller().clock().instant().minus(upgradeGracePeriod)))
+                .asList().stream()
+                .map(Application::id)
+                .collect(Collectors.toList());
 
         if ( ! failingApplications.isEmpty())
             deploymentIssues.fileUnlessOpen(failingApplications, controller().systemVersion());
     }
 
-    /** Return whether the given deployment jobs contain failures due to the given cause, past the given age. */
-    private boolean oldFailuresIn(DeploymentJobs jobs, Predicate<JobStatus> failureCause, Duration maxAge) {
-        if ( ! jobs.hasFailures()) return false;
-
-        Optional<Instant> oldestApplicationChangeFailure = jobs.jobStatus().values().stream()
-                .filter(job -> ! job.isSuccess())
-                .filter(failureCause)
-                .map(job -> job.firstFailing().get().at())
-                .min(Comparator.naturalOrder());
-
-        return oldestApplicationChangeFailure.isPresent()
-               && oldestApplicationChangeFailure.get().isBefore(controller().clock().instant().minus(maxAge));
-    }
-
-    private boolean causedByApplicationChange(JobStatus job) {
-        if ( ! job.lastSuccess().isPresent()) return true; // An application which never succeeded is surely bad.
-        if ( ! job.firstFailing().get().version().equals(job.lastSuccess().get().version())) return false; // Version change may be to blame.
-        if ( ! job.lastSuccess().get().revision().isPresent()) return true; // Indicates the component job, which is always an application change.
-        return ! job.firstFailing().get().revision().equals(job.lastSuccess().get().revision()); // Return whether there is an application change.
-    }
-
     private Tenant ownerOf(ApplicationId applicationId) {
         return controller().tenants().tenant(new TenantId(applicationId.tenant().value()))
                 .orElseThrow(() -> new IllegalStateException("No tenant found for application " + applicationId));