Consider proxy nodes as well in maintainer

Proxy nodes are part of the routing application. Consider nodes of
this type as well in OperatorChangeApplicationMaintainer

2 files changed, 32 insertions, 16 deletions

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 0a8556530e0..954e51f2337 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
@@ -37,7 +37,9 @@ public class OperatorChangeApplicationMaintainer extends ApplicationMaintainer {
 
     @Override
     protected Set<ApplicationId> applicationsNeedingMaintenance() {
-        Map<ApplicationId, List<Node>> nodesByApplication = nodeRepository().getNodes(NodeType.tenant).stream()
+        List<Node> nodes = nodeRepository().getNodes(NodeType.tenant);
+        nodes.addAll(nodeRepository().getNodes(NodeType.proxy));
+        Map<ApplicationId, List<Node>> nodesByApplication = nodes.stream()
                 .filter(node -> node.allocation().isPresent())
                 .collect(Collectors.groupingBy(node -> node.allocation().get().owner(), Collectors.toList()));
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java
index d24bd61e9f5..50c00c730bb 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java
@@ -21,7 +21,6 @@ import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
 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.provisioning.FlavorConfigBuilder;
@@ -62,35 +61,39 @@ public class OperatorChangeApplicationMaintainerTest {
 
         createReadyNodes(15, this.fixture.nodeResources, nodeRepository);
         createHostNodes(2, nodeRepository, nodeFlavors);
+        createProxyNodes(2, nodeRepository, nodeFlavors);
 
         // Create applications
         fixture.activate();
-        assertEquals("Initial applications are deployed", 2, fixture.deployer.redeployments);
+        assertEquals("Initial applications are deployed", 3, fixture.deployer.redeployments);
         OperatorChangeApplicationMaintainer maintainer = new OperatorChangeApplicationMaintainer(fixture.deployer, nodeRepository, Duration.ofMinutes(1));
         
         clock.advance(Duration.ofMinutes(2));
         maintainer.maintain();
-        assertEquals("No changes -> no redeployments", 2, fixture.deployer.redeployments);
+        assertEquals("No changes -> no redeployments", 3, fixture.deployer.redeployments);
 
         nodeRepository.fail(nodeRepository.getNodes(fixture.app1).get(3).hostname(), Agent.system, "Failing to unit test");
         clock.advance(Duration.ofMinutes(2));
         maintainer.maintain();
-        assertEquals("System change -> no redeployments", 2, fixture.deployer.redeployments);
+        assertEquals("System change -> no redeployments", 3, fixture.deployer.redeployments);
 
         clock.advance(Duration.ofSeconds(1));
         nodeRepository.fail(nodeRepository.getNodes(fixture.app2).get(4).hostname(), Agent.operator, "Manual node failing");
         clock.advance(Duration.ofMinutes(2));
         maintainer.maintain();
-        assertEquals("Operator change -> redeployment", 3, fixture.deployer.redeployments);
+        assertEquals("Operator change -> redeployment", 4, fixture.deployer.redeployments);
 
+        clock.advance(Duration.ofSeconds(1));
+        nodeRepository.fail(nodeRepository.getNodes(fixture.app3).get(1).hostname(), Agent.operator, "Manual node failing");
         clock.advance(Duration.ofMinutes(2));
         maintainer.maintain();
-        assertEquals("No further operator changes -> no (new) redeployments", 3, fixture.deployer.redeployments);
-    }
+        assertEquals("Operator change -> redeployment", 5, fixture.deployer.redeployments);
 
-    private void createReadyNodes(int count, NodeRepository nodeRepository, NodeFlavors nodeFlavors) {
-        createReadyNodes(count, nodeFlavors.getFlavorOrThrow("default"), nodeRepository);
+        clock.advance(Duration.ofMinutes(2));
+        maintainer.maintain();
+        assertEquals("No further operator changes -> no (new) redeployments", 5, fixture.deployer.redeployments);
     }
+
     private void createReadyNodes(int count, NodeResources resources, NodeRepository nodeRepository) {
         createReadyNodes(count, new Flavor(resources), nodeRepository);
     }
@@ -113,7 +116,16 @@ public class OperatorChangeApplicationMaintainerTest {
         nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName());
     }
 
-    private class Fixture {
+    private void createProxyNodes(int count, NodeRepository nodeRepository, NodeFlavors nodeFlavors) {
+        List<Node> nodes = new ArrayList<>(count);
+        for (int i = 0; i < count; i++)
+            nodes.add(nodeRepository.createNode("proxyNode" + i, "proxyHost" + i, Optional.empty(), nodeFlavors.getFlavorOrThrow("default"), NodeType.proxy));
+        nodes = nodeRepository.addNodes(nodes);
+        nodes = nodeRepository.setDirty(nodes, Agent.system, getClass().getSimpleName());
+        nodeRepository.setReady(nodes, Agent.system, getClass().getSimpleName());
+    }
+
+    private static class Fixture {
 
         final NodeRepository nodeRepository;
         final MockDeployer deployer;
@@ -121,10 +133,13 @@ public class OperatorChangeApplicationMaintainerTest {
         final NodeResources nodeResources = new NodeResources(2, 8, 50, 1);
         final ApplicationId app1 = ApplicationId.from(TenantName.from("foo1"), ApplicationName.from("bar"), InstanceName.from("fuz"));
         final ApplicationId app2 = ApplicationId.from(TenantName.from("foo2"), ApplicationName.from("bar"), InstanceName.from("fuz"));
+        final ApplicationId app3 = ApplicationId.from(TenantName.from("vespa-hosted"), ApplicationName.from("routing"), InstanceName.from("default"));
         final ClusterSpec clusterApp1 = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("test"), Version.fromString("6.42"), false);
         final ClusterSpec clusterApp2 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("test"), Version.fromString("6.42"), false);
+        final ClusterSpec clusterApp3 = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("routing"), Version.fromString("6.42"), false);
         final int wantedNodesApp1 = 5;
         final int wantedNodesApp2 = 7;
+        final int wantedNodesApp3 = 2;
 
         Fixture(Zone zone, NodeRepository nodeRepository) {
             this.nodeRepository = nodeRepository;
@@ -135,19 +150,18 @@ public class OperatorChangeApplicationMaintainerTest {
 
             Map<ApplicationId, MockDeployer.ApplicationContext> apps = Map.of(
                     app1, new MockDeployer.ApplicationContext(app1, clusterApp1, Capacity.fromCount(wantedNodesApp1, nodeResources), 1),
-                    app2, new MockDeployer.ApplicationContext(app2, clusterApp2, Capacity.fromCount(wantedNodesApp2, nodeResources), 1));
+                    app2, new MockDeployer.ApplicationContext(app2, clusterApp2, Capacity.fromCount(wantedNodesApp2, nodeResources), 1),
+                    app3, new MockDeployer.ApplicationContext(app3, clusterApp3, Capacity.fromRequiredNodeType(NodeType.proxy), 0)) ;
             this.deployer = new MockDeployer(provisioner, nodeRepository.clock(), apps);
         }
 
         void activate() {
             deployer.deployFromLocalActive(app1, false).get().activate();
             deployer.deployFromLocalActive(app2, false).get().activate();
+            deployer.deployFromLocalActive(app3, false).get().activate();
             assertEquals(wantedNodesApp1, nodeRepository.getNodes(app1, Node.State.active).size());
             assertEquals(wantedNodesApp2, nodeRepository.getNodes(app2, Node.State.active).size());
-        }
-
-        NodeList getNodes(Node.State ... states) {
-            return new NodeList(nodeRepository.getNodes(NodeType.tenant, states));
+            assertEquals(wantedNodesApp3, nodeRepository.getNodes(app3, Node.State.active).size());
         }
 
     }