Merge pull request #3789 from vespa-engine/hmusum/add-persisted-setting-for-ignoring-confidence-calculations

Add persistent setting for ignoring confidence calculations

8 files changed, 129 insertions, 23 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
index e338bc17788..c293e00ae48 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
@@ -241,6 +241,8 @@ public class Controller extends AbstractComponent {
         return chefClient;
     }
 
+    public CuratorDb curator() { return curator; }
+
     private String printableVersion(Optional<VespaVersion> vespaVersion) {
         return vespaVersion.map(v -> v.versionNumber().toFullString()).orElse("Unknown");
     }
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 0722a58e18d..44b15053a8c 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
@@ -7,13 +7,11 @@ import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.application.ApplicationList;
 import com.yahoo.vespa.hosted.controller.application.Change;
-import com.yahoo.vespa.hosted.controller.deployment.BuildSystem;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 import com.yahoo.yolean.Exceptions;
 
 import java.time.Duration;
-import java.time.Instant;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -111,4 +109,18 @@ public class Upgrader extends Maintainer {
         curator.writeUpgradesPerMinute(n);
     }
 
+    /**
+     * Returns whether to ignore confidence calculations when upgrading
+     */
+    public boolean ignoreConfidence() {
+        return curator.readIgnoreConfidence();
+    }
+
+    /**
+     * Controls whether to ignore confidence calculations or not
+     */
+    public void ignoreConfidence(boolean value) {
+        curator.writeIgnoreConfidence(value);
+    }
+
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
index 03b4dd6efe3..0a288955055 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
@@ -212,6 +212,20 @@ public class CuratorDb {
         transaction.commit();
     }
 
