Merge pull request #15791 from vespa-engine/mpolden/joining-servers

Make joining server an observer in initial config

3 files changed, 67 insertions, 13 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
index 2b8966ed541..f8ff7bcdc18 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
@@ -7,8 +7,11 @@ import com.yahoo.component.ComponentId;
 import com.yahoo.component.ComponentSpecification;
 import com.yahoo.config.FileReference;
 import com.yahoo.config.application.api.ComponentInfo;
+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.container.bundle.BundleInstantiationSpecification;
 import com.yahoo.container.di.config.ApplicationBundlesConfig;
 import com.yahoo.container.handler.metrics.MetricsProxyApiConfig;
@@ -74,6 +77,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat
 
     private final ConfigProducerGroup<Servlet> servletGroup;
     private final ConfigProducerGroup<RestApi> restApiGroup;
+    private final Set<String> previousHosts;
 
     private ContainerModelEvaluation modelEvaluation;
 
@@ -89,6 +93,12 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat
         this.tlsClientAuthority = deployState.tlsClientAuthority();
         restApiGroup = new ConfigProducerGroup<>(this, "rest-api");
         servletGroup = new ConfigProducerGroup<>(this, "servlet");
+        previousHosts = deployState.getPreviousModel().stream()
+                                   .map(Model::allocatedHosts)
+                                   .map(AllocatedHosts::getHosts)
+                                   .flatMap(Collection::stream)
+                                   .map(HostSpec::hostname)
+                                   .collect(Collectors.toUnmodifiableSet());
 
         addSimpleComponent(DEFAULT_LINGUISTICS_PROVIDER);
         addSimpleComponent("com.yahoo.container.jdisc.SecretStoreProvider");
@@ -261,10 +271,12 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat
         // Note: Default client and server ports are used, so not set here
         for (Container container : getContainers()) {
             ZookeeperServerConfig.Server.Builder serverBuilder = new ZookeeperServerConfig.Server.Builder();
-            serverBuilder.hostname(container.getHostName());
-            serverBuilder.id(container.index());
-            builder.server(serverBuilder);
-            builder.dynamicReconfiguration(true);
+            serverBuilder.hostname(container.getHostName())
+                         .id(container.index())
+                         .joining(!previousHosts.isEmpty() &&
+                                  !previousHosts.contains(container.getHostName()));
+            builder.server(serverBuilder)
+                   .dynamicReconfiguration(true);
         }
     }
 
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 96b2cfc3118..1abb72d73b5 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
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.config.model.provision;
 
+import com.yahoo.cloud.config.ZookeeperServerConfig;
 import com.yahoo.cloud.config.log.LogdConfig;
 import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.model.api.container.ContainerServiceType;
@@ -40,6 +41,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import static com.yahoo.config.model.test.TestUtil.joinLines;
@@ -1871,6 +1873,45 @@ public class ModelProvisioningTest {
         assertEquals(3, cluster.getContainers().stream().filter(c -> !c.isRetired()).count());
     }
 
+    @Test
+    public void containerWithZooKeeperJoiningServers() {
+        Function<Integer, String> servicesXml = (nodeCount) -> {
+            return "<?xml version='1.0' encoding='utf-8' ?>" +
+                   "<services>" +
+                   "  <container version='1.0' id='zk'>" +
+                   "     <zookeeper/>" +
+                   "     <nodes count='" + nodeCount + "'/>" +
+                   "  </container>" +
+                   "</services>";
+        };
+        VespaModelTester tester = new VespaModelTester();
+        tester.addHosts(5);
+        VespaModel model = tester.createModel(servicesXml.apply(3), true);
+
+        {
+            ApplicationContainerCluster cluster = model.getContainerClusters().get("zk");
+            ZookeeperServerConfig.Builder config = new ZookeeperServerConfig.Builder();
+            cluster.getContainers().forEach(c -> c.getConfig(config));
+            cluster.getConfig(config);
+            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));
+            ApplicationContainerCluster cluster = nextModel.getContainerClusters().get("zk");
+            ZookeeperServerConfig.Builder config = new ZookeeperServerConfig.Builder();
+            cluster.getContainers().forEach(c -> c.getConfig(config));
+            cluster.getConfig(config);
+            assertEquals("New nodes are joining",
+                         Map.of(0, false,
+                                1, false,
+                                2, false,
+                                3, true,
+                                4, true),
+                         config.build().server().stream().collect(Collectors.toMap(ZookeeperServerConfig.Server::id,
+                                                                                   ZookeeperServerConfig.Server::joining)));
+        }
+    }
+
     private VespaModel createNonProvisionedMultitenantModel(String services) {
         return createNonProvisionedModel(true, null, services);
     }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
index 813ca4ac0cb..7484095c5bc 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
@@ -104,22 +104,22 @@ public class VespaModelTester {
 
     /** Creates a model which uses 0 as start index */
     public VespaModel createModel(String services, boolean failOnOutOfCapacity, String ... retiredHostNames) {
-        return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, 0, retiredHostNames);
+        return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, 0, Optional.empty(), retiredHostNames);
     }
 
     /** Creates a model which uses 0 as start index */
     public VespaModel createModel(String services, boolean failOnOutOfCapacity, boolean useMaxResources, String ... retiredHostNames) {
-        return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, useMaxResources, 0, retiredHostNames);
+        return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, useMaxResources, 0, Optional.empty(), retiredHostNames);
     }
 
     /** Creates a model which uses 0 as start index */
     public VespaModel createModel(String services, boolean failOnOutOfCapacity, int startIndexForClusters, String ... retiredHostNames) {
-        return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, startIndexForClusters, retiredHostNames);
+        return createModel(Zone.defaultZone(), services, failOnOutOfCapacity, false, startIndexForClusters, Optional.empty(), retiredHostNames);
     }
 
     /** Creates a model which uses 0 as start index */
     public VespaModel createModel(Zone zone, String services, boolean failOnOutOfCapacity, String ... retiredHostNames) {
-        return createModel(zone, services, failOnOutOfCapacity, false, 0, retiredHostNames);
+        return createModel(zone, services, failOnOutOfCapacity, false, 0, Optional.empty(), retiredHostNames);
     }
 
     /**
@@ -132,7 +132,7 @@ public class VespaModelTester {
      * @return the resulting model
      */
     public VespaModel createModel(Zone zone, String services, boolean failOnOutOfCapacity, boolean useMaxResources,
-                                  int startIndexForClusters, String ... retiredHostNames) {
+                                  int startIndexForClusters, Optional<VespaModel> previousModel, String ... retiredHostNames) {
         VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(null, services, ApplicationPackageUtils.generateSearchDefinition("type1"));
         ApplicationPackage appPkg = modelCreatorWithMockPkg.appPkg;
 
@@ -148,12 +148,13 @@ public class VespaModelTester {
                 .setApplicationId(applicationId)
                 .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver);
 
-        DeployState deployState = new DeployState.Builder()
+        DeployState.Builder deployState = new DeployState.Builder()
                 .applicationPackage(appPkg)
                 .modelHostProvisioner(provisioner)
                 .properties(properties)
-                .zone(zone)
-                .build();
-        return modelCreatorWithMockPkg.create(false, deployState, configModelRegistry);
+                .zone(zone);
+        previousModel.ifPresent(deployState::previousModel);
+        return modelCreatorWithMockPkg.create(false, deployState.build(), configModelRegistry);
     }
+
 }