Keep steps and add new stepDetails

2 files changed, 26 insertions, 28 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java
index 71e5934c97e..00021c4765a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java
@@ -62,8 +62,8 @@ public class Run {
     /** Returns a new Run with the status of the given completed step set accordingly. */
     public Run with(RunStatus status, LockedStep step) {
         requireActive();
-        StepInfo stepInfo = steps.get(step.get());
-        if (stepInfo == null || stepInfo.status() != unfinished)
+        StepInfo stepInfo = getRequiredStepInfo(step.get());
+        if (stepInfo.status() != unfinished)
             throw new IllegalStateException("Step '" + step.get() + "' can't be set to '" + status + "'" +
                                      " -- it already completed with status '" + stepInfo.status() + "'!");
 
@@ -76,8 +76,8 @@ public class Run {
     /** Returns a new Run with a new start time*/
     public Run with(Instant startTime, LockedStep step) {
         requireActive();
-        StepInfo stepInfo = steps.get(step.get());
-        if (stepInfo == null || stepInfo.status() != unfinished)
+        StepInfo stepInfo = getRequiredStepInfo(step.get());
+        if (stepInfo.status() != unfinished)
             throw new IllegalStateException("Unable to set start timestamp of step " + step.get() +
                     ": it has already completed with status " + stepInfo.status() + "!");
 
@@ -132,6 +132,10 @@ public class Run {
         return Optional.ofNullable(steps.get(step));
     }
 
+    private StepInfo getRequiredStepInfo(Step step) {
+        return stepInfo(step).orElseThrow(() -> new IllegalArgumentException("There is no such step " + step + " for run " + id));
+    }
+
     /** Returns status of step. */
     public Optional<Step.Status> stepStatus(Step step) {
         return stepInfo(step).map(StepInfo::status);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java
index bc36d1d5f85..72c1891fa37 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java
@@ -68,7 +68,7 @@ class RunSerializer {
 
     // TODO: Remove "steps" when there are no traces of it in the controllers
     private static final String stepsField = "steps";
-    private static final String steps2Field = "steps2";
+    private static final String stepDetailsField = "stepDetails";
     private static final String startTimeField = "startTime";
     private static final String applicationField = "id";
     private static final String jobTypeField = "type";
@@ -106,21 +106,19 @@ class RunSerializer {
 
     private Run runFromSlime(Inspector runObject) {
         var steps = new EnumMap<Step, StepInfo>(Step.class);
-        runObject.field(steps2Field).traverse(((ObjectTraverser) (step, info) -> {
-            Inspector startTimeValue = info.field(startTimeField);
+        Inspector detailsField = runObject.field(stepDetailsField);
+        runObject.field(stepsField).traverse((ObjectTraverser) (step, status) -> {
+            Step typedStep = stepOf(step);
+
+            // For historical reasons are the step details stored in a separate JSON structure from the step statuses.
+            Inspector stepDetailsField = detailsField.field(step);
+            Inspector startTimeValue = stepDetailsField.field(startTimeField);
             Optional<Instant> startTime = startTimeValue.valid() ?
                     Optional.of(instantOf(startTimeValue.asLong())) :
                     Optional.empty();
-            Step typedStep = stepOf(step);
-            steps.put(typedStep, new StepInfo(typedStep, stepStatusOf(info.field(statusField).asString()), startTime));
-        }));
-        if (steps.isEmpty()) {
-            // backward compatibility - until all runs in zk contains steps2 field
-            runObject.field(stepsField).traverse((ObjectTraverser) (step, status) -> {
-                Step typedStep = stepOf(step);
-                steps.put(typedStep, new StepInfo(typedStep, stepStatusOf(status.asString()), Optional.empty()));
-            });
-        }
+
+            steps.put(typedStep, new StepInfo(typedStep, stepStatusOf(status.asString()), startTime));
+        });
         return new Run(new RunId(ApplicationId.fromSerializedForm(runObject.field(applicationField).asString()),
                                  JobType.fromJobName(runObject.field(jobTypeField).asString()),
                                  runObject.field(numberField).asLong()),
@@ -197,17 +195,13 @@ class RunSerializer {
         run.testerCertificate().ifPresent(certificate -> runObject.setString(testerCertificateField, X509CertificateUtils.toPem(certificate)));
 
         Cursor stepsObject = runObject.setObject(stepsField);
-        Cursor steps2Object = runObject.setObject(steps2Field);
-        run.steps().forEach((step, stepInfo) -> {
-            String stepString = valueOf(step);
-            String statusString = valueOf(stepInfo.status());
-            Cursor step2Object = steps2Object.setObject(stepString);
-            step2Object.setString(statusField, statusString);
-            stepInfo.startTime().ifPresent(startTime -> step2Object.setLong(startTimeField, valueOf(startTime)));
-
-            // backward compatibility - until all controllers have been upgraded
-            stepsObject.setString(stepString, statusString);
-        });
+        run.steps().forEach((step, statusInfo) -> stepsObject.setString(valueOf(step), valueOf(statusInfo.status())));
+
+        // For historical reasons are the step details stored in a different field from the step statuses.
+        Cursor stepDetailsObject = runObject.setObject(stepDetailsField);
+        run.steps().forEach((step, statusInfo) ->
+                statusInfo.startTime().ifPresent(startTime ->
+                        stepDetailsObject.setString(valueOf(step), valueOf(statusInfo.status()))));
 
         Cursor versionsObject = runObject.setObject(versionsField);
         toSlime(run.versions().targetPlatform(), run.versions().targetApplication(), versionsObject);