Merge pull request #16649 from vespa-engine/jonmv/cc-cleanup

Flag service for orchestrated restart when ZK myid changes

6 files changed, 38 insertions, 19 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 495a70cdf14..feac7e5a92d 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
@@ -4,13 +4,22 @@ package com.yahoo.vespa.model.admin.clustercontroller;
 import com.google.common.base.Joiner;
 import com.yahoo.cloud.config.ZookeeperServerConfig;
 import com.yahoo.cloud.config.ZookeepersConfig;
+import com.yahoo.config.model.api.Model;
+import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
+import com.yahoo.config.provision.AllocatedHosts;
+import com.yahoo.config.provision.HostSpec;
+import com.yahoo.net.HostName;
 import com.yahoo.vespa.model.Service;
 import com.yahoo.vespa.model.admin.Configserver;
 import com.yahoo.vespa.model.container.Container;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static java.util.stream.Collectors.toUnmodifiableSet;
 
 /**
  * Used if clustercontroller is run standalone (not as part of the config server ZooKeeper cluster)
@@ -24,9 +33,16 @@ public class ClusterControllerCluster extends AbstractConfigProducer<ClusterCont
 
     private static final int ZK_CLIENT_PORT = 2181; // Must match the default in CuratorConfig
     private ClusterControllerContainerCluster containerCluster = null;
+    private final Set<String> previousHosts;
 
-    public ClusterControllerCluster(AbstractConfigProducer<?> parent, String subId) {
+    public ClusterControllerCluster(AbstractConfigProducer<?> parent, String subId, DeployState deployState) {
         super(parent, subId);
+        this.previousHosts = deployState.getPreviousModel().stream()
+                                        .map(Model::allocatedHosts)
+                                        .map(AllocatedHosts::getHosts)
+                                        .flatMap(Collection::stream)
+                                        .map(HostSpec::hostname)
+                                        .collect(toUnmodifiableSet());
     }
 
     @Override
@@ -37,6 +53,7 @@ public class ClusterControllerCluster extends AbstractConfigProducer<ClusterCont
             ZookeeperServerConfig.Server.Builder serverBuilder = new ZookeeperServerConfig.Server.Builder();
             serverBuilder.hostname(container.getHostName());
             serverBuilder.id(container.index());
+            serverBuilder.joining( ! previousHosts.isEmpty() && ! previousHosts.contains(container.getHostName()));
             builder.server(serverBuilder);
         }
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java
index 861982cc2ea..eefe245f060 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java
@@ -47,8 +47,9 @@ public class ClusterControllerContainer extends Container implements
             AbstractConfigProducer<?> parent,
             int index,
             boolean runStandaloneZooKeeper,
-            DeployState deployState) {
-        super(parent, "" + index, index, deployState.isHosted());
+            DeployState deployState,
+            boolean retired) {
+        super(parent, "" + index, retired, index, deployState.isHosted());
         this.featureFlags = deployState.featureFlags();
         addHandler("clustercontroller-status",
                    "com.yahoo.vespa.clustercontroller.apps.clustercontroller.StatusHandler",
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RestartConfigs.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RestartConfigs.java
index eccb903df3a..62e80586dc2 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RestartConfigs.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RestartConfigs.java
@@ -18,7 +18,7 @@ import java.lang.annotation.Target;
  * This is different the inheritance that {@link java.lang.annotation.Inherited} provides, where sub-classes can either
  * inherit or override the annotation from the super class.
  *
- * NOTE: This annotation is will only have effect on subclasses of {@link com.yahoo.vespa.model.Service}.
+ * NOTE: This annotation will only have effect on subclasses of {@link com.yahoo.vespa.model.Service}.
  *       Do not use this annotation on other types config producers.
  *
  * @author bjorncs
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java
index e37f558c2ce..e150e5791c0 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV2Builder.java
@@ -88,7 +88,7 @@ public class DomAdminV2Builder extends DomAdminBuilderBase {
 
         boolean standaloneZooKeeper = "true".equals(controllersElements.getAttribute(ATTRIBUTE_CLUSTER_CONTROLLER_STANDALONE_ZK)) || multitenant;
         if (standaloneZooKeeper) {
-            parent = new ClusterControllerCluster(parent, "standalone");
+            parent = new ClusterControllerCluster(parent, "standalone", deployState);
         }
         var cluster = new ClusterControllerContainerCluster(parent,
                                                             "cluster-controllers",
@@ -205,7 +205,7 @@ public class DomAdminV2Builder extends DomAdminBuilderBase {
 
         @Override
         protected ClusterControllerContainer doBuild(DeployState deployState, AbstractConfigProducer parent, Element spec) {
-            return new ClusterControllerContainer(parent, i, runStandaloneZooKeeper, deployState);
+            return new ClusterControllerContainer(parent, i, runStandaloneZooKeeper, deployState, false);
         }
     }
 }
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 0fd7c55147f..3078242a04d 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
@@ -8,6 +8,7 @@ import com.yahoo.config.model.ConfigModelContext;
 import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.producer.AbstractConfigProducer;
 import com.yahoo.config.model.producer.AbstractConfigProducerRoot;
+import com.yahoo.config.provision.ClusterMembership;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.NodeResources;
@@ -169,7 +170,7 @@ public class ContentCluster extends AbstractConfigProducer implements
 
             if (context.getParentProducer().getRoot() == null) return c;
 
-            addClusterControllers(context, c.rootGroup, contentElement, c.clusterId, c);
+            addClusterControllers(context, c.rootGroup, contentElement, c.clusterId, c, deployState);
             return c;
         }
 
@@ -286,7 +287,8 @@ public class ContentCluster extends AbstractConfigProducer implements
                                            StorageGroup rootGroup,
                                            ModelElement contentElement,
                                            String contentClusterName,
-                                           ContentCluster contentCluster) {
+                                           ContentCluster contentCluster,
+                                           DeployState deployState) {
             if (admin == null) return; // only in tests
             if (contentCluster.getPersistence() == null) return;
 
@@ -303,9 +305,9 @@ public class ContentCluster extends AbstractConfigProducer implements
             else if (admin.multitenant()) {
                 String clusterName = contentClusterName + "-controllers";
                 if (context.properties().dedicatedClusterControllerCluster())
-                    clusterControllers = getDedicatedSharedControllers(contentElement, admin, context);
+                    clusterControllers = getDedicatedSharedControllers(contentElement, admin, context, deployState);
                 else {
-                    clusterControllers = createClusterControllers(new ClusterControllerCluster(contentCluster, "standalone"),
+                    clusterControllers = createClusterControllers(new ClusterControllerCluster(contentCluster, "standalone", deployState),
                                                                   drawControllerHosts(3, rootGroup),
                                                                   clusterName,
                                                                   true,
@@ -351,7 +353,7 @@ public class ContentCluster extends AbstractConfigProducer implements
         public static final NodeResources clusterControllerResources = new NodeResources(0.5, 2, 10, 0.3, NodeResources.DiskSpeed.any, NodeResources.StorageType.any);
 
         private ClusterControllerContainerCluster getDedicatedSharedControllers(ModelElement contentElement, Admin admin,
-                                                                                ConfigModelContext context) {
+                                                                                ConfigModelContext context, DeployState deployState) {
             if (admin.getClusterControllers() == null) {
                 NodesSpecification spec = NodesSpecification.exclusiveAndRequiredFromSharedParents(3,
                                                                                                    clusterControllerResources,
@@ -365,7 +367,7 @@ public class ContentCluster extends AbstractConfigProducer implements
                                                                 true)
                                                      .keySet();
 
-                admin.setClusterControllers(createClusterControllers(new ClusterControllerCluster(admin, "standalone"),
+                admin.setClusterControllers(createClusterControllers(new ClusterControllerCluster(admin, "standalone", deployState),
                                                                      hosts,
                                                                      "cluster-controllers",
                                                                      true,
@@ -374,10 +376,6 @@ public class ContentCluster extends AbstractConfigProducer implements
             return admin.getClusterControllers();
         }
 
-        private Collection<HostResource> getControllerHosts(NodesSpecification nodesSpecification, Admin admin, String clusterName, ConfigModelContext context) {
-            return nodesSpecification.provision(admin.hostSystem(), ClusterSpec.Type.admin, ClusterSpec.Id.from(clusterName), context.getDeployLogger(), false).keySet();
-        }
-
         private List<HostResource> drawControllerHosts(int count, StorageGroup rootGroup) {
             List<HostResource> hosts = drawControllerHosts(count, false, rootGroup);
             List<HostResource> retiredHosts = drawControllerHosts(count, true, rootGroup);
@@ -432,8 +430,11 @@ public class ContentCluster extends AbstractConfigProducer implements
             if (clusterControllers.getContainers().isEmpty()) {
                 int index = 0;
                 for (HostResource host : hosts) {
-                    var clusterControllerContainer =
-                            new ClusterControllerContainer(clusterControllers, index, multitenant, deployState);
+                    int ccIndex = deployState.getProperties().dedicatedClusterControllerCluster()
+                                  ? host.spec().membership().map(ClusterMembership::index).orElse(index)
+                                  : index;
+                    boolean retired = host.spec().membership().map(ClusterMembership::retired).orElse(false);
+                    var clusterControllerContainer = new ClusterControllerContainer(clusterControllers, ccIndex, multitenant, deployState, retired);
                     clusterControllerContainer.setHostResource(host);
                     clusterControllerContainer.initService(deployState.getDeployLogger());
                     clusterControllerContainer.setProp("clustertype", "admin");
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
index 72551900984..30e3767e3dc 100755
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java
@@ -383,7 +383,7 @@ public class ContainerClusterTest {
                                              ClusterControllerContainerCluster cluster,
                                              String hostName,
                                              DeployState deployState) {
-        ClusterControllerContainer container = new ClusterControllerContainer(cluster, 1, false, deployState);
+        ClusterControllerContainer container = new ClusterControllerContainer(cluster, 1, false, deployState, false);
         container.setHostResource(new HostResource(new Host(null, hostName)));
         container.initService(deployLogger);
         cluster.addContainer(container);