Create version cache once

7 files changed, 55 insertions, 54 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/CompositeOsUpgrader.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/CompositeOsUpgrader.java
index 05831d2c074..02f1b951c8e 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/CompositeOsUpgrader.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/CompositeOsUpgrader.java
@@ -3,10 +3,8 @@ package com.yahoo.vespa.hosted.provision.os;
 
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
-import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
 
 import java.util.List;
-import java.util.Optional;
 
 /**
  * An implementation of {@link OsUpgrader} that delegates calls to multiple implementations.
@@ -17,8 +15,8 @@ public class CompositeOsUpgrader extends OsUpgrader {
 
     private final List<OsUpgrader> upgraders;
 
-    public CompositeOsUpgrader(NodeRepository nodeRepository, Optional<HostProvisioner> hostProvisioner, List<OsUpgrader> upgraders) {
-        super(nodeRepository, hostProvisioner);
+    public CompositeOsUpgrader(NodeRepository nodeRepository, List<OsUpgrader> upgraders) {
+        super(nodeRepository);
         this.upgraders = List.copyOf(upgraders);
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/DelegatingOsUpgrader.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/DelegatingOsUpgrader.java
index 7353b70923c..5d8296d6f9d 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/DelegatingOsUpgrader.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/DelegatingOsUpgrader.java
@@ -6,7 +6,6 @@ import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeList;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.filter.NodeListFilter;
-import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
 
 import java.time.Instant;
 import java.util.Optional;
@@ -24,8 +23,8 @@ public class DelegatingOsUpgrader extends OsUpgrader {
 
     private static final Logger LOG = Logger.getLogger(DelegatingOsUpgrader.class.getName());
 
-    public DelegatingOsUpgrader(NodeRepository nodeRepository, Optional<HostProvisioner> hostProvisioner) {
-        super(nodeRepository, hostProvisioner);
+    public DelegatingOsUpgrader(NodeRepository nodeRepository) {
+        super(nodeRepository);
     }
 
     @Override
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsUpgrader.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsUpgrader.java
index 85a46591aa3..9e931463999 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsUpgrader.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsUpgrader.java
@@ -1,31 +1,24 @@
 // Copyright Vespa.ai. 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.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
 import com.yahoo.component.Version;
-import com.yahoo.config.provision.CloudAccount;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.flags.IntFlag;
 import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeList;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
-import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
-import com.yahoo.yolean.Exceptions;
 
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.Set;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
- * Interface for an OS upgrader.
+ * Interface for an OS upgrader. Instances of this are created on-demand because multiple implementations may be used
+ * within a single zone. This and subclasses should not have any state.
  *
  * @author mpolden
  */
