Allow unsetting target version through REST API

2 files changed, 30 insertions, 20 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureVersions.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureVersions.java
index 7044102de7a..ca2a5bb271c 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureVersions.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/InfrastructureVersions.java
@@ -37,28 +37,29 @@ public class InfrastructureVersions {
 
     public void setTargetVersion(NodeType nodeType, Version newTargetVersion, boolean force) {
         assertLegalNodeTypeForTargetVersion(nodeType);
-        if (newTargetVersion.isEmpty()) {
-            throw new IllegalArgumentException("Invalid target version: " + newTargetVersion.toFullString());
-        }
 
         try (Lock lock = db.lockInfrastructureVersions()) {
             Map<NodeType, Version> infrastructureVersions = db.readInfrastructureVersions();
-            Optional<Version> currentTargetVersion = Optional.ofNullable(infrastructureVersions.get(nodeType));
+            Version currentTargetVersion = Optional.ofNullable(infrastructureVersions.get(nodeType))
+                    .orElse(Version.emptyVersion);
 
             // Trying to set the version to the current version, skip
-            if (currentTargetVersion.equals(Optional.of(newTargetVersion))) return;
+            if (currentTargetVersion.equals(newTargetVersion)) return;
 
             // If we don't force the set, we must set the new version to higher than the already set version
-            if (!force && currentTargetVersion.isPresent()) {
-                if (currentTargetVersion.get().isAfter(newTargetVersion))
-                    throw new IllegalArgumentException(String.format("Cannot downgrade version without setting 'force'. " +
-                            "Current target version: %s, attempted to set target version: %s",
-                            currentTargetVersion.get().toFullString(), newTargetVersion.toFullString()));
+            if (!force && currentTargetVersion.isAfter(newTargetVersion)) {
+                throw new IllegalArgumentException(String.format("Cannot downgrade version without setting 'force'. Current target version: %s, attempted to set target version: %s",
+                        currentTargetVersion.toFullString(), newTargetVersion.toFullString()));
             }
 
-            infrastructureVersions.put(nodeType, newTargetVersion);
+            if (newTargetVersion.isEmpty()) {
+                infrastructureVersions.remove(nodeType);
+                logger.info("Removing target version for " + nodeType);
+            } else {
+                infrastructureVersions.put(nodeType, newTargetVersion);
+                logger.info("Setting target version for " + nodeType + " to " + newTargetVersion.toFullString());
+            }
             db.writeInfrastructureVersions(infrastructureVersions);
-            logger.info("Set target version for " + nodeType + " to " + newTargetVersion.toFullString());
         }
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
index f856ff1b47f..6524292f48c 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/RestApiTest.java
@@ -646,13 +646,6 @@ public class RestApiTest {
         assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/"),
                 "{\"versions\":{\"config\":\"6.123.456\",\"confighost\":\"6.123.456\",\"controller\":\"6.123.456\"},\"osVersions\":{},\"dockerImages\":{}}");
 
-        // Setting empty version fails
-        assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/confighost",
-                                   Utf8.toBytes("{\"version\": null}"),
-                                   Request.Method.PATCH),
-                       400,
-                       "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Invalid target version: 0.0.0\"}");
-
         // Setting version for unsupported node type fails
         assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/tenant",
                                    Utf8.toBytes("{\"version\": \"6.123.456\"}"),
@@ -685,6 +678,22 @@ public class RestApiTest {
         assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/"),
                 "{\"versions\":{\"config\":\"6.123.456\",\"confighost\":\"6.123.1\",\"controller\":\"6.123.456\"},\"osVersions\":{},\"dockerImages\":{}}");
 
+        // Setting empty version without force fails
+        assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/confighost",
+                        Utf8.toBytes("{\"version\": null}"),
+                        Request.Method.PATCH),
+                400,
+                "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot downgrade version without setting 'force'. Current target version: 6.123.1, attempted to set target version: 0.0.0\"}");
+
+        assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/confighost",
+                        Utf8.toBytes("{\"version\": null, \"force\": true}"),
+                        Request.Method.PATCH),
+                "{\"message\":\"Set version to 0.0.0 for nodes of type confighost\"}");
+
+        // Verify version has been removed
+        assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/"),
+                "{\"versions\":{\"config\":\"6.123.456\",\"controller\":\"6.123.456\"},\"osVersions\":{},\"dockerImages\":{}}");
+
         // Upgrade OS for confighost and host
         assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/confighost",
                                    Utf8.toBytes("{\"osVersion\": \"7.5.2\"}"),
@@ -697,7 +706,7 @@ public class RestApiTest {
 
         // OS versions are set
         assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/"),
-                       "{\"versions\":{\"config\":\"6.123.456\",\"confighost\":\"6.123.1\",\"controller\":\"6.123.456\"},\"osVersions\":{\"host\":\"7.5.2\",\"confighost\":\"7.5.2\"},\"dockerImages\":{}}");
+                       "{\"versions\":{\"config\":\"6.123.456\",\"controller\":\"6.123.456\"},\"osVersions\":{\"host\":\"7.5.2\",\"confighost\":\"7.5.2\"},\"dockerImages\":{}}");
 
         // Upgrade OS and Vespa together
         assertResponse(new Request("http://localhost:8080/nodes/v2/upgrade/confighost",