Merge pull request #16835 from vespa-engine/hakonhall/only-include-infra-cfg-proxy-nodes-of-active-parents-in-the-cfg-proxy-application

4 files changed, 36 insertions, 16 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java
index c3cb805499c..3ff4765dd00 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java
@@ -64,6 +64,9 @@ public interface NodeSpec {
     /** Returns true if there exist some circumstance where we may accept to have this node allocated */
     boolean acceptable(NodeCandidate node);
 
+    /** Returns true if nodes with non-active parent hosts should be rejected */
+    boolean rejectNonActiveParent();
+
     /**
      * Returns true if a node with given current resources and current spare host resources can be resized
      * in-place to resources in this spec.
@@ -164,6 +167,11 @@ public interface NodeSpec {
         public boolean acceptable(NodeCandidate node) { return true; }
 
         @Override
+        public boolean rejectNonActiveParent() {
+            return false;
+        }
+
+        @Override
         public String toString() { return "request for " + count + " nodes with " + requestedNodeResources; }
 
     }
@@ -229,6 +237,11 @@ public interface NodeSpec {
         }
 
         @Override
+        public boolean rejectNonActiveParent() {
+            return true;
+        }
+
+        @Override
         public String toString() { return "request for all nodes of type '" + type + "'"; }
 
     }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java
index 2eee3c3f01c..d2b701e5312 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java
@@ -5,17 +5,17 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterMembership;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.OutOfCapacityException;
-import com.yahoo.lang.MutableInteger;
 import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.hosted.provision.Node;
 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.Nodes;
 
 import java.util.ArrayList;
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Optional;
+import java.util.stream.Collectors;
 
 /**
  * Performs preparation of node activation changes for an application.
@@ -72,6 +72,15 @@ class Preparer {
             List<Node> accepted = groupPreparer.prepare(application, clusterGroup,
                                                         requestedNodes.fraction(wantedGroups), surplusNodes,
                                                         indices, wantedGroups);
+
+            if (requestedNodes.rejectNonActiveParent()) {
+                Nodes nodes = nodeRepository.nodes();
+                NodeList activeHosts = nodes.list(Node.State.active).parents().nodeType(requestedNodes.type().hostType());
+                accepted = accepted.stream()
+                        .filter(node -> node.parentHostname().isEmpty() || activeHosts.parentOf(node).isPresent())
+                        .collect(Collectors.toList());
+            }
+
             replace(acceptedNodes, accepted);
         }
         moveToActiveGroup(surplusNodes, wantedGroups, cluster.group());
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java
index 337bbb0cbb4..076a0e24620 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java
@@ -12,7 +12,6 @@ import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.NodeType;
-import com.yahoo.config.provision.ParentHostUnavailableException;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
@@ -53,7 +52,6 @@ import static com.yahoo.vespa.hosted.provision.testutils.MockHostProvisioner.Beh
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 /**
  * @author freva
@@ -454,12 +452,8 @@ public class DynamicProvisioningMaintainerTest {
         Supplier<Node> hostToRemove = () -> tester.nodeRepository().nodes().node(hostnameToRemove).get();
         Supplier<Node> nodeToRemove = () -> tester.nodeRepository().nodes().node(configNodes.childrenOf(hostnameToRemove).first().get().hostname()).get();
 
-        // Retire and deprovision host
+        // Set want to retire and deprovision on host and children
         tester.nodeRepository().nodes().deprovision(hostToRemove.get(), Agent.system, tester.clock().instant());
-        tester.nodeRepository().nodes().deallocate(hostToRemove.get(), Agent.system, getClass().getSimpleName());
-        assertSame("Host moves to parked", Node.State.parked, hostToRemove.get().state());
-        assertSame("Node remains active", Node.State.active, nodeToRemove.get().state());
-        assertTrue("Node wants to retire", nodeToRemove.get().status().wantToRetire());
 
         // Redeployment of config server application retires node
         tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config);
@@ -477,6 +471,10 @@ public class DynamicProvisioningMaintainerTest {
         tester.nodeRepository().nodes().removeRecursively(inactiveConfigServer, true);
         assertEquals(2, tester.nodeRepository().nodes().list().nodeType(NodeType.config).size());
 
+        // ExpiredRetirer moves host to inactive after child has moved to parked
+        tester.nodeRepository().nodes().deallocate(hostToRemove.get(), Agent.system, getClass().getSimpleName());
+        assertSame("Host moves to parked", Node.State.parked, hostToRemove.get().state());
+
         // Host is removed
         dynamicProvisioningTester.maintainer.maintain();
         assertEquals(2, tester.nodeRepository().nodes().list().nodeType(NodeType.confighost).size());
@@ -488,10 +486,9 @@ public class DynamicProvisioningMaintainerTest {
 
         // Deployment on another config server starts provisioning a new host and child
         HostName.setHostNameForTestingOnly("cfg3.example.com");
-        try {
-            tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config);
-            fail("Expected provisioning to fail");
-        } catch (ParentHostUnavailableException ignored) {}
+        assertEquals(0, tester.nodeRepository().nodes().list(Node.State.reserved).nodeType(NodeType.config).size());
+        assertEquals(2, tester.prepareAndActivateInfraApplication(configSrvApp, NodeType.config).size());
+        assertEquals(1, tester.nodeRepository().nodes().list(Node.State.reserved).nodeType(NodeType.config).size());
         Node newNode = tester.nodeRepository().nodes().list(Node.State.reserved).nodeType(NodeType.config).first().get();
 
         // Resume provisioning and activate host
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 eefbd03ce4e..a30735df01c 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
@@ -226,7 +226,7 @@ public class ProvisioningTester {
         }
     }
 
-    public void prepareAndActivateInfraApplication(ApplicationId application, NodeType nodeType, Version version) {
+    public List<HostSpec> prepareAndActivateInfraApplication(ApplicationId application, NodeType nodeType, Version version) {
         ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from(nodeType.toString()))
                                          .vespaVersion(version)
                                          .stateful(nodeType == NodeType.config || nodeType == NodeType.controller)
@@ -234,10 +234,11 @@ public class ProvisioningTester {
         Capacity capacity = Capacity.fromRequiredNodeType(nodeType);
         List<HostSpec> hostSpecs = prepare(application, cluster, capacity);
         activate(application, hostSpecs);
+        return hostSpecs;
     }
 
-    public void prepareAndActivateInfraApplication(ApplicationId application, NodeType nodeType) {
-        prepareAndActivateInfraApplication(application, nodeType, Version.fromString("6.42"));
+    public List<HostSpec> prepareAndActivateInfraApplication(ApplicationId application, NodeType nodeType) {
+        return prepareAndActivateInfraApplication(application, nodeType, Version.fromString("6.42"));
     }
 
     public void deactivate(ApplicationId applicationId) {