Merge pull request #10669 from vespa-engine/mpolden/os-version-status

Indicate whether an OS version is active

9 files changed, 197 insertions, 24 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
index fe71f832c69..3895e70376a 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java
@@ -32,7 +32,7 @@ import com.yahoo.vespa.hosted.provision.persistence.DnsNameResolver;
 import com.yahoo.vespa.hosted.provision.persistence.NameResolver;
 import com.yahoo.vespa.hosted.provision.provisioning.DockerImages;
 import com.yahoo.vespa.hosted.provision.provisioning.FirmwareChecks;
-import com.yahoo.vespa.hosted.provision.provisioning.OsVersions;
+import com.yahoo.vespa.hosted.provision.os.OsVersions;
 import com.yahoo.vespa.hosted.provision.restapi.v2.NotFoundException;
 
 import java.time.Clock;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersion.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersion.java
new file mode 100644
index 00000000000..571356b0a34
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersion.java
@@ -0,0 +1,47 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.os;
+
+import com.yahoo.component.Version;
+
+import java.util.Objects;
+
+/**
+ * An OS version and it's active status.
+ *
+ * @author mpolden
+ */
+public class OsVersion {
+
+    private final Version version;
+    private final boolean active;
+
+    public OsVersion(Version version, boolean active) {
+        this.version = version;
+        this.active = active;
+    }
+
+    /** The OS version number */
+    public Version version() {
+        return version;
+    }
+
+    /** Returns whether this is currently active and should be acted on by nodes */
+    public boolean active() {
+        return active;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        OsVersion osVersion = (OsVersion) o;
+        return active == osVersion.active &&
+               version.equals(osVersion.version);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(version, active);
+    }
+
+}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/OsVersions.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java
index 03da60bd31c..bc738400c45 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/OsVersions.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java
@@ -1,5 +1,5 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.provision.provisioning;
+package com.yahoo.vespa.hosted.provision.os;
 
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
@@ -35,7 +35,7 @@ public class OsVersions {
      * unnecessary ZK reads. When targets change, some nodes may need to wait for TTL until they see the new target,
      * this is fine.
      */
-    private volatile Supplier<Map<NodeType, Version>> currentTargets;
+    private volatile Supplier<Map<NodeType, OsVersion>> currentTargets;
 
     public OsVersions(CuratorDatabaseClient db) {
         this(db, defaultCacheTtl);
@@ -53,12 +53,12 @@ public class OsVersions {
     }
 
     /** Returns the current target versions for each node type */
-    public Map<NodeType, Version> targets() {
+    public Map<NodeType, OsVersion> targets() {
         return currentTargets.get();
     }
 
     /** Returns the current target version for given node type, if any */
-    public Optional<Version> targetFor(NodeType type) {
+    public Optional<OsVersion> targetFor(NodeType type) {
         return Optional.ofNullable(targets().get(type));
     }
 
@@ -66,7 +66,7 @@ public class OsVersions {
      * node object */
     public void removeTarget(NodeType nodeType) {
         try (Lock lock = db.lockOsVersions()) {
-            Map<NodeType, Version> osVersions = db.readOsVersions();
+            Map<NodeType, OsVersion> osVersions = db.readOsVersions();
             osVersions.remove(nodeType);
             db.writeOsVersions(osVersions);
             createCache(); // Throw away current cache
@@ -83,20 +83,20 @@ public class OsVersions {
             throw  new IllegalArgumentException("Invalid target version: " + newTarget.toFullString());
         }
         try (Lock lock = db.lockOsVersions()) {
-            Map<NodeType, Version> osVersions = db.readOsVersions();
-            Optional<Version> oldTarget = Optional.ofNullable(osVersions.get(nodeType));
+            Map<NodeType, OsVersion> osVersions = db.readOsVersions();
+            Optional<OsVersion> oldTarget = Optional.ofNullable(osVersions.get(nodeType));
 
-            if (oldTarget.filter(v -> v.equals(newTarget)).isPresent()) {
+            if (oldTarget.filter(v -> v.version().equals(newTarget)).isPresent()) {
                 return; // Old target matches new target, nothing to do
             }
 
-            if (!force && oldTarget.filter(v -> v.isAfter(newTarget)).isPresent()) {
+            if (!force && oldTarget.filter(v -> v.version().isAfter(newTarget)).isPresent()) {
                 throw new IllegalArgumentException("Cannot set target OS version to " + newTarget +
                                                    " without setting 'force', as it's lower than the current version: "
-                                                   + oldTarget.get());
+                                                   + oldTarget.get().version());
             }
 
-            osVersions.put(nodeType, newTarget);
+            osVersions.put(nodeType, new OsVersion(newTarget, true));
             db.writeOsVersions(osVersions);
             createCache(); // Throw away current cache
             log.info("Set OS target version for " + nodeType + " nodes to " + newTarget.toFullString());
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
index 2507dbc8d3c..fae314bc50f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java
@@ -24,6 +24,7 @@ import com.yahoo.vespa.hosted.provision.lb.LoadBalancer;
 import com.yahoo.vespa.hosted.provision.lb.LoadBalancerId;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.Status;
+import com.yahoo.vespa.hosted.provision.os.OsVersion;
 
 import java.time.Clock;
 import java.time.Duration;
@@ -417,15 +418,15 @@ public class CuratorDatabaseClient {
 
 
     // OS versions
-    public Map<NodeType, Version> readOsVersions() {
-        return read(osVersionsPath(), NodeTypeVersionsSerializer::fromJson).orElseGet(TreeMap::new);
+    public Map<NodeType, OsVersion> readOsVersions() {
+        return read(osVersionsPath(), OsVersionsSerializer::fromJson).orElseGet(TreeMap::new);
     }
 
-    public void writeOsVersions(Map<NodeType, Version> versions) {
+    public void writeOsVersions(Map<NodeType, OsVersion> versions) {
         NestedTransaction transaction = new NestedTransaction();
         CuratorTransaction curatorTransaction = curatorDatabase.newCuratorTransactionIn(transaction);
         curatorTransaction.add(CuratorOperations.setData(osVersionsPath().getAbsolute(),
-                                                         NodeTypeVersionsSerializer.toJson(versions)));
+                                                         OsVersionsSerializer.toJson(versions)));
         transaction.commit();
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializer.java
new file mode 100644
index 00000000000..4104a31886a
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializer.java
@@ -0,0 +1,61 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.persistence;
+
+import com.yahoo.component.Version;
+import com.yahoo.config.provision.NodeType;
+import com.yahoo.slime.ObjectTraverser;
+import com.yahoo.slime.Slime;
+import com.yahoo.slime.Type;
+import com.yahoo.vespa.config.SlimeUtils;
+import com.yahoo.vespa.hosted.provision.os.OsVersion;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.Map;
+import java.util.TreeMap;
+
+/**
+ * Serializer for a map of {@link NodeType} and {@link OsVersion}.
+ *
+ * @author mpolden
+ */
+public class OsVersionsSerializer {
+
+    private static final String VERSION_FIELD = "version";
+    private static final String ACTIVE_FIELD = "active";
+
+    private OsVersionsSerializer() {}
+
+    public static byte[] toJson(Map<NodeType, OsVersion> versions) {
+        var slime = new Slime();
+        var object = slime.setObject();
+        // TODO(mpolden): Write active status here once all readers can handle it
+        versions.forEach((nodeType, osVersion) -> object.setString(NodeSerializer.toString(nodeType),
+                                                                   osVersion.version().toFullString()));
+        try {
+            return SlimeUtils.toJsonBytes(slime);
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+    }
+
+    public static Map<NodeType, OsVersion> fromJson(byte[] data) {
+        var versions = new TreeMap<NodeType, OsVersion>(); // Use TreeMap to sort by node type
+        var inspector = SlimeUtils.jsonToSlime(data).get();
+        inspector.traverse((ObjectTraverser) (key, value) -> {
+            Version version;
+            boolean active;
+            // TODO(mpolden): Remove fallback after next version
+            if (value.type() == Type.OBJECT) {
+                version = Version.fromString(value.field(VERSION_FIELD).asString());
+                active = value.field(ACTIVE_FIELD).asBool();
+            } else {
+                version = Version.fromString(value.asString());
+                active = true;
+            }
+            versions.put(NodeSerializer.nodeTypeFromString(key), new OsVersion(version, active));
+        });
+        return versions;
+    }
+
+}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
index 1b4e23fc4bf..e1e21268ac4 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java
@@ -18,6 +18,7 @@ import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.History;
 import com.yahoo.vespa.hosted.provision.node.filter.NodeFilter;
+import com.yahoo.vespa.hosted.provision.os.OsVersion;
 import com.yahoo.vespa.orchestrator.Orchestrator;
 import com.yahoo.vespa.orchestrator.status.HostStatus;
 
@@ -169,7 +170,10 @@ class NodesResponse extends HttpResponse {
         object.setLong("rebootGeneration", node.status().reboot().wanted());
         object.setLong("currentRebootGeneration", node.status().reboot().current());
         node.status().osVersion().ifPresent(version -> object.setString("currentOsVersion", version.toFullString()));
-        nodeRepository.osVersions().targetFor(node.type()).ifPresent(version -> object.setString("wantedOsVersion", version.toFullString()));
+        nodeRepository.osVersions().targetFor(node.type())
+                      .filter(OsVersion::active) // Only include wantedOsVersion when active. When active is false, OS upgrades are paused.
+                      .map(OsVersion::version)
+                      .ifPresent(version -> object.setString("wantedOsVersion", version.toFullString()));
         node.status().firmwareVerifiedAt().ifPresent(instant -> object.setLong("currentFirmwareCheck", instant.toEpochMilli()));
         if (node.type().isDockerHost())
             nodeRepository.firmwareChecks().requiredAfter().ifPresent(after -> object.setLong("wantedFirmwareCheck", after.toEpochMilli()));
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/UpgradeResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/UpgradeResponse.java
index 87d7944f040..ae61bedd67f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/UpgradeResponse.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/UpgradeResponse.java
@@ -7,7 +7,7 @@ import com.yahoo.slime.JsonFormat;
 import com.yahoo.slime.Slime;
 import com.yahoo.vespa.hosted.provision.maintenance.InfrastructureVersions;
 import com.yahoo.vespa.hosted.provision.provisioning.DockerImages;
-import com.yahoo.vespa.hosted.provision.provisioning.OsVersions;
+import com.yahoo.vespa.hosted.provision.os.OsVersions;
 
 import java.io.IOException;
 import java.io.OutputStream;
@@ -39,7 +39,7 @@ public class UpgradeResponse extends HttpResponse {
         infrastructureVersions.getTargetVersions().forEach((nodeType, version) -> versionsObject.setString(nodeType.name(), version.toFullString()));
 
         Cursor osVersionsObject = root.setObject("osVersions");
-        osVersions.targets().forEach((nodeType, version) -> osVersionsObject.setString(nodeType.name(), version.toFullString()));
+        osVersions.targets().forEach((nodeType, osVersion) -> osVersionsObject.setString(nodeType.name(), osVersion.version().toFullString()));
 
 
         Cursor dockerImagesObject = root.setObject("dockerImages");
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/OsVersionsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
index 132d0d3c772..06f9dcfae68 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/OsVersionsTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
@@ -1,5 +1,5 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.provision.provisioning;
+package com.yahoo.vespa.hosted.provision.os;
 
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.NodeType;
@@ -34,15 +34,15 @@ public class OsVersionsTest {
         // Upgrade OS
         Version version1 = Version.fromString("7.1");
         versions.setTarget(NodeType.host, version1, false);
-        Map<NodeType, Version> targetVersions = versions.targets();
+        Map<NodeType, OsVersion> targetVersions = versions.targets();
         assertSame("Caches target versions", targetVersions, versions.targets());
-        assertEquals(version1, versions.targetFor(NodeType.host).get());
+        assertEquals(version1, versions.targetFor(NodeType.host).get().version());
 
         // Upgrade OS again
         Version version2 = Version.fromString("7.2");
         versions.setTarget(NodeType.host, version2, false);
         assertNotSame("Cache invalidated", targetVersions, versions.targets());
-        assertEquals(version2, versions.targetFor(NodeType.host).get());
+        assertEquals(version2, versions.targetFor(NodeType.host).get().version());
 
         // Downgrading fails
         try {
@@ -52,7 +52,7 @@ public class OsVersionsTest {
 
         // Forcing downgrade succeeds
         versions.setTarget(NodeType.host, version1, true);
-        assertEquals(version1, versions.targetFor(NodeType.host).get());
+        assertEquals(version1, versions.targetFor(NodeType.host).get().version());
 
         // Target can be removed
         versions.removeTarget(NodeType.host);
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializerTest.java
new file mode 100644
index 00000000000..4aec5b8370e
--- /dev/null
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/OsVersionsSerializerTest.java
@@ -0,0 +1,60 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.persistence;
+
+import com.yahoo.component.Version;
+import com.yahoo.config.provision.NodeType;
+import com.yahoo.vespa.hosted.provision.os.OsVersion;
+import org.junit.Test;
+
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+
+import static org.junit.Assert.*;
+
+/**
+ * @author mpolden
+ */
+public class OsVersionsSerializerTest {
+
+    // TODO(mpolden): Remove once no longer supported
+    @Test
+    public void legacy_format() {
+        var json = "{\"host\":\"1.2.3\",\"proxyhost\":\"4.5.6\",\"confighost\":\"7.8.9\"}";
+        var serializedFromString = OsVersionsSerializer.fromJson(json.getBytes(StandardCharsets.UTF_8));
+        var versions = Map.of(
+                NodeType.host, new OsVersion(Version.fromString("1.2.3"), true),
+                NodeType.proxyhost, new OsVersion(Version.fromString("4.5.6"), true),
+                NodeType.confighost, new OsVersion(Version.fromString("7.8.9"), true)
+        );
+        assertEquals(versions, serializedFromString);
+
+        var serialized = OsVersionsSerializer.fromJson(OsVersionsSerializer.toJson(versions));
+        assertEquals(serialized, versions);
+    }
+
+    @Test
+    public void read_future_format() {
+        var json = "{\n" +
+                   "  \"host\": {\n" +
+                   "    \"version\": \"1.2.3\",\n" +
+                   "    \"active\": false\n" +
+                   "  " +
+                   "},\n" +
+                   "  \"proxyhost\": {\n" +
+                   "    \"version\": \"4.5.6\",\n" +
+                   "    \"active\": true\n" +
+                   "  },\n" +
+                   "  \"confighost\": {\n" +
+                   "    \"version\": \"7.8.9\",\n" +
+                   "    \"active\": true\n" +
+                   "  }\n" +
+                   "}";
+        var versions = OsVersionsSerializer.fromJson(json.getBytes(StandardCharsets.UTF_8));
+        assertEquals(Map.of(
+                NodeType.host, new OsVersion(Version.fromString("1.2.3"), false),
+                NodeType.proxyhost, new OsVersion(Version.fromString("4.5.6"), true),
+                NodeType.confighost, new OsVersion(Version.fromString("7.8.9"), true)
+        ), versions);
+    }
+
+}