Emit restart actions on node resource change

The actions should track what changes will actually cause restarts
such that we can reason about the consequences of restarts based on them.

11 files changed, 218 insertions, 27 deletions

diff --git a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java
index 10649df88e1..2594c64b951 100644
--- a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java
+++ b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java
@@ -140,7 +140,7 @@ public class MockApplicationPackage implements ApplicationPackage {
     }
 
     @Override
-    public List<NamedReader> getFiles(Path dir,String fileSuffix,boolean recurse) {
+    public List<NamedReader> getFiles(Path dir, String fileSuffix, boolean recurse) {
         return new ArrayList<>();
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
index fa72a4965b0..9480690c395 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
@@ -17,6 +17,7 @@ import com.yahoo.vespa.model.application.validation.change.ContentTypeRemovalVal
 import com.yahoo.vespa.model.application.validation.change.GlobalDocumentChangeValidator;
 import com.yahoo.vespa.model.application.validation.change.IndexedSearchClusterChangeValidator;
 import com.yahoo.vespa.model.application.validation.change.IndexingModeChangeValidator;
+import com.yahoo.vespa.model.application.validation.change.NodeResourceChangeValidator;
 import com.yahoo.vespa.model.application.validation.change.ResourcesReductionValidator;
 import com.yahoo.vespa.model.application.validation.change.StartupCommandChangeValidator;
 import com.yahoo.vespa.model.application.validation.change.StreamingSearchClusterChangeValidator;
@@ -91,6 +92,7 @@ public class Validation {
                 new ClusterSizeReductionValidator(),
                 new ResourcesReductionValidator(),
                 new ContainerRestartValidator(),
+                new NodeResourceChangeValidator()
         };
         return Arrays.stream(validators)
                 .flatMap(v -> v.validate(currentModel, nextModel, overrides, now).stream())
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ChangeValidator.java
index bec7fd1518f..b720cc13f42 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ChangeValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ChangeValidator.java
@@ -12,7 +12,6 @@ import java.util.List;
  * Interface for validating changes between a current active and next config model.
  *
  * @author geirst
- * @since 2014-11-18
  */
 public interface ChangeValidator {
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidator.java
index 162f6798462..f223ba69f61 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidator.java
@@ -7,11 +7,8 @@ import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.config.application.api.ValidationId;
 import com.yahoo.config.application.api.ValidationOverrides;
-import com.yahoo.vespa.model.container.ContainerCluster;
-import com.yahoo.vespa.model.content.cluster.ContentCluster;
 
 import java.time.Instant;
-import java.util.Collections;
 import java.util.List;
 
 /**
@@ -33,7 +30,7 @@ public class ClusterSizeReductionValidator implements ChangeValidator {
                      overrides,
                      now);
         }
-        return Collections.emptyList();
+        return List.of();
     }
 
     private void validate(Capacity current,
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentClusterRemovalValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentClusterRemovalValidator.java
index 866f647a351..4a43a30c167 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentClusterRemovalValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentClusterRemovalValidator.java
@@ -8,7 +8,6 @@ import com.yahoo.config.application.api.ValidationOverrides;
 import com.yahoo.vespa.model.content.cluster.ContentCluster;
 
 import java.time.Instant;
-import java.util.Collections;
 import java.util.List;
 
 /**
@@ -30,7 +29,7 @@ public class ContentClusterRemovalValidator implements ChangeValidator {
                                   now);
         }
 
-        return Collections.emptyList();
+        return List.of();
     }
 
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentTypeRemovalValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentTypeRemovalValidator.java
index a691c8bb5c4..f3a018f2cdd 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentTypeRemovalValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentTypeRemovalValidator.java
@@ -9,7 +9,6 @@ import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.content.cluster.ContentCluster;
 
 import java.time.Instant;
-import java.util.Collections;
 import java.util.List;
 
 /**
@@ -36,7 +35,7 @@ public class ContentTypeRemovalValidator implements ChangeValidator {
                 }
             }
         }
-        return Collections.emptyList();
+        return List.of();
     }
 
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidator.java
index 198030d1f44..0b3f865c760 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidator.java
@@ -8,7 +8,6 @@ import com.yahoo.documentmodel.NewDocumentType;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.content.cluster.ContentCluster;
 
-import java.util.Collections;
 import java.time.Instant;
 import java.util.List;
 import java.util.Map;
@@ -30,7 +29,7 @@ public class GlobalDocumentChangeValidator implements ChangeValidator {
                 validateContentCluster(currentEntry.getValue(), nextCluster);
             }
         }
-        return Collections.emptyList();
+        return List.of();
     }
 
     private void validateContentCluster(ContentCluster currentCluster, ContentCluster nextCluster) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidator.java
new file mode 100644
index 00000000000..d6731c86607
--- /dev/null
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidator.java
@@ -0,0 +1,75 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.model.application.validation.change;
+
+import com.yahoo.config.application.api.ValidationOverrides;
+import com.yahoo.config.model.api.ConfigChangeAction;
+import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.NodeResources;
+import com.yahoo.vespa.model.VespaModel;
+import com.yahoo.vespa.model.container.ApplicationContainerCluster;
+import com.yahoo.vespa.model.content.cluster.ContentCluster;
+
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+/**
+ * Emits restart change actions for clusters where the node resources are changed.
+ * Nodes will restart on their own on this condition but we want to emit restart actions to
+ * defer applying new config until restart.
+ *
+ * @author bratseth
+ */
+public class NodeResourceChangeValidator implements ChangeValidator {
+
+    @Override
+    public List<ConfigChangeAction> validate(VespaModel current, VespaModel next, ValidationOverrides overrides, Instant now) {
+        var restartActions = new ArrayList<ConfigChangeAction>();
+        for (ClusterSpec.Id clusterId : current.allClusters()) {
+            Optional<NodeResources> currentResources = resourcesOf(clusterId, current);
+            Optional<NodeResources> nextResources = resourcesOf(clusterId, next);
+            if (currentResources.isEmpty() || nextResources.isEmpty()) continue; // new or removed cluster
+            if ( ! currentResources.equals(nextResources))
+                restartActions.addAll(createRestartActionsFor(clusterId, current));
+        }
+        return restartActions;
+    }
+
+    private Optional<NodeResources> resourcesOf(ClusterSpec.Id clusterId, VespaModel model) {
+        return model.allocatedHosts().getHosts().stream().filter(host -> host.membership().isPresent())
+                                                         .filter(host -> host.membership().get().cluster().id().equals(clusterId))
+                                                         .findFirst()
+                                                         .map(host -> host.realResources());
+    }
+
+    private List<ConfigChangeAction> createRestartActionsFor(ClusterSpec.Id clusterId, VespaModel model) {
+        ApplicationContainerCluster containerCluster = model.getContainerClusters().get(clusterId.value());
+        if (containerCluster != null)
+            return createRestartActionsFor(containerCluster);
+
+        ContentCluster contentCluster = model.getContentClusters().get(clusterId.value());
+        if (contentCluster != null)
+            return createRestartActionsFor(contentCluster);
+
+        return List.of();
+    }
+
+    private List<ConfigChangeAction> createRestartActionsFor(ApplicationContainerCluster cluster) {
+        return cluster.getContainers().stream()
+                                      .map(container -> new VespaRestartAction("Node resource change",
+                                                                               container.getServiceInfo(),
+                                                                               false))
+                                     .collect(Collectors.toList());
+    }
+
+    private List<ConfigChangeAction> createRestartActionsFor(ContentCluster cluster) {
+        return cluster.getSearch().getSearchNodes().stream()
+                                                   .map(node -> new VespaRestartAction("Node resource change",
+                                                                                       node.getServiceInfo(),
+                                                                                      false))
+                                                   .collect(Collectors.toList());
+    }
+
+}
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidator.java
index 24b7b0949f6..0fdfcd78323 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidator.java
@@ -1,22 +1,17 @@
 // Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.application.validation.change;
 
-import com.yahoo.collections.Pair;
 import com.yahoo.config.application.api.ValidationId;
 import com.yahoo.config.application.api.ValidationOverrides;
 import com.yahoo.config.model.api.ConfigChangeAction;
 import com.yahoo.config.provision.Capacity;
 import com.yahoo.config.provision.ClusterSpec;
-import com.yahoo.config.provision.NodeResources;
-import com.yahoo.vespa.model.HostResource;
 import com.yahoo.vespa.model.VespaModel;
 
 import java.time.Instant;
 import java.util.List;
 import java.util.Locale;
-import java.util.Map;
 import java.util.Optional;
-import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorageCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorageCluster.java
index 981ce1bc004..bacb22b0b89 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorageCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorageCluster.java
@@ -45,13 +45,12 @@ public class StorageCluster extends AbstractConfigProducer<StorageNode>
         }
     }
 
-    private Integer bucketMoverMaxFillAboveAverage = null;
-    private String clusterName;
-    private FileStorProducer fileStorProducer;
-    private IntegrityCheckerProducer integrityCheckerProducer;
-    private StorServerProducer storServerProducer;
-    private StorVisitorProducer storVisitorProducer;
-    private PersistenceProducer persistenceProducer;
+    private final String clusterName;
+    private final FileStorProducer fileStorProducer;
+    private final IntegrityCheckerProducer integrityCheckerProducer;
+    private final StorServerProducer storServerProducer;
+    private final StorVisitorProducer storVisitorProducer;
+    private final PersistenceProducer persistenceProducer;
 
     StorageCluster(AbstractConfigProducer parent,
                    String clusterName,
@@ -71,9 +70,6 @@ public class StorageCluster extends AbstractConfigProducer<StorageNode>
 
     @Override
     public void getConfig(StorBucketmoverConfig.Builder builder) {
-        if (bucketMoverMaxFillAboveAverage != null) {
-            builder.max_target_fill_rate_above_average(bucketMoverMaxFillAboveAverage);
-        }
     }
 
     @Override
@@ -127,4 +123,5 @@ public class StorageCluster extends AbstractConfigProducer<StorageNode>
     public void getConfig(StorFilestorConfig.Builder builder) {
         fileStorProducer.getConfig(builder);
     }
+
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidatorTest.java
new file mode 100644
index 00000000000..fa4887cd8ee
--- /dev/null
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidatorTest.java
@@ -0,0 +1,129 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.model.application.validation.change;
+
+import com.yahoo.config.application.api.ValidationOverrides;
+import com.yahoo.config.model.api.ConfigChangeAction;
+import com.yahoo.config.model.api.HostProvisioner;
+import com.yahoo.config.model.deploy.DeployState;
+import com.yahoo.config.model.deploy.TestProperties;
+import com.yahoo.config.provision.Capacity;
+import com.yahoo.config.provision.ClusterMembership;
+import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.HostSpec;
+import com.yahoo.config.provision.ProvisionLogger;
+import com.yahoo.vespa.model.VespaModel;
+import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg;
+import org.junit.Test;
+
+import java.time.Clock;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author bratseth
+ */
+public class NodeResourceChangeValidatorTest {
+
+    @Test
+    public void test_restart_action_count() {
+        assertEquals(0, validate(model(1, 1, 1, 1), model(1, 1, 1, 1)).size());
+        assertEquals(1, validate(model(1, 1, 1, 1), model(2, 1, 1, 1)).size());
+        assertEquals(2, validate(model(1, 1, 1, 1), model(1, 2, 1, 1)).size());
+        assertEquals(3, validate(model(1, 1, 1, 1), model(1, 1, 2, 1)).size());
+        assertEquals(4, validate(model(1, 1, 1, 1), model(1, 1, 1, 2)).size());
+        assertEquals(5, validate(model(1, 1, 1, 1), model(2, 1, 1, 2)).size());
+        assertEquals(6, validate(model(1, 1, 1, 1), model(1, 2, 1, 2)).size());
+        assertEquals(7, validate(model(1, 1, 1, 1), model(1, 1, 2, 2)).size());
+        assertEquals(8, validate(model(1, 1, 1, 1), model(2, 1, 2, 2)).size());
+        assertEquals(9, validate(model(1, 1, 1, 1), model(1, 2, 2, 2)).size());
+        assertEquals(10, validate(model(1, 1, 1, 1), model(2, 2, 2, 2)).size());
+    }
+
+    @Test
+    public void test_restart_action_details() {
+        ConfigChangeAction containerAction = validate(model(1, 1, 1, 1), model(2, 1, 1, 1)).get(0);
+        assertEquals(ConfigChangeAction.Type.RESTART, containerAction.getType());
+        assertEquals("service 'container' of type container on host0", containerAction.getServices().get(0).toString());
+        assertEquals(false, containerAction.ignoreForInternalRedeploy());
+
+        ConfigChangeAction contentAction = validate(model(1, 1, 1, 1), model(1, 1, 2, 1)).get(0);
+        assertEquals(ConfigChangeAction.Type.RESTART, contentAction.getType());
+        assertEquals("service 'searchnode' of type searchnode on host3", contentAction.getServices().get(0).toString());
+        assertEquals(false, contentAction.ignoreForInternalRedeploy());
+    }
+
+    private List<ConfigChangeAction> validate(VespaModel current, VespaModel next) {
+        return new NodeResourceChangeValidator().validate(current, next,
+                                                          ValidationOverrides.empty,
+                                                          Clock.systemUTC().instant());
+    }
+
+    private static VespaModel model(int vcpu1, int vcpu2, int vcpu3, int vcpu4) {
+        var properties = new TestProperties();
+        properties.setHostedVespa(true);
+        var deployState = new DeployState.Builder().properties(properties)
+                                                   .modelHostProvisioner(new Provisioner());
+        return new VespaModelCreatorWithMockPkg(
+                null,
+                "<?xml version='1.0' encoding='utf-8' ?>\n" +
+                "<services version='1.0'>\n" +
+                "    <container id='container1' version='1.0'>\n" +
+                "       <nodes count='1'>\n" +
+                "           <resources vcpu='" + vcpu1 + "' memory='10Gb' disk='100Gb'/>" +
+                "       </nodes>\n" +
+                "   </container>\n" +
+                "    <container id='container2' version='1.0'>\n" +
+                "       <nodes count='2'>\n" +
+                "           <resources vcpu='" + vcpu2 + "' memory='10Gb' disk='100Gb'/>" +
+                "       </nodes>\n" +
+                "   </container>\n" +
+                "   <content id='content1' version='1.0'>\n" +
+                "       <nodes count='3'>\n" +
+                "           <resources vcpu='" + vcpu3 + "' memory='10Gb' disk='100Gb'/>" +
+                "       </nodes>\n" +
+                "       <documents>\n" +
+                "           <document mode='index' type='test'/>\n" +
+                "       </documents>\n" +
+                "       <redundancy>2</redundancy>\n" +
+                "   </content>\n" +
+                "   <content id='content2' version='1.0'>\n" +
+                "       <nodes count='4'>\n" +
+                "           <resources vcpu='" + vcpu4 + "' memory='10Gb' disk='100Gb'/>" +
+                "       </nodes>\n" +
+                "       <documents>\n" +
+                "           <document mode='streaming' type='test'/>\n" +
+                "       </documents>\n" +
+                "       <redundancy>2</redundancy>\n" +
+                "   </content>\n" +
+                "</services>",
+                List.of("schema test { document test {} }"))
+                       .create(deployState);
+    }
+
+    private static class Provisioner implements HostProvisioner {
+
+        private int hostsCreated = 0;
+
+        @Override
+        public HostSpec allocateHost(String alias) {
+            return new HostSpec(alias, List.of(), Optional.empty());
+        }
+
+        @Override
+        public List<HostSpec> prepare(ClusterSpec cluster, Capacity capacity, ProvisionLogger logger) {
+            List<HostSpec> hosts = new ArrayList<>();
+            var resources = capacity.minResources().nodeResources();
+            for (int i = 0; i < capacity.minResources().nodes(); i++)
+                hosts.add(new HostSpec("host" + (hostsCreated++),
+                                       resources, resources, resources,
+                                       ClusterMembership.from(cluster, i),
+                                       Optional.empty(), Optional.empty(), Optional.empty()));
+            return hosts;
+        }
+
+    }
+
+}