@@ -34,20 +27,12 @@ public abstract class OsUpgrader {
     private final Logger LOG = Logger.getLogger(OsUpgrader.class.getName());
 
     private final IntFlag maxActiveUpgrades;
-    private final Optional<HostProvisioner> hostProvisioner;
-    // Supported versions is queried for each host to upgrade, so we cache the results for a while to avoid excessive
-    // API calls to the host provisioner
-    private final Cache<CloudAccount, Set<Version>> supportedVersions = CacheBuilder.newBuilder()
-                                                                                    .expireAfterWrite(10, TimeUnit.MINUTES)
-                                                                                    .build();
 
     final NodeRepository nodeRepository;
 
-
-    public OsUpgrader(NodeRepository nodeRepository, Optional<HostProvisioner> hostProvisioner) {
+    public OsUpgrader(NodeRepository nodeRepository) {
         this.nodeRepository = Objects.requireNonNull(nodeRepository);
         this.maxActiveUpgrades = PermanentFlags.MAX_OS_UPGRADES.bindTo(nodeRepository.flagSource());
-        this.hostProvisioner = Objects.requireNonNull(hostProvisioner);
     }
 
     /** Trigger upgrade to given target */
@@ -68,7 +53,7 @@ public abstract class OsUpgrader {
 
     /** Returns whether node can upgrade to version at given instant */
     final boolean canUpgradeTo(Version version, Instant instant, Node node) {
-        Set<Version> versions = supportedVersions(node, version);
+        Set<Version> versions = nodeRepository.osVersions().availableTo(node, version);
         boolean versionAvailable = versions.contains(version);
         if (!versionAvailable) {
             LOG.log(Level.WARNING, "Want to upgrade host " + node.hostname() + " to OS version " +
@@ -80,19 +65,6 @@ public abstract class OsUpgrader {
                 node.history().age(instant).compareTo(gracePeriod()) > 0);
     }
 
-    private Set<Version> supportedVersions(Node host, Version requestedVersion) {
-        if (hostProvisioner.isEmpty()) {
-            return Set.of(requestedVersion);
-        }
-        try {
-            return supportedVersions.get(host.cloudAccount(),
-                                         () -> hostProvisioner.get().osVersions(host, requestedVersion.getMajor()));
-        } catch (ExecutionException e) {
-            LOG.log(Level.WARNING, "Failed to list supported OS versions in " + host.cloudAccount() + ": " + Exceptions.toMessageString(e));
-            return Set.of();
-        }
-    }
-
     /** The duration this leaves new nodes alone before scheduling any upgrade */
     private Duration gracePeriod() {
         return nodeRepository.zone().system().isCd() ? Duration.ofHours(4) : Duration.ofDays(1);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java
index dc7e51caf4e..0bc074cba23 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java
@@ -1,8 +1,11 @@
 // Copyright Vespa.ai. 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.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.Cloud;
+import com.yahoo.config.provision.CloudAccount;
 import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.curator.Lock;
@@ -11,11 +14,16 @@ import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.Status;
 import com.yahoo.vespa.hosted.provision.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
+import com.yahoo.yolean.Exceptions;
 
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
 import java.util.function.UnaryOperator;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
@@ -31,12 +39,17 @@ import java.util.logging.Logger;
  */
 public class OsVersions {
 
-    private static final Logger log = Logger.getLogger(OsVersions.class.getName());
+    private static final Logger LOG = Logger.getLogger(OsVersions.class.getName());
 
     private final NodeRepository nodeRepository;
     private final CuratorDb db;
     private final Cloud cloud;
     private final Optional<HostProvisioner> hostProvisioner;
+    // Version is queried for each host to upgrade, so we cache the results for a while to avoid excessive
+    // API calls to the host provisioner
+    private final Cache<CloudAccount, Set<Version>> availableVersions = CacheBuilder.newBuilder()
+                                                                                    .expireAfterWrite(10, TimeUnit.MINUTES)
+                                                                                    .build();
 
     public OsVersions(NodeRepository nodeRepository, Optional<HostProvisioner> hostProvisioner) {
         this(nodeRepository, nodeRepository.zone().cloud(), hostProvisioner);
@@ -107,11 +120,30 @@ public class OsVersions {
                                                    + currentTarget.get().version().toFullString());
             }
 
-            log.info("Set OS target version for " + nodeType + " nodes to " + version.toFullString());
+            LOG.info("Set OS target version for " + nodeType + " nodes to " + version.toFullString());
             return change.withTarget(version, nodeType);
         });
     }
 
+    /** Returns the versions available to given host */
+    public Set<Version> availableTo(Node host, Version requestedVersion) {
+        if (hostProvisioner.isEmpty()) {
+            return Set.of(requestedVersion);
+        }
+        try {
+            return availableVersions.get(host.cloudAccount(),
+                                         () -> hostProvisioner.get().osVersions(host, requestedVersion.getMajor()));
+        } catch (ExecutionException e) {
+            LOG.log(Level.WARNING, "Failed to list supported OS versions in " + host.cloudAccount() + ": " + Exceptions.toMessageString(e));
+            return Set.of();
+        }
+    }
+
+    /** Invalidate cached versions. For testing purposes */
+    void invalidate() {
+        availableVersions.invalidateAll();
+    }
+
     /** Resume or halt upgrade of given node type */
     public void resumeUpgradeOf(NodeType nodeType, boolean resume) {
         require(nodeType);
@@ -140,13 +172,12 @@ public class OsVersions {
     private OsUpgrader chooseUpgrader(NodeType nodeType, Optional<Version> target) {
         if (cloud.dynamicProvisioning()) {
             boolean canSoftRebuild = cloud.name().equals(CloudName.AWS);
-            RetiringOsUpgrader retiringOsUpgrader = new RetiringOsUpgrader(nodeRepository, hostProvisioner, canSoftRebuild);
+            RetiringOsUpgrader retiringOsUpgrader = new RetiringOsUpgrader(nodeRepository, canSoftRebuild);
             if (canSoftRebuild) {
                 // If soft rebuild is enabled, we can use RebuildingOsUpgrader for hosts with remote storage.
                 // RetiringOsUpgrader is then only used for hosts with local storage.
                 return new CompositeOsUpgrader(nodeRepository,
-                                               hostProvisioner,
-                                               List.of(new RebuildingOsUpgrader(nodeRepository, hostProvisioner, canSoftRebuild),
+                                               List.of(new RebuildingOsUpgrader(nodeRepository, canSoftRebuild),
                                                        retiringOsUpgrader));
             }
             return retiringOsUpgrader;
@@ -159,9 +190,9 @@ public class OsVersions {
                                                 .anyMatch(osVersion -> osVersion.current().isPresent() &&
                                                                        osVersion.current().get().getMajor() < target.get().getMajor());
         if (rebuildRequired) {
-            return new RebuildingOsUpgrader(nodeRepository, hostProvisioner, false);
+            return new RebuildingOsUpgrader(nodeRepository, false);
         }
-        return new DelegatingOsUpgrader(nodeRepository, hostProvisioner);
+        return new DelegatingOsUpgrader(nodeRepository);
     }
 
     private static void requireNonEmpty(Version version) {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RebuildingOsUpgrader.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RebuildingOsUpgrader.java
index 31e6a4e6e26..a5565a6accb 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RebuildingOsUpgrader.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RebuildingOsUpgrader.java
@@ -9,7 +9,6 @@ import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.ClusterId;
 import com.yahoo.vespa.hosted.provision.node.filter.NodeListFilter;
-import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
 
 import java.time.Instant;
 import java.util.ArrayList;
@@ -36,8 +35,8 @@ public class RebuildingOsUpgrader extends OsUpgrader {
 
     private final boolean softRebuild;
 
-    public RebuildingOsUpgrader(NodeRepository nodeRepository, Optional<HostProvisioner> hostProvisioner, boolean softRebuild) {
-        super(nodeRepository, hostProvisioner);
+    public RebuildingOsUpgrader(NodeRepository nodeRepository, boolean softRebuild) {
+        super(nodeRepository);
         this.softRebuild = softRebuild;
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringOsUpgrader.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringOsUpgrader.java
index 653ff2a61c1..cb6c7683f23 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringOsUpgrader.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringOsUpgrader.java
@@ -8,7 +8,6 @@ import com.yahoo.vespa.hosted.provision.NodeList;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.filter.NodeListFilter;
-import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner;
 
 import java.time.Instant;
 import java.util.Optional;
@@ -27,8 +26,8 @@ public class RetiringOsUpgrader extends OsUpgrader {
 
     private final boolean softRebuild;
 
-    public RetiringOsUpgrader(NodeRepository nodeRepository, Optional<HostProvisioner> hostProvisioner, boolean softRebuild) {
-        super(nodeRepository, hostProvisioner);
+    public RetiringOsUpgrader(NodeRepository nodeRepository, boolean softRebuild) {
+        super(nodeRepository);
         this.softRebuild = softRebuild;
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
index b7014fbd280..dcbac44a37f 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
@@ -521,7 +521,7 @@ public class OsVersionsTest {
     public void skips_unavailable_version() {
         MockHostProvisioner hostProvisioner = new MockHostProvisioner(List.of());
         ProvisioningTester tester = new ProvisioningTester.Builder().dynamicProvisioning(true, false).hostProvisioner(hostProvisioner).build();
-        OsVersions versions = new OsVersions(tester.nodeRepository(), Cloud.builder().dynamicProvisioning(true).build(), Optional.of(hostProvisioner));
+        OsVersions versions = tester.nodeRepository().osVersions();
         tester.makeReadyHosts(1, new NodeResources(2,4,8,100));
         tester.activateTenantHosts();
         Supplier<Node> host = () -> tester.nodeRepository().nodes().list().nodeType(NodeType.host).first().get();
@@ -533,9 +533,12 @@ public class OsVersionsTest {
         versions.resumeUpgradeOf(NodeType.host, true);
         assertTrue("Upgrade is not triggered to unavailable version", host.get().status().osVersion().wanted().isEmpty());
 
-        // Version becomes available
+        // Version becomes available, but is not used until cache expires
         hostProvisioner.addOsVersion(version0);
         versions.resumeUpgradeOf(NodeType.host, true);
+        assertTrue(host.get().status().osVersion().wanted().isEmpty());
+        versions.invalidate();
+        versions.resumeUpgradeOf(NodeType.host, true);
         assertEquals("Host upgrade is triggered", version0, host.get().status().osVersion().wanted().get());
     }