Merge pull request #21501 from vespa-engine/jonmv/more-dep-orch-adjustments

Allow multiple test reports

7 files changed, 45 insertions, 23 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
index b7bfade98a6..8c7e60bd199 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
@@ -670,6 +670,7 @@ public class InternalStepRunner implements StepRunner {
             case INCONCLUSIVE:
                 long sleepMinutes = Math.max(15, Math.min(120, Duration.between(deployment.get().at(), controller.clock().instant()).toMinutes() / 20));
                 logger.log("Tests were inconclusive, and will run again in " + sleepMinutes + " minutes.");
+                controller.jobController().updateTestReport(id);
                 controller.jobController().locked(id, run -> run.sleepingUntil(controller.clock().instant().plusSeconds(60 * sleepMinutes)));
                 return Optional.of(reset);
             case ERROR:
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index dadaed9334d..66a30050f7a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -3,11 +3,7 @@ package com.yahoo.vespa.hosted.controller.deployment;
 
 import com.google.common.collect.ImmutableSortedMap;
 import com.yahoo.component.Version;
-import com.yahoo.config.application.api.DeploymentInstanceSpec;
-import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.InstanceName;
-import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.controller.Application;
@@ -28,15 +24,12 @@ import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageDiff;
-import com.yahoo.vespa.hosted.controller.application.pkg.ZipStreamReader;
 import com.yahoo.vespa.hosted.controller.persistence.BufferedLogStore;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 
-import java.io.ByteArrayInputStream;
 import java.security.cert.X509Certificate;
 import java.time.Duration;
 import java.time.Instant;
-import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
@@ -52,13 +45,10 @@ import java.util.TreeMap;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
-import java.util.function.Function;
 import java.util.function.UnaryOperator;
 import java.util.logging.Level;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import static com.yahoo.config.provision.Environment.prod;
 import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.reset;
 import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.running;
 import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded;
@@ -74,7 +64,6 @@ import static java.util.function.Predicate.not;
 import static java.util.logging.Level.INFO;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toMap;
-import static java.util.stream.Collectors.toSet;
 import static java.util.stream.Collectors.toUnmodifiableList;
 
 /**
@@ -219,8 +208,8 @@ public class JobController {
         });
     }
 
-    public Optional<String> getTestReport(RunId id) {
-        return logs.readTestReport(id);
+    public Optional<String> getTestReports(RunId id) {
+        return logs.readTestReports(id);
     }
 
     /** Stores the given certificate as the tester certificate for this run, or throws if it's already set. */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java
index 212e27ba0c9..059eb37bb59 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java
@@ -18,6 +18,8 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.stream.Collectors;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 /**
  * Stores logs in bite-sized chunks using a {@link CuratorDb}, and flushes to a
  * {@link com.yahoo.vespa.hosted.controller.api.integration.RunDataStore} when the log is final.
@@ -126,12 +128,21 @@ public class BufferedLogStore {
                                       after);
     }
 
-    public Optional<String> readTestReport(RunId id) {
-        return store.getTestReport(id).map(String::new);
+    public Optional<String> readTestReports(RunId id) {
+        return store.getTestReport(id).map(bytes -> "[" + new String(bytes, UTF_8) + "]");
     }
 
     public void writeTestReport(RunId id, TestReport report) {
-        store.putTestReport(id, report.toJson().getBytes());
+        byte[] bytes = report.toJson().getBytes(UTF_8);
+        Optional<byte[]> existing = store.getTestReport(id);
+        if (existing.isPresent()) {
+            byte[] aggregate = new byte[existing.get().length + 1 + bytes.length];
+            System.arraycopy(existing.get(), 0, aggregate, 0, existing.get().length);
+            aggregate[existing.get().length] = ',';
+            System.arraycopy(bytes, 0, aggregate, existing.get().length + 1, bytes.length);
+            bytes = aggregate;
+        }
+        store.putTestReport(id, bytes);
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java
index 5e16e2b3b47..2b52143f574 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java
@@ -8,6 +8,7 @@ import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.restapi.MessageResponse;
 import com.yahoo.restapi.SlimeJsonResponse;
+import com.yahoo.slime.ArrayTraverser;
 import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Slime;
 import com.yahoo.slime.SlimeUtils;
@@ -147,10 +148,16 @@ class JobControllerApiHandlerHelper {
         });
 
         // If a test report is available, include it in the response.
-        Optional<String> testReport = jobController.getTestReport(runId);
+        Optional<String> testReport = jobController.getTestReports(runId);
         testReport.map(SlimeUtils::jsonToSlime)
-                .map(Slime::get)
-                .ifPresent(reportCursor -> SlimeUtils.copyObject(reportCursor, detailsObject.setObject("testReport")));
+                  .map(Slime::get)
+                  .ifPresent(reportArrayCursor -> {
+                      reportArrayCursor.traverse((ArrayTraverser) (i, reportCursor) -> {
+                          if (i > 0) return;
+                          SlimeUtils.copyObject(reportCursor, detailsObject.setObject("testReport"));
+                      });
+                      SlimeUtils.copyArray(reportArrayCursor, detailsObject.setArray("testReports"));
+                  });
 
         return new SlimeJsonResponse(slime);
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index 2f4864637dd..4e538acb8f2 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -1,7 +1,6 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.deployment;
 
-import com.google.common.collect.ImmutableList;
 import com.yahoo.component.Version;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.provision.AthenzDomain;
@@ -22,6 +21,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeFilter;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.TestReport;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud;
 import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMailer;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
@@ -54,7 +54,6 @@ import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.app
 import static com.yahoo.vespa.hosted.controller.deployment.DeploymentTester.instanceId;
 import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.deploymentFailed;
 import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.installationFailed;
-import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.reset;
 import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.running;
 import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.success;
 import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.failed;
@@ -345,6 +344,7 @@ public class InternalStepRunnerTest {
         RunId id = app.startSystemTestTests();
         tester.cloud().add(new LogEntry(0, Instant.ofEpochMilli(123), info, "Not enough data!"));
         tester.cloud().set(TesterCloud.Status.INCONCLUSIVE);
+        tester.cloud().testReport(TestReport.fromJson("{\"foo\":1}"));
 
         long lastId1 = tester.jobs().details(id).get().lastId().getAsLong();
         Instant instant1 = tester.clock().instant();
@@ -371,6 +371,7 @@ public class InternalStepRunnerTest {
         assertTrue(tester.jobs().run(id).get().steps().get(Step.endTests).startTime().isPresent());
 
         tester.cloud().set(TesterCloud.Status.SUCCESS);
+        tester.cloud().testReport(TestReport.fromJson("{\"bar\":2}"));
         long lastId2 = tester.jobs().details(id).get().lastId().getAsLong();
         tester.runner().run();
         assertEquals(success, tester.jobs().run(id).get().status());
@@ -381,6 +382,8 @@ public class InternalStepRunnerTest {
                              new LogEntry(lastId1 + 15, instant1, info, "### Run will reset, and start over at " + instant1.plusSeconds(900).truncatedTo(SECONDS)),
                              new LogEntry(lastId1 + 16, instant1, info, ""),
                              new LogEntry(lastId2 + 1, tester.clock().instant(), info, "Tests completed successfully."));
+
+        assertEquals("[{\"foo\":1},{\"bar\":2}]", tester.jobs().getTestReports(id).get());
     }
 
     @Test
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json
index 66173ec4976..f675825c3b6 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-log.json
@@ -397,5 +397,13 @@
       "success": 1,
       "failed": 0
     }
-  }
+  },
+  "testReports": [
+    {
+      "summary": {
+        "success": 1,
+        "failed": 0
+      }
+    }
+  ]
 }
diff --git a/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java b/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java
index d4999f5c256..5e0cafe6527 100644
--- a/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java
+++ b/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java
@@ -62,7 +62,10 @@ public class SlimeUtils {
         }
     }
 
-    private static void copyArray(Inspector from, Cursor to) {
+    public static void copyArray(Inspector from, Cursor to) {
+        if (from.type() != Type.ARRAY) {
+            throw new IllegalArgumentException("Cannot copy array: " + from);
+        }
         from.traverse((ArrayTraverser) (i, inspector) -> addValue(inspector, to));
     }