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

Make joining server an observer in initial config

2 files changed, 71 insertions, 56 deletions

diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java
index e535f627f00..5f8ebf29ccd 100644
--- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java
+++ b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java
@@ -147,8 +147,17 @@ public class Configurator {
           .append(":")
           .append(server.quorumPort())
           .append(":")
-          .append(server.electionPort())
-          .append(";")
+          .append(server.electionPort());
+        if (server.joining()) {
+            // Servers that are joining an existing cluster must be marked as observers. Note that this will NOT
+            // actually make the server an observer, but prevent it from forming an ensemble independently of the
+            // existing cluster.
+            //
+            // See https://zookeeper.apache.org/doc/r3.6.2/zookeeperReconfig.html#sc_reconfig_modifying
+            sb.append(":")
+              .append("observer");
+        }
+        sb.append(";")
           .append(clientPort)
           .append("\n");
     }
diff --git a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java
index 0cdbd68421c..0f43fb45d9d 100644
--- a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java
+++ b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java
@@ -2,11 +2,11 @@
 package com.yahoo.vespa.zookeeper;
 
 import com.yahoo.cloud.config.ZookeeperServerConfig;
-import com.yahoo.io.IOUtils;
 import com.yahoo.security.KeyUtils;
 import com.yahoo.security.X509CertificateBuilder;
 import com.yahoo.security.X509CertificateUtils;
 import com.yahoo.security.tls.TransportSecurityOptions;
+import com.yahoo.yolean.Exceptions;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -15,6 +15,7 @@ import org.junit.rules.TemporaryFolder;
 import javax.security.auth.x500.X500Principal;
 import java.io.File;
 import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.math.BigInteger;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -22,14 +23,14 @@ import java.security.KeyPair;
 import java.security.cert.X509Certificate;
 import java.util.Optional;
 
-import static com.yahoo.cloud.config.ZookeeperServerConfig.TlsForQuorumCommunication;
 import static com.yahoo.cloud.config.ZookeeperServerConfig.TlsForClientServerCommunication;
+import static com.yahoo.cloud.config.ZookeeperServerConfig.TlsForQuorumCommunication;
 import static com.yahoo.security.KeyAlgorithm.EC;
 import static com.yahoo.security.SignatureAlgorithm.SHA256_WITH_ECDSA;
+import static com.yahoo.vespa.defaults.Defaults.getDefaults;
 import static com.yahoo.vespa.zookeeper.Configurator.ZOOKEEPER_JUTE_MAX_BUFFER;
 import static java.time.Instant.EPOCH;
 import static java.time.temporal.ChronoUnit.DAYS;
-import static com.yahoo.vespa.defaults.Defaults.getDefaults;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
@@ -53,7 +54,7 @@ public class ConfiguratorTest {
     }
 
     @Test
