Only include non-retired nodes in redundancy computation

Usually redundancy is down-adjusted to match the nodes per group,
but not if the node count requested is high enough but downscaled
by capacity policies. The redundancy is also adjusted down if there
are not enough nodes in the actual allocation, which normally
handles this case, but not if there are retired nodes.
That is fixed in this PR by not counting retired nodes
when making that adjustment.

1 files changed, 33 insertions, 5 deletions

diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
index 10019b00f61..499ec94f8ee 100644
--- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
@@ -35,7 +35,6 @@ import com.yahoo.vespa.model.test.VespaModelTester;
 import com.yahoo.vespa.model.test.utils.ApplicationPackageUtils;
 import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg;
 import com.yahoo.yolean.Exceptions;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import java.io.StringReader;
@@ -986,6 +985,35 @@ public class ModelProvisioningTest {
     }
 
     @Test
+    public void testRedundancy2DownscaledToOneNodeButOneRetired() {
+        String services =
+                "<?xml version='1.0' encoding='utf-8' ?>" +
+                "<services>" +
+                "  <content version='1.0' id='bar'>" +
+                "     <redundancy>2</redundancy>" +
+                "     <documents>" +
+                "       <document type='type1' mode='index'/>" +
+                "     </documents>" +
+                "     <nodes count='2'/>" +
+                "  </content>" +
+                "</services>";
+
+        int numberOfHosts = 3;
+        VespaModelTester tester = new VespaModelTester();
+        tester.addHosts(numberOfHosts);
+        VespaModel model = tester.createModel(services, false, false, true, "node-1-3-10-03");
+        assertEquals(numberOfHosts, model.getRoot().hostSystem().getHosts().size());
+
+        ContentCluster cluster = model.getContentClusters().get("bar");
+        assertEquals(2, cluster.getStorageNodes().getChildren().size());
+        assertEquals(1, cluster.redundancy().effectiveInitialRedundancy());
+        assertEquals(1, cluster.redundancy().effectiveFinalRedundancy());
+        assertEquals(1, cluster.redundancy().effectiveReadyCopies());
+        assertEquals(2, cluster.getRootGroup().getNodes().size());
+        assertEquals(0, cluster.getRootGroup().getSubgroups().size());
+    }
+
+    @Test
     public void testUsingNodesCountAttributesAndGettingTooFewNodes() {
         String services =
                 "<?xml version='1.0' encoding='utf-8' ?>" +
@@ -1451,7 +1479,7 @@ public class ModelProvisioningTest {
         assertEquals("We get 1 node per cluster and no admin node apart from the dedicated cluster controller", 3, model.getHosts().size());
         assertEquals(1, model.getContainerClusters().size());
         assertEquals(1, model.getContainerClusters().get("foo").getContainers().size());
-        assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes());
+        assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes(true));
         assertEquals(1, model.getAdmin().getClusterControllers().getContainers().size());
     }
 
@@ -1504,7 +1532,7 @@ public class ModelProvisioningTest {
         assertEquals(6, model.getRoot().hostSystem().getHosts().size());
         assertEquals(5, model.getAdmin().getSlobroks().size());
         assertEquals(2, model.getContainerClusters().get("foo").getContainers().size());
-        assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes());
+        assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes(true));
     }
 
     @Test
@@ -1574,7 +1602,7 @@ public class ModelProvisioningTest {
         assertEquals(1, model.getRoot().hostSystem().getHosts().size());
         assertEquals(1, model.getAdmin().getSlobroks().size());
         assertEquals(1, model.getContainerClusters().get("foo").getContainers().size());
-        assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes());
+        assertEquals(1, model.getContentClusters().get("bar").getRootGroup().countNodes(true));
     }
 
     /** Recreate the combination used in some factory tests */
@@ -1857,7 +1885,7 @@ public class ModelProvisioningTest {
             assertTrue("Initial servers are not joining", config.build().server().stream().noneMatch(ZookeeperServerConfig.Server::joining));
         }
         {
-            VespaModel nextModel = tester.createModel(Zone.defaultZone(), servicesXml.apply(5), true, false, 0, Optional.of(model), new DeployState.Builder());
+            VespaModel nextModel = tester.createModel(Zone.defaultZone(), servicesXml.apply(5), true, false, false, 0, Optional.of(model), new DeployState.Builder());
             ApplicationContainerCluster cluster = nextModel.getContainerClusters().get("zk");
             ZookeeperServerConfig.Builder config = new ZookeeperServerConfig.Builder();
             cluster.getContainers().forEach(c -> c.getConfig(config));