Revert "Don't use the same host for multiple cluster controllers"

3 files changed, 8 insertions, 68 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java
index 3847c32dfb2..b82f58fbb40 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerCluster.java
@@ -43,7 +43,7 @@ public class ClusterControllerCluster extends AbstractConfigProducer<ContainerCl
 
     @Override
     public void getConfig(ZookeepersConfig.Builder builder) {
-        Collection<String> controllerHosts = new ArrayList<>();
+        final Collection<String> controllerHosts = new ArrayList<>();
         for (Container container : containerCluster.getContainers()) {
             controllerHosts.add(container.getHostName() + ":" + ZK_CLIENT_PORT);
         }
@@ -73,6 +73,5 @@ public class ClusterControllerCluster extends AbstractConfigProducer<ContainerCl
             }
         }
     }
-
 }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
index 7e6714b88b1..8350ac3fc79 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
@@ -16,9 +16,7 @@ import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol;
 import com.yahoo.metrics.MetricsmanagerConfig;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.vespa.model.HostResource;
-import com.yahoo.vespa.model.Service;
 import com.yahoo.vespa.model.admin.Admin;
-import com.yahoo.vespa.model.admin.Configserver;
 import com.yahoo.vespa.model.admin.Metric;
 import com.yahoo.vespa.model.admin.MetricsConsumer;
 import com.yahoo.vespa.model.admin.MonitoringSystem;
@@ -321,8 +319,8 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri
 
         private List<HostResource> drawControllerHosts(int count, StorageGroup rootGroup, Collection<ContainerModel> containers) {
             List<HostResource> hosts = drawContentHostsRecursively(count, rootGroup);
-            if (hosts.size() < count) // supply with containers
-                hosts.addAll(drawContainerHosts(count - hosts.size(), containers));
+            //if (hosts.size() < count) // supply with containers TODO: Reactivate
+            //    hosts.addAll(drawContainerHosts(count - hosts.size(), containers));
             if (hosts.size() % 2 == 0) // ZK clusters of even sizes are less available (even in the size=2 case)
                 hosts = hosts.subList(0, hosts.size()-1);
             return hosts;
@@ -341,10 +339,6 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri
             List<HostResource> hosts = new ArrayList<>();
             for (ContainerCluster cluster : clustersSortedByName(containerClusters))
                 hosts.addAll(hostResourcesSortedByIndex(cluster));
-
-            // Don't use the same container to supplement multiple content clusters
-            hosts.removeIf(host -> hasClusterController(host));
-
             return hosts.subList(0, Math.min(hosts.size(), count));
         }
         
@@ -361,13 +355,6 @@ public class ContentCluster extends AbstractConfigProducer implements StorDistri
                     .map(Container::getHostResource)
                     .collect(Collectors.toList());
         }
-        
-        private boolean hasClusterController(HostResource host) {
-            for (Service service : host.getServices())
-                if (service instanceof ClusterControllerContainer)
-                    return true;
-            return false;
-        }
 
         /**
          * Draw <code>count</code> nodes from as many different content groups below this as possible.
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 1c7940fdc8f..e4f405efb93 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
@@ -458,7 +458,7 @@ public class ModelProvisioningTest {
 
         ContentCluster cluster = model.getContentClusters().get("bar");
         ContainerCluster clusterControllers = cluster.getClusterControllers();
-        assertEquals(5, clusterControllers.getContainers().size());
+        assertEquals(1, clusterControllers.getContainers().size()); // TODO: Expected 5 with this feature reactivated
     }
 
     public void testClusterControllersAreNotPlacedOnRetiredNodes() {
@@ -619,63 +619,17 @@ public class ModelProvisioningTest {
                 "  </content>" +
                 "</services>";
 
+        int numberOfHosts = 5;
         VespaModelTester tester = new VespaModelTester();
-        tester.addHosts(5);
+        tester.addHosts(numberOfHosts);
         VespaModel model = tester.createModel(services, true);
+        assertThat(model.getRoot().getHostSystem().getHosts().size(), is(numberOfHosts));
 
         ContentCluster cluster = model.getContentClusters().get("bar");
         ContainerCluster clusterControllers = cluster.getClusterControllers();
-        assertEquals(3, clusterControllers.getContainers().size());
+        assertEquals(1, clusterControllers.getContainers().size()); // TODO: Expected 3 with this feature reactivated
     }
 
-    @Test
-    public void test2ContentNodesOn2ClustersWithContainerClusterProducesMixedClusterControllerCluster() throws ParseException {
-        String services =
-                "<?xml version='1.0' encoding='utf-8' ?>\n" +
-                "<services>" +
-                "  <container version='1.0' id='container'>" +
-                "     <nodes count='3' flavor='container-node'/>" +
-                "  </container>" +
-                "  <content version='1.0' id='content1'>" +
-                "     <redundancy>2</redundancy>" +
-                "     <documents>" +
-                "       <document type='type1' mode='index'/>" +
-                "     </documents>" +
-                "     <nodes count='2' flavor='content1-node'/>" +
-                "  </content>" +
-                "  <content version='1.0' id='content2'>" +
-                "     <redundancy>2</redundancy>" +
-                "     <documents>" +
-                "       <document type='type1' mode='index'/>" +
-                "     </documents>" +
-                "     <nodes count='2' flavor='content2-node'/>" +
-                "  </content>" +
-                "</services>";
-
-        VespaModelTester tester = new VespaModelTester();
-        // use different flavors to make the test clearer
-        tester.addHosts("container-node", 3);
-        tester.addHosts("content1-node",  2);
-        tester.addHosts("content2-node",  2);
-        VespaModel model = tester.createModel(services, true);
-
-        ContentCluster cluster1 = model.getContentClusters().get("content1");
-        ContainerCluster clusterControllers1 = cluster1.getClusterControllers();
-        assertEquals(3, clusterControllers1.getContainers().size());
-        assertEquals("content1-node0",  clusterControllers1.getContainers().get(0).getHostName());
-        assertEquals("content1-node1",  clusterControllers1.getContainers().get(1).getHostName());
-        assertEquals("container-node0", clusterControllers1.getContainers().get(2).getHostName());
-
-        ContentCluster cluster2 = model.getContentClusters().get("content2");
-        ContainerCluster clusterControllers2 = cluster2.getClusterControllers();
-        assertEquals(3, clusterControllers2.getContainers().size());
-        assertEquals("content2-node0",  clusterControllers2.getContainers().get(0).getHostName());
-        assertEquals("content2-node1",  clusterControllers2.getContainers().get(1).getHostName());
-        assertEquals("We do not pick the container used to supplement another cluster",
-                     "container-node1", clusterControllers2.getContainers().get(2).getHostName());
-    }
-
-    @Test
     public void testExplicitDedicatedClusterControllers() {
         String services =
                 "<?xml version='1.0' encoding='utf-8' ?>\n" +