Allow clearing OS target in REST API

3 files changed, 48 insertions, 30 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 1c4f0994f8c..af56666c6eb 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
@@ -251,6 +251,19 @@ public class Controller extends AbstractComponent {
         }
     }
 
+    /** Clear the target OS version for given cloud in this system */
+    public void cancelOsUpgradeIn(CloudName cloudName) {
+        try (Mutex lock = curator.lockOsVersions()) {
+            Map<CloudName, OsVersionTarget> targets = curator.readOsVersionTargets().stream()
+                                                             .collect(Collectors.toMap(t -> t.osVersion().cloud(),
+                                                                                       Function.identity()));
+            if (targets.remove(cloudName) == null) {
+                throw new IllegalArgumentException("Cloud '" + cloudName.value() + " has no OS upgrade target");
+            }
+            curator.writeOsVersionTargets(new TreeSet<>(targets.values()));
+        }
+    }
+
     /** Returns the current OS version status */
     public OsVersionStatus osVersionStatus() {
         return curator.readOsVersionStatus();
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java
index 88fd3a58d23..853739ee9c3 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java
@@ -19,6 +19,7 @@ import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.Slime;
 import com.yahoo.slime.SlimeUtils;
+import com.yahoo.slime.Type;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.auditlog.AuditLoggingRequestHandler;
 import com.yahoo.vespa.hosted.controller.versions.OsVersionTarget;
@@ -32,6 +33,7 @@ import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.StringJoiner;
+import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
 
@@ -71,7 +73,7 @@ public class OsApiHandler extends AuditLoggingRequestHandler {
 
     private HttpResponse patch(HttpRequest request) {
         Path path = new Path(request.getUri());
-        if (path.matches("/os/v1/")) return new SlimeJsonResponse(setOsVersion(request));
+        if (path.matches("/os/v1/")) return setOsVersion(request);
         return ErrorResponse.notFoundError("Nothing at " + path);
     }
 
@@ -130,36 +132,20 @@ public class OsApiHandler extends AuditLoggingRequestHandler {
         return zones.zones().stream().map(ZoneApi::getId).collect(Collectors.toList());
     }
 
-    private Slime setOsVersion(HttpRequest request) {
+    private HttpResponse setOsVersion(HttpRequest request) {
         Slime requestData = toSlime(request.getData());
         Inspector root = requestData.get();
-        Inspector versionField = root.field("version");
-        Inspector cloudField = root.field("cloud");
-        Inspector upgradeBudgetField = root.field("upgradeBudget");
-        boolean force = root.field("force").asBool();
-        if (!versionField.valid() || !cloudField.valid() || !upgradeBudgetField.valid()) {
-            throw new IllegalArgumentException("Fields 'version', 'cloud' and 'upgradeBudget' are required");
-        }
-
-        CloudName cloud = CloudName.from(cloudField.asString());
-        Version target;
-        try {
-            target = Version.fromString(versionField.asString());
-        } catch (IllegalArgumentException e) {
-            throw new IllegalArgumentException("Invalid version '" + versionField.asString() + "'", e);
-        }
-        Duration upgradeBudget;
-        try {
-            upgradeBudget = Duration.parse(upgradeBudgetField.asString());
-        } catch (Exception e) {
-            throw new IllegalArgumentException("Invalid duration '" + upgradeBudgetField.asString() + "'", e);
+        CloudName cloud = parseStringField("cloud", root, CloudName::from);
+        if (requireField("version", root).type() == Type.NIX) {
+            controller.cancelOsUpgradeIn(cloud);
+            return new MessageResponse("Cleared target OS version for cloud '" + cloud.value() + "'");
         }
+        Version target = parseStringField("version", root, Version::fromString);
+        Duration upgradeBudget = parseStringField("upgradeBudget", root, Duration::parse);
+        boolean force = root.field("force").asBool();
         controller.upgradeOsIn(cloud, target, upgradeBudget, force);
-        Slime response = new Slime();
-        Cursor cursor = response.setObject();
-        cursor.setString("message", "Set target OS version for cloud '" + cloud.value() + "' to " +
-                                    target.toFullString() + " with upgrade budget " + upgradeBudget);
-        return response;
+        return new MessageResponse("Set target OS version for cloud '" + cloud.value() + "' to " +
+                                   target.toFullString() + " with upgrade budget " + upgradeBudget);
     }
 
     private Slime osVersions() {
@@ -196,4 +182,19 @@ public class OsApiHandler extends AuditLoggingRequestHandler {
         }
     }
 
+    private static <T> T parseStringField(String name, Inspector root, Function<String, T> parser) {
+        String fieldValue = requireField(name, root).asString();
+        try {
+            return parser.apply(fieldValue);
+        } catch (Exception e) {
+            throw new IllegalArgumentException("Invalid " + name + " '" + fieldValue + "'", e);
+        }
+    }
+
+    private static Inspector requireField(String name, Inspector root) {
+        Inspector field = root.field(name);
+        if (!field.valid()) throw new IllegalArgumentException("Field '" + name + "' is required");
+        return field;
+    }
+
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java
index 5c210616cb1..15e7a804143 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java
@@ -95,14 +95,18 @@ public class OsApiTest extends ControllerContainerTest {
         assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"7.5.1\", \"cloud\": \"cloud1\", \"force\": true, \"upgradeBudget\": \"PT0S\"}", Request.Method.PATCH),
                        "{\"message\":\"Set target OS version for cloud 'cloud1' to 7.5.1 with upgrade budget PT0S\"}", 200);
 
+        // Clear target for a given cloud
+        assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": null, \"cloud\": \"cloud2\"}", Request.Method.PATCH),
+                       "{\"message\":\"Cleared target OS version for cloud 'cloud2'\"}", 200);
+
         // Error: Missing fields
         assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"7.6\"}", Request.Method.PATCH),
-                       "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Fields 'version', 'cloud' and 'upgradeBudget' are required\"}", 400);
+                       "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Field 'cloud' is required\"}", 400);
         assertResponse(new Request("http://localhost:8080/os/v1/", "{\"cloud\": \"cloud1\"}", Request.Method.PATCH),
-                       "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Fields 'version', 'cloud' and 'upgradeBudget' are required\"}", 400);
+                       "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Field 'version' is required\"}", 400);
 
         // Error: Invalid versions
-        assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": null, \"cloud\": \"cloud1\", \"upgradeBudget\": \"PT0S\"}", Request.Method.PATCH),
+        assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"0.0.0\", \"cloud\": \"cloud1\", \"upgradeBudget\": \"PT0S\"}", Request.Method.PATCH),
                        "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Invalid version '0.0.0'\"}", 400);
         assertResponse(new Request("http://localhost:8080/os/v1/", "{\"version\": \"foo\", \"cloud\": \"cloud1\", \"upgradeBudget\": \"PT0S\"}", Request.Method.PATCH),
                        "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Invalid version 'foo': For input string: \\\"foo\\\"\"}", 400);