Merge pull request #4546 from vespa-engine/jvenstad/report-more-failing-apps-in-platform-issue

Jvenstad/report more failing apps in platform issue

8 files changed, 34 insertions, 32 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 4d1a009806f..07d51b2b9c7 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
@@ -121,9 +121,9 @@ public class ApplicationList {
         return listOf(list.stream().filter(a -> !a.productionDeployments().isEmpty()));
     }
 
-    /** Returns the subset of applications which started failing after the given instant */
-    public ApplicationList startedFailingOnVersionAfter(Version version, Instant instant) {
-        return listOf(list.stream().filter(application -> JobList.from(application).firstFailing().on(version).firstFailing().after(instant).anyMatch()));
+    /** Returns the subset of applications which started failing on the given version */
+    public ApplicationList startedFailingOn(Version version) {
+        return listOf(list.stream().filter(application -> ! JobList.from(application).firstFailing().on(version).isEmpty()));
     }
 
     /** Returns the subset of applications which has the given upgrade policy */
@@ -209,32 +209,32 @@ public class ApplicationList {
     }
 
     private static boolean failingOn(Version version, Application application) {
-        return JobList.from(application)
+        return ! JobList.from(application)
                 .failing()
                 .lastCompleted().on(version)
-                .anyMatch();
+                .isEmpty();
     }
 
     private static boolean currentlyUpgrading(Change.VersionChange change, Application application, Instant jobTimeoutLimit) {
-        return JobList.from(application)
+        return ! JobList.from(application)
                 .running(jobTimeoutLimit)
                 .lastTriggered().on(change.version())
-                .anyMatch();
+                .isEmpty();
     }
 
     private static boolean failingUpgradeToVersionSince(Application application, Version version, Instant threshold) {
-        return JobList.from(application)
+        return ! JobList.from(application)
                 .not().failingApplicationChange()
                 .firstFailing().before(threshold)
                 .lastCompleted().on(version)
-                .anyMatch();
+                .isEmpty();
     }
 
     private static boolean failingApplicationChangeSince(Application application, Instant threshold) {
-        return JobList.from(application)
+        return ! JobList.from(application)
                 .failingApplicationChange()
                 .firstFailing().before(threshold)
-                .anyMatch();
+                .isEmpty();
     }
 
     /** Convenience converter from a stream to an ApplicationList */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java
index 5b104f19937..eea94411109 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java
@@ -101,12 +101,12 @@ public class DeploymentJobs {
 
     /** Returns whether this has some job status which is not a success */
     public boolean hasFailures() {
-        return JobList.from(status.values()).failing().anyMatch();
+        return ! JobList.from(status.values()).failing().isEmpty();
     }
 
     /** Returns whether any job is currently in progress */
     public boolean isRunning(Instant timeoutLimit) {
-        return JobList.from(status.values()).running(timeoutLimit).anyMatch();
+        return ! JobList.from(status.values()).running(timeoutLimit).isEmpty();
     }
 
     /** Returns whether the given job type is currently running and was started after timeoutLimit */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java
index 6223b07d27a..161035b1164 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java
@@ -57,8 +57,6 @@ public class JobList {
 
     public boolean isEmpty() { return list.isEmpty(); }
 
-    public boolean anyMatch() { return ! isEmpty(); }
-
     public int size() { return list.size(); }
 
     // ----------------------------------- Basic filters
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
index 6a9db3ae917..90237a17fb9 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
@@ -344,10 +344,10 @@ public class DeploymentTrigger {
     }
 
     private boolean isRunningProductionJob(Application application) {
-        return JobList.from(application)
+        return ! JobList.from(application)
                 .production()
                 .running(jobTimeoutLimit())
-                .anyMatch();
+                .isEmpty();
     }
 
     /**
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 324868878af..e30ccbe7950 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
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.maintenance;
 
+import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
@@ -75,17 +76,21 @@ 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() == broken))
+        Version systemVersion = controller().systemVersion();
+
+        if ((controller().versionStatus().version(systemVersion).confidence() != broken))
+            return;
+
+        if (ApplicationList.from(applications)
+                .failingUpgradeToVersionSince(systemVersion, controller().clock().instant().minus(upgradeGracePeriod))
+                .isEmpty())
             return;
 
         List<ApplicationId> failingApplications = ApplicationList.from(applications)
-                .failingUpgradeToVersionSince(controller().systemVersion(), controller().clock().instant().minus(upgradeGracePeriod))
-                .asList().stream()
-                .map(Application::id)
-                .collect(Collectors.toList());
+                .failingUpgradeToVersionSince(systemVersion, controller().clock().instant())
+                .idList();
 
-        if ( ! failingApplications.isEmpty())
-            deploymentIssues.fileUnlessOpen(failingApplications, controller().systemVersion());
+        deploymentIssues.fileUnlessOpen(failingApplications, systemVersion);
     }
 
     private Tenant ownerOf(ApplicationId applicationId) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
index 77bd25124c9..13eec52b97a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
@@ -184,11 +184,11 @@ public class VersionStatus {
         VespaVersion.Confidence confidence;
         // Always compute confidence for system version
         if (isSystemVersion) {
-            confidence = VespaVersion.confidenceFrom(statistics, controller, releasedAt);
+            confidence = VespaVersion.confidenceFrom(statistics, controller);
         } else {
             // Keep existing confidence for non-system versions if already computed
             confidence = confidenceFor(statistics.version(), controller)
-                    .orElse(VespaVersion.confidenceFrom(statistics, controller, releasedAt));
+                    .orElse(VespaVersion.confidenceFrom(statistics, controller));
         }
         return new VespaVersion(statistics,
                                 gitSha.sha, releasedAt,
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 4bcee5782ee..ea89a70543c 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
@@ -42,8 +42,7 @@ public class VespaVersion implements Comparable<VespaVersion> {
         this.confidence = confidence;
     }
 
-    public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller,
-                                            Instant releasedAt) {
+    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())
@@ -58,7 +57,7 @@ public class VespaVersion implements Comparable<VespaVersion> {
             return Confidence.broken;
 
         // 'broken' if 4 non-canary was broken by this, and that is at least 10% of all
-        if (nonCanaryApplicationsBroken(statistics.version(), failingOnThis, productionOnThis, releasedAt, controller.curator()))
+        if (nonCanaryApplicationsBroken(statistics.version(), failingOnThis, productionOnThis, controller.curator()))
             return Confidence.broken;
 
         // 'low' unless all canary applications are upgraded
@@ -145,9 +144,8 @@ public class VespaVersion implements Comparable<VespaVersion> {
     private static boolean nonCanaryApplicationsBroken(Version version,
                                                        ApplicationList failingOnThis,
                                                        ApplicationList productionOnThis,
-                                                       Instant releasedAt,
                                                        CuratorDb curator) {
-        ApplicationList failingNonCanaries = failingOnThis.without(UpgradePolicy.canary).startedFailingOnVersionAfter(version, releasedAt);
+        ApplicationList failingNonCanaries = failingOnThis.without(UpgradePolicy.canary).startedFailingOn(version);
         ApplicationList productionNonCanaries = productionOnThis.without(UpgradePolicy.canary);
 
         if (productionNonCanaries.size() + failingNonCanaries.size() == 0 || curator.readIgnoreConfidence()) return false;
@@ -156,4 +154,5 @@ public class VespaVersion implements Comparable<VespaVersion> {
         int brokenByThisVersion = failingNonCanaries.size();
         return brokenByThisVersion >= 4 && brokenByThisVersion >= productionOnThis.size() * 0.1;
      }
+
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java
index 7b689923138..c3ce5c95dcd 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java
@@ -85,7 +85,7 @@ public class DeploymentApiTest extends ControllerContainerTest {
                                            version.releasedAt(), 
                                            version.isCurrentSystemVersion(), 
                                            ImmutableSet.of("config1.test", "config2.test"),
-                                           VespaVersion.confidenceFrom(version.statistics(), controller, version.releasedAt())
+                                           VespaVersion.confidenceFrom(version.statistics(), controller)
                 );
             censored.add(version);
         }