Browse Source

Docker fix additional nodes (#81082)

Allow users to pass an ENROLLMENT_TOKEN environment variable that
would be passed as the `--enrollment-token` parameter to the
elasticsearch executable running in the container. This allows an
elasticsearch node running in docker to enroll itself to an
existing secured cluster.

Resolves: #81068
Ioannis Kakavas 3 years ago
parent
commit
54eb955a65

+ 7 - 1
distribution/docker/src/docker/bin/docker-entrypoint.sh

@@ -73,6 +73,12 @@ if [[ -n "$ES_LOG_STYLE" ]]; then
   esac
 fi
 
+if [[ -n "$ENROLLMENT_TOKEN" ]]; then
+  POSITIONAL_PARAMETERS="--enrollment-token $ENROLLMENT_TOKEN"
+else
+  POSITIONAL_PARAMETERS=""
+fi
+
 # Signal forwarding and child reaping is handled by `tini`, which is the
 # actual entrypoint of the container
-exec /usr/share/elasticsearch/bin/elasticsearch <<<"$KEYSTORE_PASSWORD"
+exec /usr/share/elasticsearch/bin/elasticsearch $POSITIONAL_PARAMETERS <<<"$KEYSTORE_PASSWORD"

+ 5 - 0
docs/changelog/81082.yaml

@@ -0,0 +1,5 @@
+pr: 81082
+summary: Add Docker support for enrollment tokens
+area: "Packaging"
+type: bug
+issues: []

+ 17 - 2
qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java

@@ -147,7 +147,9 @@ public class DockerTests extends PackagingTestCase {
      */
     public void test012SecurityCanBeDisabled() throws Exception {
         // restart container with security disabled
-        runContainer(distribution(), builder().envVar("xpack.security.enabled", "false"));
+        // We need to set discovery to single-node as with security disabled, autoconfiguration won't run and we won't set
+        // cluster.initial_master_nodes
+        runContainer(distribution(), builder().envVar("xpack.security.enabled", "false").envVar("discovery.type", "single-node"));
         waitForElasticsearch(installation);
         final int unauthStatusCode = ServerUtils.makeRequestAndGetStatus(Request.Get("http://localhost:9200"), null, null, null);
         assertThat(unauthStatusCode, equalTo(200));
@@ -452,11 +454,15 @@ public class DockerTests extends PackagingTestCase {
         Files.setPosixFilePermissions(tempDir.resolve(autoConfigurationDirName), p750);
 
         // Restart the container
+        // We need to set discovery to single-node as autoconfiguration has already run when the node started the first time
+        // cluster.initial_master_nodes is set to the name of the original docker container
+        ServerUtils.removeSettingFromExistingConfiguration(tempDir, "cluster.initial_master_nodes");
         runContainer(
             distribution(),
             builder().volume(tempDir, "/usr/share/elasticsearch/config")
                 .envVar("ES_JAVA_OPTS", "-XX:-UseCompressedOops")
                 .envVar("ELASTIC_PASSWORD", PASSWORD)
+                .envVar("discovery.type", "single-node")
         );
 
         waitForElasticsearch(installation, "elastic", PASSWORD);
@@ -536,6 +542,9 @@ public class DockerTests extends PackagingTestCase {
 
         try {
             // Restart the container
+            // We need to set discovery to single-node as autoconfiguration has already run when the node started the first time
+            // cluster.initial_master_nodes is set to the name of the original docker container
+            ServerUtils.removeSettingFromExistingConfiguration(tempEsConfigDir, "cluster.initial_master_nodes");
             runContainer(
                 distribution(),
                 builder().envVar("ELASTIC_PASSWORD", PASSWORD)
@@ -543,6 +552,7 @@ public class DockerTests extends PackagingTestCase {
                     .volume(tempEsDataDir.toAbsolutePath(), installation.data)
                     .volume(tempEsConfigDir.toAbsolutePath(), installation.config)
                     .volume(tempEsLogsDir.toAbsolutePath(), installation.logs)
+                    .envVar("discovery.type", "single-node")
             );
 
             waitForElasticsearch(installation, "elastic", PASSWORD);
@@ -560,7 +570,12 @@ public class DockerTests extends PackagingTestCase {
      */
     public void test073RunEsAsDifferentUserAndGroupWithoutBindMounting() {
         // Restart the container
-        runContainer(distribution(), builder().extraArgs("--group-add 0").uid(501, 501).envVar("ELASTIC_PASSWORD", PASSWORD));
+        // We need to set discovery to single-node as autoconfiguration won't run, and we won't set
+        // cluster.initial_master_nodes
+        runContainer(
+            distribution(),
+            builder().extraArgs("--group-add 0").uid(501, 501).envVar("ELASTIC_PASSWORD", PASSWORD).envVar("discovery.type", "single-node")
+        );
 
         waitForElasticsearch(installation, "elastic", PASSWORD);
     }

+ 53 - 7
qa/os/src/test/java/org/elasticsearch/packaging/test/EnrollmentProcessTests.java

@@ -12,30 +12,37 @@ import org.elasticsearch.common.Strings;
 import org.elasticsearch.packaging.util.Archives;
 import org.elasticsearch.packaging.util.Distribution;
 import org.elasticsearch.packaging.util.Shell;
-import org.junit.BeforeClass;
+import org.elasticsearch.packaging.util.docker.Docker;
 
 import java.io.IOException;
 import java.net.InetAddress;
 import java.net.Socket;
 import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 
 import static org.elasticsearch.packaging.util.Archives.installArchive;
 import static org.elasticsearch.packaging.util.Archives.verifyArchiveInstallation;
 import static org.elasticsearch.packaging.util.FileUtils.getCurrentVersion;
+import static org.elasticsearch.packaging.util.docker.Docker.removeContainer;
+import static org.elasticsearch.packaging.util.docker.Docker.runAdditionalContainer;
+import static org.elasticsearch.packaging.util.docker.Docker.runContainer;
+import static org.elasticsearch.packaging.util.docker.Docker.verifyContainerInstallation;
+import static org.elasticsearch.packaging.util.docker.Docker.waitForElasticsearch;
+import static org.elasticsearch.packaging.util.docker.DockerRun.builder;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.emptyOrNullString;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.junit.Assume.assumeTrue;
 
 public class EnrollmentProcessTests extends PackagingTestCase {
 
-    @BeforeClass
-    public static void filterDistros() {
-        assumeTrue("only archives", distribution.isArchive());
-    }
-
-    public void test10AutoFormCluster() throws Exception {
+    public void test10ArchiveAutoFormCluster() throws Exception {
         /* Windows issue awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */
         assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS);
+        assumeTrue("only archives", distribution.isArchive());
         installation = installArchive(sh, distribution(), getRootTempDir().resolve("elasticsearch-node1"), getCurrentVersion(), true);
         verifyArchiveInstallation(installation, distribution());
         setFileSuperuser("test_superuser", "test_superuser_password");
@@ -81,6 +88,45 @@ public class EnrollmentProcessTests extends PackagingTestCase {
         assertThat(makeRequest("https://localhost:9200/_cluster/health"), containsString("\"number_of_nodes\":2"));
     }
 
+    public void test20DockerAutoFormCluster() throws Exception {
+        assumeTrue("only docker", distribution.isDocker());
+        // First node
+        installation = runContainer(distribution(), builder().envVar("ELASTIC_PASSWORD", "password"));
+        verifyContainerInstallation(installation);
+        verifySecurityAutoConfigured(installation);
+        waitForElasticsearch(installation);
+        final String node1ContainerId = Docker.getContainerId();
+        String createTokenResult = installation.executables().createEnrollmentToken.run("-s node").stdout;
+        assertThat(createTokenResult, not(emptyOrNullString()));
+        final List<String> noWarningOutputLines = createTokenResult.lines()
+            .filter(line -> line.startsWith("WARNING:") == false)
+            .collect(Collectors.toList());
+        assertThat(noWarningOutputLines.size(), equalTo(1));
+        // installation refers to second node from now on
+        installation = runAdditionalContainer(
+            distribution(),
+            builder().envVar("ENROLLMENT_TOKEN", noWarningOutputLines.get(0)).extraArgs("--publish 9301:9300 --publish 9201:9200")
+        );
+        // TODO Make our packaging test methods aware of multiple installations, see https://github.com/elastic/elasticsearch/issues/79688
+        waitForElasticsearch(installation);
+        verifyContainerInstallation(installation);
+        verifySecurityAutoConfigured(installation);
+        // Allow some time for the second node to join the cluster, we can probably do this more elegantly in
+        // https://github.com/elastic/elasticsearch/issues/79688
+        // Then verify that the two nodes formed a cluster
+        assertBusy(
+            () -> assertThat(
+                makeRequestAsElastic("https://localhost:9200/_cluster/health", "password"),
+                containsString("\"number_of_nodes\":2")
+            ),
+            20,
+            TimeUnit.SECONDS
+        );
+
+        // Cleanup the first node that is still running
+        removeContainer(node1ContainerId);
+    }
+
     private void waitForSecondNode() {
         int retries = 60;
         while (retries > 0) {

+ 46 - 21
qa/os/src/test/java/org/elasticsearch/packaging/util/docker/Docker.java

@@ -120,6 +120,23 @@ public class Docker {
         return Installation.ofContainer(dockerShell, distribution);
     }
 
+    /**
+     * Runs an Elasticsearch Docker container without removing any existing containers first,
+     * and checks that it has started up successfully.
+     *
+     * @param distribution details about the docker image being tested
+     * @param builder the command to run
+     * @return an installation that models the running container
+     */
+    public static Installation runAdditionalContainer(Distribution distribution, DockerRun builder) {
+        // TODO Maybe revisit this as part of https://github.com/elastic/elasticsearch/issues/79688
+        final String command = builder.distribution(distribution).build();
+        logger.info("Running command: " + command);
+        containerId = sh.run(command).stdout.trim();
+        waitForElasticsearchToStart();
+        return Installation.ofContainer(dockerShell, distribution);
+    }
+
     /**
      * Similar to {@link #runContainer(Distribution, DockerRun)} in that it runs an Elasticsearch Docker
      * container, expect that the container expecting it to exit e.g. due to configuration problem.
@@ -213,31 +230,39 @@ public class Docker {
     }
 
     /**
-     * Removes the currently running container.
+     * Removes the container with a given id
      */
-    public static void removeContainer() {
+    public static void removeContainer(String containerId) {
         if (containerId != null) {
-            try {
-                // Remove the container, forcibly killing it if necessary
-                logger.debug("Removing container " + containerId);
-                final String command = "docker rm -f " + containerId;
-                final Shell.Result result = sh.runIgnoreExitCode(command);
-
-                if (result.isSuccess() == false) {
-                    boolean isErrorAcceptable = result.stderr.contains("removal of container " + containerId + " is already in progress")
-                        || result.stderr.contains("Error: No such container: " + containerId);
-
-                    // I'm not sure why we're already removing this container, but that's OK.
-                    if (isErrorAcceptable == false) {
-                        throw new RuntimeException("Command was not successful: [" + command + "] result: " + result);
-                    }
+            // Remove the container, forcibly killing it if necessary
+            logger.debug("Removing container " + containerId);
+            final String command = "docker rm -f " + containerId;
+            final Shell.Result result = sh.runIgnoreExitCode(command);
+
+            if (result.isSuccess() == false) {
+                boolean isErrorAcceptable = result.stderr.contains("removal of container " + containerId + " is already in progress")
+                    || result.stderr.contains("Error: No such container: " + containerId);
+
+                // I'm not sure why we're already removing this container, but that's OK.
+                if (isErrorAcceptable == false) {
+                    throw new RuntimeException("Command was not successful: [" + command + "] result: " + result);
                 }
-            } finally {
-                // Null out the containerId under all circumstances, so that even if the remove command fails
-                // for some reason, the other tests will still proceed. Otherwise they can get stuck, continually
-                // trying to remove a non-existent container ID.
-                containerId = null;
             }
+
+        }
+    }
+
+    /**
+     * Removes the currently running container.
+     */
+    public static void removeContainer() {
+        try {
+            removeContainer(containerId);
+        } finally {
+            // Null out the containerId under all circumstances, so that even if the remove command fails
+            // for some reason, the other tests will still proceed. Otherwise they can get stuck, continually
+            // trying to remove a non-existent container ID.
+            containerId = null;
         }
     }
 

+ 5 - 5
qa/os/src/test/java/org/elasticsearch/packaging/util/docker/DockerRun.java

@@ -108,12 +108,12 @@ public class DockerRun {
 
         this.envVars.forEach((key, value) -> cmd.add("--env " + key + "=\"" + value + "\""));
 
-        // The container won't run without configuring discovery
-        cmd.add("--env discovery.type=single-node");
-
         // Map ports in the container to the host, so that we can send requests
-        cmd.add("--publish 9200:9200");
-        cmd.add("--publish 9300:9300");
+        // allow ports to be overridden by tests
+        if (this.extraArgs.stream().anyMatch(arg -> arg.startsWith("-p") || arg.startsWith("--publish")) == false) {
+            cmd.add("--publish 9200:9200");
+            cmd.add("--publish 9300:9300");
+        }
 
         // Bind-mount any volumes
         volumes.forEach((localPath, containerPath) -> {

+ 0 - 1
x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/AutoConfigureNode.java

@@ -294,7 +294,6 @@ public class AutoConfigureNode extends EnvironmentAwareCommand {
                         () -> null,
                         CommandLineHttpClient::responseBuilder
                     );
-                    break;
                 } catch (Exception e) {
                     terminal.errorPrint(
                         Terminal.Verbosity.NORMAL,