-    public void config_is_written_correctly_when_one_server() throws IOException {
+    public void config_is_written_correctly_when_one_server() {
         ZookeeperServerConfig.Builder builder = createConfigBuilderForSingleHost(cfgFile, idFile, jksKeyStoreFile);
         new Configurator(builder.build()).writeConfigToDisk(Optional.empty());
         validateConfigFileSingleHost(cfgFile);
@@ -61,12 +62,12 @@ public class ConfiguratorTest {
     }
 
     @Test
-    public void config_is_written_correctly_when_multiple_servers() throws IOException {
+    public void config_is_written_correctly_when_multiple_servers() {
         ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder();
         builder.zooKeeperConfigFile(cfgFile.getAbsolutePath());
-        builder.server(newServer(0, "foo", 123, 321));
-        builder.server(newServer(1, "bar", 234, 432));
-        builder.server(newServer(2, "baz", 345, 543));
+        builder.server(newServer(0, "foo", 123, 321, false));
+        builder.server(newServer(1, "bar", 234, 432, false));
+        builder.server(newServer(2, "baz", 345, 543, true));
         builder.myidFile(idFile.getAbsolutePath());
         builder.myid(1);
         new Configurator(builder.build()).writeConfigToDisk(Optional.empty());
@@ -75,7 +76,7 @@ public class ConfiguratorTest {
     }
 
     @Test
-    public void config_is_written_correctly_with_tls_for_quorum_communication_port_unification() throws IOException {
+    public void config_is_written_correctly_with_tls_for_quorum_communication_port_unification() {
         ZookeeperServerConfig.Builder builder = createConfigBuilderForSingleHost(cfgFile, idFile, jksKeyStoreFile);
         builder.tlsForQuorumCommunication(TlsForQuorumCommunication.PORT_UNIFICATION);
         builder.tlsForClientServerCommunication(TlsForClientServerCommunication.PORT_UNIFICATION);
@@ -86,7 +87,7 @@ public class ConfiguratorTest {
     }
 
     @Test
-    public void config_is_written_correctly_with_tls_for_quorum_communication_tls_with_port_unification() throws IOException {
+    public void config_is_written_correctly_with_tls_for_quorum_communication_tls_with_port_unification() {
         ZookeeperServerConfig.Builder builder = createConfigBuilderForSingleHost(cfgFile, idFile, jksKeyStoreFile);
         builder.tlsForQuorumCommunication(TlsForQuorumCommunication.TLS_WITH_PORT_UNIFICATION);
         builder.tlsForClientServerCommunication(TlsForClientServerCommunication.TLS_WITH_PORT_UNIFICATION);
@@ -97,7 +98,7 @@ public class ConfiguratorTest {
     }
 
     @Test
-    public void config_is_written_correctly_with_tls_for_quorum_communication_tls_only() throws IOException {
+    public void config_is_written_correctly_with_tls_for_quorum_communication_tls_only() {
         ZookeeperServerConfig.Builder builder = createConfigBuilderForSingleHost(cfgFile, idFile, jksKeyStoreFile);
         builder.tlsForQuorumCommunication(TlsForQuorumCommunication.TLS_ONLY);
         builder.tlsForClientServerCommunication(TlsForClientServerCommunication.TLS_ONLY);
@@ -107,28 +108,18 @@ public class ConfiguratorTest {
         validateThatJksKeyStoreFileExists(jksKeyStoreFile);
     }
 
-    private ZookeeperServerConfig.Builder createConfigBuilderForSingleHost(File cfgFile, File idFile, File jksKeyStoreFile) {
-        ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder();
-        builder.zooKeeperConfigFile(cfgFile.getAbsolutePath());
-        builder.myidFile(idFile.getAbsolutePath());
-        builder.server(newServer(0, "foo", 123, 321));
-        builder.myid(0);
-        builder.jksKeyStoreFile(jksKeyStoreFile.getAbsolutePath());
-        return builder;
-    }
-
     @Test(expected = RuntimeException.class)
     public void require_that_this_id_must_be_present_amongst_servers() {
         ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder();
         builder.zooKeeperConfigFile(cfgFile.getAbsolutePath());
-        builder.server(newServer(1, "bar", 234, 432));
-        builder.server(newServer(2, "baz", 345, 543));
+        builder.server(newServer(1, "bar", 234, 432, false));
+        builder.server(newServer(2, "baz", 345, 543, false));
         builder.myid(0);
         new Configurator(builder.build()).writeConfigToDisk(Optional.empty());
     }
 
     @Test
-    public void juteMaxBufferCanBeSet() throws IOException {
+    public void jute_max_buffer_can_be_set() throws IOException {
         ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder();
         builder.myid(0);
         File idFile = folder.newFile();
@@ -147,17 +138,28 @@ public class ConfiguratorTest {
         assertEquals("" + max_buffer, System.getProperty(ZOOKEEPER_JUTE_MAX_BUFFER));
     }
 
-    private ZookeeperServerConfig.Server.Builder newServer(int id, String hostName, int electionPort, int quorumPort) {
+    private ZookeeperServerConfig.Builder createConfigBuilderForSingleHost(File cfgFile, File idFile, File jksKeyStoreFile) {
+        ZookeeperServerConfig.Builder builder = new ZookeeperServerConfig.Builder();
+        builder.zooKeeperConfigFile(cfgFile.getAbsolutePath());
+        builder.myidFile(idFile.getAbsolutePath());
+        builder.server(newServer(0, "foo", 123, 321, false));
+        builder.myid(0);
+        builder.jksKeyStoreFile(jksKeyStoreFile.getAbsolutePath());
+        return builder;
+    }
+
+    private ZookeeperServerConfig.Server.Builder newServer(int id, String hostName, int electionPort, int quorumPort, boolean joining) {
         ZookeeperServerConfig.Server.Builder builder = new ZookeeperServerConfig.Server.Builder();
         builder.id(id);
         builder.hostname(hostName);
         builder.electionPort(electionPort);
         builder.quorumPort(quorumPort);
+        builder.joining(joining);
         return builder;
     }
 
-    private void validateIdFile(File idFile, String expected) throws IOException {
-        String actual = IOUtils.readFile(idFile);
+    private void validateIdFile(File idFile, String expected) {
+        String actual = Exceptions.uncheck(() -> Files.readString(idFile.toPath()));
         assertEquals(expected, actual);
     }
 
@@ -201,7 +203,7 @@ public class ConfiguratorTest {
         return sb.toString();
     }
 
-    private void validateConfigFileSingleHost(File cfgFile) throws IOException {
+    private void validateConfigFileSingleHost(File cfgFile) {
         String expected =
                 commonConfig() +
                 "server.0=foo:321:123;2181\n" +
@@ -229,12 +231,12 @@ public class ConfiguratorTest {
                "ssl.protocol=TLS\n";
     }
 
-    private void validateConfigFileMultipleHosts(File cfgFile) throws IOException {
+    private void validateConfigFileMultipleHosts(File cfgFile) {
         String expected =
                 commonConfig() +
                 "server.0=foo:321:123;2181\n" +
                 "server.1=bar:432:234;2181\n" +
-                "server.2=baz:543:345;2181\n" +
+                "server.2=baz:543:345:observer;2181\n" +
                 commonTlsQuorumConfig() +
                 "sslQuorum=false\n" +
                 "portUnification=false\n" +
@@ -243,7 +245,7 @@ public class ConfiguratorTest {
         validateConfigFile(cfgFile, expected);
     }
 
-    private void validateConfigFilePortUnification(File cfgFile, File jksKeyStoreFile, File caCertificatesFile) throws IOException {
+    private void validateConfigFilePortUnification(File cfgFile, File jksKeyStoreFile, File caCertificatesFile) {
         String expected =
                 commonConfig() +
                 "server.0=foo:321:123;2181\n" +
@@ -257,7 +259,7 @@ public class ConfiguratorTest {
         validateConfigFile(cfgFile, expected);
     }
 
-    private void validateConfigFileTlsWithPortUnification(File cfgFile, File jksKeyStoreFile, File caCertificatesFile) throws IOException {
+    private void validateConfigFileTlsWithPortUnification(File cfgFile, File jksKeyStoreFile, File caCertificatesFile) {
         String expected =
                 commonConfig() +
                 "server.0=foo:321:123;2181\n" +
@@ -271,7 +273,7 @@ public class ConfiguratorTest {
         validateConfigFile(cfgFile, expected);
     }
 
-    private void validateConfigFileTlsOnly(File cfgFile, File jksKeyStoreFile, File caCertificatesFile) throws IOException {
+    private void validateConfigFileTlsOnly(File cfgFile, File jksKeyStoreFile, File caCertificatesFile) {
         String expected =
                 commonConfig() +
                 "server.0=foo:321:123;2181\n" +
@@ -285,8 +287,8 @@ public class ConfiguratorTest {
         validateConfigFile(cfgFile, expected);
     }
 
-    private void validateConfigFile(File cfgFile, String expected) throws IOException {
-        String actual = IOUtils.readFile(cfgFile);
+    private void validateConfigFile(File cfgFile, String expected) {
+        String actual = Exceptions.uncheck(() -> Files.readString(cfgFile.toPath()));
         assertEquals(expected, actual);
     }
 
@@ -294,25 +296,29 @@ public class ConfiguratorTest {
         assertTrue(cfgFile.exists() && cfgFile.canRead());
     }
 
-    private Optional<TransportSecurityOptions> createTransportSecurityOptions() throws IOException {
-        KeyPair keyPair = KeyUtils.generateKeypair(EC);
-        Path privateKeyFile = folder.newFile().toPath();
-        Files.writeString(privateKeyFile, KeyUtils.toPem(keyPair.getPrivate()));
-
-        X509Certificate certificate = X509CertificateBuilder
-                .fromKeypair(keyPair, new X500Principal("CN=dummy"), EPOCH, EPOCH.plus(1, DAYS), SHA256_WITH_ECDSA, BigInteger.ONE)
-                .build();
-        Path certificateChainFile = folder.newFile().toPath();
-        String certificatePem = X509CertificateUtils.toPem(certificate);
-        Files.writeString(certificateChainFile, certificatePem);
-
-        Path caCertificatesFile = folder.newFile().toPath();
-        Files.writeString(caCertificatesFile, certificatePem);
-
-        return Optional.of(new TransportSecurityOptions.Builder()
-                                   .withCertificates(certificateChainFile, privateKeyFile)
-                                   .withCaCertificates(caCertificatesFile)
-                                   .build());
+    private Optional<TransportSecurityOptions> createTransportSecurityOptions() {
+        try {
+            KeyPair keyPair = KeyUtils.generateKeypair(EC);
+            Path privateKeyFile = folder.newFile().toPath();
+            Files.writeString(privateKeyFile, KeyUtils.toPem(keyPair.getPrivate()));
+
+            X509Certificate certificate = X509CertificateBuilder
+                    .fromKeypair(keyPair, new X500Principal("CN=dummy"), EPOCH, EPOCH.plus(1, DAYS), SHA256_WITH_ECDSA, BigInteger.ONE)
+                    .build();
+            Path certificateChainFile = folder.newFile().toPath();
+            String certificatePem = X509CertificateUtils.toPem(certificate);
+            Files.writeString(certificateChainFile, certificatePem);
+
+            Path caCertificatesFile = folder.newFile().toPath();
+            Files.writeString(caCertificatesFile, certificatePem);
+
+            return Optional.of(new TransportSecurityOptions.Builder()
+                                       .withCertificates(certificateChainFile, privateKeyFile)
+                                       .withCaCertificates(caCertificatesFile)
+                                       .build());
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
     }
 
 }