Test warning

8 files changed, 38 insertions, 17 deletions

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 506564f9ffa..ad91bdef478 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
@@ -130,7 +130,7 @@ public class NodeRepositoryProvisioner implements Provisioner {
             logger.logApplicationPackage(Level.WARNING, "Requested disk (" + resources.diskGb() +
                                                         "Gb) in " + cluster.id() + " is not large enough to fit " +
                                                         "core/heap dumps. Minimum recommended disk resources " +
-                                                        "is 3x memory for content and 2x memory for containers");
+                                                        "is 2x memory for containers and 3x memory for content");
         }
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/SystemOutProvisionLogger.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/InMemoryProvisionLogger.java
index ebcf72a11ff..65abcbef698 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/SystemOutProvisionLogger.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/InMemoryProvisionLogger.java
@@ -3,21 +3,33 @@ package com.yahoo.vespa.hosted.provision.testutils;
 
 import com.yahoo.config.provision.ProvisionLogger;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.logging.Level;
 
 /**
+ * A logger which remembers all messages logged in addition to writing them to standard out.
+ *
  * @author bratseth
  */
-public class SystemOutProvisionLogger implements ProvisionLogger {
+public class InMemoryProvisionLogger implements ProvisionLogger {
+
+    private final List<String> systemLog = new ArrayList<>();
+    private final List<String> applicationLog = new ArrayList<>();
 
     @Override
     public void log(Level level, String message) {
         System.out.println("ProvisionLogger system " + level + ": " + message);
+        systemLog.add(level + ": " + message);
     }
 
     @Override
     public void logApplicationPackage(Level level, String message) {
         System.out.println("ProvisionLogger application " + level + ": " + message);
+        applicationLog.add(level + ": " + message);
     }
 
+    public List<String> systemLog() { return systemLog; }
+    public List<String> applicationLog() { return applicationLog; }
+
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
index 053633da80a..3ed01e00ee6 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
@@ -261,7 +261,7 @@ public class MockDeployer implements Deployer {
         public ClusterSpec cluster() { return cluster; }
 
         private List<HostSpec> prepare(NodeRepositoryProvisioner provisioner) {
-            return provisioner.prepare(id, cluster, capacity, new SystemOutProvisionLogger());
+            return provisioner.prepare(id, cluster, capacity, new InMemoryProvisionLogger());
         }
 
     }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
index 64c5dff0718..f49537c3ec8 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
@@ -312,7 +312,6 @@ public class AutoscalingTest {
         fixture.deactivateRetired(capacity);
         fixture.tester().clock().advance(Duration.ofDays(1));
         fixture.loader().applyCpuLoad(0.8, 120);
-        System.out.println("Autoscaling ----------");
         assertEquals(DiskSpeed.any, fixture.autoscale(capacity).resources().get().nodeResources().diskSpeed());
     }
 
@@ -480,6 +479,18 @@ public class AutoscalingTest {
     }
 
     @Test
+    public void too_small_disk_compared_to_memory() {
+        var resources = new ClusterResources(2, 1, new NodeResources(1, 10, 19, 1));
+        var fixture = DynamicProvisioningTester.fixture()
+                                               .awsProdSetup(true)
+                                               .initialResources(Optional.of(resources))
+                                               .build();
+        assertEquals(2, fixture.tester().provisionLogger().applicationLog().size()); // tester deploys twice
+        assertEquals("WARNING: Requested disk (19.0Gb) in cluster 'cluster1' is not large enough to fit core/heap dumps. Minimum recommended disk resources is 2x memory for containers and 3x memory for content",
+                     fixture.tester().provisionLogger().applicationLog().get(0));
+    }
+
+    @Test
     public void test_autoscaling_group_size_unconstrained() {
         var min = new ClusterResources( 2, 2, new NodeResources(1, 1, 1, 1));
         var now = new ClusterResources(5, 5, new NodeResources(3, 100, 100, 1));
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java
index 3d5297ad573..00ae9ac5a9d 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java
@@ -30,7 +30,7 @@ import com.yahoo.vespa.hosted.provision.testutils.MockDeployer;
 import com.yahoo.vespa.hosted.provision.testutils.ServiceMonitorStub;
 import com.yahoo.vespa.service.duper.InfraApplication;
 import com.yahoo.vespa.service.duper.TenantHostApplication;
-import com.yahoo.vespa.hosted.provision.testutils.SystemOutProvisionLogger;
+import com.yahoo.vespa.hosted.provision.testutils.InMemoryProvisionLogger;
 
 import java.time.Clock;
 import java.time.Duration;
@@ -271,7 +271,7 @@ public class NodeFailTester {
     }
 
     public void activate(ApplicationId applicationId, ClusterSpec cluster, Capacity capacity) {
-        List<HostSpec> hosts = provisioner.prepare(applicationId, cluster, capacity, new SystemOutProvisionLogger());
+        List<HostSpec> hosts = provisioner.prepare(applicationId, cluster, capacity, new InMemoryProvisionLogger());
         try (var lock = provisioner.lock(applicationId)) {
             NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(curator));
             provisioner.activate(hosts, new ActivationContext(0), new ApplicationTransaction(lock, transaction));
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java
index e1e83ad2fb3..df0654de8d5 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java
@@ -505,7 +505,7 @@ public class DynamicProvisioningTest {
     }
 
     @Test
-    public void gpu_host()  {
+    public void gpu_host() {
         List<Flavor> flavors = List.of(new Flavor("gpu", new NodeResources(4, 16, 125, 10, fast, local,
                                                                            Architecture.x86_64, new NodeResources.GpuResources(1, 16))));
         ProvisioningTester tester = new ProvisioningTester.Builder().dynamicProvisioning(true, false)
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTester.java
index b043a1cfb0f..de9b3a4db33 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTester.java
@@ -24,9 +24,7 @@ import com.yahoo.vespa.hosted.provision.autoscale.Autoscaling;
 import com.yahoo.vespa.hosted.provision.autoscale.Fixture;
 import com.yahoo.vespa.hosted.provision.autoscale.MetricsDb;
 import com.yahoo.vespa.hosted.provision.node.IP;
-import com.yahoo.vespa.hosted.provision.provisioning.CapacityPolicies;
-import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator;
-import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester;
+import com.yahoo.vespa.hosted.provision.testutils.InMemoryProvisionLogger;
 
 import java.time.Duration;
 import java.util.ArrayList;
@@ -44,7 +42,7 @@ import static org.junit.Assert.assertTrue;
  * - Supports autoscaling testing.
  *
  * TODO: All provisioning testing should migrate to use this, and then the provisionging tester should be collapsed
- *       into this.
+ *       into this.   ... or we should just use autoscalingtester for everything.
  *
  * @author bratseth
  */
@@ -89,6 +87,8 @@ public class DynamicProvisioningTester {
         return flavors;
     }
 
+    public InMemoryProvisionLogger provisionLogger() { return provisioningTester.provisionLogger(); }
+
     public static Fixture.Builder fixture() { return new Fixture.Builder(); }
 
     public static Fixture.Builder fixture(ClusterResources min, ClusterResources now, ClusterResources max) {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
index 9bbc5c5e25a..a3a90d58c2c 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java
@@ -47,6 +47,7 @@ import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.node.IP;
 import com.yahoo.vespa.hosted.provision.node.filter.NodeHostFilter;
 import com.yahoo.vespa.hosted.provision.persistence.NameResolver;
+import com.yahoo.vespa.hosted.provision.testutils.InMemoryProvisionLogger;
 import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver;
 import com.yahoo.vespa.hosted.provision.testutils.MockProvisionServiceProvider;
 import com.yahoo.vespa.hosted.provision.testutils.OrchestratorMock;
@@ -93,7 +94,7 @@ public class ProvisioningTester {
     private final HostProvisioner hostProvisioner;
     private final NodeRepositoryProvisioner provisioner;
     private final CapacityPolicies capacityPolicies;
-    private final ProvisionLogger provisionLogger;
+    private final InMemoryProvisionLogger provisionLogger;
     private final LoadBalancerServiceMock loadBalancerService;
 
     private int nextHost = 0;
@@ -132,7 +133,7 @@ public class ProvisioningTester {
                                                  1000);
         this.provisioner = new NodeRepositoryProvisioner(nodeRepository, zone, provisionServiceProvider);
         this.capacityPolicies = new CapacityPolicies(nodeRepository);
-        this.provisionLogger = new NullProvisionLogger();
+        this.provisionLogger = new InMemoryProvisionLogger();
         this.loadBalancerService = loadBalancerService;
     }
 
@@ -162,6 +163,7 @@ public class ProvisioningTester {
     public CapacityPolicies capacityPolicies() { return capacityPolicies; }
     public NodeList getNodes(ApplicationId id, Node.State ... inState) { return nodeRepository.nodes().list(inState).owner(id); }
     public InMemoryFlagSource flagSource() { return (InMemoryFlagSource) nodeRepository.flagSource(); }
+    public InMemoryProvisionLogger provisionLogger() { return provisionLogger; }
     public Node node(String hostname) { return nodeRepository.nodes().node(hostname).get(); }
 
     public int decideSize(Capacity capacity, ApplicationId application) {
@@ -773,10 +775,6 @@ public class ProvisioningTester {
 
     }
 
-    public static class NullProvisionLogger implements ProvisionLogger {
-        @Override public void log(Level level, String message) { }
-    }
-
     static class MockResourcesCalculator implements HostResourcesCalculator {
 
         private final int memoryTaxGb;