Reject invalid nodes also if allocated

This will never happen now, but I think it's clearer to show that
explicitly in the code.

2 files changed, 15 insertions, 8 deletions

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 240963a8c0d..1e98160955c 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
@@ -112,8 +112,11 @@ class NodeAllocation {
                 boolean resizeable = requestedNodes.considerRetiring() && candidate.isResizable;
                 boolean acceptToRetire = acceptToRetire(candidate);
 
-                if ((! saturated() && hasCompatibleFlavor(candidate) && requestedNodes.acceptable(candidate)) || acceptToRetire)
-                    accepted.add(acceptNode(candidate, shouldRetire(candidate), resizeable));
+                if ((! saturated() && hasCompatibleFlavor(candidate) && requestedNodes.acceptable(candidate)) || acceptToRetire) {
+                    candidate = candidate.withNode();
+                    if (candidate.isValid())
+                        accepted.add(acceptNode(candidate, shouldRetire(candidate), resizeable));
+                }
             }
             else if (! saturated() && hasCompatibleFlavor(candidate)) {
                 if ( ! nodeResourceLimits.isWithinRealLimits(candidate, cluster)) {
@@ -240,7 +243,6 @@ class NodeAllocation {
     }
 
     private Node acceptNode(NodeCandidate candidate, boolean wantToRetire, boolean resizeable) {
-        candidate = candidate.withNode();
         Node node = candidate.toNode();
 
         if (node.allocation().isPresent()) // Record the currently requested resources
@@ -356,7 +358,7 @@ class NodeAllocation {
             candidate = candidate.withNode();
             Allocation allocation = candidate.allocation().get();
             candidate = candidate.withNode(candidate.toNode().with(allocation.with(allocation.membership()
-                                .with(allocation.membership().cluster().exclusive(requestedNodes.isExclusive())))));
+                                 .with(allocation.membership().cluster().exclusive(requestedNodes.isExclusive())))));
             nodes.put(candidate.toNode().hostname(), candidate);
         }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java
index b915053fff5..a8457eef374 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java
@@ -358,10 +358,12 @@ abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidate> {
             Optional<IP.Allocation> allocation;
             try {
                 allocation = parent.get().ipConfig().pool().findAllocation(allNodes, nodeRepository.nameResolver());
-                if (allocation.isEmpty()) return new InvalidNodeCandidate(resources, freeParentCapacity, parent.get());
+                if (allocation.isEmpty()) return new InvalidNodeCandidate(resources, freeParentCapacity, parent.get(),
+                                                                          "No IP addresses available on parent host");
             } catch (Exception e) {
                 log.warning("Failed allocating IP address on " + parent.get() +": " + Exceptions.toMessageString(e));
-                return new InvalidNodeCandidate(resources, freeParentCapacity, parent.get());
+                return new InvalidNodeCandidate(resources, freeParentCapacity, parent.get(),
+                                                "Failed when allocating IP address on host");
             }
 
             Node node = Node.createDockerNode(allocation.get().addresses(),
@@ -409,10 +411,13 @@ abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidate> {
     static class InvalidNodeCandidate extends NodeCandidate {
 
         private final NodeResources resources;
+        private final String invalidReason;
 
-        private InvalidNodeCandidate(NodeResources resources, NodeResources freeParentCapacity, Node parent) {
+        private InvalidNodeCandidate(NodeResources resources, NodeResources freeParentCapacity, Node parent,
+                                     String invalidReason) {
             super(freeParentCapacity, Optional.of(parent), false, false, false, true, false);
             this.resources = resources;
+            this.invalidReason = invalidReason;
         }
 
         @Override
@@ -453,7 +458,7 @@ abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidate> {
 
         @Override
         public Node toNode() {
-            throw new IllegalStateException("Candidate node on " + parent.get() + " is invalid");
+            throw new IllegalStateException("Candidate node on " + parent.get() + " is invalid: " + invalidReason);
         }
 
         @Override