+    public boolean readIgnoreConfidence() {
+        Optional<byte[]> value = curator.getData(ignoreConfidencePath());
+        if (! value.isPresent() || value.get().length == 0) {
+            return false; // Default if value has never been written
+        }
+        return ByteBuffer.wrap(value.get()).getInt() == 1;
+    }
+
+    public void writeIgnoreConfidence(boolean value) {
+        NestedTransaction transaction = new NestedTransaction();
+        curator.set(ignoreConfidencePath(), ByteBuffer.allocate(Integer.BYTES).putInt(value ? 1 : 0).array());
+        transaction.commit();
+    }
+
     public void writeVersionStatus(VersionStatus status) {
         VersionStatusSerializer serializer = new VersionStatusSerializer();
         NestedTransaction transaction = new NestedTransaction();
@@ -305,6 +319,10 @@ public class CuratorDb {
         return root.append("upgrader").append("upgradesPerMinute");
     }
 
+    private Path ignoreConfidencePath() {
+        return root.append("upgrader").append("ignoreConfidence");
+    }
+
     private Path versionStatusPath() { return root.append("versionStatus"); }
 
     private Path provisionStatePath() {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java
index 03ac073a34a..162827cdb99 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java
@@ -7,9 +7,10 @@ import com.yahoo.container.jdisc.LoggingRequestHandler;
 import com.yahoo.container.logging.AccessLog;
 import com.yahoo.io.IOUtils;
 import com.yahoo.slime.Inspector;
-import com.yahoo.slime.Slime;
+import com.yahoo.text.Utf8;
 import com.yahoo.vespa.config.SlimeUtils;
 import com.yahoo.vespa.hosted.controller.maintenance.ControllerMaintenance;
+import com.yahoo.vespa.hosted.controller.maintenance.Upgrader;
 import com.yahoo.vespa.hosted.controller.restapi.ErrorResponse;
 import com.yahoo.vespa.hosted.controller.restapi.MessageResponse;
 import com.yahoo.vespa.hosted.controller.restapi.Path;
@@ -62,7 +63,7 @@ public class ControllerApiHandler extends LoggingRequestHandler {
         Path path = new Path(request.getUri().getPath());
         if (path.matches("/controller/v1/")) return root(request);
         if (path.matches("/controller/v1/maintenance/")) return new JobsResponse(maintenance.jobControl());
-        if (path.matches("/controller/v1/jobs/upgrader")) return new UpgraderResponse(maintenance.upgrader().upgradesPerMinute());
+        if (path.matches("/controller/v1/jobs/upgrader")) return new UpgraderResponse(maintenance.upgrader());
         return notFound(path);
     }
 
@@ -101,18 +102,26 @@ public class ControllerApiHandler extends LoggingRequestHandler {
 
     private HttpResponse configureUpgrader(HttpRequest request) {
         String upgradesPerMinuteField = "upgradesPerMinute";
-        Slime slime = toSlime(request.getData());
-        Inspector inspect = slime.get();
+        String ignoreConfidenceField = "ignoreConfidence";
+
+        byte[] jsonBytes = toJsonBytes(request.getData());
+        Inspector inspect = SlimeUtils.jsonToSlime(jsonBytes).get();
+        Upgrader upgrader = maintenance.upgrader();
         if (inspect.field(upgradesPerMinuteField).valid()) {
-            maintenance.upgrader().setUpgradesPerMinute(inspect.field(upgradesPerMinuteField).asDouble());
+            upgrader.setUpgradesPerMinute(inspect.field(upgradesPerMinuteField).asDouble());
+        } else if (inspect.field(ignoreConfidenceField).valid()) {
+            upgrader.ignoreConfidence(inspect.field(ignoreConfidenceField).asBool());
+        } else {
+            return ErrorResponse.badRequest("Unable to configure upgrader with data in request: '" +
+                                                    Utf8.toString(jsonBytes) + "'");
         }
-        return new UpgraderResponse(maintenance.upgrader().upgradesPerMinute());
+
+        return new UpgraderResponse(maintenance.upgrader());
     }
 
-    private Slime toSlime(InputStream jsonStream) {
+    private byte[] toJsonBytes(InputStream jsonStream) {
         try {
-            byte[] jsonBytes = IOUtils.readBytes(jsonStream, 1000 * 1000);
-            return SlimeUtils.jsonToSlime(jsonBytes);
+            return IOUtils.readBytes(jsonStream, 1000 * 1000);
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/UpgraderResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/UpgraderResponse.java
index fe88a0f1f22..3444f710d4c 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/UpgraderResponse.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/UpgraderResponse.java
@@ -4,6 +4,7 @@ import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.slime.Cursor;
 import com.yahoo.slime.JsonFormat;
 import com.yahoo.slime.Slime;
+import com.yahoo.vespa.hosted.controller.maintenance.Upgrader;
 
 import java.io.IOException;
 import java.io.OutputStream;
@@ -13,18 +14,19 @@ import java.io.OutputStream;
  */
 public class UpgraderResponse extends HttpResponse {
 
-    private final double upgradesPerMinute;
+    private final Upgrader upgrader;
 
-    public UpgraderResponse(double upgradesPerMinute) {
+    public UpgraderResponse(Upgrader upgrader) {
         super(200);
-        this.upgradesPerMinute = upgradesPerMinute;
+        this.upgrader = upgrader;
     }
 
     @Override
     public void render(OutputStream outputStream) throws IOException {
         Slime slime = new Slime();
         Cursor root = slime.setObject();
-        root.setDouble("upgradesPerMinute", upgradesPerMinute);
+        root.setDouble("upgradesPerMinute", upgrader.upgradesPerMinute());
+        root.setBool("ignoreConfidence", upgrader.ignoreConfidence());
         new JsonFormat(true).encode(outputStream, slime);
     }
 
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 c1b9c045fbe..1541e4d35f8 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
@@ -6,6 +6,7 @@ import com.yahoo.component.Version;
 import com.yahoo.component.Vtag;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.application.ApplicationList;
+import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 
 import java.time.Instant;
 import java.util.Collection;
@@ -57,8 +58,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
-        int brokenByThisVersion = failingOnThis.without(UpgradePolicy.canary).startedFailingAfter(releasedAt).size();
-        if (brokenByThisVersion >= 4 && brokenByThisVersion >= productionOnThis.size() * 0.1)
+        if (nonCanaryApplicationsBroken(failingOnThis, productionOnThis, releasedAt, controller.curator()))
             return Confidence.broken;
 
         // 'low' unless all canary applications are upgraded
@@ -136,4 +136,17 @@ public class VespaVersion implements Comparable<VespaVersion> {
 
     }
 
+    private static boolean nonCanaryApplicationsBroken(ApplicationList failingOnThis,
+                                                       ApplicationList productionOnThis,
+                                                       Instant releasedAt,
+                                                       CuratorDb curator) {
+        ApplicationList failingNonCanaries = failingOnThis.without(UpgradePolicy.canary).startedFailingAfter(releasedAt);
+        ApplicationList productionNonCanaries = productionOnThis.without(UpgradePolicy.canary);
+
+        if (productionNonCanaries.size() + failingNonCanaries.size() == 0 || curator.readIgnoreConfidence()) return false;
+
+        // 'broken' if 4 non-canary was broken by this, and that is at least 10% of all
+        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/controller/ControllerApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java
index e1c5cdb7742..8047b0d48c9 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java
@@ -7,6 +7,7 @@ import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest;
 import org.junit.Test;
 
 import java.io.File;
+import java.io.IOException;
 
 /**
  * @author bratseth
@@ -42,7 +43,7 @@ public class ControllerApiTest extends ControllerContainerTest {
 
         // Get current configuration
         tester.assertResponse(new Request("http://localhost:8080/controller/v1/jobs/upgrader"),
-                              "{\"upgradesPerMinute\":0.5}",
+                              "{\"upgradesPerMinute\":0.5,\"ignoreConfidence\":false}",
                               200);
 
         // Set invalid configuration
@@ -51,16 +52,22 @@ public class ControllerApiTest extends ControllerContainerTest {
                               "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Upgrades per minute must be >= 0\"}",
                               400);
 
-        // Unrecognized fields are ignored
+        // Unrecognized field
         tester.assertResponse(new Request("http://localhost:8080/controller/v1/jobs/upgrader",
                                           "{\"foo\":bar}", Request.Method.PATCH),
-                              "{\"upgradesPerMinute\":0.5}",
+                              "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Unable to configure upgrader with data in request: '{\\\"foo\\\":bar}'\"}",
+                              + 400);
+
+        // Patch configuration
+        tester.assertResponse(new Request("http://localhost:8080/controller/v1/jobs/upgrader",
+                                          "{\"upgradesPerMinute\":42.0}", Request.Method.PATCH),
+                              "{\"upgradesPerMinute\":42.0,\"ignoreConfidence\":false}",
                               200);
 
-        // Set configuration
+        // Patch configuration
         tester.assertResponse(new Request("http://localhost:8080/controller/v1/jobs/upgrader",
-                                          "{\"upgradesPerMinute\":42}", Request.Method.PATCH),
-                              "{\"upgradesPerMinute\":42.0}",
+                                          "{\"ignoreConfidence\":true}", Request.Method.PATCH),
+                              "{\"upgradesPerMinute\":42.0,\"ignoreConfidence\":true}",
                               200);
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
index 17935906186..bc31079cfe0 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java
@@ -233,6 +233,49 @@ public class VersionStatusTest {
                      VespaVersion.Confidence.broken, confidence(tester.controller(), version3));
     }
 
+
+    @Test
+    public void testIgnoreConfigdeince() {
+        DeploymentTester tester = new DeploymentTester();
+
+        Version version0 = new Version("5.0");
+        tester.upgradeSystem(version0);
+
+        // Setup applications - all running on version0
+        Application canary0 = tester.createAndDeploy("canary0", 1, "canary");
+        Application canary1 = tester.createAndDeploy("canary1", 2, "canary");
+        Application default0 = tester.createAndDeploy("default0", 3, "default");
+        Application default1 = tester.createAndDeploy("default1", 4, "default");
+        Application default2 = tester.createAndDeploy("default2", 5, "default");
+        Application default3 = tester.createAndDeploy("default3", 6, "default");
+        Application default4 = tester.createAndDeploy("default4", 7, "default");
+
+        // New version is released
+        Version version1 = new Version("5.1");
+        tester.upgradeSystem(version1);
+
+        // All canaries upgrade successfully, 1 default apps ok, 3 default apps fail
+        tester.completeUpgrade(canary0, version1, "canary");
+        tester.completeUpgrade(canary1, version1, "canary");
+        tester.upgradeSystem(version1);
+        tester.completeUpgrade(default0, version1, "default");
+        tester.completeUpgradeWithError(default1, version1, "default", stagingTest);
+        tester.completeUpgradeWithError(default2, version1, "default", stagingTest);
+        tester.completeUpgradeWithError(default3, version1, "default", stagingTest);
+        tester.completeUpgradeWithError(default4, version1, "default", stagingTest);
+        tester.updateVersionStatus();
+
+        assertEquals("Canaries have upgraded, 1 of 4 default apps failing: Broken",
+                     Confidence.broken, confidence(tester.controller(), version1));
+
+        // Same as above, but ignore confidence calculations, will force normal confidence
+        tester.controllerTester().curator().writeIgnoreConfidence(true);
+        tester.updateVersionStatus();
+        assertEquals("Canaries have upgraded, 1 of 4 default apps failing, but confidence ignored: Low",
+                     Confidence.normal, confidence(tester.controller(), version1));
+        tester.controllerTester().curator().writeIgnoreConfidence(false);
+    }
+
     @Test
     public void testComputeIgnoresVersionWithUnknownGitMetadata() {
         ControllerTester tester = new ControllerTester();