Collect all out of capacity reasons

7 files changed, 42 insertions, 41 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 436b1e49a54..bb06db3e78b 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
@@ -791,13 +791,13 @@ public class NodeRepository extends AbstractComponent {
     }
 
     public boolean canAllocateTenantNodeTo(Node host) {
-        if (!host.type().canRun(NodeType.tenant)) return false;
+        if ( ! host.type().canRun(NodeType.tenant)) return false;
 
         // Do not allocate to hosts we want to retire or are currently retiring
         if (host.status().wantToRetire() || host.allocation().map(alloc -> alloc.membership().retired()).orElse(false))
             return false;
 
-        if (!zone.getCloud().dynamicProvisioning()) return host.state() == State.active;
+        if ( ! zone.getCloud().dynamicProvisioning()) return host.state() == State.active;
         else return EnumSet.of(State.active, State.ready, State.provisioned).contains(host.state());
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java
index 06300e45e9e..a762f718ab7 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java
@@ -74,12 +74,16 @@ public abstract class ApplicationMaintainer extends NodeRepositoryMaintainer {
     /** Returns the applications that should be maintained by this now. */
     protected abstract Set<ApplicationId> applicationsNeedingMaintenance();
 
-    /** Redeploy this application. A lock will be taken for the duration of the deployment activation */
-    protected final void deployWithLock(ApplicationId application) {
+    /**
+     * Redeploy this application. A lock will be taken for the duration of the deployment activation
+     *
+     * @return whether it was successfully deployed
+     */
+    protected final boolean deployWithLock(ApplicationId application) {
         try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, metric, nodeRepository())) {
-            if ( ! deployment.isValid()) return; // this will be done at another config server
-            if ( ! canDeployNow(application)) return; // redeployment is no longer needed
-            deployment.activate();
+            if ( ! deployment.isValid()) return false; // this will be done at another config server
+            if ( ! canDeployNow(application)) return false; // redeployment is no longer needed
+            return deployment.activate();
         } finally {
             pendingDeployments.remove(application);
         }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java
index 3319db33730..18471637da7 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java
@@ -71,6 +71,11 @@ class MaintenanceDeployment implements Closeable {
         return doStep(() -> deployment.get().prepare());
     }
 
+    /**
+     * Attempts to activate this deployment
+     *
+     * @return whether it was successfully activated
+     */
     public boolean activate() {
         return doStep(() -> deployment.get().activate());
     }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java
index 9f829c095f4..b5933196ecc 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainer.java
@@ -55,9 +55,10 @@ public class OperatorChangeApplicationMaintainer extends ApplicationMaintainer {
      */
     @Override
     protected void deploy(ApplicationId application) {
-        deployWithLock(application);
-        log.info("Redeployed application " + application.toShortString() +
-                 " as a manual change was made to its nodes");
+        boolean deployed = deployWithLock(application);
+        if (deployed)
+            log.info("Redeployed application " + application.toShortString() +
+                     " as a manual change was made to its nodes");
     }
 
     private boolean hasNodesWithChanges(ApplicationId applicationId, List<Node> nodes) {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java
index 3a07283fc72..30f9001093d 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java
@@ -17,6 +17,7 @@ import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -115,7 +116,7 @@ public class GroupPreparer {
                 if (! allocation.fulfilled() && requestedNodes.canFail())
                     throw new OutOfCapacityException("Could not satisfy " + requestedNodes + " for " + cluster +
                                                      " in " + application.toShortString() +
-                                                     outOfCapacityDetails(allocation));
+                                                     allocation.outOfCapacityDetails());
 
                 // Carry out and return allocation
                 nodeRepository.reserve(allocation.reservableNodes());
@@ -126,17 +127,4 @@ public class GroupPreparer {
         }
     }
 
-    private static String outOfCapacityDetails(NodeAllocation allocation) {
-        if (allocation.wouldBeFulfilledWithoutExclusivity())
-            return ": Not enough nodes available due to host exclusivity constraints.";
-        else if (allocation.wouldBeFulfilledWithClashingParentHost())
-            return ": Not enough nodes available on separate physical hosts.";
-        else if (allocation.wouldBeFulfilledWithRetiredNodes())
-            return ": Not enough nodes available due to retirement.";
-        else if (allocation.wouldBeFulfilledWithSufficientRealResources())
-            return ": Not enough real resources on hosts";
-        else
-            return ".";
-    }
-
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
index 0544b675a6f..83c68c91fc5 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
@@ -296,22 +296,6 @@ class NodeAllocation {
         return requestedNodes.fulfilledBy(accepted);
     }
 
-    boolean wouldBeFulfilledWithRetiredNodes() {
-        return requestedNodes.fulfilledBy(accepted + wasRetiredJustNow);
-    }
-
-    boolean wouldBeFulfilledWithClashingParentHost() {
-        return requestedNodes.fulfilledBy(accepted + rejectedDueToClashingParentHost);
-    }
-
-    boolean wouldBeFulfilledWithoutExclusivity() {
-        return requestedNodes.fulfilledBy(accepted + rejectedDueToExclusivity);
-    }
-
-    boolean wouldBeFulfilledWithSufficientRealResources() {
-        return requestedNodes.fulfilledBy(accepted + rejectedDueToInsufficientRealResources);
-    }
-
     /**
      * Returns {@link FlavorCount} describing the docker node deficit for the given {@link NodeSpec}.
      *
@@ -406,6 +390,21 @@ class NodeAllocation {
         return Comparator.comparing((PrioritizableNode n) -> n.node.allocation().get().membership().index());
     }
 
+    public String outOfCapacityDetails() {
+        List<String> reasons = new ArrayList<>();
+        if (rejectedDueToExclusivity > 0)
+            reasons.add("host exclusivity constraints");
+        if (rejectedDueToClashingParentHost > 0)
+            reasons.add("insufficient nodes available on separate physical hosts");
+        if (wasRetiredJustNow > 0)
+            reasons.add("retirement of allocated nodes");
+        if (rejectedDueToInsufficientRealResources > 0)
+            reasons.add("insufficient real resources on hosts");
+
+        if (reasons.isEmpty()) return "";
+        return ": Not enough nodes available due to " + reasons.stream().collect(Collectors.joining(", "));
+    }
+
     static class FlavorCount {
 
         private final NodeResources flavor;
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java
index 5cbdf44c183..441538fc305 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java
@@ -247,7 +247,11 @@ public class DockerProvisioningTest {
         }
         catch (Exception e) {
             assertEquals("No room for 3 nodes as 2 of 4 hosts are exclusive",
-                         "Could not satisfy request for 3 nodes with [vcpu: 1.0, memory: 4.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, storage type: local] for container cluster 'myContainer' group 0 6.39 in tenant1.app1: Not enough nodes available due to host exclusivity constraints.",
+                         "Could not satisfy request for 3 nodes with " +
+                         "[vcpu: 1.0, memory: 4.0 Gb, disk 100.0 Gb, bandwidth: 1.0 Gbps, storage type: local] " +
+                         "for container cluster 'myContainer' group 0 6.39 in tenant1.app1: " +
+                         "Not enough nodes available due to host exclusivity constraints, " +
+                         "insufficient nodes available on separate physical hosts",
                          e.getMessage());
         }