Job serialisation and log store changes

11 files changed, 244 insertions, 53 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/LogStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/LogStore.java
index 959ac2b8680..918047edca1 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/LogStore.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/LogStore.java
@@ -21,10 +21,10 @@ public interface LogStore {
     void setConvergenceLog(RunId id, String convergenceLog);
 
     /** @return the result of prepare of the test application for the given deployment job. */
-    PrepareResponse getPrepareResponse(RunId id);
+    String getDeploymentLog(RunId id);
 
     /** Stores the given result of prepare of the test application for the given deployment job. */
-    void setPrepareResponse(RunId id, PrepareResponse prepareResponse);
+    void setDeploymentLog(RunId id, String deploymentLog);
 
     /** Deletes all data associated with test of a given deployment job */
     void deleteTestData(RunId id);
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockLogStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockLogStore.java
index c5dba3b8fa7..bc9f6247055 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockLogStore.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockLogStore.java
@@ -35,17 +35,12 @@ public class MockLogStore implements LogStore {
     }
 
     @Override
-    public PrepareResponse getPrepareResponse(RunId id) {
-        PrepareResponse prepareResponse = new PrepareResponse();
-        prepareResponse.message = "foo";
-        prepareResponse.configChangeActions = new ConfigChangeActions(Collections.emptyList(),
-                                                                      Collections.emptyList());
-        prepareResponse.tenant = new TenantId("tenant");
-        return prepareResponse;
+    public String getDeploymentLog(RunId id) {
+        return "SUCCESS";
     }
 
     @Override
-    public void setPrepareResponse(RunId id, PrepareResponse prepareResponse) {
+    public void setDeploymentLog(RunId id, String deploymentLog) {
 
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 7c7fb27f3eb..0dd80d54919 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -55,7 +55,6 @@ import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.time.Clock;
-import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
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 466a658b49c..8093c4d7a18 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
@@ -30,9 +30,12 @@ import static com.google.common.collect.ImmutableList.copyOf;
  *
  * Keys are the {@link ApplicationId} of the real application, for which the deployment job is run, and the
  * {@link JobType} of the real deployment to test.
- *
  * Although the deployment jobs are themselves applications, their IDs are not to be referenced.
  *
+ * Jobs consist of sets of {@link Step}s, defined in {@link JobProfile}s.
+ * Each run is represented by a {@link RunStatus}, which holds the status of each step of the run, as well as
+ * some other meta data.
+ *
  * @author jonmv
  */
 public class JobController {
@@ -45,10 +48,43 @@ public class JobController {
         this.controller = controller;
         this.curator = controller.curator();
         this.logs = logStore;
+
+    }
+
+    /** Rewrite all job data with the newest format. */
+    public void updateStorage() {
+        for (ApplicationId id : applications())
+            for (JobType type : jobs(id)) {
+                locked(id, type, runs -> {
+                });
+                last(id, type).ifPresent(last -> locked(last.id(), run -> run));
+            }
     }
 
-    public LogStore logs() {
-        return logs;
+    /** Returns the details currently logged for the given run. */
+    public RunDetails details(RunId id) {
+        return new RunDetails(logs.getDeploymentLog(id), logs.getConvergenceLog(id), logs.getTestLog(id));
+    }
+
+    /** Appends the given string to the currently stored deployment logs for the given run. */
+    public void logDeployment(RunId id, String appendage) {
+        try (Lock __ = curator.lock(id.application(), id.type())) {
+            logs.setDeploymentLog(id, logs.getDeploymentLog(id).concat(appendage));
+        }
+    }
+
+    /** Appends the given string to the currently stored convergence logs for the given run. */
+    public void logConvergence(RunId id, String appendage) {
+        try (Lock __ = curator.lock(id.application(), id.type())) {
+            logs.setConvergenceLog(id, logs.getConvergenceLog(id).concat(appendage));
+        }
+    }
+
+    /** Appends the given string to the currently stored test logs for the given run. */
+    public void logTest(RunId id, String appendage) {
+        try (Lock __ = curator.lock(id.application(), id.type())) {
+            logs.setTestLog(id, logs.getTestLog(id).concat(appendage));
+        }
     }
 
     // TODO jvenstad: Remove this, and let the DeploymentTrigger trigger directly with the correct BuildService.
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java
index d2e0a4fe705..82464820d9b 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobProfile.java
@@ -58,7 +58,7 @@ public enum JobProfile {
             case test: return systemTest;
             case staging: return stagingTest;
             case prod: return production;
-            default: throw new IllegalArgumentException("Unexpected environment " + type.environment());
+            default: throw new AssertionError("Unexpected environment '" + type.environment() + "'!");
         }
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunDetails.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunDetails.java
index 300452ac0a8..98969bd4508 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunDetails.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunDetails.java
@@ -1,8 +1,5 @@
 package com.yahoo.vespa.hosted.controller.deployment;
 
-import com.yahoo.vespa.hosted.controller.api.ActivateResult;
-import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse;
-
 /**
  * Contains details about a deployment job run.
  *
@@ -10,14 +7,26 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareRes
  */
 public class RunDetails {
 
-    private final PrepareResponse deploymentResult;
+    private final String deploymentLog;
     private final String convergenceLog;
     private final String testLog;
 
-    public RunDetails(PrepareResponse deploymentResult, String convergenceLog, String testLog) {
-        this.deploymentResult = deploymentResult;
+    public RunDetails(String deploymentLog, String convergenceLog, String testLog) {
+        this.deploymentLog = deploymentLog;
         this.convergenceLog = convergenceLog;
         this.testLog = testLog;
     }
 
+    public String getDeploymentLog() {
+        return deploymentLog;
+    }
+
+    public String getConvergenceLog() {
+        return convergenceLog;
+    }
+
+    public String getTestLog() {
+        return testLog;
+    }
+
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunStatus.java
index 8fa463d3f1b..ba94b21b8bd 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RunStatus.java
@@ -26,6 +26,7 @@ public class RunStatus {
     private final Map<Step, Step.Status> steps;
     private final Instant start;
     private final Optional<Instant> end;
+    // TODO jvenstad: Add a Versions object and a reason String. Requires shortcutting of triggering of these runs.
 
     // For deserialisation only -- do not use!
     public RunStatus(RunId id, Map<Step, Step.Status> steps, Instant start, Optional<Instant> end) {
@@ -42,6 +43,9 @@ public class RunStatus {
     }
 
     public RunStatus with(Step.Status status, LockedStep step) {
+        if (end.isPresent())
+            throw new IllegalStateException("This step ended at " + end.get() + " -- it can't be further modified!");
+
         EnumMap<Step, Step.Status> steps = new EnumMap<>(this.steps);
         steps.put(step.get(), requireNonNull(status));
         return new RunStatus(id, steps, start, end);
@@ -80,36 +84,66 @@ public class RunStatus {
         return end;
     }
 
+    /** Returns whether the run has failed, and should switch to its run-always steps. */
     public boolean hasFailed() {
         return steps.values().contains(failed);
     }
 
+    /** Returns whether the run has ended, i.e., has become inactive, and can no longer be updated. */
     public boolean hasEnded() {
         return end.isPresent();
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if ( ! (o instanceof RunStatus)) return false;
+
+        RunStatus status = (RunStatus) o;
+
+        return id.equals(status.id);
+    }
+
+    @Override
+    public int hashCode() {
+        return id.hashCode();
+    }
+
+    @Override
+    public String toString() {
+        return "RunStatus{" +
+               "id=" + id +
+               ", start=" + start +
+               ", end=" + end +
+               ", steps=" + steps +
+               '}';
+    }
+
+    /** Returns the list of steps to run for this job right now, depending on whether the job has failed. */
     public List<Step> readySteps() {
         return hasFailed() ? forcedSteps() : normalSteps();
     }
 
+    /** Returns the list of unfinished steps whose prerequisites have all succeeded. */
     private List<Step> normalSteps() {
         return ImmutableList.copyOf(steps.entrySet().stream()
                                          .filter(entry ->    entry.getValue() == unfinished
-                                                           && entry.getKey().prerequisites().stream()
-                                                                   .allMatch(step -> steps.get(step) == null
-                                                                                     || steps.get(step) == succeeded))
+                                                          && entry.getKey().prerequisites().stream()
+                                                                  .allMatch(step ->    steps.get(step) == null
+                                                                                    || steps.get(step) == succeeded))
                                          .map(Map.Entry::getKey)
                                          .iterator());
     }
 
+    /** Returns the list of not-yet-succeeded run-always steps whose run-always prerequisites have all succeeded. */
     private List<Step> forcedSteps() {
         return ImmutableList.copyOf(steps.entrySet().stream()
                                          .filter(entry ->    entry.getValue() != succeeded
-                                                           && JobProfile.of(id.type()).alwaysRun().contains(entry.getKey())
-                                                           && entry.getKey().prerequisites().stream()
-                                                                   .filter(JobProfile.of(id.type()).alwaysRun()::contains)
-                                                                   .allMatch(step -> steps.get(step) == null
-                                                                                     || steps.get(step) == succeeded))
+                                                          && JobProfile.of(id.type()).alwaysRun().contains(entry.getKey())
+                                                          && entry.getKey().prerequisites().stream()
+                                                                  .filter(JobProfile.of(id.type()).alwaysRun()::contains)
+                                                                  .allMatch(step ->    steps.get(step) == null
+                                                                                    || steps.get(step) == succeeded))
                                          .map(Map.Entry::getKey)
                                          .iterator());
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RealStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RealStepRunner.java
index 9f539e854e8..1ed55548ee7 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RealStepRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RealStepRunner.java
@@ -1,7 +1,9 @@
 package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
+import com.yahoo.vespa.hosted.controller.api.ActivateResult;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
@@ -54,21 +56,7 @@ public class RealStepRunner implements StepRunner {
     }
 
     private Step.Status deployInitialReal(RunId id) {
-        try {
-            // TODO jvenstad: Do whatever is required based on the result, and log all of this.
-            controller.applications().deploy(id.application(),
-                                             id.type().zone(controller.system()).get(),
-                                             Optional.empty(),
-                                             new DeployOptions(false, Optional.empty(), false, true));
-            return succeeded;
-        }
-        catch (ConfigServerException e) {
-            // TODO jvenstad: Consider retrying different things as well.
-            // TODO jvenstad: Log error information.
-            if (id.type().isTest() && e.getErrorCode() == ConfigServerException.ErrorCode.OUT_OF_CAPACITY)
-                return unfinished;
-        }
-        return failed;
+        return deployReal(id, true);
     }
 
     private Step.Status installInitialReal(RunId id) {
@@ -79,7 +67,7 @@ public class RealStepRunner implements StepRunner {
 
     private Step.Status deployReal(RunId id) {
         // Separate out deploy logic from above, and reuse.
-        throw new AssertionError();
+        return deployReal(id,false);
     }
 
     private Step.Status deployTester(RunId id) {
@@ -124,4 +112,29 @@ public class RealStepRunner implements StepRunner {
         throw new AssertionError();
     }
 
+    private Step.Status deployReal(RunId id, boolean setTheStage) {
+        try {
+            // TODO jvenstad: Do whatever is required based on the result, and log all of this.
+            ActivateResult result = controller.applications().deploy(id.application(),
+                                                                     id.type().zone(controller.system()).get(),
+                                                                     Optional.empty(),
+                                                                     new DeployOptions(false,
+                                                                                       Optional.empty(),
+                                                                                       false,
+                                                                                       setTheStage));
+            return succeeded;
+        }
+        catch (ConfigServerException e) {
+            // TODO jvenstad: Consider retrying different things as well.
+            // TODO jvenstad: Log error information.
+            if (id.type().isTest() && e.getErrorCode() == ConfigServerException.ErrorCode.OUT_OF_CAPACITY)
+                return unfinished;
+        }
+        return failed;
+    }
+
+    private Application application(ApplicationId id) {
+        return controller.applications().require(id);
+    }
+
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/JobSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/JobSerializer.java
index ba66bb67636..f34cde002d0 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/JobSerializer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/JobSerializer.java
@@ -96,15 +96,15 @@ public class JobSerializer {
         run.steps().forEach((step, status) -> stepsObject.setString(valueOf(step), valueOf(status)));
     }
 
-    private static String valueOf(Step step) {
+    static String valueOf(Step step) {
         switch (step) {
             case deployInitialReal  : return "DIR";
             case installInitialReal : return "IIR";
             case deployReal         : return "DR" ;
-            case installReal        : return "ID" ;
+            case installReal        : return "IR" ;
             case deactivateReal     : return "DAR";
             case deployTester       : return "DT" ;
-            case installTester      : return "IR" ;
+            case installTester      : return "IT" ;
             case deactivateTester   : return "DAT";
             case startTests         : return "ST" ;
             case storeData          : return "SD" ;
@@ -113,15 +113,15 @@ public class JobSerializer {
         }
     }
 
-    private static Step stepOf(String step) {
+    static Step stepOf(String step) {
         switch (step) {
             case "DIR" : return deployInitialReal ;
             case "IIR" : return installInitialReal;
             case "DR"  : return deployReal        ;
-            case "ID"  : return installReal       ;
+            case "IR"  : return installReal       ;
             case "DAR" : return deactivateReal    ;
             case "DT"  : return deployTester      ;
-            case "IR"  : return installTester     ;
+            case "IT"  : return installTester     ;
             case "DAT" : return deactivateTester  ;
             case "ST"  : return startTests        ;
             case "SD"  : return storeData         ;
@@ -130,7 +130,7 @@ public class JobSerializer {
         }
     }
 
-    private static String valueOf(Status status) {
+    static String valueOf(Status status) {
         switch (status) {
             case unfinished : return "U";
             case failed     : return "F";
@@ -139,7 +139,7 @@ public class JobSerializer {
         }
     }
 
-    private static Status statusOf(String status) {
+    static Status statusOf(String status) {
         switch (status) {
             case "U" : return unfinished;
             case "F" : return failed    ;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/JobSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/JobSerializerTest.java
new file mode 100644
index 00000000000..e6e726bec5e
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/JobSerializerTest.java
@@ -0,0 +1,84 @@
+package com.yahoo.vespa.hosted.controller.persistence;
+
+import com.google.common.collect.ImmutableMap;
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.vespa.config.SlimeUtils;
+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.deployment.RunStatus;
+import com.yahoo.vespa.hosted.controller.deployment.Step;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.util.Collections;
+
+import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.failed;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.unfinished;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.deactivateReal;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.deactivateTester;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.deployInitialReal;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.deployReal;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.deployTester;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.installInitialReal;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.installReal;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.installTester;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.report;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.startTests;
+import static com.yahoo.vespa.hosted.controller.deployment.Step.storeData;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class JobSerializerTest {
+
+    private static final JobSerializer serializer = new JobSerializer();
+    private static final Path runFile = Paths.get("src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json");
+    private static final RunId id = new RunId(ApplicationId.from("tenant", "application", "default"),
+                                               JobType.productionUsEast3,
+                                               (long) 112358);
+    private static final Instant start = Instant.parse("2007-12-03T10:15:30.00Z");
+
+    @Test
+    public void testSerialization() throws IOException {
+        for (Step step : Step.values())
+            assertEquals(step, JobSerializer.stepOf(JobSerializer.valueOf(step)));
+
+        for (Step.Status status : Step.Status.values())
+            assertEquals(status, JobSerializer.statusOf(JobSerializer.valueOf(status)));
+
+        // The purpose of this serialised data is to ensure a new format does not break everything, so keep it up to date!
+        RunStatus run = serializer.runsFromSlime(SlimeUtils.jsonToSlime(Files.readAllBytes(runFile))).get(id);
+        for (Step step : Step.values())
+            assertTrue(run.steps().containsKey(step));
+
+        assertEquals(id, run.id());
+        assertEquals(start, run.start());
+        assertFalse(run.hasEnded());
+        assertEquals(ImmutableMap.<Step, Step.Status>builder()
+                    .put(deployInitialReal, unfinished)
+                    .put(installInitialReal, failed)
+                    .put(deployReal, succeeded)
+                    .put(installReal, unfinished)
+                    .put(deactivateReal, failed)
+                    .put(deployTester, succeeded)
+                    .put(installTester, unfinished)
+                    .put(deactivateTester, failed)
+                    .put(startTests, succeeded)
+                    .put(storeData, unfinished)
+                    .put(report, failed)
+                    .build(),
+                     run.steps());
+
+        RunStatus phoenix = serializer.runsFromSlime(serializer.toSlime(Collections.singleton(run))).get(id);
+        assertEquals(run.id(), phoenix.id());
+        assertEquals(run.start(), phoenix.start());
+        assertEquals(run.end(), phoenix.end());
+        assertEquals(run.steps(), phoenix.steps());
+    }
+
+}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json
new file mode 100644
index 00000000000..4db94eb429a
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/run-status.json
@@ -0,0 +1,21 @@
+[
+  {
+    "id": "tenant:application:default",
+    "type": "production-us-east-3",
+    "number": 112358,
+    "start": 1196676930000,
+    "steps": {
+      "DIR": "U",
+      "IIR": "F",
+      "DR":  "S",
+      "IR":  "U",
+      "DAR": "F",
+      "DT":  "S",
+      "IT":  "U",
+      "DAT": "F",
+      "ST":  "S",
+      "SD":  "U",
+      "R":   "F"
+    }
+  }
+]
\ No newline at end of file