Remove legacy serialization format of rotation status

4 files changed, 15 insertions, 83 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
index cc021f4e095..421805231a1 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
@@ -6,7 +6,6 @@ import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.application.api.ValidationOverrides;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterSpec;
-import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.slime.ArrayTraverser;
 import com.yahoo.slime.Cursor;
@@ -46,7 +45,6 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.OptionalLong;
-import java.util.TreeMap;
 import java.util.stream.Collectors;
 
 /**
@@ -84,7 +82,6 @@ public class ApplicationSerializer {
     private static final String assignedRotationEndpointField = "endpointId";
     private static final String assignedRotationClusterField = "clusterId";
     private static final String assignedRotationRotationField = "rotationId";
-    private static final String legacyRotationStatusField = "rotationStatus";
     private static final String applicationCertificateField = "applicationCertificate";
 
     // Deployment fields
@@ -162,7 +159,6 @@ public class ApplicationSerializer {
     private static final String rotationIdField = "rotationId";
     private static final String rotationStateField = "state";
     private static final String statusField = "status";
-    private static final String hostnameField = "hostname";
 
     // ------------------ Serialization
 
@@ -185,15 +181,6 @@ public class ApplicationSerializer {
         application.pemDeployKey().ifPresent(pemDeployKey -> root.setString(pemDeployKeyField, pemDeployKey));
         assignedRotationsToSlime(application.rotations(), root, assignedRotationsField);
         toSlime(application.rotationStatus(), root.setArray(rotationStatusField));
-        { // TODO(mpolden): Remove this block after September 2019
-            var firstRotationStatus = application.rotations().stream().findFirst()
-                                                 .map(AssignedRotation::rotationId)
-                                                 .flatMap(rotation -> application.rotationStatus().asMap().entrySet().stream()
-                                                                                 .filter(kv -> kv.getKey().equals(rotation))
-                                                                                 .map(Map.Entry::getValue).findFirst())
-                                                 .orElse(Map.of());
-            legacyToSlime(firstRotationStatus, root.setArray(legacyRotationStatusField));
-        }
         return slime;
     }
 
@@ -345,14 +332,6 @@ public class ApplicationSerializer {
         });
     }
 
-    private void legacyToSlime(Map<ZoneId, RotationState> state, Cursor array) {
-        state.forEach((zone, status) -> {
-            Cursor object = array.addObject();
-            object.setString(hostnameField, zone.value());
-            object.setString(statusField, status.name());
-        });
-    }
-
     private void assignedRotationsToSlime(List<AssignedRotation> rotations, Cursor parent, String fieldName) {
         var rotationsArray = parent.setArray(fieldName);
         for (var rotation : rotations) {
@@ -383,7 +362,7 @@ public class ApplicationSerializer {
                                                             root.field(writeQualityField).asDouble());
         Optional<String> pemDeployKey = Serializers.optionalString(root.field(pemDeployKeyField));
         List<AssignedRotation> assignedRotations = assignedRotationsFromSlime(deploymentSpec, root);
-        RotationStatus rotationStatus = rotationStatusFromSlime(root, assignedRotations.stream().findFirst());
+        RotationStatus rotationStatus = rotationStatusFromSlime(root);
 
         return new Application(id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs,
                                deploying, outstandingChange, ownershipIssueId, owner, majorVersion, metrics,
@@ -430,41 +409,14 @@ public class ApplicationSerializer {
         return Collections.unmodifiableMap(warnings);
     }
 
-    private RotationStatus rotationStatusFromSlime(Inspector parentObject, Optional<AssignedRotation> firstRotation) {
+    private RotationStatus rotationStatusFromSlime(Inspector parentObject) {
         var object = parentObject.field(rotationStatusField);
-        if (firstRotation.isEmpty()) {
-            return RotationStatus.EMPTY;
-        }
-        if (!object.valid()) {
-            // TODO(mpolden): Remove compatibility after September 2019
-            var legacyRotationStatus = legacyRotationStatusFromSlime(parentObject.field(legacyRotationStatusField));
-            var status = new LinkedHashMap<ZoneId, RotationState>();
-            for (var kv : legacyRotationStatus.entrySet()) {
-                // Old format stores hostname instead of zone, but this is only used in substring matching so we can get
-                // away with this hack
-                status.put(ZoneId.from("prod", kv.getKey().value()), kv.getValue());
-            }
-            return new RotationStatus(Map.of(firstRotation.get().rotationId(), status));
-        }
         var statusMap = new LinkedHashMap<RotationId, Map<ZoneId, RotationState>>();
         object.traverse((ArrayTraverser) (idx, statusObject) -> statusMap.put(new RotationId(statusObject.field(rotationIdField).asString()),
                                                                               singleRotationStatusFromSlime(statusObject.field(statusField))));
         return new RotationStatus(statusMap);
     }
 
-    private Map<HostName, RotationState> legacyRotationStatusFromSlime(Inspector object) {
-        if (!object.valid()) {
-            return Collections.emptyMap();
-        }
-        Map<HostName, RotationState> rotationStatus = new TreeMap<>();
-        object.traverse((ArrayTraverser) (idx, inspect) -> {
-            HostName hostname = HostName.from(inspect.field(hostnameField).asString());
-            RotationState status = RotationState.valueOf(inspect.field(statusField).asString());
-            rotationStatus.put(hostname, status);
-        });
-        return Collections.unmodifiableMap(rotationStatus);
-    }
-
     private Map<ZoneId, RotationState> singleRotationStatusFromSlime(Inspector object) {
         if (!object.valid()) {
             return Collections.emptyMap();
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationStatus.java
index 62750a5b608..45ba83712a3 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/rotation/RotationStatus.java
@@ -34,8 +34,7 @@ public class RotationStatus {
     /** Get status of deployment in given rotation, if any */
     public RotationState of(RotationId rotation, Deployment deployment) {
         return of(rotation).entrySet().stream()
-                           // TODO(mpolden): Change to exact comparison after September 2019
-                           .filter(kv -> kv.getKey().value().contains(deployment.zone().value()))
+                           .filter(kv -> kv.getKey().equals(deployment.zone()))
                            .map(Map.Entry::getValue)
                            .findFirst()
                            .orElse(RotationState.unknown);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
index 50ef0f5a7d3..353511a017d 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
@@ -9,7 +9,6 @@ import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.config.SlimeUtils;
 import com.yahoo.vespa.hosted.controller.Application;
-import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision;
@@ -31,7 +30,6 @@ import com.yahoo.vespa.hosted.controller.rotation.RotationState;
 import com.yahoo.vespa.hosted.controller.rotation.RotationStatus;
 import org.junit.Test;
 
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -39,7 +37,6 @@ import java.time.Instant;
 import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -52,7 +49,6 @@ import static com.yahoo.config.provision.SystemName.main;
 import static com.yahoo.vespa.hosted.controller.ControllerTester.writable;
 import static java.util.Optional.empty;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 
 /**
  * @author bratseth
@@ -104,10 +100,9 @@ public class ApplicationSerializerTest {
 
         DeploymentJobs deploymentJobs = new DeploymentJobs(projectId, statusList, empty(), true);
 
-        var rotationStatusMap = new LinkedHashMap<ZoneId, RotationState>();
-        rotationStatusMap.put(ZoneId.from("prod", "us-west-1"), RotationState.in);
-        rotationStatusMap.put(ZoneId.from("prod", "us-east-3"), RotationState.out);
-        var rotationStatus = new RotationStatus(Map.of(new RotationId("my-rotation"), rotationStatusMap));
+        var rotationStatus = new RotationStatus(Map.of(new RotationId("my-rotation"),
+                                                       Map.of(ZoneId.from("prod", "us-west-1"), RotationState.in,
+                                                              ZoneId.from("prod", "us-east-3"), RotationState.out)));
 
         Application original = new Application(ApplicationId.from("t1", "a1", "i1"),
                                                Instant.now().truncatedTo(ChronoUnit.MILLIS),
@@ -250,20 +245,4 @@ public class ApplicationSerializerTest {
         // ok if no error
     }
 
-    @Test // TODO(mpolden): Remove after september 2019
-    public void testLegacyRotationStatus() throws Exception {
-        var json = Files.readAllBytes(testData.resolve("complete-application.json"));
-        var application = applicationSerializer.fromSlime(SlimeUtils.jsonToSlime(json));
-        var expected = new RotationStatus(Map.of(new RotationId("rotation-foo"),
-                                                 Map.of(ZoneId.from("prod", "host1.fqdn"), RotationState.out,
-                                                        ZoneId.from("prod", "host2.fqdn"), RotationState.in)));
-        assertEquals(expected, application.rotationStatus());
-
-        // Writes both new and old format
-        var serializedJson = new String(SlimeUtils.toJsonBytes(applicationSerializer.toSlime(application)), StandardCharsets.UTF_8);
-        var jsonFragment = "\"rotationStatus2\":[{\"rotationId\":\"rotation-foo\",\"status\":[{\"environment\":\"prod\",\"region\":\"host1.fqdn\",\"state\":\"out\"},{\"environment\":\"prod\",\"region\":\"host2.fqdn\",\"state\":\"in\"}]}]," +
-                           "\"rotationStatus\":[{\"hostname\":\"prod.host1.fqdn\",\"status\":\"out\"},{\"hostname\":\"prod.host2.fqdn\",\"status\":\"in\"}]}";
-        assertTrue("Writes both new and old format", serializedJson.contains(jsonFragment));
-    }
-
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json
index c43b666d636..28f505e88ec 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json
@@ -519,14 +519,16 @@
       "endpointId": "default"
     }
   ],
-  "rotationStatus": [
+  "rotationStatus2": [
     {
-      "status": "out",
-      "hostname": "host1.fqdn"
-    },
-    {
-      "status": "in",
-      "hostname": "host2.fqdn"
+      "rotationId": "rotation-foo",
+      "status": [
+        {
+          "environment": "prod",
+          "region": "us-east-3",
+          "state": "in"
+        }
+      ]
     }
   ]
 }