From 0e359755d48671231b82358565f450b29ca635b3 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 2 Dec 2021 09:16:28 +0000 Subject: [PATCH 1/8] Use Cloudflare's zlib in Docker images Closes #81208. Elasticsearch uses zlib for two purposes: * Compression of stored fields with `index.codec: best_compression`, which we use for observability and security data. * Request / response compression. Historically, zlib was packaged within the JDK, so that users wouldn't have to have zlib installed for basic usage of Java. However, the original zlib optimizes for portability and misses a number of important optimizations such as leveraging vectorization support for x86 and ARM architectures. Several forks have been created in order to address this. Since version 9, the JDK uses the system's zlib when available and falls back to the zlib that is packaged within the JDK if a system zlib cannot be found. This commit changes the Docker image to install the Cloudflare fork of zlib, and run Java using the fork instead of the original zlib, so that users of the Docker image can get better performance. Other ES distribution types are out-of-scope, since configuring the JVM to use an alternative zlib requires an environment config as well as installed another zlib, and Docker is the only distribution type where we can control both. --- distribution/docker/build.gradle | 24 ++++++++- distribution/docker/src/docker/Dockerfile | 49 ++++++++++++++----- .../src/docker/bin/docker-entrypoint.sh | 4 ++ 3 files changed, 63 insertions(+), 14 deletions(-) diff --git a/distribution/docker/build.gradle b/distribution/docker/build.gradle index 86c2b88c67aad..0bba047581af1 100644 --- a/distribution/docker/build.gradle +++ b/distribution/docker/build.gradle @@ -16,6 +16,8 @@ apply plugin: 'elasticsearch.test.fixtures' apply plugin: 'elasticsearch.internal-distribution-download' apply plugin: 'elasticsearch.rest-resources' +ext.cloudflareZlibVersion = '1.2.8' + String buildId = providers.systemProperty('build.id').forUseAtConfigurationTime().getOrNull() boolean useLocalArtifacts = buildId != null && buildId.isBlank() == false @@ -34,6 +36,15 @@ repositories { content { includeGroup 'krallin' } } + ivy { + url 'https://github.com/' + patternLayout { + artifact '/[organisation]/[module]/archive/refs/tags/v[revision].[ext]' + } + metadataSources { artifact() } + content { includeGroup 'cloudflare' } + } + // Cloud builds bundle some beats ivy { if (useLocalArtifacts) { @@ -63,6 +74,7 @@ configurations { nonRepositoryPlugins filebeat metricbeat + cloudflareZlib } String beatsArch = Architecture.current() == Architecture.AARCH64 ? 'arm64' : 'x86_64' @@ -77,6 +89,7 @@ dependencies { nonRepositoryPlugins project(path: ':plugins', configuration: 'nonRepositoryPlugins') filebeat "beats:filebeat:${VersionProperties.elasticsearch}:${beatsArch}@tar.gz" metricbeat "beats:metricbeat:${VersionProperties.elasticsearch}:${beatsArch}@tar.gz" + cloudflareZlib "cloudflare:zlib:${cloudflareZlibVersion}@tar.gz" } ext.expansions = { Architecture architecture, DockerBase base -> @@ -100,7 +113,8 @@ ext.expansions = { Architecture architecture, DockerBase base -> 'docker_base' : base.name().toLowerCase(), 'version' : VersionProperties.elasticsearch, 'major_minor_version': "${major}.${minor}", - 'retry' : ShellRetry + 'retry' : ShellRetry, + 'zlib_version' : cloudflareZlibVersion ] } @@ -259,6 +273,7 @@ void addBuildDockerContextTask(Architecture architecture, DockerBase base) { boolean includeBeats = VersionProperties.isElasticsearchSnapshot() == true || buildId != null from configurations.repositoryPlugins + if (includeBeats) { from configurations.filebeat from configurations.metricbeat @@ -289,7 +304,10 @@ void addTransformDockerContextTask(Architecture architecture, DockerBase base) { from(tarTree("${project.buildDir}/distributions/${archiveName}.tar.gz")) { eachFile { FileCopyDetails details -> if (details.name.equals("Dockerfile")) { - filter { it.replaceAll('^RUN curl.*artifacts-no-kpi.*$', "COPY ${distributionName} /tmp/elasticsearch.tar.gz") } + filter { String filename -> + String result = filename.replaceAll('^RUN curl.*artifacts-no-kpi.*$', "COPY ${distributionName} /tmp/elasticsearch.tar.gz") + return result.replaceAll('^RUN curl.*cloudflare/zlib.*$', "COPY zlib-${cloudflareZlibVersion}.tar.gz /tmp") + } } } } @@ -302,6 +320,8 @@ void addTransformDockerContextTask(Architecture architecture, DockerBase base) { from configurations.dockerSource } + from configurations.cloudflareZlib + if (base == DockerBase.IRON_BANK) { from (configurations.tini) { rename { _ -> 'tini' } diff --git a/distribution/docker/src/docker/Dockerfile b/distribution/docker/src/docker/Dockerfile index 283f85f064f30..328e571cbc454 100644 --- a/distribution/docker/src/docker/Dockerfile +++ b/distribution/docker/src/docker/Dockerfile @@ -20,28 +20,51 @@ */ %> <% if (docker_base == 'iron_bank') { %> -################################################################################ -# Build stage 0 `builder`: -# Extract Elasticsearch artifact -################################################################################ - ARG BASE_REGISTRY=registry1.dso.mil ARG BASE_IMAGE=ironbank/redhat/ubi/ubi8 ARG BASE_TAG=8.4 +<% } %> + +################################################################################ +# Build stage 0 `zlib`: +# Compile zlib for the current architecture +################################################################################ + +FROM ${base_image} AS zlib + +<% if (docker_base == 'default' || docker_base == 'cloud') { %> +RUN <%= retry.loop(package_manager, "${package_manager} update && DEBIAN_FRONTEND=noninteractive ${package_manager} install -y curl gcc make") %> +<% } else { %> +RUN <%= retry.loop(package_manager, "${package_manager} install -y curl gcc make tar gzip") %> +<% } %> + +<% +// Fetch the zlib source. Keep this command on one line - it is replaced +// with a `COPY` during local builds. +%> +WORKDIR /tmp +RUN curl --retry 10 -S -L -o zlib-${zlib_version}.tar.gz https://github.com/cloudflare/zlib/archive/refs/tags/v${zlib_version}.tar.gz +RUN echo "7e393976368975446b263ae4143fb404bc33bf3b436e72007700b5b88e5be332cd461cdec42d31a4b6dffdca2368550f01b9fa1165d81c0aa818bbf2b1ac191e zlib-${zlib_version}.tar.gz" \\ + | sha512sum --check - +RUN tar xf zlib-${zlib_version}.tar.gz + +WORKDIR /tmp/zlib-${zlib_version} +RUN ./configure --prefix=/usr/local/cloudflare-zlib && make && make install + +################################################################################ +# Build stage 1 `builder`: +# Extract Elasticsearch artifact +################################################################################ FROM ${base_image} AS builder +<% if (docker_base == 'iron_bank') { %> # `tini` is a tiny but valid init for containers. This is used to cleanly # control how ES and any child processes are shut down. COPY tini /bin/tini RUN chmod 0555 /bin/tini <% } else { %> -################################################################################ -# Build stage 0 `builder`: -# Extract Elasticsearch artifact -################################################################################ -FROM ${base_image} AS builder # Install required packages to extract the Elasticsearch distribution <% if (docker_base == 'default' || docker_base == 'cloud') { %> @@ -138,9 +161,10 @@ RUN chmod -R 0555 /opt/plugins <% } %> ################################################################################ -# Build stage 1 (the actual Elasticsearch image): +# Build stage 2 (the actual Elasticsearch image): # -# Copy elasticsearch from stage 0 +# Copy zlib from stage 2 +# Copy elasticsearch from stage 1 # Add entrypoint ################################################################################ @@ -198,6 +222,7 @@ WORKDIR /usr/share/elasticsearch COPY --from=builder --chown=0:0 /usr/share/elasticsearch /usr/share/elasticsearch COPY --from=builder --chown=0:0 /bin/tini /bin/tini +COPY --from=zlib --chown=0:0 /usr/local/cloudflare-zlib /usr/local/cloudflare-zlib <% if (docker_base == 'default' || docker_base == 'cloud') { %> COPY --from=builder --chown=0:0 /etc/ssl/certs/java/cacerts /etc/ssl/certs/java/cacerts diff --git a/distribution/docker/src/docker/bin/docker-entrypoint.sh b/distribution/docker/src/docker/bin/docker-entrypoint.sh index 51c6a641ae700..e12d55d386d38 100755 --- a/distribution/docker/src/docker/bin/docker-entrypoint.sh +++ b/distribution/docker/src/docker/bin/docker-entrypoint.sh @@ -73,6 +73,10 @@ if [[ -n "$ES_LOG_STYLE" ]]; then esac fi +if [[ -d /usr/local/cloudflare-zlib/lib ]]; then + export LD_LIBRARY_PATH=/usr/local/cloudflare-zlib/lib +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" From ec62027d474f179bbf5260e6392cd1bc7e753c14 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 2 Dec 2021 09:29:34 +0000 Subject: [PATCH 2/8] Update docs/changelog/81245.yaml --- docs/changelog/81245.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/81245.yaml diff --git a/docs/changelog/81245.yaml b/docs/changelog/81245.yaml new file mode 100644 index 0000000000000..068bfa1c1eb7a --- /dev/null +++ b/docs/changelog/81245.yaml @@ -0,0 +1,6 @@ +pr: 81245 +summary: Use Cloudflare's zlib in Docker images +area: Packaging +type: enhancement +issues: + - 81208 From 1595e6ccad110da0abed5117d771358c9a1312c1 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 2 Dec 2021 12:38:11 +0000 Subject: [PATCH 3/8] Move LD_LIBRARY_PATH change to elasticsearch script --- distribution/docker/src/docker/bin/docker-entrypoint.sh | 4 ---- distribution/src/bin/elasticsearch | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/distribution/docker/src/docker/bin/docker-entrypoint.sh b/distribution/docker/src/docker/bin/docker-entrypoint.sh index e12d55d386d38..51c6a641ae700 100755 --- a/distribution/docker/src/docker/bin/docker-entrypoint.sh +++ b/distribution/docker/src/docker/bin/docker-entrypoint.sh @@ -73,10 +73,6 @@ if [[ -n "$ES_LOG_STYLE" ]]; then esac fi -if [[ -d /usr/local/cloudflare-zlib/lib ]]; then - export LD_LIBRARY_PATH=/usr/local/cloudflare-zlib/lib -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" diff --git a/distribution/src/bin/elasticsearch b/distribution/src/bin/elasticsearch index a4bcde1a92af6..f6d91165485ac 100755 --- a/distribution/src/bin/elasticsearch +++ b/distribution/src/bin/elasticsearch @@ -52,6 +52,10 @@ if [ -z "$LIBFFI_TMPDIR" ]; then export LIBFFI_TMPDIR fi +if [ "$ES_DISTRIBUTION_TYPE" = 'docker' ] && [ -d '/usr/local/cloudflare-zlib/lib' ]; then + export LD_LIBRARY_PATH="/usr/local/cloudflare-zlib/lib:$LD_LIBRARY_PATH" +fi + # get keystore password before setting java options to avoid # conflicting GC configurations for the keystore tools unset KEYSTORE_PASSWORD From 08525d96903f9dd061f4068817c3ff02691f7377 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 2 Dec 2021 17:16:16 +0000 Subject: [PATCH 4/8] Add test case for zlib --- .../org/elasticsearch/packaging/test/DockerTests.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index 33c9ec77e7821..ecae4f5d95892 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -415,6 +415,17 @@ public void test050BasicApiTests() throws Exception { runElasticsearchTestsAsElastic(PASSWORD); } + /** + * Check that the JDK uses the Cloudflare zlib, instead of the default one. + */ + public void test060JavaUsesCloudflareZlib() throws Exception { + waitForElasticsearch(installation, "elastic", PASSWORD); + + final boolean matches = sh.run("bash -c 'pmap -p $(pidof java)'").stdout.lines().anyMatch(line -> line.contains("cloudflare-zlib")); + + assertTrue("Expect java to be using cloudflare-zlib", matches); + } + /** * Check that the default config can be overridden using a bind mount, and that env vars are respected */ From 3c6dd97a8a71154e7d90d587973c07a6fd21471d Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 2 Dec 2021 18:55:57 +0000 Subject: [PATCH 5/8] Emit pmap lines on assertion failure --- .../java/org/elasticsearch/packaging/test/DockerTests.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index ecae4f5d95892..1adfc72a75242 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -79,6 +79,7 @@ import static org.hamcrest.Matchers.emptyString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize; @@ -418,12 +419,12 @@ public void test050BasicApiTests() throws Exception { /** * Check that the JDK uses the Cloudflare zlib, instead of the default one. */ - public void test060JavaUsesCloudflareZlib() throws Exception { + public void test060JavaUsesCloudflareZlib() { waitForElasticsearch(installation, "elastic", PASSWORD); - final boolean matches = sh.run("bash -c 'pmap -p $(pidof java)'").stdout.lines().anyMatch(line -> line.contains("cloudflare-zlib")); + final List output = sh.run("bash -c 'pmap -p $(pidof java)'").stdout.lines().collect(Collectors.toList()); - assertTrue("Expect java to be using cloudflare-zlib", matches); + assertThat("Expected java to be using cloudflare-zlib", output, hasItem(containsString("cloudflare-zlib"))); } /** From 8dc0d388f436311fab4d56e1380187dc12b5a502 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 2 Dec 2021 18:57:33 +0000 Subject: [PATCH 6/8] Don't bother splitting pmap output --- .../java/org/elasticsearch/packaging/test/DockerTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index 1adfc72a75242..8bd19c3f7f814 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -422,9 +422,9 @@ public void test050BasicApiTests() throws Exception { public void test060JavaUsesCloudflareZlib() { waitForElasticsearch(installation, "elastic", PASSWORD); - final List output = sh.run("bash -c 'pmap -p $(pidof java)'").stdout.lines().collect(Collectors.toList()); + final String output = sh.run("bash -c 'pmap -p $(pidof java)'").stdout; - assertThat("Expected java to be using cloudflare-zlib", output, hasItem(containsString("cloudflare-zlib"))); + assertThat("Expected java to be using cloudflare-zlib", output, containsString("cloudflare-zlib")); } /** From 0d528414884646ac42ac79a604f5127fdd3c8131 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 2 Dec 2021 20:21:03 +0000 Subject: [PATCH 7/8] Decouple setting LD_LIBRARY_PATH from the value to set --- distribution/docker/src/docker/Dockerfile | 1 + distribution/src/bin/elasticsearch | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/distribution/docker/src/docker/Dockerfile b/distribution/docker/src/docker/Dockerfile index 328e571cbc454..fa5f818299c29 100644 --- a/distribution/docker/src/docker/Dockerfile +++ b/distribution/docker/src/docker/Dockerfile @@ -233,6 +233,7 @@ COPY --from=builder --chown=0:0 /opt /opt <% } %> ENV PATH /usr/share/elasticsearch/bin:\$PATH +ENV ES_ZLIB_PATH /usr/local/cloudflare-zlib/lib COPY ${bin_dir}/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh diff --git a/distribution/src/bin/elasticsearch b/distribution/src/bin/elasticsearch index f6d91165485ac..1ac3c56cf8795 100755 --- a/distribution/src/bin/elasticsearch +++ b/distribution/src/bin/elasticsearch @@ -52,8 +52,12 @@ if [ -z "$LIBFFI_TMPDIR" ]; then export LIBFFI_TMPDIR fi -if [ "$ES_DISTRIBUTION_TYPE" = 'docker' ] && [ -d '/usr/local/cloudflare-zlib/lib' ]; then - export LD_LIBRARY_PATH="/usr/local/cloudflare-zlib/lib:$LD_LIBRARY_PATH" +if [ -n "$ES_ZLIB_PATH" ]; then + if [ ! -d "$ES_ZLIB_PATH" ]; then + echo "zlib path specified in ES_ZLIB_PATH does not exist or is not a directory: $ES_ZLIB_PATH" >&2 + exit 1 + fi + export LD_LIBRARY_PATH="$ES_ZLIB_PATH:$LD_LIBRARY_PATH" fi # get keystore password before setting java options to avoid From 3c7a03054b3bb795bd2892ed35b3a6a9b520e13a Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Thu, 2 Dec 2021 20:27:52 +0000 Subject: [PATCH 8/8] Imports --- .../test/java/org/elasticsearch/packaging/test/DockerTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index 8bd19c3f7f814..46fb67c70fc62 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -79,7 +79,6 @@ import static org.hamcrest.Matchers.emptyString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; -import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize;