Trigger host restart when encrypting

6 files changed, 58 insertions, 20 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypter.java
index 50f77a7df7b..79fc72bfa36 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypter.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypter.java
@@ -11,6 +11,7 @@ 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.ClusterId;
+import com.yahoo.vespa.hosted.provision.node.filter.NodeListFilter;
 
 import java.time.Duration;
 import java.time.Instant;
@@ -49,6 +50,7 @@ public class HostEncrypter extends NodeRepositoryMaintainer {
         for (var nodeType : NodeType.values()) {
             if (!nodeType.isHost()) continue;
             unencryptedHosts(allNodes, nodeType).forEach(host -> encrypt(host, now));
+            triggerRestart(allNodes, nodeType);
         }
         return true;
     }
@@ -95,6 +97,12 @@ public class HostEncrypter extends NodeRepositoryMaintainer {
         return Math.max(0, limit - hosts.encrypting().size());
     }
 
+    /** Trigger restart of encrypting nodes to allow disk encryption to happen */
+    private void triggerRestart(NodeList allNodes, NodeType nodeType) {
+        NodeList hostsReadyToEncrypt = allNodes.nodeType(nodeType).state(Node.State.parked).encrypting();
+        nodeRepository().nodes().restart(NodeListFilter.from(hostsReadyToEncrypt.asList()));
+    }
+
     private void encrypt(Node host, Instant now) {
         LOG.info("Retiring and encrypting " + host);
         nodeRepository().nodes().encrypt(host.hostname(), Agent.HostEncrypter, now);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java
index 59cee0a469e..70fac32f7a6 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java
@@ -305,10 +305,12 @@ public class Nodes {
     }
 
     public Node deallocate(Node node, Agent agent, String reason, NestedTransaction transaction) {
-        if (parkOnDeallocationOf(node, agent))
-            return park(node.hostname(), false, agent, reason, transaction);
-        else
+        if (parkOnDeallocationOf(node, agent)) {
+            boolean keepAllocation = node.reports().getReport(Report.WANT_TO_ENCRYPT_ID).isPresent();
+            return park(node.hostname(), keepAllocation, agent, reason, transaction);
+        } else {
             return db.writeTo(Node.State.dirty, List.of(node), agent, Optional.of(reason), transaction).get(0);
+        }
     }
 
     /**
@@ -367,7 +369,7 @@ public class Nodes {
         return parked;
     }
 
-    public Node park(String hostname, boolean keepAllocation, Agent agent, String reason, NestedTransaction transaction) {
+    private Node park(String hostname, boolean keepAllocation, Agent agent, String reason, NestedTransaction transaction) {
         return move(hostname, Node.State.parked, agent, keepAllocation, Optional.of(reason), transaction);
     }
 
@@ -573,14 +575,22 @@ public class Nodes {
     }
 
     /**
-     * Increases the restart generation of the active nodes matching the filter.
+     * Increases the restart generation of the active nodes matching given filter.
+     *
+     * @return the nodes in their new state
+     */
+    public List<Node> restartActive(Predicate<Node> filter) {
+        return restart(StateFilter.from(Node.State.active).and(filter));
+    }
+
+    /**
+     * Increases the restart generation of the any nodes matching given filter.
      *
      * @return the nodes in their new state
      */
     public List<Node> restart(Predicate<Node> filter) {
-        return performOn(StateFilter.from(Node.State.active).and(filter),
-                         (node, lock) -> write(node.withRestart(node.allocation().get().restartGeneration().withIncreasedWanted()),
-                                               lock));
+        return performOn(filter, (node, lock) -> write(node.withRestart(node.allocation().get().restartGeneration().withIncreasedWanted()),
+                                                       lock));
     }
 
     /**
@@ -813,6 +823,7 @@ public class Nodes {
                                                     .orElse(false);
         return node.status().wantToDeprovision() ||
                node.status().wantToRebuild() ||
+               node.reports().getReport(Report.WANT_TO_ENCRYPT_ID).isPresent() ||
                retirementRequestedByOperator;
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Report.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Report.java
index 4eb76828131..f5c5b9f2857 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Report.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Report.java
@@ -28,6 +28,7 @@ public class Report {
     public static final String DESCRIPTION_FIELD = "description";
 
     /** Known report IDs */
+    // TODO(mpolden): Remove together with HostEncrypter
     public static final String WANT_TO_ENCRYPT_ID = "wantToEncrypt";
     public static final String DISK_ENCRYPTED_ID = "diskEncrypted";
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java
index ee8ce23a5c0..2cc7d7e2555 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java
@@ -8,7 +8,6 @@ import com.yahoo.config.provision.ApplicationTransaction;
 import com.yahoo.config.provision.Capacity;
 import com.yahoo.config.provision.ClusterResources;
 import com.yahoo.config.provision.ClusterSpec;
-import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.HostFilter;
 import com.yahoo.config.provision.HostSpec;
 import com.yahoo.config.provision.NodeResources;
@@ -122,7 +121,7 @@ public class NodeRepositoryProvisioner implements Provisioner {
 
     @Override
     public void restart(ApplicationId application, HostFilter filter) {
-        nodeRepository.nodes().restart(ApplicationFilter.from(application).and(NodeHostFilter.from(filter)));
+        nodeRepository.nodes().restartActive(ApplicationFilter.from(application).and(NodeHostFilter.from(filter)));
     }
 
     @Override
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java
index c7bd12204a6..24e297de179 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java
@@ -195,7 +195,7 @@ public class NodesV2ApiHandler extends LoggingRequestHandler {
     private HttpResponse handlePOST(HttpRequest request) {
         Path path = new Path(request.getUri());
         if (path.matches("/nodes/v2/command/restart")) {
-            int restartCount = nodeRepository.nodes().restart(toNodeFilter(request)).size();
+            int restartCount = nodeRepository.nodes().restartActive(toNodeFilter(request)).size();
             return new MessageResponse("Scheduled restart of " + restartCount + " matching nodes");
         }
         if (path.matches("/nodes/v2/command/reboot")) {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypterTest.java
index b3c78aa4627..be0a7a84025 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypterTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostEncrypterTest.java
@@ -22,22 +22,26 @@ import java.time.Instant;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
+import java.util.function.Consumer;
 import java.util.function.Supplier;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
 
 /**
  * @author mpolden
  */
 public class HostEncrypterTest {
 
+    private final ApplicationId infraApplication = ApplicationId.from("hosted-vespa", "infra", "default");
     private final ProvisioningTester tester = new ProvisioningTester.Builder().build();
+    private final HostEncrypter encrypter = new HostEncrypter(tester.nodeRepository(), Duration.ofDays(1), new MockMetric());
 
     @Test
     public void no_hosts_encrypted_with_default_flag_value() {
         provisionHosts(1);
-        HostEncrypter encrypter = new HostEncrypter(tester.nodeRepository(), Duration.ofDays(1), new MockMetric());
         encrypter.maintain();
         assertEquals(0, tester.nodeRepository().nodes().list().encrypting().size());
     }
@@ -46,7 +50,6 @@ public class HostEncrypterTest {
     public void encrypt_hosts() {
         tester.flagSource().withIntFlag(Flags.MAX_ENCRYPTING_HOSTS.id(), 3);
         Supplier<NodeList> hosts = () -> tester.nodeRepository().nodes().list().nodeType(NodeType.host);
-        HostEncrypter encrypter = new HostEncrypter(tester.nodeRepository(), Duration.ofDays(1), new MockMetric());
 
         // Provision hosts and deploy applications
         int hostCount = 5;
@@ -116,19 +119,31 @@ public class HostEncrypterTest {
         List<Node> provisionedHosts = tester.makeReadyNodes(hostCount, new NodeResources(48, 128, 2000, 10), NodeType.host, 10);
         // Set OS version supporting encryption
         tester.patchNodes(provisionedHosts, (host) -> host.with(host.status().withOsVersion(host.status().osVersion().withCurrent(Optional.of(Version.fromString("8.0"))))));
-        tester.activateTenantHosts();
+        tester.prepareAndActivateInfraApplication(infraApplication, NodeType.host);
+    }
+
+    private void parkRetiredHosts() {
+        // Redeploy to park retired hosts
+        replaceNodes(infraApplication, (application) -> tester.prepareAndActivateInfraApplication(application, NodeType.host));
+        // Trigger restart of parked nodes
+        encrypter.maintain();
     }
 
     private void completeEncryptionOf(List<Node> nodes) {
         Instant now = tester.clock().instant();
-        tester.patchNodes(nodes, (node) -> {
-            if (node.reports().getReport(Report.WANT_TO_ENCRYPT_ID).isEmpty()) throw new IllegalArgumentException(node + " is not requested to encrypt");
+        parkRetiredHosts();
+        List<Node> patchedNodes = tester.patchNodes(nodes, (node) -> {
+            assertSame(Node.State.parked, node.state());
+            assertTrue(node + " has restart pending", node.allocation().get().restartGeneration().pending());
+            assertTrue(node + " wants to encrypt", node.reports().getReport(Report.WANT_TO_ENCRYPT_ID).isPresent());
             return node.with(node.reports().withReport(Report.basicReport(Report.DISK_ENCRYPTED_ID,
                                                                           Report.Type.UNSPECIFIED,
                                                                           now,
-                                                                          "Host is encrypted")))
-                       .withWantToRetire(false, Agent.system, now);
+                                                                          "Host is encrypted")));
         });
+        patchedNodes = tester.nodeRepository().nodes().deallocate(patchedNodes, Agent.system, getClass().getSimpleName());
+        tester.nodeRepository().nodes().setReady(patchedNodes, Agent.system, getClass().getSimpleName());
+        tester.activateTenantHosts();
     }
 
     private void deployApplication(ApplicationId application) {
@@ -138,14 +153,18 @@ public class HostEncrypterTest {
     }
 
     private void replaceNodes(ApplicationId application) {
+        replaceNodes(application, this::deployApplication);
+    }
+
+    private void replaceNodes(ApplicationId application, Consumer<ApplicationId> deployer) {
         // Deploy to retire nodes
-        deployApplication(application);
+        deployer.accept(application);
         List<Node> retired = tester.nodeRepository().nodes().list().owner(application).retired().asList();
         assertFalse("At least one node is retired", retired.isEmpty());
         tester.nodeRepository().nodes().setRemovable(application, retired);
 
         // Redeploy to deactivate removable nodes and allocate new ones
-        deployApplication(application);
+        deployer.accept(application);
         tester.nodeRepository().nodes().list(Node.State.inactive).owner(application)
               .forEach(node -> tester.nodeRepository().nodes().removeRecursively(node, true));
     }